Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: testify subtests stopped working #275

Open
2 tasks done
uroborosq opened this issue Jan 20, 2025 · 10 comments · May be fixed by #280
Open
2 tasks done

bug: testify subtests stopped working #275

uroborosq opened this issue Jan 20, 2025 · 10 comments · May be fixed by #280
Labels
bug Something isn't working

Comments

@uroborosq
Copy link

uroborosq commented Jan 20, 2025

Did you check docs and existing issues?

Neovim version (nvim -v)

0.10.3

Operating system/version

archlinux

Output from :checkhealth neotest-golang

neotest-golang: require("neotest-golang.health").check()

Requirements ~
- OK Binary 'go' found on PATH: /usr/bin/go
- OK Found go.mod file for /home/dmitriy.tikhomirov/desktop/control_path/pkg/cp_errors/errors_test.go in /home/dmitriy.tikhomirov/desktop/control_path/go.mod
- OK Treesitter parser for go is installed
- OK neotest is available
- OK nvim-treesitter is available
- OK nio is available
- OK plenary is available

DAP (optional) ~
- OK Binary 'dlv' found on PATH: /home/dmitriy.tikhomirov/.local/share/nvim/mason/bin/dlv
- OK dap is available
- OK dapui is available
- OK dap-go is available

Gotestsum (optional) ~
- OK Binary 'gotestsum' found on PATH: /home/dmitriy.tikhomirov/.local/share/go/bin/gotestsum
- OK Tests will be executed by gotestsum.

Sanitization (optional) ~
- OK Sanitization is disabled

Describe the bug

Testify subtests no more detected by neotest, only suites and methods.
Looks like it related to bd8e69e, because usually receiver for suites is s, but this fix forbid any other receivers except t

If i rename s to t, everything works as expected

Steps To Reproduce

  1. Write a simple suite with such method
func (s *Suite) TestFoo() {
    s.Run("foo", func() {
        // do smth
    })
    s.Run("bar", func() {
        // do smth other
    })
}
  1. Open test summary, TestFoo is detected, but subtests foo and bar are not
  2. Also can change receiver to (t *Suite), when it works as expected

Expected Behavior

Subtests detected with any receiver name

Your Lua setup

@uroborosq uroborosq added the bug Something isn't working label Jan 20, 2025
@fredrikaverpil
Copy link
Owner

fredrikaverpil commented Jan 20, 2025

Thanks for reporting. Can you please check if suite.Run works for you?

func (suite *Suite) TestFoo() {
    suite.Run("foo", func() {
        // do smth
    })
    suite.Run("bar", func() {
        // do smth other
    })
}

@fredrikaverpil
Copy link
Owner

fredrikaverpil commented Jan 21, 2025

I haven't got the time right now, but if you wish you can try adding a test among the existing testify tests and then modify the queries in https://github.com/fredrikaverpil/neotest-golang/blob/main/lua/neotest-golang/features/testify/lookup.lua that mentions suite.

For example, you should be able to do something like this (two occurrences in the file):

- operand: (identifier) @suite_lib (#eq? @suite_lib "suite")
+ operand: (identifier) @suite_lib (#match? @suite_lib "^[suite|s]$")

@uroborosq
Copy link
Author

Hello, thanks for answering.
Case with suite doesn't help unfortunately.
I changed all lines with operand @suite_lib to add s receiver as you provided, but without success either.

As I can understand these queries, testify/lookup.lua contains queries for parsing like "starter" function for testify, which let standart golang test mechanism start testify suite.

My problem is with detecting subtests (e.g. table tests) which are defined in query.lua at the root (almost).
There were some changes, which now allows using only t as a receiver in subtests

@fredrikaverpil
Copy link
Owner

Ah yes, I think I made a mistake earlier then. I need to dig into this and see what to do. I think I would like to limit the use of s.Run and potential other variants to when you have the option testify_enabled = true only.

@fredrikaverpil fredrikaverpil linked a pull request Jan 24, 2025 that will close this issue
@fredrikaverpil
Copy link
Owner

fredrikaverpil commented Jan 24, 2025

@uroborosq would you mind giving the testify-operand branch a try?

{
  "fredrikaverpil/neotest-golang",
  branch = "testify-operand",
}

I think this might work. I just need to add some tests into #280 to verify it.

@uroborosq
Copy link
Author

@fredrikaverpil ok, thanks I will try in the nearest future

@uroborosq
Copy link
Author

Yes, it works but only for usual tests like

s.Run("foo", func() {
 // smth
})

but unfortunately doesn't work for table tests with such structure

cases := []testCase{ {name: "foo" ... }}
for _, test := range cases {
    s.Run(test.name, func() {
    ...
    }
}
}

@fredrikaverpil
Copy link
Owner

fredrikaverpil commented Jan 24, 2025

Table tests are not something I ever added support for, when it comes to testify. They are only supported for "vanilla" tests. Are you saying table tests used to work with testify? 🤔

The current testify specific tree-sitter queries would have to be extended to support table tests. I'm open to PRs which would enable this, but I don't plan on implementing this myself, as I don't use testify.

@uroborosq
Copy link
Author

Table tests had been working good enough until bd8e69e

There is no any significant differences between vanilla testing and testify table tests in my opinion, just using suite receiver instead of testing t, so yes, it worked

@fredrikaverpil
Copy link
Owner

Okay, interesting. I can probably re-introduce this behavior fairly simply in that case.

It's somewhat funny that all of that was a side effect of having the base treesitter queries mixed with the testify treesitter queries. It's probably a good idea to not mix them and instead have two different set of queries so to avoid future bugs.

I'll look into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants