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 subresults support for reportportal report plugin #3331

Merged
merged 1 commit into from
Feb 25, 2025

Conversation

seberm
Copy link
Collaborator

@seberm seberm commented Oct 30, 2024

This PR adds support for tmt subresults for tmt reportportal plugin. It tries to map the subresults as child items of parent test result in a specific launch.

Please don't review this PR while it is marked as a draft.

More info:

TODOs:

  • Core functionality (import of tmt subresults into RP)
  • Find a way to map the subresults to parent test/step reportportal launch items
  • Should the import of subresults be optional via argument? Or via fmf test metadata proposed by @kkaarreell ? What should be the default behavior?
  • Try to resolve status:interrupted issues for subresults in RP, why is this happening?
  • There are still some problems in local reportportal tests (subresults need special handling) - will be handled by @4N0body5
  • Missing tests for beakerlib subresults (subresults reported by shell tests using the tmt-report-result command should be already covered) - Nice to have (can be added in follow-up PR)

Blocked by:

Related:

Pull Request Checklist

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

@seberm seberm added step | report Stuff related to the report step plugin | reportportal The reportportal report plugin labels Oct 30, 2024
@seberm seberm force-pushed the feature/reportportal-subresults branch from 1e50f45 to 757da3f Compare October 30, 2024 17:49
@seberm seberm requested review from kkaarreell and 4N0body5 October 30, 2024 17:53
@seberm seberm force-pushed the feature/reportportal-subresults branch from 757da3f to 11d5bc3 Compare October 30, 2024 17:54
@seberm seberm force-pushed the feature/reportportal-subresults branch 2 times, most recently from 0014f19 to 0fb5d3f Compare November 5, 2024 17:51
@seberm seberm force-pushed the feature/reportportal-subresults branch from 0fb5d3f to a6da49f Compare November 12, 2024 11:19
@seberm seberm changed the base branch from main to feature/reportportal-improvements November 12, 2024 11:52
@seberm seberm force-pushed the feature/reportportal-subresults branch from a6da49f to 3b7b5a4 Compare November 12, 2024 11:52
@seberm seberm added the status | blocked The merging of PR is blocked on some other issue label Nov 12, 2024
@seberm seberm force-pushed the feature/reportportal-improvements branch from b2f5688 to c1518ce Compare November 12, 2024 15:50
@seberm seberm force-pushed the feature/reportportal-subresults branch 3 times, most recently from 0d383fb to bdb49f9 Compare November 14, 2024 16:33
@kkaarreell
Copy link
Collaborator

I think it might be handy to control subresults import on a per test level through test fmf metadata.

@seberm
Copy link
Collaborator Author

seberm commented Nov 19, 2024

These are just some screenshots from my testing.

BTW: @kkaarreell Do you have any idea why the subresult items have the status:interrupted tag assigned to them? I still can't figure out why is this happening.

Launch overview (parent tmt results):

01-Screenshot 2024-11-19 at 12-56-28 Report Portal

Beakerlib test detail (subresults / child test items):

02-Screenshot 2024-11-19 at 12-57-08 Report Portal-bkrlib-subresults

Details of shell test subresults when calling tmt-report-result command multiple times:

03-Screenshot 2024-11-19 at 12-57-37 Report Portal-tmt-report-result-subresults

@kkaarreell
Copy link
Collaborator

I have tested the current code but it doesn't work (at least) for beakerlib tests. There are no logs for individual test phases (I am not even sure if beakerlib even produces them at all) and therefore there is nothing RP AA could work with or a reviewer look at when reviewing a particular failed phase. One can access logs for the whole test but because subresults are imported one can't assign a test result for a whole test as it needs to be done for individual phases. As a result, this feature is not usable ATM. As a precaution, phase reporting should be enabled only for tests that provide individual phase logs.

@seberm
Copy link
Collaborator Author

seberm commented Nov 20, 2024

Hello @kkaarreell,
thanks for your testing and input!

I have tested the current code but it doesn't work (at least) for beakerlib tests. There are no logs for individual test phases (I am not even sure if beakerlib even produces them at all) and therefore there is nothing RP AA could work with or a reviewer look at when reviewing a particular failed phase.

I am aware the logs for subresults are not imported. I was troubleshooting this yesterday and the problem why the shell subresults (created by tmt-report-result -o <logfile> ...) and also beakerlib phases (see below) are not there should be fixed by this PR:

Regarding the logs for beakerlib subresults/phases, we want them, for sure. I was troubleshooting why the phase logs are not saved by tmt-report-result script. I've described the reason in this PR:

One can access logs for the whole test but because subresults are imported one can't assign a test result for a whole test as it needs to be done for individual phases. As a result, this feature is not usable ATM. As a precaution, phase reporting should be enabled only for tests that provide individual phase logs.

If I understand this correctly, if we always provide logs for ReportPortal child items (beakerlib phases and logs reported by calling tmt-report-result), we are fine and everything is nice and shiny. Right?

With both PRs (#3370, #3372) applied, the result is following:

Parent beakerlib result detail:

01-Screenshot 2024-11-20 at 14-06-07 Report Portal-parent-view

Parent beakerlib result logs:

02-Screenshot 2024-11-20 at 14-08-05 Report Portal-parent-logs

Subresult logs (setup phase):

03-Screenshot 2024-11-20 at 14-08-36 Report Portal-setup-subresult-logs

Subresult logs (myfail phase):

04-Screenshot 2024-11-20 at 14-09-26 Report Portal-myfail-subresult-logs

@kkaarreell
Copy link
Collaborator

On the last screenshot there is a test phase failure but the error log is missing.

@psss psss added this to the 1.40 milestone Nov 21, 2024
@seberm seberm force-pushed the feature/reportportal-improvements branch from f19af6c to d3fbee8 Compare November 22, 2024 15:34
@seberm seberm force-pushed the feature/reportportal-subresults branch from ba94e41 to c8d1ec9 Compare November 22, 2024 20:37
@seberm seberm force-pushed the feature/reportportal-improvements branch 2 times, most recently from 6477dce to 50bc017 Compare November 28, 2024 08:24
@seberm
Copy link
Collaborator Author

seberm commented Jan 29, 2025

@thrix , thanks for the summary.

we decided we want to make this opt-in, i.e. there should be a boolean field so users opt-in on reporting with subresults, default will be without subresults

I've added --upload-subresults option which makes the subresults import optional. By default, the subresults will not be imported. Please take a look at the change. Is it sufficient? Thanks.

@seberm seberm added the code | schema Schema used for validating config files label Jan 29, 2025
@seberm
Copy link
Collaborator Author

seberm commented Jan 29, 2025

Hello @4N0body5

Using 'provision -h local' on my PC caused major problems, preventing the report of subresults. It is to be determined whether this issue is reproducible or specific to my device.

I would like to ask if someone could help us to test the beakerlib subresults (https://github.com/teemtee/tmt/blob/1e522228b8f213b8acb300b409006d494c9128b2/tests/report/reportportal/data/beaker-phases-subresults.sh) and their import into the reportportal with the -h local.

Also, I would appreciate if someone more experienced could help @4N0body5's with the local environment because I suspect it's somehow broken and I was unable to help (also discussed on tmt slack). @psss, @happz?

Reportportal test requires significant attention, including fixing the phase uuid-based rerun stuck in an endless loop, addressing minor failures caused by additional subresult tests, rewriting magic constants and hard-coded test commands for scalability, and adding a new phase to support additional Beakerlib test. Here I can offer my help throughout following month.

My understanding is that changes in this PR do not cause the endless loop in uuid-based rerun test phase. The endless loop was there before. Is that correct?

I agree we should fix the tests. They should be stable at least to the same level as before. E.g. endless loop could be fixed in another PR. I would appreciate your help, thanks a lot!

In addition to the option enabling report of subresults, the implementation requires additional improvement, specifically in the propagation of test item attributes into subresult item's Item Details. It is mainly environment variables (parameters), tmt test ID (test case ID), test web link (code ref), which are uploaded in the test item's background but they do not appear in its UI due to how the RP works for root test items. Additionally, description and test attributes can be propagated as well, although these are displayed at least on the higher level. They can be either simply copied from the test level (description, test id, etc.) or updated based on the particular subresult/phase (environment variables, ie.). These improvements should be straightforward and can eventually be done in a follow-up PR, which I can manage.

I agree the metadata should be propagated from the parent test items to their children's items. Because the import of subresults is now optional (1e52222) I also think we should propate them in a follow-up PR.

@seberm
Copy link
Collaborator Author

seberm commented Jan 29, 2025

Hello @4N0body5
to sync up. Hopefully, I've fixed the test for multiple test contacts. I probably reintroduced this fail by a bad rebase when fixing the conflicts against the main branch: 258460e

The current status of the reportportal tests running inside of a -h container is (at least in my environment) the following. Hopefully, you will get the same results :).

My env:

provision
        queued provision.provision task #1: default-0
        provision.provision task #1: default-0
        how: container
        primary address: tmt-5wE-ifdrWDGm
        topology address: tmt-5wE-ifdrWDGm
        name: tmt-5wE-ifdrWDGm
        multihost name: default-0
        arch: x86_64
        distro: Fedora Linux 41 (Container Image)
        kernel: 6.9.7-1.qubes.fc32.x86_64
        package manager: dnf5
        selinux: no
        is superuser: yes

Extended Functionality - HISTORY AGGREGATION

I am getting three fails for subresults.

...
:: [ 15:10:37 ] :: [   FAIL   ] :: Assert the previous item is not in history (Assert: "944762" should not equal "944762")
...
:: [ 15:10:38 ] :: [   FAIL   ] :: Assert the previous item is not in history (Assert: "944763" should not equal "944763")
...
:: [ 15:10:39 ] :: [   FAIL   ] :: Assert the previous item is not in history (Assert: "944764" should not equal "944764")
...

Extended Functionality - NAME-BASED RERUN

Six fails, also caused by subresults.

...
:: [ 15:11:25 ] :: [   FAIL   ] :: Assert the test item has correct UUID (Assert: 'f34081dd-7e5f-46f6-95fb-2070242e8585' should equal '9fb03147-45ee-4d63-a25e-3bc023ae9ea1')
:: [ 15:11:25 ] :: [   FAIL   ] :: Assert the test item retry item has a correct UUID (Assert: 'null' should equal 'b2b7eff8-950c-43a0-a356-499c84145df0')
:: [ 15:11:25 ] :: [   FAIL   ] :: Assert the test item has correct UUID (Assert: '9fb03147-45ee-4d63-a25e-3bc023ae9ea1' should equal 'd9b1df1e-75ff-4ad4-bd4c-9bb71b9ec959')
:: [ 15:11:25 ] :: [   FAIL   ] :: Assert the test item retry item has a correct UUID (Assert: 'b2b7eff8-950c-43a0-a356-499c84145df0' should equal '384fa5f1-a44b-4672-84f2-f2fe31382e8b')
:: [ 15:11:26 ] :: [   FAIL   ] :: Assert the test item has correct UUID (Assert: 'd9b1df1e-75ff-4ad4-bd4c-9bb71b9ec959' should equal 'f34081dd-7e5f-46f6-95fb-2070242e8585')
:: [ 15:11:26 ] :: [   FAIL   ] :: Assert the test item retry item has a correct UUID (Assert: '384fa5f1-a44b-4672-84f2-f2fe31382e8b' should equal 'f34081dd-7e5f-46f6-95fb-2070242e8585')
...

Extended Functionality - UUID-BASED RERUN

Two failures occur here, and then it enters an endless loop.

...
:: [ 15:11:53 ] :: [   FAIL   ] :: Assert the message of the info log is correct (Assert: 'Something bad happened!' should equal '')
:: [ 15:11:53 ] :: [   PASS   ] :: Assert the level of the info log is correct (Assert: 'ERROR' should equal 'ERROR')
:: [ 15:11:53 ] :: [   FAIL   ] :: Assert the message of the info log is correct (Assert: 'Something bad happened!' should equal '')
...

@seberm seberm force-pushed the feature/reportportal-subresults branch 2 times, most recently from d60e374 to 641c073 Compare January 30, 2025 12:02
@happz happz requested a review from thrix February 13, 2025 20:35
@happz
Copy link
Collaborator

happz commented Feb 13, 2025

@seberm please, rebase when you find some spare time (ruff format has been added, most conflicts will be probably just formatting changes). Also, there are some unmarked checkboxes in the PR description, are there lose ends that need to be tackled before re-reviewing the PR & eventual merge?

@psss psss added the priority | should medium priority, should be included in the next release label Feb 18, 2025
@seberm seberm force-pushed the feature/reportportal-subresults branch 2 times, most recently from f8a3f93 to 7017bef Compare February 18, 2025 11:31
@seberm
Copy link
Collaborator Author

seberm commented Feb 18, 2025

@seberm please, rebase when you find some spare time (ruff format has been added, most conflicts will be probably just formatting changes). Also, there are some unmarked checkboxes in the PR description, are there lose ends that need to be tackled before re-reviewing the PR & eventual merge?

Hello @happz,
I've rebased the changes on top of the main. Hopefully, I haven't introduced any bugs :). I will test it as soon as I finish the system configuration on my new laptop.

