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

Ava slows when there are many non-run files #1288

Closed
taylor1791 opened this issue Feb 25, 2017 · 11 comments · Fixed by #2103
Closed

Ava slows when there are many non-run files #1288

taylor1791 opened this issue Feb 25, 2017 · 11 comments · Fixed by #2103
Labels
bug current functionality does not work as desired scope:globbing

Comments

@taylor1791
Copy link

taylor1791 commented Feb 25, 2017

Description

I started a project a while ago and I noticed that ava slowed down over time. At the start it used to clock in at under a second. As I added more code and my configuration changed it is now around 15 seconds. This is an extremely long feedback cycle. Using --watch does not appear to produce a significant improvement.

I managed to reproduce the problem in a test repo. Further testing show it has to do with the number of files in the directory. It does not matter what type of files they are. In the example repo, I have a 1 simple test, a copy of node, and a benchmark script. When I remove the copy of node it is significantly faster. Here is a sample run of the script.

user@computer:~/src/ava-slow-test% ./benchmark.sh
Running tests...

  1 passed


real	0m7.377s
user	0m6.863s
sys	0m1.379s
Removing lib and re-running tests...

  1 passed


real	0m0.737s
user	0m0.589s
sys	0m0.100s
Resetting lib...

Environment

user@computer:~/src/ava-slow-test% node -e "var os=require('os');console.log('Node.js ' + process.version + '\n' + os.platform() + ' ' + os.release())"
Node.js v7.4.0
darwin 16.4.0
user@computer:~/src/ava-slow-test% ./node_modules/.bin/ava --version
0.18.2
user@computer:~/src/ava-slow-test% npm --version
4.0.5

Links

Test Repository

@taylor1791
Copy link
Author

I just noticed that this may be related to #1228.

@novemberborn
Copy link
Member

AVA needs to find your tests. It uses globbing patterns, which by default are quite wide (test.js test-*.js test/**/*.js **/__tests__/**/*.js **/*.test.js). If all your tests are in one particular folder you could specify the files option in the AVA config and point it at that directory.

@avajs/core I think we can consider this a common pitfall, so we should at least document it. Perhaps we should log a warning if globbing takes X amount of time?

@sotojuan
Copy link
Contributor

I definitely think it should at least be a common pitfall. Thanks @taylor1791 for the detailed issue!

@taylor1791
Copy link
Author

taylor1791 commented Feb 26, 2017

With that guidance, I put in some console.times around the globbing. time ./node_modules/.bin/ava src/add.spec.js produced 6.74s user 1.25s system 113% cpu 7.042 total. Globbing files reported 13.409ms and globbing helpers reported 6406.377ms. I looked around and I didn't see a way to change the helpers patterns. Assuming I didn't miss it, would this be a feature worth adding?

For what it is worth, if there were some debugs in the code with the timings for the globs and I could out find how to configure the default helper patterns, it may have prevented me from reporting this issue.

@novemberborn
Copy link
Member

Globbing files reported 13.409ms and globbing helpers reported 6406.377ms.

Wow, that's unexpectedly high.

I looked around and I didn't see a way to change the helpers patterns. Assuming I didn't miss it, would this be a feature worth adding?

Actually I think we need to optimize how we find helpers. The search starts here:

return handlePaths(defaultHelperPatterns(), ['!**/node_modules/**'], {
, which is called from

ava/api.js

Line 123 in bd5ed60

.findTestHelpers()
and in turn

ava/api.js

Line 149 in bd5ed60

return this._precompileHelpers()
.

It strikes me that we already know where the test files are when we start looking for helpers. We can find test and __test__ directories based on that list, and then search for helpers/**/*.js and **/_*.js files inside those directories. That should prevent us from searching your entire project tree.

I'm hesitant when it comes to making helper patterns configurable. I think we first need to get the default behavior right, lest the answer becomes "nah just change these patterns".

For what it is worth, if there were some debugs in the code with the timings for the globs and I could out find how to configure the default helper patterns, it may have prevented me from reporting this issue.

I think it's a bug actually, so I'm glad you reported it! 😄

@simonepri
Copy link

@novemberborn still there isn't a way to exclude those large non-tests folders?
In my case i have a large folder (4GB) and if the folder is present, ava never starts.
Also if I pass a single test file:
ava test/some-test.js
it hangs forever.

@novemberborn
Copy link
Member

novemberborn commented Jul 16, 2017

@simonepri you can negate test file patterns, e.g. ava "!lib/big-directory" "lib/**/*.test.js" should work.

(Assuming the underlying globbing implementation behaves efficiently enough.)

@simonepri
Copy link

simonepri commented Jul 16, 2017

@novemberborn
I've already tried this (Let's suppose that bin contains lots of subfolders and files that aren't code):

"ava": {
  "match": [
    "!bin/**",
    "test/**/*.js"
  ]
}

But nothing changes.

I'll try your suggestion 👍 but I think is pretty the same thing I've already done, am I wrong?

@novemberborn
Copy link
Member

match selectively runs tests whose title matches… You want files instead.

@simonepri
Copy link

simonepri commented Jul 16, 2017

Already tried also with files.

"ava": {
  "files": [
    "!bin/**",
    "test/**/*.js"
  ],
  "source": [
    "index.js"
  ]
}

I think I've tried any possible configuration 😆
It seems that the glob recursively visit the paths also if they are ignored (I think because it allows you to unignore a specific path inside an ignored one)

E.g:
"!bin/**" "bin/**/somepath/**"

I think we should add some options to filter the paths before passing it to the matcher.
Like --exclude -e

@novemberborn
Copy link
Member

I think we should add some options to filter the paths before passing it to the matcher.
Like --exclude -e

That's what ! is supposed to do, but it looks like globby is still spending too much time searching. Perhaps related to sindresorhus/globby#43.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug current functionality does not work as desired scope:globbing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants