vitest-dev / vitest

Next generation testing framework powered by Vite.
https://vitest.dev
MIT License
13.13k stars 1.18k forks source link

Please Add The Ability To Define Custom Test Trigger Patterns #6457

Open klondikemarlen opened 2 months ago

klondikemarlen commented 2 months ago

Clear and concise description of the problem

As a developer using Vitest I want the ability to have an .html or .txt file trigger the re-run of a specific test file so that I can have an easier time testing mailers and email templates.

I'd like to see an option similar to what Ruby's Guard has. In Guard it would look something like this:

guard :minitest, cli: '--force' do
  # Watch for changes in .ts, .html, or .txt files in mailers or templates directories
  watch(%r{^src/(mailers|templates)/(.*)\.(ts|html|txt)$}) do |m|
    "api/tests/mailers/#{m[2]}.test.ts"
  end
end

This would cause changes to:

Suggested solution

Given that the JS equivalent might be something like

const chokidar = require('chokidar');
const { exec } = require('child_process');

// Watch for changes in .ts, .html, or .txt files in mailers or templates directories
chokidar.watch('src/{mailers,templates}/**/*.{ts,html,txt}')
  .on('change', (path) => {
    const match = path.match(/^src\/(mailers|templates)\/(.*)\.(ts|html|txt)$/);
    if (match) {
      const testFile = `api/tests/mailers/${match[2]}.test.ts`;
      console.log(`Detected change in: ${path}. Running ${testFile}`);
      exec(`npx vitest run ${testFile}`, (err, stdout, stderr) => {
        if (err) {
          console.error(`Error: ${stderr}`);
          return;
        }
        console.log(stdout);
      });
    }
  });

It would probably be possible to make a vitest config option test.watch.triggerPatterns with an interface like:

import { defineConfig } from "vitest/config";

export default defineConfig({
  test: {
    watch: {
      triggerPatterns: [{
        pattern: /^src\/(mailers|templates)\/(.*)\.(ts|html|txt)$/,
        testToRun: (match) => {
          return `api/tests/mailers/${match[2]}.test.ts`
        },
      }],
    },
  },
});

Alternative

Option 1

Use https://vitest.dev/config/#forcereruntriggers

import { defineConfig } from "vitest/config";

export default defineConfig({
  test: {
    forceRerunTriggers: [
      "**/*.(html|txt)", // Rerun tests when data files change
    ],
  },
});

While I'm happy that this option exists; I don't love it, because it forces all the tests to re-run, not just the relevant one.

Option 2

Use direct import at top of test file e.g.

// in api/tests/mailers/workflow-steps/next-step-mailer.test.ts
import "@/templates/workflow-steps/next-step-mailer.html?raw"
import "@/templates/workflow-steps/next-step-mailer.txt?raw"

I can't decide if I like this option more or less than the previous option. On one hand, its efficient, and pretty straight forward. On the other hand its not a global configuration, so I'm effectively leaking config into individual files. Also its likely that future developers will have no idea what this does without an explanatory comment.

Additional context

Guard docs at https://github.com/guard/guard/wiki/Custom-matchers-in-watches#using-custom-matchers-in-watches Guard DSL source https://github.com/guard/guard/blob/7ccce79c46a6e9d1acaca989bc1a2b72d3806221/lib/guard/dsl.rb#L209 Guard Watcher implementation source https://github.com/guard/guard/blob/7ccce79c46a6e9d1acaca989bc1a2b72d3806221/lib/guard/watcher.rb#L13

Validations

hi-ogawa commented 2 months ago

I think this is mostly possible with a vite plugin using addWatchFile (or also injecting ?raw import from transform would also work). I made a simple demo here:

https://stackblitz.com/edit/vitest-dev-vitest-qq31nt?file=vite.config.ts

function watchReRunDepPlugin(): Plugin {
  return {
    name: 'watch-rerun-dep-plugin',
    transform(code, id, options) {
      // matching a test file such as
      //   - test/xxx.test.ts
      // then call `this.addWatchFile` for
      //   - src/xxx.txt
      //   - src/xxx.html
      const match = id.match(/\/(\w*)\.test\.ts$/);
      if (match) {
        const name = match[1]!;
        for (const ext of ['.txt', '.html']) {
          const file = path.resolve(id, '../../src', name + ext);
          if (fs.existsSync(file)) {
            this.addWatchFile(file);
          }
        }
      }
    },
  };
}

Slight difference is that the pattern is reversed from what you suggested since it needs to list "dependencies" for a given "test file". If that part is abstracted in some ways, then the plugin itself is really a tiny wrapper of addWatchFile, for example:

function watchReRunDepPlugin({
  getDeps,
}: {
  getDeps: (file: string) => string[];
}): Plugin {
  return {
    name: "watch-rerun-dep-plugin",
    transform(code, id, options) {
      for (const dep of getDeps(id)) {
        this.addWatchFile(dep);
      }
    },
  };
}
klondikemarlen commented 2 months ago

