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.

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

Switch to new Chrome headless mode #3543

Merged
merged 1 commit into from
May 22, 2023
Merged

Conversation

colinrotherham
Copy link
Contributor

@colinrotherham colinrotherham commented Apr 25, 2023

Following #3542 just a quick PR for:

For more info on our headless mode deprecation issues see:

@colinrotherham colinrotherham self-assigned this Apr 25, 2023
@colinrotherham colinrotherham linked an issue Apr 25, 2023 that may be closed by this pull request
1 task
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3543 April 25, 2023 08:52 Inactive
@colinrotherham
Copy link
Contributor Author

colinrotherham commented Apr 25, 2023

Timing stats using Chrome for Testing on Puppeteer v20

Note: Percy previously downloaded its own Chromium even though we used Puppeteer

Before

Using old headless mode { headless: true }

Accessibility tests: 1m 53s
Behaviour tests: 31s
Component tests: 39s
Percy screenshots: 46s

See GitHub Action run

After

Using new headless mode { headless: 'new' }

Accessibility tests: 2m 8s
Behaviour tests: 31s
Component tests: 36s
Percy screenshots: 37s

See GitHub Action run

@colinrotherham colinrotherham requested a review from a team as a code owner April 26, 2023 08:14
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3543 April 26, 2023 08:14 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3543 April 26, 2023 08:17 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3543 April 26, 2023 08:20 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3543 April 26, 2023 08:27 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3543 April 26, 2023 08:43 Inactive
@colinrotherham colinrotherham changed the title [WIP] Switch to new Chrome headless mode Switch to new Chrome headless mode Apr 26, 2023
@36degrees
Copy link
Contributor

Do you know if the Puppeteer team are aware that tests running under the new headless modes are slower? Is it something they're working to improve?

Sharding the accessibility tests is a neat idea (TIL that's a thing!) but I feel like our GitHub Actions are starting to get a little complex. Wondering if we should either wait until the performance of the new headless mode improves, or just accept that our tests will be a little bit slower for a while?

@colinrotherham
Copy link
Contributor Author

colinrotherham commented May 4, 2023

Do you know if the Puppeteer team are aware that tests running under the new headless modes are slower? Is it something they're working to improve?

From some of the comments they're aware of it (approx 30% slower), but appreciate it's now the same full browser for both headless and headful modes

Sharding the accessibility tests is a neat idea (TIL that's a thing!) but I feel like our GitHub Actions are starting to get a little complex. Wondering if we should either wait until the performance of the new headless mode improves, or just accept that our tests will be a little bit slower for a while?

Ha that's great. I'd always wanted to try --shard out and this seemed perfect for it. That said, I'm more than happy to drop the commit (especially as it changes required status checks again!) or even hold off "new headless" for a while

For context this PR was thinking our { headless: 1 } loophole to silence the deprecation warning would be closed:

But it's still holding strong

@colinrotherham colinrotherham force-pushed the chrome-headless-new branch from f0b8d76 to e722993 Compare May 4, 2023 14:32
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3543 May 4, 2023 14:32 Inactive
@colinrotherham
Copy link
Contributor Author

I've rebased with #3491 and removed the jest --shard commit

No rush to approve this one but good to know it works when we need it

@OrKoN
Copy link

OrKoN commented May 9, 2023

The second timing block in #3543 (comment) is the "After" one (both say "before")?

@colinrotherham
Copy link
Contributor Author

colinrotherham commented May 9, 2023

The second timing block in #3543 (comment) is the "After" one (both say "before")?

@OrKoN Ahh thanks, updated!

@colinrotherham
Copy link
Contributor Author

colinrotherham commented May 12, 2023

@36degrees I've updated the timing stats after rebasing with Puppeteer v20:

The switch to Chrome for Testing (not Chromium) has closed the gap 🙌

@colinrotherham
Copy link
Contributor Author

Although since the ~600 accessibility tests merged we've drifted a lot from the 2m 30s sweet spot we once had

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3543 May 16, 2023 15:33 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3543 May 18, 2023 14:27 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3543 May 22, 2023 15:25 Inactive
Copy link
Member

@romaricpascal romaricpascal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The timings now look pretty much the same as on main, let's merge this 🥳 ⛵

@colinrotherham colinrotherham merged commit 8c6a934 into main May 22, 2023
@colinrotherham colinrotherham deleted the chrome-headless-new branch May 22, 2023 15:40
colinrotherham added a commit that referenced this pull request Jun 13, 2023
Since #3543 (or perhaps in recent Puppeteer updates) we're seeing much more accurate timers

Running assertions at the exact `setTimeout()` or `setInterval()` periods can fail the test so I've added 50ms on
colinrotherham added a commit that referenced this pull request Jun 13, 2023
Since #3543 (or perhaps in recent Puppeteer updates) we're seeing much more accurate timers

Running assertions at the exact `setTimeout()` or `setInterval()` periods can fail the test so I've added 50ms on
colinrotherham added a commit that referenced this pull request Jun 13, 2023
Instead of counting events that may or may not bubble up to a `<form>` we now count every click + debounce

Since #3543 (or perhaps in recent Puppeteer updates) we're seeing much more accurate timers

Running assertions at the exact `setTimeout()` or `setInterval()` periods can fail the test so I've added 50ms on
colinrotherham added a commit that referenced this pull request Jun 14, 2023
Since #3543 (or perhaps in recent Puppeteer updates) we're seeing much more accurate timers

Running assertions at the exact `setTimeout()` or `setInterval()` periods can fail the test so I've added 50ms on
colinrotherham added a commit that referenced this pull request Jun 14, 2023
Instead of counting events that may or may not bubble up to a `<form>` we now count every click + debounce

Since #3543 (or perhaps in recent Puppeteer updates) we're seeing much more accurate timers

Running assertions at the exact `setTimeout()` or `setInterval()` periods can fail the test so I've added 50ms on
querkmachine pushed a commit that referenced this pull request Jun 23, 2023
Since #3543 (or perhaps in recent Puppeteer updates) we're seeing much more accurate timers

Running assertions at the exact `setTimeout()` or `setInterval()` periods can fail the test so I've added 50ms on
querkmachine pushed a commit that referenced this pull request Jun 23, 2023
Instead of counting events that may or may not bubble up to a `<form>` we now count every click + debounce

Since #3543 (or perhaps in recent Puppeteer updates) we're seeing much more accurate timers

Running assertions at the exact `setTimeout()` or `setInterval()` periods can fail the test so I've added 50ms on
colinrotherham added a commit that referenced this pull request Jul 19, 2023
Switch to new Chrome headless mode
colinrotherham added a commit that referenced this pull request Jul 19, 2023
Since #3543 (or perhaps in recent Puppeteer updates) we're seeing much more accurate timers

Running assertions at the exact `setTimeout()` or `setInterval()` periods can fail the test so I've added 50ms on
colinrotherham added a commit that referenced this pull request Jul 19, 2023
Instead of counting events that may or may not bubble up to a `<form>` we now count every click + debounce

Since #3543 (or perhaps in recent Puppeteer updates) we're seeing much more accurate timers

Running assertions at the exact `setTimeout()` or `setInterval()` periods can fail the test so I've added 50ms on
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Switch to new Chrome headless mode
5 participants