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

Cloud Run npm test vs. npm run e2e-test #1493

Closed
grayside opened this issue Sep 23, 2019 · 2 comments
Closed

Cloud Run npm test vs. npm run e2e-test #1493

grayside opened this issue Sep 23, 2019 · 2 comments
Labels
type: cleanup An internal cleanup or hygiene concern.

Comments

@grayside
Copy link
Collaborator

Cloud Run testing using the convention of test and e2e tests as two different testing types. This was my original intention for a couple reasons:

  • Minimum possible external dependencies on shell scripts, environment variables, or external services for npm test
  • Minimum wall time for npm test (unit tests take seconds, e2e tests take minutes)
  • High visibility in reviewing CI logs on the distinction between a unit test failing and an end-to-end test failing

This has led to a couple odd things:

  1. npm test does not run all the tests
  2. The Manual Logging sample and the forthcoming hello-broken sample (run: add hello-broken sample #1480) do not have unit tests, and so carry a no-op configuration for npm test. This is a bit odd.

To converge on npm test doing all the things while preserving some of the above goals, here's what we might do:

  1. Converge on npm test: for samples with e2e-test configured this would look something like: test/runner.sh mocha test/*.test.js --timeout=20000
  2. Within tests, top-level describe statements should be rigorous about stating "Unit Tests", "Integration Tests", and "End-to-End Tests" for visibility of the test results.
  3. Add an environment variable to skip e2e tests for those times when they will not work (e.g., offline)
  4. Remove npm run --if-present e2e-test from .kokoro/build-with-run.sh
  5. Modify sample README instructions on running e2e tests to running tests generally

@grant carry-over from #1480.

@fhinkel
Copy link
Contributor

fhinkel commented Sep 25, 2019

I'd prefer if we call them system-test instead of e2e. That's a more common term. I'm fine with splitting into test and system-test.

@JustinBeckwith JustinBeckwith added 🚨 This issue needs some love. triage me I really want to be triaged. type: cleanup An internal cleanup or hygiene concern. and removed 🚨 This issue needs some love. triage me I really want to be triaged. labels Oct 3, 2019
@fhinkel
Copy link
Contributor

fhinkel commented Nov 18, 2019

Closing this due to inactivity. Please reopen if still needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: cleanup An internal cleanup or hygiene concern.
Projects
None yet
Development

No branches or pull requests

3 participants