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

Store the original test, check and subresult outcome in results #3147

Merged
merged 4 commits into from
Oct 1, 2024

Conversation

happz
Copy link
Collaborator

@happz happz commented Aug 14, 2024

Pull Request Checklist

  • implement the feature
  • write the documentation
  • update the specification
  • mention the version
  • include a release note

@happz happz added specification Metadata specification (core, tests, plans, stories) area | results Related to how tmt stores and shares results labels Aug 14, 2024
@happz happz added this to the 1.36 milestone Aug 14, 2024
@happz happz modified the milestones: 1.36, 1.37 Sep 2, 2024
@happz happz force-pushed the result-store-original-result branch from cf9f4b6 to 4c4cbbe Compare September 9, 2024 13:03
@happz happz marked this pull request as ready for review September 9, 2024 13:04
@happz happz force-pushed the result-store-original-result branch from 4c4cbbe to f04b452 Compare September 9, 2024 13:06
@happz happz changed the title Store the original test outcome in results Store the original test, check and subresult outcome in results Sep 9, 2024
@happz happz force-pushed the result-store-original-result branch from f04b452 to 276ba11 Compare September 16, 2024 07:30
@happz happz added status | ready for merge The only missing piece is to do the rebase the current 'main' and let the CI finish. and removed status | ready for merge The only missing piece is to do the rebase the current 'main' and let the CI finish. labels Sep 17, 2024
@happz happz added the ci | full test Pull request is ready for the full test execution label Sep 25, 2024
@happz
Copy link
Collaborator Author

happz commented Sep 25, 2024

/packit test

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.

I wish I saw this before working on it in #3239 lol

@martinhoyer
Copy link
Collaborator

btw, in results.py

        # Extend existing note or set a new one
        if self.note:
            self.note += f', original result: {self.result.value}'
        elif self.note is None:
            self.note = f'original result: {self.result.value}'

shouldn't it be self.original_result.value?

@happz
Copy link
Collaborator Author

happz commented Sep 30, 2024

btw, in results.py

        # Extend existing note or set a new one
        if self.note:
            self.note += f', original result: {self.result.value}'
        elif self.note is None:
            self.note = f'original result: {self.result.value}'

shouldn't it be self.original_result.value?

At this moment, self.result and self.original_result should be the same value, I'm looking at code and self.result is modified a few lines later.

tmt/schemas/results.yaml Outdated Show resolved Hide resolved
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.

Looks good, thanks!

@psss psss force-pushed the result-store-original-result branch from 2ca36d8 to 76cd36c Compare September 30, 2024 21:07
@psss
Copy link
Collaborator

psss commented Oct 1, 2024

Red tests are irrelevant failures, merging.

@psss psss merged commit 1d58b6d into main Oct 1, 2024
21 of 23 checks passed
@psss psss deleted the result-store-original-result branch October 1, 2024 07:23
@psss psss self-assigned this Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area | results Related to how tmt stores and shares results ci | full test Pull request is ready for the full test execution specification Metadata specification (core, tests, plans, stories)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants