-
Notifications
You must be signed in to change notification settings - Fork 43
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
[Feature request] Support for Testify framework #6
Comments
@lourenci features are driven by people in the community contributing what changes they need for their work, see https://github.com/nvim-neotest/neotest-go#contributing Although tbh I'm not sure why this isn't already working, I don't use the suite feature myself, but it's just a function call inside a |
As far as I can see // In order for 'go test' to run this suite, we need to create
// a normal test function and pass our suite to suite.Run
func TestExampleTestSuite(t *testing.T) {
suite.Run(t, new(ExampleTestSuite))
} is still wrapped by a regular go test func so this should run like anything else 🤷🏿 |
Sorry. I didn't mean to urge for a feature. I know OSS is community-driven, but, unfortunately, I don't have enough knowledge in Lua to properly do that by myself. 😕 Please, leave it open for someone willing to help. Yes. It's weird. In the attached print, after running all the tests ( |
Oh, that's sort of a different issue, it isn't so much that testify isn't working since from what I can see the test does run, the issue you are reporting is that the test isn't detected in the correct place? Is that right? That's makes more sense now, so the reason that happens is that this adapter needs to be told exactly "what" constitutes a test for a given language, which it does with treesitter queries. Currently, the queries identify It shouldn't be too much work and tbh I think if there are things you are going to want to tweak it's worth looking into lua, it's really not a complex language I'd say easier than go. Although in this case what you probably want to do is add a new treesitter query to neotest-go/lua/neotest-go/init.lua Lines 177 to 193 in e67dd4c
I'd also suggest pasting a test here so that someone picking this up can add it to the tests in the sample go project in this repo so anyone developing can test against it |
@lourenci I added this in b3b8cb8, but there are some limitations to this which are limitations in @rcarriga there's another issue, go test returns the result of the subtest as func TestFunc() { // This is just a test
assert.True(...)
}
func TestFunc2() { // This is namespace with one test
t.Run("one", func() {...})
} I tried tweaking the query to capture the test func as both test definition and namespace definition, but that doesn't work for some reason it fails silently in |
Hey, @akinsho. Thank you so much for going into this. In Testify, we can run individual tests like this stretchr/testify#460 (comment). I'll try to look into this if no one gets this before. |
@lourenci that's a great find and good to know. I think it might be a good use case for adding the first test runner to the plugin since running that command is dependent on using testify, so there should probably be a runner like there is for https://github.com/nvim-neotest/neotest-python. e.g. require('neotest-go')({
runner = 'testify',
}) And then if running the test nearest command with the runner set to testify then it should run the command you found |
Yep I'm not surprised it fails, neotest doesn't expect overlapping positions without a parent/child link.
I'm not really sure on why the IDs can't be linked as is, could you clarify what prevents this? |
@rcarriga so the current way that ids are generated in this plugin is that the namespaces are prefixed with the test name which is transformed to match what go test expects e.g. trimming white space etc so the average test will have an id like The output from go test though looks like Essentially the issue is that there is no way when generating ids for me to namespace a subtest underneath the parent. So I can't correctly map the nested tests to the correct result later on. Hopefully that makes sense |
I think I understand the issue with IDs being linked, my gap in understanding is how setting the parent test to be a namespace would solve this issue. Or am I am misunderstanding the solution too? 😅 |
Ah right so currently when generating the IDs there is no namespace returned by the ID generation function, so my thinking is that if the parent test is correctly identified as a namespace via a TS query then it will be returned as part of the namespaces in the id gen function and so I can prefix tests at the id gen stage. local function generate_position_id(position, namespaces) <--- these don't have the parent in them since it is not a namespace
local prefix = {}
for _, namespace in ipairs(namespaces) do
if namespace.type ~= 'file' then
table.insert(prefix, namespace.name)
end
end
local name = transform_test_name(position.name)
return table.concat(vim.tbl_flatten({ position.path, prefix, name }), '::')
end
|
Ah OK now I see what the issue is! So you should be seeing the parent tests in the namespaces arg (should really be called parents to be clearer). If you're not that would be a bug, but I'd ask to double check that because there is a test checking that it works. Note that right now the parsing assumes that parent tests/namespaces are defined around their children, so if they don't overlap it won't detect the relationship at all. |
@rcarriga that's part of the problem here I think because in go especially in this case they might not overlap because the testify library allows specifying a test as a method which is defined outside the parent test function |
func (suite *ExampleTestSuite) TestExampleFailure() {
assert.Equal(suite.T(), 5, 3)
}
// In order for 'go test' to run this suite, we need to create
// a normal test function and pass our suite to suite.Run
func TestExampleTestSuite(t *testing.T) {
suite.Run(t, new(ExampleTestSuite))
} This is one such example where |
OK so the actual issue then is detecting children not within the range of the parent. The logic to determine if a position is a child is the |
🤔 this might be a little tough since a user could theoretically alias the name of the Tbh I don't use the suite function of |
If anyone does wish to pick this up, I'd be happy to assist 👍 |
I think a good way to begin solving this issue would be with func TestSuite(t *testing.T) {
t.Run("TestOne", func(t *testing.T){})
t.Run("TestTwo", func(t *testing.T){})
t.Run("TestThree", func(t *testing.T){})
} |
They do it in VSCode pretty well, I'll try to look into it. |
@rcarriga I'd suggest adding two new captures, something like: If any namespace N Example:
If the query captures the following:
Neotest will consider This would probably be best implemented in neotest itself, not in the adapters, the adapters would just add the captures to the treesitter query. Let me know if you like the idea and I can find some time to implement it. |
This would be a pretty large departure from how the treesitter parsing currently works. Not at all opposed to seeing a |
Did anyone find a good workaround for that? |
Nope, tried tweaking the code a bit to make it run but I don't understand the code enough to make it work. I tried making the adapter run using For now this is a blocker for me to use neotest as the codebase I work on will not drop testify. I have somewhat of a workaround, I made a script which will get the method name under my cursor(or the closest) and run |
I hacked together a solution that works reasonably well, see #67 |
For those of you who wish to try it out, https://github.com/fredrikaverpil/neotest-golang now has testify support. |
Hi, thank you for the plugin.
When using the frameworks' suite feature, this plugin fails to run individual tests. Since the usage of the Testify in Go projects has been increasing daily, it would be awesome to have this plugin supporting it.
The text was updated successfully, but these errors were encountered: