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

feature: Support array of globs as testLocator #45

Closed
wants to merge 1 commit into from

Conversation

docnoe
Copy link

@docnoe docnoe commented Aug 16, 2019

It's sometimes very complicated, if not impossible to construct a glob string that matches all test files in some locations. This change makes it simpler by supporting arrays of globs.
E.g. in package.json the "testLocator" property can now look like this:

"testLocator": [
  "./@(models|routes|middleware)/**/*.test.js",
  "./config.test.js",
  "./node_modules/my-custom-module/**/*.test.js"
]

A single string is still supported, of course.

It's sometimes very complicated, if not impossible to construct a glob string that matches all test files in some locations. This change makes it simpler by supporting arrays of globs. 
E.g. in package.json the "testLocator" property can now look like this:
```json
[
"./@(models|routes|middleware)/**/*.test.js",
"./config.test.js"
"./node_modules/my-custom-module/**/*.test.js"
]
```
A single string is still supported, of course.
@searls
Copy link
Member

searls commented Aug 19, 2019

My only concern here is that it seems to deviate from the implementation #44 would take us (which is to rely on shell expansion as the final argument). I think @cpruitt & @agent-0028 were working on that one. Maybe they care to weigh in?

This seems to shed light on a path of just splitting the module's behavior: if invoked with > 0 paths, use those. If not, look for this testLocator and use the glob package as this PR does in package.json. If neither, run the default.

@cpruitt
Copy link
Contributor

cpruitt commented Aug 20, 2019

Based on what we're currently working on, shell expansion will probably mean that load will end up receiving an array of files for which shell/glob expansion has already been performed. Moving globs to the shell, we can take advantage of all of the shell's expansions. In the current work in progress we still have a separate command line argument for --testLocator which is intended to function the same way as the quoted path string does now.

I'm putting some time into this this week so I'd be curious about whether there is a specific use case for needing --testLocator to be able to support an array and parse out globs apart from shell expansion the way it does now. If so, I'd like to take that feedback into account.

@searls
Copy link
Member

searls commented Aug 20, 2019

Ok, @cpruitt I hereby declare you in charge of this series of related features. @docnoe I ask for your patience :)

@searls
Copy link
Member

searls commented Mar 14, 2020

I believe this was implemented as part of #47, right @cpruitt?

@searls searls closed this Mar 14, 2020
@cpruitt
Copy link
Contributor

cpruitt commented Mar 14, 2020

@searls Yep, that’s correct.

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.

3 participants