thenbe / neotest-playwright

A playwright adapter for neotest.
MIT License
42 stars 5 forks source link

Tests reported as failures even though they all passed #21

Open jay-babu opened 8 months ago

jay-babu commented 8 months ago

image

thenbe commented 8 months ago

I'm having trouble reproducing this on my machine (using playwright v1.41.2)

Does this only happen in a certain scenario? If so, can you guide me to reproduce it?

Or does it happen every time?

vietch2612 commented 8 months ago

I have the same issue :(

thenbe commented 8 months ago

What version of @playwright/test are you using? And does it happen every time, or only in certain scenarios?

vietch2612 commented 8 months ago

What version of @playwright/test are you using? And does it happen every time, or only in certain scenarios?

I'm using Version 1.41.2.

It happens after I modify any codes in the spec file.

https://github.com/thenbe/neotest-playwright/assets/14076603/3f71faa6-3b68-4737-b6b0-33eac476b3ef

thenbe commented 8 months ago

Thanks, I'm able to reproduce this now.

thenbe commented 8 months ago

The underlying issue happens when both of the following are true:

For more info on why it happens, see: https://github.com/thenbe/neotest-playwright?#caveats


Can you confirm if the following workaround resolves the issue?

When the issue occurs, do the following:

  1. Run :NeotestPlaywrightRefresh
  2. Save file
  3. Run the test again
  4. Observe that test result is now reported correctly.
vietch2612 commented 8 months ago

The underlying issue happens when both of the following are true:

  • enable_dynamic_test_discovery = true
  • The line on which the test is declared has changed OR the test has been renamed.

For more info on why it happens, see: https://github.com/thenbe/neotest-playwright?#caveats

Can you confirm if the following workaround resolves the issue?

When the issue occurs, do the following:

  1. Run :NeotestPlaywrightRefresh
  2. Save file
  3. Run the test again
  4. Observe that test result is now reported correctly.

Hi @thenbe, following those steps, the test result is correct. Do we have a plan to fix this?

thenbe commented 8 months ago

We'd need a heuristic that runs :NeotestPlaywrightRefresh for us automatically. But one that does not run it very often so as not to impact performance. I'm thinking out loud here, but maybe something that runs :NeotestPlaywrightRefresh after a test is run and we detect that the issue is happening. This won't fix 100% of the cases (moving or renaming the test then saving will still throw off the test indicator), but at least when you re-run the test it'll fix itself.

FWIW, I have :NeotestPlaywrightRefresh set to a keymap to work around this myself. But I agree that it's annoying and should ideally be something users should not have to think about.

jay-babu commented 8 months ago

Running the command on file save should be enough right via autocmd?

thenbe commented 4 months ago

Running the command on file save should be enough right via autocmd?

I must've missed your comment. Yeah that should be enough.