vitest-dev / vitest

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

Following documentation, regarding a tip about `@ts-expect-error`, I cannot make tests behave as I would expect from the documentation #5604

Open danwithabox opened 6 months ago

danwithabox commented 6 months ago

Describe the bug

There's a "tip" over at https://vitest.dev/guide/testing-types.html#run-typechecking, saying:

When using @ts-expect-error syntax, you might want to make sure that you didn't make a typo. You can do that by including your type files in test.include config option, so Vitest will also actually run these tests and fail with ReferenceError.

After adjusting the test.include array in vitest.config.ts, I cannot make all my tests work.

There are 3 test files to present the issue:

There are 2 npm scripts

Upon running npm run test, the results are:

Upon running npm run test-no-typecheck, the results are:

What I would expect, is for at least one of the npm scripts to result in every test failing.

Reproduction

https://github.com/danwithabox/issue_vitest_vitest-dev_5604

System Info

System:
  OS: Windows 11 10.0.22631
  CPU: (16) x64 AMD Ryzen 7 3700X 8-Core Processor
  Memory: 37.14 GB / 63.93 GB
Binaries:
  Node: 20.10.0 - C:\Program Files\nodejs\node.EXE
  npm: 10.3.0 - C:\Program Files\nodejs\npm.CMD
Browsers:
  Chrome: 124.0.6367.60
  Edge: Chromium (123.0.2420.97)
  Internet Explorer: 11.0.22621.1
npmPackages:
  vitest: ^1.5.0 => 1.5.0

Used Package Manager

npm

Validations

hi-ogawa commented 6 months ago

It might be that "tip" is outdated or simply wrong. Currently, vitest --typecheck cannot test single test file in both mode and prioritized typecheck mode. See also:

I'm not sure what's the best way to do this currently other than running vitest twice.

mrazauskas commented 6 months ago

I'm not sure what's the best way to do this currently other than running vitest twice.

Note that // @ts-expect-error can hide typos in type names as well. Those are stripped entirely. Nor executing the test files, neither running vitest twelve times can help.

Solution: the old good Separation of Concerns principle. Use JavaScript testing tool to test runtime behaviour. Use specialised type testing tool to test types. There are at least two type testing tools which offer a type error assertion instead of // @ts-expect-error. The comment belongs to compiler, it is not meant for testing types.

danwithabox commented 6 months ago

@mrazauskas

There are at least two type testing tools which offer a type error assertion instead of // @ts-expect-error.

Could you please refer to these tools for clarity?

With Vitest, I gravitated towards @ts-expect-error as the most readable solution, and I will probably keep using it despite the possible issues highlighted, unless said tools are better, because the alternative was rather messy.

A minimal example of my use case, for context:

// to test against
function argFail<const T extends [1, never, 3]>(...arg: T) {}

//#region readable
argFail(
    1,
    // @ts-expect-error invalid
    2,
    3,
);
//#endregion

//#region messy, but intended way of testing, with expect-type?
type argFail_params = Parameters<typeof argFail>;
const arg = [1, 2, 3] as const;
expectTypeOf(arg[1]).not.toMatchTypeOf<argFail_params[1]>();
//#endregion

In a realistic scenario, argFail's arg: T parameter might also become too complex to properly extract it and make it testable without going somewhat mad.

Much more practical to just test a natural invocation of argFail with a not-quite-intended @ts-expect-error, that is resilient to refactors, than to bring in what I would consider a brittle and indirect way of expressing what I want to test.

mrazauskas commented 6 months ago

Could you please refer to these tools for clarity?

Sure. I had in mind:

To be honest, a type error assertion is not an ideal solution too. It would be the best to have .not.toBeCallableWith() matcher. As you know, expect-type has .toBeCallableWith(), but it does not work with function overloads and generic functions. They do not implement .not.toBeCallableWith() at all, but suggest to use // @ts-expect-error.

This matcher is complex. The most interesting part is to resolve argument types of generic functions. Programatically this is possible and I have a working prototype. That means that eventually tstyche will have .not.toBeCallableWith() with support for overloaded and generic functions.

danwithabox commented 6 months ago

Cheers, as I suspected there's not really a canonical way to go about this, and so I will stick to @ts-expect-error.

I will leave it to the maintainers to decide about the tip in the docs, but if I may provide input, it did waste me some time to realize that it's not working as intended, and at least some warning around it would be a welcome addition while the upstream issues are resolved.