Tests stop failing after including regular tests in typecheck

[murolem]

Describe the bug

After enabling typecheck and including regular test files (using include typecheck config option), when running the tests, only typecheck is performed, with the tests themselves passing regardless of any value assertions.

Setting only typecheck option to false doesn't seem to do anything.

Upd1: Current solution is to separate the regular tests and type tests to different files.

Upd2: Separating regular tests and type tests seems to result in an issue when saving the regular test file. Instead of rerunning the just-saved regular test file, instead the type test file is being rerun. Created an issue: #5020

Reproduction

Repo: https://github.com/murolem/vitest-issue-when-typecheck-is-used-tests-are-not-failing

Run npm run test.

Enable/disable typecheck in the config to see the issue.

System Info

System:
    OS: Windows 11 10.0.22621
    CPU: (16) x64 AMD Ryzen 7 5800X 8-Core Processor
    Memory: 8.01 GB / 31.93 GB
  Binaries:
    Node: 21.1.0 - C:\Program Files\nodejs\node.EXE
    npm: 10.2.2 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Chromium (120.0.2210.144)
  npmPackages:
    vite: ^5.0.11 => 5.0.11
    vitest: ^1.2.1 => 1.2.0

Used Package Manager

npm

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
• The provided reproduction is a minimal reproducible example of the bug.

[mayank99]

Running into the same issue. It's not clear from the docs whether this behavior is intentional or a bug. My impression was that the same .test.ts file could be used for writing regular runtime tests as well as type tests, and have the whole file be validated using tsc --noEmit when using the typecheck option.

If this is working as intended, then it means the regular .test.ts files need to be manually run through tsc to ensure there are no type errors. This is not necessarily a problem, but the docs could be made more clear and maybe the option should actually be called typetest.

[sheremet-va]

Upd1: Current solution is to separate the regular tests and type tests to different files.

This is the expected solution. Typechecking tests have a separate include property just for that. It takes precedence over test.include option. You can still run both runtime and typecheck by using poolMatchGlobs.

The problem is that it's not that easy to aggregate test results that were collected during running and type-checking for several reasons:

1. It's possible to have dynamic test names but they cannot be statically analyzed by TypeScript which makes it harder to know which test block has failed.
2. Type failures are considered test failures - when both runtime and typechecking fail it is impossible to merge them because there is no way to know what errors should stay - currently both runtime and typecheck just empty the errors property first - this is particularly hard to do when using watch mode (because runtime or type could've been fixed but we don't know if the previous error should be removed or not). This is also prone to race conditions.

If this is working as intended, then it means the regular .test.ts files need to be manually run through tsc to ensure there are no type errors.

You do not need to run tsc separately. The current API already runs it - it's just the errors are reported as source errors because they are outside of typecheck.include glob patterns. This is a deliberate decision (see point 1).

[mrmckeb]

We hit this issue today, as we didn't realise that we couldn't have both types of tests in the same test file.

We'd prefer to do this, as it saves us writing some tests twice.

Are you able to provide a small example of how poolMatchGlobs solves this @sheremet-va?

[vasiliy-pdk]

Same issue here. I got confused by the behaviour. I expect if there's a type error then the tests should fail. Otherwise, devs might be pushing misleading test code to the repository without a way to prevent it reliably ( CI wouldn't catch this because no error has risen ). What is more important is that enabling typechecks should not suppress existing assertions, otherwise, someone could ship bugs to production.

Here's how I encountered it ( fortunately it was local tiny pet project ).

I was working on a function whose first version accepted a single argument. The function code and corresponding test are written in Typescript.

I needed to introduce another argument for the function.
I changed the test code first without changing the implementation - I passed an extra argument to the function and was baffled that the test passed. There was an obvious type error which IDE highlighted. The tests stayed green.

I thought I missed some config for the vitest, so I enabled typecheck and added the include option. This started showing type errors. I proceeded to work on the implementation. After several minutes I realized that regular assertions like expect(1).toBe(2) stopped failing the tests - so my function wasn't working but the tests kept telling me that everything was fine :)

[akellbl4]

I've been trying poolMatchGlobs for running tsc over all code and tests at the same time and got no luck.
When glob overlapping with the previous one only first pool is gonna be used, which makes sense but still doesn't allow check if types are fine and tests are passing.

Here is an example:

export default defineConfig({
  test: {
    include: ['src/**/*.test.ts?(x)'],
    poolMatchGlobs: [
      ['src/**/*.test.ts?(x)', 'threads'],
      ['src/**/*.ts?(x)', 'typescript'],
    ],
  }
})

upd. the only way I found is to add vite-plugin-checker but it would not fail the tests

[illiaChaban]

same issue here. My expectation was also to be able to run type tests and functionality tests in the same file. For now just using tsc manually. ("test": "tsc --noEmit && vitest run" in package.json )