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

Jump to bottom

Use newer version of Puppeteer for UI tests #17880

Merged
merged 23 commits into from Sep 1, 2021
Merged

Use newer version of Puppeteer for UI tests #17880

merged 23 commits into from Sep 1, 2021

Conversation

sgiehl
Copy link
Member

@sgiehl sgiehl commented Aug 12, 2021 •
edited

Description:

We are still using Puppeteer 2.1.1, which uses a quite "old" Chromium version.
Puppeteer 8.0.0 will update that to Chromium 90.

Note: I'm note yet updating to version 10.2.0, as with version 10 some methods that we are using were removed, also for some reason the tests are hanging after a fixture is set up. Don't want to waste time trying to figure out what exactly needs to be changed.

Review

  • Functional review done
  • Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
  • Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
  • Security review done see checklist
  • Code review done
  • Tests were added if useful/possible
  • Reviewed for breaking changes
  • Developer changelog updated if needed
  • Documentation added if needed
  • Existing documentation updated if needed

@sgiehl sgiehl added this to the 4.5.0 milestone Aug 12, 2021
@sgiehl sgiehl changed the title Use latest version of Puppeteer for UI tests Use newer version of Puppeteer for UI tests Aug 12, 2021
@sgiehl sgiehl force-pushed the updatepupeteer branch 20 times, most recently from 2a77f01 to 9ef353f Compare August 17, 2021 17:10
@mattab
Copy link
Member

mattab commented Aug 18, 2021

Is there any benefit to this project, besides maybe preventing some tech debt? Maybe this can wait for some months, or is there value in continuing this work now?

@sgiehl
Copy link
Member Author

sgiehl commented Aug 18, 2021

This ensures our tests are running with a more modern Chrome. In theory the newer version would even support running the tests with Firefox as well. Also there are a lot more features, that can be quite useful for writing better tests. In addition I hope this brings some more stability into our tests and reduces the random failures.

@Findus23
Copy link
Member

Especially this makes sure that the UI tests (and other JS tests) represent the reality of people using modern versions of web browsers instead of Chrom(e|ium) 80 which was released in 2019 and might not trigger bugs in Matomo caused by changes in how browsers work since then.

@sgiehl sgiehl force-pushed the updatepupeteer branch 5 times, most recently from 67076b1 to c5f95cd Compare August 19, 2021 14:17
This was referenced Aug 26, 2021
Merged
Merged
@sgiehl
Use latest version of Puppeteer for UI tests
bbc0a8e
@sgiehl
use newer node on travis
9ea1e24
@sgiehl
test
03c272f
@sgiehl
use waitForTimeout / waitForSelector instead of waitFor
7e07af8
@sgiehl
Handle BC for removed waitFor method
d0a43b3
@sgiehl
Updates expected UI test files
4d1b4d0
@sgiehl
update submodules
0cbd4b5
@sgiehl
improve/fix some UI tests
eb47850
@sgiehl
Updates expected UI test files
69b5ffc
@sgiehl
improve/fix some UI tests
f2ca626
@sgiehl
update url-parse dependency
f4c3d8b
@sgiehl
simplify some UI tests
03775bd
@sgiehl
update screenshot
225baae
@sgiehl
avoid using merged assets that are broken
799b3fd
@sgiehl
fix expected compare exit code
2d8a322
@sgiehl
don't use merged assets for TwoFactorAuth UI tests
1a8b32d 
it seems due to too many requests to login, logout and other pages merging the CoreJS assets somehow fails, and results in returning an empty file. Even the reload fails to serve the correct content, so maybe the file, the content should be written to, is somehow locked. Disabling the merged assets seems to fix this issue for this tests. Locally the tests are also running correctly with merged assets.
@sgiehl
improve handling of automatic css / js reloading in UI tests
c4ad8fd
@sgiehl
increase timeouts of TwoFactorAuth UI tests
574b064
@sgiehl
fix 2fa ui test
5ebabbc
@sgiehl
revert change
07362c9
@sgiehl
update new screenshot
5b42f66
@sgiehl sgiehl marked this pull request as ready for review September 1, 2021 13:29
@sgiehl sgiehl merged commit a3ee690 into 4.x-dev Sep 1, 2021
@sgiehl sgiehl deleted the updatepupeteer branch September 1, 2021 13:37
@justinvelluppillai justinvelluppillai added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Oct 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tsteur tsteur tsteur left review comments

Assignees
No one assigned
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Milestone
4.5.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@sgiehl @mattab @Findus23 @tsteur @justinvelluppillai