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

Improve speed of tests #117

Closed
jackdomleo7 opened this issue Oct 9, 2021 · 14 comments · Fixed by #149
Closed

Improve speed of tests #117

jackdomleo7 opened this issue Oct 9, 2021 · 14 comments · Fixed by #149
Labels
project enhancement Enhancement to improving the overall project

Comments

@jackdomleo7
Copy link
Owner

jackdomleo7 commented Oct 9, 2021

Describe the new a11y feature or project enhancement

The Cypress tests can take up to 2 minutes to run, which uses up minutes and storage in the GitHub workflow. The tests are extremely simple, so can we find a way to make the tests a lot quicker? I.e. Could we change to Jest?

Describe the solution you'd like

Tests to runner quicker. Anything is an improvement.

Link(s)

@jackdomleo7 jackdomleo7 added the project enhancement Enhancement to improving the overall project label Oct 9, 2021
@tannerdolby
Copy link
Contributor

tannerdolby commented Oct 12, 2021

While I work on migrating from Cypress 6 to 8 in #113, I'll keep an eye out for any "low hanging fruit" in terms of performance improvements. Rewriting the tests in Jest might be the key to obtaining the greatest performance gains and I'd be happy to do it but I can't say for certain it will yield a performance increase until we prototype it.

@jackdomleo7
Copy link
Owner Author

Thank you! I've been meaning to get around to this. Off the top of my head, I'm not sure what the benefits are of Jest over Cypress or Cypress over Jest.

@Manishgupta200
Copy link

Ya, I'm interested. please assign to me

@jackdomleo7
Copy link
Owner Author

@Manishgupta200 just need to double-check with @tannerdolby whether he's ok with this since he said he might keep a look out for low hanging fruit while working on a related ticket.

If @tannerdolby is happy for you to take this ticket then I am too. Would be worth keep @tannerdolby in the loop though, because you'll both have tickets relating to the Cypress tests and I'd hope for any work to be duplicated or discarded due to recent changes.

@tannerdolby
Copy link
Contributor

tannerdolby commented Oct 20, 2021

👋 @Manishgupta200 I'm in the process of migrating Cypress from 6 to 8 in #113 and might catch some "quick fixes" while I finish up that PR. For now, lets keep this on hold (atleast until tomorrow) as I should have the 113 ticket close to finished up by tonight or tomorrow morning.

Then we can have a better idea of what improvements might be made (e.g. staying with Cypress and looking for things to improve or transitioning to using Jest). Hopefully that works for you @jackdomleo7! We can reconnect tomorrow to address things more.

@tannerdolby
Copy link
Contributor

tannerdolby commented Oct 20, 2021

Off the top of my head, I'm not sure what the benefits are of Jest over Cypress or Cypress over Jest.

Yeah, this is a good question @jackdomleo7. I did a bit of digging and it "seems" Cypress is the faster of the two and provides the "fastest/reliable" testing for anything running the browser. But again Cypress/Jest provide different features so comparing the two is sort of "apples to apples" with one apple providing a bit more features.

  • Cypress is more of a Integration/UI testing framework (with its own test runner) that handles browser sessions for us and runs in the same "run-loop" as the application running which attributes to the speed of Cypress for testing in the browser.
  • Jest is a nice unit testing framework (which is also fast) but makes us handle browser sessions (Cypress does this for us) which can be annoying as we are used to the nice UI and testing sessions provided by Cypress. Jest is essentially a test runner but without the nice UI and other features of Cypress.

All in all, both are performant for testing in the browser and unless we benchmark our test suites, I think the performance difference might be negligible.

@jackdomleo7
Copy link
Owner Author

Thank you for the breakdown @tannerdolby! I think my main concern is finding the fastest tool since I feel these tests are relatively simple.

@tannerdolby
Copy link
Contributor

tannerdolby commented Oct 21, 2021

Your welcome @jackdomleo7! Yeah agreed. The test cases are relatively simple so finding the most performant tool would be ideal. After researching, I couldn't confidently declare a clear "performance" winner between the two. From my experience in MDN Web Docs and GoogleChrome repos, they are using Jest for unit testing browser related stuff.

If we can find a definitive place that says "X is faster than Y" then I think we should go with that. Cypress does a good job here, but if we don't want the nice UI and only want unit tests (as the test cases are relatively simple to begin with), Jest "might" be slightly more performant. I just haven't found anything that puts it plainly and says Jest is faster than Cypress or vice-versa.

We have to decide whether we want our tests to run in a "real browser" environment like Chrome etc, and that is possible with Cypress. Or if we want to use Jest, where were in a "js-dom" simulated browser environment and not a real browser (just providing what a browser "should" have).

Although not directly related to performance, this Cypress.io vs Jest comparison sums up the feature differences.

@jackdomleo7
Copy link
Owner Author

I think the Cypress UI is unnecessary here. We can probably integrate a UI diffing tool in Jest.

I'm happy with js-dom for a project like this, to be honest. I've used js-dom for years in other projects (mainly Vue) and not had any issues so I think for a small CSS project, js-dom will be fine. Would you agree?

That is a nice resource! Thanks for sharing.

@tannerdolby
Copy link
Contributor

tannerdolby commented Oct 21, 2021

Yeah I agree. And your welcome!

With that being said, should we plan on converting #113 to handle switching from Cypress to Jest? Or maybe create a new issue dedicated to the change?

@jackdomleo7
Copy link
Owner Author

I think including it in #113 would be good 🙂

@tannerdolby
Copy link
Contributor

tannerdolby commented Nov 6, 2021

Ok will do :)

Fortunately for us there weren't very many changes to be made for updating Cypress from 6 to the latest version 8.7.0. In terms of performance improvements, I'd say its safe to remove any looping using .each() when only checking a single element in index.html.

This occurs for the accesskey test and a few others, the majority of them benefit from using .each() as there are multiple elements needing to be asserted but removing each() for singular elements will yield small improvements as we don't add extra each() calls for single element assertions.

@jackdomleo7
Copy link
Owner Author

Thanks again @tannerdolby for the recent PR! @Manishgupta200, if you're still interested, if you can find any other ways that Tanner may have missed to improve the tests, I'd be happy for you to do so. No worries if not though 🙂

@Manishgupta200
Copy link

Thanks again @tannerdolby for the recent PR! @Manishgupta200, if you're still interested, if you can find any other ways that Tanner may have missed to improve the tests, I'd be happy for you to do so. No worries if not though 🙂

Yaa, thanks for your appreciation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
project enhancement Enhancement to improving the overall project
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants