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

[WEB-3840] remove Cypress in favour of Storybook's test-runner #365

Merged
merged 2 commits into from
Aug 14, 2024

Conversation

jamiehenson
Copy link
Member

@jamiehenson jamiehenson commented Jun 6, 2024

Jira Ticket Link / Motivation

https://ably.atlassian.net/browse/WEB-3840

Summary of changes

Since the Storybook migration (pre v14), Cypress has been inactive. Upon reflection, now we have Storybook, Cypress seems a somewhat heavy handed approach to testing when we don't need deep integration testing - something like snapshot testing would probably be a good starting point but there wasn't a clear resolution to the conversation.

Thankfully for us, Storybook has options for all of the above. With its own test-runner, we get smoke tests (i.e. does this component render or not) for free for each story, and we have the option of writing more comprehensive testing journeys using Play functions - which given that they're written alongside the stories themselves seems more ergonomic.

This PR brings in test-runner, guts all the cypress stuff, and makes some adjustments to the Github workflows so that the test suite runs on push.

How do you manually test this?

Pull down, run yarn test, Storybook should build and the test suite should run, resulting in a successful set of smoke tests like this

Screenshot 2024-06-11 at 16 25 43

Reviewer Tasks (optional)

Merge/Deploy Checklist

  • Written automated tests for implemented features/fixed bugs
  • Rebased and squashed commits
  • Commits have clear descriptions of their changes
  • Checked for any performance regressions

Frontend Checklist

  • No frontend changes in this PR
  • Added before/after screenshots for changes
  • Tested on different platforms/browsers with Browserstack
  • Compared with the initial design / our brand guidelines
  • Checked the code for accessibility issues (VoiceOver User Guide)?

@@ -1,13 +1,13 @@
{
"jsc": {
"target": "es2018",
"target": "es2021",
Copy link
Member Author

Choose a reason for hiding this comment

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

This aligns with Node 16, which is the minimum requirement across our other apps

@jamiehenson jamiehenson changed the title feat: remove Cypress tests in favour of Storybook's test-runner [WEB-3840] remove Cypress tests in favour of Storybook's test-runner Jun 11, 2024
@jamiehenson jamiehenson changed the title [WEB-3840] remove Cypress tests in favour of Storybook's test-runner [WEB-3840] remove Cypress in favour of Storybook's test-runner Jun 11, 2024
@jamiehenson jamiehenson marked this pull request as ready for review June 11, 2024 15:31
@jamiehenson jamiehenson force-pushed the update-testing-strategy branch 2 times, most recently from 6c02398 to b0f1b2a Compare August 14, 2024 10:38
Copy link
Member

@kennethkalmer kennethkalmer left a comment

Choose a reason for hiding this comment

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

This looks great @jamiehenson, thanks!

Just an aside, the tests for the Status component is definitely flaky and we will get false negatives. This is because it goes from a loading state to whatever the story dictates, and depending on the time of the snapshot it might still be doing its animated pulse rather than rendering the status colour. I don't think it should block us for now, just as long as we're aware :)

Copy link
Contributor

@aralovelace aralovelace left a comment

Choose a reason for hiding this comment

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

cant wait for this

@jamiehenson jamiehenson merged commit e97f0a8 into main Aug 14, 2024
3 checks passed
@jamiehenson jamiehenson deleted the update-testing-strategy branch August 14, 2024 11:43
@jamiehenson
Copy link
Member Author

@kennethkalmer I removed the "Live" story on Status which was causing some flakiness, but the others seemed to be well stubbed so not expecting further flakiness - might not have gone far enough though

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants