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

cockpit-ci: Update container to 2025-01-18 #911

Closed
wants to merge 1 commit into from
Closed

cockpit-ci: Update container to 2025-01-18 #911

wants to merge 1 commit into from

Conversation

github-actions[bot]
Copy link
Contributor

No description provided.

@github-actions github-actions bot added the bot label Jan 20, 2025
@github-actions github-actions bot changed the title [no-test] cockpit-ci: Update container to 2025-01-18 cockpit-ci: Update container to 2025-01-18 Jan 20, 2025
@github-actions github-actions bot force-pushed the tasks-container-update-cockpit-ci-container-20250120-025025 branch from 770c197 to bfcad0e Compare January 20, 2025 02:50
@martinpitt
Copy link
Member

Urgh, what? that's hopefully not a browser regression. Running again to compare. If that is true, we have a major bug.

@martinpitt
Copy link
Member

second run has one case fixed, but the disappearing "Change" button is persistent. On to the pilot board.

@martinpitt martinpitt self-assigned this Jan 20, 2025
@martinpitt
Copy link
Member

This effortlessly reproduces locally (fortunately I already have that new container locally, so train material).

  • It's suspicious that this only happens in the mobile view. With interactive firefox that is fine, I can resize or switch to mobile emulation and the change button is always visible.
  • It still fails with wait_after_layout_change=True, and still with an additional wait_delay=3.
  • It doesn't happen with interactive chromium with TEST_SHOW_BROWSER=pixels. When I change webdriver_bidi.py to force using /usr/bin/chromium-browser over headless_shell, I get similar noise as with interactive chromium, but also not the missing "Change" button.

So in summary, this is a chromium headless bug.

@martinpitt
Copy link
Member

This is the second headless bug after cockpit-project/cockpit@0ec830d that I remember. Replacing chromium-headless with chromium in our tasks container will result in a net 100 MB increase, but avoid these two bugs, and also provide us with a reproducible TEST_SHOW_BROWSER=pixels experience (see previous comment, headless and full change all tests slightly due to font rendering).

So I propose to change our tasks container accordingly, suffer through the mass pixel change once, but from then on our lives will be a bit easier. @jelly WDYT?

martinpitt added a commit to cockpit-project/cockpituous that referenced this pull request Jan 20, 2025
@martinpitt
tasks: Install full chromium
bb0addd 
The headless version has several bugs, at least [1] and [2]. Also,
headless and full Chromium render texts slightly differently, so we
can't debug/fix pixel tests with `TEST_SHOW_BROWSER=pixels`.

This is a net 100 MB size increase of the tasks container, and will
break all pixel tests once, but long-term it is a better browser.

[1] https://issues.redhat.com/browse/COCKPIT-1159
[2] cockpit-project/cockpit-files#911 (comment)
@martinpitt martinpitt mentioned this pull request Jan 20, 2025
Merged
martinpitt
martinpitt requested changes Jan 20, 2025
View reviewed changes
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.

Let's not do this, and instead do cockpit-project/cockpituous#637

@martinpitt martinpitt closed this Jan 20, 2025
@martinpitt martinpitt deleted the tasks-container-update-cockpit-ci-container-20250120-025025 branch January 20, 2025 11:11
@martinpitt martinpitt mentioned this pull request Jan 20, 2025
Merged
martinpitt added a commit to cockpit-project/cockpituous that referenced this pull request Jan 20, 2025
@martinpitt
tasks: Install full chromium
07b3c11 
The headless version has several bugs, at least [1] and [2]. Also,
headless and full Chromium render texts slightly differently, so we
can't debug/fix pixel tests with `TEST_SHOW_BROWSER=pixels`.

This is a net 100 MB size increase of the tasks container, and will
break all pixel tests once, but long-term it is a better browser.

[1] https://issues.redhat.com/browse/COCKPIT-1159
[2] cockpit-project/cockpit-files#911 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martinpitt martinpitt martinpitt requested changes

Assignees

@martinpitt martinpitt

Labels
bot
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@martinpitt @cockpituous