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.

Sign up for GitHub

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

Jump to bottom

Refactor and fix of some Errata UI tests #14773

Closed
wants to merge 2 commits into from
Closed

Refactor and fix of some Errata UI tests #14773

wants to merge 2 commits into from

Conversation

LadislavVasina1
Copy link
Contributor

Problem Statement

I have fixed some failing Errata UI tests.
Tests were mainly failing because they were trying to test the old, no longer-used content_host page and used old fixtures.

Solution

After a discussion with @vsedmik I have rewritten the test that I have fixed so they test the similar/same thing on the new host page on Content-Errata Tab.
To reflect the changes made in test I have renamed the test in the following way

  • test_positive_content_host_library -> test_positive_check_errata
  • test_positive_content_host_search_type -> test_positive_errata_search_type
  • test_positive_show_count_on_content_host_details_page -> test_positive_check_errata_counts_by_type_on_host_details_page

Needs SatelliteQE/airgun#1337

@LadislavVasina1 LadislavVasina1 added UI Issues and PRs involving the UI CherryPick PR needs CherryPick to previous branches AutoMerge_Cherry_Picked The cherrypicked PRs of master PR would be automerged if all checks passing 6.13.z Introduced in or relating directly to Satellite 6.13 6.14.z Introduced in or relating directly to Satellite 6.14 6.15.z Introduced in or relating directly to Satellite 6.15 labels Apr 15, 2024
@LadislavVasina1 LadislavVasina1 requested a review from vsedmik April 15, 2024 08:49
@LadislavVasina1 LadislavVasina1 self-assigned this Apr 15, 2024
@LadislavVasina1 LadislavVasina1 requested a review from a team as a code owner April 15, 2024 08:49
@LadislavVasina1
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/ui/test_errata.py -k 'test_positive_check_errata'
airgun: 1337

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 6498
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/ui/test_errata.py -k test_positive_check_errata --external-logging
Test Result : ========== 2 passed, 14 deselected, 86 warnings in 1095.45s (0:18:15) ==========

@Satellite-QE Satellite-QE added the PRT-Passed Indicates that latest PRT run is passed for the PR label Apr 15, 2024
@LadislavVasina1
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/ui/test_errata.py -k 'test_positive_errata_search_type'
airgun: 1337

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 6499
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/ui/test_errata.py -k test_positive_errata_search_type --external-logging
Test Result : ========== 1 passed, 15 deselected, 28 warnings in 867.10s (0:14:27) ===========

@LadislavVasina1
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/ui/test_errata.py -k 'test_positive_check_errata_counts_by_type_on_host_details_page'
airgun: 1337

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 6500
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/ui/test_errata.py -k test_positive_check_errata_counts_by_type_on_host_details_page --external-logging
Test Result : ========== 1 passed, 15 deselected, 27 warnings in 887.28s (0:14:47) ===========

vsedmik
vsedmik approved these changes Apr 15, 2024
View reviewed changes
Copy link
Contributor

@vsedmik vsedmik left a comment

Choose a reason for hiding this comment

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

Few nitpicks left, otherwise looks good to me. 👍

tests/foreman/ui/test_errata.py Outdated Show resolved Hide resolved
@Satellite-QE Satellite-QE removed the PRT-Passed Indicates that latest PRT run is passed for the PR label Apr 15, 2024
vijaysawant
vijaysawant approved these changes Apr 15, 2024
View reviewed changes
Copy link
Contributor

@vijaysawant vijaysawant 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 to me
I am interested in airgun suggested changes, if the suggestion works locally/in CI then fine, otherwise keep this changes in as it is.

tests/foreman/ui/test_errata.py Show resolved Hide resolved
@damoore044
Copy link
Contributor

trigger: test-robottelo
pytest: tests/foreman/ui/test_errata.py -k 'test_positive_check_errata or test_positive_errata_search_type or test_positive_check_errata_counts_by_type_on_host_details_page'
airgun: 1337

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 6524
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/ui/test_errata.py -k test_positive_check_errata or test_positive_errata_search_type or test_positive_check_errata_counts_by_type_on_host_details_page --external-logging
Test Result : ===== 2 failed, 13 deselected, 85 warnings, 1 error in 2187.77s (0:36:27) ======

@Satellite-QE Satellite-QE added the PRT-Failed Indicates that latest PRT run is failed for the PR label Apr 16, 2024
@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 79
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/ui/test_errata.py -k test_positive_check_errata or test_positive_errata_search_type or test_positive_check_errata_counts_by_type_on_host_details_page --external-logging
Test Result : ===== 2 failed, 13 deselected, 83 warnings, 1 error in 2108.64s (0:35:08) ======

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 6530
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/ui/test_errata.py -k test_positive_check_errata or test_positive_errata_search_type or test_positive_check_errata_counts_by_type_on_host_details_page --external-logging
Test Result : ===== 2 failed, 13 deselected, 84 warnings, 1 error in 2124.28s (0:35:24) ======

@vijaysawant
Copy link
Contributor

trigger: test-robottelo
pytest: tests/foreman/ui/test_errata.py -k 'test_positive_check_errata or test_positive_errata_search_type or test_positive_check_errata_counts_by_type_on_host_details_page'
airgun: 1337

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 6535
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/ui/test_errata.py -k test_positive_check_errata or test_positive_errata_search_type or test_positive_check_errata_counts_by_type_on_host_details_page --external-logging
Test Result : ===== 2 failed, 13 deselected, 84 warnings, 1 error in 2133.53s (0:35:33) ======

