Don't bind to fixed ports in tests #126
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously we used fixed ports for tests. If someone else happened to be using those ports, then the test failed. This commit has test listening on port 0, which causes the OS to allocate an unused port. In order that we can connect, we need snare to tell us what port it's bound to, a feature which is gated behind an `_internal_testing` feature. It's quite hard to integrate the old `bad_config` test into this new framework (my attempt ended up with some surprising type inference errors). Since it's testing something that's well captured elsewhere (the unittests in `Config`), I've removed it rather than make the test runner even more complex than it now is.
Since we no longer have any tests which don't cause snare to exit successfully, we don't need to carry this around as a parameter. This slightly simplifies the code.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is overtly about fixing a problem in the new tests, which is that they used fixed port numbers (06a7e8f). Fixing that made it obvious to me where the commonality in tests was, so this PR also cuts down a lot of the boilerplate.