Several tweaks to the testing clock. #3
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
1) Actually wire up the tests for the testing clock. In moving them out of 'util' we missed having the TestAll function. 2) Fix a test that checked the logs, but didn't tell the logging infrastructure that it needed to read all Debug level log messages. 3) Change clock.WaitAdvance to not break the time into 10 even slices. This is because *many* tests use WaitAdvance(..., LongWait), which means that if they aren't ready immediately, then they way 1s before checking again, which is way too long. Instead we cap out at 10ms, so we don't spin waiting for the right number of waiters, but we also don't wait too long. We still stop at the same time, and we still stop as soon as we have the expected number of waiters. 4) Track the stack traces of who is waiting for the clock. This makes it possible to know *who* has actually called clock.After() and co, so you can figure out why there are a particular number of Waiters, and whether that is the correct number or not.
jameinel
force-pushed
the
clock-stacks
branch
from
6a56b20
to
8b60cf4
Compare
howbazaar
approved these changes
Feb 3, 2019
We assumed that doing WaitAdvance(0,0,0) was valid. However, that triggers final before next, which means we don't see the 'everything is ok' event. Instead put one final check during WaitAdvance that we got what we wanted.
|
|
jujubot
added a commit
to juju/juju
that referenced
this pull request
Feb 5, 2019
#9709 ## Description of change The testing clock now iterates faster when given a long timeout, and gives stack traces when the waiters don't match expectations. This is just cleanup of the testing clock that helped when I was debugging something else, but we may as well get it into our core code. Only changes the testing clock, so it may change the test suite, but doesn't change production code. juju/clock#3 juju/clock#4 We need (4) #9712 ## QA steps None. ## Documentation changes None. ## Bug reference None.
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.
of 'util' we missed having the TestAll function.
infrastructure that it needed to read all Debug level log messages.
This is because many tests use WaitAdvance(..., LongWait), which
means that if they aren't ready immediately, then they way 1s before
checking again, which is way too long. Instead we cap out at 10ms, so
we don't spin waiting for the right number of waiters, but we also
don't wait too long. We still stop at the same time, and we still
stop as soon as we have the expected number of waiters.
possible to know who has actually called clock.After() and co, so
you can figure out why there are a particular number of Waiters, and
whether that is the correct number or not.
Nothing about the interface to the testing clock changes with this, it is just hopefully slightly nicer to debug issues when using the testing clock.