Skip to content

coverage: Don't panic if flag.CommandLine is reassigned#4384

Merged
fmeum merged 2 commits intobazel-contrib:masterfrom
abhinav:abg/cover-no-panic
Jun 27, 2025
Merged

coverage: Don't panic if flag.CommandLine is reassigned#4384
fmeum merged 2 commits intobazel-contrib:masterfrom
abhinav:abg/cover-no-panic

Conversation

@abhinav
Copy link
Copy Markdown
Contributor

@abhinav abhinav commented Jun 26, 2025

What type of PR is this?

Bug fix

What does this PR do? Why is it needed?

When run with go test -cover, the following test does not panic:

-- foo.go --
package foo

func foo() int {
	return 42
}
-- foo_test.go --
package foo

import (
	"flag"
	"testing"
)

func TestReassign(t *testing.T) {
	flag.CommandLine = flag.NewFlagSet("test", flag.ExitOnError)
	_ = foo()
}

However, with bazel coverage, it does panic:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x20 pc=0x100f8dbe4]

goroutine 1 [running]:
github.com/bazelbuild/rules_go/go/tools/bzltestutil.ConvertCoverToLcov()
        external/io_bazel_rules_go/go/tools/bzltestutil/lcov.go:39 +0x54
github.com/bazelbuild/rules_go/go/tools/bzltestutil.lcovAtExitHook()
        external/io_bazel_rules_go/go/tools/bzltestutil/lcov.go:181 +0x1c
github.com/bazelbuild/rules_go/go/tools/bzltestutil.LcovTestDeps.SetPanicOnExit0({{}, 0x18?}, 0x4b?)
        external/io_bazel_rules_go/go/tools/bzltestutil/lcov.go:175 +0x24
testing.(*M).after(0x1400007c280?)
        GOROOT/src/testing/testing.go:2374 +0x74
testing.(*M).Run(0x1400007c280)
        GOROOT/src/testing/testing.go:2185 +0x940
main.main()
        bazel-out/darwin_arm64-fastbuild/bin/go_default_test_/testmain.go:157 +0x7a4

The cause is that the test reassigned flag.CommandLine
and did not restore the original value.
While that's bad behavior, go test does not explode,
so neither should bazel coverage.

This change fixes the issue by grabbing a reference to
flag.CommandLine before the test runs.

Which issues(s) does this PR fix?

Fixes #4383

When run with `go test -cvoer`, the following test does not panic:

```
-- foo.go --
package foo

func foo() int {
	return 42
}
-- foo_test.go --
package foo

import (
	"flag"
	"testing"
)

func TestReassign(t *testing.T) {
	flag.CommandLine = flag.NewFlagSet("test", flag.ExitOnError)
	_ = foo()
}
```

However, with `bazel coverage`, it does panic:

```
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x20 pc=0x100f8dbe4]

goroutine 1 [running]:
github.com/bazelbuild/rules_go/go/tools/bzltestutil.ConvertCoverToLcov()
        external/io_bazel_rules_go/go/tools/bzltestutil/lcov.go:39 +0x54
github.com/bazelbuild/rules_go/go/tools/bzltestutil.lcovAtExitHook()
        external/io_bazel_rules_go/go/tools/bzltestutil/lcov.go:181 +0x1c
github.com/bazelbuild/rules_go/go/tools/bzltestutil.LcovTestDeps.SetPanicOnExit0({{}, 0x18?}, 0x4b?)
        external/io_bazel_rules_go/go/tools/bzltestutil/lcov.go:175 +0x24
testing.(*M).after(0x1400007c280?)
        GOROOT/src/testing/testing.go:2374 +0x74
testing.(*M).Run(0x1400007c280)
        GOROOT/src/testing/testing.go:2185 +0x940
main.main()
        bazel-out/darwin_arm64-fastbuild/bin/go_default_test_/testmain.go:157 +0x7a4
```

The cause is that the test reassigned `flag.CommandLine`
and did not restore the original value.
While that's bad behavior, `go test` does not explode,
so neither should `bazel coverage`.

This change fixes the issue by grabbing a reference to
`flag.CommandLine` before the test runs.
@abhinav
Copy link
Copy Markdown
Contributor Author

abhinav commented Jun 26, 2025

Windows test failed. Is there any way to access the generated artifact?

@fmeum
Copy link
Copy Markdown
Member

fmeum commented Jun 26, 2025

Windows test failed. Is there any way to access the generated artifact?

bazel coverage doesn't work on Windows, so the bazelci config for Windows needs to explicitly exclude the new test. You can search for some other coverage test's name to find the locations you need to update.

@fmeum fmeum merged commit bf5bfda into bazel-contrib:master Jun 27, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bazel coverage panics if flag.CommandLine is reassigned

2 participants