fix(cli): Remove default files from CLI #876
Conversation
|
I hope this could be patch-released soonish because the feature was advertized as part of the most recent release but does not actually work :-/ |
|
Yes. I want to land this ASAP (i.e. add some tests then let's publish). Here is the test from Also, see these tests. Those launch a new process for the CLI (which in turn launches test processes), so they aren't fast tests. We could do one or two just to verify it's all glued together correctly, but we should build up a more thorough set of faster tests that fully exercise For now, let's just land one of each and ship a patch release. |
kentcdodds
force-pushed
the
pr/remove-default-from-cli
branch
from
9c88f3a to
ce6a22d
Compare
|
I updated with one test. I think it's pretty solid. Let me know if you want an integration test with the CLI as well. |
kentcdodds
force-pushed
the
pr/remove-default-from-cli
branch
from
ce6a22d to
79d6104
Compare
|
Linter errors! |
|
How embarrassing! |
The default files should be in one place (`ava-files.js`). Right now the defaults provided in `ava-files.js` aren't being used because the CLI pre-populates them. Closes avajs#875
kentcdodds
force-pushed
the
pr/remove-default-from-cli
branch
from
79d6104 to
4e22ee1
Compare
|
Fixed! |
| @@ -104,3 +104,29 @@ test('findFiles - does not return duplicates of the same file', function (t) { | |||
| t.end(); | |||
| }); | |||
| }); | |||
|
|
|||
| test('findFiles - finds the correct files by default', function (t) { | |||
| var avaFiles = new AvaFiles(['**/ava-files/default-patterns/**']); | |||
There was a problem hiding this comment.
Argh. I'm realizing this is actually kinda hard to test.
This will actually find every file in the default-patterns folder.
Instead of this argument, can you just process.chdir() to the default-patterns folder, and drop the constructor argument?
You are technically overriding the default here.
#863 introduce cwd and resolveTestsFrom options, that would make this doable without changing the directory.
There was a problem hiding this comment.
Also, you should sprinkle in a few files that should NOT be found (and ensure they are not).
There was a problem hiding this comment.
I can do that. Unfortunately I've run out of time. Someone can feel free to branch off of my branch and make the improvements in another PR if it needs to be fixed sooner than I can get to it.
There was a problem hiding this comment.
Sorry Kent,
I see why you did it this way - It's exactly like the test I told you to copy. I'm updating your PR now. Expect a new PR and release soon.
There was a problem hiding this comment.
Thanks. It's family time for me now. Sorry I didn't quite get to it.
|
I'm not sure what's wrong with the windows build. It does look my test is causing the issue, but I'm not sure why... |
| avaFiles.findTestFiles().then(function (files) { | ||
| var allFilesFound = filesToFind.every(function (fileToFind) { | ||
| return files.some(function (file) { | ||
| return endsWith(file, fileToFind); |
There was a problem hiding this comment.
Use slash to convert the results from \ results, to / results (I think).
|
Closing in favor of #878 Thanks @kentcdodds |
WIP: Not sure where to put tests/how to test this.