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.

Sign up for GitHub

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

Jump to bottom

Cancel previous running compile process in watch mode #194

Closed
andys8 opened this issue Aug 2, 2017 · 5 comments · Fixed by #197
Closed

Cancel previous running compile process in watch mode #194

andys8 opened this issue Aug 2, 2017 · 5 comments · Fixed by #197

Comments

@andys8
Copy link
Contributor

andys8 commented Aug 2, 2017

I've noticed node-test-runner started in watch mode is looking for file changes and triggers build and tests. This is not aware of currently running compile or test processes.

If you hit save several times, this is what will happen:
screenshot from 2017-08-02 18-52-24
screenshot from 2017-08-02 18-53-18

This is for demonstration but it happens for me while developing: Elm-format will run on save and triggers elm-test twice. If it's possible, I think it could be a solution to cancel currently executed tasks before starting a new compile process. Or is this a known issue or not possible to recognize?

Issue could be related to #110
@zwilias
Copy link
Collaborator

zwilias commented Aug 2, 2017

I think this should (at least) debounce 🤔

@andys8
Copy link
Contributor Author

andys8 commented Aug 2, 2017

@zwilias It would help. If the test execution duration isn't short it will still be painful to watch outdated code being tested ;)

I started digging into the implementation. A node module called "Chokidar" is used to wrap file system events. I'm curious about the provided options. There is awaitWriteFinish which is not activated by default. And there is awaitWriteFinish.stabilityThreshold described as follows:

Amount of time in milliseconds for a file size to remain constant before emitting its event.

Sounds interesting, could be a potential option.

@rtfeldman
Copy link
Owner

@andys8 I would love to accept a PR for this! ❤️

@andys8
Copy link
Contributor Author

andys8 commented Aug 3, 2017

Another interesting thing is that node-test-runner is using node-elm-compiler which is using node-cross-spawn to spawn children processes on different platforms, but hasn't the functionality to stop them.

There is this discussion:
moxystudio/node-cross-spawn#40

The result was @kentcdodds creating cross-spawn-with-kill.

@andys8 andys8 mentioned this issue Aug 4, 2017
Merged
@andys8
Copy link
Contributor Author

andys8 commented Aug 4, 2017

@rtfeldman I changed the configuration of chokidar and it works for me to solve the problem with elm-format triggering elm-test in watch mode twice in practice. It will not help if somebody keeps hitting the save button. I think it could also make sense to stop the previous running process with the suggested changes in node-elm-compiler and using the cross-span fork. It's hard to estimate possible problems due to killing running processes (will they leave files behind?). That's why I added the simple PR which could work. It's your decision :) I'm happy if you try the PR and compare the changes.

@andys8 andys8 mentioned this issue Nov 19, 2017
Closed
@lydell lydell mentioned this issue Dec 12, 2020
Merged
harrysarson pushed a commit that referenced this issue Dec 17, 2020
@lydell
Improve watcher (#475)
45c5245 
This makes the watcher faster and more stable! As a bonus, this made the code much more organized.

- Concentrate all `process.exit` in elm-test.js (except one in Install.js where I found no clean solution). Besides making the code easier to follow, it allows the watcher to continue running (and recover) even if there are errors.

- Don’t crash/stop running tests if files are removed.

- Pick up new Elm files added.

- Fixes #225

  We already had code for waiting for the previous run to finish before starting the next run, but it queued up every single file change/add/remove event and ran that many runs one after the other. Now, for all watch events during a run we run _once_ after the current run for all of them. We also sleep 100 ms at the start of each run to catch more events happening close by (such as when adding/removing a whole folder of tests). (Note: We used to sleep at least 500 ms via the [awaitWriteFinish](https://github.com/paulmillr/chokidar#performance) chokidar option.)

  I’ve also tested manually that running elm-format on save doesn’t cause double test runs: #194

- Fixes #355

  We now find the closest elm.json upwards in the folder hierarchy.

- Resolves #453 (comment)

  The watcher now listens for changes to elm.json and properly recalculates things. I thought that I would have to recalculate some things only when elm.json changes for performance, but after making the globbing faster there was no need so we simply rebuild everything on each run.

- Closes #463 and closes #464

  I can’t find any more places to use async/await.

- elm.json is now decoded properly in one place rather than here and there and just being `any`. We now have 0 explicit `any` type annotations!

- elm-test.js now only contains CLI logic and subcommand dispatching. The code for running tests have been moved to RunTests.js and to FindTests.js (which used to be called Runner.js).

- Improved .flowconfig that is stricter. Closes #478

- And lots of little code improvements along the way! I only touched things that I needed to change anyway in order to not make the diff balloon too much.

I’ve done some manual testing on Windows, Mac and Linux to make sure things work like they should.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Chokidar file watcher: Await write finish andys8/node-test-runner
3 participants
@zwilias @rtfeldman @andys8