[gold] GetDigestDetails should support old digests
Change-Id: I1d6ffcbe9651957076c982555fe2d53b786366c8
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/251461
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
diff --git a/golden/go/search/search.go b/golden/go/search/search.go
index 8853f32..b22625b 100644
--- a/golden/go/search/search.go
+++ b/golden/go/search/search.go
@@ -163,16 +163,17 @@
}
// GetDigestDetails implements the SearchAPI interface.
+// TODO(stephana): Make the metric, match and ignores parameters for the comparison.
func (s *SearchImpl) GetDigestDetails(ctx context.Context, test types.TestName, digest types.Digest) (*frontend.DigestDetails, error) {
defer metrics2.FuncTimer().Stop()
idx := s.indexSource.GetIndex()
// Make sure we have valid data, i.e. we know about that test/digest
dct := idx.DigestCountsByTest(types.IncludeIgnoredTraces)
- if digests, ok := dct[test]; !ok {
+
+ digests, ok := dct[test]
+ if !ok {
return nil, skerr.Fmt("unknown test %s", test)
- } else if _, ok := digests[digest]; !ok {
- return nil, skerr.Fmt("unknown digest %s for test %s", digest, test)
}
tile := idx.Tile().GetTile(types.IncludeIgnoredTraces)
@@ -183,19 +184,20 @@
}
oneInter := newSrIntermediate(test, digest, "", nil, nil)
- byTrace := idx.DigestCountsByTrace(types.IncludeIgnoredTraces)
- for traceId, t := range tile.Traces {
- gTrace := t.(*types.GoldenTrace)
- if gTrace.TestName() != test {
- continue
- }
- if _, ok := byTrace[traceId][digest]; ok {
- oneInter.add(traceId, t, nil)
+ if _, ok := digests[digest]; ok {
+ // We know a digest is somewhere in at least one trace. Iterate through all of them
+ // to find which ones.
+ byTrace := idx.DigestCountsByTrace(types.IncludeIgnoredTraces)
+ for traceId, t := range tile.Traces {
+ gTrace := t.(*types.GoldenTrace)
+ if gTrace.TestName() != test {
+ continue
+ }
+ if _, ok := byTrace[traceId][digest]; ok {
+ oneInter.add(traceId, t, nil)
+ }
}
}
-
- // TODO(stephana): Make the metric, match and ignores parameters for the comparison.
-
// If there are no traces or params then set them to nil to signal there are none.
hasTraces := len(oneInter.traces) > 0
if !hasTraces {
diff --git a/golden/go/search/search_test.go b/golden/go/search/search_test.go
index c95c48e..7899ed8 100644
--- a/golden/go/search/search_test.go
+++ b/golden/go/search/search_test.go
@@ -2,6 +2,7 @@
import (
"context"
+ "errors"
"testing"
"github.com/stretchr/testify/assert"
@@ -825,23 +826,86 @@
}, details)
}
-func TestDigestDetailsThreeDevicesBadDigest(t *testing.T) {
+// TestDigestDetailsThreeDevicesOldDigest represents the scenario in which a user is requesting
+// data about a digest that just went off the tile.
+func TestDigestDetailsThreeDevicesOldDigest(t *testing.T) {
unittest.SmallTest(t)
- const digestWeWantDetailsAbout = types.Digest("invalid digest")
- const testWeWantDetailsAbout = data.AlphaTest
+ const digestWeWantDetailsAbout = types.Digest("digest-too-old")
+ const testWeWantDetailsAbout = data.BetaTest
+ mes := &mocks.ExpectationsStore{}
mi := &mock_index.IndexSource{}
+ mds := &mock_diffstore.DiffStore{}
+ defer mes.AssertExpectations(t)
defer mi.AssertExpectations(t)
+ defer mds.AssertExpectations(t)
fis := makeThreeDevicesIndex()
mi.On("GetIndex").Return(fis)
- s := New(nil, nil, mi, nil, nil, everythingPublic)
+ mes.On("Get").Return(data.MakeTestExpectations(), nil)
+
+ mds.On("UnavailableDigests", testutils.AnyContext).Return(map[types.Digest]*diff.DigestFailure{}, nil)
+
+ mds.On("Get", testutils.AnyContext, digestWeWantDetailsAbout, types.DigestSlice{data.BetaGood1Digest}).
+ Return(map[types.Digest]*diff.DiffMetrics{
+ data.BetaGood1Digest: makeSmallDiffMetric(),
+ }, nil)
+
+ s := New(mds, mes, mi, nil, nil, everythingPublic)
+
+ d, err := s.GetDigestDetails(context.Background(), testWeWantDetailsAbout, digestWeWantDetailsAbout)
+ require.NoError(t, err)
+ // spot check is fine for this test because other tests do a more thorough check of the
+ // whole struct.
+ assert.Equal(t, digestWeWantDetailsAbout, d.Digest.Digest)
+ assert.Equal(t, testWeWantDetailsAbout, d.Digest.Test)
+ assert.Equal(t, map[common.RefClosest]*frontend.SRDiffDigest{
+ common.PositiveRef: {
+ DiffMetrics: makeSmallDiffMetric(),
+ Digest: data.BetaGood1Digest,
+ Status: "positive",
+ ParamSet: paramtools.ParamSet{
+ "device": []string{data.AnglerDevice, data.BullheadDevice},
+ types.PRIMARY_KEY_FIELD: []string{string(data.BetaTest)},
+ types.CORPUS_FIELD: []string{"gm"},
+ },
+ OccurrencesInTile: 6,
+ },
+ common.NegativeRef: nil,
+ }, d.Digest.RefDiffs)
+}
+
+// TestDigestDetailsThreeDevicesOldDigest represents the scenario in which a user is requesting
+// data about a digest that never existed.
+func TestDigestDetailsThreeDevicesBadDigest(t *testing.T) {
+ unittest.SmallTest(t)
+
+ const digestWeWantDetailsAbout = types.Digest("unknown-digest")
+ const testWeWantDetailsAbout = data.BetaTest
+
+ mes := &mocks.ExpectationsStore{}
+ mi := &mock_index.IndexSource{}
+ mds := &mock_diffstore.DiffStore{}
+ defer mes.AssertExpectations(t)
+ defer mi.AssertExpectations(t)
+ defer mds.AssertExpectations(t)
+
+ fis := makeThreeDevicesIndex()
+ mi.On("GetIndex").Return(fis)
+
+ mes.On("Get").Return(data.MakeTestExpectations(), nil)
+
+ mds.On("UnavailableDigests", testutils.AnyContext).Return(map[types.Digest]*diff.DigestFailure{}, nil)
+
+ mds.On("Get", testutils.AnyContext, digestWeWantDetailsAbout, types.DigestSlice{data.BetaGood1Digest}).Return(nil, errors.New("invalid digest"))
+
+ s := New(mds, mes, mi, nil, nil, everythingPublic)
_, err := s.GetDigestDetails(context.Background(), testWeWantDetailsAbout, digestWeWantDetailsAbout)
require.Error(t, err)
- assert.Contains(t, err.Error(), "unknown")
+ assert.Contains(t, err.Error(), "invalid")
}
func TestDigestDetailsThreeDevicesBadTest(t *testing.T) {
