Mock clock - #191
Mock clock#191rnro wants to merge 2 commits into
Conversation
A call site wanting parameters on a particular context had to declare them `var` and reassign `context` on the next line, whether or not it went on to change anything else. `Parameters.context` is already public, so this adds no surface beyond the initializer itself. * Added `Parameters.init(context:)` outside the platform conditionals, so the private and embedded builds have it as well as the open-source one. * Adopted it at 15 call sites, dropping the `context` reassignment from each. * Switched ten of those sites to `let`, since they no longer mutate the value.
42c686f to
91066e0
Compare
| swiftSettings: availabilityMacros + settings | ||
| ), | ||
| .target( | ||
| name: "SwiftNetworkTestSupport", |
There was a problem hiding this comment.
Could we put the support functionality into SwiftNetworkTestHarness? Happy to rename that module, but it seems pretty much the same overall purpose
There was a problem hiding this comment.
I think they have a different purpose, and I had actually omitted something initially which made that unclear. I think that the new target should be a product, so that external adopters can also use mock time in their tests if needed.
agnosticdev
left a comment
agnosticdev
left a comment
There was a problem hiding this comment.
In which cases will we adopt this? Will it be strictly in our unit tests or will it be in QUIC stack tests as well? The reason I ask is that for the tests that are exercising loss recovery scenarios it would be good to still use real time for those cases.
`ManualScheduler` and `ManualTimeContext` let a test drive the stack's timers without reading a real clock, now that the scheduler owns time. They sit outside the library so nothing in it depends on them, and are offered as their own product so an adopter can drive time in tests of their own code. * Added `run(until:)` analogous to XCTest's `wait(for:timeout:)`, reporting whether the condition held rather than blocking on real time. * Mocking fires each timer with `now` set to its own deadline, so a timer that reads the clock sees the instant it was scheduled for rather than the end of the advance. * Moved `NetworkClock.Instant.testBase` into the new product and deleted `Tests/QUICTests/TestClock.swift`, which held nothing else. * Imported the new product in the five suites that use `testBase`. * Added `ManualSchedulerTests` to cover the scheduler.
e7c10cc to
03cb069
Compare
So this PR only converts the unit tests that build a context and never wait on anything. My intention is to move the QUIC stack tests over in follow-ups, loss recovery included. The intention is that everyone agrees on one source of truth for time, so that to any code reading the clock through the context, advancing it by hand is indistinguishable from the wall clock advancing. I'd like all our tests on mocked time eventually, with two exceptions: anything driving real sockets, and the handful that exist to exercise the real dispatch timer — On loss recovery specifically: nothing in those tests is real already. The transport is an in-memory bridge, loss comes from Do you see any issues with this plan which I'm missing? |
My concern is really the Recovery and Ack timers firing to run actions such as delayed ack, PTO, etc.. |
I agree with this approach. It would be nice though if we could have an option to do an integration test with the real clock. We wouldn't use it by default, but we could use it when we are tracking bugs and want to run a test with real time. |
4 participants
Add a test-support target for driving virtual time
ManualSchedulerandManualTimeContextlet a test drive the stack's timers without reading a real clock, now that the scheduler owns time. They need a home outside the library, so nothing ships them, and outside either test target, so both can import them.run(until:)analogous to XCTest'swait(for:timeout:), reporting whether the condition held rather than blocking on real time.nowset to its own deadline, so a timer that reads the clock sees the instant it was scheduled for rather than the end of the advance.NetworkClock.Instant.testBaseinto the new target and deletedTests/QUICTests/TestClock.swift, which held nothing else.testBase.ManualSchedulerTeststo cover the scheduler.NOTE: this is not yet adopted, that comes in a follow-up PR