grpctest: new package

[dsymonds]

This package makes some of the setup and teardown simpler and more
reliable for test of code that interacts with gRPC servers. It is
non-trivial to write this code in a way that is portable (across OS
types, across supported network interface types, etc.).

This change also demonstrates grpctest's use in the roundrobin test.

[thelinuxfoundation]

Thank you for your pull request. Before we can look at your contribution, we need to ensure all contributors are covered by a Contributor License Agreement.

After the following items are addressed, please respond with a new comment here, and the automated system will re-verify.

• User @dsymonds isn't covered by a CLA. They will need to complete the form at https://identity.linuxfoundation.org/projects/cncf

Regards,
CLA GitHub bot

[dsymonds]

I don't know why that CLA check is firing. I should be covered by the Google corporate CLA.

[dfawley]

This uses and encourages an anti-pattern for Go testing, which is passing testing.Ts around (which is particularly discouraged when crossing package boundaries and storing in structs AIUI).

Further, I'm not convinced this package is worth the extra API surface it introduces. It is easy to start and stop a server/client with grpc. The main complexity is the Listen-on-localhost problem (which really should be solved in the net package itself, but that's irrelevant here). In the not-too-distant future (~1Q or so), we should be introducing an in-process transport that will be even easier to serve/dial.

[dsymonds]

It's fine to pass around testing.T in limited circumstances. That's why t.Helper exists, after all. It solves the problems that passing around a testing.T produces.

It may be easy to start and stop a server for people like us who know what's going on, but lots of people aren't so comfortable. I see a lot of people getting it wrong in various ways (e.g. binding to :0 instead of a loopback address, or starting the server before registering service handlers). It's not a lot of code, but it cuts down the error rate a lot.

[dfawley]

I see a lot of people getting it wrong in various ways (e.g. binding to :0 instead of a loopback address

This is a problem with Go, though, not grpc; binding to :0 is/should be a loopback address. net.Listen should work with it. This is presumably golang/go#22826.

or starting the server before registering service handlers

This approach wouldn't help solve that problem, though.

It's not a lot of code, but it cuts down the error rate a lot.

Let's wait until we have a real in-process transport, and write a doc on "how to start test servers & clients". It should be pretty short right now and even shorter then - not significant enough to justify a new library to maintain IMO.

[dsymonds]

You can say that binding to :0 is a Go problem not a grpc problem (though I don't know how you'd then listen on all interfaces), but from the perspective of a user of grpc-go that is a distinction without a difference. The fact is that people get it wrong, and the point of having common libraries is to make it harder to get things like that wrong.

An in-process transport seems like a fine direction (though then you miss out on testing with the OS's network code, so your test becomes less representative), but note that my proposed code here actually makes it easier to switch over to that and people don't have to specifically use some in-process transport in their tests but rather grpc-go will handle it for them.

I don't think there's much to maintain here; it's very similar to the Google-internal rpctest package that I wrote, and it had only about 10 notable changes in 6 years. But if you really don't want this package, that's fine, it's your call; I'll leave it in my own project and then nobody else can benefit from it.

[dfawley]

Let's revisit this after the in-process transport functionality is in place. I think once we have that, the process for starting/connecting to a service in tests should be just as simple as this package provides.