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

Restore all jobs in the job state map (bugfix) #1605

Merged
merged 8 commits into from
Nov 20, 2024

Conversation

Hook25
Copy link
Collaborator

@Hook25 Hook25 commented Nov 18, 2024

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 does

Resolved issues

Fixes: CHECKBOX-1627
Fixes: #1557

Documentation

N/A

Tests

This adds a new metabox scenario for the regression

Copy link

codecov bot commented Nov 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 48.04%. Comparing base (542c748) to head (f9c2870).
Report is 9 commits behind head on main.

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     
Flag Coverage Δ
checkbox-ng 68.67% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

Copy link
Collaborator

@pieqq pieqq left a 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!

checkbox-ng/plainbox/impl/session/test_resume.py Outdated Show resolved Hide resolved
pieqq
pieqq previously approved these changes Nov 19, 2024
Copy link
Collaborator

@pieqq pieqq left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks!

pieqq
pieqq previously approved these changes Nov 20, 2024
Copy link
Collaborator

@pieqq pieqq left a 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.

metabox/metabox/scenarios/restart/agent_respawn.py Outdated Show resolved Hide resolved
@Hook25 Hook25 merged commit 953ffbf into main Nov 20, 2024
57 checks passed
@Hook25 Hook25 deleted the restore_all_jobs_job_state_map branch November 20, 2024 10:23
eugene-yujinwu pushed a commit to eugene-yujinwu/checkbox that referenced this pull request Dec 31, 2024
* 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>
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.

Cannot export the submission tar file after finishing the session when the test plan is not whole
3 participants