vitest-dev / vscode

VS Code extension for Vitest
https://vitest.dev/vscode
MIT License
740 stars 83 forks source link

`Turn on Continuous Run` not working for single test #287

Closed MangelMaxime closed 5 months ago

MangelMaxime commented 6 months ago

Describe the bug

I am trying to activate Continuous Run for a single test, so I can focus on this one and have a faster feedback loop.

However, click on the Turn on Continuous Run icon for a single test does nothing:

CleanShot 2024-03-14 at 22 35 02@2x

Reproduction

  1. Create a test suite

    import { expect, test } from 'vitest'
    
    test("run me please", () => {
    
        expect(1).toBe(2)
    })
    
    test("run me please2", () => {
    
        expect(1).toBe(2)
    })
  2. Click on the icon mentioned above and see that nothing happens.

  3. If you click on the top level icon, then all the tests are running in Continous Run

https://github.com/vitest-dev/vscode/assets/4760796/422d6ed0-0813-48db-a53f-4d312b8d3490

System Info

System:
    OS: macOS 14.1.1
    CPU: (10) arm64 Apple M1 Max
    Memory: 2.52 GB / 64.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.9.0 - ~/Library/Caches/fnm_multishells/8162_1710196074541/bin/node
    npm: 10.1.0 - ~/Library/Caches/fnm_multishells/8162_1710196074541/bin/npm
    pnpm: 8.13.1 - ~/Library/Caches/fnm_multishells/8162_1710196074541/bin/pnpm
    bun: 1.0.18 - ~/.bun/bin/bun
  IDEs:
    VSCode: 1.87.1 - /usr/local/bin/code
    Vim: 9.0 - /usr/bin/vim
    Xcode: /undefined - /usr/bin/xcodebuild
  Browsers:
    Chrome: 122.0.6261.129
    Safari: 17.1
  npmPackages:
    @vitejs/plugin-react: ^4.2.1 => 4.2.1 
    @vitest/ui: ^1.3.1 => 1.3.1 
    vite: ^5.1.5 => 5.1.5 
    vitest: ^1.3.1 => 1.3.1

Used Package Manager

pnpm

Validations

ffMathy commented 6 months ago

Might be fixed by #253.

sheremet-va commented 6 months ago

This should be fixed in pre-release 0.5.0 and higher. Note that the extension now requires Vitest 1.4.0 or higher.

MangelMaxime commented 6 months ago

I don't think this work.

I still need to use the top level button for it to trigger, and it will still run all the tests depends on the file that changed.

However, the startup time improvement is game changer thanks to the contributor for that 🎉

sheremet-va commented 6 months ago

Cannot confirm. Works correctly in the basic example apart from the known issues: https://github.com/vitest-dev/vscode/tree/main/samples/basic

MangelMaxime commented 6 months ago

Indeed, using the example your provided it seems to work.

I trimmed down my project, to make a reproduction. https://github.com/glutinum-org/cli/tree/vitest_continous_mode_bug

Make sure to use the vitest_continuous_mode_bug branch otherwise, you will have much more files to deal with and also generates the tests yourself.

Steps to reproduce should be:

  1. Clone https://github.com/glutinum-org/cli/tree/vitest_continous_mode_bug
  2. pnpm install
  3. Open VSCode
  4. Try to run the tests using Vitest UI + Continuous mode

[!NOTE] I tried to not change my tests too much in case, it was due to how they are setup/what they execute.

https://github.com/vitest-dev/vscode/assets/4760796/df1058c3-bef0-4fc1-bc5d-4f6f6ee104ac

sheremet-va commented 6 months ago

Still cannot reproduce it, - do you use pre-release 0.5.0+ version?

Screenshot 2024-03-16 at 09 30 13

(The error is shown on the wrong line because of a bug in Vitest, but it is shown)

sheremet-va commented 6 months ago

The error is shown on the wrong line because of a bug in Vitest

Well, actually, it's not a bug. You need to wait for toMatchFileSnapshot assertion:

await expect(result).toMatchFileSnapshot(expectedFile) // the error is shown correctly here
MangelMaxime commented 6 months ago

Thank you for pointing out that toMatchFileSnapshot is an async operation.

I think I found out what the problem is.

