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 <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
This commit is contained in:
Than McIntosh 2023-05-30 19:16:56 -04:00
parent 094c75219a
commit 0f91f92ee0
2 changed files with 19 additions and 16 deletions

View file

@ -166,6 +166,9 @@ func collectPodsImpl(files []string, dirIndices []int, warn bool) []Pod {
pods := make([]Pod, 0, len(mm)) pods := make([]Pod, 0, len(mm))
for _, p := range mm { for _, p := range mm {
sort.Slice(p.elements, func(i, j int) bool { 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 return p.elements[i].file < p.elements[j].file
}) })
pod := Pod{ pod := Pod{

View file

@ -40,10 +40,9 @@ func TestPodCollection(t *testing.T) {
return mkfile(dir, fn) 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)) hash := md5.Sum([]byte(tag))
dummyPid := int(42) fn := fmt.Sprintf(coverage.CounterFileTempl, coverage.CounterFilePref, hash, pid, nt)
fn := fmt.Sprintf(coverage.CounterFileTempl, coverage.CounterFilePref, hash, dummyPid, nt)
return mkfile(dir, fn) 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. // Add a meta-data file with two counter files to first dir.
mkmeta(o1, "m1") mkmeta(o1, "m1")
mkcounter(o1, "m1", 1) mkcounter(o1, "m1", 1, 42)
mkcounter(o1, "m1", 2) mkcounter(o1, "m1", 2, 41)
mkcounter(o1, "m1", 2) mkcounter(o1, "m1", 2, 40)
// Add a counter file with no associated meta file. // 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. // Add a meta-data file with three counter files to second dir.
mkmeta(o2, "m2") mkmeta(o2, "m2")
mkcounter(o2, "m2", 1) mkcounter(o2, "m2", 1, 38)
mkcounter(o2, "m2", 2) mkcounter(o2, "m2", 2, 37)
mkcounter(o2, "m2", 3) mkcounter(o2, "m2", 3, 36)
// Add a duplicate of the first meta-file and a corresponding // Add a duplicate of the first meta-file and a corresponding
// counter file to the second dir. This is intended to capture // 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 // coverage-instrumented binary, but with the output files
// sent to separate directories. // sent to separate directories.
mkmeta(o2, "m1") mkmeta(o2, "m1")
mkcounter(o2, "m1", 11) mkcounter(o2, "m1", 11, 35)
// Collect pods. // Collect pods.
podlist, err := pods.CollectPods([]string{o1, o2}, true) podlist, err := pods.CollectPods([]string{o1, o2}, true)
@ -114,14 +113,15 @@ func TestPodCollection(t *testing.T) {
expected := []string{ expected := []string{
`o1/covmeta.ae7be26cdaa742ca148068d5ac90eaca [ `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.1 o:0
o1/covcounters.ae7be26cdaa742ca148068d5ac90eaca.42.2 o:0 o2/covcounters.ae7be26cdaa742ca148068d5ac90eaca.35.11 o:1
o2/covcounters.ae7be26cdaa742ca148068d5ac90eaca.42.11 o:1
]`, ]`,
`o2/covmeta.aaf2f89992379705dac844c0a2a1d45f [ `o2/covmeta.aaf2f89992379705dac844c0a2a1d45f [
o2/covcounters.aaf2f89992379705dac844c0a2a1d45f.42.1 o:1 o2/covcounters.aaf2f89992379705dac844c0a2a1d45f.36.3 o:1
o2/covcounters.aaf2f89992379705dac844c0a2a1d45f.42.2 o:1 o2/covcounters.aaf2f89992379705dac844c0a2a1d45f.37.2 o:1
o2/covcounters.aaf2f89992379705dac844c0a2a1d45f.42.3 o:1 o2/covcounters.aaf2f89992379705dac844c0a2a1d45f.38.1 o:1
]`, ]`,
} }
for k, exp := range expected { for k, exp := range expected {