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 and build in separate jobs #351

Merged
merged 6 commits into from
Mar 29, 2021
Merged

Conversation

aminya
Copy link
Contributor

@aminya aminya commented Mar 27, 2021

This allows using the latest tools for building on the latest Node.js while supporting older versions for running the code

Fixes the errors in #349

@aminya aminya force-pushed the separate-build-test branch 3 times, most recently from 23efc8b to 236b759 Compare March 27, 2021 16:46
@aminya aminya marked this pull request as draft March 27, 2021 16:50
@aminya aminya marked this pull request as ready for review March 27, 2021 19:24
@aminya
Copy link
Contributor Author

aminya commented Mar 27, 2021

This is ready now

@andywer
Copy link
Owner

andywer commented Mar 28, 2021

Cool, great idea!

I'm just wondering why the test/ directory has been renamed to test-dev/ and what's the story behind the two npm test scripts? 😉

@aminya
Copy link
Contributor Author

aminya commented Mar 28, 2021

Thanks!

I separated the runtime tests from dev time tests. Dev time tests (such as bundling with Webpack and Rollup) are tested on the latest Node, and all the other tests are tested on old Node. This allows us to use the latest tools while keeping backward compatibility for old runtimes.

@andywer
Copy link
Owner

andywer commented Mar 28, 2021

Sounds reasonable. My only issue is that the npm script names failed to convey their purpose to me…

How about test:library & test:tooling instead? Or maybe test:integration for the latter. WDYT?

Don't want to unnecessarily delay the merge, but since we cannot add comments to the package.json, we should pay a bit of extra attention to the naming there 🙂

@aminya
Copy link
Contributor Author

aminya commented Mar 28, 2021

Sounds reasonable. My only issue is that the npm script names failed to convey their purpose to me…

How about test:library & test:tooling instead? Or maybe test:integration for the latter. WDYT?

Don't want to unnecessarily delay the merge, but since we cannot add comments to the package.json, we should pay a bit of extra attention to the naming there 🙂

Sure, that sounds good to me.

@andywer andywer merged commit d8aa0c7 into andywer:master Mar 29, 2021
@aminya aminya deleted the separate-build-test branch March 29, 2021 17:04
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