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

Upgrade to the latest version of Cypress #5976

Merged
merged 35 commits into from
Apr 16, 2020
Merged

Upgrade to the latest version of Cypress #5976

merged 35 commits into from
Apr 16, 2020

Conversation

FK78
Copy link
Contributor

@FK78 FK78 commented Mar 25, 2020

Resolves https://github.com/bbc/simorgh-infrastructure/issues/837

Overall change:
Upgrade to the latest version of Cypress

Code changes:

  • Convert the string timestamp into a number to meet the parameter requirements for Chai.
  • Bump Cypress from 3.8.1 to 4.4.0

  • I have assigned myself to this PR and the corresponding issues
  • I have added labels to this PR for the relevant pod(s) affected by these changes
  • I have assigned this PR to the Simorgh project

Testing:

  • Automated (jest and/or cypress) tests added (for new features) or updated (for existing features)
  • If necessary, I have run the local E2E non-smoke tests relevant to my changes (CYPRESS_APP_ENV=local CYPRESS_SMOKE=false npm run test:e2e:interactive)
  • This PR requires manual testing

@FK78 FK78 added cypress Cypress changes are required ws-articles Tasks for the WS Articles Team ws-home ws-media World Service Media labels Mar 25, 2020
@FK78 FK78 self-assigned this Mar 25, 2020
@FK78 FK78 changed the title Upgrade to the latest version of Cypress [DNM] Upgrade to the latest version of Cypress Mar 25, 2020
@FK78
Copy link
Contributor Author

FK78 commented Apr 14, 2020

@jblaut @FK78 did either of you get any further with this? Let me know if you need a hand, would love to get Cypress updated as soon as possible.

Not currently looking into the issue as I am waiting on space to become available on CodeBuild, but last time I ran the job we had 24 tests fail for a single service only, test time did not change significantly.

@j-pendlebury j-pendlebury linked an issue Apr 16, 2020 that may be closed by this pull request
1 task
@j-pendlebury
Copy link
Contributor

j-pendlebury commented Apr 16, 2020

I'm still looking into this in between other things but here's what I got so far.

I ran the front page e2e's on version 3.8.1 (current) and 4.3.0 (latest version available), with CYPRESS_APP_ENV=local CYPRESS_SMOKE=false npm run test:e2e:interactive and here are the results:

3.8.1

Run 1: 3413s => 57min
Run 2: 3477s => 58min
Run 3: 3449s => 57min

Average: (3413 + 3477 + 3449)/3 = 3446.333333333 => 3446s => 57min

4.3.0

Run 1: 4217s => 1 hour 10 min
Run 2: 4223s => 1 hour 10 min
Run 3: 4368s => 1 hours 13 min

Average: (4217 + 4223 + 4368)/3 = 4269.333333333 => 4269s => 1 hour 11min

So difference of about 14 minutes for front page tests.

This was all done locally, so tests ran slower than on CodeBuild.

Currently the front page tests take about 25 minutes (on TEST, not LIVE) on CodeBuild, so with version 4.3.0, front page tests would take roughly 32 minutes.

Here are some predictions for how long the tests will take with Cypress@4.3.0 on CodeBuild based on how long they run for now:
image
Skipped tests which take seconds or milliseconds to run (e.g. errorPage tests)

Also found these two issues:

which shows we're not the only ones experiencing this issue

@amywalkerdev
Copy link
Contributor

Thanks for the analysis Jamie, I'm happy to update cypress knowing it will add time to our runs for the moment, then we can look to streamline and improve our tests after.

@jamesdonoh
Copy link
Contributor

+1 thanks Jamie and Fahad. I think we should push ahead with this too.

@FK78 FK78 closed this Apr 16, 2020
@FK78 FK78 reopened this Apr 16, 2020
@j-pendlebury j-pendlebury changed the title [DNM] Upgrade to the latest version of Cypress Upgrade to the latest version of Cypress Apr 16, 2020
@j-pendlebury j-pendlebury merged commit a9e4987 into latest Apr 16, 2020
@j-pendlebury j-pendlebury deleted the cypress-upgrade branch April 16, 2020 15:51
@sareh sareh restored the cypress-upgrade branch April 17, 2020 12:34
karinathomasbbc added a commit that referenced this pull request Apr 17, 2020
This reverts commit a9e4987, reversing
changes made to b54d1d6.
@karinathomasbbc karinathomasbbc mentioned this pull request Apr 17, 2020
6 tasks
@sareh sareh deleted the cypress-upgrade branch May 12, 2020 06:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cypress Cypress changes are required ws-articles Tasks for the WS Articles Team ws-media World Service Media
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove npm audit whitelisted package - minimist
7 participants