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

Fix log paths for subresults loaded from tmt-report-results.yaml #3370

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

seberm
Copy link
Collaborator

@seberm seberm commented Nov 19, 2024

The result/subresult log file path loaded from the tmt-report-results.yaml file must be modified to be relative to execute directory the same way as for custom / restraint results (see ExecutePlugin._process_results_partials(...)).

Pull Request Checklist

  • implement the feature
  • extend the test coverage
  • include a release note - should I include one?

@seberm seberm added bug Something isn't working area | results Related to how tmt stores and shares results ci | full test Pull request is ready for the full test execution labels Nov 19, 2024
@seberm seberm added this to the 1.39 milestone Nov 19, 2024
@seberm seberm force-pushed the feature/fix-internal-subresults-log-path branch from c46a241 to 2b4974d Compare November 19, 2024 20:31
@seberm seberm added the code | trivial A simple patch - a couple of lines, an easy-to-understand change, a typo fix. label Nov 19, 2024
@seberm seberm force-pushed the feature/fix-internal-subresults-log-path branch from 2b4974d to 3eaaf33 Compare November 20, 2024 13:52
@seberm seberm removed the code | trivial A simple patch - a couple of lines, an easy-to-understand change, a typo fix. label Nov 20, 2024
@seberm seberm force-pushed the feature/fix-internal-subresults-log-path branch from 3eaaf33 to 0920dba Compare November 21, 2024 13:32
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.

I quickly checked the generated results.yaml file and it does not seem to be correct. I used the following test:

cd tests/execute/restraint/report-result/data
tmt run --all test --name output report --how html --open

Seems that while the subresult logs have been fixed and are working fine the main result logs are now broken.

@psss psss modified the milestones: 1.39, 1.40 Nov 21, 2024
@seberm seberm force-pushed the feature/fix-internal-subresults-log-path branch from 0920dba to 753cd1f Compare November 22, 2024 15:13
@seberm
Copy link
Collaborator Author

seberm commented Nov 22, 2024

I quickly checked the generated results.yaml file and it does not seem to be correct. I used the following test:

cd tests/execute/restraint/report-result/data
tmt run --all test --name output report --how html --open

Seems that while the subresult logs have been fixed and are working fine the main result logs are now broken.

Great catch @psss ! The problem is that the log path fix is applied two times - once by my changes in _load_tmt_report_results_file() and secondly in _process_results_paritals() which is used by extract_tmt_report_results_restraint(). That's why the restraint results had a bad path.

I've modified my changes to apply the fix in extract_tmt_report_results() and keep the behavior the same for custom and restraint results (handled in _process_results_partials(). Now it seems it's working correctly for all our cases.

Could you (@psss @happz) please check again and help me with the testing? I do not want to break anything for the users :). Thanks!

@seberm seberm requested review from happz and psss November 22, 2024 15:29
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 bug Something isn't working ci | full test Pull request is ready for the full test execution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants