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

Add end-to-end test suite #80

Merged
merged 18 commits into from
Apr 14, 2023
Merged

Add end-to-end test suite #80

merged 18 commits into from
Apr 14, 2023

Conversation

mcous
Copy link
Member

@mcous mcous commented Apr 14, 2023

Overview

This PR adds a Verdaccio-based end-to-end test suite to CI, which spins up an NPM registry on localhost in the runner and tests that:

  • The CLI is able to publish a package
  • The action is able to publish a package
  • The CLI will not publish a package if the version is already published
  • The action will not publish a package if the version is already published

These tests run through a lot of the code, which means we get pretty decent bang for our buck, and we can defer a lot of smaller behaviors to isolated unit tests. Most importantly, they only work if our registry auth logic is working, which is probably the most mission-critical logic in the module.

This PR should merge before anything else lands. I've added the E2E status check as a requirement for any PR merging into the main branch.

Background

The existing test suite of this action can't be used for actual regression protection, because it mocks out the npm binary, in the classical sense. Each test forms a set of expected calls to npm and return values for each of those calls. If any of those calls doesn't happen, or happens in a different manner than specified, the test fails.

From a development standpoint, this has the following downsides:

  • The tests don't tell us if the module actually works
    • Instead, they tell us that the module calls npm in the way we think npm is supposed to be called
    • We could be wrong about how we're supposed to call npm!
  • If we change how we call npm, the tests will automatically fail, even if the changes are correct
    • That means we can't refactor how we call npm without also refactoring the tests
    • Instead of being a stable platform that we develop against, the tests are now just another thing we have to change where we could introduce bugs

Changes

  • Add E2E job to CI, powered by Docker and a few small bash scripts in ./e2e
  • Various CI improvements
    • Run CI weekly rather than monthly
    • Upgrade coveralls action
    • Move common Node.js setup to little composite action
    • Separate linting job from matrixed test job so we don't get duplicate linting warnings
    • Separate build job from publish jobs
    • Make sure E2E tests run against the code that will actually be published (or is committed, in the case of the action) rather than building fresh every time

@mcous mcous added the chore General maintenance task label Apr 14, 2023
@mcous mcous requested a review from razor-x April 14, 2023 16:46
@mcous mcous merged commit 3ee021c into master Apr 14, 2023
@mcous mcous deleted the e2e-tests branch April 14, 2023 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore General maintenance task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant