Skip to content
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

Testing Utility: FakeScheduler #809

Merged
merged 1 commit into from
Jan 4, 2024
Merged

Testing Utility: FakeScheduler #809

merged 1 commit into from
Jan 4, 2024

Conversation

JakenVeina
Copy link
Collaborator

Added a new implementation of IScheduler to allow full control over execution of scheduled actions, by test orchestration code.

Needed this to be able to simulate inaccuracies in real-world schedulers, for intervals around 1ms, in particular with reference to #716.

@dwcullop
Copy link
Member

dwcullop commented Jan 2, 2024

I don't have an issue with this, but I'd like to understand the need better. The regular test scheduler allows for control down to the Tick (100ns). What does this give us that the other doesn't?

A replacement Test Scheduler should also have the time control methods the other one does, like AdvanceTo and AdvanceBy (not sure if those are exactly the names).

@JakenVeina
Copy link
Collaborator Author

JakenVeina commented Jan 2, 2024

The test secheduler does not allow simulation of inaccuracies. It will always invoke scheduled operations at exactly the (virtual) time they were scheduled for.

This class does not have the .AdvanceTo() the .AdvanceBy() methods because those APIs couple together control over time simulation and action invokation, and the need this fills is specifically to control those things separately.

This class is not intended to replace the existing TestScheduler, it is intended to support edge cases that TestScheduler cannot.

… execution of scheduled actions, by test orchestration code.
@JakenVeina
Copy link
Collaborator Author

I've identified the build failure as an un-related faulty test, which intermittently fails, and documented it as #817.

@RolandPheasant RolandPheasant self-requested a review January 4, 2024 17:35
Copy link
Collaborator

@RolandPheasant RolandPheasant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I won't know the difference this makes until you use it. However it's for testing, so let's get it in.

@JakenVeina JakenVeina merged commit d0afe51 into main Jan 4, 2024
1 check passed
@JakenVeina JakenVeina deleted the feature/fake-scheduler branch January 4, 2024 17:52
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants