[gold] Make SQL-based diffs the default
We retain the way to opt out, by adding use_sql=false
(just in case).
Bug: skia:10582
Change-Id: Ieb5b9e84a76778381b1ebf488a5093f383ba19d4
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/373776
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
diff --git a/golden/go/web/web.go b/golden/go/web/web.go
index 73a3fc3..79d6fb5 100644
--- a/golden/go/web/web.go
+++ b/golden/go/web/web.go
@@ -555,7 +555,7 @@
ctx, cancel := context.WithTimeout(r.Context(), 3*time.Minute)
defer cancel()
var span *trace.Span
- if r.Form.Get("use_sql") != "" {
+ if r.Form.Get("use_sql") != "false" {
ctx = context.WithValue(ctx, search.UseSQLDiffMetricsKey, true)
ctx, span = trace.StartSpan(ctx, "SearchHandler_sql")
} else {
@@ -702,7 +702,7 @@
}
ctx := r.Context()
- if r.Form.Get("use_sql") != "" {
+ if r.Form.Get("use_sql") != "false" {
ctx = context.WithValue(ctx, search.UseSQLDiffMetricsKey, true)
}
ret, err := wh.SearchAPI.DiffDigests(ctx, types.TestName(test), types.Digest(left), types.Digest(right), clID, crs)
@@ -1065,7 +1065,7 @@
testName := testNames[0]
ctx := r.Context()
var span *trace.Span
- if r.Form.Get("use_sql") != "" {
+ if r.Form.Get("use_sql") != "false" {
ctx = context.WithValue(ctx, search.UseSQLDiffMetricsKey, true)
ctx, span = trace.StartSpan(ctx, "ClusterDiff_sql")
} else {
diff --git a/golden/go/web/web_test.go b/golden/go/web/web_test.go
index 9300414..91432b7 100644
--- a/golden/go/web/web_test.go
+++ b/golden/go/web/web_test.go
@@ -2592,7 +2592,7 @@
test("invalid trace values", "/json/list?corpus=gm&trace_values=%zz")
}
-func TestDiffHandler_DefaultContext_Success(t *testing.T) {
+func TestDiffHandler_DefaultUsesSQL_Success(t *testing.T) {
unittest.SmallTest(t)
ms := &mock_search.SearchAPI{}
@@ -2600,7 +2600,12 @@
const testAlpha = types.TestName("alpha")
const leftDigest = types.Digest("11111111111111111111111111111111")
const rightDigest = types.Digest("22222222222222222222222222222222")
- ms.On("DiffDigests", testutils.AnyContext, testAlpha, leftDigest, rightDigest, "", "").Return(&search_fe.DigestComparison{
+ ctxMatcher := mock.MatchedBy(func(ctx context.Context) bool {
+ v := ctx.Value(search.UseSQLDiffMetricsKey)
+ assert.NotNil(t, v)
+ return true
+ })
+ ms.On("DiffDigests", ctxMatcher, testAlpha, leftDigest, rightDigest, "", "").Return(&search_fe.DigestComparison{
// Arbitrary data from a search unit test.
Left: search_fe.SearchResult{
Test: testAlpha,
@@ -2699,6 +2704,62 @@
assertJSONResponseWas(t, http.StatusOK, expectedResponse, w)
}
+func TestDiffHandler_OptOutOfSQLDiffMetrics_NoContextValueAdded(t *testing.T) {
+ unittest.SmallTest(t)
+
+ ms := &mock_search.SearchAPI{}
+
+ const testAlpha = types.TestName("alpha")
+ const leftDigest = types.Digest("11111111111111111111111111111111")
+ const rightDigest = types.Digest("22222222222222222222222222222222")
+ ctxMatcher := mock.MatchedBy(func(ctx context.Context) bool {
+ v := ctx.Value(search.UseSQLDiffMetricsKey)
+ assert.Nil(t, v)
+ return true
+ })
+ ms.On("DiffDigests", ctxMatcher, testAlpha, leftDigest, rightDigest, "", "").Return(&search_fe.DigestComparison{
+ // Arbitrary data from a search unit test.
+ Left: search_fe.SearchResult{
+ Test: testAlpha,
+ Digest: leftDigest,
+ Status: expectations.Untriaged,
+ ParamSet: paramtools.ParamSet{
+ "device": []string{data.BullheadDevice},
+ types.PrimaryKeyField: []string{string(data.AlphaTest)},
+ types.CorpusField: []string{"gm"},
+ "ext": {data.PNGExtension},
+ },
+ },
+ Right: &search_fe.SRDiffDigest{
+ Digest: rightDigest,
+ Status: expectations.Positive,
+ NumDiffPixels: 13,
+ PixelDiffPercent: 0.5,
+ MaxRGBADiffs: [4]int{8, 9, 10, 11},
+ DimDiffer: true,
+ CombinedMetric: 4.2,
+ ParamSet: paramtools.ParamSet{
+ "device": []string{data.AnglerDevice, data.CrosshatchDevice},
+ types.PrimaryKeyField: []string{string(data.AlphaTest)},
+ types.CorpusField: []string{"gm"},
+ "ext": {data.PNGExtension},
+ },
+ },
+ }, nil)
+
+ wh := Handlers{
+ HandlersConfig: HandlersConfig{
+ SearchAPI: ms,
+ },
+ anonymousCheapQuota: rate.NewLimiter(rate.Inf, 1),
+ }
+ w := httptest.NewRecorder()
+ r := httptest.NewRequest(http.MethodGet, "/json/v1/diff?use_sql=false&test=alpha&left=11111111111111111111111111111111&right=22222222222222222222222222222222", nil)
+ wh.DiffHandler(w, r)
+ const expectedResponse = `{"left":{"digest":"11111111111111111111111111111111","test":"alpha","status":"untriaged","triage_history":null,"paramset":{"device":["bullhead"],"ext":["png"],"name":["test_alpha"],"source_type":["gm"]},"traces":{"tileSize":0,"traces":null,"digests":null,"total_digests":0},"refDiffs":null,"closestRef":""},"right":{"numDiffPixels":13,"combinedMetric":4.2,"pixelDiffPercent":0.5,"maxRGBADiffs":[8,9,10,11],"dimDiffer":true,"digest":"22222222222222222222222222222222","status":"positive","paramset":{"device":["angler","crosshatch"],"ext":["png"],"name":["test_alpha"],"source_type":["gm"]},"n":0}}`
+ assertJSONResponseWas(t, http.StatusOK, expectedResponse, w)
+}
+
// Because we are calling our handlers directly, the target URL doesn't matter. The target URL
// would only matter if we were calling into the router, so it knew which handler to call.
const requestURL = "/does/not/matter"