About unmarked checkboxes - I've added more info into the PR description. The tests for beakerlib reportportal subresults are nice to have. The rest of the issues in the local reportportal tests will be handled by @4N0body5 (see #3331 (comment)).

@seberm seberm force-pushed the feature/reportportal-subresults branch from 7017bef to bc18ded Compare February 25, 2025 09:09
@psss psss requested a review from happz February 25, 2025 09:52
@lukaszachy lukaszachy mentioned this pull request Feb 25, 2025
8 tasks
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 and works are expected. Thanks much for implementing this! After discussion we've agreed that the necessary test adjustments will be done in a separate pull request once @4N0body5 has capacity. Added just one release note nitpick.

Copy link
Collaborator

@therazix therazix left a comment

Choose a reason for hiding this comment

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

LGTM

@psss psss force-pushed the feature/reportportal-subresults branch from bc18ded to 2c03006 Compare February 25, 2025 15:15
@psss psss added the ci | full test Pull request is ready for the full test execution label Feb 25, 2025
@lukaszachy lukaszachy force-pushed the feature/reportportal-subresults branch from 2c03006 to fc0d463 Compare February 25, 2025 15:43
@lukaszachy lukaszachy force-pushed the feature/reportportal-subresults branch from fc0d463 to 0c05e74 Compare February 25, 2025 15:52
@lukaszachy
Copy link
Collaborator

rebased and squashed locally, push ---force.

If someone else will be doing the merge, please do it manually so commit hash is not modified.
I'll rebase #3547 to this commit so there is no waste of time by running another round of CI jobs after this PR is merged.

@lukaszachy lukaszachy self-assigned this Feb 25, 2025
@lukaszachy
Copy link
Collaborator

testing-farm:fedora-41-x86_64:provision fails only in tests using centos-stream-10 -> expected

@lukaszachy
Copy link
Collaborator

testing-farm:fedora-41-x86_64:full has passed in #3547, let's not waste evening time -> merging

@lukaszachy lukaszachy merged commit 0c05e74 into main Feb 25, 2025
20 of 22 checks passed
@lukaszachy lukaszachy deleted the feature/reportportal-subresults branch February 25, 2025 19:29
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 code | schema Schema used for validating config files plugin | reportportal The reportportal report plugin priority | should medium priority, should be included in the next release step | report Stuff related to the report step
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants