Test glob support #2359

pugnascotia · 2016-12-19T15:47:00Z

Implement a new testGlob configuration option. This allows test files to be matched using extended glob patterns instead of regular expressions, as implemented by micromatch.

Per #926, globs are a frequently-used tool for matching files, and can be easier to use for filename matching. Other popular tools also use globs instead of regexs.

testRegex is still supported in this PR, but testGlob becomes the favoured option.

Existing tests have been modified where necessary to make them pass. New test cases have been added to SearchSource-test.js to check that glob matching works in addition to regex matching. A test has also been added to normalize-test.js to check that a deprecation warning is printed to the console.

codecov-io · 2016-12-19T16:00:35Z

Codecov Report

Merging #2359 into master will increase coverage by 0.09%.

@@            Coverage Diff            @@
##           master   #2359      +/-   ##
=========================================
+ Coverage    67.9%     68%   +0.09%     
=========================================
  Files         140     140              
  Lines        5057    5072      +15     
=========================================
+ Hits         3434    3449      +15     
  Misses       1623    1623

Impacted Files	Coverage Δ
packages/jest-editor-support/src/Settings.js	`0% <ø> (ø)`	✅
packages/jest-config/src/validConfig.js	`100% <ø> (ø)`	✅
packages/jest-runtime/src/transform.js	`87.87% <ø> (ø)`	✅
packages/jest-runtime/src/index.js	`86% <ø> (-0.06%)`	❌
packages/jest-config/src/defaults.js	`100% <ø> (ø)`	✅
packages/jest-matchers/src/toThrowMatchers.js	`78.84% <ø> (ø)`	✅
packages/jest-validate/src/validate.js	`100% <100%> (ø)`	✅
packages/jest-cli/src/SearchSource.js	`61.62% <100%> (+4.48%)`	✅
packages/jest-config/src/normalize.js	`86.01% <100%> (+0.4%)`	✅
packages/jest-util/src/messages.js	`73.33% <100%> (+0.36%)`	✅
... and 2 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 852ac99...0dddf39. Read the comment docs.

pksjce · 2016-12-19T17:33:32Z

packages/jest-cli/src/__tests__/SearchSource-test.js

+    // micromatch doesn't support '..' through the globstar ('**') to avoid
+    // infinite recursion.
+
+    it('supports ../ paths and unix separators via textRegex', () => {


testRegex is not used in this test at all?

Good catch, this used to rely on the default, so I've changed it to explicitly set a regex.

develar · 2017-01-05T17:04:44Z

Is it going to be merged?

pugnascotia · 2017-01-06T09:02:04Z

The CI job is failing and I don't understand why - anyone have any suggestions?

cpojer · 2017-01-06T09:38:05Z

There is some issue with eslint on travis, I'll figure it out. @pugnascotia thanks for working on this, it'll take me a bit longer to get to this PR as there are currently 20+ open requests and I'm still catching up with work at FB after my vacation.

pugnascotia · 2017-01-06T09:38:46Z

No worries, just pleased to help out.

pugnascotia · 2017-01-15T21:59:22Z

Any luck figuring out what was up with CI? Do you want me to merge up to master again?

cpojer · 2017-01-16T11:50:59Z

Do you mind rebasing this?

pugnascotia · 2017-01-16T12:35:29Z

I've rebased and squashed, and npm test passes, though the PR seems stuck on thinking there are still conflicts.

cpojer · 2017-01-16T12:36:06Z

Are you sure you rebased against Jest's master branch? It doesn't seem like it.

pugnascotia · 2017-01-16T12:46:15Z

What...I'm sure I fetched before rebasing. Weird. OK, should be there now.

thymikee · 2017-01-16T12:47:20Z

@pugnascotia maybe you fetched from your fork's master, happens to me sometimes 😄

jest-bot · 2017-01-18T13:36:10Z

	Warnings
⚠️	Changes were made to package.json, but not to yarn.lock - Perhaps you need to run `yarn install`?

Generated by 🚫 dangerJS

pugnascotia · 2017-01-18T13:57:33Z

I've rebased (again) onto master, and this time I've actually fixed the problems that arose. In particular, in commit b4f65e1 I altered the order in which config keys are validated, so that keys that are still in force but are deprecated will trigger warnings.

Re: the package.json change, there isn't a dep change so it's right that yarn.lock hasn't been updated.

cpojer · 2017-01-21T14:53:47Z

packages/jest-util/src/messages.js

 import type {AssertionResult, TestResult} from 'types/TestResult';

 const chalk = require('chalk');
+const mm = require('micromatch');


can we call this variable micromatch everywhere please?

cpojer · 2017-01-21T14:54:43Z

packages/jest-runtime/src/shouldInstrument.js

@@ -23,6 +23,11 @@ const shouldInstrument = (filename: Path, config: Config): boolean => {
    return false;
  }

+  if (config.testGlob && config.testGlob.length
+    && micromatch.any(filename, config.testGlob)) {


the code style in Jest is:

if ( config.testGlob && config.testGlob.length && micromatch.any(filename, config.testGlob) ) {

cpojer · 2017-01-21T14:55:18Z

packages/jest-runtime/src/index.js

@@ -116,7 +114,6 @@ class Runtime {
    this._mocksPattern =
      config.mocksPattern ? new RegExp(config.mocksPattern) : null;
    this._shouldAutoMock = config.automock;
-    this._testRegex = new RegExp(replacePathSepForRegex(config.testRegex));


Nice, this seems to have been dead code.

cpojer · 2017-01-21T14:55:37Z

packages/jest-editor-support/src/Settings.js

+      testGlob: [
+        '**/__tests__/**/*.js?(x)',
+        '**/?(*.)(spec|test).js?(x)',
+      ],
      testRegex: '(/__tests__/.*|\\.(test|spec))\\.jsx?$',


Why do we need both?

Um...we probably don't :-)

cpojer · 2017-01-21T14:56:08Z

packages/jest-config/src/normalize.js

+  if (config.testRegex && (!config.testGlob || config.testGlob.length === 0)) {
+    // Prevent default glob conflicting with any explicit `testRegex` value
+    newConfig.testGlob = [];
+  }


Seems like we should warn if both are given?

Wait, I don't really understand this. Why are you setting testGlob to an empty array if testGlob is empty?

Sorry, that's little messy, I'll tidy that up.

cpojer · 2017-01-21T14:58:04Z

.gitignore

@@ -2,6 +2,8 @@
 .eslintcache
 *.swp
 *~
+.idea
+*.iml


do you mind removing these here and instead moving these into a global gitignore config for your computer?

cpojer · 2017-01-21T15:00:58Z

packages/jest-config/src/normalize.js

+
+      case 'testRegex':
+        value = config[key];
+        break;


why did you add this as a separate option in this switch case?

Gah, that's a leftover from an earlier implementation, when the deprecation warnings were generated here. Good catch.

cpojer · 2017-01-21T15:03:14Z

Finally had some time to look at this. Am I understanding this correctly – it'll support both testGlob and testRegex and have the following behaviors:

Use both testGlob and testRegex if they are specified.
Print a deprecation warning for testRegex.

?

pugnascotia · 2017-01-22T21:20:52Z

That was my thinking, yes. Since we can't (AFAIK) turn an arbitrary regex into a glob, I felt it was best to allow testRegex but print a warning, them remove it entirely in a later release.

pugnascotia · 2017-01-23T10:54:25Z

That's the branch rebased onto facebook/master again.

cpojer · 2017-01-23T23:21:36Z

I feel like we shouldn't show the deprecation warning for testRegex yet. Instead, I'd recommend this:

warn/throw if both testGlob and testRegex are given.
don't warn about use of testRegex. We'll visibly deprecate it in two majors from now to reduce the churn.

Are there any better names than "testGlob"? Any way we can drop the "glob" from it? I don't like it, especially as we'll be ending up with a singular option in the future :)

thymikee · 2017-01-24T10:33:35Z

maybe testPattern? But this may be misleading, as we use the word pattern in watch mode and it's for regex.

edit:
or testMatch

pugnascotia · 2017-01-24T11:28:14Z

Rebased again and updated as per your comments. I liked testMatch so I went with that.

perl -p -i -e 's/testGlob/testMatch/g' $(git grep -l testGlob)

The existing validation in jest-validate did not allow for fields that will be replaced but still have an effect, owing to the order in which the validation is carried out. Swap the order so that the deprecation check comes first.

pugnascotia · 2017-01-30T10:56:21Z

Rebased onto master again - any more feedback on this?

cpojer · 2017-02-01T14:29:46Z

Thank you for working on this and for being so thorough. Sorry for the delay in reviews. This will be in Jest 19.

* Validate for deprecated fields first The existing validation in jest-validate did not allow for fields that will be replaced but still have an effect, owing to the order in which the validation is carried out. Swap the order so that the deprecation check comes first. * Implement testGlob option * Fixes from code review * Rename testGlob to testMatch, and remove testMatch deprecation * Forbid testMatch and testRegex being used together * Update SearchSource.js * Update messages.js

github-actions · 2021-05-14T00:21:07Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

pugnascotia mentioned this pull request Dec 19, 2016

[proposal] use globs instead of regex in config #926

Closed

facebook-github-bot added the CLA Signed ✔️ label Dec 19, 2016

pksjce reviewed Dec 19, 2016

View reviewed changes

pugnascotia force-pushed the test-glob-support branch 2 times, most recently from af14c3d to f02cc5b Compare January 16, 2017 12:33

pugnascotia changed the title ~~WIP - Test glob support~~ Test glob support Jan 16, 2017

pugnascotia force-pushed the test-glob-support branch from f02cc5b to e0d73bb Compare January 16, 2017 12:41

pugnascotia force-pushed the test-glob-support branch 2 times, most recently from 5891d3e to 1b88af0 Compare January 18, 2017 13:25

cpojer reviewed Jan 21, 2017

View reviewed changes

pugnascotia force-pushed the test-glob-support branch 2 times, most recently from 1cb273a to 1b0f158 Compare January 23, 2017 10:28

cpojer mentioned this pull request Jan 24, 2017

testPathDirs does not work with React Native, but testRegex does #2670

Closed

pugnascotia force-pushed the test-glob-support branch from 90b0f57 to 9abfc68 Compare January 24, 2017 11:25

pugnascotia added 5 commits January 30, 2017 10:26

Validate for deprecated fields first

80999fc

The existing validation in jest-validate did not allow for fields that will be replaced but still have an effect, owing to the order in which the validation is carried out. Swap the order so that the deprecation check comes first.

Implement testGlob option

52d6e82

Fixes from code review

fdf41ce

Rename testGlob to testMatch, and remove testMatch deprecation

29bc905

Forbid testMatch and testRegex being used together

0dddf39

pugnascotia force-pushed the test-glob-support branch from 9abfc68 to 0dddf39 Compare January 30, 2017 10:34

cpojer added 2 commits February 1, 2017 14:23

Update SearchSource.js

05635d1

Update messages.js

f80e0b7

cpojer merged commit 2464593 into jestjs:master Feb 1, 2017

github-actions bot locked as resolved and limited conversation to collaborators May 14, 2021

Test glob support #2359

Test glob support #2359

Conversation

pugnascotia commented Dec 19, 2016

codecov-io commented Dec 19, 2016 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

develar commented Jan 5, 2017

pugnascotia commented Jan 6, 2017

cpojer commented Jan 6, 2017

pugnascotia commented Jan 6, 2017

pugnascotia commented Jan 15, 2017

cpojer commented Jan 16, 2017

pugnascotia commented Jan 16, 2017

cpojer commented Jan 16, 2017

pugnascotia commented Jan 16, 2017

thymikee commented Jan 16, 2017

jest-bot commented Jan 18, 2017 • edited Loading

pugnascotia commented Jan 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cpojer commented Jan 21, 2017

pugnascotia commented Jan 22, 2017

pugnascotia commented Jan 23, 2017 • edited Loading

cpojer commented Jan 23, 2017

thymikee commented Jan 24, 2017 • edited Loading

pugnascotia commented Jan 24, 2017

pugnascotia commented Jan 30, 2017

cpojer commented Feb 1, 2017

github-actions bot commented May 14, 2021

codecov-io commented Dec 19, 2016 •

edited

Loading

jest-bot commented Jan 18, 2017 •

edited

Loading

pugnascotia commented Jan 23, 2017 •

edited

Loading

thymikee commented Jan 24, 2017 •

edited

Loading