Run full E2E test suite on PRs #4996

flozia · 2024-08-30T11:03:44Z

References:

Description

This PR adds a GitHub workflow to run the full E2E test suite on PRs against the main branch. Ideally, we’ll mark this new workflow as optional for now so we can monitor and address its success rate before blocking PRs on it. Adding the ability to track metrics regarding the E2E test runs will be part of a follow-up effort.

The main changes for the E2E tests introduced by this PR are:

Adds workflow to run the ”full” E2E test suite on PRs in addition to the existing cronjob that runs against the stage environment.
Run E2E tests in parallel.
Clean up to make sure running E2E tests are passing in CI as well as locally with the same env variables set.

github-actions · 2024-08-30T11:19:43Z

Preview URL 🚀 : https://blurts-server-pr-4996-mgjlpikfea-uk.a.run.app

.github/workflows/e2e_pr_full.yml

.github/workflows/e2e_pr_smoke.yml

playwright.config.js

.github/workflows/e2e_pr_full.yml

mansaj · 2024-09-18T17:38:13Z

.github/workflows/e2e_pr_smoke.yml

@@ -1,4 +1,4 @@
-name: Monitor e2e Smoke Tests
+name: Monitor E2E Test Suite (smoke)


@flozia are we comfortable deleting this yml as a part of this PR?

@mansaj I think the renaming should not cause any issues or what do you think?

Renaming wouldn't cause issue, I'm more asking if we can delete the smoke PR workflow

@mansaj Yes, we can delete the smoke test suite soon. I'd like to have keep it for now and leave it mandatory for merging PR’s. The full test suite will be not mandatory until we better learn about and fix some of the cery flaky tests.

playwright.config.js

src/e2e/specs/auth.spec.ts

src/e2e/specs/purchase.spec.ts

mansaj · 2024-09-18T19:01:43Z

LGTM overall!

Vinnl

Some great improvements here Florian, nice job! Looking forward to seeing how they run on PRs in the coming weeks.

