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

Execute E2E tests more frequently #3499

Merged
merged 3 commits into from
Jun 27, 2023
Merged

Conversation

michalinacienciala
Copy link
Contributor

@michalinacienciala michalinacienciala commented Jun 23, 2023

So far, we've been running E2E tests only when a PR was changing feature flag, contained e2e in the name of the feature branch or got merged to main. This prevented us from noticing changes braking E2E tests fast enough - we sometimes were noticing such changes only after they got already merged to main. In this commit we modify CI config in a way that most of the E2E tests will be run on every PR update. We also allow for specifying tests which shouldn't be run this often - such tests should be marked with @slow tag in the test name (read more here: https://playwright.dev/docs/test-annotations#tag-tests). We may want to use this feature when we'll have tests that require spending some funds.

Note: depending on which PR gets merged first, we may need to update either this PR or #3418.

Latest build: extension-builds-3499 (as of Tue, 27 Jun 2023 01:23:53 GMT).

@michalinacienciala michalinacienciala self-assigned this Jun 23, 2023
So far, we've been running E2E tests only when a PR was changing feature flag,
contained `e2e` in the name of the feature branch or got merged to `main`. This
prevented us from noticing changes braking E2E tests fast enough - we sometimes
were noticing such changes only after they got already merged to `main`.
In this commit we modify CI config in a way that most of the E2E tests will be
run on every PR update. We also allow for specifying tests which shouldn't be
run this often - such tests should be marked with `@slow` tag in the test name
(read more here: https://playwright.dev/docs/test-annotations#tag-tests). We
may want to use this feature when we'll have tests that require spending some
funds.
When no `@slow`-tagged tests are found, the step is failing.
Copy link
Contributor

@Shadowfiend Shadowfiend left a comment

Choose a reason for hiding this comment

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

Any reason not to land this fellow now?

Copy link
Contributor

@Shadowfiend Shadowfiend left a comment

Choose a reason for hiding this comment

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

Don't really see a reason to block here so we're going to land this fellow.

- name: Run Playwright tests
run: xvfb-run npx playwright test
# Some tests that we have configured in the `e2e-tests` folder may require
# spending funds. Although they're desined to not spend much, with
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# spending funds. Although they're desined to not spend much, with
# spending funds. Although they're designed to not spend much, with

# frequent execution that can accumulate. We don't want to execute such
# tests on every PR update. We'll tag those tests with the `@slow` tag.
- name: Run free Playwright tests
run: xvfb-run npx playwright test --grep-invert @slow
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use yarn instead of npx for consistency/simplicity across the repo.

@Shadowfiend Shadowfiend merged commit b2abea9 into main Jun 27, 2023
@Shadowfiend Shadowfiend deleted the run-e2e-tests-more-frequently branch June 27, 2023 01:20
@hyphenized hyphenized mentioned this pull request Jun 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants