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

Save results before raising exception #2996

Merged
merged 5 commits into from
Jul 20, 2024
Merged

Conversation

pholica
Copy link
Contributor

@pholica pholica commented Jun 7, 2024

This is essential for storing at least partial results and making report work if there was some issue with the system (e.g. became unreachable) during the execution.

Pull Request Checklist

  • implement the feature
  • extend the test coverage
  • include a release note

@happz happz added the step | execute Stuff related to the execute step label Jul 2, 2024
@happz happz added this to the 1.35 milestone Jul 2, 2024
@psss
Copy link
Collaborator

psss commented Jul 17, 2024

This is an important item for Upgrades & Conversions, marking as a must.

@psss psss added the priority | must high priority, must be included in the next release label Jul 17, 2024
pholica and others added 4 commits July 19, 2024 16:08
This is essential for storing at least partial results and making report
work if there was some issue with the system (e.g. became unreachable)
during the execution.
When waking up an unfinished step, we should not clear the workdir
unless the step in question is actually enabled, which covers the
scenario when user wants to give the step another try.
Let's fetch all artifacts stored in the plan data directory after
each test execution to prevent losing useful information if the
guest becomes unresponsive.
@psss psss added the ci | full test Pull request is ready for the full test execution label Jul 19, 2024
@psss
Copy link
Collaborator

psss commented Jul 19, 2024

Except for missing save() there were two issues which caused the problem:

  • workdir cleanup of disabled steps fixed in 972febb
  • missing plan data fixed in b615f25

Test coverage in fa2c32a suggests it's now working as expected.

@psss
Copy link
Collaborator

psss commented Jul 19, 2024

/packit build

@psss psss requested review from martinhoyer and happz July 19, 2024 14:39
Copy link
Collaborator

@martinhoyer martinhoyer left a comment

Choose a reason for hiding this comment

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

Looks good to me and works. I don't know enough this code to 'approve' stuff though.

Copy link
Collaborator

@thrix thrix left a comment

Choose a reason for hiding this comment

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

LGTM, no comments from me

@happz happz merged commit b81c362 into teemtee:main Jul 20, 2024
18 of 19 checks passed
@happz happz added the status | ready for merge The only missing piece is to do the rebase the current 'main' and let the CI finish. label Jul 20, 2024
The-Mule pushed a commit to The-Mule/tmt that referenced this pull request Oct 14, 2024
This is essential for storing at least partial results and making `report`
work if there was some issue with the system (e.g. became unreachable)
during the execution. Let's fetch all artifacts stored in the plan data directory
after each test execution to prevent losing useful information if the
guest becomes unresponsive.

As part of the solution, when waking up an unfinished step, we should not
clear the workdir unless the step in question is actually enabled, which covers
the scenario when user wants to give the step another try.

Co-authored-by: Petr Šplíchal <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci | full test Pull request is ready for the full test execution priority | must high priority, must be included in the next release status | ready for merge The only missing piece is to do the rebase the current 'main' and let the CI finish. step | execute Stuff related to the execute step
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants