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

race: make code coverage + data race detection default for tests #666

Closed
henvic opened this issue Sep 19, 2020 · 3 comments
Closed

race: make code coverage + data race detection default for tests #666

henvic opened this issue Sep 19, 2020 · 3 comments

Comments

@henvic
Copy link

henvic commented Sep 19, 2020

Is your feature request related to a problem? Please describe.
Most likely not, but perhaps I might've identified the opportunity to improve this due to a bug (unsure).

Describe the solution you'd like
I want developers to learn about running Go tests with code coverage and data race detector sooner. This means better libraries and programs will be written in the long run because developers will find issues earlier (i.e., understanding when and how to use mutex).

A simple solution might be just to change the default -covermode from 'set' to 'atomic'.

Describe alternatives you've considered
Given that such a change makes running tests a magnitude slower, it might be seen as a bad thing. An extra button close to "run tests | debug tests" on top of the testing functions (when editing test files) might be to add something like a "run test with code coverage and -race" option there too.

On the Go extension settings panel, we might be able to choose to use it by default. In this case, the options might end up as something like this:

run tests | debug tests | fast

Where fast would be the same as executing a vanilla go test -run=TestFoo$.

Additional context
I only noticed this issue after updating go to 1.15.2 and VS Code to 1.49.1 + latest Go extension as of today. Previously, everything seemed to be working smoothly. I haven't investigated how or why.

  • By everything I mean data race detection + code coverage.
@hyangah
Copy link
Contributor

hyangah commented Sep 19, 2020

@henvic thanks for the feature request + bug report.
@pjweinbgo I think "go.coverMode" should default to an empty so go test should find the right mode.

@hyangah hyangah added this to the v0.17.1 milestone Sep 19, 2020
@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/256117 mentions this issue: package.json: default 'go.coverMode' to be empty

@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/257602 mentions this issue: [release] package.json: default 'go.coverMode' to be 'default'

gopherbot pushed a commit that referenced this issue Sep 25, 2020
According to `go help testflag`, the actual default of `-covermode`
when it's not explicitly set is affected by the `-race` flag.

        -covermode set,count,atomic
            Set the mode for coverage analysis for the package[s]
            being tested. The default is "set" unless -race is enabled,
            in which case it is "atomic".

Introduce 'default' as the default option for 'go.coverMode' to indicate
the covermode is unspecified.

Fixes #666

Change-Id: I7c7059140947639e3fd0438e1bc6cc50c712ebbb
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/256117
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Trust: Peter Weinberger <pjw@google.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Peter Weinberger <pjw@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
(cherry picked from commit 8d0bafa)
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/257602
@golang golang locked and limited conversation to collaborators Sep 25, 2021
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

3 participants