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

feature: test args evaluated at runtime #186

Closed
2 tasks done
kvalv opened this issue Sep 16, 2024 · 4 comments · Fixed by #188
Closed
2 tasks done

feature: test args evaluated at runtime #186

kvalv opened this issue Sep 16, 2024 · 4 comments · Fixed by #188
Labels
enhancement New feature or request

Comments

@kvalv
Copy link

kvalv commented Sep 16, 2024

Did you check docs and existing issues?

Is your feature request related to a problem? Please describe.

Hi, first of all thanks for taking the effort to write this plugin! Very happy to see a neotest adapter that works in monorepos, and the documentation provided in the README has made the adapter easy to configure.

I am currently working in a monorepo that roughly has this structure:

/serviceA
    /something_test.go
/serviceB
    /something_else_test.go
...

If I'm in service A, I would like to test with go test ./... -flagA and in service B I would run tests like go test ./... -flagB. So the flags I want to use can differ on each service. Based on the configuration I can pass in go_test_args for providing flags. However, the go_test_args is initialized on startup (or whenever neotest-golang is configured), so it's not possible to adjust them depending on the current file - they are static.

Describe the solution you'd like to see.

Approach 1 - static

I already have one workaround - depending on the current working directory I start neovim in, set the appropriate flags. Something like this:

local function get_test_args()
	local cwd = vim.fn.getcwd()
	local flags = { }
	if string.find(cwd, "serviceA") then
		table.insert(flags, "-flagA")
	elseif string.find(cwd, "serviceB") then
		table.insert(flags, "-flagB")
	end
	return flags
end

require("neotest").setup({
	adapters = {
		require("neotest-golang")({
			go_test_args = get_test_args(),
		}),
	}
})

A disadvantage of this approach is that I must start the neovim instance in the /serviceA directory to be able to run tests in serviceA. Ideally, I don't want this to matter -- if I'm in a test file, I should be able to run a test appropriately, without having to worry about where I started the neovim session.

Approach 2 - dynamic

Instead of go_test_args being a table, it can either be a table or a function that returns a table. If it's a function, evaluate the function to produce the arguments used for testing. Continuing on the example above, it would be something like this:

require("neotest").setup({
	adapters = {
		require("neotest-golang")({
			experimental = {
				test_table = true,
				recursive_run = true,
			},
+          		go_test_args = get_test_args,
-                       go_test_args = get_test_args(),
		}),
	}
})

Describe alternatives you've considered.

  • See above, the static approach
  • I also checked the neotest documentation to see whether neotest.run.run() supported arguments, but it did not.
  • I looked at adapters for other languages to see if they had something similar - only picked a few of them, and did not see that they had something similar as what I request - might mean that it's possible to do what I want to do in a different way.

Additional context

A very naive implementation would probably be something like this? Haven't tested it, and don't have good understanding of how neotest adapters are written -- just my attempt to figure out whether this is a large feature request or a little one 😄

diff --git a/lua/neotest-golang/lib/cmd.lua b/lua/neotest-golang/lib/cmd.lua
index a1e31c3..1d2a7a1 100644
--- a/lua/neotest-golang/lib/cmd.lua
+++ b/lua/neotest-golang/lib/cmd.lua
@@ -81,7 +81,12 @@ end
 
 function M.go_test(go_test_required_args)
   local cmd = { "go", "test", "-json" }
-  cmd = vim.list_extend(vim.deepcopy(cmd), options.get().go_test_args)
+  local args = options.get().go_test_args
+  -- if it's a function, evaluate
+  if type(args) == "function" then
+    args = args()
+  end
+  cmd = vim.list_extend(vim.deepcopy(cmd), args)
   cmd = vim.list_extend(vim.deepcopy(cmd), go_test_required_args)
   return cmd
 end
@kvalv kvalv added the enhancement New feature or request label Sep 16, 2024
@fredrikaverpil
Copy link
Owner

Hi @kvalv,

Hi, first of all thanks for taking the effort to write this plugin! Very happy to see a neotest adapter that works in monorepos, and tWhe documentation provided in the README has made the adapter easy to configure.

Many thanks for these kind words ❤️ I'm really happy the monorepo support works "in the wild" as well 😄 as I haven't heard much on this particular topic (could also be a sign it just works for a majority of the cases, heh).

I'd be happy to have the adapter support your use case of wanting to provide a function, which would be called just-in-time style, rather than be sourced on adapter intialization. It just happens that there was another inquiry in #187 that might be of interest to you, which talks about expanding the ability to customize the test runner and its logic. You can subscribe/follow that if you wish.

I think your naive implementation of detecting whether the go_test_args is a table vs a function will work fine. Did you try it out on your end?
It would be a pretty small change to the codebase too (it's basically your diff + a test) and we should be able to continue to support this with any work that spring out of the discussion I linked above.

I guess we could whip up a quick PR just to have something to have you try out.

@fredrikaverpil
Copy link
Owner

fredrikaverpil commented Sep 17, 2024

Please give something like this a try:

return {
  {
    "nvim-neotest/neotest",
    dependencies = {
      "nvim-neotest/nvim-nio",
      "nvim-lua/plenary.nvim",
      "antoinemadec/FixCursorHold.nvim",
      "nvim-treesitter/nvim-treesitter",
      {
        "fredrikaverpil/neotest-golang", -- Installation
        branch = "args-function", -- Custom branch
      },
    },
    config = function()
      local opts = { -- Specify configuration
        go_test_args = function()
          -- Place your custom logic here for the 'go test' args
          return {
            "-v",
            "-race",
            "-count=1",
            "-coverprofile=" .. vim.fn.getcwd() .. "/coverage.out",
          }
        end,
      }
      require("neotest").setup({
        adapters = {
          require("neotest-golang")(opts), -- Apply configuration
        },
      })
    end,
  },
}

It uses the args-function branch (#188) and you should be able to assign a function to the go_test_args. Let me know how this works for you.

By the way, are these build flags?
If so, you need to also supply the flags to go_list_args. I added function support there too.

@kvalv
Copy link
Author

kvalv commented Sep 17, 2024

Hi, nice to see a PR for this out already! I checked out the branch and it works as expected. So believe if this gets merged it would certainly solve my problems 😄

By the way, are these build flags?

No, I just have some tests that depend on external services that are skipped if no flags are provided. For example, if I run go test ./serviceA -postgres I would run all unit tests + those depending on postgres.

Regarding go_list_args: probably not required for my case (as I don't use build flags). Whether you want to keep it or not is up to you, just saying I wouldn't be upset if it got removed from the PR.

Thanks again for the quick response!

@fredrikaverpil
Copy link
Owner

Ok, nice. I'll just add some tests for the options (so that function support is covered) and then I'll merge this in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants