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

Vertical menu search test #13421

Merged
merged 1 commit into from
Feb 14, 2024
Merged

Vertical menu search test #13421

merged 1 commit into from
Feb 14, 2024

Conversation

pnovotny
Copy link
Contributor

New module and test case for vertical menu search functionality.

TC: test_positive_vertical_navigation_search_end_to_end

Requires: SatelliteQE/airgun#1076

@pnovotny pnovotny added CherryPick PR needs CherryPick to previous branches AutoMerge_Cherry_Picked The cherrypicked PRs of master PR would be automerged if all checks passing 6.15.z Introduced in or relating directly to Satellite 6.15 labels Dec 13, 2023
@pnovotny pnovotny self-assigned this Dec 13, 2023
@pnovotny pnovotny requested review from pondrejk, lhellebr and a team December 13, 2023 07:51
@pnovotny
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/ui/test_search.py -k test_positive_vertical_navigation_search_end_to_end
airgun: 1076

2 similar comments
@pnovotny
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/ui/test_search.py -k test_positive_vertical_navigation_search_end_to_end
airgun: 1076

@pnovotny
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/ui/test_search.py -k test_positive_vertical_navigation_search_end_to_end
airgun: 1076

@pnovotny
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/ui/test_search.py -k test_positive_vertical_navigation_search_end_to_end
airgun: 1076
env:
ROBOTTELO_ui__record_video: true

1 similar comment
@pnovotny
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/ui/test_search.py -k test_positive_vertical_navigation_search_end_to_end
airgun: 1076
env:
ROBOTTELO_ui__record_video: true

@pnovotny pnovotny marked this pull request as ready for review December 15, 2023 19:03
@pnovotny pnovotny mentioned this pull request Dec 15, 2023
Merged
lhellebr
lhellebr reviewed Jan 11, 2024
View reviewed changes
Copy link
Contributor

@lhellebr lhellebr left a comment

Choose a reason for hiding this comment

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

Overally good but some comments left

@adamruzicka
Copy link

@pnovotny Gentle ping

@pnovotny
Vertical menu search test
04edbe1 
New test for vertical navigation menu search functionality.
@pnovotny
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/ui/test_search.py -k test_positive_vertical_navigation_search_end_to_end

@pnovotny
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/ui/test_search.py -k test_positive_vertical_navigation_search_end_to_end
env:
ROBOTTELO_ui__record_video: true

@lvrtelov
Copy link
Contributor

lvrtelov commented Feb 6, 2024

@pnovotny can we get this merge soon? it's open for a while now

@pnovotny
Copy link
Contributor Author

pnovotny commented Feb 7, 2024

I'm working on an airgun PR that fixes the PRT failure, because of funky behavior of the search input widget.
I'll link it here once ready, it's basically a trial-and-error approach. The ETA is tomorrow, EOW tops.

@pnovotny
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/ui/test_search.py -k test_positive_vertical_navigation_search_end_to_end
airgun: 1233
env:
ROBOTTELO_ui__record_video: true

@pnovotny pnovotny mentioned this pull request Feb 13, 2024
Merged
@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 5702
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/ui/test_search.py -k test_positive_vertical_navigation_search_end_to_end
Test Result : ================= 2 passed, 33 warnings in 1047.12s (0:17:27) ==================

@Satellite-QE Satellite-QE added the PRT-Passed Indicates that latest PRT run is passed for the PR label Feb 13, 2024
Griffin-Sullivan
Griffin-Sullivan approved these changes Feb 14, 2024
View reviewed changes
tests/foreman/ui/test_search.py
), f'Search string {search_string} does not match result {result}!'


def menu_search_should_not_find(search_func: callable, values: list[str]):
Copy link
Contributor

Choose a reason for hiding this comment

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

not a blocker but I'm curious if running the above function with not in front of it would achieve the same result?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd agree, there's probably a way to do this without a separate helper. Doesn't really affect the test overall though, more of a style difference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I get that the first three lines of the two functions are pretty similar, but merging both functions together to one and adding some "bool" argument, i.e., expect_to_find=True/False, would lead to more if..else branching and thus to less readability, IMO. Esp. comparing to how many lines of code we save by that.

Using not menu_search_should_find() here won't help, because menu_search_should_find() doesn't return bool, but does asserts instead - so a test failure is easily identifiable in the results.
Doing something like this

with pytest.raises(AssertionError):
    menu_search_should_find()

would solve it, but then there is a case that AssertionError is thrown by the second assert (line 31) and that would be wrong.

@Griffin-Sullivan Griffin-Sullivan merged commit 8f6ce39 into SatelliteQE:master Feb 14, 2024
7 checks passed
github-actions bot pushed a commit that referenced this pull request Feb 14, 2024
@pnovotny @web-flow
Vertical menu search test (#13421)
d85f1e6 
New test for vertical navigation menu search functionality.

(cherry picked from commit 8f6ce39)
@Satellite-QE Satellite-QE mentioned this pull request Feb 14, 2024
Merged
@pnovotny
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/ui/test_search.py -k test_positive_vertical_navigation_search_end_to_end
airgun: 1250

@pnovotny pnovotny deleted the navigation-search branch February 20, 2024 14:56
shweta83 pushed a commit to shweta83/robottelo that referenced this pull request Apr 10, 2024
@pnovotny @shweta83
Vertical menu search test (SatelliteQE#13421)
012762b 
New test for vertical navigation menu search functionality.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lhellebr lhellebr lhellebr left review comments

@sambible sambible sambible approved these changes

@pondrejk pondrejk pondrejk approved these changes

@Griffin-Sullivan Griffin-Sullivan Griffin-Sullivan approved these changes

Assignees

@pnovotny pnovotny

Labels
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
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@pnovotny @adamruzicka @lvrtelov @Satellite-QE @lhellebr @pondrejk @sambible @Griffin-Sullivan