tests(smoke): reenable oopif smokes with ToT #14153
Conversation
.github/workflows/package-test.yml
Outdated
| - name: Define ToT chrome path | ||
| run: echo "CHROME_PATH=/home/runner/chrome-linux-tot/chrome" >> $GITHUB_ENV | ||
|
|
||
| - name: Install Chrome ToT |
There was a problem hiding this comment.
should this (and the other cases) include a TODO to eventually move back to stable?
Also, what's left testing stable?
There was a problem hiding this comment.
- Bundled Lighthouse (Legacy navigation)
- Package test (Legacy navigation)
- Chrome stable smokes from the standard matrix
- Windows
There was a problem hiding this comment.
should this (and the other cases) include a TODO to eventually move back to stable?
TBH I'm fine shifting everything (except our explicitly Chrome stable smokes) to use ToT
|
Just a DevTools / e2e failure, nobody panic |
|
Welp This was the lone failure on
|
| @@ -41,7 +41,7 @@ class Driver { | |||
| * @private | |||
| * Used to save network and lifecycle protocol traffic. Just Page and Network are needed. | |||
| */ | |||
| _devtoolsLog = new DevtoolsMessageLog(/^(Page|Network)\./); | |||
There was a problem hiding this comment.
@paulirish was this intentionally left out of #14080?
There was a problem hiding this comment.
This was just for debugging. Can you open a new PR?
I'm thinking it was a merge mistake that dropped this.
There was a problem hiding this comment.
yeah im pretty sure i messed up a conflict resolution merge. thx for the driveby
Trying to piece together oopif-requests output in this log makes my brain hurt :) |
FYI you can link to an exact line. https://github.com/GoogleChrome/lighthouse/runs/7032720874?check_suite_focus=true#step:12:203 It says a YT .js is missing, but... I do see other YT resources. so, is the expectation just wrong?
FWIW 5 samples is not significant. Careful not to represent changes as improvements when they could just be statistical anomalies–for example, if this conclusion is incorrect it could be misleading to tell caseq "it helped .... a bit" when it did no such thing. Or was it failing 100% of the time before? I can't keep track any more. in that case maybe it is significant :P |
|
So no failures for oopif-scripts? |
Yeah I'm not sure. The missing YT .js isn't referenced anywhere in the failing DT log or trace, so maybe the YT embed simply didn't fetch it for some reason?
The FWIW each "run" contains multiple points of failure (i.e. Smoke FR, Smoke Bundled FR, package FR, DT smoke * 3 flaky smokes * 5 runs). So there were ~50-60 problematic smoke tests that could have resulted in a failure but only 1 actually had a failure. |
|
TODO:
|
| @@ -32,5 +32,5 @@ fi | |||
| if [ -e "$CHROME_PATH" ]; then | |||
| echo "cached chrome found" | |||
| else | |||
| wget "$url" --no-check-certificate -q -O chrome.zip && unzip -q chrome.zip | |||
| curl "$url" -Lo chrome.zip && unzip -q chrome.zip | |||
There was a problem hiding this comment.
wget is not in windows but curl is
@caseq says he landed a change that resolves our attach+waitForDebugger issues. My only verification so far is trying this PR ~3 times in CI without seeing a failure. I will run CI a few more times to verify.
In theory this should:
fix #13861
fix #14099
fix #14093
Knocking on wood because I've been tricked by this before. Either way, any PR that resolves our oopif issues will probably look like this one, so it doesn't hurt to have it ready to go.