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

Added inaccurate test schedulers #72

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

runloop
Copy link

@runloop runloop commented Jan 11, 2023

A test scheduler that adds the minimumTolerance to the now property prior to executing units of work. This provides developers a means of testing against drift with a predictable value.

Copy link
Member

@stephencelis stephencelis left a comment

Choose a reason for hiding this comment

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

Thanks! Just a few small things to consider and discuss.

Sources/CombineSchedulers/TestScheduler.swift Outdated Show resolved Hide resolved
Sources/CombineSchedulers/TestScheduler.swift Outdated Show resolved Hide resolved
Sources/CombineSchedulers/TestScheduler.swift Outdated Show resolved Hide resolved

self.now = next.date
self.now = next.date.advanced(by: minimumTolerance)
Copy link
Member

Choose a reason for hiding this comment

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

I think my hunch was that a call to scheduler.advance() would incur the minimum tolerance and no more, but it sounds like this will incur the minimum tolerance for every work item, which seems a little less easy to understand from the outside, and more prone to causing test failures when your code changes. What do you think about calling it a single time instead of in the loop?

Copy link
Author

Choose a reason for hiding this comment

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

The idea was to replicate in a predictable way, how schedulers work in real life. If I set a timer for 1 second, it's never just exactly 1 second. There is always an additional bit of time albeit very small. But it can add up quite quickly if not accounted for.

If there are several units of work scheduled, you would want them all to be triggered at their scheduled date plus the the delay which you specified when you create the scheduler.

Perhaps it's confusing using the minimum tolerance for this.

Copy link
Author

Choose a reason for hiding this comment

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

Another thought on this.

As the motivation for this is to replicate the slight difference between the scheduled time and the time the action actually takes place, it might make more sense to update the schedule date directly as the action is scheduled.

This way it's easier to reason. You know if you have a test scheduler with a drift value of 0.05 and you schedule some work at 1 second, that it will occur once the scheduler advances 1.05 seconds.

The reason you might want this is if you are performing repeated scheduling and precision matters to you. If you know what time an action was scheduled for you can take the difference between the scheduled time and the actual time, and offset the next scheduled event.

runloop and others added 5 commits January 12, 2023 09:32
Co-authored-by: Stephen Celis <stephen.celis@gmail.com>
Co-authored-by: Stephen Celis <stephen.celis@gmail.com>
Co-authored-by: Stephen Celis <stephen.celis@gmail.com>
@stephencelis
Copy link
Member

@runloop Sorry for the delay! We intend to return to this soon but it's been a busy month 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants