-
Notifications
You must be signed in to change notification settings - Fork 50
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
Restore all jobs in the job state map (bugfix) #1605
Conversation
It only worked when the program would stop, now it always does
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1605 +/- ##
=======================================
Coverage 48.03% 48.04%
=======================================
Files 371 371
Lines 39852 39855 +3
Branches 6734 6736 +2
=======================================
+ Hits 19142 19147 +5
+ Misses 19993 19992 -1
+ Partials 717 716 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
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.
Great stuff! Thanks for spending the time to figure out this issue.
For the unit tests, it looks like SessionJobsAndResultsResumeTests
is parametrized somehow, so it might be worth making sure the right version of SessionResumeHelper
is tested.
Otherwise, I really like the Metabox test, it's very clear.
A few other comments in the unit tests, but otherwise I'd say it's good to land!
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.
Awesome! Thanks!
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.
Awesome stuff! I just found a little typo (loose/lose), but otherwise it's good to land.
* Initial bugfix * New agent_respawn scenario * Fix ExpectNot on timeout It only worked when the program would stop, now it always does * Robust to missing fields in metadata * Test that rejected_jobs are restored in the job_state_map * Update tests as per review * Split test into remote + local * Update metabox/metabox/scenarios/restart/agent_respawn.py --------- Co-authored-by: Pierre Equoy <pierre.equoy@canonical.com>
Description
This fixes the bug with the session that when a test is de-selected, any resume of the session will make the submission.json generation fail. This was because the job_state_map is not entirely re-loaded after the resume, but only the tests that were selected are correctly put in the map.
Additionally, the validation of tests would not cover rejected tests (which is dangerous, as a test may be deselected for a reason, and that reason may not hold anymore).
Finally there was a bug in the session resume where tests were only incidentally not re-run as the rejected job list was not re-evaluated after resume.
Minor: this fixes a bug with
ExpectNot
, as it wouldn't work if the checkbox controller didnt finish running, now it doesResolved issues
Fixes: CHECKBOX-1627
Fixes: #1557
Documentation
N/A
Tests
This adds a new metabox scenario for the regression