Replace local tests with E2E tests #60
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andrewiggins
commented
Aug 12, 2020
| @@ -64,6 +64,6 @@ function run(str, opts, isWatch) { | |||
| '\n' | |||
| ); | |||
| } | |||
| process.exit(err.code || 1); | |||
| process.exit(typeof err.code == 'number' ? err.code : 1); | |||
There was a problem hiding this comment.
Caught this bug while running these tests - some NodeJS errors have the .code property set to a string, which if you pass here will cause the process to exit cleanly
andrewiggins
force-pushed
the
e2e-tests
branch
4 times, most recently
from
66f8f4d to
a83f6f6
Compare
marvinhagemeister
approved these changes
Aug 13, 2020
Can rely on --preserve-symlinks-main flag when things get complicated (i.e. uninstalling karmatic's devDep on webpack)
So task fails if test fail
andrewiggins
force-pushed
the
e2e-tests
branch
from
a83f6f6 to
6c923e2
Compare
|
To unblock rollup integration, I'm going to go ahead and merge this change, but I'm happy to address any other concerns people have. Just @-mention me :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While working on #59, I wanted a way to test that our rollup code worked as expected. That rabbit hole lead me here 😅
This PR replaces the "local" test suite we had with what I'm calling E2E tests. The
e2e-testdirectory contains sub directories that model how a user of karmatic might use karmatic. Each subdir contains a package.json, some source code, and some tests. To run the tests we just runnpm installin each directory and runkarmaticin that directory (therun-e2e-tests.mjsscript automates this)Each e2e test package.json includes karmatic using a local
file:../..reference so the local karmatic source is symlinked into the node_modules folder of the e2e tests so they test the local build output.While this kind of test is a little overkill for karmatic now, I think it really becomes useful in light of #59 because of how we do webpack/rollup detection. Since we determine which bundler to use based on what is installed, we need to run tests in directories that either have webpack or rollup installed, not both. Because of this, we can't just install webpack and rollup and devDeps cause our local tests will only ever run using webpack since that is the preferred bundler if it present. This testing infra allows us to use different kinds of setups to verify karmatic works as expected.
I hope a lot of this becomes easier with npm v7's workspace support.
EDIT: Oh, I forgot to mention - the
webpack-defaulttest includes all the tests that were previous test suite