From d2983f56260eb1b368ef4e251617c3df768a4590 Mon Sep 17 00:00:00 2001 From: Kartik Joshi Date: Thu, 22 Jan 2026 11:28:21 +0000 Subject: [PATCH 1/3] Add Go coverage reporting to CI (end-to-end) This PR adds automated Go coverage reporting to the CI pipeline. Unit and integration tests now generate coverage profiles (one per integration matrix suite). It also collects coverage for the Dalec BuildKit frontend by building it with coverage enabled and exporting its covdata during solves. A new coverage-report job downloads all profiles, merges them, and uploads a combined report (including an index.html coverage page) as a workflow artifact, plus a summary in the Actions run. Fixes: #889 Signed-off-by: Kartik Joshi --- .github/workflows/ci.yml | 154 +++++++++++++++++++++++++++++- Dockerfile | 7 +- cmd/frontend/coverage.go | 113 ++++++++++++++++++++++ cmd/frontend/main.go | 4 +- test/testenv/build.go | 9 +- test/testenv/buildx.go | 24 ++++- test/testenv/frontend_coverage.go | 99 +++++++++++++++++++ 7 files changed, 401 insertions(+), 9 deletions(-) create mode 100644 cmd/frontend/coverage.go create mode 100644 test/testenv/frontend_coverage.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9da214d13..52b15d8f6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -270,6 +270,7 @@ jobs: set -eu docker buildx bake frontend + if [ "${TEST_SUITE}" = "other" ]; then exit 0 fi @@ -283,19 +284,64 @@ jobs: docker buildx bake worker env: TEST_SUITE: ${{ matrix.suite }} - - name: Run integration tests + - name: Run integration tests (with coverage tracking) run: | set -ex - if [ -n "${TEST_SUITE}" ] && [ ! "${TEST_SUITE}" = "other" ]; then + mkdir -p coverage + + # The frontend covdata files (covmeta/covcounters) are written by the test harness + # (writeFrontendCovdata) on the RUNNER filesystem. + export DALEC_FRONTEND_GOCOVERDIR="${GITHUB_WORKSPACE}/coverage/frontend-${TEST_SUITE}" + rm -rf "${DALEC_FRONTEND_GOCOVERDIR}" + mkdir -p "${DALEC_FRONTEND_GOCOVERDIR}" + + run="" + skip="" + if [ -n "${TEST_SUITE}" ] && [ "${TEST_SUITE}" != "other" ]; then run="-run=${TEST_SUITE}" fi if [ -n "${TEST_SKIP}" ]; then skip="-skip=${TEST_SKIP}" fi - go test -timeout=59m -v -json ${run} ${skip} ./test | go run ./cmd/test2json2gha --slow 120s --logdir /tmp/testlogs + + go test -timeout=59m -v -json \ + -covermode=atomic -coverpkg=./... \ + -coverprofile="coverage/integration-${TEST_SUITE}.out" \ + ${run} ${skip} ./test \ + | go run ./cmd/test2json2gha --slow 120s --logdir /tmp/testlogs + + # Convert frontend covdata -> legacy coverprofile + if ! ls "${DALEC_FRONTEND_GOCOVERDIR}"/covmeta.* >/dev/null 2>&1; then + echo "::group::frontend coverage debug" + echo "DALEC_FRONTEND_GOCOVERDIR=${DALEC_FRONTEND_GOCOVERDIR}" + echo "Contents:" + ls -la "${DALEC_FRONTEND_GOCOVERDIR}" || true + echo "Searching workspace for covmeta/covcounters..." + find "${GITHUB_WORKSPACE}" \( -name 'covmeta.*' -o -name 'covcounters.*' \) 2>/dev/null | head -n 200 || true + echo "::endgroup::" + echo "::error::No frontend coverage covmeta.* found in ${DALEC_FRONTEND_GOCOVERDIR} (frontend coverage not collected)" + exit 1 + fi + + go tool covdata textfmt \ + -i="${DALEC_FRONTEND_GOCOVERDIR}" \ + -o="coverage/frontend-${TEST_SUITE}.out" env: TEST_SUITE: ${{ matrix.suite }} TEST_SKIP: ${{ matrix.skip }} + + + - name: Upload integration coverage profile + if: always() + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 + with: + name: coverage-integration-${{ matrix.suite }} + path: | + coverage/integration-${{ matrix.suite }}.out + coverage/frontend-${{ matrix.suite }}.out + if-no-files-found: ignore + retention-days: 7 + - name: Get traces if: always() run: | @@ -354,8 +400,27 @@ jobs: cache: false - name: download deps run: go mod download - - name: Run unit tests - run: go test -v --test.short --json ./... | go run ./cmd/test2json2gha + - name: Run unit tests (with coverage tracking) + run: | + set -eux + mkdir -p coverage + + pkgs="$(go list ./... | grep -v '/test$' | grep -v '/test/' )" + go test -v --test.short --json \ + -covermode=atomic \ + -coverprofile="coverage/unit.out" \ + ${pkgs} \ + | go run ./cmd/test2json2gha + - name: Upload unit coverage profile + if: always() + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 + with: + name: coverage-unit + path: coverage/unit.out + if-no-files-found: ignore + retention-days: 7 + + e2e: runs-on: ubuntu-22.04 @@ -443,3 +508,82 @@ jobs: path: ${{ steps.dump-logs.outputs.DOCKERD_LOG_PATH }} retention-days: 1 + coverage-report: + runs-on: ubuntu-22.04 + needs: + - unit + - integration + + steps: + - name: Harden Runner + uses: step-security/harden-runner@e3f713f2d8f53843e71c69a996d56f51aa9adfb9 # v2.14.1 + with: + egress-policy: audit + + - name: Checkout + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + + - name: Setup Go + uses: actions/setup-go@7a3fe6cf4cb3a834922a1244abfce67bcef6a0c5 # v6.2.0 + with: + go-version: "1.25" + cache: false + + - name: Download deps + run: go mod download + + - name: Download unit coverage artifact + uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 + with: + name: coverage-unit + path: coverage + + - name: Download integration coverage artifacts + uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 + with: + path: coverage/_integration + + - name: Merge coverage + generate report + run: | + set -eux + go install github.com/wadey/gocovmerge@latest + + integration_profiles="$(find coverage/_integration -type f -name 'integration-*.out' | sort | tr '\n' ' ')" + frontend_profiles="$(find coverage/_integration -type f -name 'frontend-*.out' | sort | tr '\n' ' ')" + if [ -z "${integration_profiles}" ]; then + echo "::error::No integration coverage profiles found" + exit 1 + fi + + if [ -z "${frontend_profiles}" ]; then + echo "::error::No frontend coverage profiles found" + exit 1 + fi + + if [ ! -f coverage/unit.out ]; then + echo "::error::Unit coverage profile not found (coverage/unit.out)" + exit 1 + fi + + "$(go env GOPATH)/bin/gocovmerge" coverage/unit.out ${integration_profiles} ${frontend_profiles} > coverage/all.out + + go tool cover -func=coverage/all.out | tee coverage/summary.txt + go tool cover -html=coverage/all.out -o coverage/index.html + + total="$(tail -n 1 coverage/summary.txt | awk '{print $3}')" + { + echo "## Coverage" + echo + echo "- Total: **${total}**" + echo "- Profiles merged: $(echo "${integration_profiles}" | wc -w) integration + $(echo "${frontend_profiles}" | wc -w) frontend" + } >> "${GITHUB_STEP_SUMMARY}" + + - name: Upload merged coverage report + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 + with: + name: coverage-report + path: | + coverage/all.out + coverage/summary.txt + coverage/index.html + retention-days: 14 diff --git a/Dockerfile b/Dockerfile index b7b1a575d..3d3b6510d 100644 --- a/Dockerfile +++ b/Dockerfile @@ -5,11 +5,16 @@ WORKDIR /build COPY . . ENV CGO_ENABLED=0 ARG TARGETARCH TARGETOS GOFLAGS=-trimpath +ARG DALEC_FRONTEND_COVERAGE=0 ENV GOOS=${TARGETOS} GOARCH=${TARGETARCH} GOFLAGS=${GOFLAGS} RUN \ --mount=type=cache,target=/go/pkg/mod \ --mount=type=cache,target=/root/.cache/go-build \ - go build -o /frontend ./cmd/frontend + if [ "${DALEC_FRONTEND_COVERAGE}" = "1" ]; then \ + go build -cover -covermode=atomic -coverpkg=./... -o /frontend ./cmd/frontend ; \ + else \ + go build -o /frontend ./cmd/frontend ; \ + fi FROM scratch AS frontend COPY --from=frontend-build /frontend /frontend diff --git a/cmd/frontend/coverage.go b/cmd/frontend/coverage.go new file mode 100644 index 000000000..82befcdf8 --- /dev/null +++ b/cmd/frontend/coverage.go @@ -0,0 +1,113 @@ +package main + +import ( + "bytes" + "compress/gzip" + "context" + "errors" + "strings" + + "runtime/coverage" + + gwclient "github.com/moby/buildkit/frontend/gateway/client" +) + +const ( + frontendCoverageOptKey = "dalec.coverage" + frontendCovMetaKey = "dalec.coverage.frontend.meta.gz" + frontendCovCountersKey = "dalec.coverage.frontend.counters.gz" +) + +func isNoMetaErr(err error) bool { + if err == nil { + return false + } + // runtime/coverage: "no meta-data available (binary not built with -cover?)" + return strings.Contains(strings.ToLower(err.Error()), "no meta-data available") +} + +// Enabled per solve via SolveRequest.FrontendOpt["dalec.coverage"]="1" +func wantFrontendCoverage(c gwclient.Client) bool { + v, ok := c.BuildOpts().Opts[frontendCoverageOptKey] + if !ok { + return false + } + v = strings.ToLower(strings.TrimSpace(v)) + return v == "1" || v == "true" || v == "yes" || v == "on" +} + +func gzipBytes(in []byte) ([]byte, error) { + var buf bytes.Buffer + zw := gzip.NewWriter(&buf) + if _, err := zw.Write(in); err != nil { + _ = zw.Close() + return nil, err + } + if err := zw.Close(); err != nil { + return nil, err + } + return buf.Bytes(), nil +} + +func attachFrontendCoverage(c gwclient.Client, res *gwclient.Result) error { + if res == nil || !wantFrontendCoverage(c) { + return nil + } + if res.Metadata == nil { + res.Metadata = map[string][]byte{} + } + + var metaBuf, ctrBuf bytes.Buffer + + if err := coverage.WriteMeta(&metaBuf); err != nil { + if isNoMetaErr(err) { + return nil + } + return err + } + if err := coverage.WriteCounters(&ctrBuf); err != nil { + if isNoMetaErr(err) { + return nil + } + return err + } + + metaGz, err := gzipBytes(metaBuf.Bytes()) + if err != nil { + return err + } + ctrGz, err := gzipBytes(ctrBuf.Bytes()) + if err != nil { + return err + } + + res.Metadata[frontendCovMetaKey] = metaGz + res.Metadata[frontendCovCountersKey] = ctrGz + + // Avoid cross-solve accumulation if the frontend process is reused. + // Only works for binaries built with -cover (and typically atomic counters). + _ = coverage.ClearCounters() + + return nil +} + +func wrapWithCoverage(next gwclient.BuildFunc) gwclient.BuildFunc { + return func(ctx context.Context, c gwclient.Client) (*gwclient.Result, error) { + res, err := next(ctx, c) + + // If coverage is requested, make sure we have a result object to attach + // metadata to even on error paths. + if wantFrontendCoverage(c) && res == nil { + res = gwclient.NewResult() + } + + if covErr := attachFrontendCoverage(c, res); covErr != nil { + if err != nil { + return res, errors.Join(err, covErr) + } + return res, covErr + } + + return res, err + } +} diff --git a/cmd/frontend/main.go b/cmd/frontend/main.go index c8fc46dd9..4cf88c3ee 100644 --- a/cmd/frontend/main.go +++ b/cmd/frontend/main.go @@ -69,8 +69,10 @@ func dalecMain() { if err != nil { bklog.L.WithError(err).Fatal("error creating frontend router") } + handler := mux.Handler(frontend.WithTargetForwardingHandler) + handler = wrapWithCoverage(handler) - if err := grpcclient.RunFromEnvironment(ctx, mux.Handler(frontend.WithTargetForwardingHandler)); err != nil { + if err := grpcclient.RunFromEnvironment(ctx, handler); err != nil { bklog.L.WithError(err).Fatal("error running frontend") os.Exit(70) // 70 is EX_SOFTWARE, meaning internal software error occurred } diff --git a/test/testenv/build.go b/test/testenv/build.go index b423647ee..baa8e32fb 100644 --- a/test/testenv/build.go +++ b/test/testenv/build.go @@ -50,10 +50,17 @@ func buildBaseFrontend(ctx context.Context, c gwclient.Client) (*gwclient.Result return nil, errors.Wrap(err, "error marshaling Dockerfile context") } + // If the test runner requested frontend coverage, build the frontend binary + // with coverage instrumentation (Dockerfile uses DALEC_FRONTEND_COVERAGE). + frontendOpt := map[string]string{} + if os.Getenv("DALEC_FRONTEND_GOCOVERDIR") != "" { + frontendOpt["build-arg:DALEC_FRONTEND_COVERAGE"] = "1" + } + defPB := def.ToPB() return c.Solve(ctx, gwclient.SolveRequest{ Frontend: "dockerfile.v0", - FrontendOpt: map[string]string{}, + FrontendOpt: frontendOpt, FrontendInputs: map[string]*pb.Definition{ dockerui.DefaultLocalNameContext: defPB, dockerui.DefaultLocalNameDockerfile: dockerfileDef.ToPB(), diff --git a/test/testenv/buildx.go b/test/testenv/buildx.go index 95edf06d9..9fe80c437 100644 --- a/test/testenv/buildx.go +++ b/test/testenv/buildx.go @@ -10,6 +10,7 @@ import ( "net" "os" "os/exec" + "path/filepath" "strings" "sync" "testing" @@ -452,6 +453,7 @@ type clientForceDalecWithInput struct { } func (c *clientForceDalecWithInput) Solve(ctx context.Context, req gwclient.SolveRequest) (*gwclient.Result, error) { + covRoot := os.Getenv("DALEC_FRONTEND_GOCOVERDIR") if req.Definition == nil { // Only inject the frontend when there is no "definition" set. // If a definition is set, it is intended for this to go directly to the buildkit solver. @@ -459,7 +461,27 @@ func (c *clientForceDalecWithInput) Solve(ctx context.Context, req gwclient.Solv return nil, err } } - return c.Client.Solve(ctx, req) + + // IMPORTANT: set this *after* withDalecInput, since it may replace/normalize FrontendOpt. + if covRoot != "" { + if req.FrontendOpt == nil { + req.FrontendOpt = map[string]string{} + } + // Frontend-only toggle (NOT a dalec build arg) + req.FrontendOpt["dalec.coverage"] = "1" + } + res, err := c.Client.Solve(ctx, req) + + if covRoot != "" { + if covErr := writeFrontendCovdata(filepath.Clean(covRoot), res); covErr != nil { + if err != nil { + return res, errors.Join(err, covErr) + } + return res, covErr + } + } + + return res, err } // gwClientInputInject is a gwclient.Client that injects the result of a build func into the solve request as an input named by the id. diff --git a/test/testenv/frontend_coverage.go b/test/testenv/frontend_coverage.go new file mode 100644 index 000000000..94fe81238 --- /dev/null +++ b/test/testenv/frontend_coverage.go @@ -0,0 +1,99 @@ +package testenv + +import ( + "bytes" + "compress/gzip" + "crypto/rand" + "crypto/sha256" + "encoding/hex" + "fmt" + "io" + "os" + "path/filepath" + "time" + + gwclient "github.com/moby/buildkit/frontend/gateway/client" +) + +const ( + frontendCovMetaKey = "dalec.coverage.frontend.meta.gz" + frontendCovCountersKey = "dalec.coverage.frontend.counters.gz" +) + +func gunzip(b []byte) (out []byte, retErr error) { + zr, err := gzip.NewReader(bytes.NewReader(b)) + if err != nil { + return nil, err + } + defer func() { + if err := zr.Close(); retErr == nil && err != nil { + retErr = err + } + }() + + out, retErr = io.ReadAll(zr) + return out, retErr +} + +// Writes files compatible with `go tool covdata`: +// +// covmeta. +// covcounters.... +func writeFrontendCovdata(outDir string, res *gwclient.Result) error { + if outDir == "" || res == nil || res.Metadata == nil { + return nil + } + + metaGz := res.Metadata[frontendCovMetaKey] + ctrGz := res.Metadata[frontendCovCountersKey] + if metaGz == nil || ctrGz == nil { + // Not every Solve necessarily runs the dalec frontend; only write when present. + return nil + } + + meta, err := gunzip(metaGz) + if err != nil { + return err + } + counters, err := gunzip(ctrGz) + if err != nil { + return err + } + + sum := sha256.Sum256(meta) + hash := hex.EncodeToString(sum[:]) + + if err := os.MkdirAll(outDir, 0o755); err != nil { + return err + } + + // Write meta once (best-effort; safe under concurrency) + metaPath := filepath.Join(outDir, "covmeta."+hash) + if _, err := os.Stat(metaPath); os.IsNotExist(err) { + tmp, err := os.CreateTemp(outDir, "covmeta."+hash+".tmp-*") + if err != nil { + return err + } + if _, err := tmp.Write(meta); err != nil { + tmp.Close() + _ = os.Remove(tmp.Name()) + return err + } + if err := tmp.Close(); err != nil { + _ = os.Remove(tmp.Name()) + return err + } + // If rename fails because another goroutine already created it, ignore. + if err := os.Rename(tmp.Name(), metaPath); err != nil { + _ = os.Remove(tmp.Name()) + } + } + + var rb [4]byte + _, _ = rand.Read(rb[:]) + + pid := os.Getpid() + ts := time.Now().UnixNano() + ctrPath := filepath.Join(outDir, fmt.Sprintf("covcounters.%s.%d.%d.%x", hash, pid, ts, rb)) + return os.WriteFile(ctrPath, counters, 0o644) +} From 0f7b13d89d6edb6a168f267e4914387d6e70b25d Mon Sep 17 00:00:00 2001 From: Kartik Joshi Date: Wed, 22 Apr 2026 10:39:07 +0000 Subject: [PATCH 2/3] attach coverage to a gRPC error detail on failure Signed-off-by: Kartik Joshi --- cmd/frontend/coverage.go | 73 +++++----- cmd/frontend/coverage_test.go | 132 ++++++++++++++++++ internal/frontendcoverage/payload.go | 155 +++++++++++++++++++++ test/frontend_coverage_integration_test.go | 58 ++++++++ test/testenv/buildx.go | 5 +- test/testenv/frontend_coverage.go | 22 ++- test/testenv/frontend_coverage_test.go | 102 ++++++++++++++ 7 files changed, 496 insertions(+), 51 deletions(-) create mode 100644 cmd/frontend/coverage_test.go create mode 100644 internal/frontendcoverage/payload.go create mode 100644 test/frontend_coverage_integration_test.go create mode 100644 test/testenv/frontend_coverage_test.go diff --git a/cmd/frontend/coverage.go b/cmd/frontend/coverage.go index 82befcdf8..76cd664a1 100644 --- a/cmd/frontend/coverage.go +++ b/cmd/frontend/coverage.go @@ -10,12 +10,7 @@ import ( "runtime/coverage" gwclient "github.com/moby/buildkit/frontend/gateway/client" -) - -const ( - frontendCoverageOptKey = "dalec.coverage" - frontendCovMetaKey = "dalec.coverage.frontend.meta.gz" - frontendCovCountersKey = "dalec.coverage.frontend.counters.gz" + "github.com/project-dalec/dalec/internal/frontendcoverage" ) func isNoMetaErr(err error) bool { @@ -26,14 +21,9 @@ func isNoMetaErr(err error) bool { return strings.Contains(strings.ToLower(err.Error()), "no meta-data available") } -// Enabled per solve via SolveRequest.FrontendOpt["dalec.coverage"]="1" +// Enabled per solve via SolveRequest.FrontendOpt[frontendcoverage.OptKey]="1" func wantFrontendCoverage(c gwclient.Client) bool { - v, ok := c.BuildOpts().Opts[frontendCoverageOptKey] - if !ok { - return false - } - v = strings.ToLower(strings.TrimSpace(v)) - return v == "1" || v == "true" || v == "yes" || v == "on" + return frontendcoverage.Want(c.BuildOpts().Opts) } func gzipBytes(in []byte) ([]byte, error) { @@ -49,65 +39,74 @@ func gzipBytes(in []byte) ([]byte, error) { return buf.Bytes(), nil } -func attachFrontendCoverage(c gwclient.Client, res *gwclient.Result) error { - if res == nil || !wantFrontendCoverage(c) { - return nil - } - if res.Metadata == nil { - res.Metadata = map[string][]byte{} - } +var frontendCoverageCollector = collectFrontendCoveragePayload +func collectFrontendCoveragePayload() (*frontendcoverage.Payload, error) { var metaBuf, ctrBuf bytes.Buffer if err := coverage.WriteMeta(&metaBuf); err != nil { if isNoMetaErr(err) { - return nil + return nil, nil } - return err + return nil, err } if err := coverage.WriteCounters(&ctrBuf); err != nil { if isNoMetaErr(err) { - return nil + return nil, nil } - return err + return nil, err } metaGz, err := gzipBytes(metaBuf.Bytes()) if err != nil { - return err + return nil, err } ctrGz, err := gzipBytes(ctrBuf.Bytes()) if err != nil { - return err + return nil, err } - res.Metadata[frontendCovMetaKey] = metaGz - res.Metadata[frontendCovCountersKey] = ctrGz - // Avoid cross-solve accumulation if the frontend process is reused. // Only works for binaries built with -cover (and typically atomic counters). _ = coverage.ClearCounters() - return nil + return &frontendcoverage.Payload{ + MetaGz: metaGz, + CountersGz: ctrGz, + }, nil } func wrapWithCoverage(next gwclient.BuildFunc) gwclient.BuildFunc { return func(ctx context.Context, c gwclient.Client) (*gwclient.Result, error) { res, err := next(ctx, c) - - // If coverage is requested, make sure we have a result object to attach - // metadata to even on error paths. - if wantFrontendCoverage(c) && res == nil { - res = gwclient.NewResult() + if !wantFrontendCoverage(c) { + return res, err } - if covErr := attachFrontendCoverage(c, res); covErr != nil { + payload, covErr := frontendCoverageCollector() + if covErr != nil { if err != nil { return res, errors.Join(err, covErr) } return res, covErr } + if payload == nil { + return res, err + } + + if err != nil { + errWithCoverage, attachErr := payload.AttachToError(err) + if attachErr != nil { + return res, errors.Join(err, attachErr) + } + return res, errWithCoverage + } + + if res == nil { + res = gwclient.NewResult() + } + payload.AttachToResult(res) - return res, err + return res, nil } } diff --git a/cmd/frontend/coverage_test.go b/cmd/frontend/coverage_test.go new file mode 100644 index 000000000..9c98c9116 --- /dev/null +++ b/cmd/frontend/coverage_test.go @@ -0,0 +1,132 @@ +package main + +import ( + "bytes" + "context" + "errors" + "testing" + + "github.com/moby/buildkit/client/llb" + "github.com/moby/buildkit/client/llb/sourceresolver" + gwclient "github.com/moby/buildkit/frontend/gateway/client" + "github.com/moby/buildkit/solver/pb" + digest "github.com/opencontainers/go-digest" + "github.com/project-dalec/dalec/internal/frontendcoverage" +) + +func TestWrapWithCoverageAttachesResultMetadataOnSuccess(t *testing.T) { + t.Cleanup(setFrontendCoverageCollectorForTest(func() (*frontendcoverage.Payload, error) { + return &frontendcoverage.Payload{ + MetaGz: []byte("meta-gz"), + CountersGz: []byte("counters-gz"), + }, nil + })) + + res, err := wrapWithCoverage(func(context.Context, gwclient.Client) (*gwclient.Result, error) { + return nil, nil + })(context.Background(), &fakeGatewayClient{ + opts: map[string]string{frontendcoverage.OptKey: "1"}, + }) + if err != nil { + t.Fatalf("expected nil error, got %v", err) + } + if res == nil { + t.Fatal("expected result to be created when coverage is enabled") + } + + payload, payloadErr := frontendcoverage.PayloadFromSolve(res, nil) + if payloadErr != nil { + t.Fatalf("expected nil payload error, got %v", payloadErr) + } + if payload == nil { + t.Fatal("expected payload to be attached to result metadata") + } + if !bytes.Equal(payload.MetaGz, []byte("meta-gz")) { + t.Fatalf("unexpected meta payload: %q", payload.MetaGz) + } + if !bytes.Equal(payload.CountersGz, []byte("counters-gz")) { + t.Fatalf("unexpected counters payload: %q", payload.CountersGz) + } +} + +func TestWrapWithCoverageAttachesGRPCDetailOnError(t *testing.T) { + t.Cleanup(setFrontendCoverageCollectorForTest(func() (*frontendcoverage.Payload, error) { + return &frontendcoverage.Payload{ + MetaGz: []byte("meta-gz"), + CountersGz: []byte("counters-gz"), + }, nil + })) + + frontendErr := errors.New("frontend failed") + res, err := wrapWithCoverage(func(context.Context, gwclient.Client) (*gwclient.Result, error) { + return nil, frontendErr + })(context.Background(), &fakeGatewayClient{ + opts: map[string]string{frontendcoverage.OptKey: "1"}, + }) + if res != nil { + t.Fatal("expected nil result on error path") + } + if err == nil { + t.Fatal("expected error from wrapped frontend") + } + if !errors.Is(err, frontendErr) { + t.Fatalf("expected wrapped error to preserve original error, got %v", err) + } + if err.Error() != frontendErr.Error() { + t.Fatalf("expected wrapped error message %q, got %q", frontendErr.Error(), err.Error()) + } + + payload, payloadErr := frontendcoverage.PayloadFromError(err) + if payloadErr != nil { + t.Fatalf("expected nil payload error, got %v", payloadErr) + } + if payload == nil { + t.Fatal("expected payload to be attached to error details") + } + if !bytes.Equal(payload.MetaGz, []byte("meta-gz")) { + t.Fatalf("unexpected meta payload: %q", payload.MetaGz) + } + if !bytes.Equal(payload.CountersGz, []byte("counters-gz")) { + t.Fatalf("unexpected counters payload: %q", payload.CountersGz) + } +} + +func setFrontendCoverageCollectorForTest(f func() (*frontendcoverage.Payload, error)) func() { + previous := frontendCoverageCollector + frontendCoverageCollector = f + return func() { + frontendCoverageCollector = previous + } +} + +type fakeGatewayClient struct { + opts map[string]string +} + +func (c *fakeGatewayClient) Solve(context.Context, gwclient.SolveRequest) (*gwclient.Result, error) { + panic("unexpected call to Solve") +} + +func (c *fakeGatewayClient) ResolveImageConfig(context.Context, string, sourceresolver.Opt) (string, digest.Digest, []byte, error) { + panic("unexpected call to ResolveImageConfig") +} + +func (c *fakeGatewayClient) ResolveSourceMetadata(context.Context, *pb.SourceOp, sourceresolver.Opt) (*sourceresolver.MetaResponse, error) { + panic("unexpected call to ResolveSourceMetadata") +} + +func (c *fakeGatewayClient) BuildOpts() gwclient.BuildOpts { + return gwclient.BuildOpts{Opts: c.opts} +} + +func (c *fakeGatewayClient) Inputs(context.Context) (map[string]llb.State, error) { + panic("unexpected call to Inputs") +} + +func (c *fakeGatewayClient) NewContainer(context.Context, gwclient.NewContainerRequest) (gwclient.Container, error) { + panic("unexpected call to NewContainer") +} + +func (c *fakeGatewayClient) Warn(context.Context, digest.Digest, string, gwclient.WarnOpts) error { + panic("unexpected call to Warn") +} diff --git a/internal/frontendcoverage/payload.go b/internal/frontendcoverage/payload.go new file mode 100644 index 000000000..58316c33d --- /dev/null +++ b/internal/frontendcoverage/payload.go @@ -0,0 +1,155 @@ +package frontendcoverage + +import ( + "bytes" + "encoding/base64" + "strings" + + gwclient "github.com/moby/buildkit/frontend/gateway/client" + "google.golang.org/genproto/googleapis/rpc/errdetails" + "google.golang.org/grpc/status" +) + +const ( + OptKey = "dalec.coverage" + MetaKey = "dalec.coverage.frontend.meta.gz" + CountersKey = "dalec.coverage.frontend.counters.gz" + + errorInfoReason = "DALEC_FRONTEND_COVERAGE" + errorInfoDomain = "github.com/project-dalec/dalec" + + errorInfoMetaKey = "frontend_meta_gz" + errorInfoCountersKey = "frontend_counters_gz" +) + +type Payload struct { + MetaGz []byte + CountersGz []byte +} + +func Want(opts map[string]string) bool { + v, ok := opts[OptKey] + if !ok { + return false + } + + v = strings.ToLower(strings.TrimSpace(v)) + return v == "1" || v == "true" || v == "yes" || v == "on" +} + +func (p *Payload) empty() bool { + return p == nil || len(p.MetaGz) == 0 || len(p.CountersGz) == 0 +} + +func (p *Payload) AttachToResult(res *gwclient.Result) { + if p.empty() || res == nil { + return + } + if res.Metadata == nil { + res.Metadata = map[string][]byte{} + } + + res.Metadata[MetaKey] = bytes.Clone(p.MetaGz) + res.Metadata[CountersKey] = bytes.Clone(p.CountersGz) +} + +func PayloadFromResult(res *gwclient.Result) *Payload { + if res == nil || res.Metadata == nil { + return nil + } + + meta := res.Metadata[MetaKey] + counters := res.Metadata[CountersKey] + if len(meta) == 0 || len(counters) == 0 { + return nil + } + + return &Payload{ + MetaGz: bytes.Clone(meta), + CountersGz: bytes.Clone(counters), + } +} + +func (p *Payload) AttachToError(err error) (error, error) { + if err == nil || p.empty() { + return err, nil + } + + st, attachErr := status.Convert(err).WithDetails(&errdetails.ErrorInfo{ + Reason: errorInfoReason, + Domain: errorInfoDomain, + Metadata: map[string]string{ + errorInfoMetaKey: base64.StdEncoding.EncodeToString(p.MetaGz), + errorInfoCountersKey: base64.StdEncoding.EncodeToString(p.CountersGz), + }, + }) + if attachErr != nil { + return err, attachErr + } + + return &errorWithStatusDetail{err: err, st: st}, nil +} + +func PayloadFromError(err error) (*Payload, error) { + if err == nil { + return nil, nil + } + + st, ok := status.FromError(err) + if !ok { + return nil, nil + } + + for _, detail := range st.Details() { + info, ok := detail.(*errdetails.ErrorInfo) + if !ok || info.Reason != errorInfoReason || info.Domain != errorInfoDomain { + continue + } + + meta, err := base64.StdEncoding.DecodeString(info.Metadata[errorInfoMetaKey]) + if err != nil { + return nil, err + } + + counters, err := base64.StdEncoding.DecodeString(info.Metadata[errorInfoCountersKey]) + if err != nil { + return nil, err + } + + if len(meta) == 0 || len(counters) == 0 { + return nil, nil + } + + return &Payload{ + MetaGz: meta, + CountersGz: counters, + }, nil + } + + return nil, nil +} + +func PayloadFromSolve(res *gwclient.Result, err error) (*Payload, error) { + if payload := PayloadFromResult(res); payload != nil { + return payload, nil + } + + return PayloadFromError(err) +} + +type errorWithStatusDetail struct { + err error + st *status.Status +} + +func (e *errorWithStatusDetail) Error() string { + return e.err.Error() +} + +func (e *errorWithStatusDetail) Unwrap() error { + return e.err +} + +func (e *errorWithStatusDetail) GRPCStatus() *status.Status { + return e.st +} diff --git a/test/frontend_coverage_integration_test.go b/test/frontend_coverage_integration_test.go new file mode 100644 index 000000000..ef14ffa18 --- /dev/null +++ b/test/frontend_coverage_integration_test.go @@ -0,0 +1,58 @@ +package test + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + gwclient "github.com/moby/buildkit/frontend/gateway/client" + "github.com/project-dalec/dalec" +) + +func TestFrontendCoverageExportedOnSolveError(t *testing.T) { + ctx := startTestSpan(baseCtx, t) + covDir := t.TempDir() + t.Setenv("DALEC_FRONTEND_GOCOVERDIR", covDir) + + testEnv.RunTest(ctx, t, func(ctx context.Context, gwc gwclient.Client) { + req := newSolveRequest( + withSpec(ctx, t, &dalec.Spec{ + Name: "frontend-coverage-error", + Version: "0.0.1", + Revision: "1", + }), + withBuildTarget("does-not-exist"), + ) + + _, err := gwc.Solve(ctx, req) + const expect = "no such handler for target" + if err == nil || !strings.Contains(err.Error(), expect) { + t.Fatalf("expected error containing %q, got %v", expect, err) + } + + assertNonEmptyGlob(t, filepath.Join(covDir, "covmeta.*")) + assertNonEmptyGlob(t, filepath.Join(covDir, "covcounters.*")) + }) +} + +func assertNonEmptyGlob(t *testing.T, pattern string) { + t.Helper() + + matches, err := filepath.Glob(pattern) + if err != nil { + t.Fatalf("expected nil glob error for %q, got %v", pattern, err) + } + if len(matches) == 0 { + t.Fatalf("expected at least one file matching %q", pattern) + } + + info, err := os.Stat(matches[0]) + if err != nil { + t.Fatalf("expected stat to succeed for %q, got %v", matches[0], err) + } + if info.Size() == 0 { + t.Fatalf("expected %q to be non-empty", matches[0]) + } +} diff --git a/test/testenv/buildx.go b/test/testenv/buildx.go index 9fe80c437..4c4cfab7d 100644 --- a/test/testenv/buildx.go +++ b/test/testenv/buildx.go @@ -25,6 +25,7 @@ import ( "github.com/moby/buildkit/solver/pb" spb "github.com/moby/buildkit/sourcepolicy/pb" pkgerrors "github.com/pkg/errors" + "github.com/project-dalec/dalec/internal/frontendcoverage" "github.com/project-dalec/dalec/sessionutil/socketprovider" "google.golang.org/grpc" "google.golang.org/grpc/codes" @@ -468,12 +469,12 @@ func (c *clientForceDalecWithInput) Solve(ctx context.Context, req gwclient.Solv req.FrontendOpt = map[string]string{} } // Frontend-only toggle (NOT a dalec build arg) - req.FrontendOpt["dalec.coverage"] = "1" + req.FrontendOpt[frontendcoverage.OptKey] = "1" } res, err := c.Client.Solve(ctx, req) if covRoot != "" { - if covErr := writeFrontendCovdata(filepath.Clean(covRoot), res); covErr != nil { + if covErr := writeFrontendCovdata(filepath.Clean(covRoot), res, err); covErr != nil { if err != nil { return res, errors.Join(err, covErr) } diff --git a/test/testenv/frontend_coverage.go b/test/testenv/frontend_coverage.go index 94fe81238..7ba7054a8 100644 --- a/test/testenv/frontend_coverage.go +++ b/test/testenv/frontend_coverage.go @@ -13,11 +13,7 @@ import ( "time" gwclient "github.com/moby/buildkit/frontend/gateway/client" -) - -const ( - frontendCovMetaKey = "dalec.coverage.frontend.meta.gz" - frontendCovCountersKey = "dalec.coverage.frontend.counters.gz" + "github.com/project-dalec/dalec/internal/frontendcoverage" ) func gunzip(b []byte) (out []byte, retErr error) { @@ -39,23 +35,25 @@ func gunzip(b []byte) (out []byte, retErr error) { // // covmeta. // covcounters.... -func writeFrontendCovdata(outDir string, res *gwclient.Result) error { - if outDir == "" || res == nil || res.Metadata == nil { +func writeFrontendCovdata(outDir string, res *gwclient.Result, solveErr error) error { + if outDir == "" { return nil } - metaGz := res.Metadata[frontendCovMetaKey] - ctrGz := res.Metadata[frontendCovCountersKey] - if metaGz == nil || ctrGz == nil { + payload, err := frontendcoverage.PayloadFromSolve(res, solveErr) + if err != nil { + return err + } + if payload == nil { // Not every Solve necessarily runs the dalec frontend; only write when present. return nil } - meta, err := gunzip(metaGz) + meta, err := gunzip(payload.MetaGz) if err != nil { return err } - counters, err := gunzip(ctrGz) + counters, err := gunzip(payload.CountersGz) if err != nil { return err } diff --git a/test/testenv/frontend_coverage_test.go b/test/testenv/frontend_coverage_test.go new file mode 100644 index 000000000..52a01c66b --- /dev/null +++ b/test/testenv/frontend_coverage_test.go @@ -0,0 +1,102 @@ +package testenv + +import ( + "bytes" + "compress/gzip" + "crypto/sha256" + "encoding/hex" + "errors" + "os" + "path/filepath" + "testing" + + gwclient "github.com/moby/buildkit/frontend/gateway/client" + "github.com/project-dalec/dalec/internal/frontendcoverage" +) + +func TestWriteFrontendCovdata(t *testing.T) { + rawMeta := []byte("raw-meta") + rawCounters := []byte("raw-counters") + payload := &frontendcoverage.Payload{ + MetaGz: gzipBytesForTest(t, rawMeta), + CountersGz: gzipBytesForTest(t, rawCounters), + } + + testCases := []struct { + name string + setup func(t *testing.T) (*gwclient.Result, error) + }{ + { + name: "result metadata", + setup: func(t *testing.T) (*gwclient.Result, error) { + res := gwclient.NewResult() + payload.AttachToResult(res) + return res, nil + }, + }, + { + name: "grpc error detail", + setup: func(t *testing.T) (*gwclient.Result, error) { + errWithPayload, err := payload.AttachToError(errors.New("solve failed")) + if err != nil { + t.Fatalf("expected nil attach error, got %v", err) + } + return nil, errWithPayload + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + outDir := t.TempDir() + res, err := tc.setup(t) + + if writeErr := writeFrontendCovdata(outDir, res, err); writeErr != nil { + t.Fatalf("expected nil write error, got %v", writeErr) + } + + hash := sha256.Sum256(rawMeta) + hashHex := hex.EncodeToString(hash[:]) + + metaPath := filepath.Join(outDir, "covmeta."+hashHex) + gotMeta, readErr := os.ReadFile(metaPath) + if readErr != nil { + t.Fatalf("expected covmeta file, got %v", readErr) + } + if !bytes.Equal(gotMeta, rawMeta) { + t.Fatalf("unexpected covmeta contents: %q", gotMeta) + } + + counterFiles, globErr := filepath.Glob(filepath.Join(outDir, "covcounters."+hashHex+".*")) + if globErr != nil { + t.Fatalf("expected nil glob error, got %v", globErr) + } + if len(counterFiles) != 1 { + t.Fatalf("expected exactly one counter file, got %d", len(counterFiles)) + } + + gotCounters, readErr := os.ReadFile(counterFiles[0]) + if readErr != nil { + t.Fatalf("expected covcounters file, got %v", readErr) + } + if !bytes.Equal(gotCounters, rawCounters) { + t.Fatalf("unexpected covcounters contents: %q", gotCounters) + } + }) + } +} + +func gzipBytesForTest(t *testing.T, in []byte) []byte { + t.Helper() + + var buf bytes.Buffer + zw := gzip.NewWriter(&buf) + if _, err := zw.Write(in); err != nil { + t.Fatalf("expected nil gzip write error, got %v", err) + } + if err := zw.Close(); err != nil { + t.Fatalf("expected nil gzip close error, got %v", err) + } + + return buf.Bytes() +} From 1784c3f740f6a32f74c9cd1fa7dacb4fce5130e8 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Fri, 1 May 2026 16:47:01 -0700 Subject: [PATCH 3/3] test: fix frontend coverage covdata extraction Signed-off-by: Brian Goff --- internal/frontendcoverage/payload.go | 3 +- test/frontend_coverage_integration_test.go | 44 ++++++++++++++++++ test/testenv/frontend_coverage.go | 52 +++++++++++++++++----- test/testenv/frontend_coverage_test.go | 21 +++++++-- 4 files changed, 105 insertions(+), 15 deletions(-) diff --git a/internal/frontendcoverage/payload.go b/internal/frontendcoverage/payload.go index 58316c33d..ca267198b 100644 --- a/internal/frontendcoverage/payload.go +++ b/internal/frontendcoverage/payload.go @@ -6,6 +6,7 @@ import ( "strings" gwclient "github.com/moby/buildkit/frontend/gateway/client" + "github.com/moby/buildkit/util/grpcerrors" "google.golang.org/genproto/googleapis/rpc/errdetails" "google.golang.org/grpc/status" ) @@ -95,7 +96,7 @@ func PayloadFromError(err error) (*Payload, error) { return nil, nil } - st, ok := status.FromError(err) + st, ok := grpcerrors.AsGRPCStatus(err) if !ok { return nil, nil } diff --git a/test/frontend_coverage_integration_test.go b/test/frontend_coverage_integration_test.go index ef14ffa18..6a067c66c 100644 --- a/test/frontend_coverage_integration_test.go +++ b/test/frontend_coverage_integration_test.go @@ -1,9 +1,13 @@ package test import ( + "bufio" + "bytes" "context" "os" + "os/exec" "path/filepath" + "strconv" "strings" "testing" @@ -34,9 +38,49 @@ func TestFrontendCoverageExportedOnSolveError(t *testing.T) { assertNonEmptyGlob(t, filepath.Join(covDir, "covmeta.*")) assertNonEmptyGlob(t, filepath.Join(covDir, "covcounters.*")) + assertFrontendCoverageHasCounters(t, covDir) }) } +func assertFrontendCoverageHasCounters(t *testing.T, covDir string) { + t.Helper() + + profile := filepath.Join(t.TempDir(), "frontend.out") + cmd := exec.Command("go", "tool", "covdata", "textfmt", "-i", covDir, "-o", profile) + var stderr bytes.Buffer + cmd.Stderr = &stderr + if err := cmd.Run(); err != nil { + t.Fatalf("expected covdata textfmt to succeed, got %v: %s", err, stderr.String()) + } + + f, err := os.Open(profile) + if err != nil { + t.Fatalf("expected coverage profile to open, got %v", err) + } + defer f.Close() + + scanner := bufio.NewScanner(f) + for scanner.Scan() { + fields := strings.Fields(scanner.Text()) + if len(fields) != 3 || fields[0] == "mode:" { + continue + } + + count, err := strconv.ParseUint(fields[2], 10, 64) + if err != nil { + t.Fatalf("expected coverage count to parse from %q, got %v", scanner.Text(), err) + } + if count > 0 { + return + } + } + if err := scanner.Err(); err != nil { + t.Fatalf("expected coverage profile scan to succeed, got %v", err) + } + + t.Fatal("expected frontend coverage profile to contain at least one non-zero counter") +} + func assertNonEmptyGlob(t *testing.T, pattern string) { t.Helper() diff --git a/test/testenv/frontend_coverage.go b/test/testenv/frontend_coverage.go index 7ba7054a8..7725e0378 100644 --- a/test/testenv/frontend_coverage.go +++ b/test/testenv/frontend_coverage.go @@ -3,8 +3,7 @@ package testenv import ( "bytes" "compress/gzip" - "crypto/rand" - "crypto/sha256" + "encoding/binary" "encoding/hex" "fmt" "io" @@ -16,6 +15,8 @@ import ( "github.com/project-dalec/dalec/internal/frontendcoverage" ) +const covMetaFileHashOffset = 4 + 4 + 8 + 8 + func gunzip(b []byte) (out []byte, retErr error) { zr, err := gzip.NewReader(bytes.NewReader(b)) if err != nil { @@ -34,7 +35,7 @@ func gunzip(b []byte) (out []byte, retErr error) { // Writes files compatible with `go tool covdata`: // // covmeta. -// covcounters.... +// covcounters... func writeFrontendCovdata(outDir string, res *gwclient.Result, solveErr error) error { if outDir == "" { return nil @@ -58,8 +59,10 @@ func writeFrontendCovdata(outDir string, res *gwclient.Result, solveErr error) e return err } - sum := sha256.Sum256(meta) - hash := hex.EncodeToString(sum[:]) + hash, err := covdataMetaHash(meta) + if err != nil { + return err + } if err := os.MkdirAll(outDir, 0o755); err != nil { return err @@ -87,11 +90,40 @@ func writeFrontendCovdata(outDir string, res *gwclient.Result, solveErr error) e } } - var rb [4]byte - _, _ = rand.Read(rb[:]) - pid := os.Getpid() ts := time.Now().UnixNano() - ctrPath := filepath.Join(outDir, fmt.Sprintf("covcounters.%s.%d.%d.%x", hash, pid, ts, rb)) - return os.WriteFile(ctrPath, counters, 0o644) + for { + ctrPath := filepath.Join(outDir, fmt.Sprintf("covcounters.%s.%d.%d", hash, pid, ts)) + f, err := os.OpenFile(ctrPath, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o644) + if os.IsExist(err) { + ts++ + continue + } + if err != nil { + return err + } + if _, err := f.Write(counters); err != nil { + _ = f.Close() + _ = os.Remove(ctrPath) + return err + } + if err := f.Close(); err != nil { + _ = os.Remove(ctrPath) + return err + } + return nil + } +} + +func covdataMetaHash(meta []byte) (string, error) { + if len(meta) < covMetaFileHashOffset+16 { + return "", fmt.Errorf("coverage metadata is too short: %d bytes", len(meta)) + } + + length := binary.LittleEndian.Uint64(meta[8:16]) + if int(length) != len(meta) { + return "", fmt.Errorf("coverage metadata length mismatch: header=%d actual=%d", length, len(meta)) + } + + return hex.EncodeToString(meta[covMetaFileHashOffset : covMetaFileHashOffset+16]), nil } diff --git a/test/testenv/frontend_coverage_test.go b/test/testenv/frontend_coverage_test.go index 52a01c66b..8be9b464b 100644 --- a/test/testenv/frontend_coverage_test.go +++ b/test/testenv/frontend_coverage_test.go @@ -3,11 +3,12 @@ package testenv import ( "bytes" "compress/gzip" - "crypto/sha256" + "encoding/binary" "encoding/hex" "errors" "os" "path/filepath" + "strings" "testing" gwclient "github.com/moby/buildkit/frontend/gateway/client" @@ -15,7 +16,8 @@ import ( ) func TestWriteFrontendCovdata(t *testing.T) { - rawMeta := []byte("raw-meta") + metaHash := [16]byte{0x01, 0x02, 0x03, 0x04, 0x05} + rawMeta := covMetaForTest(t, metaHash) rawCounters := []byte("raw-counters") payload := &frontendcoverage.Payload{ MetaGz: gzipBytesForTest(t, rawMeta), @@ -55,8 +57,7 @@ func TestWriteFrontendCovdata(t *testing.T) { t.Fatalf("expected nil write error, got %v", writeErr) } - hash := sha256.Sum256(rawMeta) - hashHex := hex.EncodeToString(hash[:]) + hashHex := hex.EncodeToString(metaHash[:]) metaPath := filepath.Join(outDir, "covmeta."+hashHex) gotMeta, readErr := os.ReadFile(metaPath) @@ -74,6 +75,9 @@ func TestWriteFrontendCovdata(t *testing.T) { if len(counterFiles) != 1 { t.Fatalf("expected exactly one counter file, got %d", len(counterFiles)) } + if gotParts := len(strings.Split(filepath.Base(counterFiles[0]), ".")); gotParts != 4 { + t.Fatalf("expected covcounter filename to have 4 dot-separated parts, got %d: %s", gotParts, counterFiles[0]) + } gotCounters, readErr := os.ReadFile(counterFiles[0]) if readErr != nil { @@ -100,3 +104,12 @@ func gzipBytesForTest(t *testing.T, in []byte) []byte { return buf.Bytes() } + +func covMetaForTest(t *testing.T, hash [16]byte) []byte { + t.Helper() + + meta := make([]byte, covMetaFileHashOffset+len(hash)) + binary.LittleEndian.PutUint64(meta[8:16], uint64(len(meta))) + copy(meta[covMetaFileHashOffset:], hash[:]) + return meta +}