Server signals ephemeral port when ready

[robs-zeynet]

Creating client/server unit tests has two classical problems:

1. How do you find a port that's unused that the server can bind but also communicate it to the client. Typically hard coding one in a test means the test will fail anytime that port is bound (1338 in current code)
2. How do you start the server and the client as fast as possible (think 100s of tests running) without using a sleep() which either is too small (causing race conditions) or too large (causing testing delays)

This patch proposes a way for the client to register a signal and the server to send the ephemeral port back on that signal when it's ready, solving both problems.

I created an example unit test to show how this could be used but I can fix up the rest and add the right documentation if folks agree with the general direction.

Feedback welcome.

Motivation

Solution

[davidpdrsn]

The recommend way of doing this is with Router::serve_with_incoming:

#[tokio::test]
async fn some_test() {
    let service = GreeterServer::new(MyGreeter);

    // create the listener up front so the server is immediately ready
    // bind to port `0` so the OS finds a free port
    let listener = tokio::net::TcpListener::bind("0.0.0.0:0").await.unwrap();
    let addr = listener.local_addr().unwrap();

    tokio::spawn(async move {
        tonic::transport::Server::builder()
            .add_service(service)
            .serve_with_incoming(tokio_stream::wrappers::TcpListenerStream::new(listener))
            .await
            .unwrap();
    });

    // make the channel bound to `addr`
}

Or mock all the IO if thats more your thing. See https://github.com/hyperium/tonic/blob/master/examples/src/mock/mock.rs.

So not sure if we need something like this.

[robs-zeynet]

Thanks for the reply!

I had looked at this, but it wasn't clear to me how to get from a vanilla TcpListener to something that serve_with_listener() would accept. It looks like tokio_stream::wrappers::TcpListenerStream() is the magic for that - thanks.

That said, all of the unit tests that I saw have both the errors that I'm trying to fix, e.g., they use fixed ports and they use a small sleep (which is either a race condition or a performance issue). I'm happy to junk this patch and rewrite those (if only for better reference) if folks think it would be useful.

If nothing else, I can provide some example code to do this (e.g., in tonic/tonic/examples) so the more people don't lose time on it. Thoughts?

Thanks again.

[davidpdrsn]

That said, all of the unit tests that I saw have both the errors that I'm trying to fix, e.g., they use fixed ports and they use a small sleep (which is either a race condition or a performance issue). I'm happy to junk this patch and rewrite those (if only for better reference) if folks think it would be useful.

I think we should just update the existing tests. You're very welcome to do that 🙏 I already it in the compression tests.