[gold] Fix two tests drawing the same digest
Bug: skia:10582
Change-Id: I74e0e4f90e08c03a636e35529926e8a9a57c5ac5
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/404319
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
diff --git a/golden/go/search2/search2.go b/golden/go/search2/search2.go
index 4716ee9..0b12e1d 100644
--- a/golden/go/search2/search2.go
+++ b/golden/go/search2/search2.go
@@ -716,16 +716,21 @@
ctx, span := trace.StartSpan(ctx, "getClosestDiffs")
defer span.End()
byGrouping := map[schema.MD5Hash][]stageOneResult{}
- byDigest := map[schema.MD5Hash]stageTwoResult{}
+ // Even if two groupings draw the same digest, we want those as two different results because
+ // they could be triaged differently.
+ byDigestAndGrouping := map[groupingDigestKey]stageTwoResult{}
for _, input := range inputs {
gID := sql.AsMD5Hash(input.groupingID)
byGrouping[gID] = append(byGrouping[gID], input)
- dID := sql.AsMD5Hash(input.digest)
- bd := byDigest[dID]
+ key := groupingDigestKey{
+ digest: sql.AsMD5Hash(input.digest),
+ groupingID: sql.AsMD5Hash(input.groupingID),
+ }
+ bd := byDigestAndGrouping[key]
bd.leftDigest = input.digest
bd.groupingID = input.groupingID
bd.traceIDs = append(bd.traceIDs, input.traceID)
- byDigest[dID] = bd
+ byDigestAndGrouping[key] = bd
}
for groupingID, inputs := range byGrouping {
@@ -780,8 +785,11 @@
PixelDiffPercent: row.PercentPixelsDiff,
QueryMetric: row.CombinedMetric,
}
- leftDigest := sql.AsMD5Hash(row.LeftDigest)
- stageTwo := byDigest[leftDigest]
+ key := groupingDigestKey{
+ digest: sql.AsMD5Hash(row.LeftDigest),
+ groupingID: groupingID,
+ }
+ stageTwo := byDigestAndGrouping[key]
stageTwo.rightDigests = append(stageTwo.rightDigests, row.RightDigest)
if label == schema.LabelPositive {
stageTwo.closestPositive = srdd
@@ -798,15 +806,15 @@
// there is only one type of diff, so it defaults to the closest.
stageTwo.closestDigest = srdd
}
- byDigest[leftDigest] = stageTwo
+ byDigestAndGrouping[key] = stageTwo
}
rows.Close()
}
q := getQuery(ctx)
bulkTriageData := map[groupingDigestKey]expectations.Label{}
- results := make([]stageTwoResult, 0, len(byDigest))
- for _, s2 := range byDigest {
+ results := make([]stageTwoResult, 0, len(byDigestAndGrouping))
+ for _, s2 := range byDigestAndGrouping {
// Filter out any results without a closest triaged digest (if that option is selected).
if q.MustIncludeReferenceFilter && s2.closestDigest == nil {
continue
diff --git a/golden/go/search2/search2_test.go b/golden/go/search2/search2_test.go
index d3f2165..fe3c9c5 100644
--- a/golden/go/search2/search2_test.go
+++ b/golden/go/search2/search2_test.go
@@ -16,6 +16,7 @@
"go.skia.org/infra/golden/go/search/frontend"
"go.skia.org/infra/golden/go/search/query"
"go.skia.org/infra/golden/go/sql"
+ "go.skia.org/infra/golden/go/sql/databuilder"
dks "go.skia.org/infra/golden/go/sql/datakitchensink"
"go.skia.org/infra/golden/go/sql/schema"
"go.skia.org/infra/golden/go/sql/sqltest"
@@ -2081,6 +2082,181 @@
assert.Equal(t, expectedCondition, statement)
}
+func TestSearch_DifferentTestsDrawTheSame_SearchResultsAreSeparate(t *testing.T) {
+ unittest.LargeTest(t)
+
+ ctx := context.Background()
+ db := sqltest.NewCockroachDBForTestsWithProductionSchema(ctx, t)
+ b := databuilder.TablesBuilder{TileWidth: 100}
+ b.CommitsWithData().
+ Insert("0111", "don't care", "commit 111", "2021-05-01T00:00:00Z").
+ Insert("0222", "don't care", "commit 222", "2021-05-02T00:00:00Z").
+ Insert("0333", "don't care", "commit 333", "2021-05-03T00:00:00Z")
+
+ b.SetDigests(map[rune]types.Digest{
+ 'A': dks.DigestA01Pos,
+ 'b': dks.DigestA05Unt,
+ })
+ b.SetGroupingKeys(types.CorpusField, types.PrimaryKeyField)
+ b.AddTracesWithCommonKeys(paramtools.Params{
+ types.CorpusField: dks.CornersCorpus,
+ dks.DeviceKey: dks.WalleyeDevice,
+ }).History(
+ "Abb",
+ "AAb",
+ ).Keys([]paramtools.Params{
+ {types.PrimaryKeyField: "draw_a_square"},
+ {types.PrimaryKeyField: "draw_a_square_but_faster"},
+ }).OptionsAll(paramtools.Params{"ext": "png"}).
+ IngestedFrom([]string{"don't care", "don't care 2", "don't care 3"}, []string{
+ "2021-05-01T00:01:00Z", "2021-05-02T00:02:00Z", "2021-05-03T00:03:00Z",
+ })
+ b.AddTriageEvent("somebody", "2021-02-01T01:01:01Z").
+ ExpectationsForGrouping(map[string]string{types.CorpusField: dks.CornersCorpus, types.PrimaryKeyField: "draw_a_square"}).
+ Positive(dks.DigestA01Pos).
+ ExpectationsForGrouping(map[string]string{types.CorpusField: dks.CornersCorpus, types.PrimaryKeyField: "draw_a_square_but_faster"}).
+ Positive(dks.DigestA01Pos)
+ b.AddIgnoreRule("user", "user", "2030-12-30T15:16:17Z", "Doesn't match anything",
+ paramtools.ParamSet{
+ "some key": []string{"foo bar"},
+ })
+ b.ComputeDiffMetricsFromImages(dks.GetImgDirectory(), "2021-05-03T06:00:00Z")
+ require.NoError(t, sqltest.BulkInsertDataTables(ctx, db, b.Build()))
+
+ s := New(db, 100)
+ res, err := s.Search(ctx, &query.Search{
+ OnlyIncludeDigestsProducedAtHead: true,
+ IncludePositiveDigests: false,
+ IncludeNegativeDigests: false,
+ IncludeUntriagedDigests: true,
+ Sort: query.SortDescending,
+ IncludeIgnoredTraces: false,
+ TraceValues: paramtools.ParamSet{
+ types.CorpusField: []string{dks.CornersCorpus},
+ },
+ RGBAMinFilter: 0,
+ RGBAMaxFilter: 255,
+ })
+ require.NoError(t, err)
+ assert.Equal(t, &frontend.SearchResponse{
+ Results: []*frontend.SearchResult{{
+ Digest: dks.DigestA05Unt,
+ Test: "draw_a_square_but_faster",
+ Status: expectations.Untriaged,
+ ParamSet: paramtools.ParamSet{
+ types.CorpusField: []string{dks.CornersCorpus},
+ types.PrimaryKeyField: []string{"draw_a_square_but_faster"},
+ dks.DeviceKey: []string{dks.WalleyeDevice},
+ "ext": []string{"png"},
+ },
+ TraceGroup: frontend.TraceGroup{
+ Traces: []frontend.Trace{{
+ ID: "04fe79d1ad2d6d472aafdb997bbb26d3",
+ DigestIndices: []int{1, 1, 0},
+ Params: paramtools.Params{
+ types.CorpusField: dks.CornersCorpus,
+ types.PrimaryKeyField: "draw_a_square_but_faster",
+ dks.DeviceKey: dks.WalleyeDevice,
+ "ext": "png",
+ },
+ }},
+ Digests: []frontend.DigestStatus{
+ {Digest: dks.DigestA05Unt, Status: expectations.Untriaged},
+ {Digest: dks.DigestA01Pos, Status: expectations.Positive},
+ },
+ TotalDigests: 2,
+ },
+ RefDiffs: map[common.RefClosest]*frontend.SRDiffDigest{
+ common.PositiveRef: {
+ CombinedMetric: 0.20710422, QueryMetric: 0.20710422, PixelDiffPercent: 3.125, NumDiffPixels: 2,
+ MaxRGBADiffs: [4]int{7, 0, 0, 0},
+ DimDiffer: false,
+ Digest: dks.DigestA01Pos,
+ Status: expectations.Positive,
+ ParamSet: paramtools.ParamSet{
+ types.CorpusField: []string{dks.CornersCorpus},
+ types.PrimaryKeyField: []string{"draw_a_square", "draw_a_square_but_faster"},
+ dks.DeviceKey: []string{dks.WalleyeDevice},
+ "ext": []string{"png"},
+ },
+ },
+ common.NegativeRef: nil,
+ },
+ ClosestRef: common.PositiveRef,
+ }, {
+ Digest: dks.DigestA05Unt,
+ Test: "draw_a_square",
+ Status: "untriaged",
+ ParamSet: paramtools.ParamSet{
+ types.CorpusField: []string{dks.CornersCorpus},
+ types.PrimaryKeyField: []string{"draw_a_square"},
+ dks.DeviceKey: []string{dks.WalleyeDevice},
+ "ext": []string{"png"},
+ },
+ TraceGroup: frontend.TraceGroup{
+ Traces: []frontend.Trace{{
+ ID: "9253edab166210e7198cf7e901ac87ec",
+ DigestIndices: []int{1, 0, 0},
+ Params: paramtools.Params{
+ types.CorpusField: dks.CornersCorpus,
+ types.PrimaryKeyField: "draw_a_square",
+ dks.DeviceKey: dks.WalleyeDevice,
+ "ext": "png",
+ },
+ }},
+ Digests: []frontend.DigestStatus{
+ {Digest: dks.DigestA05Unt, Status: expectations.Untriaged},
+ {Digest: dks.DigestA01Pos, Status: expectations.Positive},
+ },
+ TotalDigests: 2,
+ },
+ RefDiffs: map[common.RefClosest]*frontend.SRDiffDigest{
+ common.PositiveRef: {
+ CombinedMetric: 0.20710422, QueryMetric: 0.20710422, PixelDiffPercent: 3.125, NumDiffPixels: 2,
+ MaxRGBADiffs: [4]int{7, 0, 0, 0},
+ DimDiffer: false,
+ Digest: dks.DigestA01Pos,
+ Status: expectations.Positive,
+ ParamSet: paramtools.ParamSet{
+ types.CorpusField: []string{dks.CornersCorpus},
+ types.PrimaryKeyField: []string{"draw_a_square", "draw_a_square_but_faster"},
+ dks.DeviceKey: []string{dks.WalleyeDevice},
+ "ext": []string{"png"},
+ },
+ },
+ common.NegativeRef: nil,
+ },
+ ClosestRef: common.PositiveRef,
+ }},
+ Offset: 0,
+ Size: 2,
+ BulkTriageData: map[types.TestName]map[types.Digest]expectations.Label{
+ "draw_a_square": {
+ dks.DigestA05Unt: "positive",
+ },
+ "draw_a_square_but_faster": {
+ dks.DigestA05Unt: "positive",
+ },
+ },
+ Commits: []web_frontend.Commit{{
+ Subject: "commit 111",
+ CommitTime: 1619827200,
+ Hash: "6c505df96f1faab539199949572820b2c90f6959",
+ Author: "don't care",
+ }, {
+ Subject: "commit 222",
+ CommitTime: 1619913600,
+ Hash: "aac89c40628a35265f632940b678104349122a9f",
+ Author: "don't care",
+ }, {
+ Subject: "commit 333",
+ CommitTime: 1620000000,
+ Hash: "cb197b480a07a794b94ca9d50661db1fad2e3873",
+ Author: "don't care",
+ }},
+ }, res)
+}
+
var kitchenSinkCommits = makeKitchenSinkCommits()
func makeKitchenSinkCommits() []web_frontend.Commit {
diff --git a/golden/go/sql/databuilder/databuilder.go b/golden/go/sql/databuilder/databuilder.go
index 65daba8..48768a1 100644
--- a/golden/go/sql/databuilder/databuilder.go
+++ b/golden/go/sql/databuilder/databuilder.go
@@ -257,6 +257,7 @@
}
}
}
+ duplicates := map[string]bool{}
// For each grouping, compare each digest to every other digest and create the metric rows
// for that.
for _, xd := range toCompute {
@@ -267,6 +268,13 @@
for rightIdx := leftIdx + 1; rightIdx < len(digests); rightIdx++ {
rightDigest := digests[rightIdx]
rightImg := images[rightDigest]
+ key1 := string(leftDigest) + string(rightDigest)
+ key2 := string(rightDigest) + string(leftDigest)
+ if duplicates[key1] || duplicates[key2] {
+ continue
+ }
+ duplicates[key1] = true
+ duplicates[key2] = true
dm := diff.ComputeDiffMetrics(leftImg, rightImg)
if dm.NumDiffPixels == 0 {
logAndPanic("%s and %s aren't different", leftDigest, rightDigest)
diff --git a/golden/go/sql/datakitchensink/kitchensink.go b/golden/go/sql/datakitchensink/kitchensink.go
index 20c89fe..ef666cb 100644
--- a/golden/go/sql/datakitchensink/kitchensink.go
+++ b/golden/go/sql/datakitchensink/kitchensink.go
@@ -435,13 +435,13 @@
types.CorpusField: RoundCorpus, types.PrimaryKeyField: RoundRectTest,
}).Triage(DigestE03Unt_CL, schema.LabelNegative, schema.LabelUntriaged)
- b.ComputeDiffMetricsFromImages(getImgDirectory(), "2020-12-12T12:12:12Z")
+ b.ComputeDiffMetricsFromImages(GetImgDirectory(), "2020-12-12T12:12:12Z")
return b
}
-// getImgDirectory returns the path to the img directory in this folder that is friendly to both
+// GetImgDirectory returns the path to the img directory in this folder that is friendly to both
// `go test` and `bazel test`.
-func getImgDirectory() string {
+func GetImgDirectory() string {
root, err := repo_root.Get()
if err != nil {
panic(err)
