Fixes for scheduler test issues#127
Merged
AlexJones0 merged 4 commits intolowRISC:masterfrom Mar 27, 2026
Merged
Conversation
Modify these test cases so that they are more robust in checking every status is correct, rather than just the statuses of passing jobs. Also, fix an erroneous test case which accidentally included a cycle! Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
machshev
reviewed
Mar 27, 2026
machshev
approved these changes
Mar 27, 2026
Collaborator
machshev
left a comment
machshev
left a comment
There was a problem hiding this comment.
Thanks @AlexJones0
Small nit. I generally prefer pyhamcrest if possible as it gives more helpful error messages if the assertion fails.
Change the expectation of the dependency cycle test to be explicit that we expect a `ValueError` with some reference to cycles to be raised in the _constructor_ of the scheduler. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
The `full_name` of the `job_spec` is intended to be a fully unique identifier in the current DVSim interface, so rather than complicate the scheduler by allowing jobs of the same name to exist in different targets (treating targets as separate things), instead we say that jobs must have globally unique names. Then the targets are just like labels on the JobSpec DAG and otherwise are not really involved in the scheduler itself. Based on this, this test doesn't really make sense, so just remove it. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
This was not correctly comparing the JobSpec items to other JobSpec items, it was instead comparing to the names. Fix this and also do a little more cleanup. This wasn't caught because the test is already failing (DVSim does not support this case currently), so the error wasn't easily exposed. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
AlexJones0
force-pushed
the
scheduler_test_fixes
branch
from
71ea967 to
072000a
Compare
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
2 participants
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.
This PR is the eighth of a series of PRs to rewrite DVSim's core scheduling functionality (Scheduler, status display, launchers / runtime backends) to use an async design, with key goals of long term maintainability and extensibility.
This PR contains a small set of fixes for the scheduler tests that will be required for the tests to pass when moving to use the new async scheduler. For the most part, this addresses issues in tests that were masked by the fact that they are currently expected to fail due to deficiencies in the existing scheduler.
See the commit messages for more information.