nvim-neotest / neotest-go

MIT License
133 stars 43 forks source link

Test coverage generation is not working properly due to cwd being changed to the test file directory. #57

Open ddomurad opened 1 year ago

ddomurad commented 1 year ago

Hello. Recently a PR Enable run file tests despite nvim current directory was merged, and introduced a major change in how the test are being executed. It all comes down to the fact that currently, the go test ... command will be executed inside the tests file directory. This behavior has been hard-coded into the test command itself:

 local command = vim.tbl_flatten({
    "cd",
    dir,
    "&&",
    "go",
    "test",
    "-v",
    "-json",
    utils.get_build_tags(),
    vim.list_extend(get_args(), args.extra_args or {}),
    unpack(cmd_args),
  })

making it impossible to disable.

First of all I think it is a rather unexpected behavior, and I would assume that the go test... command would run from nvim cwd by default.

Second of all I found out that I'm not able to properly generate code coverage when the go test... is not executed from go.mod dir. Specifically my problem is with generating coverage for modules outside the test directory. Usually I would generate the coverage with a command like: go test -coverprofile=./coverage.out -coverpkg ./... <TEST_PATH_AND_TAGS> This would generate coverage for the whole project. Now when I comment out the 3 lines

    "cd",
    dir,
    "&&",

and run require("neotest").run.run({ extra_args = {"-coverprofile=./coverage.out", "-coverpkg ./..."} }) I get the result I'm expecting. However with the cd in place I get only the coverage from the level of the test dir. I have tried to provide absolute paths: require("neotest").run.run({ extra_args = {"-coverprofile=".. vim.fn.getcwd() .."/coverage.out", "-coverpkg " .. vim.fn.getcwd() .. "/..."} }) and while this trick works for the coverprofile output file, it does not work for coverpkg path, and i get an error: warning: no packages being tested depend on matches for pattern PATH_TO_MY_PROJECT.

If this wouldn't be to much trouble, I would like to see at least an option to enable/disable the cwd change. Thanks !

sergii4 commented 1 year ago

Hi @ddomurad,

Thank for your feedback and kicking off that discussion! Let me start with neotest itself. In Usage section it stated that have two option:

I assume it's understandable that the adapter has to comply with plugin purpose. As you know Go runs test per package not per file:

go test [build/test flags] [packages] [build/test flags & test binary flags]

So the less atomic structure for us to run test is a package and to execute nearest file/test we have do it within a package. Otherwise you bump into issue #38. It makes neotest-go ineffective tools in terms of big commercial projects and day to day use.

With all stated above running within package directory seems as expected behaviour and shouldn't run from nvim cwd by default.

Test Coverage is a nice-to-have feature but it's outside of neotest plugin scope.

When it comes to generating test coverage it could be easily mapped in your vim setting I assume. You don't need a separate plugin for it. The plugin is about running tests, highlighting results and navigating between it. And all it in scope of nearest test/file.

Hope it helps. I am open for further discussion, please share your thoughts.

ddomurad commented 1 year ago

@sergii4 Thank you for the detailed answer. I might wrongly assumed how everyone is or should use the plugin.

However for my needs the directory change is a problem. If you think that an option for controlling the current working directory would be beneficial to the plugin, I could prepare a PR, that could start further discussion on how this could be achieved. On the other hand, If you don't think this should be added, then I will just fork this repo and have it changed there. Let me know :)

Regarding test coverage. I really like how it integrates with neotest-go. I have mapped <leader>tr to run the nearest test, and <leader>rT to run the nearest test with code coverage only for that one test, this allows me to quickly skim what part of my function/package is "touched" by a test. I have similar binding for file testing .

sergii4 commented 1 year ago

Hi @ddomurad,

I am glad we have some common ground 🙂

But I still don't understand why do you need test coverage outside the test directory:

Specifically my problem is with generating coverage for modules outside the test directory.

I am not sure I fully understand your problem. You want always run go test for the whole project and generate coverage for it?

Is plugin in current state able to generate coverage for the package? Or it isn't what you want?

ddomurad commented 1 year ago

Let me explain with an arbitrary example: Assuming we have a directory structure like this:

pkga/
     foo.go
     foo_test.go
pkgb/
    bar.go
go.mod
go.sum
main.go

Nvim cwd is at the project root.

And:

// foo.go
package pkga

import (
  "fmt"
  "test/pkgb"
)

func Foo() {
  fmt.Println("Look at me! I'm a Foo!")
  pkgb.Bar()
}

// bar.go
package pkgb

import "fmt"

func Bar() {
  fmt.Println("Look at me! I'm a bar!")
}

// foo_test.go
package pkga

import "testing"

func Test_Foo(t *testing.T) {
  Foo()
}

My problem is with generating coverage for pkgb. I have tried to generate the coverage with something like this: require("neotest").run.run { extra_args = { "-coverprofile=./coverage.out", "-coverpkg ./..." } } However this generates a ./coverage.out file inside ./pkga/ folder, and the coverage is generated only for ./pkga/... . I would like to have the ./coverage.out in the root of my project (to easy integrate with another plugin), and ofc. I would like to have the coverage being generated for the whole project.

Now, I was able to overcome the first problem, by providing an absolute path to the coverprofile option. However when I try to do this for coverpkg it fails with the error warning: no packages being tested depend on matches for pattern... .

BTW. This works totally fine when I revert the mentioned changes in my local repo. What is working for me is basically equivalent to running go test -coverprofile=./coverage.out -coverpkg ./... -v -json -run ^Test_Foo$ ./pkga from the project root.

I see now that I have wrongly used the word module. This Specifically my problem is with generating coverage for modules outside the test directory. should read Specifically my problem is with generating coverage for packages outside the test package.

Maybe there is an easy solution to this, that I don't see.

Thanks and sorry for the confusion!

fredrikaverpil commented 1 year ago

I noticed this today too. I'm using andythigpen/nvim-coverage to load the coverage into Neovim and this plugin expects one file (in the cwd).

I've resorted to pinning to the previous commit of neotest-go so to keep coverage working as expected.

ddomurad commented 1 year ago

@sergii4 Do you have any plans regarding this issue ? If you don't have time to look into it, I could prepare a PR with a potential solution that would make possible to configure the behavior of the working dir. Pls. Let me know. :)

sergii4 commented 1 year ago

@ddomurad sorry for a long reply. Please prepare a PR and we can have a look together.

folliehiyuki commented 11 months ago

Hi, @ddomurad! Are you still interested in submitting the PR?

fredrikaverpil commented 8 months ago

I solved this on my end (I think), by customizing my lua setup instead of expecting this to be supported by neotest-go.

My workflow looks like this:

  1. Run tests with neotest-go in any way (e.g. nearest test, all tests etc... it doesn't matter).
  2. Using andythigpen/nvim-coverage: a. Run <CMD>Coverage<CR> to show the coverage in the gutter. b. Run <CMD>CoverageLoad<CR><CMD>CoverageSummary<CR> to show a summary of the coverage.

By telling both neotest-go and andythigpen/nvim-coverage to write/read the coverage.out file from the current working dir (where nvim was started from), it seems to now consistently work as desired:

  {
    "nvim-neotest/neotest",

    dependencies = {
      { "nvim-lua/plenary.nvim" },
      { "nvim-treesitter/nvim-treesitter" },
      { "antoinemadec/FixCursorHold.nvim" },
      { "folke/neodev.nvim" },

      -- adapters
      { "nvim-neotest/neotest-go" }
    },

    opts = {
      adapters = {
        ["neotest-go"] = {
          args = { "-coverprofile=" .. vim.fn.getcwd() .. "/coverage.out" },
        },
      },
    },
  },
  {
    "andythigpen/nvim-coverage",
    dependencies = { "nvim-lua/plenary.nvim" },
    keys = {
      { "<leader>tc", "<cmd>Coverage<cr>", desc = "Coverage in gutter" },
      { "<leader>tC", "<cmd>CoverageLoad<cr><cmd>CoverageSummary<cr>", desc = "Coverage summary" },
    },
    opts = {
      auto_reload = true,
      lang = {
        go = {
          coverage_file = vim.fn.getcwd() .. "/coverage.out",
        },
      },
    },
  },

@sergii4 what do you make of all this? Do you think this is the best solution, or would it make more sense to try and introduce a new neotest-go option to do this?