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

fix disabled button check #1831

Merged
merged 1 commit into from
Aug 20, 2024
Merged

Conversation

tomasmatus
Copy link
Member

@tomasmatus tomasmatus commented Aug 19, 2024

Test checks if a warning is displayed therefore create button should be disabled.

https://cockpit-logs.us-east-1.linodeobjects.com/pull-1821-bfa093ee-20240819-082934-debian-testing/log.html#10-2

@tomasmatus
Copy link
Member Author

b.click("#containers-images tr:contains('my-busybox') td.pf-v5-c-table__toggle button")
b.click(f"#containers-images tr:contains('{second_tag}') td.pf-v5-c-table__toggle button")
Copy link
Member

Choose a reason for hiding this comment

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

This isn't a "fix" in the sense that the old behaviour was wrong. The original code checks that adding a second tag to an existing image correctly gets picked up by the UI and shown as "additional tag" names in the original image info.

You are welcome to add code which checks that the new tag also appears as an image by itself (and then also shows my-busybox as alternative tag name), but let's please keep that check.

Copy link
Member Author

Choose a reason for hiding this comment

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

I see, my bad. I'll revert this then

b.wait_visible(".pf-v5-c-modal-box__footer #create-image-create-run-btn:not(:disabled)")
b.wait_visible(".pf-v5-c-modal-box__footer #create-image-create-run-btn:disabled")
Copy link
Member

Choose a reason for hiding this comment

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

The old code indeed looks wrong. However, two lines down the test already waits for the button to be aria-disabled, which looks redundant. One of these two is enough?

@tomasmatus tomasmatus changed the title test fixes fix disabled button check Aug 20, 2024
Copy link
Member

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

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

Cheers!

@martinpitt martinpitt merged commit f236b6f into cockpit-project:main Aug 20, 2024
29 of 31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants