nvim-neotest / neotest-go

MIT License
124 stars 43 forks source link

feat: use --run flag when executing single test ( run single test instead of all of them ) #81

Open encero opened 4 months ago

encero commented 4 months ago

Run the go test command with --run flag when executing single test.

Support for neotest type=test was removed in https://github.com/nvim-neotest/neotest-go/pull/72

This is unfortunate but i believe in wisdom of the maintainers. Anyway i really need this functionality. Otherwise the plugin is unusable on projects with e2e/integration tests. I can't execute the whole suite each time, that is job for the CI.


The solution is untested for all the different nested, map/slice based tests. This works for me right now. This PR is mostly for visibility only. I hope when someone encounter the same roadblock they can use this a inspiration how to solve it.

Plus multiple people are trying to merge stuff to the same piece of code and i don't want to fight over it right now.

Way of avoiding this all together would be providing extension point inside the build_spec function for users to change the outcome themselves. Something in line of

require("neotest-go")({
     test_command_hook = function(command, position) 
         table.insert(command, "--run")
         table.insert(command, position.name)
     end,
      args = { "-count=1", "-timeout=60s" }
    })
fredrikaverpil commented 4 months ago

I just added an issue about this here without realizing there was a PR up for this already: https://github.com/nvim-neotest/neotest-go/issues/83

fredrikaverpil commented 4 months ago

@encero I think the regex looks a bit wrong for subtests (nested t.Run()).

Let's say you have this and run "Level 3":

func Test_Level_1(t *testing.T) {
    t.Run("Level 2", func(t *testing.T) {
        t.Run("Level 3", func(t *testing.T) {  // I had my cursor here when I ran "nearest test"
            assert.Equal(t, 1, 1)
        })
    })
}

The neotest-go command then gets generated like this on my end:

{ "cd", "/Users/fredrik/code/public/go-playground/bugs/neotest-go", "&&", "go", "test", "-v", "-json", "", "--run", '\\^"Level 2"/"Level 3"\\$', "./" }

Here, the regex does not look right: \\^"Level 2"/"Level 3"\\$. The test's actual name is Test_Level_1/Level_2/Level_3.

fredrikaverpil commented 4 months ago

I took a quick stab at trying to get the regex cleaner (as pointed out here). But I have no idea how brittle my approach is.

commit 41a50d2feb9320e830bf9e37f40606b04bb6f81e
Author: Fredrik Averpil <fredrik.averpil@gmail.com>
Date:   Sat Feb 24 16:39:21 2024 +0100

    fix: run nearest tests

diff --git a/lua/neotest-go/init.lua b/lua/neotest-go/init.lua
index f2b0ab7..92b35ea 100644
--- a/lua/neotest-go/init.lua
+++ b/lua/neotest-go/init.lua
@@ -147,6 +147,14 @@ function adapter.discover_positions(path)
   })
 end

+function cleanString(str)
+  str = str:match(".*%.go::(.*)") -- Remove path up to and including '.go::'
+  str = str:gsub("[\"']", "") -- Remove quotes
+  str = str:gsub("%s", "_") -- Replace spaces with underscores
+  str = str:gsub("::", "/") -- Replace double colons with slashes
+  return str
+end
+
 ---@async
 ---@param args neotest.RunArgs
 ---@return neotest.RunSpec
@@ -161,6 +169,14 @@ function adapter.build_spec(args)
   if fn.isdirectory(position.path) ~= 1 then
     location = fn.fnamemodify(position.path, ":h")
   end
+
+  local run_flag = {}
+  if position.type == "test" then
+    run_flag = { "-run", "^" .. cleanString(position.id) .. "$" }
+  end
+
   local command = vim.tbl_flatten({
     "cd",
     location,
@@ -171,8 +187,12 @@ function adapter.build_spec(args)
     "-json",
     utils.get_build_tags(),
     vim.list_extend(get_args(), args.extra_args or {}),
+    run_flag,
     dir,
   })
+
   return {
     command = table.concat(command, " "),
     context = {

This will yield the regex ^Test_Level_1/Level_2/Level_3$

Unfortunately, I'm not sure how to figure out dynamically named tests, like this one:

type Skipper struct {
    name string
    skip bool
}

func Test_Level_1(t *testing.T) {
    for _, c := range []Skipper{{name: "Level 2a", skip: false}, {name: "Level 2b", skip: true}} {
        c := c
        t.Run(c.name, func(t *testing.T) {
            if c.skip {
                t.Skip()
            }
            t.Run("Level 3", func(t *testing.T) {
                if c.skip {
                    t.Skip()
                }
            })
        })
    }
}
encero commented 4 months ago

Unfortunately, I'm not sure how to figure out dynamically named tests, like this one:

I don't think you can do that without a go "interpreter".

I would expect those kind of test to shop up in the summary window. I they would then a workaround will be to run the whole test func and then select a sub test in the summary window. But that is not the case, the test doesn't show up in the summary 🤔.


  str = str:match(".*%.go::(.*)") -- Remove path up to and including '.go::'

You can maybe avoid doing regex magic by walking the tree?

There is already "partial" walk in the utils.get_prefix. I believe you can walk the whole tree and build the test name bit by bit that way.

function utils.get_prefix(tree, name)
  local parent_tree = tree:parent()
  if not parent_tree or parent_tree:data().type == "file" then
    return name
  end
  local parent_name = parent_tree:data().name
  return parent_name .. "/" .. name
end

Thank you @fredrikaverpil for spending time on this.

fredrikaverpil commented 4 months ago

Dynamically generated sub-tests

I don't think you can do that without a go "interpreter".

@encero you are right, after some investigation it doesn't seem that we can figure out what the names are of dynamically runtime-generated sub-tests here.

I would expect those kind of test to shop up in the summary window.

The test summary doesn't show the full picture, and you cannot run the "Level 3" test unless you "run nearest" at Level 1 or Level 2.

image

So putting that dream to rest, maybe let's focus on landing this PR as it is super useful and I'm also in the same integration test scenario as you, where I cannot have all tests run.

And just in general, we shouldn't run all tests when you choose to "run nearest".

Regex

You can maybe avoid doing regex magic by walking the tree?

Maybe. Not sure if it's worth it though, since we get the data we need from the position, it just needs to be reformatted, like I did in the regex.

What do you think @sergii4 (see my diff here)? It does the job IMHO.

encero commented 4 months ago

Found few minutes to spent on this

Maybe. Not sure if it's worth it though, since we get the data we need from the position, it just needs to be reformatted, like I did in the regex.

I personally hate string manipulation, and regexes in general. Sometimes they are unavoidable sure.

you are right, after some investigation it doesn't seem that we can figure out what the names are of dynamically runtime-generated sub-tests here.

You could unleash an ast on it, i sure don't have a time for that though.


Still need to find a will power to test the approach for all the different supported test types ( tables, spec, etc... )

fredrikaverpil commented 3 months ago

@encero @sergii4 what are your opinions on the best approach here?

I would really like us to try and move forward here with any kind of implementation that avoids having to maintain a local fork of neotest-go, which is currently what I have to do.

sergii4 commented 3 months ago

@fredrikaverpil what's the urgency? Why do you have to maintain a local fork ?

sergii4 commented 3 months ago

@encero could you please fix formatting?

fredrikaverpil commented 3 months ago

@fredrikaverpil what's the urgency? Why do you have to maintain a local fork ?

I am relying on neotest-go at work and I am dependent on being able to run just one single test.

encero commented 2 months ago

@encero could you please fix formatting?

@sergii4 the formatting should be fixed now, at least is green in my fork.