That looks perfect! For other people, like me, who have never heard of addWatchFile, its documentation can be found at https://rollupjs.org/plugin-development/#this-addwatchfile. Vite uses Rollup, as interpreted from Vite docs comment "... which are based on Rollup's well-designed plugin interface .."

Maybe I'll investigate writing a plugin to clone the watch feature from Ruby Guard.

klondikemarlen commented 2 months ago

I've made it as far as creating a repository and npm package stub, and I'll try to pull in the code written by @hi-ogawa. I know almost nothing about publishing npm packages so apologies if it's a mess.

klondikemarlen commented 2 months ago

I wrote a basic plugin concept at https://github.com/klondikemarlen/vite-plugin-guard-like-watch and https://www.npmjs.com/package/vite-plugin-guard-like-watch.

I need to test out what happens when I import this package into another project; and probably set up a build system so that it only includes the complied version or something.

klondikemarlen commented 2 months ago

I have no idea how the build system works, but I tested the built version in a secont project and it worked. So I'm going 1.0 for https://www.npmjs.com/package/vite-plugin-guard-like-watch, and moving on.

Final usage instructions looks like

npm install --save-dev vite-plugin-guard-like-watch

Example vite.config.ts

/// <reference types="vitest/config" />

// Configure Vitest (https://vitest.dev/config/)
import { defineConfig } from "vite"
import guardLikeWatch from "vite-plugin-guard-like-watch"

export default defineConfig({
  test: {
    /* for example, use global to avoid globals imports (describe, test, expect): */
    // globals: true,
  },
  plugins: [
    // Note: debug is optional, but you'll probably need it to get set up.
    guardLikeWatch(/(.*\/example)\.ts/, (match) => [`${match[1]}.html`, `${match[1]}.txt`], true),
    guardLikeWatch({
      pattern: /(.*)\/example\/example\.ts/,
      action: (match) => [`${match[1]}/src/vite-plugin-guard-like-watch.ts`],
      debug: true,
    }),
    // Relative paths will also work, though I'm not entirely sure of the implications. e.g.
    guardLikeWatch(
      /tests\/mailers\/(.*)\.test\.ts/,
      (match) => [`src/templates/${match[1]}.html`, `src/templates/${match[1]}.txt`],
      true
    ),
    guardLikeWatch({
      pattern: /tests\/mailers\/(.*)\.test\.ts/,
      action: (match) => [`src/templates/${match[1]}.html`, `src/templates/${match[1]}.txt`],
      debug: true,
    }),
    // You can also use a string instead of a RegExp. e.g.
    guardLikeWatch(
      "tests/mailers/(.*).test.ts",
      (match) => [`src/templates/${match[1]}.html`, `src/templates/${match[1]}.txt`],
      true
    ),
    guardLikeWatch({
      pattern: "tests/mailers/(.*).test.ts",
      action: (match) => [`src/templates/${match[1]}.html`, `src/templates/${match[1]}.txt`],
      debug: true,
    }),
  ],
})
sheremet-va commented 1 month ago

We should implement it as a config option similar to your proposal here:

import { defineConfig } from "vitest/config";

export default defineConfig({
  test: {
    watchTriggerPatterns: [
      {
        pattern: /^src\/(mailers|templates)\/(.*)\.(ts|html|txt)$/,
        testToRun: (match) => {
          return `api/tests/mailers/${match[2]}.test.ts`
        },
      }
    ],
  },
});

We can just test the regexp when the watcher triggers it here:

https://github.com/vitest-dev/vitest/blob/445367bf5aa6a05c781641dc1f18967fef126c17/packages/vitest/src/node/core.ts#L844

aleclarson commented 1 month ago

I think it'd be better to have a test be able to specify its file dependencies.

test('foo', () => {
  const foo = fs.readFileSync('foo.html')
  expect.watch('foo.html')
})

I have no opinion on the exact naming or implementation.

edit: Oh I see this was proposed by #3378. What is the rationale for not allowing per-test file dependencies?

klondikemarlen commented 1 month ago

@aleclarson The feature you are talking about already exists: if you use lazy imports like const foo = () => import("some/dependency") (or something like that, I forget the syntax) at the top of a test file, editing the dependency will trigger a test re-run.

The dynamic imports on a per-file basis is great on a small scale, for one-off things, but on a larger scale, you end up polluting your tests with test configuration code. And the pattern listed here is a clean and centralized solution that this.

What is the rationale for not allowing per-test file dependencies?

Oops, I race your question; I'm not sure the answer to this, but I'm guessing it's an architecture conflict with the way Rollup generates dependencies?

hi-ogawa commented 1 month ago

I think it'd be better to have a test be able to specify its file dependencies.

@aleclarson Like @klondikemarlen mentioned in alternative 2, we recommend import "xxx?raw" for "per test file" re-run. If you are talking about "per test case" re-run, we don't have such mechanism and it's probably out of scope for now.

import "./foo.html?raw"; // this

test('foo', () => {
  await import("./foo.html?raw") // or this should also work, but still "per-test-file" re-run
  const foo = fs.readFileSync('foo.html')
})