vitest-dev / vitest

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

Feature request: Make "--changed" respect coverage or use static code analysis #6735

Open ffMathy opened 1 month ago

ffMathy commented 1 month ago

Clear and concise description of the problem

What if Vitest kept track of which lines each test runs through. That way, the "--changed" flag would be truly only "changed", and could save a huge amount of test execution time.

In our test suite, we have a lot of end-to-end tests, as well as a lot of integration tests. So here, we don't care about Vitest's performance, because that's typically not the limiting factor. IO such as database operations and API calls is limiting instead.

So for that, to really cut off some test execution time, we need to be able to better restrict what tests are run.

--affected does a great job already by looking at the import tree, but it could become even better. If this was implemented, it would be equal to how it works in IntelliJ based IDEs like WebStorm, IntelliJ and Rider, and it could be one of the only test libraries for node that would support this. So there's a lot of opportunity here.

Suggested solution

The way I see it, it could be achieved in three ways.

  1. Using code coverage: Some tools already do this today. However, it may require generating coverage in a specific format which includes this information. Perhaps there's also some performance considerations in basing it on files (so maybe suggestion 2 is better).
  2. Using in-memory code coverage: Same as above, but maybe based on some kind of in-memory data. Vitest must generate existing coverage files today based on some data in memory (that's an assumption), so this could maybe be "easy" to implement directly into the "watch" mechanism of Vite.
  3. Analyzing the AST: Similarly to how imports are analyzed today, we could use the Visitor design pattern or something similar to be able to recursively traverse function calls etc. But that might be more complex, as Node projects can be configured in a variety of ways.

Alternative

No response

Additional context

This was sparked here: https://github.com/vitest-dev/vitest/discussions/6734

Validations

sheremet-va commented 1 month ago

Sorry, but there is no --affected flag. Do you mean --changed? Or do you want a separate flag?

ffMathy commented 1 month ago

Ah yes, brain fart from my side. I did mean changing the existing flag's behavior.

ffMathy commented 1 month ago

Will you accept PRs for this?

sheremet-va commented 1 month ago

All suggested solutions seem like overkill to me. The second one seems impossible because Vitest doesn't have an in-memory coverage as far as I know (I might be wrong). To analyse AST, we need to process files with Vite into JavaScript first, which is not a problem and might be the easiest solution out of all of them, to be honest. (Btw, how do you parse dynamic test names?) To even work with coverage, you need to generate it, which means you need to store it just for --changed, which seems very inefficient to me and will just pollute the git repo (assuming you are pushing it to use in CI).

This seems like a huge project, and I don't think anyone on the team wants to maintain it (We usually say no to features we can't maintain because Vitest is already pretty big). This will also require a big refactoring of the internals to have a map of tests to files ahead of time.

Maybe others on the team have a different opinion? @hi-ogawa @AriPerkkio @antfu

AriPerkkio commented 1 month ago

As said before, in order to use coverage for this we would first need to run all tests so that we have coverage results. That kind of breaks the intention of --change completely. And coverage is collected after a single test file has run. If we wanted to know what files/lines a single test executed, we would need to change that logic too. I'm not sure what test.concurrent would look like.

Are there any existing Javascript tools that have implemented this? What kind of logic would the static analysis part have? Implementing static analyses for dynamic cases is simply not possible. We need to evaluate the code for many cases. Tests are often very dynamic.

Will you accept PRs for this?

I'm quite curious how you would even start implementing this. 😄 Feel free to build some proof-of-concept but it's likely that this won't be accepted.