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

TestHistoryMetrics.testEvents mobile pixel view flake #20338

Closed
cockpituous opened this issue Apr 21, 2024 · 5 comments · Fixed by #20426
Closed

TestHistoryMetrics.testEvents mobile pixel view flake #20338

cockpituous opened this issue Apr 21, 2024 · 5 comments · Fixed by #20426
Assignees
@martinpitt
Labels
flake unstable test nightly

Comments

@cockpituous
Copy link
Contributor

cockpituous commented Apr 21, 2024 •
edited by martinpitt

The job fedora-39/daily failed on commit 61e5add.

Log: https://cockpit-logs.us-east-1.linodeobjects.com/pull-0-61e5addb-20240421-012835-fedora-39-daily/log.html

example failure
@martinpitt martinpitt changed the title Tests failed on 61e5addb28d7d9ad8cc4d5c4ec4a5e2bac248b64 TestHistoryMetrics.testEvents mobile pixel view flake Apr 22, 2024
@martinpitt
Copy link
Member

This is becoming too annoying -- the weather report confirms this as second-highest flake with a 34 to 50% failure rate. Pilot board yellow.

@martinpitt martinpitt added the flake unstable test label Apr 22, 2024
@martinpitt martinpitt mentioned this issue Apr 24, 2024
Closed
@jelly jelly mentioned this issue May 2, 2024
Closed
@jelly
Copy link
Member

jelly commented May 2, 2024

This happens daily now, yesterday and today

@allisonkarlitskaya
Copy link
Member

allisonkarlitskaya commented May 2, 2024 •
edited

I just hit this in #20411

https://cockpit-logs.us-east-1.linodeobjects.com/pull-20411-b119488b-20240502-110637-fedora-40-other/log.html#30

This was referenced May 3, 2024
Closed
Closed
@martinpitt
Copy link
Member

Promoting to urgent. This breaks pretty much every PR now. I retried #20421 four times in vain.

@martinpitt martinpitt self-assigned this May 3, 2024
@martinpitt martinpitt mentioned this issue May 3, 2024
Merged
@martinpitt
Copy link
Member

I ran TZ=Etc/UTC test/verify/check-metrics TestHistoryMetrics.testEvents in three parallel loops locally, and that doesn't reproduce it. So this is highly timing dependant, and presumably coincides with our loss of e2e machines (i.e. prone to happen on rhos machines). Note that the "UTC" is important, otherwise the test immediately fails due to a different local time zone (two hours off for me in CEST).

I tested this interactively both in Firefox mobile mode (Ctrl+Shift+M), and in Chromium with TEST_SHOW_BROWSER=pixels. This works just fine, so this brokenness is really just a test artifact, not an actual bug on the page. The .scrollIntoViewIfNeeded() call somehow messes up the rendering of the menu bar. The function does wait for animations, plus an extra 0.5 seconds.

This has been ignored by pilots for two weeks, we are too busy, and is a very deep rabbit hole with a very low benefit/effort ratio. @mvollmer already attempted this in commits e700960 and 790b377. So the pragmatic solution is to just stop the mobile test for this.

martinpitt added a commit to martinpitt/cockpit that referenced this issue May 3, 2024
@martinpitt
test: Drop mobile pixel tests in TestHistoryMetrics.testEvents
2e072c6 
The mobile pixel test is way too flaky (> 50% failure rate on our CI).
scrollIntoViewIfNeeded() somehow misplaces the menu bar. This isn't a
real problem, an interactive Firefox/Chromium works fine.

Nobody has figured this out in two weeks, earlier attempts at fixing
that (commits e700960 and 790b377) also weren't effective. Figuring this
out is too much effort for too little benefit, and this flake breaks
pretty much every PR. So disable the mobile pixel test.

Fixes cockpit-project#20338
@martinpitt martinpitt mentioned this issue May 3, 2024
Merged
mvollmer pushed a commit that referenced this issue May 3, 2024
@martinpitt @mvollmer
test: Drop mobile pixel tests in TestHistoryMetrics.testEvents
12eb2c7 
The mobile pixel test is way too flaky (> 50% failure rate on our CI).
scrollIntoViewIfNeeded() somehow misplaces the menu bar. This isn't a
real problem, an interactive Firefox/Chromium works fine.

Nobody has figured this out in two weeks, earlier attempts at fixing
that (commits e700960 and 790b377) also weren't effective. Figuring this
out is too much effort for too little benefit, and this flake breaks
pretty much every PR. So disable the mobile pixel test.

Fixes #20338
@martinpitt martinpitt mentioned this issue May 3, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@martinpitt martinpitt

Labels
flake unstable test nightly
Projects
Pilot tasks
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

test: Drop mobile pixel tests in TestHistoryMetrics.testEvents martinpitt/cockpit
4 participants
@jelly @martinpitt @cockpituous @allisonkarlitskaya