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

Build/782 parallel cypress webpack 5 #800

Merged
merged 43 commits into from
Apr 4, 2023
Merged

Conversation

jps
Copy link
Contributor

@jps jps commented Apr 3, 2023

#782

What

  • Background - why this is needed our cypress tests are now taking > 10 mins to run. I looked at doing this through the CircleCI orb but cypress has been a bit scummy and made it so you can only use the feature if you sign up for their cloud service.
  • What did you do - cypress-parallel came out as the best way for us to proceed at this point in time ^1
  • Bonus WebPack upgrade included as I couldn't run the e2e tests locally likely because I am using WSL, this should improve build speeds a bit and does open the way for rspack which @JohnTParsons has been looking at.
  • What should the reviewers expect? faster tests and builds

image

The following image shows some of the tunings that I looked at as part of this work. unfortunately, cypress-parallel becomes flakey at higher worker counts. It's not immediately clear what the issue is here.

image

^1 because of the limitation / bug in cypress-parallel it may be beneficial to take a look at sorry-cypress again at some point in the future so we can better utilize large instance sizes in CirlceIC and increased benefit. Setting this up will be less simple than cypress-parallel as it requires it's orchestration service to be running. I do have an example of running this locally available in this commit 72cd996

I have done:

  • Written unit tests against changes
  • Written functional tests against the component and/or NewsKit site
  • Updated relevant documentation

I have tested manually:

  • The feature's functionality is working as expected on Chrome, Firefox, Safari and Edge
  • The screen reader reads and flows through the elements as expected.
  • There are no new errors in the browser console coming from this PR.
  • When visual test is not added, it renders correctly on different browsers and mobile viewports (Safari, Firefox, small mobile viewport, tablet)
  • The Playground feature is working as expected

Before:

After:

Who should review this PR:

How to test:

jps and others added 30 commits February 17, 2023 13:29
@jps jps linked an issue Apr 3, 2023 that may be closed by this pull request
@jps jps added the ready for review Please assist in getting this reviewed label Apr 3, 2023
cy.wait(1000);
)}${route}`, () => {
cy.mockConsentAndVisit(route);
cy.wait(1000); // FIX: This accounts for 2mins of the test run time
Copy link
Collaborator

Choose a reason for hiding this comment

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

I presume it fails without that wait? Perhaps it could wait for something to appear in the DOM instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would expect it to get increasingly flakey the lower the number gets, It was in there from before, I'll spin it out into a separate task to take a look at as would like to get this merged. The Impact of this will be a bit less than 2mins given it's now parallelized. so possibly not as high priority as say compared to the rspack work.

Copy link
Contributor

Choose a reason for hiding this comment

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

I remember @baburay23 looked into this when there was a bug and added intercepts for all the api calls. So I'm not sure what the wait is waiting for. So it might be that this can just be deleted. Did you have issues without it?

@JohnTParsons
Copy link
Collaborator

I've noticed that package.json contains "@types/webpack": "^4.4.34"
This could be upgraded to version 5.28.1 (in this PR or a future one)

@jps jps merged commit ca33f69 into main Apr 4, 2023
@jps jps deleted the build/782-parallel-cypress-webpack-5 branch April 4, 2023 12:25
@jps
Copy link
Contributor Author

jps commented Apr 4, 2023

I've noticed that package.json contains "@types/webpack": "^4.4.34" This could be upgraded to version 5.28.1 (in this PR or a future one)

addressed in #804

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review Please assist in getting this reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

upgrade to webpack 5 & improve pipeline speed
5 participants