feat: detect subtests

[akinsho]

This PR adds a query that will allow subtests declared using t.Run to be detected and run (?separately).

TODO

• [x] The correct test command is not run for these tests currently

@rcarriga I'm trying to get subtests like the example in this PR working

func TestDoesAThingAnotherWay(t *testing.T) {
    t.Run("test one", func(t *testing.T) {
        assert.Equal(t, 3, doesAThing(1, 2))
    })

    t.Run("test two", func(t *testing.T) {
        assert.Equal(t, 5, doesAThing(1, 2))
    })

    variable := "string"
    t.Run(variable, func(t *testing.T) {
        assert.Equal(t, 3, doesAThing(1, 2))
    })
}

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

[rcarriga]

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) }, " ")

[akinsho]

Thanks, @rcarriga tried that and seems to work as expected, but definitely strange that the string works differently. Are you using vim.fn.jobstart I know that that has some different behaviour based on string versus string array?

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 <path>:"test one" but I'd rather it was labelled as <path>:TestDoesAThingAnotherWay/"test one" where would I intervene to control how this works?

Do I need to specify the wrapping function as a namespace?

[akinsho]

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

[rcarriga]

Yep it is using jobstart :+1:

To change the ID you just need to change it before returning from discover_positions. Not sure how easy it will be to find the nested tests, let me know if the exposing lower level functions in the treesitter lib would make it easier.

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.

[akinsho]

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

value.id: "<path-to-file>::TestDoesAThingAnotherWay::TestDoesAThingAnotherWay::TestDoesAThingAnotherWay::variable" 

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 lib.treesitter.parse_positions but running tree:iter or tree:iter_nodes seems to mutate the tree and so no tests are found subsequently.

[akinsho]

To be clear just calling iter or iter_nodes is enough to clear out the tree

[rcarriga]

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.

[akinsho]

if there are assertions being made inside a function then it is a test, if it only contains tests then it is a namespace.

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 t.Run and class it as a namespace if it did or otherwise have it be a test but whilst this might be the most correct answer that sounds like quite a complex solution.

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
}

[rcarriga]

I've added a position_id function with signature fun(position: neotest.Position, namespaces: neotest.Position[]): string which should work for your needs I believe.

The tree is not constructed when creating IDs so it can't be supplied

[akinsho]

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 Parent/child) although this will break if tests are nested more deeply.

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 ParentTestFunc/string whereas the ID used in neotest will point to the name of variable e.g. variable not it's content. I'm not sure how feasible it is to do this though

[rcarriga]

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.

One lingering issue I have is that some tests might have the description be a variable

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

[rcarriga]

It doesn't look like VSCode can handle variable names in the test discovery and actively ignores them https://github.com/golang/vscode-go/issues/1602#issuecomment-881139861 with no plans to change that.

[akinsho]

So regarding the namespaces argument, I've tried using it, but it's always empty? It doesn't contain the parent positions as far as I can tell, which is why I added the workaround, not sure if that might be a bug or if they are missing, but I've not been able to find any values in that table.

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 do something like

for _, case := range testcases {
  t.Run(case.name, func(t *testing.T) {
   // ...
  })
}

[rcarriga]

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.