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

compress test server responses and add test matrix with partial retries #951

Conversation

romainmenke
Copy link
Collaborator

@romainmenke romainmenke commented Dec 20, 2020

Since merging #924 test runs fail a lot more often.

I ran tests a bunch of times to see if there was any pattern and it seems that if the first request from test-runner.handlebars is a large bundle there is a higher chance of a failure.

Failure rate on iOS Safari 10.x was about 60%.
This alone was enough to make CI frustratingly flaky.

  • Intl.* test runs fail often.
  • Any run with all polyfills loaded fails more often.
  • Not every browser is affected equally, older iOS is particularly sensitive to this.

In the end I found that compressing responses from server.js helps enough.
This change does add a new dependency : https://www.npmjs.com/package/compression

I also converted the github action to a matrix as it has the ability to continue with other jobs when one fails while still having an overal failure of the workflow. A test run "witness" is "recorded" to skip jobs when a retry is needed. This is done by caching a file (which only happens when the job succeeds) and checking for existence of this cache.

This also makes it easier to get a good overal view of which browsers have issues after a change instead of first fixing IE, retrying CI, then fixing Chrome,...

the name "witness" comes from make, but I can't find the reference anymore.

Android fails, Chrome still runs :

Screenshot 2020-12-20 at 21 03 54

IE is skipped on retry :

Screenshot 2020-12-20 at 21 21 02

@github-actions github-actions bot added the library Relates to an Origami library label Dec 20, 2020
@origamiserviceuser origamiserviceuser added this to incoming in Origami ✨ Dec 20, 2020
@romainmenke romainmenke force-pushed the compress-test-server-responses-and-add-more-pauses-considerate-grouse-0ed3784ad0 branch from 2bb2cf5 to 31b6f24 Compare December 20, 2020 17:30
@romainmenke romainmenke force-pushed the compress-test-server-responses-and-add-more-pauses-considerate-grouse-0ed3784ad0 branch from 6145971 to 806a594 Compare December 20, 2020 19:47
@romainmenke romainmenke marked this pull request as ready for review December 20, 2020 22:30
@romainmenke romainmenke requested a review from a team as a code owner December 20, 2020 22:30

- name: Test ${{ matrix.browser }}
run: node ./test/polyfills/server.js & node ./test/polyfills/remotetest.js targeted director browser=${{ matrix.browser }}
if: steps.witness.outputs.cache-hit != 'true'
Copy link
Owner

Choose a reason for hiding this comment

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

This is very smart, nice idea @romainmenke 🙌

@romainmenke romainmenke changed the title compress test server responses and add more pauses compress test server responses and add test matrix with partial retries Dec 20, 2020
@JakeChampion JakeChampion enabled auto-merge (rebase) December 20, 2020 23:59
@JakeChampion JakeChampion merged commit b8224c7 into JakeChampion:master Dec 21, 2020
Origami ✨ automation moved this from incoming to complete Dec 21, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 21, 2021
@robertboulton robertboulton removed this from Done in Origami ✨ Jul 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
library Relates to an Origami library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants