[gold] Preslice data in indexer by IgnoreState/Corpus/Test
This decreases the latency of /json/byblame by at least 3x
and can be used in general searching too.
Timings of /json/byblame on skia-gold-public instance for
the given corpus (average of 3)
Before:
gm: 7.8s
image: 5.1s
svg: 2.3s
After:
gm: 1.2s
image: 1.8s
svg: .74s
Bug: skia:9080
Change-Id: I8d200ebe4dd687ed2f51220dc8254038abfd3d99
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/253882
Reviewed-by: Ben Wagner aka dogben <benjaminwagner@google.com>
diff --git a/go/tiling/tiling.go b/go/tiling/tiling.go
index c5b54a1..5b57764 100644
--- a/go/tiling/tiling.go
+++ b/go/tiling/tiling.go
@@ -44,11 +44,6 @@
// Just like a Go [:] slice this is inclusive of begin and exclusive of end.
// The length on the Trace will then become end-begin.
Trim(begin, end int) error
-
- // SetAt sets the value of the measurement at index.
- //
- // Each specialization will convert []byte to the correct type.
- SetAt(index int, value []byte) error
}
// TraceID helps document when strings should represent TraceIds
diff --git a/golden/go/indexer/indexer.go b/golden/go/indexer/indexer.go
index 34b74f2..e8dd8c6 100644
--- a/golden/go/indexer/indexer.go
+++ b/golden/go/indexer/indexer.go
@@ -49,6 +49,7 @@
dCounters [2]digest_counter.DigestCounter
summaries [2]countsAndBlames
paramsetSummaries [2]paramsets.ParamSummary
+ preSliced map[preSliceGroup][]*types.TracePair
cpxTile types.ComplexTile
blamer blame.Blamer
@@ -58,6 +59,12 @@
testNames types.TestNameSet
}
+type preSliceGroup struct {
+ IgnoreState types.IgnoreState
+ Corpus string
+ Test types.TestName
+}
+
// countsAndBlame makes the type declaration of SearchIndex a little nicer to read.
type countsAndBlames []*summary.TriageStatus
@@ -78,23 +85,26 @@
dCounters: [2]digest_counter.DigestCounter{},
summaries: [2]countsAndBlames{},
paramsetSummaries: [2]paramsets.ParamSummary{},
+ preSliced: map[preSliceGroup][]*types.TracePair{},
cpxTile: cpxTile,
}
}
// SearchIndexForTesting returns filled in search index to be used when testing. Note that the
// indices of the arrays are the int values of types.IgnoreState
-func SearchIndexForTesting(cpxTile types.ComplexTile, dc [2]digest_counter.DigestCounter, pm [2]paramsets.ParamSummary, exp expstorage.ExpectationsStore, b blame.Blamer) *SearchIndex {
- return &SearchIndex{
+func SearchIndexForTesting(cpxTile types.ComplexTile, dc [2]digest_counter.DigestCounter, pm [2]paramsets.ParamSummary, exp expstorage.ExpectationsStore, b blame.Blamer) (*SearchIndex, error) {
+ s := &SearchIndex{
searchIndexConfig: searchIndexConfig{
expectationsStore: exp,
},
dCounters: dc,
summaries: [2]countsAndBlames{},
paramsetSummaries: pm,
+ preSliced: map[preSliceGroup][]*types.TracePair{},
blamer: b,
cpxTile: cpxTile,
}
+ return s, preSliceData(s)
}
// Tile implements the IndexSearcher interface.
@@ -139,7 +149,7 @@
return nil, skerr.Wrap(err)
}
d := summary.Data{
- Traces: idx.cpxTile.GetTile(is).Traces,
+ Traces: idx.slicedTraces(is, query),
Expectations: exp,
ByTrace: idx.dCounters[is].ByTrace(),
Blamer: idx.blamer,
@@ -167,6 +177,35 @@
return idx.blamer.GetBlame(test, digest, commits)
}
+// slicedTraces returns a slice of TracePairs that match the query and the ignore state.
+// This is meant to be a superset of traces, as only the corpus and testname from the query are
+// used for this pre-filter step.
+func (idx *SearchIndex) slicedTraces(is types.IgnoreState, query map[string][]string) []*types.TracePair {
+ if len(query[types.CORPUS_FIELD]) == 0 {
+ return idx.preSliced[preSliceGroup{
+ IgnoreState: is,
+ }]
+ }
+ var rv []*types.TracePair
+ for _, corpus := range query[types.CORPUS_FIELD] {
+ if len(query[types.PRIMARY_KEY_FIELD]) == 0 {
+ rv = append(rv, idx.preSliced[preSliceGroup{
+ IgnoreState: is,
+ Corpus: corpus,
+ }]...)
+ } else {
+ for _, tn := range query[types.PRIMARY_KEY_FIELD] {
+ rv = append(rv, idx.preSliced[preSliceGroup{
+ IgnoreState: is,
+ Corpus: corpus,
+ Test: types.TestName(tn),
+ }]...)
+ }
+ }
+ }
+ return rv
+}
+
type IndexerConfig struct {
DiffStore diff.DiffStore
EventBus eventbus.EventBus
@@ -206,6 +245,8 @@
// in large repos like Skia.
countsNodeExclude := root.Child(calcDigestCountsExclude)
+ preSliceNode := root.Child(preSliceData)
+
// Node that triggers blame and writing baselines.
// This is used to trigger when expectations change.
// We don't need to re-calculate DigestCounts if the
@@ -227,7 +268,7 @@
writeHashes := countsNodeInclude.Child(writeKnownHashesList)
// Summaries depend on DigestCounter and Blamer.
- summariesNode := pdag.NewNodeWithParents(calcSummaries, countsNodeInclude, countsNodeExclude, blamerNode)
+ summariesNode := pdag.NewNodeWithParents(calcSummaries, countsNodeInclude, countsNodeExclude, blamerNode, preSliceNode)
// The Warmer depends on summaries.
pdag.NewNodeWithParents(runWarmer, summariesNode)
@@ -377,6 +418,7 @@
cpxTile: lastIdx.cpxTile,
dCounters: lastIdx.dCounters, // stay the same even if expectations change.
paramsetSummaries: lastIdx.paramsetSummaries, // stay the same even if expectations change.
+ preSliced: lastIdx.preSliced, // stay the same even if expectations change.
summaries: [2]countsAndBlames{
// the objects inside the summaries are immutable, but may be replaced if expectations
@@ -422,6 +464,48 @@
return nil
}
+// preSliceData is the pipeline function to pre-slice our traces. Currently, we pre-slice by
+// corpus name and then by test name because this breaks our traces up into groups of <1000.
+func preSliceData(state interface{}) error {
+ idx := state.(*SearchIndex)
+ for _, is := range types.IgnoreStates {
+ t := idx.cpxTile.GetTile(is)
+ for id, tr := range t.Traces {
+ gt, ok := tr.(*types.GoldenTrace)
+ if !ok {
+ sklog.Warningf("Unexpected trace type: %#v", gt)
+ continue
+ }
+ tp := types.TracePair{
+ ID: id,
+ Trace: gt,
+ }
+ // Pre-slice the data by IgnoreState, then by IgnoreState and Corpus, finally by all
+ // three of IgnoreState/Corpus/Test. We shouldn't allow queries by Corpus w/o specifying
+ // IgnoreState, nor should we allow queries by TestName w/o specifying a Corpus or
+ // IgnoreState.
+ ignoreOnly := preSliceGroup{
+ IgnoreState: is,
+ }
+ idx.preSliced[ignoreOnly] = append(idx.preSliced[ignoreOnly], &tp)
+
+ ignoreAndCorpus := preSliceGroup{
+ IgnoreState: is,
+ Corpus: gt.Corpus(),
+ }
+ idx.preSliced[ignoreAndCorpus] = append(idx.preSliced[ignoreAndCorpus], &tp)
+
+ ignoreCorpusTest := preSliceGroup{
+ IgnoreState: is,
+ Corpus: gt.Corpus(),
+ Test: gt.TestName(),
+ }
+ idx.preSliced[ignoreCorpusTest] = append(idx.preSliced[ignoreCorpusTest], &tp)
+ }
+ }
+ return nil
+}
+
// calcSummaries is the pipeline function to calculate the summaries.
func calcSummaries(state interface{}) error {
idx := state.(*SearchIndex)
@@ -431,7 +515,7 @@
}
for _, is := range types.IgnoreStates {
d := summary.Data{
- Traces: idx.cpxTile.GetTile(is).Traces,
+ Traces: idx.slicedTraces(is, nil),
Expectations: exp,
ByTrace: idx.dCounters[is].ByTrace(),
Blamer: idx.blamer,
diff --git a/golden/go/indexer/indexer_test.go b/golden/go/indexer/indexer_test.go
index 2e40688..ca5decf 100644
--- a/golden/go/indexer/indexer_test.go
+++ b/golden/go/indexer/indexer_test.go
@@ -190,9 +190,11 @@
digest_counter.New(partialTile),
digest_counter.New(fullTile),
},
+ preSliced: map[preSliceGroup][]*types.TracePair{},
cpxTile: ct,
}
+ require.NoError(t, preSliceData(ixr.lastIndex))
ixr.indexTests([]expstorage.Delta{
{
@@ -229,6 +231,99 @@
wg.Wait()
}
+// TestPreSlicedTracesCreatedCorrectly makes sure that we pre-slice the data based on IgnoreState,
+// then Corpus, then TestName.
+func TestPreSlicedTracesCreatedCorrectly(t *testing.T) {
+ unittest.SmallTest(t)
+
+ ct, _, _ := makeComplexTileWithCrosshatchIgnores()
+
+ si := &SearchIndex{
+ preSliced: map[preSliceGroup][]*types.TracePair{},
+ cpxTile: ct,
+ }
+ require.NoError(t, preSliceData(si))
+
+ // (2 IgnoreStates) + (2 IgnoreStates * 1 corpus) + (2 IgnoreStates * 1 corpus * 2 tests)
+ assert.Len(t, si.preSliced, 8)
+ allCombos := []preSliceGroup{
+ {
+ IgnoreState: types.IncludeIgnoredTraces,
+ },
+ {
+ IgnoreState: types.ExcludeIgnoredTraces,
+ },
+ {
+ IgnoreState: types.IncludeIgnoredTraces,
+ Corpus: "gm",
+ },
+ {
+ IgnoreState: types.ExcludeIgnoredTraces,
+ Corpus: "gm",
+ },
+ {
+ IgnoreState: types.IncludeIgnoredTraces,
+ Corpus: "gm",
+ Test: data.AlphaTest,
+ },
+ {
+ IgnoreState: types.IncludeIgnoredTraces,
+ Corpus: "gm",
+ Test: data.BetaTest,
+ },
+ {
+ IgnoreState: types.ExcludeIgnoredTraces,
+ Corpus: "gm",
+ Test: data.AlphaTest,
+ },
+ {
+ IgnoreState: types.ExcludeIgnoredTraces,
+ Corpus: "gm",
+ Test: data.BetaTest,
+ },
+ }
+ for _, psg := range allCombos {
+ assert.Contains(t, si.preSliced, psg)
+ }
+}
+
+// TestPreSlicedTracesQuery tests that querying slicedTraces returns the correct set of tracePairs
+// from the preSliced data. This especially includes if multiple tests are in the query.
+func TestPreSlicedTracesQuery(t *testing.T) {
+ unittest.SmallTest(t)
+
+ ct, _, _ := makeComplexTileWithCrosshatchIgnores()
+
+ si := &SearchIndex{
+ preSliced: map[preSliceGroup][]*types.TracePair{},
+ cpxTile: ct,
+ }
+ require.NoError(t, preSliceData(si))
+
+ allTraces := si.slicedTraces(types.IncludeIgnoredTraces, nil)
+ assert.Len(t, allTraces, 6)
+
+ withIgnores := si.slicedTraces(types.ExcludeIgnoredTraces, nil)
+ assert.Len(t, withIgnores, 4)
+
+ justCorpus := si.slicedTraces(types.IncludeIgnoredTraces, map[string][]string{
+ types.CORPUS_FIELD: {"gm"},
+ })
+ assert.Len(t, justCorpus, 6)
+
+ bothTests := si.slicedTraces(types.IncludeIgnoredTraces, map[string][]string{
+ types.CORPUS_FIELD: {"gm"},
+ types.PRIMARY_KEY_FIELD: {string(data.BetaTest), string(data.AlphaTest)},
+ })
+ assert.Len(t, bothTests, 6)
+
+ oneTest := si.slicedTraces(types.ExcludeIgnoredTraces, map[string][]string{
+ types.CORPUS_FIELD: {"gm"},
+ types.PRIMARY_KEY_FIELD: {string(data.AlphaTest)},
+ })
+ assert.Len(t, oneTest, 2)
+}
+
const (
// valid, but arbitrary md5 hash
unavailableDigest = types.Digest("fed541470e246b63b313930523220de8")
@@ -238,7 +333,8 @@
// condition similar to https://github.com/stretchr/testify/issues/625 In essence, try
// to avoid having a mock (A) assert it was called with another mock (B) where the
// mock B is used elsewhere. There's a race because mock B is keeping track of what was
-// called on it while mock A records what it was called with.
+// called on it while mock A records what it was called with. Additionally, the general guidelines
+// are to prefer to use the real thing instead of a mock.
func makeComplexTileWithCrosshatchIgnores() (types.ComplexTile, *tiling.Tile, *tiling.Tile) {
fullTile := data.MakeTestTile()
partialTile := data.MakeTestTile()
diff --git a/golden/go/search/search.go b/golden/go/search/search.go
index 480d7eb..e0c44dd 100644
--- a/golden/go/search/search.go
+++ b/golden/go/search/search.go
@@ -134,8 +134,7 @@
if getRefDiffs {
// Diff stage: Compare all digests found in the previous stages and find
// reference points (positive, negative etc.) for each digest.
- err := s.getReferenceDiffs(ctx, ret, q.Metric, q.Match, q.RTraceValues, q.IgnoreState(), exp, idx)
- if err != nil {
+ if err := s.getReferenceDiffs(ctx, ret, q.Metric, q.Match, q.RTraceValues, q.IgnoreState(), exp, idx); err != nil {
return nil, skerr.Wrapf(err, "fetching reference diffs for %#v", q)
}
diff --git a/golden/go/search/search_test.go b/golden/go/search/search_test.go
index 546b488..baf79dd 100644
--- a/golden/go/search/search_test.go
+++ b/golden/go/search/search_test.go
@@ -953,7 +953,12 @@
cpxTile := types.NewComplexTile(data.MakeTestTile())
dc := digest_counter.New(data.MakeTestTile())
ps := paramsets.NewParamSummary(data.MakeTestTile(), dc)
- return indexer.SearchIndexForTesting(cpxTile, [2]digest_counter.DigestCounter{dc, dc}, [2]paramsets.ParamSummary{ps, ps}, nil, nil)
+ si, err := indexer.SearchIndexForTesting(cpxTile, [2]digest_counter.DigestCounter{dc, dc}, [2]paramsets.ParamSummary{ps, ps}, nil, nil)
+ if err != nil {
+ // Something is horribly broken with our test data/setup
+ panic(err.Error())
+ }
+ return si
}
// This is arbitrary data.
diff --git a/golden/go/summary/summary.go b/golden/go/summary/summary.go
index 0b45f64..ed2e876 100644
--- a/golden/go/summary/summary.go
+++ b/golden/go/summary/summary.go
@@ -69,7 +69,7 @@
// Data is a helper struct containing the data that goes into computing a summary.
type Data struct {
- Traces map[tiling.TraceID]tiling.Trace
+ Traces []*types.TracePair
Expectations expectations.ReadOnly
// ByTrace maps all traces in Trace to the counts of digests that appeared
// in those traces.
@@ -92,19 +92,18 @@
var ret []*TriageStatus
// Filter down to just the traces we are interested in, based on query.
- filtered := map[grouping][]*tracePair{}
+ filtered := map[grouping][]*types.TracePair{}
t := shared.NewMetricsTimer("calc_summaries_filter_traces")
- for id, tr := range s.Traces {
- gt := tr.(*types.GoldenTrace)
- if len(testNames) > 0 && !testNames[gt.TestName()] {
+ for _, tp := range s.Traces {
+ if len(testNames) > 0 && !testNames[tp.Trace.TestName()] {
continue
}
- if tiling.Matches(tr, query) {
- k := grouping{test: gt.TestName(), corpus: gt.Corpus()}
+ if tiling.Matches(tp.Trace, query) {
+ k := grouping{test: tp.Trace.TestName(), corpus: tp.Trace.Corpus()}
if slice, ok := filtered[k]; ok {
- filtered[k] = append(slice, &tracePair{tr: gt, id: id})
+ filtered[k] = append(slice, tp)
} else {
- filtered[k] = []*tracePair{{tr: gt, id: id}}
+ filtered[k] = []*types.TracePair{tp}
}
}
}
@@ -117,24 +116,24 @@
for _, pair := range traces {
if head {
// Find the last non-missing value in the trace.
- for i := len(pair.tr.Digests) - 1; i >= 0; i-- {
- if pair.tr.IsMissing(i) {
+ for i := len(pair.Trace.Digests) - 1; i >= 0; i-- {
+ if pair.Trace.IsMissing(i) {
continue
} else {
- digestMap[pair.tr.Digests[i]] = true
+ digestMap[pair.Trace.Digests[i]] = true
break
}
}
} else {
// Use the digests by trace if available, otherwise just inspect the trace.
- if t, ok := s.ByTrace[pair.id]; ok {
+ if t, ok := s.ByTrace[pair.ID]; ok {
for d := range t {
digestMap[d] = true
}
} else {
- for i := len(pair.tr.Digests) - 1; i >= 0; i-- {
- if !pair.tr.IsMissing(i) {
- digestMap[pair.tr.Digests[i]] = true
+ for i := len(pair.Trace.Digests) - 1; i >= 0; i-- {
+ if !pair.Trace.IsMissing(i) {
+ digestMap[pair.Trace.Digests[i]] = true
}
}
}
@@ -160,12 +159,6 @@
corpus string
}
-// tracePair is used to hold traces, along with their ids.
-type tracePair struct {
- id tiling.TraceID
- tr *types.GoldenTrace
-}
-
// makeSummary returns a TriageStatus for the given digests.
func (s *Data) makeSummary(name types.TestName, corpus string, digests types.DigestSlice) *TriageStatus {
pos := 0
diff --git a/golden/go/summary/summary_test.go b/golden/go/summary/summary_test.go
index b894c41..7179ce7 100644
--- a/golden/go/summary/summary_test.go
+++ b/golden/go/summary/summary_test.go
@@ -210,8 +210,10 @@
blamer, err := blame.New(makeFullTile(), makeExpectations())
require.NoError(t, err)
+ tr := asSlice(t, tile.Traces)
+
d := Data{
- Traces: tile.Traces,
+ Traces: tr,
Expectations: makeExpectations(),
ByTrace: dc.ByTrace(),
Blamer: blamer,
@@ -222,6 +224,19 @@
return d.Calculate(testNames, query, head)
}
+func asSlice(t *testing.T, traces map[tiling.TraceID]tiling.Trace) []*types.TracePair {
+ xt := make([]*types.TracePair, 0, len(traces))
+ for id, tr := range traces {
+ gt, ok := tr.(*types.GoldenTrace)
+ require.True(t, ok)
+ xt = append(xt, &types.TracePair{
+ ID: id,
+ Trace: gt,
+ })
+ }
+ return xt
+}
+
// TestSummaryMap_FullBugRevert checks the entire return value, rather
// than just the spot checks above.
func TestSummaryMap_FullBugRevert(t *testing.T) {
@@ -320,8 +335,10 @@
blamer, err := blame.New(bug_revert.MakeTestTile(), bug_revert.MakeTestExpectations())
require.NoError(t, err)
+ tr := asSlice(t, bug_revert.MakeTestTile().Traces)
+
d := Data{
- Traces: bug_revert.MakeTestTile().Traces,
+ Traces: tr,
Expectations: bug_revert.MakeTestExpectations(),
ByTrace: dc.ByTrace(),
Blamer: blamer,
@@ -380,8 +397,10 @@
blamer, err := blame.New(tile, &e)
require.NoError(t, err)
+ tr := asSlice(t, tile.Traces)
+
d := Data{
- Traces: tile.Traces,
+ Traces: tr,
Expectations: &e,
ByTrace: dc.ByTrace(),
Blamer: blamer,
diff --git a/golden/go/types/complex_tile.go b/golden/go/types/complex_tile.go
index 9e5e79e..77c2d23 100644
--- a/golden/go/types/complex_tile.go
+++ b/golden/go/types/complex_tile.go
@@ -104,11 +104,3 @@
// Make sure ComplexTileImpl fulfills the ComplexTile Interface
var _ ComplexTile = (*ComplexTileImpl)(nil)
-
-// TODO(kjlubick): Most (all?) places in gold, we don't look anything up by trace id
-// Maps aren't the best choice in those cases, so maybe instead of
-// handing around a map of TraceID -> Trace we can hand around a []TracePair
-type TracePair struct {
- ID tiling.TraceID
- Trace tiling.Trace
-}
diff --git a/golden/go/types/types.go b/golden/go/types/types.go
index c589d32..58563f0 100644
--- a/golden/go/types/types.go
+++ b/golden/go/types/types.go
@@ -1,19 +1,11 @@
package types
import (
- "encoding/gob"
"fmt"
"go.skia.org/infra/go/tiling"
)
-func init() {
- // Register *GoldenTrace in gob so that it can be used as a
- // concrete type for Trace when writing and reading Tiles in gobs.
- // TODO(kjlubick) It does not appear we gob encode traces anymore.
- gob.Register(&GoldenTrace{})
-}
-
const (
// PRIMARY_KEY_FIELD is the field that uniquely identifies a key.
PRIMARY_KEY_FIELD = "name"
@@ -196,23 +188,6 @@
return nil
}
-// SetAt implements the tiling.Trace interface.
-func (g *GoldenTrace) SetAt(index int, value []byte) error {
- if index < 0 || index > len(g.Digests) {
- return fmt.Errorf("Invalid index: %d", index)
- }
- g.Digests[index] = Digest(value)
- return nil
-}
-
-// LastDigest returns the last digest in the trace (HEAD) or the empty string otherwise.
-func (g *GoldenTrace) LastDigest() Digest {
- if idx := g.LastIndex(); idx >= 0 {
- return g.Digests[idx]
- }
- return MISSING_DIGEST
-}
-
// LastIndex returns the index of last non-empty value in this trace and -1 if
// if the entire trace is empty.
func (g *GoldenTrace) LastIndex() int {
@@ -228,3 +203,10 @@
func (g *GoldenTrace) String() string {
return fmt.Sprintf("Keys: %#v, Digests: %q", g.Keys, g.Digests)
}
+
+// TracePair represents a single Golden trace and its ID. A slice of TracePair is faster to
+// iterate over than a map of TraceID -> Trace
+type TracePair struct {
+ ID tiling.TraceID
+ Trace *GoldenTrace
+}
diff --git a/golden/go/types/types_test.go b/golden/go/types/types_test.go
index f5c0ed8..501aa57 100644
--- a/golden/go/types/types_test.go
+++ b/golden/go/types/types_test.go
@@ -58,30 +58,6 @@
assert.Equal(t, 0, g.Len(), "final size wrong")
}
-func TestSetAt(t *testing.T) {
- unittest.SmallTest(t)
- testCases := []struct {
- want Digest
- }{
- {
- want: "",
- },
- {
- want: "abcd",
- },
- {
- want: MISSING_DIGEST,
- },
- }
- tr := NewEmptyGoldenTrace(len(testCases), nil)
- for i, tc := range testCases {
- require.NoError(t, tr.SetAt(i, []byte(tc.want)))
- }
- for i, tc := range testCases {
- assert.Equal(t, tc.want, tr.Digests[i], "Bad at test case %d", i)
- }
-}
-
var _tn TestName
// BenchmarkTraceTestName shows that a map-lookup in go for this example param map is about
diff --git a/golden/go/web/web.go b/golden/go/web/web.go
index 99db2b9..16d063d 100644
--- a/golden/go/web/web.go
+++ b/golden/go/web/web.go
@@ -296,7 +296,11 @@
Commits: commitinfo[groupid],
})
}
- sort.Sort(ByBlameEntrySlice(blameEntries))
+ sort.Slice(blameEntries, func(i, j int) bool {
+ return blameEntries[i].NDigests > blameEntries[j].NDigests ||
+ // For test determinism, use GroupID as a tie-breaker
+ (blameEntries[i].NDigests == blameEntries[j].NDigests && blameEntries[i].GroupID < blameEntries[j].GroupID)
+ })
return blameEntries, nil
}
@@ -320,16 +324,6 @@
Commits []*tiling.Commit `json:"commits"`
}
-type ByBlameEntrySlice []ByBlameEntry
-
-func (b ByBlameEntrySlice) Len() int { return len(b) }
-func (b ByBlameEntrySlice) Less(i, j int) bool {
- return b[i].NDigests > b[j].NDigests ||
- // For test determinism, use GroupID as a tie-breaker
- (b[i].NDigests == b[j].NDigests && b[i].GroupID < b[j].GroupID)
-}
-func (b ByBlameEntrySlice) Swap(i, j int) { b[i], b[j] = b[j], b[i] }
-
// ByBlame describes a single digest and its blames.
type ByBlame struct {
Test types.TestName `json:"test"`
diff --git a/golden/go/web/web_test.go b/golden/go/web/web_test.go
index f2cbff2..240d00d 100644
--- a/golden/go/web/web_test.go
+++ b/golden/go/web/web_test.go
@@ -161,7 +161,12 @@
panic(err) // this means our static data is horribly broken
}
- return indexer.SearchIndexForTesting(cpxTile, [2]digest_counter.DigestCounter{dc, dc}, [2]paramsets.ParamSummary{ps, ps}, exp, b)
+ si, err := indexer.SearchIndexForTesting(cpxTile, [2]digest_counter.DigestCounter{dc, dc}, [2]paramsets.ParamSummary{ps, ps}, exp, b)
+ if err != nil {
+ // Something is horribly broken with our test data
+ panic(err.Error())
+ }
+ return si
}
// TestGetChangeListsSunnyDay tests the core functionality of