It looks like the problem comes from using both Vitest and Ionide (F# support) at the same time.

Ionide set Run F# Tests profile as the default test profile, which I suspect is picked up when run single test on continuous mode. Not sure why it is not picked up when clicking the global Start continuous run.

If I change the default profile to Vitest, then the Continuous run for a single test works.

CleanShot 2024-03-16 at 16 05 35@2x

I am not familiar with the VSCode test API so I am not sure if the problem needs to be solved here or inside of Ionide. I opened an issue on Ionide repo too. https://github.com/ionide/ionide-vscode-fsharp/issues/1995

sheremet-va commented 6 months ago

Interesting. I would expect the button to use our profile when clicked 🤔

sheremet-va commented 6 months ago

Does it work if you pick all of them as the default?

MangelMaxime commented 6 months ago

Interesting. I would expect the button to use our profile when clicked 🤔

It does use the selected profile, if I select Glutinum.Converter/vitest.workspace.ts as the default then Continuous Run works when I click on the eye icon.

https://github.com/vitest-dev/vscode/assets/4760796/cae9fc99-bd25-4098-9fda-99670b712d6c

It does not work if Run F# Tests is the default.

CleanShot 2024-03-16 at 16 45 46@2x

Does it work if you pick all of them as the default?

If I select all of them as Default then it works indeed.

CleanShot 2024-03-16 at 16 45 24@2x


I think I discovered something else which does not work. If I activate Continus Run for a single test and edit that tests, then it is re-run but I believe it is skipped as it does not report success or error. It is reported using a bent arrow:

https://github.com/vitest-dev/vscode/assets/4760796/f220ba8a-fe24-4ee1-bf1b-c623219029f9

If you prefer, I can open a new issue to report this problem but as it seems on the same subject I thought it was fine to mention it here too.

MangelMaxime commented 6 months ago

Quoting one of Ionide maintainer, it seems like VSCode allows to attach a tag to test runners.

It looks like we can use 'tags' to target test run profiles - perhaps vite should consider tagging their discovered tests, and we should consider tagging our discovered tests as well. A brief description of how this works is here: code.visualstudio.com/api/extension-guides/testing#test-tags

https://github.com/ionide/ionide-vscode-fsharp/issues/1995#issuecomment-2002029247


But what I don't understand is why if I have only Run F# test selected as the default, then running a single test in a one shot works but it doesn't not for Continuous Run. What is the difference between these invocations ?

https://github.com/vitest-dev/vscode/assets/4760796/51f850b7-edd7-4772-861e-1ad92e60a873

sheremet-va commented 6 months ago

I implemented tag support in 0.5.6. Please have a look when it hits the marketplace (should be around 5 minutes).

MangelMaxime commented 6 months ago

The tag addition didn't improve the situation, I still need to have Vitest selected as one of the default profile.

But what I don't understand is why if I have only Run F# test selected as the default, then running a single test in a one shot works but it doesn't not for Continuous Run. What is the difference between these invocations ?

Looking into how VSCode API works and trying to debug a local instance of Ionide, it seems like Ionide is never notified when a test request comes from Vitest.

Would it be possible to be a bug in VSCode itself which doesn't dispatch the test request correctly when clicking the eye icon on the test line?

sheremet-va commented 6 months ago

I can reproduce the bug. This is an issue with the Vitest extension.

sheremet-va commented 6 months ago

So, this happens because we set testNamePattern to invalid regexp so tests won't actually run unless we need them to:

https://github.com/vitest-dev/vscode/blob/dad58842672344c686a63eedd80b48aa662dc1ec/src/worker/rpc.ts#L36

This breaks Vitest watch mode - tests are only collected, they are not running. This is good when continuous mode is not enabled because we need to still collects tests on file change (in case the test is removed/renamed/moved), but when continues mode is enabled, we need to actually run tests in selected files, but still only collect tests in other files.

Ideally, we need a way to manipulate Vitest watch mode to not run certain tests - we can provide this API on Vitest side in the next version. (There is already watchTests method, but it will run test files even when its dependency changes, which is bad for CPU usage especially on large monorepos)

Another issue is here:

https://github.com/vitest-dev/vscode/blob/dad58842672344c686a63eedd80b48aa662dc1ec/src/runner/runner.ts#L136

Even if we run tests, they are not reported in the UI because we use incorrect testRequest when starting a new testRun. I don't know the good solution for this problem yet.

sheremet-va commented 5 months ago

I updated the implementation in 0.5.8. The "Turn on Continues Mode" doesn't run tests anymore, it just marks it to be reported. Any subsequent change will trigger a rerun.

Please try it out.

MangelMaxime commented 5 months ago

Thanks a lot for all the work you put in this extension / ecosystem.

I think this issue is now fixed, but I discovered a new one #318.

Once #318 is fixed, if confirmed to be an issue I will re-make a pass this verify this issue.