Use tokio::time instead of std::time, to enable tokio's cool testing helpers - #947
Open
njsmith wants to merge 1 commit into
Open
Use tokio::time instead of std::time, to enable tokio's cool testing helpers#947njsmith wants to merge 1 commit into
njsmith wants to merge 1 commit into
Conversation
These are identical in all situations, except when using tokio's virtualized-wallclock testing mode, which makes time-based tests fast and deterministic, and is fantastic for things like fuzzing. This makes h2 behave correctly in any downstream codebases with this style of test.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
tokio::time::Instant is a trivial newtype wrapper around
std::time::Instant, and normallytokio::time::Instant::now()simply calls the std version, so it's a zero-cost drop-in replacement. But! If you build tokio with thetest-utilfeature enabled, then it also adds a quick check to see if you've overridden the clock for testing.It's not documented/publicized as much as it should be, but tokio's clock testing mode is actually really cool. When you call
sleep, tokio doesn't actually sleep, it just makes a note that the task should be awakened when the virtual clock reaches the given timestamp, and then whenever all tasks are idle it peeks at the timer wheel and advances the virtual time to whatever thing wants to wake up next. So as long as you make sure your test code doesn't perform any real system IO, then you can write arbitrarily complex tokio programs involving sleeps and time calculations, and they run super fast and deterministically. It's awesome for testing, fuzzing, etc.Anyway, you can almost use h2 in these tests, except there are two places where it uses
std::time::Instantinstead oftokio::time::Instant. This PR fixes that.I also switched two existing tests that use real sleeps to use the fake sleeps instead. I doubt it makes much difference because the sleeps were pretty short anyway, but in principle it should make them faster + less likely to get broken by janky CI runners with noisy neighbors, etc.
I also asked Claude to go look for invariants that would be good to test but aren't tested currently b/c real timing tests are so annoying and it came up with a few tests that look plausible enough to me, but I don't understand the code enough to really judge so
I'll stick those up as a followup PR that you can take or leave.edit: I can't figure out how to post a stacked-PR from a fork into another org's repo, so I'll just link to the claude-slop tests here: https://github.com/njsmith/h2/compare/tokio-instant...tokio-instant-tests?expand=1