-
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
feat: detect subtests #1
Conversation
OK so the command being run looks like it's the correct one but for whatever reason it's not running correctly. I found that running with a shell by returning command as a string seems to work but I've no idea why local command = table.concat({ 'go', 'test', '-v', '-json', unpack(cmd_args) }, " ") |
Thanks, @rcarriga tried that and seems to work as expected, but definitely strange that the string works differently. Are you using Anyway, another question 😅 I'm trying to control how test IDs are created for nested tests. For example, it is using the name of the test which is great it identified it at all but I would like to nest them under the containing test .e.g func TestDoesAThingAnotherWay(t *testing.T) {
t.Run("test one", func(t *testing.T) {
assert.Equal(t, 3, doesAThing(1, 2))
})
} The nested test has an ID of Do I need to specify the wrapping function as a namespace? |
Alright looking through the code it seems that I'd have to identify the parent function as a namespace which is probably correct anyway/what I'd want to do |
Yep it is using jobstart 👍 To change the ID you just need to change it before returning from You can have nested tests, it was actually the summary consumer just arbitrarily didn't show them. I've removed the restriction so it should show correctly. There might be other places where nested tests are presumed to not happen but I'd consider them a bug as it's possible in multiple languages so neotest should allow it. |
Hmm, so I went down the path of trying to register the containing function as a namespace for its descendants using ((function_declaration
name: (identifier) @namespace.name
body: (block
(call_expression
function: (selector_expression
field: (field_identifier) @_method)
(#match? @_method "^Run"))))
(#match? @namespace.name "^Test"))
@namespace.definition So this correctly identifies the right function (needs work to be more general) but now I'm getting test IDs that look like
I'm a little stumped about how to approach this since it almost fits together, maybe the parent test function shouldn't be a namespace, tbh I don't know what a namespace should represent I thought maybe a package/module. I then went down the path of trying to manipulate the tree gotten from |
To be clear just calling iter or iter_nodes is enough to clear out the tree |
So a namespace is pretty arbitrary, it really comes down to what makes sense for the language. In my mind if there are assertions being made inside a function then it is a test, if it only contains tests then it is a namespace. Leaving it as a test would solve your issue of the test function being registered as a test and a namespace and avoid the nested ID. The tree is not really designed to be changed. There is a function to merge them but it is pretty rough, I definitely wouldn't encourage adapters to do so. This is why I was thinking of exposing lower level functionality inside the treesitter lib so that you can get it in the form of nested lists which is parsed by the Tree class. Then it would be easier to alter. |
So this is tricky in go because both things are possible i.e. a function can just make assertions, and you can structure your tests as one function with a bunch of subtests, or you can create several functions in a test file each testing functions i.e. a test function can be a test itself or a namespace. It seems like conceptually they should be namespaces in the example above ideally I'd check if a function contained any So maybe just the ability to prefix each subtest would be best, maybe rather than exposing a bunch of low level functions, the treesitter position function could take a callback in the opts table that can be used to create an ID, so I could do something like opts = {
id_gen = function(position, ...)
return position.file .. ":" .. position:parent():name() .. "/" .. position:data():name
end
} |
Allow overriding the default ID constructor. See nvim-neotest/neotest-go#1
I've added a The tree is not constructed when creating IDs so it can't be supplied |
Thanks 👍🏿 I think it'll be useful to have that function but since it didn't return the parent it didn't quite match my use case, I'd ideally like to prefix each test name with all the parents as the ID, similar to the namespaces. It did make me realize I could just test for a subtest by getting the parent node whilst iterating the tree and then prefix it to the subtest and see if there's a match (because go test automatically names tests using this One lingering issue I have is that some tests might have the description be a variable, e.g. variable := "string"
t.Run(variable, func(t *testing.T) {
assert.Equal(t, 3, doesAThing(1, 2))
}) for the above when run go test will return |
Sorry the parameter name is a little misleading, the second argument is an ordered list of all parent positions which will includes namespaces and tests.
This is a tricky one. For plenary I was able to work around this by using the debug module to find where a test was declared and matching that to the known test positions but of course here you're not hooked into go test. I'm not sure there is a solution without hooking into the language to be honest. Is this handled by other editors like VSCode? If so you could try see how they do it |
It doesn't look like VSCode can handle variable names in the test discovery and actively ignores them golang/vscode-go#1602 (comment) with no plans to change that. |
So regarding the Regarding the variables and their underlying values, one difference vs. vscode is that we're using treesitter which is what made me think this might be possible since I'm wondering if it isn't possible to traverse the tree and find the value it points to? Not that I know how In either case that's kind of an enhancement that can come later since I'm just trying to get this usable and there are other things to sort out. It is quite a common use case in go though since it's very common to for _, case := range testcases {
t.Run(case.name, func(t *testing.T) {
// ...
})
} |
Ah sorry about that you're right there was a bug, should be working now. That's a very cool idea alright, especially if it's more common to have static variables for the test cases so they could be parsed. |
for easier parsing of subtest ids
61bb9d7
to
1700f40
Compare
This PR adds a query that will allow subtests declared using
t.Run
to be detected and run (?separately).TODO
@rcarriga I'm trying to get subtests like the example in this PR working
But whilst my query detects these correctly and they show signs and I can run the nearest the output from the test runner shows that it's not running the same command as I'm expecting. I verified that the command generated works in the terminal but with my cursor on one of those tests and running the
run.run
funciton despite the command created being the right one the output seems to show like some other command was run since it doesn't match the terminal output