tests: default to chrome-launcher path #13912
Conversation
| @@ -31,12 +31,6 @@ jobs: | |||
| - run: yarn install --frozen-lockfile --network-timeout 1000000 | |||
| - run: yarn build-all | |||
|
|
|||
| - name: Define ToT chrome path | |||
There was a problem hiding this comment.
I think these can be removed because the tests can then use the stable Chrome already installed on the test runners, but we'll see how that goes :)
The ToT Chrome installation typically takes a whole 6 seconds, so not a big deal if these blocks need to stay instead.
There was a problem hiding this comment.
But what's the benefit to using stable chrome vs ToT for these tests? presumably we used ToT for a reason?
There was a problem hiding this comment.
At least with the ones added in #13783, there is no particular reason for using ToT.
There was a problem hiding this comment.
ToT lets us catch things early. We'd be losing that with this change. Don't recall if that has ever happened for unit/basics, but def. has for smoke.
There was a problem hiding this comment.
Don't recall if that has ever happened for unit/basics
well ToT has only been used since April 15th, before that it was using the Chromium shipped with puppeteer.
Making tests simpler to run is almost always better, you end up with fewer hidden assumptions (like CHROME_PATH will always be set), but I can leave this in and add a comment it's specifically to catch ToT behavior
| @@ -316,7 +317,7 @@ async function run() { | |||
| } | |||
|
|
|||
| const browser = await puppeteer.launch({ | |||
| executablePath: process.env.CHROME_PATH, | |||
| executablePath: getChromePath(), | |||
There was a problem hiding this comment.
This still defaults to CHROME_PATH but is the one I felt less sure about. Let me know if anyone would like it the other way and I can always revert.
If you don't have
CHROME_PATHset locally, runningapi-test-pptranddisconnect-test-pptrfails because a Chrome can't be found for puppeteer to use. Usingchrome-launcher'sgetChromePath()will let a Chrome already installed on the system be used, and it still defaults toCHROME_PATHif it's set.