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

[MVP] Watch mode #46

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

[MVP] Watch mode #46

wants to merge 6 commits into from

Conversation

DmitrySharabin
Copy link
Collaborator

First, let's see this in action.

Manually re-run the tests with the (new) R key

Re-run.tests.mp4

Automatically re-run the tests on file change

Watch.changes.mp4

Notes

To add support for the R key, there were not so many changes (even though the diff shows there are many):

  • The main change is that we need to track whether the tests were run for the first time or not. So, I moved most of the existing code inside the else block, which is why it looks like I've changed a lot.
  • We don't want to repeatedly show the hint about keyboard shortcuts, and more importantly, we don't want to add more listeners for key presses. Otherwise, we should press any key to quit the interactive mode more than once. So, we continue listening for key presses between the runs of the tests.
  • Since we run and re-run tests in the same process, when we re-import files with tests, Node returns them from the cache, so no changes are picked up. To ensure we work with fresh copies, I borrowed a snippet from a Node module, which solves exactly this issue. Unfortunately, this snippet causes a memory leak, so it is recommended for short-lived tests only.

To add support for the watch mode, I used the chokidar Node module. As the module README.md explains, it normalizes watch events on different platforms and fixes some other issues with the built-in fs.watch and fs.watchFile.

Even though we ensure that we work with fresh copies of tests, our workaround does not cover another issue. If tests have one entry point (e.g., index.js), and this file imports all other files with tests, we get files from the cache again on every re-run of the tests.

So, this MVP works in simple cases (and might be used as a starting point), and I would love for us to discuss how to make it work in general.

I looked at how this functionality is implemented in Rollup. It looks like they have implemented their own cache system.

Copy link

netlify bot commented Jun 24, 2024

Deploy Preview for h-test ready!

Name Link
🔨 Latest commit 7531d6a
🔍 Latest deploy log https://app.netlify.com/sites/h-test/deploys/6679771d685e6a00089ecced
😎 Deploy Preview https://deploy-preview-46--h-test.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@DmitrySharabin DmitrySharabin requested a review from LeaVerou June 24, 2024 13:40
@LeaVerou
Copy link
Collaborator

I think we need to split this into a separate PR for the r key, which is already a huge ergonomics improvement and much easier to implement than a watch mode, and a separate PR for the watch mode.

We should probably keep track of how many times the tests have been rerun, that's useful data, and could also simplify the code.
WRT the memory leak, is there any way to free up this memory?
What if we spawn a process to run the tests and kill it, then spawn a new one?

@DmitrySharabin DmitrySharabin marked this pull request as draft January 7, 2025 16:52
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

Successfully merging this pull request may close these issues.

2 participants