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(e2e): isolate E2E tests and remove destructive actions #7132

Merged
merged 2 commits into from
Mar 26, 2025

Conversation

ndhoule
Copy link
Contributor

@ndhoule ndhoule commented Mar 25, 2025

The E2E suite has a litany of problems, including that it is fairly
complex, doesn't work very well locally and destructively modifies the
workspace, and doesn't isolate tests from each other particularly well.
The tests rely on wrapper scripts to work properly, which makes it
unnecessarily difficult to run E2E tests.

This changeset makes a few changes to the E2E test to help address these
issues The tl;dr is that tests are now self contained without any need
for wrapper scripts and are better isolated. Other notable changes
include:

  • Each test now gets its own isolated Verdaccio registry; the Verdaccio
    registry storage is no longer shared between tests and is no longer
    persisted between test invocations.
  • The CLI is now published to the (isolated) Verdaccio registry from a
    temporary workspace, which is a copy of the project workspace. We no
    longer publish to Verdaccio from the project workspace. This ensures
    that destructive actions that occur on publish don't alter the
    workspace, and ensures that one test can't modify the registry in a
    way that impacts another test.
  • The tests no longer rely on wrapper scripts (which ran vitest in a
    subprocess and didn't always work locally (depending on your PATH).
  • I also converted the tests to TypeScript while I was at it.

Theoretically, these changes mean we can run E2E tests concurrently,
though I haven't done that in this changeset. We could also now merge
the E2E vitest configuration into the primary Vitest config (though I
haven't done that here, either).

Base automatically changed from more-unreversion to main March 25, 2025 19:52
@ndhoule ndhoule force-pushed the test/e2e-test-isolation-local-dev-experience branch from dfd17db to 51871e5 Compare March 25, 2025 19:52
Copy link

github-actions bot commented Mar 25, 2025

📊 Benchmark results

Comparing with 2fba447

  • Dependency count: 1,171 ⬆️ 0.17% increase vs. 2fba447
  • Package size: 284 MB ⬆️ 0.09% increase vs. 2fba447
  • Number of ts-expect-error directives: 716 ⬆️ 0.14% increase vs. 2fba447

@ndhoule ndhoule force-pushed the test/e2e-test-isolation-local-dev-experience branch from 51871e5 to d7c1547 Compare March 25, 2025 20:00
@ndhoule ndhoule marked this pull request as ready for review March 25, 2025 21:37
@ndhoule ndhoule requested a review from a team as a code owner March 25, 2025 21:37
@ndhoule ndhoule force-pushed the test/e2e-test-isolation-local-dev-experience branch from c57cccb to 1df5817 Compare March 25, 2025 21:42
Copy link
Collaborator

@serhalp serhalp left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for improving it! 💯

@ndhoule ndhoule force-pushed the test/e2e-test-isolation-local-dev-experience branch 3 times, most recently from 1c3f054 to 038d440 Compare March 26, 2025 20:16
@ndhoule ndhoule enabled auto-merge (rebase) March 26, 2025 20:16
ndhoule added 2 commits March 26, 2025 14:57
The E2E suite has a litany of problems, including that it is fairly
complex, doesn't work very well locally and destructively modifies the
workspace, and doesn't isolate tests from each other particularly well.
The tests rely on wrapper scripts to work properly, which makes it
unnecessarily difficult to run E2E tests.

This changeset makes a few changes to the E2E test to help address these
issues The tl;dr is that tests are now self contained without any need
for wrapper scripts and are better isolated. Other notable changes
include:

- Each test now gets its own isolated Verdaccio registry; the Verdaccio
  registry storage is no longer shared between tests and is no longer
  persisted between test invocations.
- The CLI is now published to the (isolated) Verdaccio registry from a
  temporary workspace, which is a copy of the project workspace. We no
  longer publish to Verdaccio from the project workspace. This ensures
  that destructive actions that occur on publish don't alter the
  workspace, and ensures that one test can't modify the registry in a
  way that impacts another test.
- The tests no longer rely on wrapper scripts (which ran `vitest` in a
  subprocess and didn't always work locally (depending on your `PATH`).
- I also converted the tests to TypeScript while I was at it.

Theoretically, these changes mean we can run E2E tests concurrently,
though I haven't done that in this changeset. We could also now merge
the E2E vitest configuration into the primary Vitest config (though I
haven't done that here, either).
@ndhoule ndhoule force-pushed the test/e2e-test-isolation-local-dev-experience branch from 038d440 to 82f3522 Compare March 26, 2025 21:57
@ndhoule ndhoule merged commit c4b9e4d into main Mar 26, 2025
52 checks passed
@ndhoule ndhoule deleted the test/e2e-test-isolation-local-dev-experience branch March 26, 2025 22:13
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