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.

Sign up for GitHub

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

Jump to bottom

Implement check result key, checks affect test result by default #3239

Merged
merged 35 commits into from
Oct 25, 2024
Merged

Implement check result key, checks affect test result by default #3239

merged 35 commits into from
Oct 25, 2024

Conversation

martinhoyer
Copy link
Collaborator

@martinhoyer martinhoyer commented Sep 25, 2024 •
edited
Loading

Trying to address #3185

Pull Request Checklist

  • implement the feature
  • write the documentation
  • extend the test coverage
  • update the specification
  • modify the json schema
  • mention the version
  • include a release note

@martinhoyer martinhoyer added status | need help Extra attention is needed status | need tests Test coverage to be added for the affected code status | need docs Documentation to be added for the affected code area | results Related to how tmt stores and shares results labels Sep 25, 2024
@martinhoyer martinhoyer self-assigned this Sep 25, 2024
@martinhoyer martinhoyer mentioned this pull request Sep 26, 2024
Merged
5 tasks
@martinhoyer martinhoyer changed the title Have failure in check result propagate to the test result Implement check-result: test check failure affects test result by default Sep 26, 2024
happz
happz reviewed Sep 26, 2024
View reviewed changes
tmt/result.py Outdated Show resolved Hide resolved
@martinhoyer
Copy link
Collaborator Author

martinhoyer commented Sep 26, 2024 •
edited
Loading

Rebased to @happz's result-store-original-result #3147 and tried to use beakerlib for the first time. Feels werid to do all the assertgreps, but I've been merely following the existing examples.

Still proof of concept, but at least I finally figured that the "after-test" check were not available without the change in execute/internal.py.

@happz
Copy link
Collaborator

happz commented Sep 26, 2024

@martinhoyer if you change the base branch to the one from #3147, the diff should reduce a bit.

happz
happz reviewed Sep 26, 2024
View reviewed changes
tmt/result.py Show resolved Hide resolved
happz
happz reviewed Sep 27, 2024
View reviewed changes
tmt/result.py Outdated Show resolved Hide resolved
@martinhoyer
Copy link
Collaborator Author

tmt.utils.GeneralError: Test check 'dmesg' was not found in check registry.

perplexing

martinhoyer
martinhoyer commented Sep 30, 2024
View reviewed changes
tmt/result.py Outdated Show resolved Hide resolved
@martinhoyer martinhoyer marked this pull request as ready for review October 1, 2024 05:53
martinhoyer
martinhoyer commented Oct 1, 2024
View reviewed changes
docs/releases.rst Outdated Show resolved Hide resolved
martinhoyer
martinhoyer commented Oct 1, 2024
View reviewed changes
spec/tests/check.fmf Outdated Show resolved Hide resolved
martinhoyer
martinhoyer commented Oct 1, 2024
View reviewed changes
spec/tests/result.fmf Outdated Show resolved Hide resolved
happz
happz reviewed Oct 1, 2024
View reviewed changes
tmt/result.py Outdated Show resolved Hide resolved
happz
happz reviewed Oct 1, 2024
View reviewed changes
tmt/checks/__init__.py Outdated Show resolved Hide resolved
@psss psss added this to the 1.38 milestone Oct 1, 2024
@martinhoyer martinhoyer removed the status | need help Extra attention is needed label Oct 1, 2024
martinhoyer and others added 19 commits October 25, 2024 14:04
@martinhoyer @psss
[PoC] use metadata.choices for the enums
1ecc935
@martinhoyer @psss
Improve xfail check result handling
83e16e3
@psss
A minor spec wording adjustment
e0970ee
@psss
Small release notes adjustment, fix a typo
6389548
@psss
Add missing coversions to and from spec
0e36df1
@psss
Define the to_minimal_spec() method as well
7bc7973 
This one is used in test export.
@martinhoyer @psss
Improve tests
b7bdfa4
@martinhoyer @psss
Add missing parameter in docstring
f4cc82a
@martinhoyer @psss
Fix avc, dmesg tests expecting exit code 0
d7c37c8
@martinhoyer @psss
Addressing comments
2296307
@martinhoyer @psss
fixup! Fix avc, dmesg tests expecting exit code 0
2830254
@martinhoyer @psss
Addressing yet more valuable comments
f71767a
@martinhoyer @psss
Aggregating check results, attempt 1
96c03df
@martinhoyer @psss
Unit tests attempt 1
308f78a
@martinhoyer @psss
Aggregating check results first
03877ba
@martinhoyer @psss
Aggregate check results attempt 2
21181dd
@psss
Aggregate check results attempt 3
4b1972a
@psss
Adjust tests accordingly
3f75c53
@psss
Adjust tests accordingly
45889d7
psss
psss approved these changes Oct 25, 2024
View reviewed changes
Copy link
Collaborator

@psss psss left a comment

Choose a reason for hiding this comment

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

Now looks good. Thanks much for working on this!

@psss psss changed the title Implement check-result: test check failure affects test result by default Implement check result key, checks affect test result by default Oct 25, 2024
thrix
thrix reviewed Oct 25, 2024
View reviewed changes
spec/tests/check.fmf Outdated Show resolved Hide resolved
thrix
thrix approved these changes Oct 25, 2024
View reviewed changes
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

tmt/result.py Outdated Show resolved Hide resolved
seberm
seberm approved these changes Oct 25, 2024
View reviewed changes
Copy link
Collaborator

@seberm seberm left a comment

Choose a reason for hiding this comment

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

LGTM. Great work Martin!

@psss psss merged commit 60c2d37 into teemtee:main Oct 25, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@psss psss psss approved these changes

@KwisatzHaderach KwisatzHaderach KwisatzHaderach approved these changes

@thrix thrix thrix approved these changes

@seberm seberm seberm approved these changes

@lukaszachy lukaszachy Awaiting requested review from lukaszachy lukaszachy is a code owner

@janhavlin janhavlin Awaiting requested review from janhavlin janhavlin is a code owner

@happz happz Awaiting requested review from happz happz is a code owner

@sbertramrh sbertramrh Awaiting requested review from sbertramrh

Assignees

@martinhoyer martinhoyer

Labels
area | results Related to how tmt stores and shares results ci | full test Pull request is ready for the full test execution priority | must high priority, must be included in the next release
Projects
None yet
Milestone
1.38
Development

Successfully merging this pull request may close these issues.

Implement the check result key
7 participants
@martinhoyer @happz @psss @seberm @thrix @KwisatzHaderach @sbertramrh