nvim-neotest / neotest-go

MIT License
124 stars 43 forks source link

Error with Neotest-go, subtests and skipping them. #80

Closed MortenGerdes closed 4 months ago

MortenGerdes commented 4 months ago

I ran into an error where some strange behaviour happens when you mix tests and subtests that may or may not print out stuff.

I'm not too much into the Neotest-go code to know how why this happens, but here's the code and the error for anyone wanting to reproduce it and dive in.

package lexer

import (
    "testing"
)

func TestNeoTestGo(t *testing.T) {
    t.Run("TestNeoTestGo2", func(t *testing.T) {
        t.Parallel()
        t.Log("Hello NeotestGo")
    })

    t.Run("TestNeoTestGo3", func(t *testing.T) {
        t.Run("TestNeoTestGo3.1", func(t *testing.T) {
            t.Skip()
            t.Parallel()
            t.Log("TestNeoTestGo3.1")
        })
    })
}

When I hold my cursor over line 13, which says "TestNeoTestGo3" and run the test file, I get the following error:

...cal/share/nvim/lazy/neotest-go/lua/neotest-go/output.lua:84: bad argument #1 to 'insert' (table expected, got nil)
stack traceback:
    [C]: in function 'insert'
    ...cal/share/nvim/lazy/neotest-go/lua/neotest-go/output.lua:84: in function 'marshal_gotest_output'
    ...local/share/nvim/lazy/neotest-go/lua/neotest-go/init.lua:214: in function 'results'
    ...al/share/nvim/lazy/neotest/lua/neotest/client/runner.lua:132: in function '_run_spec'
    ...al/share/nvim/lazy/neotest/lua/neotest/client/runner.lua:89: in function <...al/share/nvim/lazy/neotest/lua/neotest/client/runner.lua:88>

Neovim stats:

NVIM v0.10.0-dev
Build type: RelWithDebInfo
LuaJIT 2.1.0-beta3
Run "nvim -V1 -v" for more info

Neotest branch 89a5b1f
Neotest-go branch ba5d536

Looking into the code of Neotest-go, looking into the neotest-go/output.lua file We have:

       .....
          else
            tests[testname].file_output[testfile][linenumber] = {}
          end
        end

        -- if we are in the context of a file, collect the logged data
        if testfile and linenumber and utils.is_test_logoutput(parsed.Output) then
          table.insert(
            tests[testname].file_output[testfile][linenumber],
            sanitize_output(parsed.Output)
          )
        end
      .....

If I add the following line, I seem to fix it. However I have zero clue if this is actually the appropriate fix or if it breaks something else. I was hoping someone with a bit more knowledge of the repo could tip in. I added the following:

       .....
          else
            tests[testname].file_output[testfile][linenumber] = {}
          end
        end

        -- if we are in the context of a file, collect the logged data
        if testfile and linenumber and utils.is_test_logoutput(parsed.Output) then
            if not tests[testname].file_output[testfile][linenumber] then
              tests[testname].file_output[testfile][linenumber] = {}
            end
          table.insert(
            tests[testname].file_output[testfile][linenumber],
            sanitize_output(parsed.Output)
          )
        end
      .....

I reproduced this both on WSL and MacOS. Let me know if I can help provide more information if needed. Thanks in advance.

tkrause commented 4 months ago

I'm also seeing this but with a very cryptic error about a nil pointer in a coroutine.

Seems to happen with subtests consistently where I have multiple levels of subtests.

          if not tests[testname].file_output[testfile] then
            tests[testname].file_output[testfile] = {}
          end

          if not tests[testname].file_output[testfile][linenumber] then
            tests[testname].file_output[testfile][linenumber] = {}
          end

In that same file, at least keeps it from crashing. But this seems like a bigger issue with parsing subtests.

fredrikaverpil commented 4 months ago

I am also seeing an error around marshal_gotest_output when using nested tests that might be using t.Skip():

   Warn  10:59:47 notify.warn Neotest neotest-go: ...edrik/.local/share/LazyVim/lazy/neotest/lua/nio/init.lua:105: The coroutine failed with this message: 
.../share/LazyVim/lazy/neotest-go/lua/neotest-go/output.lua:82: attempt to index a nil value
stack traceback:
    .../share/LazyVim/lazy/neotest-go/lua/neotest-go/output.lua: in function 'marshal_gotest_output'
    ...al/share/LazyVim/lazy/neotest-go/lua/neotest-go/init.lua:214: in function 'results'
    ...share/LazyVim/lazy/neotest/lua/neotest/client/runner.lua:132: in function '_run_spec'
    ...share/LazyVim/lazy/neotest/lua/neotest/client/runner.lua:89: in function <...share/LazyVim/lazy/neotest/lua/neotest/client/runner.lua:88>

In my case I have a test looking like this:

func TestAddSubTestLevelSkipping(t *testing.T) {
    for _, skip := range []bool{true, false} {
        t.Run("Subtest1", func(t *testing.T) {
            t.Run("Subtest2", func(t *testing.T) {
                if !skip {
                    t.Skip("skipping test")
                }
            })
        })
    }
}

It works fine on the commandline:

go test -timeout 30s -v -run ^TestAddSubTestLevelSkipping$ yolo/cmd
=== RUN   TestAddSubTestLevelSkipping
=== RUN   TestAddSubTestLevelSkipping/Subtest1
=== RUN   TestAddSubTestLevelSkipping/Subtest1/Subtest2
=== RUN   TestAddSubTestLevelSkipping/Subtest1#01
=== RUN   TestAddSubTestLevelSkipping/Subtest1#01/Subtest2
    app_test.go:42: skipping test
--- PASS: TestAddSubTestLevelSkipping (0.00s)
    --- PASS: TestAddSubTestLevelSkipping/Subtest1 (0.00s)
        --- PASS: TestAddSubTestLevelSkipping/Subtest1/Subtest2 (0.00s)
    --- PASS: TestAddSubTestLevelSkipping/Subtest1#01 (0.00s)
        --- SKIP: TestAddSubTestLevelSkipping/Subtest1#01/Subtest2 (0.00s)
PASS

What's interesting here is that by removing the ! in front of skip, the test passes without issues in neotest-go:

-               if !skip {
+               if skip {

Could it be that neotest-go doesn't properly parse the /#01/ in the path TestAddSubTestLevelSkipping/Subtest1#01/Subtest2?

Also, note that the second subtest (Subtest2) is also subject to this bug: https://github.com/nvim-neotest/neotest-go/issues/52

fredrikaverpil commented 4 months ago

I added a reproducible example here:

sergii4 commented 4 months ago

@MortenGerdes could you please update and check now?

fredrikaverpil commented 4 months ago

I can confirm I no longer get the marshal_gotest_output error I reported here on my end anymore. 🎉

MortenGerdes commented 4 months ago

@sergii4 Seems fixed on my end after your update. I had two places where I ran into the issue and both are now working 🙌

Great work!