-
Notifications
You must be signed in to change notification settings - Fork 507
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
api: fix intermittent test failures #6461
api: fix intermittent test failures #6461
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - though one comment against the documentation.
for i, test := range openWithSNIHostnameTests { | ||
c.Logf("test %d: %v", i, test.about) | ||
s.testSNIHostName(c, test.info, test.expectDial) | ||
} | ||
} | ||
|
||
// testSNIHostName tests that when the API is dialed with the given info, | ||
// api.newWebsocketDialer is called with the expected information | ||
// (one element for each call to newWebsocketDialer) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the 'one element for each...' still hold true?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, thanks. Fixed.
fc140bd
to
ba5e377
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
The test code assumed that we'd get dial requests in the
order they are specified, but in a heavily loaded test server
this is not necessarily the case, leading to failures like this one:
http://reports.vapour.ws/releases/4490/job/run-unit-tests-xenial-amd64/attempt/1427#highlight
This fixes the test so that we don't rely on the sequential try ordering - the
concurrent retry mechanism isn't something we need to test in this test,
so just connect to a single address in each test.