nvim-neotest / neotest-go

MIT License
133 stars 43 forks source link

refactor: parse go test errors and return them to neotest #14

Closed jens1205 closed 2 years ago

jens1205 commented 2 years ago

HI,

this PR adds parsing of go test errors into the adapter.

To do this, the normalized key of the intermediate representation of the test results was changed to "package-name::test-name::subtest::name"

Regards

Jens

akinsho commented 2 years ago

@jens1205 so this change generally looks good, but I'm not sure exactly how to test what it's supposed to be doing since there are quite a few changes. Can you give more details about what exactly this changes, maybe some before and after notes?

jens1205 commented 2 years ago

One thing I'm finding a little tough is following exactly all the changes that this PR introduces. It seems to be doing quite a few things which aren't explained clearly in the PR description

I don't know, 99% of the changes were done just to be able to deal with the error output of go test. I added

To do this, the normalized key of the intermediate representation of the test results was changed to "package-name::test-name::subtest::name"

to the PR description (the only major shift besides the introduction of the parsing of the test output)

I accepted all your review comments. If you have further questions, don't hesitate to ask.

Regards

Jens

akinsho commented 2 years ago

@jens1205 thanks for the PR tested it a little and seemed to work. Couldn't find any of the exact situations where go test errors needed to be reported back, but since it all still works 🀷🏿

alesbrelih commented 2 years ago

This broke the plugin for me. Every test gives some info about the test which results into failed tests.

Example output:

{"Time":"2022-09-01T12:13:17.458992+02:00","Action":"output","Package":"..","Test":"TestUpdate","Output":"=== RUN   TestUpdate\n"}
{"Time":"2022-09-01T12:13:17.458994+02:00","Action":"output","Package":"...","Test":"TestUpdate","Output":"=== PAUSE TestUpdate\n"}
{"Time":"2022-09-01T12:13:17.459061+02:00","Action":"pause","Package":"....","Test":"TestUpdate"}

What I can think of is that the pattern matching differs from OS 🀷

akinsho commented 2 years ago

@alesbrelih what OS are you on? I can't reproduce your issue, so likely you'll need to do some debugging on your side to figure out what isn't working.

lunarxlark commented 2 years ago

@akinsho I use macOS. I can reproduce this issue in ./neotest-go/*_test.go. Do this plugin in your PC work correctly in ./neotest-go/*_test.go? When I specify to commit 2ad3c27, this work correctly.

OS   : macOS Monterey version 12.5.1
nvim : v0.7.2 (and v0.8.0-dev-2465-g12fe197cf-dirty)

γ‚Ήγ‚―γƒͺγƒΌγƒ³γ‚·γƒ§γƒƒγƒˆ 2022-09-02 13 37 48

akinsho commented 2 years ago

@lunarxlark thanks for pointing this out. I tested this PR in a different project that didn't have any special characters (for lua) in the name and that worked fine, which was why I couldn't reproduce. Seems the issue is with special characters like _ etc. I can reproduce it in neotest_go. I've just pushed a fix, can people in this thread let me know if it's fixed for them. Seems to be fixed for me

lunarxlark commented 2 years ago

@akinsho Thanks for fix.It work correctly for me. πŸ‘ πŸ‘ πŸ‘

akinsho commented 2 years ago

I made another change actually since the OPs example still wasn't working for me, can you verify it still works with the latest commit @lunarxlark make sure I didn't just break it again

lunarxlark commented 2 years ago

the OPs example still wasn't working for me sorry. some examplewasn't work, me too. for below tests, display all test json result.

  • main_tagged_test.go
  • suite_test.go
  • TestExample
  • TestExampleFailure

γ‚Ήγ‚―γƒͺγƒΌγƒ³γ‚·γƒ§γƒƒγƒˆ 2022-09-02 16 40 25

akinsho commented 2 years ago

@lunarxlark can you confirm you're on the latest commit which now uses substitute?

alesbrelih commented 2 years ago

This is somehow connected to the comment I left you on my PR, the current test file name pattern doesn't allow _ or - in the file names.

lunarxlark commented 2 years ago

@akinsho i use commit 6649d89 (latest). When i execute test in sidebar, it work correctly. But, when I execute test in test code, it failed to parse test result and display resutl json. and test for directory, it failed parse test result.

akinsho commented 2 years ago

Ok should all be fixed now thanks to @alesbrelih PR #17 πŸ‘πŸΏ