-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add integration tests #36
Comments
The emulator only listening on tcp ports, and bigtable-rs only being able to connect to tcp ports made it a bit annoying to write integration tests in my application, because we had to find a free port. I added support to connect to unix domain sockets in #72, to match the golang client sdk behaviour. I also PR'ed teaching the emulator how to bind on a unix domain socket directly in googleapis/google-cloud-go#9665 - though things like ip2unix could be workarounds. This should allow having the emulator bind on a unix socket in a temporary directory, that's unique for each test run. |
We've been using #72 for a while now. I'm happy to contribute some simulator plumbing to here (ideally in a way so I can also just reuse it from my crates ;-) ). What do you think about having some helper struct / glue code, potentially behind a feature flag, that gives you a In addition to the connection parameters for If we don't want to run our own patch on top of |
@flokli Hi there, I haven't put too much thought into the integration tests but what I would like is that we do not patch more for The integration test code can be just inside of a standalone package/crate, and using some fixed or some emulator default port for communication is fine as those tests usually just run in CI environment. |
Ok, no worries! I think we'll just keep the code on our end then. I'll watch this issue though, in case any of this changes. |
The main bigtable result parsing package has some good test coverage, but not much for the helper/util code yet.
Adding some simple integration tests will make the overall test coverage look good.
The text was updated successfully, but these errors were encountered: