Don't rebuild packages when test file gets updated

[gziolo]

As discussed on Slack with @youknowriad, it looks like our watch script for packages is too greedy and rebuilds them when non-production code gets updated: https://wordpress.slack.com/archives/C5UNMSU4R/p1542097900320400

We should exclude test files from the list of files which trigger rebuild of the packages. It looks like we already exclude some types of files, so it might be possible to extend the following function:

gutenberg/bin/packages/watch.js

Lines 25 to 28 in 1de0887

	// Exclude deceitful source-like files, such as editor swap files.
	const isSourceFile = ( filename ) => {
	return /.\.(js|scss)$/.test( filename );
	};

By the way, it looks like the message printed on the console is also wrong. I didn't rename any file but I added a few modifications.

[Jackie6]

@gziolo Hi greg
According to the documentation here, all test files are in the "test" folder. So can we just skip those files in the "test" folder using the following code?

// Exclude test files and deceitful source-like files, such as editor swap files.

const isSourceFile = ( filename ) => {
	return ! /\/test\//.test( filename ) && /.\.(js|scss)$/.test( filename );
};

You mention that the message printed on the console is also wrong.But I do not know how to reproduce this error. When I run npm run dev, the webpack will not watch for changes and rebuild packages even I have modified the code. BTW, I am using varying vagrant vagrants and Mac OS.

[gziolo]

@Jackie6, it's almost true. There are some exceptions with how tests are discovered. There are 3 patterns which can be used:
https://github.com/WordPress/gutenberg/blob/master/packages/jest-preset-default/jest-preset.json#L18-L20
However, the one you shared covers 95% of cases so it would be a good start. We might want to include /__tests__/ as well. Can you open PR with your proposal?

When I run npm run dev, the webpack will not watch for changes and rebuild packages even I have modified the code. BTW, I am using varying vagrant vagrants and Mac OS.

That's surprising, this command starts watch mode so it should trigger changes any time any code is updated inside packages folder according to the rule you are about to update.

[Jackie6]

@gziolo Hi Greg. Thank you for pointing out the test match pattern.
However, the testMatch is not a standard regex, it is based on jest micromatch.

gutenberg/packages/jest-preset-default/jest-preset.json

Lines 17 to 21 in 7779964

	"testMatch": [
	"**/__tests__/**/*.js",
	"**/?(*.)(spec|test).js",
	"**/test/*.js"
	],

So I translate the patterns to regex. I have created a pull request to fix this issue. The modified code is as follows:

// Exclude test files including .js files inside of __tests__ or test folders
// and files with a suffix of .test or .spec (e.g. blocks.test.js),
// and deceitful source-like files, such as editor swap files.
const isSourceFile = ( filename ) => {
	return ! [/\/(__tests__|test)\/.+.js$/, /.\.(spec|test)\.js$/].some( regex => regex.test( filename ) ) && /.\.(js|scss)$/.test( filename );
};