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

pf4 bookmarks dropdown #1200

Merged
merged 1 commit into from
Feb 6, 2024
Merged

pf4 bookmarks dropdown #1200

merged 1 commit into from
Feb 6, 2024

Conversation

pondrejk
Copy link
Contributor

This change fixes around 90 tests in the bookmark component. The locator for bookmarks dropdown was not accurate for pf4 searchboxes, plus views were not updated to use pf4 search. The remaining legacy searchboxes continue to work.

Test result for component with pf4 searcbox:

 pytest tests/foreman/ui/test_bookmarks.py -k  test_positive_end_to_end[Architecture]
=============================================== test session starts ===============================================                                                               

tests/foreman/ui/test_bookmarks.py .

Page with legacy searchbox:


pytest tests/foreman/ui/test_bookmarks.py -k  test_positive_end_to_end[product]
=============================================== test session starts ===============================================                                                               

tests/foreman/ui/test_bookmarks.py .

@pondrejk pondrejk added Automation failure Makes Robottelo automation fail CherryPick PR needs CherryPick to previous branches 6.13.z AutoMerge_Cherry_Picked Automatically merge the PR is PRT and all checks are passing 6.14.z 6.15.z labels Jan 25, 2024
@pondrejk pondrejk self-assigned this Jan 25, 2024
@pondrejk pondrejk force-pushed the bookmark-search-pf4 branch 2 times, most recently from 72640df to e11f466 Compare January 25, 2024 10:49
omkarkhatavkar
omkarkhatavkar suggested changes Jan 30, 2024
View reviewed changes
Copy link

@omkarkhatavkar omkarkhatavkar left a comment

Choose a reason for hiding this comment

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

I think better to avoid adding extra variable is all are those coming from the SearchableViewMixinPF4. Apart from that looks solid

airgun/views/ansible_variable.py Outdated Show resolved Hide resolved
airgun/views/dashboard.py
@@ -75,7 +75,7 @@ class DashboardView(BaseLoggedInView):
title = Text("//h1[normalize-space(.)='Overview']")
Copy link

Choose a reason for hiding this comment

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

Good to add SearchableViewMixinPF4 class inheritance

airgun/views/discoveryrule.py Outdated Show resolved Hide resolved
@pondrejk
Copy link
Contributor Author

thanks @omkarkhatavkar, I didn't realize there is inheritance, updated

@omkarkhatavkar
Copy link

trigger: test-robottelo
pytest: tests/foreman/ui/test_bookmarks.py -k test_positive_end_to_end

3 similar comments
@omkarkhatavkar
Copy link

trigger: test-robottelo
pytest: tests/foreman/ui/test_bookmarks.py -k test_positive_end_to_end

@pondrejk
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/ui/test_bookmarks.py -k test_positive_end_to_end

@omkarkhatavkar
Copy link

trigger: test-robottelo
pytest: tests/foreman/ui/test_bookmarks.py -k test_positive_end_to_end

@omkarkhatavkar
Copy link

@pondrejk Almost everything looks okay, there 1-2 cases missing for search. but other failures seem are unrelated to search

@pondrejk
Copy link
Contributor Author

pondrejk commented Feb 6, 2024

@omkarkhatavkar thank you, I updated the package view that shouldn't have been changed, the other failures need to be fixed on the robottelo side, I'll make a pr soon

@omkarkhatavkar
Copy link

@pondrejk I don't want to wait more time for 1 more reviewer. This looks good I will merge your PR. We need to have more participation from reviewers.

@omkarkhatavkar omkarkhatavkar merged commit 39f73c6 into SatelliteQE:master Feb 6, 2024
5 checks passed
github-actions bot pushed a commit that referenced this pull request Feb 6, 2024
@pondrejk @web-flow
pf4 bookmarks dropdown (#1200)
4773988 
(cherry picked from commit 39f73c6)
This was referenced Feb 6, 2024
Merged
Closed
Closed
omkarkhatavkar pushed a commit that referenced this pull request Feb 16, 2024
@Satellite-QE @pondrejk
pf4 bookmarks dropdown (#1200) (#1228)
842a66e 
(cherry picked from commit 39f73c6)

Co-authored-by: Peter Ondrejka <[email protected]>
This was referenced Feb 28, 2024
Merged
Merged
Merged
dosas
dosas reviewed Mar 19, 2024
View reviewed changes
airgun/views/repository.py
@@ -22,7 +26,7 @@
)


class RepositoriesView(BaseLoggedInView, SearchableViewMixin):
class RepositoriesView(BaseLoggedInView, SearchableViewMixinPF4):
Copy link
Collaborator

Choose a reason for hiding this comment

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

@pondrejk Is foreman 3.9 (katello 4.11.1) (6.15.z) really using this? For me this breaks some of the following tests:

tests/foreman/ui/test_repository.py

This was referenced Mar 20, 2024
Merged
Merged
Merged
@vsedmik vsedmik mentioned this pull request Apr 5, 2024
Merged
This was referenced Apr 8, 2024
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dosas dosas dosas left review comments

@omkarkhatavkar omkarkhatavkar omkarkhatavkar approved these changes

Assignees

@pondrejk pondrejk

Labels
6.13.z 6.14.z 6.15.z Automation failure Makes Robottelo automation fail AutoMerge_Cherry_Picked Automatically merge the PR is PRT and all checks are passing CherryPick PR needs CherryPick to previous branches
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pondrejk @omkarkhatavkar @dosas