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

Add skip as a supported custom result outcome #2326

Merged
merged 1 commit into from
Oct 6, 2023
Merged

Conversation

mkoncek
Copy link
Collaborator

@mkoncek mkoncek commented Sep 12, 2023

As we discussed with @lukaszachy probably a month ago the purpose of this is to explicitly state that a test was not executed.
The use case is when a single tmt test produces multiple results but some of them were not executed because the implementation decided so. It should be visible to the user that such a test exists and that there is not a bug in the implementation which caused the test to be missing.
It is still an open question how such cases should be displayed in Fedora's CI.

tmt/result.py Outdated Show resolved Hide resolved
tmt/result.py Outdated Show resolved Hide resolved
spec/plans/results.fmf Outdated Show resolved Hide resolved
tmt/steps/execute/__init__.py Outdated Show resolved Hide resolved
@mkoncek mkoncek changed the title Add NOT_EXECUTED custom result outcome Add SKIP custom result outcome Sep 13, 2023
@mkoncek
Copy link
Collaborator Author

mkoncek commented Sep 14, 2023

@happz Rebased, I think i addressed all the issues.

@happz happz added this to the 1.28 milestone Sep 14, 2023
@happz happz added the results Related to how tmt stores and shares results label Sep 14, 2023
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.

Thanks for the improvement. Looks good, added just two comments. And thinking: What about extending the /tests/execute/basic test to exercise the new skip status as well?

spec/plans/results.fmf Outdated Show resolved Hide resolved
spec/plans/results.fmf Outdated Show resolved Hide resolved
@psss
Copy link
Collaborator

psss commented Sep 14, 2023

The final summary shows:

    summary: 1 test passed, 1 test failed, 1 warn, 1 error and 1 skip

Perhaps we could use skipped there to be consistent with the rest of the wording.

@psss
Copy link
Collaborator

psss commented Sep 14, 2023

/packit test

@psss psss changed the title Add SKIP custom result outcome Add skip custom result outcome Sep 14, 2023
@psss
Copy link
Collaborator

psss commented Oct 4, 2023

@mkoncek, are you going to address the pending comments?

@mkoncek
Copy link
Collaborator Author

mkoncek commented Oct 4, 2023

@mkoncek, are you going to address the pending comments?

Yes, I have been meaning to do it for some time...

Regarding documentation, the wording will need to be very precise.

@psss
Copy link
Collaborator

psss commented Oct 5, 2023

/packit test

Copy link
Collaborator

@lukaszachy lukaszachy left a comment

Choose a reason for hiding this comment

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

There should be a test for this outcome as well.

tmt/steps/execute/__init__.py Outdated Show resolved Hide resolved
@psss psss requested a review from lukaszachy October 6, 2023 07:16
@psss
Copy link
Collaborator

psss commented Oct 6, 2023

/packit test

@psss
Copy link
Collaborator

psss commented Oct 6, 2023

Some changes will be needed also on the Testing Farm side: TFT-2282

@psss
Copy link
Collaborator

psss commented Oct 6, 2023

/packit test

The use case is when a single tmt test produces multiple results
but some of them were not executed because the process decided so.
It should be visible to the user that such a test exists and that
there is not a bug in the implementation which caused the test to
be missing.

Co-authored-by: Petr Šplíchal <psplicha@redhat.com>
@psss
Copy link
Collaborator

psss commented Oct 6, 2023

/packit test

@psss psss dismissed lukaszachy’s stale review October 6, 2023 12:45

Should be addressed.

@psss psss changed the title Add skip custom result outcome Add skip as a supported custom result outcome Oct 6, 2023
@psss psss merged commit cb1724d into teemtee:main Oct 6, 2023
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
results Related to how tmt stores and shares results
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants