-
Notifications
You must be signed in to change notification settings - Fork 15
Strengthen scheduler tests against preexisting tasks in agenda #483
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
Conversation
Also, update block numbers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes correctly improve test robustness by handling multiple tasks in the agenda. However, the test logic is incomplete as it identifies the task but does not capture its index, which is required for the subsequent cancellation step that currently relies on a hardcoded index.
Specifically, the scheduled task cancellation tests. More to come.
In particular, refactor the priority-weighted test, and add/change existing checks.
|
Ultimately led to a few more scheduler tests:
With this, tests should no longer occasionally fail due to the presence of real tasks in the agenda of the block selected for a test to run on. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes improve the robustness of scheduler tests by introducing helpers to find tasks in the agenda instead of relying on fixed indices. However, there is a likely incorrect assertion in the scheduleToFullAgenda test regarding the error type returned when the agenda is full.
Closes #475 .
Issue contains more information.
TL;DR is that this PR makes scheduler E2E tests more robust in cases in which the forked network's scheduler's agenda contains tasks scheduled for blocks that tests themselves schedule tasks in, without having to delete those tasks.