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

Watcher should only recompile changed files, and its dependents. #28

Closed
Paulskit opened this issue Aug 30, 2017 · 5 comments
Closed

Watcher should only recompile changed files, and its dependents. #28

Paulskit opened this issue Aug 30, 2017 · 5 comments

Comments

@Paulskit
Copy link

Paulskit commented Aug 30, 2017

Default behavior for karma is to trigger processing only on changed file. This is very fast and convenient if you debug tests in watch mode.
Current implementation forces all files to be processed on any file change. Consequently, it takes a lot of time on large tests amount just to check 1 file change.

Just an example:

  1. Default karma file watch + rollup
    30 08 2017 15:18:36.617:INFO [watcher]: Changed file "./test/unit/behavior/noResultsBehavior.spec.js".
    30 08 2017 15:18:36.791:INFO [watcher]: Changed file "./test/unit/behavior/noResultsBehavior.spec.js".
    Less than 200ms to recompile and trigger test run.

  2. Current implementation with custom watcher:
    75 seconds to respond to single file change. (66 files and 564 tests recompiled in total)

@jlmakes
Copy link
Owner

jlmakes commented Aug 30, 2017

Figure 1:

Submodules: A, B
Modules: 1, 2, 3, 4
Tests: T1, T2
Example Dependency Tree
Karma’s default behavior ignores dependent modules… so if B changes, only B will be reprocessed… and everything downstream that depends on B will now be stale. This is what #3 is about, and what #11 improved in version 3.

So, the custom watcher should take more time, because it’s processing more files. Our goal is to ensure the updated file—and its dependents downstream—are processed again. Ideally, we'd recompile only the changed file, and its descendants… so if B changes, then 1, 3, T1, and T2 are recompiled.

Figure 2:
Example Dependency Tree

This is exactly what version 3 does, except that it unintentionally re-ran tests for each processed file (#17). That means in the example above, the tests would have ran 5 times after changing B. Aside from spamming your terminal, Karma would encounter errors and false negatives.

So v5.0 addresses #3 and #17 with a new watcher that simply reprocesses all files; there’s definitely room for improvement (potentially using a strategy similar to #11). I’ll take a look at making this improvement in future releases.

@jlmakes jlmakes changed the title All files are recompiled on any file change in watch mode Watcher should only recompile changed files, and its dependents. Aug 30, 2017
@Paulskit
Copy link
Author

Paulskit commented Aug 30, 2017

While I completely agree with your info graphics, I'm not sure what I see locally corresponds to what you're saying. Saving a single .spec.js file with no external dependencies (no imports, requires, etc) now triggers full rebuild of all files with rollup.

Running all tests is not an issue, you can always limit only desired set of tests using something like 'fdescribe'.

Edit: I have created a demo project for this:

https://github.com/Paulskit/rollup-karma-performance-bug

Steps to reproduce:

  1. npm i
  2. npm test
  3. Save translate.js or translate.spec.js

As you can see, on every translate file change (which has no external dependencies) you see warnings in console about not used dependencies. Those are coming from the mock1 file specifically for this reason. It proves that despite the file doesn't have any external dependencies, the entire bundle’s recompilation is triggered.

Repository owner deleted a comment from Paulskit Aug 30, 2017
@jlmakes
Copy link
Owner

jlmakes commented Aug 30, 2017

I appreciate the reproduction, but the current behavior is by design.

I agree it has room for improvement—but there are constraints when integrating with Karma, and the current file watcher is the first one that:

  • Correctly recompiles dependencies
  • Does not excessively re-run tests
  • Does not throw errors and false negatives

The cost is performance. Pull requests are welcome, but I will do my best to improve it.

@Paulskit
Copy link
Author

Clear enough. Thank you.

@danielnaab
Copy link

For anyone encountering this, here's my solution: create a test manifest that explicitly imports every spec in your test suite, and put only that single entry-point in the Karma config.

Watch tasks are much faster, due to the single compilation.

Presumably this is something that could be automated as part of the preprocessor, but is easy enough to maintain as an additional module in the test suite itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants