Fix chrome 145 web tests - #12486
Conversation
…erences_tool out of non-vm testing
d0e535b to
ac64fbe
Compare
|
Looks like you're running into the same crash/timeout issues I am now. |
| - file_selector | ||
| # Waiting on https://github.com/flutter/flutter/issues/145149 | ||
| - google_maps_flutter/google_maps_flutter | ||
| # Moved to its own dedicated shard due to long test duration |
There was a problem hiding this comment.
I'm very skeptical that long test duration is the problem here. I looked at the presubmit run before you added this commit, and found this header in the log:
|| Running for packages/camera/camera_web [@1:59]
That means it started running camera_web tests two minutes into the run, and then timed out 58 minutes later. It seems much more likely to me that the camera_web tests are hanging than that they take exactly 59 minutes to successfully run. (Also, they only took 4.5 minutes the last time I see them succeeding in a roll.)
If we have all the tests except this working, and this one is still hanging for reasons we don't understand yet, we should just disable this test entirely with a link to an issue tracking investigating, fixing, and re-enabling the test.
There was a problem hiding this comment.
The timeout being triggered here is the default integrationDriver() timeout of 20 minutes.
You can see that this suite starts at:
============================================================
|| Running for packages/camera/camera_web [@0:14]
============================================================
And the next one starts at:
============================================================
|| Running for packages/camera/camera_windows [@22:32]
============================================================
There was a problem hiding this comment.
Hm, I was looking at a different run I guess. The one I saw was a full task timeout.
Do you see it running for more than 20 minutes locally? It seems odd that updating to a newer Chrome would make a test that use to run in 4-5 minutes take more than 20 if it's actually running correctly.
There was a problem hiding this comment.
In older Chrome versions, it had support for two headless modes: --headless=old and --headless=new. The default for --headless was "old" to maintain backwards compatibility, I presume.
Chrome 128 switched the default for --headless to be "new". And later in 132 they completely removed the "old" mode.
It's my understanding that the "old" headless mode used to run a lighter weight binary, and the "new" one runs a full Chrome browser.
There was a problem hiding this comment.
Per my other comment, I think these files and the new shards should be reverted.
There was a problem hiding this comment.
(Also, keeping the new shard would cause every PR to unconditionally run camera_web tests no matter what was being touched, which combined with the fact that it's presumably going to consume a machine for 60 minutes every time would be extremely wasteful of CI resources.)
There was a problem hiding this comment.
I'll revert the shards and instead increase the integrationDriver() timeout or split camera_web_test.dart into multiple files (I think the timeout is per file).
| # TODO(mdebbar): Upgrade to Chrome 145 once Flutter stable includes the | ||
| # upstream flutter_tools fixes for Chrome 145 (flutter/flutter#190155 and | ||
| # flutter/flutter#190584) which pass --disable-search-engine-choice-screen | ||
| # and --disable-background-timer-throttling. |
There was a problem hiding this comment.
Let's get (and link to) an issue for this with packages and p: waiting for stable update so we find it when doing the next stable update.
tarrinneal
left a comment
tarrinneal
left a comment
There was a problem hiding this comment.
lgtm, I'll be out for a bit, but if you can land it that would be great! If you need me to re-review due to significant changes, feel free to ping me and I can later today.
3 participants
Fixes flutter/flutter#191149