My main suggestions are around removing the need for manually setting so many env vars for the common case. In my ideal world, if you have a working local setup (i.e. the vars under ## Secrets in .env.local.example are set), running npm run e2e:full should be enough for a successful run, but I don't know if there's a reason that that's not feasible.

.github/workflows/e2e_pr_full.yml

Vinnl · 2024-09-25T10:36:09Z

.github/workflows/e2e_pr_full.yml

+      - name: Run Playwright tests
+        if: github.actor != 'dependabot[bot]'
+        run: npm run e2e -- --update-snapshots
+        timeout-minutes: 40


They're running a lot faster than this now, right? If so, we might want to catch stalling tests (or execution speed regressions) earlier?

That’s right. I halved the value 20 minutes for now. If we still see the tests performing consistently way below that, we can decrease to 10.

Vinnl · 2024-09-25T10:36:52Z

.github/workflows/e2e_pr_full.yml

+
+      - name: Run Playwright tests
+        if: github.actor != 'dependabot[bot]'
+        run: npm run e2e -- --update-snapshots


Doesn't this make visual regression tests always succeed, and hence not catch any issues?

Suggested change

run: npm run e2e -- --update-snapshots

run: npm run e2e

That is right! This is the same configuration we are using from the cronjob running the full E2E test suite. Happy to update this later, but for now I’d like to focus on catching functional test failure.

Works for me, I created https://mozilla-hub.atlassian.net/browse/MNTOR-3655 to follow up.

Vinnl · 2024-09-25T10:37:44Z

.github/workflows/e2e_pr_full.yml

+        timeout-minutes: 40
+        env:
+          E2E_TEST_ENV: local
+          E2E_TEST_BASE_URL: http://localhost:6060


Potential for a small improvement: if this env var is unset, auto-detect it based on E2E_TEST_ENV?

Right now we rely on all those env variables being set here. For easier debugging and for consistent behavior the env variables were taken over from the workflow that runs the daily cronjob on stage.

There is a task to audit and consolidate the workflow env variables as a next step.

Vinnl · 2024-09-25T10:38:44Z

.github/workflows/e2e_pr_full.yml

+          ONEREP_API_KEY: ${{ secrets.ONEREP_API_KEY }}
+          NEXTAUTH_SECRET: ${{ secrets.NEXTAUTH_SECRET }}
+          NEXTAUTH_URL: ${{ secrets.NEXTAUTH_URL }}
+          DATABASE_URL: postgres://postgres:postgres@localhost:5432/blurts


This doesn't need to be set either, right? Since it's already set in .env?

See comment above: #4996 (comment).

Vinnl · 2024-09-25T10:43:10Z

.github/workflows/e2e_pr_full.yml

+          E2E_TEST_ACCOUNT_EMAIL: ${{ secrets.E2E_TEST_ACCOUNT_EMAIL }}
+          E2E_TEST_ACCOUNT_EMAIL_ZERO_BREACHES: ${{ secrets.E2E_TEST_ACCOUNT_EMAIL_ZERO_BREACHES }}
+          E2E_TEST_ACCOUNT_EMAIL_EXPOSURES_STARTED: ${{ secrets.E2E_TEST_ACCOUNT_EMAIL_EXPOSURES_STARTED }}
+          E2E_TEST_ACCOUNT_PASSWORD: ${{ secrets.E2E_TEST_ACCOUNT_PASSWORD }}
+          E2E_TEST_PAYPAL_LOGIN: ${{ secrets.E2E_TEST_PAYPAL_LOGIN }}
+          E2E_TEST_PAYPAL_PASSWORD: ${{ secrets.E2E_TEST_PAYPAL_PASSWORD }}


Can we just add these to .env? I don't think they're sensitive right - worst case, someone can abuse them to make our e2e tests fail.

Not sensitive but I think we'd want to keep them private. Someone can reset the password for these emails and it'd be annoying for us.

.github/workflows/e2e_pr_smoke.yml

src/e2e/pages/welcomeScanPage.ts

src/e2e/specs/auth.spec.ts

src/e2e/specs/dashboard.spec.ts

github-actions · 2024-09-26T15:42:30Z

Cleanup completed - database 'blurts-server-pr-4996' destroyed, cloud run service 'blurts-server-pr-4996' destroyed

flozia self-assigned this Aug 30, 2024

flozia added the 🛑 Do Not Merge Do not merge this PR, even if approved. label Aug 30, 2024

flozia force-pushed the e2e-convert-cron-to-pr branch 2 times, most recently from cf6f9b5 to 6a7fc3f Compare August 30, 2024 11:29

feat: Add Github workflow to run the full e2e test suite on PRs

edd158c

flozia force-pushed the e2e-convert-cron-to-pr branch 12 times, most recently from c096357 to 67367f7 Compare September 2, 2024 19:24

flozia added 3 commits September 3, 2024 13:57

chore: Run E2E tests in parallel

e088792

chore: Run E2E tests in parallel

7e40cfd

Merge branch 'main' into e2e-convert-cron-to-pr

553f582

flozia force-pushed the e2e-convert-cron-to-pr branch 2 times, most recently from d4e90a3 to c2ee1db Compare September 3, 2024 16:29

chore: Update success URLs for E2E tests

a540ed9

flozia force-pushed the e2e-convert-cron-to-pr branch from c2ee1db to a540ed9 Compare September 3, 2024 16:36

flozia added 2 commits September 3, 2024 18:37

chore: Update success URLs for E2E tests

91068dc

Merge branch 'main' into e2e-convert-cron-to-pr

848b7dd

flozia force-pushed the e2e-convert-cron-to-pr branch from 848b7dd to 91068dc Compare September 4, 2024 13:25

merge: main -> e2e-convert-cron-to-pr

c37e0b3

flozia force-pushed the e2e-convert-cron-to-pr branch from 03d949d to 5a7f7f3 Compare September 4, 2024 14:18

flozia requested review from mansaj and pdehaan September 17, 2024 12:03

flozia removed the 🛑 Do Not Merge Do not merge this PR, even if approved. label Sep 17, 2024

fix: Update node version for full E2E test suite

8d3813b

flozia force-pushed the e2e-convert-cron-to-pr branch from f5177a9 to 8d3813b Compare September 17, 2024 12:08

flozia changed the title ~~WIP: Run full E2E test suite on PRs~~ Run full E2E test suite on PRs Sep 17, 2024

Merge branch 'main' into e2e-convert-cron-to-pr

7e556ce

flozia requested a review from rhelmer September 18, 2024 14:35

Merge branch 'main' into e2e-convert-cron-to-pr

cb8591a

flozia commented Sep 18, 2024

View reviewed changes

.github/workflows/e2e_pr_full.yml Show resolved Hide resolved

.github/workflows/e2e_pr_smoke.yml Show resolved Hide resolved

playwright.config.js Show resolved Hide resolved

playwright.config.js Show resolved Hide resolved

playwright.config.js Show resolved Hide resolved

mansaj reviewed Sep 18, 2024

View reviewed changes

flozia requested a review from mansaj September 18, 2024 19:43

Merge branch 'main' into e2e-convert-cron-to-pr

53b6dfa

mansaj approved these changes Sep 23, 2024

View reviewed changes

rhelmer approved these changes Sep 23, 2024

View reviewed changes

Vinnl approved these changes Sep 25, 2024

View reviewed changes

chore: Lower timeout for full E2E test suite

52d8b8f

flozia force-pushed the e2e-convert-cron-to-pr branch 5 times, most recently from 26854a3 to 52d8b8f Compare September 25, 2024 15:35

flozia and others added 4 commits September 25, 2024 17:36

merge: main -> e2e-convert-cron-to-pr

b0b4d0b

chore: Remove auth path for pre Plus users in E2E

319fc47

chore: Rename E2E test workflows

bc7962c

Merge branch 'main' into e2e-convert-cron-to-pr

a1d45f4

flozia merged commit 9305737 into main Sep 26, 2024
16 checks passed

flozia deleted the e2e-convert-cron-to-pr branch September 26, 2024 15:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run full E2E test suite on PRs #4996

Run full E2E test suite on PRs #4996

flozia commented Aug 30, 2024 •

edited by jira bot

Loading

github-actions bot commented Aug 30, 2024

mansaj Sep 18, 2024

flozia Sep 18, 2024

mansaj Sep 23, 2024

flozia Sep 24, 2024

mansaj commented Sep 18, 2024

Vinnl left a comment

Vinnl Sep 25, 2024

flozia Sep 25, 2024

Vinnl Sep 25, 2024

flozia Sep 25, 2024

Vinnl Sep 25, 2024

Vinnl Sep 25, 2024

flozia Sep 26, 2024 •

edited

Loading

Vinnl Sep 25, 2024

flozia Sep 26, 2024

Vinnl Sep 25, 2024

mansaj Sep 25, 2024

github-actions bot commented Sep 26, 2024

		@@ -1,4 +1,4 @@
		name: Monitor e2e Smoke Tests
		name: Monitor E2E Test Suite (smoke)

Run full E2E test suite on PRs #4996

Run full E2E test suite on PRs #4996

Conversation

flozia commented Aug 30, 2024 • edited by jira bot Loading

References:

Description

github-actions bot commented Aug 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mansaj commented Sep 18, 2024

Vinnl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flozia Sep 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Sep 26, 2024

flozia commented Aug 30, 2024 •

edited by jira bot

Loading

flozia Sep 26, 2024 •

edited

Loading