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

Watch mode can get slow with heavier test setup scripts #20

Open
marcelbeumer opened this issue Feb 29, 2020 · 7 comments
Open

Watch mode can get slow with heavier test setup scripts #20

marcelbeumer opened this issue Feb 29, 2020 · 7 comments

Comments

@marcelbeumer
Copy link
Collaborator

Example: marcelbeumer/react-shared-state-hook#1

Because of the isolated test execution, jsdom and babel will be initialized each time the watcher re-runs a test file, which takes a second or two on my machine.

@ddneat
Copy link
Owner

ddneat commented Feb 29, 2020

@marcel Thanks for reporting this issue. Sounds like there should be an argument like --require-once or --disable-isolation.

I think --require-once might be the more flexible solution. Whats your thought?

Cheers

@ddneat
Copy link
Owner

ddneat commented Feb 29, 2020

In general I think watchmode might need a review and maybe a refactoring. It feels not yet as fast as it could be, since there is a hardcoded timespan of 20ms where filechanges are collected to avoid to early triggering/cancel on multiple filechanges. I guess there might be an improvement in this matters.

@marcelbeumer
Copy link
Collaborator Author

marcelbeumer commented Mar 1, 2020

@ddneat yes, afaik you can't do it with fork/spawn, so I was also thinking of disable-isolation and use require cache clearing.

watcher: I think most people use chokidar (https://github.com/paulmillr/chokidar) nowadays, have good experience with it too

saw that mocha is basically clearing all paths their watcher watches, seems simple/smart on first thought:

@ddneat
Copy link
Owner

ddneat commented Mar 1, 2020

@marcelbeumer yes, looks great.

Chokidar would for sure contribute greatly in terms of making tropic much more stable and performant! I would be happy to receive a pullrequest.

I'm currently gathering a list of features/refactorings necessary to release tropic v1.0.0 within the next weeks and latest by early April. I feel highly motivated to push the state of tropic! Thanks for contributing!

Cheers

@ddneat
Copy link
Owner

ddneat commented Mar 7, 2020

@marcelbeumer I just reviewed the jest docs and learned about --env which might offer an alternative solution to the discussed problem. https://jestjs.io/docs/en/cli#--envenvironment

Rather than --disable-isolation the --env might be a more reasonable naming and even offer the oppertunity to use both --require as well as using a globally shared module like jsdom.

What's your opinion?

@marcelbeumer
Copy link
Collaborator Author

what kind of env values were you thinking of?
"isolation" seems to be very accurate, I was even thinking shouldn't non-isolated not be the default, and do --isolated?

@ddneat
Copy link
Owner

ddneat commented Mar 12, 2020

Well env might be actually behave the same as --require-once. Just with a more reasonable naming.

Yes, going for non-isolated would be the more mocha compatible way and additionally increase performance as well. Having a performance increase, might be a great marketing argument. Therefore I would also see the benefit of having the non-isolated as the default and --isolated as an option to choose.

To summarise current insights:

  • move to non-isolated as default option, while introducing --isolated as an option
  • replace currently used fs.watch with chokidar

Further point for performance increase:
Since non-isolated will be the default, it might be much more performant to write the terminal output directly from test execution. Currently the communication between the the spawned testing processing uses the IPC channel, which involves serialisation/deserialisation json data. This refactoring highly depends on the implementation of the non-isolated mode and on therefore on the fact if the tests are executed by using only require or a forked/spawned process.

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

No branches or pull requests

2 participants