ogajduse
ogajduse requested changes Apr 17, 2024
View reviewed changes
Copy link
Member

@ogajduse ogajduse left a comment

Choose a reason for hiding this comment

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

co-reviewed with @jameerpathan111

One small change requested.

tests/foreman/ui/test_errata.py
for errata_type in ('security', 'bug_fix', 'enhancement'):
assert int(content_host_values['details'][errata_type]) == 0
read_errata = session.host_new.get_details(hostname, 'Content.Errata')
assert int(len(read_errata['Content']['Errata']['pagination'])) == 0
Copy link
Member

Choose a reason for hiding this comment

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

What is this assert supposed to assert/test? Can you please comment on this assertion?

Also, do we need to convert the result of len to int?

After going through the whole test function I see what this assertion is good for, but a comment would not hurt. 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

@ogajduse I've made a follow-up PR pulling in Ladislav's fixes, and an update to fixtures for combined sessions.
I also addressed this request for a comment clarifying reason for initial read & assertion. Let me know what you think! #14822

@jameerpathan111 jameerpathan111 self-requested a review April 17, 2024 09:15
jameerpathan111
jameerpathan111 approved these changes Apr 17, 2024
View reviewed changes
Copy link
Contributor

@jameerpathan111 jameerpathan111 left a comment

Choose a reason for hiding this comment

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

Ack. Merge pending actions from @ogajduse's comment.

@vsedmik
Copy link
Contributor

vsedmik commented Apr 17, 2024

trigger: test-robottelo
pytest: tests/foreman/ui/test_errata.py -k 'test_positive_check_errata or test_positive_errata_search_type or test_positive_check_errata_counts_by_type_on_host_details_page'
airgun: 1337

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 6576
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/ui/test_errata.py -k test_positive_check_errata or test_positive_errata_search_type or test_positive_check_errata_counts_by_type_on_host_details_page --external-logging
Test Result : ===== 2 passed, 13 deselected, 91 warnings, 1 error in 1104.40s (0:18:24) ======

@damoore044
Copy link
Contributor

6.16.0 stream 54: Local result running the 3 modified tests, combined in one session -

(venvrobottelo): robottelo $ pytest -s tests/foreman/ui/test_errata.py -k 'test_positive_check_errata or test_positive_errata_search_type or test_positive_check_errata_counts_by_type_on_host_details_page'
2024-04-17 09:40:59 - robottelo - WARNING - Dynaconf validation failed, continuing for the sake of unit tests
    certs.cert_file is required in env main
======================================================== test session starts =========================================================
platform darwin -- Python 3.12.2, pytest-8.1.1, pluggy-1.4.0
. . .
collecting ... 2024-04-17 09:41:01 - robottelo.collection - INFO - Processing test items to add testimony token markers
collected 17 items / 14 deselected / 3 selected
. . .
===================================== 3 passed, 14 deselected, 94 warnings in 1104.39s (0:18:24) =====================================

@damoore044
Copy link
Contributor

trigger: test-robottelo
pytest: tests/foreman/ui/test_errata.py::test_positive_check_errata_counts_by_type_on_host_details_page
airgun: 1337

@damoore044 damoore044 mentioned this pull request Apr 17, 2024
Merged
@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 6579
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/ui/test_errata.py::test_positive_check_errata_counts_by_type_on_host_details_page --external-logging
Test Result : ================== 1 passed, 12 warnings in 855.56s (0:14:15) ==================

@Satellite-QE Satellite-QE added PRT-Passed Indicates that latest PRT run is passed for the PR and removed PRT-Failed Indicates that latest PRT run is failed for the PR labels Apr 17, 2024
@Satellite-QE Satellite-QE mentioned this pull request May 1, 2024
Merged
@damoore044
Copy link
Contributor

the excellent ErrataUI changes here, were squashed and included in MR #14822.
The linked pr also addressed a requested change from here, and fixed CI/PRT issues seen in combined sessions for local fixture registered_contenthost.
Great work @LadislavVasina1 !

@LadislavVasina1
Copy link
Contributor Author

Thank you very much @damoore044 for dealing with that while I am absent. 👍🏻

@damoore044 damoore044 closed this Jun 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@damoore044 damoore044 damoore044 left review comments

@vijaysawant vijaysawant vijaysawant approved these changes

@ogajduse ogajduse ogajduse requested changes

@vsedmik vsedmik vsedmik approved these changes

@jameerpathan111 jameerpathan111 jameerpathan111 approved these changes

Assignees

@LadislavVasina1 LadislavVasina1

Labels
6.13.z Introduced in or relating directly to Satellite 6.13 6.14.z Introduced in or relating directly to Satellite 6.14 6.15.z Introduced in or relating directly to Satellite 6.15 AutoMerge_Cherry_Picked The cherrypicked PRs of master PR would be automerged if all checks passing CherryPick PR needs CherryPick to previous branches PRT-Passed Indicates that latest PRT run is passed for the PR UI Issues and PRs involving the UI
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@LadislavVasina1 @Satellite-QE @damoore044 @vijaysawant @vsedmik @ogajduse @jameerpathan111