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 + --changed is unreliable with unexpected DX #3666

Closed
6 tasks done
o-alexandrov opened this issue Jun 26, 2023 · 4 comments · Fixed by #3844
Closed
6 tasks done

watch + --changed is unreliable with unexpected DX #3666

o-alexandrov opened this issue Jun 26, 2023 · 4 comments · Fixed by #3844
Labels
enhancement New feature or request

Comments

@o-alexandrov
Copy link

o-alexandrov commented Jun 26, 2023

Describe the bug

Running vitest with --changed is unreliable and produces unexpected developer experience.

Reproduction

When running the command below, there are two different bugs:

vitest watch --changed

Case 1 (no changes initially)

You'll see No test files found, exiting with code 0

  • however, since the cmd is watch, shouldn't the script continue to watch all files that it compared against that were initially checked if there are any changes?
    • in other words, imho, watch shouldn't exit if there are no changes, it should keep running so when a change happens, it reacts to the file

Case 2 (file X was changed)

The file X would be tested.
Then, if you change file Y, vitest wouldn't run tests in file Y.

System Info

System:
    OS: macOS 13.4
    CPU: (10) arm64 Apple M1 Pro
    Memory: 56.77 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.16.0 - ~/.nvm/versions/node/v18.16.0/bin/node
    npm: 9.5.1 - ~/.nvm/versions/node/v18.16.0/bin/npm
  Browsers:
    Chrome: 110.0.5481.100
    Edge: 112.0.1722.39
    Firefox: 114.0.2
    Safari: 16.5
  npmPackages:
    vite: ^4.3.9 => 4.3.9
    vitest: 0.32.2

Used Package Manager

npm

Validations

@HendrikPetertje
Copy link

HendrikPetertje commented Jul 20, 2023

dealing with this as well, we migrated an existing project from jest to vitest. our project has several thousand tests, waiting for the initial test run takes around 3 minutes. I'd like Vitest to only test what was changed when i boot it up and then keep watching for further changes that i make as I make them.

I'm currently working around this with a little testWatch.mjs, feel free to use it if you are in the same situation as me.

import { watch } from 'fs';
import { spawn } from 'child_process';
import readline from 'readline';

let timeout = null;
let isBusy = false;
let ls;

function watchHandler () {
  if (timeout) clearTimeout(timeout);
  // Debounce the event, since multiple events can be emitted on save
  timeout = setTimeout(async () => {
    isBusy = true;
    await new Promise((resolve) => {
      console.clear();
      let ls = spawn('yarn', ['test', 'run', '--changed'], { stdio: [0, 1, 2]});

      ls.on('exit', resolve);
    });

    isBusy = false;
    ls = null;
    console.log('watching for new changes')
  }, 200)
}

// Recursive doesn't always work on all OS-es!
const options = { persistent: true, recursive: true }

console.log('watching for changes')
const watcher = watch('src/', options, watchHandler)

// windows
if (process.platform === "win32") {
  var rl = readline.createInterface({
    input: process.stdin,
    output: process.stdout
  });

  rl.on("SIGINT", function () {
    watcher.close();
    if (ls) ls.kill();
    process.emit("SIGINT");
  });
}

// Unix
process.on("SIGINT", function () {
  //graceful shutdown
  watcher.close();
  if (ls) ls.kill();
  process.exit();
});

@sheremet-va
Copy link
Member

--changed is meant for CI or a quick run before pushing. You don't need this flag to rerun tests that you changed while vitest watch was running.

@o-alexandrov
Copy link
Author

o-alexandrov commented Jul 25, 2023

@sheremet-va thank you, please kindly clarify your reply:

--changed is meant for CI or a quick run before pushing.

There are 2 whys about this statement. Why is --changed allowed for vitest watch then? And if it's allowed, why should vitest have a different behavior to jest's --onlyChanged? (imho, there's no benefit in having a different behavior)

You don't need this flag to rerun tests that you changed while vitest watch was running.

Is it irrational for the user to want to run tests for the changed tests only at first for vitest watch?
In other words, by you don't need would you like to state: the tests should always run for all existing tests in the first watch run

  • don't you think it's the opposite? You need to run tests only for the changed files at first when you run vitest watch; otherwise, in large codebases, as @HendrikPetertje commented above, you'd be forced after vitest watch to always cancel, so the watch then waits for the changes

@sheremet-va
Copy link
Member

@o-alexandrov I can see your point. The behavior will change in the next version:

  • If no affected test files are found and watch mode is enabled, it will wait for file changes
  • If no affected test files are found and watch mode is disabled, it will exit with code 0
  • If another file that didn't have any effect on tests previously is changed, Vitest will rerun all affected tests

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
3 participants