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

[BUG] [regression] Broken status report for early failed requests after e121f683c28f99a7d071121852e8dcd4e08eed66 merge #70

Closed
fernflower opened this issue Mar 20, 2024 · 4 comments · Fixed by #71
Labels
bug Something isn't working

Comments

@fernflower
Copy link
Collaborator

fernflower commented Mar 20, 2024

tesar report -c UUID doesn't work as expected

(.venv) [ivasilev@ivasilev-thinkpadp1gen4i tesar]$ tesar report -c 6b4ea828-52b9-4575-b4be-382b2b510b0a
Reporting for the requested tasks:
RHEL-8.8.0-Nightly None https://api.dev.testing-farm.io/v0.1/requests/6b4ea828-52b9-4575-b4be-382b2b510b0a COMPLETE
Traceback (most recent call last):
  File "/home/ivasilev/projects/tesar/.venv/lib/python3.11/site-packages/report/__main__.py", line 234, in parse_request_xunit
    testsuite_arch = elem.xpath("./testing-environment/property[@name='arch']/@value")[0]
                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^
IndexError: list index out of range

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/ivasilev/projects/tesar/.venv/bin/tesar", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/home/ivasilev/projects/tesar/.venv/lib/python3.11/site-packages/tesar/__init__.py", line 23, in main
    sys.exit(report())
             ^^^^^^^^
  File "/home/ivasilev/projects/tesar/.venv/lib/python3.11/site-packages/report/__main__.py", line 511, in main
    result_table = build_table_comparison() if ARGS.compare else build_table()
                                                                 ^^^^^^^^^^^^^
  File "/home/ivasilev/projects/tesar/.venv/lib/python3.11/site-packages/report/__main__.py", line 407, in build_table
    parsed_dict = parse_request_xunit(skip_pass=ARGS.skip_pass)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ivasilev/projects/tesar/.venv/lib/python3.11/site-packages/report/__main__.py", line 236, in parse_request_xunit
    testsuite_arch = elem.xpath("./@name")[0].split(":")[1]
                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^
IndexError: list index out of range

Git blame points to

commit e121f683c28f99a7d071121852e8dcd4e08eed66
Author: Pavel Holica <pholica@redhat.com>
Date:   Tue Aug 29 09:37:06 2023 +0200

    Add --showarch option to report command
    
    This option adds new Arch column to the report table.

I believe it's time we introduced mandatory unit tests for basic apis and any new features.

@fernflower fernflower added the bug Something isn't working label Mar 20, 2024
Copy link

Hi, there.

Thank you @fernflower for submitting the bug report. Someone will take a look at this shortly.


This is an automated comment created by the peaceiris/actions-label-commenter. Responding to the bot or mentioning it won't have any effect.

@fernflower fernflower changed the title [BUG] [regression] Broken status report after e121f683c28f99a7d071121852e8dcd4e08eed66 merge [BUG] [regression] Broken status report for early failed requests after e121f683c28f99a7d071121852e8dcd4e08eed66 merge Mar 20, 2024
@danmyway
Copy link
Owner

Hi! After an initial investigation, this does not look like a regression.
What happens is that the request ended with complete state, but the overall "testsuite" result was an error.
We correctly fall back to the results.xml file, we just need to bail out, when the only result available are testsuite name - pipeline and testsuite overall_result is an error.
I have already drafted a fix for this, will provide in a PR soon.

@pholica
Copy link
Collaborator

pholica commented Mar 20, 2024

Yeah, I've just tried it before the commit and it also crashed before:

$ git checkout e121f683c28f99a7d071121852e8dcd4e08eed66^
...
$ PYTHONPATH=$(pwd)/src python3 -m tesar report -c 6b4ea828-52b9-4575-b4be-382b2b510b0a
Reporting for the requested tasks:
RHEL-8.8.0-Nightly None https://api.dev.testing-farm.io/v0.1/requests/6b4ea828-52b9-4575-b4be-382b2b510b0a COMPLETE
Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "/home/pholica/git/ddiblik/tesar/src/tesar/__main__.py", line 6, in <module>
    sys.exit(main())
             ^^^^^^
  File "/home/pholica/git/ddiblik/tesar/src/tesar/__init__.py", line 23, in main
    sys.exit(report())
             ^^^^^^^^
  File "/home/pholica/git/ddiblik/tesar/src/report/__main__.py", line 495, in main
    result_table = build_table_comparison() if ARGS.compare else build_table()
                                                                 ^^^^^^^^^^^^^
  File "/home/pholica/git/ddiblik/tesar/src/report/__main__.py", line 401, in build_table
    parsed_dict = parse_request_xunit(skip_pass=ARGS.skip_pass)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/pholica/git/ddiblik/tesar/src/report/__main__.py", line 232, in parse_request_xunit
    testsuite_result = elem.xpath("./@result")[0].upper()
                       ~~~~~~~~~~~~~~~~~~~~~~~^^^
IndexError: list index out of range

danmyway added a commit that referenced this issue Mar 20, 2024
* fixes #70
* bail out when the xml yields just one entry with name pipeline and
  overall result error

Signed-off-by: Daniel Diblik <ddiblik@redhat.com>
@danmyway
Copy link
Owner

fix available in #71 @fernflower

danmyway added a commit that referenced this issue Mar 21, 2024
* fixes #70
* bail out when the xml yields just one entry with name pipeline and
  overall result error
* try request overall result if xml unavailable

Signed-off-by: Daniel Diblik <ddiblik@redhat.com>
danmyway added a commit that referenced this issue Mar 25, 2024
* fixes #70
* bail out when the xml yields just one entry with name pipeline and
  overall result error
* try request overall result if xml unavailable

Signed-off-by: Daniel Diblik <ddiblik@redhat.com>
danmyway added a commit that referenced this issue Mar 25, 2024
* fixes #70
* bail out when the xml yields just one entry with name pipeline and
  overall result error
* try request overall result if xml unavailable

Signed-off-by: Daniel Diblik <ddiblik@redhat.com>
danmyway added a commit that referenced this issue Mar 25, 2024
* fixes #70
* bail out when the xml yields just one entry with name pipeline and
  overall result error
* try request overall result if xml unavailable

Signed-off-by: Daniel Diblik <ddiblik@redhat.com>
danmyway added a commit that referenced this issue Mar 25, 2024
* PoC leapp copr

* can find package and build the dict for leapp

Signed-off-by: Daniel Diblik <ddiblik@redhat.com>

* PoC workable state

* copr and brew build testing supported for leapp-repository, not
  documented

Signed-off-by: Daniel Diblik <ddiblik@redhat.com>

* Refactor report table generation

* Update issue templates (#29)

* Add github workflows (#30)

* add label commenter for issues and PR's

Signed-off-by: Daniel Diblik <ddiblik@redhat.com>

* Fix install (#32)

* Fix entrypoint, add main

* add main() function to tesar/__main__
* bumb version
* fix url in setup.py

Signed-off-by: Daniel Diblik <ddiblik@redhat.com>

* Fix packaging

- Instead of setup.py a setup.cfg and pyproject.toml have been
  introduced;
- Added __init__.py to the tesar module

Ideally I'd personally not have dispatch and report as separate
packages, but rather have them part of tesar module, but for
the moment let's make minimally invasive changes and just focus
on making tesar installable.

Closes: #27

---------

Signed-off-by: Daniel Diblik <ddiblik@redhat.com>
Co-authored-by: Daniel Diblik <ddiblik@redhat.com>

* hotfix/Fix not assigned variables (#34)

* source_release and target_release were not assigned for dispatching
  c2r test jobs

Signed-off-by: Daniel Diblik <ddiblik@redhat.com>

* Make -c option append test results (#26)

* Make -c option append test results

Instead of having -c accept a list of tests like '-c test1 test2 test3'
let's have a more natural way of '-c test1 -c test2 -c test3'.
Covered by a unit test.

Closes: #23

* Move code under src/ directory

- Change has been reflected in setup.cfg;
- setup.py introduced, removed pyproject.toml for now

OAMG-9529

* Some refactoring

- COMPOSE_MAPPING variable autoloading based on cli options
  upon dispatch module load removed. Now the code that needs
  access to COMPOSE_MAPPING will load it on-demand via
  dispatch.get_compose_mapping method.
- dispatch global variables have been moved out to dispatch_globals.py

OAMG-9529

* Make tests runnable by tox

Now a single `tox` command will build&run unit tests
under tests/ directory.

OAMG-9529

* Introduce a unit-tests github action

Shamelessly borrowed as is from the official docs

https://docs.github.com/en/actions/automating-builds-and-tests/

* Update changelog and readme

* Fix missing libkrb5 in unit-tests github action

This should help with failures upon package installation
in the tox env.

* Run main function in __main__ (#44)

Also move the main definition to __init__.
The purpose of the __main__ is to be executed when the module is
executed via python -m tesar.

* Bump targets in dispatch_globals

* bump OL latest target to 8.8
* update the distro context

Signed-off-by: Daniel Diblik <ddiblik@redhat.com>

* Bump to Alma and Rocky to 8.9

* bump targets, keep EUS (8.6, 8.8)

Signed-off-by: Daniel Diblik <ddiblik@redhat.com>

* Update Changelog and README

Signed-off-by: Daniel Diblik <ddiblik@redhat.com>

* Handle no target specified for leapp-repository

* When no target was specified for leapp-repository the utility failed
  to parse None type objects
* Handle the same way as for c2r, when no target specified dispatch for
  all mapped targets/upgrade paths
* add docstring

Fixes #36

Signed-off-by: Daniel Diblik <ddiblik@redhat.com>

* Treat overall result as single value

* Return code for report

* New option to skip passed results

* Don't create logs directory when the result is skipped

* Add debug messages to report command

Those debug messages also serve as comments describing
what's happening in the code.

* Add forgotten import

* Workaround missing xunit

This commit fixes 'tesar report' in case the json on api-dev.testing-farm.io
is missing the xml in the xunit field. The results are pulled from
artifacts/results.xml file.

* Introduce 2 test runs comparison mode

Currently 2 modes are possible (switch is controlled by --level2
argument) - show plans-list comparison or detailed tests-list
comparison.

Some additional arguments to the tesar report command
have been added:
* comparison is triggered with --compare flag
* you can use -u/--unify_results to combine several plan results
  for comparison. Can be useful if you are comparing 2 runs where
  tests have been renamed.

To try out comparison:
tesar report -c 9b44f3a9-dd92-46c4-9606-5396b1771753 -c 4e304b84-8ffb-49f2-8be9-1c3d8953ab03 --short --level2 --unify tier0_7to8=tier0 --compare

* Add docstrings to newly introduced functions

Also CHANGELOG and README have information about `tesar report --compare`
now.

* Hotfix update reval and continue if not xunit

* patch #57

Signed-off-by: Daniel Diblik <ddiblik@redhat.com>

* Add --showarch option to report command

This option adds new Arch column to the report table.

* Make --skip-pass respected by test comparison mode

Let's fix this slight oversight so that tesar report --compare
--skip-pass will be showing only problematic results.

* Add provision UEFI option (#62)

* `-u/--uefi` to request UEFI target if available

Signed-off-by: Daniel Diblik <ddiblik@redhat.com>

* Bump parallel limit, add centos stream (#67)

* bump the limit for plans run in parallel to 42
** add cli option to override the default
* add CentOS stream to targets

Signed-off-by: Daniel Diblik <ddiblik@redhat.com>

* Accept testfilter and/or planfilter with plan option (#64)

* Previously the filters were mutually exclusive, which is unusable if
  one needs to specify one test ouf of many inside a plan
* make the filters compatible
* if the plans argument has more than one value with additional filters, exit

Signed-off-by: Daniel Diblik <ddiblik@redhat.com>

* Warn if build non existent (#35)

* Refactor copr_api, modify nargs

* refactor get_info() in copr_api module
* add warning, when referenced build is not found in the query
* modify reference and build_id nargs to accept only one option
* warn and exit when build_id doesn't point to correct project/package
  and/or the state of the build is failed
* Add docstring, append changelog

Signed-off-by: Daniel Diblik <ddiblik@redhat.com>

---------

Signed-off-by: Daniel Diblik <ddiblik@redhat.com>

* Add specfile and packit.yaml (#68)

* Fix broken requirements
 * remove envparse listed as requirement, but not used anywhere
* Refactor dir hierarchy
* Push devel builds to the devel repository
* Fix imports
* fix tests
* changelog, readme modifications

Signed-off-by: Daniel Diblik <ddiblik@redhat.com>

* Fix IndexError with unexpected xml (#71)

* fixes #70
* bail out when the xml yields just one entry with name pipeline and
  overall result error
* try request overall result if xml unavailable

Signed-off-by: Daniel Diblik <ddiblik@redhat.com>

* Bumb version, changelog

Signed-off-by: Daniel Diblik <ddiblik@redhat.com>

---------

Signed-off-by: Daniel Diblik <ddiblik@redhat.com>
Co-authored-by: Pavel Holica <pholica@redhat.com>
Co-authored-by: ina vasilevskaya <inavasilevskaya@gmail.com>
Co-authored-by: ina vasilevskaya <ivasilev@redhat.com>
Co-authored-by: Michal Macura <mmacura@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants