Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Code Coverage for multiple packages #816

Closed
ajackson-cpi opened this issue Feb 23, 2017 · 12 comments
Closed

Code Coverage for multiple packages #816

ajackson-cpi opened this issue Feb 23, 2017 · 12 comments

Comments

@ajackson-cpi
Copy link

'go test cover' doesn't consider coverage via other packages' tests.
Please make it possible to use coverage tools that do, like:
https://github.com/haya14busa/goverage

@mattetti
Copy link
Contributor

mattetti commented Mar 1, 2017

I would personally recommend to keep it simple for now. Also, the go recommended way is to have the tests with the code, what are the scenarios where you'd write tests in other packages that the original one?

@ajackson-cpi
Copy link
Author

I'm building a simple REST server that reads from a database. Instead of reinventing DB processing for each call, most of the logic is in a "db-helpers" package.

To test both packages, I would need to copy-paste tests to both.
It would be far cleaner if the API tests could indicate that they tested the db-helpers package.

@gaffney
Copy link

gaffney commented Apr 3, 2017

Agreed, this is annoying... it has resulted in writing tests for the sake of coverage even though in reality the code in question is indirectly covered all over the place. Really slows down productivity and is an obstacle towards refactoring when you know you not only have to extract out all the common code, but you have to pull out all the common tests as well.

@ramya-rao-a
Copy link
Contributor

ramya-rao-a commented Jan 9, 2018

We could add support for https://github.com/haya14busa/goverage, but keep it behind a setting, so that this a feature that users opt in to and is not the default one.

PRs are welcome

@ramya-rao-a ramya-rao-a changed the title Code Coverage Code Coverage for multiple packages Jan 9, 2018
@AlekSi
Copy link
Contributor

AlekSi commented May 11, 2018

Go 1.10 supports go test -cover for multiple packages: https://golang.org/doc/go1.10#test Extending existing code to pass -coverpkg=all should be easy, no external tool is needed now.

@ramya-rao-a
Copy link
Contributor

@mattetti, @AlekSi Could either of you try passing the -coverpkg=all flag via the go.testflags setting and share if any extra work is needed from the extension to achieve this?

@jirfag
Copy link

jirfag commented Oct 27, 2019

@ramya-rao-a I've added "go.testFlags": ["-coverpkg=all"], to settings, tests are running with this flag, but coverage is shown only in the tested package.

@ShoshinNikita
Copy link

@ramya-rao-a I've added "go.testFlags": ["-coverpkg=all"], to settings, tests are running with this flag, but coverage is shown only in the tested package.

I found the reason why coverage isn't shown. The bug is in applyCodeCoverageToAllEditors function in src/goCovers.ts file

Setup

Let's use this package structure (package is github.com/user/repo, root is /home/usr/go/src/github.com/user/repo)

|-- main.go
|-- first
    |-- first.go
|--second
    |-- second.go
    |-- second_test.go

Package first contains a function which is called in package second. When we run tests for package second (with -coverpkg=github.com/user/repo/...), we get the following cover profile:

mode: set
github.com/user/repo/first/first.go:5.16,7.2 1 1
github.com/user/repo/second/second.go:7.23,9.2 1 1

Bug

export function applyCodeCoverageToAllEditors(coverProfilePath: string, packageDirPath: string): Promise<void> {
  return new Promise((resolve, reject) => {
    try {

      // ...

      lines.on('line', function(data: string) {
          // go test coverageprofile generates output:
          //    filename:StartLine.StartColumn,EndLine.EndColumn Hits CoverCount
          // The first line will be "mode: set" which will be ignored
          const fileRange = data.match(/([^:]+)\:([\d]+)\.([\d]+)\,([\d]+)\.([\d]+)\s([\d]+)\s([\d]+)/);
          if (!fileRange) {
            return;
          }

          // => HERE
          const filePath = path.join(packageDirPath, path.basename(fileRange[1]));

          // ...
      }
    }
  }
}

Tests were run github.com/user/repo/second. So, packageDirPath is /home/usr/go/src/github.com/user/repo/second.

Let's examine processing of the cover profile file:

  • First line (github.com/user/repo/first/first.go:5.16,7.2 1 1)

    • packageDirPath is /home/usr/go/src/github.com/user/repo/second
    • fileRange[1] is github.com/user/repo/first/first.go
    • path.basename(fileRange[1]) is first.go
    • filePath is /home/usr/go/src/github.com/user/repo/second/first.go

    There's no such file ❌ :(

  • Second line (github.com/user/repo/second/second.go:7.23,9.2 1 1)

    • packageDirPath is /home/usr/go/src/github.com/user/repo/second
    • fileRange[1] is github.com/user/repo/second/second.go
    • path.basename(fileRange[1]) is second.go
    • filePath is /home/usr/go/src/github.com/user/repo/second/second.go

    There's such file ✔️

Solution

I think, the best solution is to pass the path of a project root instead of the path of tested package: /home/usr/go/src/github.com/user/repo instead of /home/usr/go/src/github.com/user/repo/second.

Then we should trim module path (github.com/user/repo, example.com/package and etc) from fileRange[1] instead of using path.basename() function.

Updated processing of the cover profile file:

  • First line (github.com/user/repo/first/first.go:5.16,7.2 1 1)

    • packageRoot is /home/usr/go/src/github.com/user/repo
    • fileRange[1] is github.com/user/repo/first/first.go
    • fileRange[1].TrimPrefix(modulePath) is /first/first.go
    • filePath is /home/usr/go/src/github.com/user/repo/first/first.go

    There's such file ✔️ :)

  • Second line (github.com/user/repo/second/second.go:7.23,9.2 1 1)

    • packageDirPath is /home/usr/go/src/github.com/user/repo
    • fileRange[1] is github.com/user/repo/second/second.go
    • fileRange[1].TrimPrefix(modulePath) is /second/second.go
    • filePath is /home/usr/go/src/github.com/user/repo/second/second.go

    There's such file ✔️


I tried to realize this, but I am not sure how to define Module Path (and I couldn't find any function for that). Also because of lack of experience with working on VS Code Extensions I am not sure how to pass project root (can we just use workspace root?)

I hope my little investigation will help to fix this bug :)

@ShoshinNikita
Copy link

@ramya-rao-a hello. What do you think about this "investigation"? I can fix this bug myself, but I have some questions:

I tried to realize this, but I am not sure how to define Module Path (and I couldn't find any function for that). Also because of lack of experience with working on VS Code Extensions I am not sure how to pass project root (can we just use workspace root?)

Also I would like to hear someone else's thoughts about the solution I suggested.

@ghost
Copy link

ghost commented Dec 27, 2019

I believe that for go the workspace root makes more sense. Looking at the Standard Go Project Layout suggests that projects are organised this way. But that is just my opinion.

@ramya-rao-a
Copy link
Contributor

Your investigation is right on the spot @ajackson-cpi
The problem lies in converting the relative paths in the cover profile to absolute paths that the editor can understand.

Also because of lack of experience with working on VS Code Extensions I am not sure how to pass project root (can we just use workspace root?)

You can use the utility function getWorkspaceFolderPath() to get the workspace root.

I tried to realize this, but I am not sure how to define Module Path (and I couldn't find any function for that)

An option that applies only if you are using GOPATH and not modules is to use the getCurrentGoWorkspaceFromGOPATH(). One can join this with the the relative path in the cover profile to get the absolute path.

@ramya-rao-a
Copy link
Contributor

ramya-rao-a commented May 28, 2020

Hello all,

We are in the midst of a repo move, see We are moving section in our readme for more details.

Closing this issue in favor of golang/vscode-go#108. Please subscribe to it for further updates.

Thanks for all the support & Happy Coding!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants