From 0f91f92ee0021b10930bb9eefa24b2e244b7d2e3 Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Tue, 30 May 2023 19:16:56 -0400 Subject: [PATCH] internal/coverage/pods: sort counter files first by origin, then name This patch fixes a problem with the way pods (clumps of related coverage meta+counter data files) are collected, which was causing problems for "go tool covdata subtract". A subtract operation such as "go tool covdata subtract -i=dir1,dir2 -o=out" works by loading in all the counter data files from "dir1" before any of the data files from "dir2" are loaded. The sorting function in the pods code was sorting counter files for a given pod based purely on name, which meant that differences in process ID assignment could result in some files from "dir2" being presented before "dir1". The fix is to change the sorting compare function to prefer origin directory over filename. Fixes #60526. Change-Id: I2226ea675fc99666a9a28e6550d823bcdf2d6977 Reviewed-on: https://go-review.googlesource.com/c/go/+/499317 Run-TryBot: Than McIntosh TryBot-Result: Gopher Robot Reviewed-by: Michael Pratt --- src/internal/coverage/pods/pods.go | 3 +++ src/internal/coverage/pods/pods_test.go | 32 ++++++++++++------------- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/internal/coverage/pods/pods.go b/src/internal/coverage/pods/pods.go index 432c7b6bd6..e08f82ec59 100644 --- a/src/internal/coverage/pods/pods.go +++ b/src/internal/coverage/pods/pods.go @@ -166,6 +166,9 @@ func collectPodsImpl(files []string, dirIndices []int, warn bool) []Pod { pods := make([]Pod, 0, len(mm)) for _, p := range mm { sort.Slice(p.elements, func(i, j int) bool { + if p.elements[i].origin != p.elements[j].origin { + return p.elements[i].origin < p.elements[j].origin + } return p.elements[i].file < p.elements[j].file }) pod := Pod{ diff --git a/src/internal/coverage/pods/pods_test.go b/src/internal/coverage/pods/pods_test.go index da28c06328..69c16e00eb 100644 --- a/src/internal/coverage/pods/pods_test.go +++ b/src/internal/coverage/pods/pods_test.go @@ -40,10 +40,9 @@ func TestPodCollection(t *testing.T) { return mkfile(dir, fn) } - mkcounter := func(dir string, tag string, nt int) string { + mkcounter := func(dir string, tag string, nt int, pid int) string { hash := md5.Sum([]byte(tag)) - dummyPid := int(42) - fn := fmt.Sprintf(coverage.CounterFileTempl, coverage.CounterFilePref, hash, dummyPid, nt) + fn := fmt.Sprintf(coverage.CounterFileTempl, coverage.CounterFilePref, hash, pid, nt) return mkfile(dir, fn) } @@ -76,18 +75,18 @@ func TestPodCollection(t *testing.T) { // Add a meta-data file with two counter files to first dir. mkmeta(o1, "m1") - mkcounter(o1, "m1", 1) - mkcounter(o1, "m1", 2) - mkcounter(o1, "m1", 2) + mkcounter(o1, "m1", 1, 42) + mkcounter(o1, "m1", 2, 41) + mkcounter(o1, "m1", 2, 40) // Add a counter file with no associated meta file. - mkcounter(o1, "orphan", 9) + mkcounter(o1, "orphan", 9, 39) // Add a meta-data file with three counter files to second dir. mkmeta(o2, "m2") - mkcounter(o2, "m2", 1) - mkcounter(o2, "m2", 2) - mkcounter(o2, "m2", 3) + mkcounter(o2, "m2", 1, 38) + mkcounter(o2, "m2", 2, 37) + mkcounter(o2, "m2", 3, 36) // Add a duplicate of the first meta-file and a corresponding // counter file to the second dir. This is intended to capture @@ -95,7 +94,7 @@ func TestPodCollection(t *testing.T) { // coverage-instrumented binary, but with the output files // sent to separate directories. mkmeta(o2, "m1") - mkcounter(o2, "m1", 11) + mkcounter(o2, "m1", 11, 35) // Collect pods. podlist, err := pods.CollectPods([]string{o1, o2}, true) @@ -114,14 +113,15 @@ func TestPodCollection(t *testing.T) { expected := []string{ `o1/covmeta.ae7be26cdaa742ca148068d5ac90eaca [ +o1/covcounters.ae7be26cdaa742ca148068d5ac90eaca.40.2 o:0 +o1/covcounters.ae7be26cdaa742ca148068d5ac90eaca.41.2 o:0 o1/covcounters.ae7be26cdaa742ca148068d5ac90eaca.42.1 o:0 -o1/covcounters.ae7be26cdaa742ca148068d5ac90eaca.42.2 o:0 -o2/covcounters.ae7be26cdaa742ca148068d5ac90eaca.42.11 o:1 +o2/covcounters.ae7be26cdaa742ca148068d5ac90eaca.35.11 o:1 ]`, `o2/covmeta.aaf2f89992379705dac844c0a2a1d45f [ -o2/covcounters.aaf2f89992379705dac844c0a2a1d45f.42.1 o:1 -o2/covcounters.aaf2f89992379705dac844c0a2a1d45f.42.2 o:1 -o2/covcounters.aaf2f89992379705dac844c0a2a1d45f.42.3 o:1 +o2/covcounters.aaf2f89992379705dac844c0a2a1d45f.36.3 o:1 +o2/covcounters.aaf2f89992379705dac844c0a2a1d45f.37.2 o:1 +o2/covcounters.aaf2f89992379705dac844c0a2a1d45f.38.1 o:1 ]`, } for k, exp := range expected { -- 2.50.0