[DRAFT] Separate cypress tests into "essential" tests that run on PRs vs "comprehensive" tests that run on main #7859

DejayJD · 2024-03-15T21:28:15Z

Description

(This would be a draft PR but I want to see the CCI workflows changed here)

Context/Motivation

We've been discussing in our FE syncs how we want to increase cypress test coverage overall, but also want to avoid constantly spinning our wheels in PRs either waiting for long running test suites or spending time debugging flaky tests.

The solution proposed here is to migrate a portion of our suite to not run on PRs and instead run on every main commit. This way we are able to quickly ship PRs but still have test coverage on each change identifying which commit caused issues.

We also discussed putting stuff further away from PRs, in frequencies like scheduled builds (morning/nightly/etc), on release branchs only, and some other potential options. If we decide to do any of those it will be pretty easy to adjust; we just change what triggers the "comprehensive" suite flow

Changes in this PR

Using cypress-grep, this PR enables adding an @essential tag to a cypress test block to dictate whether a test runs on PRs or not (all tests run on main no matter what)

I took a stab at picking which of our existing tests should be "essential" or not (spoiler; mostly essential)
Please do give feedback on these and whether some belong in/out of the "essential" list.

Essential (runs on every PR commit; aka existing flow)

signIn.cy.ts
signOut.cy.ts
signUp.cy.ts
uploadTrack.cy.ts
playTrack.cy.ts
smoke suite (I honestly think these are lower priority but they're so simple that it feels easy to include with the essential suite)

Non-Essential (runs on main commits only; aka the new flow)

favoriteTrack.cy.ts
repostTrack.cy.ts
(I was on the fence about even putting these in non-essential, so please do give feedback)

Future TODOs

Make an alerting system for tests failing on main
More tests (right now there's not much difference in the "essential" test suite) 💯
Decide if running on main is the best place
Keep accountable on breaking main tests / addressing test flakes

How Has This Been Tested?

TODO: watch CCI and make sure it works, also will verify suite runs once it goes to main (a little bit of 🚢 and 🙏)

audius-infra · 2024-03-15T21:35:03Z

Preview this change https://demo.audius.co/jd/cypress-essential-test-suite

rickyrombo · 2024-03-15T21:38:45Z

Thanks for taking the initiative! Couple of thoughts:

I think all the current tests (and any new tests) should be "essential" until we experience issues with them taking too long - ie wait for complaints before delegating them to main-only
Defaults are powerful, I think everything should be "essential" by default and require an opt-out rather than opt-in. Maybe the grep tag should be inverted? Would require another name though... might need to brainstorm

rickyrombo · 2024-03-15T21:41:04Z

.circleci/src/workflows/web.yml

@@ -52,10 +52,21 @@ jobs:
        branches:
          only: /(^release.*)$/

+  - web-test-staging-essential:


probably should rename both of these to web-test-cypress-pr and web-test-cypress-main or something

yeah I was on the fence about the naming here, I think I might go with your suggestion and just name things PR or no

I'm fine with the way you have it tbh, @essential is a good token to grep for and it makes sense to use it consistently all the way through the ci workflow name

rickyrombo · 2024-03-15T21:41:22Z

.circleci/src/workflows/web.yml

  - web-test-staging:
      context: Audius Client
      requires:
        - web-build-staging
+      filters:
+        branches:
+          only: /^main$/


probably want release branches too

rickyrombo · 2024-03-15T21:41:48Z

packages/probers/cypress/support/commands.ts


 import user from '../fixtures/user.json'
+
+// Enable the CLI grep tool
+registerCypressGrep()


this is cool!

DejayJD · 2024-03-15T22:19:37Z

Thanks for taking the initiative! Couple of thoughts:

I think all the current tests (and any new tests) should be "essential" until we experience issues with them taking too long - ie wait for complaints before delegating them to main-only

I think its totally fair to say we should mark everything as "essential" in the current suite; I'm down for that and was already on the fence about it.

I totally see your point about delegating to main after they've become an issue but I would argue that there are two different strategies and we should probably converge on one. aka should we be more "reactive" or "prescriptive" about the two suites. I would say a reactive strategy is keeping all or most things in the "essential" PR suite until we deem them an issue vs "prescriptive" in the sense that we're more picky/intentional about keeping the "essential" PR suite lightweight. I was personally envisioning more of the 'prescriptive' camp myself but thats a "loosely held opinion" of mine, so I'm happy to adjust. This is just a draft for now so I'm happy to let it simmer for a while.

Defaults are powerful, I think everything should be "essential" by default and require an opt-out rather than opt-in. Maybe the grep tag should be inverted? Would require another name though... might need to brainstorm

I think this falls under the same strategy convergence conversation, but if we want to go this approach the grep has an inverse feature so we can just flip it if we want.

rickyrombo · 2024-03-15T22:46:19Z

I totally see your point about delegating to main after they've become an issue but I would argue that there are two different strategies and we should probably converge on one. aka should we be more "reactive" or "prescriptive" about the two suites.

Yeah 100% agree. My opinion is loosely held too, so I'm fine either way. I could also see an extreme where spinning up cypress itself is already too slow for PRs even as a baseline and pushing all cypress tests to main, and would be fine living with that as well if that's the consensus. The requirements or things to optimize for as I see them are:

Catch bugs before they are deployed (faster they're caught the better)
Keep PRs fast and lightweight (lighter they are the better)
Can easily identify which change caused the failure (easier to determine the better)

And the debate is how to prioritize and balance 1 vs 2, but I think no matter what we decide there we're able to hit a solid baseline for all 3

sliptype

This is awesome!

Yeah the way you've grouped essential & non-essential seems good to me, let's work on growing the non-essential bucket over time

sliptype · 2024-03-18T15:16:42Z

packages/probers/cypress/e2e/flaky-test-jail.md

-"should create an account" tests are skipped until they can be updated
+#### [Sign In](./signIn.cy.ts) - [C-4005]
+
+The 'can sign in' test is in jail until we can hard-code an OTP code


We could also whitelist the test account from OTP:
https://github.com/AudiusProject/audius-protocol/blob/main/packages/identity-service/src/utils/otp.js#L6

sliptype · 2024-03-18T15:18:03Z

.circleci/src/workflows/web.yml

@@ -52,10 +52,21 @@ jobs:
        branches:
          only: /(^release.*)$/

+  - web-test-staging-essential:


I'm fine with the way you have it tbh, @essential is a good token to grep for and it makes sense to use it consistently all the way through the ci workflow name

rickyrombo · 2024-04-08T23:44:38Z

I think it's probably safe to close this

Start process for separating essential PR tests vs non essential

def1185

pull-request-size bot added the size/M label Mar 15, 2024

DejayJD requested review from sliptype, rickyrombo and a team March 15, 2024 21:29

rickyrombo reviewed Mar 15, 2024

View reviewed changes

sliptype approved these changes Mar 18, 2024

View reviewed changes

DejayJD requested review from amendelsohn, dylanjeffers and schottra March 19, 2024 22:35

sliptype mentioned this pull request Mar 25, 2024

Add Playwright for Web E2E Testing #7922

Merged

rickyrombo closed this Apr 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DRAFT] Separate cypress tests into "essential" tests that run on PRs vs "comprehensive" tests that run on main #7859

[DRAFT] Separate cypress tests into "essential" tests that run on PRs vs "comprehensive" tests that run on main #7859

DejayJD commented Mar 15, 2024 •

edited

Loading

audius-infra commented Mar 15, 2024

rickyrombo commented Mar 15, 2024

rickyrombo Mar 15, 2024

DejayJD Mar 15, 2024

sliptype Mar 18, 2024

rickyrombo Mar 15, 2024

rickyrombo Mar 15, 2024

DejayJD commented Mar 15, 2024

rickyrombo commented Mar 15, 2024

sliptype left a comment •

edited

Loading

sliptype Mar 18, 2024

sliptype Mar 18, 2024

rickyrombo commented Apr 8, 2024

[DRAFT] Separate cypress tests into "essential" tests that run on PRs vs "comprehensive" tests that run on main #7859

[DRAFT] Separate cypress tests into "essential" tests that run on PRs vs "comprehensive" tests that run on main #7859

Conversation

DejayJD commented Mar 15, 2024 • edited Loading

Description

Context/Motivation

Changes in this PR

Future TODOs

How Has This Been Tested?

audius-infra commented Mar 15, 2024

rickyrombo commented Mar 15, 2024

rickyrombo Mar 15, 2024

Choose a reason for hiding this comment

DejayJD Mar 15, 2024

Choose a reason for hiding this comment

sliptype Mar 18, 2024

Choose a reason for hiding this comment

rickyrombo Mar 15, 2024

Choose a reason for hiding this comment

rickyrombo Mar 15, 2024

Choose a reason for hiding this comment

DejayJD commented Mar 15, 2024

rickyrombo commented Mar 15, 2024

sliptype left a comment • edited Loading

Choose a reason for hiding this comment

sliptype Mar 18, 2024

Choose a reason for hiding this comment

sliptype Mar 18, 2024

Choose a reason for hiding this comment

rickyrombo commented Apr 8, 2024

DejayJD commented Mar 15, 2024 •

edited

Loading

sliptype left a comment •

edited

Loading