Scripts: Provide the default configuration for the test command

[gziolo]

This PR provides zero configuration mode for wp-scripts test-unit command in the case when a project does not have a config for Jest and/or Babel. I also refactored code to make it easier to test. New utility methods were inspired by work done by @kentcdodds in https://github.com/kentcdodds/kcd-scripts. Kudos to him for amazing work and excellent coding skills. It was my pleasure to follow along and cherry pick what we need! 💯

This PR also renames test command to test-unit to better reflect its intent. It was raised by @youknowriad during the integration with Gutenberg. See: WordPress/gutenberg#4705 (comment):

I guess this is more a package issue, but I wonder if this command should be wp-scripts unit-test to allow for other kinds of test commands.

In addition, I noticed some errors in the docs when updating the script name from test to test-unit.

TODO

• Add more tests to cover newly introduced utils.
• Nice to have: add some tests that make sure that wp-scripts throws proper errors when an unknown script name passed as the first argument.

[codecov]

Codecov Report

Merging #83 into master will increase coverage by 3.73%.
The diff coverage is 70.96%.

@@            Coverage Diff             @@
##           master      #83      +/-   ##
==========================================
+ Coverage    71.5%   75.23%   +3.73%     
==========================================
  Files          29       34       +5     
  Lines         393      432      +39     
  Branches       79       84       +5     
==========================================
+ Hits          281      325      +44     
+ Misses         92       89       -3     
+ Partials       20       18       -2

Impacted Files	Coverage Δ
packages/scripts/scripts/test-unit.js	0% <0%> (ø)
packages/scripts/config/jest.config.js	0% <0%> (ø)
packages/scripts/scripts/test-unit-jest.js	0% <0%> (ø)
packages/scripts/config/babel-transform.js	0% <0%> (ø)
packages/scripts/utils/process.js	100% <100%> (ø)
packages/scripts/utils/package.js	100% <100%> (ø)
packages/scripts/utils/index.js	100% <100%> (ø)
... and 4 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 445072e...573e8df. Read the comment docs.

[gziolo]

I want to test if that excludes such files from publishing to npm or this is only used to detect changed files when deciding if bump version. If the latter I will investigate how to prevent those files from being published in a different way.

[gziolo]

It is only useful to prevent publishing a new version when no code related changes are applied. See what gets downloaded from the npm:

test folder is there. I'm wondering if there is a different way to stop publishing test file. We can always move code src and let build script do the file filtering.

[ntwb]

That would work 👍 (i.e. let the build script take care of it)

[ntwb]

I'd like to see this be even more generic, more focused might be a better term.

In considering the widest adoption possible for these scripts instead of test-unit I'd like to see this as test-unit-jest, we could then add test-unit-qunit, test-unit-phpunit, and test-unit-mocha to name but a few for repos that have yet to switch or do not use Jest.

[ntwb]

On a train and got to thinking a little more about what I wrote above, I don’t think we should look to add any zero-config support to the alternatives I listed, rather for example bbPress, currently it only uses PHPUnit, so whilst I plan on adding some Jest tests to bbPress and leverage this awesome Jest zero-config configuration you’ve put together here in this PR @gziolo I’d like to be able to have some standardised task names of sorts in that repo, in this case test-unit-jest and test-unit-phpunit. Likewise for core, test-unit-jest for when Jest is added and the existing test-unit-phpunit and test-unit-qunit tasks.

So I’m looking to be able to support more than just Jest in some scripts, I’m not concerned if the naming scheme is something other than what I’ve suggested above either.

Does that make sense, and what are your thoughts on this @gziolo ?

[gziolo]

I took a peek at Gutenberg's package.json file: https://github.com/WordPress/gutenberg/blob/master/package.json#L132. I didn't think about PHP part at all in this context, but it looks like we have to take it into account, too. I'm fine with having a different naming for scripts, but I'm wondering if we should also provide aliases to signal what we have set as defaults, e.g.:

• wp-scripts test-unit-js ->wp-scripts test-unit-jest
• wp-scripts test-unit-php -> wp-scripts test-unit-phpunit

Otherwise, it is going to be difficult to pick one for someone who is not familiar with all those tools. One more thing to consider here, should we make only wp-scripts test-unit-js capable of having zero-configuration fallback, or should it be included in wp-scripts test-unit-jest, too?

On a train and got to thinking a little more about what I wrote above, I don’t think we should look to add any zero-config support to the alternatives I listed

Yes, I would prefer we support one tool for PHP and one for JS. The same we use in Gutenberg, packages and hopefully in WordPress core.

[gziolo]

Writing tests for CLI scripts is hard, tons of juggling to make sure we can mock env bits :( This is ready for a review. As discussed I renamed wp-scripts test-unit to wp-scripts test-unit-jest and aliased back to the old version :) In theory this package is advertised as A collection of JS scripts for WordPress development. so we still need to discuss if we want to have PHP scripts included in here.

[ntwb]

but I'm wondering if we should also provide aliases to signal what we have set as defaults,

A great idea, could the alias reference multiple scripts?

i.e. Could the alias wp-scripts test-unit-js call both wp-scripts test-unit-jest and wp-scripts test-unit-qunit, this would be great for WP core when the time comes. (Doesn't have to be in this PR, we can add support for this in a future iteration)

zero config fallbacks

Having wp-scripts test-unit-js have a zero-configuration fallback sounds good, it would by default fallback to wp-scripts test-unit-jest would it not? As such adding the zero configuration fallback to wp-scripts test-unit-jest would be the one to add it to wouldn't it?

Yes, I would prefer we support one tool for PHP and one for JS. The same we use in Gutenberg, packages and hopefully in WordPress core.

In a perfect world I'd agree, for WordPress Core QUnit will not be going away anytime soon, if we are to add Jest it will need to coexist with QUnit. I've not taken a close look at what would be required to for WordPress Core to drop QUnit and switch to Jest but I expect this would require a significant amount of work.

[ntwb]

In theory this package is advertised as A collection of JS scripts for WordPress development. so we still need to discuss if we want to have PHP scripts included in here.

WordPress and its sister projects use Grunt & Webpack build tooling within those projects, and as we are already looking to use this repo to simplify some of the complexity of setting up JavaScript build tooling with wp-scripts. wp-scripts I see as a generic collection of JavaScript scripts for WordPress build tools and not limited to JavaScript only build tool tasks. I'd like to try and explore this to include PHP and CSS build tooling in a similar manner, even if the scripts are just wrappers it will hopefully allow for some flexibility of what tasks wp-scripts can perform.

[gziolo]

i.e. Could the alias wp-scripts test-unit-js call both wp-scripts test-unit-jest and wp-scripts test-unit-qunit, this would be great for WP core when the time comes. (Doesn't have to be in this PR, we can add support for this in a future iteration)

We would have to check. I guess we could to do it if we don't want to pass additional params. Otherwise, it might be difficult to distribute those params properly between those 2. What you can do is use npm-run-all and alias them properly in WP core as follows:

{
    "scripts": {
        "test-unit:js:qunit": "wp-scripts test-unit-qunit",
        "test-unit:js:jest": "wp-scripts test-unit-jest",
        "test-unit:js": "npm-run-all test-unit:js:*",
        "test-unit:php": "wp-scripts test-unit-phunit",
        "test": "npm-run-all test-unit:**",
    },
}

Just and idea to show the intent.

Having wp-scripts test-unit-js have a zero-configuration fallback sounds good, it would by default fallback to wp-scripts test-unit-jest would it not? As such adding the zero configuration fallback to wp-scripts test-unit-jest would be the one to add it to wouldn't it?

Yes. What I did here is test-unit-js includes test-unit-jest which provides the zero configuration fallback.

In a perfect world I'd agree, for WordPress Core QUnit will not be going away anytime soon, if we are to add Jest it will need to coexist with QUnit. I've not taken a close look at what would be required to for WordPress Core to drop QUnit and switch to Jest but I expect this would require a significant amount of work.

I migrate Mocha -> Jest for Calypso, over 1 000 files, nearly 10 000 tests and they use a very similar API. So I totally understand your concern. Calypso still uses Chai and Sinon in many places although Jest provides its own alternatives :)

[gziolo]

I'd like to try and explore this to include PHP and CSS build tooling in a similar manner, even if the scripts are just wrappers it will hopefully allow for some flexibility of what tasks wp-scripts can perform.

Yes, that makes sense to have it all available in the long run. 👍

[ntwb]

Yes. What I did here is test-unit-js includes test-unit-jest which provides the zero configuration fallback.

I don't see test-unit-js referenced at all in the PR, it's still test-unit

[omarreiss]

We'd like to merge this as it adds value as is. @gziolo could you resolve the merge conflict and document all the unresolved discussion / ideas in separate issues so those can be separate iterations?