Repository navigation
coverage: Don't panic if flag.CommandLine is reassigned - #4384
Merged
Merged
Conversation
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.
Contributor
Author
|
Windows test failed. Is there any way to access the generated artifact? |
Member
|
gh-worker-dd-mergequeue-cf854d Bot
pushed a commit
to DataDog/datadog-agent
that referenced
this pull request
Aug 31, 2026
### What does this PR do? This PR ports #52373 onto the current `main` branch and makes Datadog the sole code-coverage backend. #### Remove the redundant Codecov integration **Before:** CI uploaded coverage to both Codecov and Datadog. This required a Codecov secret, configuration, helper code, tests, and documentation badges. **Now:** Codecov configuration, credentials, upload steps, helpers, tests, and badges are removed. Datadog Code Coverage remains the supported backend. **Impact:** There is one less external integration and secret to maintain, without losing coverage reporting. #### Separate coverage-cache management from report submission **Before:** S3 cache management was coupled to the coverage upload step. Reusing that step for additional jobs would also make them pull or push the shared cache, even when they already generated complete reports. **Now:** CI has separate operations for: - Managing the S3 coverage cache. - Uploading a report to Datadog. Impacted-package jobs run both operations. Flavor and Bazel jobs upload their reports without modifying the shared cache. **Impact:** Fast test jobs still merge cached coverage from unaffected packages, while additional uploaders cannot overwrite or race on cache entries they do not own. #### Submit previously unreported Linux flavor coverage The following jobs already generated `coverage.out`, but only uploaded JUnit results and retained coverage as a CI artifact: - `tests_flavor_iot_linux-x64` - `tests_flavor_dogstatsd_linux-x64` - `tests_flavor_heroku_linux-x64` They now upload their coverage reports to Datadog. **Impact:** Coverage from flavor-specific code paths is now included in the aggregate coverage view instead of remaining only in GitLab artifacts. A shared `.linux_flavor_tests` template also removes duplicated job configuration. #### Submit Bazel coverage reports The following jobs generated named coverage artifacts but did not submit them to any backend: - `bazel:coverage:linux-amd64` - `bazel:coverage:linux-arm64` - `bazel:coverage:macos-amd64` - `bazel:coverage:macos-arm64` They now pass their `coverage-bazel-*.out` reports to the shared Datadog upload step. **Impact:** Bazel coverage is visible in Datadog and can be compared with the legacy gotestsum coverage while the repository transitions toward Bazel-based testing. #### Fix the GPU coverage submission **Before:** `tests_gpu` attempted to upload coverage, but `gocovmerge` produced a zero-byte profile. Datadog rejected it because a Go coverprofile must begin with a `mode:` header. The upload was non-fatal, so the job remained green even though no report was accepted. **Now:** If a merged profile is empty, the coverage helper adds the configured Go coverage mode header. Non-empty profiles are left unchanged. **Impact:** The GPU job now submits a syntactically valid report and is recorded by Datadog instead of failing silently. #### Fix Windows coverage submission **Before:** `tests_windows-x64` generated coverage and merged the S3 cache correctly, but its upload attempted to execute `datadog-ci.exe`, which was unavailable in the build image. Direct PowerShell resolution of `datadog-ci` also failed, while the job remained green because uploads are non-fatal. **Now:** A dedicated Invoke task resolves the platform-specific `datadog-ci` command with `shutil.which` and executes it through Python. This follows the same command-resolution pattern already used successfully by Windows JUnit uploads. **Impact:** Windows coverage now reaches Datadog and contributes to the same commit-level aggregate as Linux and macOS coverage. #### Preserve E2E coverage without Codecov coupling **Before:** E2E profile conversion also read Codecov-specific metadata and uploaded to Codecov before Datadog processed the converted profiles. **Now:** E2E coverage folders are converted directly into Go coverprofiles for Datadog. Conversion no longer depends on Codecov metadata. **Impact:** Existing E2E coverage remains available while the conversion pipeline is simpler and backend-independent. ### Motivation The repository already used Datadog Code Coverage, so maintaining Codecov in parallel added secrets, configuration, and CI work without providing a necessary capability. At the same time, several jobs generated useful reports that Datadog never received, and the GPU and Windows upload paths failed silently. This change removes the redundant integration while making the Datadog submission path complete and verifiable. ### Describe how you validated your changes - `dda inv invoke-unit-tests.run --tests=coverage,go --directory=tasks/unit_tests --no-buffer --verbosity=1` - `dda inv linter.releasenote-unique-ids` - `dda inv linter.full-gitlab-ci -t main --pre-push-linters --fail-fast --verbosity=2` - The [pipeline](https://gitlab.ddbuild.io/DataDog/datadog-agent/-/pipelines/132663591) passed. - Datadog [confirmed](https://app.datadoghq.com/ci/code-coverage/github.com%2Fdatadog%2Fdatadog-agent/pull-requests/55278) all 11 expected non-E2E reports were ingested. ### Additional Notes Windows Bazel jobs remain test-only because `rules_go 0.63.0` [does not](bazel-contrib/rules_go#4384 (comment)) reliably support Bazel coverage on Windows. `tests_windows-x64` remains the supported Windows coverage producer. Co-authored-by: nikita-tkachenko-datadog <nikita.tkachenko@datadoghq.com> Co-authored-by: ofek.lev <ofek.lev@datadoghq.com>
github-actions Bot
pushed a commit
to DataDog/datadog-agent
that referenced
this pull request
Aug 31, 2026
### What does this PR do? This PR ports #52373 onto the current `main` branch and makes Datadog the sole code-coverage backend. #### Remove the redundant Codecov integration **Before:** CI uploaded coverage to both Codecov and Datadog. This required a Codecov secret, configuration, helper code, tests, and documentation badges. **Now:** Codecov configuration, credentials, upload steps, helpers, tests, and badges are removed. Datadog Code Coverage remains the supported backend. **Impact:** There is one less external integration and secret to maintain, without losing coverage reporting. #### Separate coverage-cache management from report submission **Before:** S3 cache management was coupled to the coverage upload step. Reusing that step for additional jobs would also make them pull or push the shared cache, even when they already generated complete reports. **Now:** CI has separate operations for: - Managing the S3 coverage cache. - Uploading a report to Datadog. Impacted-package jobs run both operations. Flavor and Bazel jobs upload their reports without modifying the shared cache. **Impact:** Fast test jobs still merge cached coverage from unaffected packages, while additional uploaders cannot overwrite or race on cache entries they do not own. #### Submit previously unreported Linux flavor coverage The following jobs already generated `coverage.out`, but only uploaded JUnit results and retained coverage as a CI artifact: - `tests_flavor_iot_linux-x64` - `tests_flavor_dogstatsd_linux-x64` - `tests_flavor_heroku_linux-x64` They now upload their coverage reports to Datadog. **Impact:** Coverage from flavor-specific code paths is now included in the aggregate coverage view instead of remaining only in GitLab artifacts. A shared `.linux_flavor_tests` template also removes duplicated job configuration. #### Submit Bazel coverage reports The following jobs generated named coverage artifacts but did not submit them to any backend: - `bazel:coverage:linux-amd64` - `bazel:coverage:linux-arm64` - `bazel:coverage:macos-amd64` - `bazel:coverage:macos-arm64` They now pass their `coverage-bazel-*.out` reports to the shared Datadog upload step. **Impact:** Bazel coverage is visible in Datadog and can be compared with the legacy gotestsum coverage while the repository transitions toward Bazel-based testing. #### Fix the GPU coverage submission **Before:** `tests_gpu` attempted to upload coverage, but `gocovmerge` produced a zero-byte profile. Datadog rejected it because a Go coverprofile must begin with a `mode:` header. The upload was non-fatal, so the job remained green even though no report was accepted. **Now:** If a merged profile is empty, the coverage helper adds the configured Go coverage mode header. Non-empty profiles are left unchanged. **Impact:** The GPU job now submits a syntactically valid report and is recorded by Datadog instead of failing silently. #### Fix Windows coverage submission **Before:** `tests_windows-x64` generated coverage and merged the S3 cache correctly, but its upload attempted to execute `datadog-ci.exe`, which was unavailable in the build image. Direct PowerShell resolution of `datadog-ci` also failed, while the job remained green because uploads are non-fatal. **Now:** A dedicated Invoke task resolves the platform-specific `datadog-ci` command with `shutil.which` and executes it through Python. This follows the same command-resolution pattern already used successfully by Windows JUnit uploads. **Impact:** Windows coverage now reaches Datadog and contributes to the same commit-level aggregate as Linux and macOS coverage. #### Preserve E2E coverage without Codecov coupling **Before:** E2E profile conversion also read Codecov-specific metadata and uploaded to Codecov before Datadog processed the converted profiles. **Now:** E2E coverage folders are converted directly into Go coverprofiles for Datadog. Conversion no longer depends on Codecov metadata. **Impact:** Existing E2E coverage remains available while the conversion pipeline is simpler and backend-independent. ### Motivation The repository already used Datadog Code Coverage, so maintaining Codecov in parallel added secrets, configuration, and CI work without providing a necessary capability. At the same time, several jobs generated useful reports that Datadog never received, and the GPU and Windows upload paths failed silently. This change removes the redundant integration while making the Datadog submission path complete and verifiable. ### Describe how you validated your changes - `dda inv invoke-unit-tests.run --tests=coverage,go --directory=tasks/unit_tests --no-buffer --verbosity=1` - `dda inv linter.releasenote-unique-ids` - `dda inv linter.full-gitlab-ci -t main --pre-push-linters --fail-fast --verbosity=2` - The [pipeline](https://gitlab.ddbuild.io/DataDog/datadog-agent/-/pipelines/132663591) passed. - Datadog [confirmed](https://app.datadoghq.com/ci/code-coverage/github.com%2Fdatadog%2Fdatadog-agent/pull-requests/55278) all 11 expected non-E2E reports were ingested. ### Additional Notes Windows Bazel jobs remain test-only because `rules_go 0.63.0` [does not](bazel-contrib/rules_go#4384 (comment)) reliably support Bazel coverage on Windows. `tests_windows-x64` remains the supported Windows coverage producer. Co-authored-by: nikita-tkachenko-datadog <nikita.tkachenko@datadoghq.com> Co-authored-by: ofek.lev <ofek.lev@datadoghq.com> 6aeb8ae
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:However, with
bazel coverage, it does panic:The cause is that the test reassigned
flag.CommandLineand did not restore the original value.
While that's bad behavior,
go testdoes not explode,so neither should
bazel coverage.This change fixes the issue by grabbing a reference to
flag.CommandLinebefore the test runs.Which issues(s) does this PR fix?
Fixes #4383