-
Notifications
You must be signed in to change notification settings - Fork 649
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.
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
collect: use new headless mode #907
Conversation
@@ -78,7 +78,7 @@ describe('Lighthouse CI collect CLI with puppeteer', () => { | |||
const files = fs.readdirSync(path.join(autorunDir, '.lighthouseci')); | |||
const report = files.find(file => /lhr.*\.json$/.test(file)); | |||
const lhr = JSON.parse(fs.readFileSync(path.join(autorunDir, '.lighthouseci', report))); | |||
expect(lhr.userAgent).toContain('HeadlessChrome/111.0.5555.0'); // make sure the right chrome was used | |||
expect(lhr.userAgent).toContain('Chrome/111.0.0.0'); // make sure the right chrome was used |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needed because --headless=new
no longer announces itself in the UA string.
From #336 the condition being asserted seems to be that puppeteer's Chrome was used, not that it was headless in particular (that's just the default without headful: true
in the options), so this change seems ok.
Guys, any chance this Pull Request will be merged soon? |
122480d
to
75fba62
Compare
Switch to --headless=new when
headful
isn't requested.This is better in general, but should also fix bfcache results which have issues with the old headless mode (see treosh/lighthouse-ci-action#126, catchpoint/WebPageTest.agent#605).
I'd like to add a test that the bfcache audit isn't blocked by getting the "Back/forward cache is not supported by delegate" failure anymore, but I haven't found a good place to add that. Happy to take any suggestions!