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

test: [M3-5858] - Cypress component tests #10134

Merged

Conversation

jdamore-linode
Copy link
Contributor

Description 📝

I'll add a more detailed PR description tomorrow, but basically wanted to put together a quick Cypress component testing POC to explore whether there is any value here for us.

Usage:
Install dependencies with yarn, and then run yarn cy:component. Cypress will open, prompt you to select a browser, and then prompt you to select a spec.

Included are two sets of component tests:

  • beta-chip.spec.tsx demonstrates a really basic component test and shows the bare minimum that's needed to get started
  • region-select.spec.tsx is more complete and demonstrates more advanced features like interaction, mocking, and feature flag management

Changes 🔄

(Will fill this out soon.)

Preview 📷

(Will fill this out soon.)

How to test 🧪

Prerequisites

(How to setup test environment)

  • ...
  • ...

Reproduction steps

(How to reproduce the issue, if applicable)

  • ...
  • ...

Verification steps

(How to verify changes)

  • ...
  • ...

Copy link

github-actions bot commented Jan 31, 2024

Coverage Report:
Base Coverage: 86.13%
Current Coverage: 86.13%

Copy link
Contributor

@abailly-akamai abailly-akamai left a comment

Choose a reason for hiding this comment

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

This is looking great! I ran this locally with the RegionSelect and it looks really good.

As far as weighing on how useful those could be, I think some tests for important components would be really useful, but I'd love to hear what the rest of the team think.

I assume we could run this in parallel in the CI?

I also think we'd need to look at make our e2e suite lighter by shaving out our testing of components themselves and focus on flows?

@jdamore-linode
Copy link
Contributor Author

Thanks for taking a look @abailly-akamai!

As far as weighing on how useful those could be, I think some tests for important components would be really useful, but I'd love to hear what the rest of the team think.

Also eager to hear from the rest of the team, but this is along the lines of what I'm thinking.

I assume we could run this in parallel in the CI?

Yes! These don't require test accounts so that gives us a lot of flexibility in how we could run them, including via GitHub Actions

I also think we'd need to look at make our e2e suite lighter by shaving out our testing of components themselves and focus on flows?

++. We've got a bunch of tests that take detours, so to speak, to confirm small component details that aren't strictly related to the flow(s) being tested. If we move forward with this, I'd love to move all of that to component tests where possible

Copy link
Contributor

@cpathipa cpathipa left a comment

Choose a reason for hiding this comment

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

@jdamore-linode Nice work on the POC.

Having Cypress interactive testing for components along with unit tests can significantly enhance our testing strategy, with covers both depth (with unit tests) and breath (with E2E test). It's important, however to balance the testing strategy to ensure efficiency and minimize redundancy for each testing scenario.
IMO, the biggest concerns here are test redundancy and time constraints. Considering our stack, React Testing Library provides interactive testing (example: RegionMultiSelect). Maybe we could consider this for areas where we were unable to cover interactive testing with it.

@bnussman-akamai
Copy link
Contributor

This is neat! I've come to really enjoy the ability to write e2e-like unit tests with MSW and React Testing Library so I'm interested in understanding what pros and cons this may have compared to our existing unit testing patterns.

@jdamore-linode
Copy link
Contributor Author

@jdamore-linode Nice work on the POC.

Having Cypress interactive testing for components along with unit tests can significantly enhance our testing strategy, with covers both depth (with unit tests) and breath (with E2E test). It's important, however to balance the testing strategy to ensure efficiency and minimize redundancy for each testing scenario. IMO, the biggest concerns here are test redundancy and time constraints. Considering our stack, React Testing Library provides interactive testing (example: RegionMultiSelect). Maybe we could consider this for areas where we were unable to cover interactive testing with it.

Thanks for the feedback, @cpathipa! Completely agreed on the redundancy point, and I think an even broader discussion might be warranted to explore how (or whether) these fit in with our current stack and testing patterns, when we'd opt for one over the other, etc..

This is neat! I've come to really enjoy the ability to write e2e-like unit tests with MSW and React Testing Library so I'm interested in understanding what pros and cons this may have compared to our existing unit testing patterns.

Thanks, @bnussman-akamai!

You have to weigh this against the fact that I spend a lot of time working with Cypress anyway, so I've got a whole lot of muscle memory and familiarity with Cypress and its debugger which I don't necessarily have to the same extent with Vitest — however, I was pretty pleased with the dev experience writing these few tests, and I can try to sum up why:

  • The Cypress debugger (in essence Chrome's dev tools) gives you a lot to work with if you're iterating on a test, or trying to troubleshoot an assertion failure. You have full access to inspect the DOM, you can inspect network requests and responses, and so on. I didn't appreciate this as something particularly useful for a unit-like test and was surprised by how helpful it was while writing the RegionSelect tests.
  • You get Cypress's scheduling and implicit waiting, so you don't have to distinguish between findBy, getBy, and queryBy in your asserts. That's basically the only major differentiator between React Testing Library and Cypress Testing Library, so RTL knowledge/muscle memory is pretty transferable between the test suites
  • While there's no advantage to using Cypress's mocking tools vs MSW, being part of the Cypress test suite does give us access to all of the mocking utils we've already written for our UI tests which really helped accelerate development of the Region Select tests

Technically speaking, there's very little that we can't do via Cypress Component tests that we can do via Vitest, and vice-versa, but these are the technical limitations of each that I'm aware of:

  • Cypress doesn't offer any (effective) spying tools, so e.g. we can't test that a component executes some callback the way we can with vi.fn() and expect(...).toHaveBeenCalled()
  • If we're interested in automated accessibility testing, axe-core supposedly has some compatibility issues with js-dom that limit what it can validate. I'm not certain what the limitations are or whether they might also apply to happy-dom.
  • I didn't do any measurements but vitest performance certainly still reigns supreme, I imagine by a pretty wide margin. That said, the Cypress component tests still seemed really performant and I would say they fit comfortably in the middle between Vitest and Cypress UI tests

I don't think these should replace our Vitest tests or that we should stop writing new Vitest component tests or anything like that, but I think there might be an opportunity for these to serve as a helpful supplement to some of our components and their tests (or maybe not!).

@jaalah-akamai
Copy link
Contributor

Overall in terms of debugging I could see this being helpful. I'll add that RTL is still a better way to test our hooks, whereas I don't think Cypress Component Tests offers a solution here.

I could see this being useful if we wanted to test a larger more complex component rather than hitting a URL to test flows, but the argument could be made that it's the role of an E2E to do this.

@abailly-akamai
Copy link
Contributor

@jdamore-linode I'd love to get this in and start writing more of those as well as moving a couple (RegionSelect, PlanSelection) out of the e2e suite. What do we need to keep this moving? Can we get it merged as is then have a follow up PR for the CI integration?

@jdamore-linode
Copy link
Contributor Author

@jdamore-linode I'd love to get this in and start writing more of those as well as moving a couple (RegionSelect, PlanSelection) out of the e2e suite. What do we need to keep this moving? Can we get it merged as is then have a follow up PR for the CI integration?

Thanks @abailly-akamai! Been meaning to make this a topic for cafe the past few weeks but haven't had the time to do some exploratory research I wanted to, specifically about using these as a foundation for visual regression testing, but that's no good reason to hold things up.

I'll get these merge conflicts fixed and make any other changes that might be needed to get this up and running, and we can discuss during cafe! (I think you and I are pretty much on the same page, however)

@jaalah-akamai jaalah-akamai added the Approved Multiple approvals and ready to merge! label May 30, 2024
Copy link

This PR is stale because it has been open 15 days with no activity. Please attend to this PR or it will be closed in 5 days

@github-actions github-actions bot added the Stale label Jul 17, 2024
@github-actions github-actions bot closed this Jul 23, 2024
@jdamore-linode
Copy link
Contributor Author

I'm bringing this back from the dead cc @abailly-akamai

@jdamore-linode jdamore-linode changed the title test: [M3-5858] - Cypress component tests (proof of concept) test: [M3-5858] - Cypress component tests Aug 26, 2024
@jdamore-linode jdamore-linode merged commit e7d17b1 into linode:develop Sep 3, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge! Proof of Concept
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants