diff --git a/api/checks/update.go b/api/checks/update.go index 4569301311e..a70481369ee 100644 --- a/api/checks/update.go +++ b/api/checks/update.go @@ -135,6 +135,7 @@ func loadRunsToCompare(ctx context.Context, filter shared.TestRunFilter) ( filter.To, &one, nil, + nil, ) if err != nil { return nil, nil, err @@ -176,6 +177,7 @@ func loadPRRun(ctx context.Context, filter shared.TestRunFilter, extraLabel stri nil, &one, nil, + nil, ) run := runs.First() if err != nil { @@ -199,7 +201,7 @@ func loadMasterRunBefore( one := 1 to := headRun.TimeStart.Add(-time.Millisecond) labels := mapset.NewSetWith(headRun.Channel(), shared.MasterLabel) - runs, err := store.TestRunQuery().LoadTestRuns(filter.Products, labels, nil, nil, &to, &one, nil) + runs, err := store.TestRunQuery().LoadTestRuns(filter.Products, labels, nil, nil, &to, &one, nil, nil) baseRun := runs.First() if err != nil { return nil, err diff --git a/api/checks/webhook.go b/api/checks/webhook.go index 55002cd7dd4..01593ba038d 100644 --- a/api/checks/webhook.go +++ b/api/checks/webhook.go @@ -306,7 +306,7 @@ func scheduleProcessingForExistingRuns(ctx context.Context, sha string, products // Jump straight to completed check_run for already-present runs for the SHA. store := shared.NewAppEngineDatastore(ctx, false) products = shared.ProductSpecs(products).OrDefault() - runsByProduct, err := store.TestRunQuery().LoadTestRuns(products, nil, shared.SHAs{sha}, nil, nil, nil, nil) + runsByProduct, err := store.TestRunQuery().LoadTestRuns(products, nil, shared.SHAs{sha}, nil, nil, nil, nil, nil) if err != nil { return false, fmt.Errorf("Failed to load test runs: %s", err.Error()) } diff --git a/api/query/cache/backfill/backfill.go b/api/query/cache/backfill/backfill.go index c80318cd2db..28954c8c3e8 100644 --- a/api/query/cache/backfill/backfill.go +++ b/api/query/cache/backfill/backfill.go @@ -118,7 +118,7 @@ func FillIndex( func startBackfillMonitor(store shared.Datastore, logger shared.Logger, maxBytes uint64, m *backfillMonitor) error { // FetchRuns will return at most N runs for each product, so divide the upper bound by the number of products. limit := int(maxBytes/bytesPerRun) / len(shared.GetDefaultProducts()) - runsByProduct, err := store.TestRunQuery().LoadTestRuns(shared.GetDefaultProducts(), nil, nil, nil, nil, &limit, nil) + runsByProduct, err := store.TestRunQuery().LoadTestRuns(shared.GetDefaultProducts(), nil, nil, nil, nil, &limit, nil, nil) if err != nil { return err } diff --git a/api/query/cache/backfill/backfill_medium_test.go b/api/query/cache/backfill/backfill_medium_test.go index 2b6b6119d60..11816ea212b 100644 --- a/api/query/cache/backfill/backfill_medium_test.go +++ b/api/query/cache/backfill/backfill_medium_test.go @@ -60,7 +60,7 @@ func TestStopImmediately(t *testing.T) { query := sharedtest.NewMockTestRunQuery(ctrl) product, _ := shared.ParseProductSpec("chrome") store.EXPECT().TestRunQuery().Return(query) - query.EXPECT().LoadTestRuns(gomock.Any(), nil, nil, nil, nil, gomock.Any(), nil).Return(shared.TestRunsByProduct{ + query.EXPECT().LoadTestRuns(gomock.Any(), nil, nil, nil, nil, gomock.Any(), nil, nil).Return(shared.TestRunsByProduct{ shared.ProductTestRuns{Product: product, TestRuns: shared.TestRuns{ shared.TestRun{ID: 1}, shared.TestRun{ID: 2}, @@ -89,7 +89,7 @@ func TestIngestSomeRuns(t *testing.T) { query := sharedtest.NewMockTestRunQuery(ctrl) product, _ := shared.ParseProductSpec("chrome") store.EXPECT().TestRunQuery().Return(query) - query.EXPECT().LoadTestRuns(gomock.Any(), nil, nil, nil, nil, gomock.Any(), nil).Return(shared.TestRunsByProduct{ + query.EXPECT().LoadTestRuns(gomock.Any(), nil, nil, nil, nil, gomock.Any(), nil, nil).Return(shared.TestRunsByProduct{ shared.ProductTestRuns{ Product: product, TestRuns: shared.TestRuns{ diff --git a/api/query/cache/backfill/backfill_test.go b/api/query/cache/backfill/backfill_test.go index 2ea3bd723e8..483e95169fe 100644 --- a/api/query/cache/backfill/backfill_test.go +++ b/api/query/cache/backfill/backfill_test.go @@ -33,7 +33,7 @@ func TestFetchErr(t *testing.T) { idx := index.NewMockIndex(ctrl) expected := errors.New("Fetch error") store.EXPECT().TestRunQuery().Return(query) - query.EXPECT().LoadTestRuns(gomock.Any(), nil, nil, nil, nil, gomock.Any(), nil).Return(nil, expected) + query.EXPECT().LoadTestRuns(gomock.Any(), nil, nil, nil, nil, gomock.Any(), nil, nil).Return(nil, expected) _, err := FillIndex(store, nil, nil, 1, uint(10), uint64(1), 0.0, idx) assert.Equal(t, expected, err) } diff --git a/api/query/cache/poll/poll.go b/api/query/cache/poll/poll.go index 47dfdc4e6df..b0a50816aa1 100644 --- a/api/query/cache/poll/poll.go +++ b/api/query/cache/poll/poll.go @@ -32,7 +32,7 @@ func KeepRunsUpdated(store shared.Datastore, logger shared.Logger, interval time for { start := time.Now() - runs, err := store.TestRunQuery().LoadTestRuns(shared.GetDefaultProducts(), nil, nil, nil, nil, &limit, nil) + runs, err := store.TestRunQuery().LoadTestRuns(shared.GetDefaultProducts(), nil, nil, nil, nil, &limit, nil, nil) if err != nil { logger.Errorf("Error fetching runs for update: %v", err) wait(start, interval) diff --git a/api/query/query.go b/api/query/query.go index f36a0cb2999..695b0fe7b10 100644 --- a/api/query/query.go +++ b/api/query/query.go @@ -106,7 +106,7 @@ func (qh queryHandler) getRunsAndFilters(in shared.QueryFilter) (shared.TestRuns limit := 1 products := runFilters.GetProductsOrDefault() runsByProduct, err := qh.store.TestRunQuery().LoadTestRuns( - products, runFilters.Labels, []string{sha}, runFilters.From, runFilters.To, &limit, nil) + products, runFilters.Labels, []string{sha}, runFilters.From, runFilters.To, &limit, nil, nil) if err != nil { return testRuns, filters, err } diff --git a/api/query/query_test.go b/api/query/query_test.go index d2335f920c4..6f4de5da861 100644 --- a/api/query/query_test.go +++ b/api/query/query_test.go @@ -171,7 +171,7 @@ func TestGetRunsAndFilters_default(t *testing.T) { } filters := shared.QueryFilter{} - mockQuery.EXPECT().LoadTestRuns(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(testRuns, nil) + mockQuery.EXPECT().LoadTestRuns(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), nil).Return(testRuns, nil) trs, fs, err := sh.getRunsAndFilters(filters) assert.Nil(t, err) diff --git a/api/results_redirect_handler.go b/api/results_redirect_handler.go index 6cd5061ec85..2084466d895 100644 --- a/api/results_redirect_handler.go +++ b/api/results_redirect_handler.go @@ -33,7 +33,7 @@ func apiResultsRedirectHandler(w http.ResponseWriter, r *http.Request) { store := shared.NewAppEngineDatastore(ctx, true) one := 1 testRuns, err := store.TestRunQuery().LoadTestRuns( - filters.Products, filters.Labels, filters.SHAs, nil, nil, &one, nil) + filters.Products, filters.Labels, filters.SHAs, nil, nil, &one, nil, nil) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) diff --git a/api/shas.go b/api/shas.go index e2bbe93391d..f283c5518a3 100644 --- a/api/shas.go +++ b/api/shas.go @@ -70,6 +70,7 @@ func (h SHAsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { filters.To, filters.MaxCount, filters.Offset, + nil, ) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) diff --git a/api/test_run.go b/api/test_run.go index 522a03a8ad8..26997c44ec5 100644 --- a/api/test_run.go +++ b/api/test_run.go @@ -74,6 +74,7 @@ func apiTestRunHandler(w http.ResponseWriter, r *http.Request) { nil, &one, nil, + nil, ) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) diff --git a/api/test_runs.go b/api/test_runs.go index ef08c06824a..af558375429 100644 --- a/api/test_runs.go +++ b/api/test_runs.go @@ -146,7 +146,7 @@ func LoadTestRunKeysForFilters(store shared.Datastore, filters shared.TestRunFil return keys, err } - return q.LoadTestRunKeys(products, filters.Labels, filters.SHAs, from, filters.To, limit, offset) + return q.LoadTestRunKeys(products, filters.Labels, filters.SHAs, from, filters.To, limit, offset, filters.QueryOpts) } // LoadTestRunsForFilters deciphers the filters and executes a corresponding query to load diff --git a/shared/params.go b/shared/params.go index 3f45e87f16c..dc76fde3204 100644 --- a/shared/params.go +++ b/shared/params.go @@ -339,6 +339,38 @@ func ParseViewParam(v url.Values) (*string, error) { return nil, nil } +// QueryOptions is a container for pre-set query options. +type QueryOptions struct { + ExcludeBadRanges bool +} + +// AppendToURLValues appends the current options in QueryOptions into the +// given query parameters. +func (o QueryOptions) AppendToURLValues(v *url.Values) { + if o.ExcludeBadRanges { + v.Add("option", "no-bad-ranges") + } +} + +// ParseOptionParam parses the 'option' parameter and ensures it is a valid value. +func ParseOptionParam(v url.Values) (*QueryOptions, error) { + optionParams, found := v["option"] + if !found { + return nil, nil + } + queryOptions := new(QueryOptions) + for _, param := range optionParams { + switch strings.ToLower(param) { + case "no-bad-ranges": + queryOptions.ExcludeBadRanges = true + default: + return nil, fmt.Errorf("unknown 'option' parameter: %s", param) + } + } + + return queryOptions, nil +} + // ParseDateTimeParam flexibly parses a date/time param with the given name as a time.Time. func ParseDateTimeParam(v url.Values, name string) (*time.Time, error) { if fromParam := v.Get(name); fromParam != "" { @@ -595,6 +627,9 @@ func ParseTestRunFilterParams(v url.Values) (filter TestRunFilter, err error) { if filter.View, err = ParseViewParam(v); err != nil { return filter, err } + if filter.QueryOpts, err = ParseOptionParam(v); err != nil { + return filter, err + } return filter, nil } diff --git a/shared/params_test.go b/shared/params_test.go index 7104c464aa9..5899aaad39d 100644 --- a/shared/params_test.go +++ b/shared/params_test.go @@ -1,3 +1,4 @@ +//go:build small // +build small // Copyright 2017 The WPT Dashboard Project. All rights reserved. @@ -8,6 +9,7 @@ package shared import ( "bytes" + "errors" "fmt" "io/ioutil" "net/http" @@ -649,6 +651,10 @@ func TestParseTestRunFilterParams(t *testing.T) { r = httptest.NewRequest("GET", "http://wpt.fyi/?from=2018-01-01T00%3A00%3A00Z", nil) filter, _ = ParseTestRunFilterParams(r.URL.Query()) assert.Equal(t, "from=2018-01-01T00%3A00%3A00Z", filter.ToQuery().Encode()) + + r = httptest.NewRequest("GET", "http://wpt.fyi/?option=no-bad-ranges", nil) + filter, _ = ParseTestRunFilterParams(r.URL.Query()) + assert.Equal(t, "option=no-bad-ranges", filter.ToQuery().Encode()) } func TestParseTestRunFilterParams_Invalid(t *testing.T) { @@ -798,3 +804,55 @@ func TestParseTestRunFilterParams_Page(t *testing.T) { _, err := ParseTestRunFilterParams(values) assert.NotNil(t, err) } + +func TestParseOptionParams_NoBadRanges(t *testing.T) { + values := make(url.Values) + values.Set("option", "no-bad-ranges") + opt, err := ParseOptionParam(values) + assert.NoError(t, err) + assert.Equal(t, &QueryOptions{ + ExcludeBadRanges: true, + }, opt) +} + +func TestParseOptionParams_InvalidOption(t *testing.T) { + values := make(url.Values) + values.Set("option", "bad") + opt, err := ParseOptionParam(values) + assert.Equal(t, errors.New("unknown 'option' parameter: bad"), err) + assert.Nil(t, opt) +} + +func TestQueryOptionAppendToURLValues(t *testing.T) { + testCases := []struct { + name string + inputURLValues url.Values + options QueryOptions + expectedURLValues url.Values + }{ + { + name: "does not add anything", + inputURLValues: map[string][]string{}, + options: QueryOptions{}, + expectedURLValues: map[string][]string{}, + }, + { + name: "adds no-bad-ranges", + inputURLValues: map[string][]string{}, + options: QueryOptions{ + ExcludeBadRanges: true, + }, + expectedURLValues: map[string][]string{ + "option": { + "no-bad-ranges", + }, + }, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + tc.options.AppendToURLValues(&tc.inputURLValues) + assert.Equal(t, tc.expectedURLValues, tc.inputURLValues) + }) + } +} diff --git a/shared/run_diff.go b/shared/run_diff.go index e23167440d8..a65ef08a32e 100644 --- a/shared/run_diff.go +++ b/shared/run_diff.go @@ -310,7 +310,7 @@ func FetchRunForSpec(ctx context.Context, spec ProductSpec) (*TestRun, error) { one := 1 store := NewAppEngineDatastore(ctx, true) q := store.TestRunQuery() - testRuns, err := q.LoadTestRuns(ProductSpecs{spec}, nil, SHAs{spec.Revision}, nil, nil, &one, nil) + testRuns, err := q.LoadTestRuns(ProductSpecs{spec}, nil, SHAs{spec.Revision}, nil, nil, &one, nil, nil) if err != nil { return nil, err } diff --git a/shared/sharedtest/test_run_query_mock.go b/shared/sharedtest/test_run_query_mock.go index e545b4f3914..0d44908f78d 100644 --- a/shared/sharedtest/test_run_query_mock.go +++ b/shared/sharedtest/test_run_query_mock.go @@ -53,33 +53,33 @@ func (mr *MockTestRunQueryMockRecorder) GetAlignedRunSHAs(arg0, arg1, arg2, arg3 } // LoadTestRunKeys mocks base method. -func (m *MockTestRunQuery) LoadTestRunKeys(arg0 []shared.ProductSpec, arg1 mapset.Set, arg2 []string, arg3, arg4 *time.Time, arg5, arg6 *int) (shared.KeysByProduct, error) { +func (m *MockTestRunQuery) LoadTestRunKeys(arg0 []shared.ProductSpec, arg1 mapset.Set, arg2 []string, arg3, arg4 *time.Time, arg5, arg6 *int, arg7 *shared.QueryOptions) (shared.KeysByProduct, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "LoadTestRunKeys", arg0, arg1, arg2, arg3, arg4, arg5, arg6) + ret := m.ctrl.Call(m, "LoadTestRunKeys", arg0, arg1, arg2, arg3, arg4, arg5, arg6, arg7) ret0, _ := ret[0].(shared.KeysByProduct) ret1, _ := ret[1].(error) return ret0, ret1 } // LoadTestRunKeys indicates an expected call of LoadTestRunKeys. -func (mr *MockTestRunQueryMockRecorder) LoadTestRunKeys(arg0, arg1, arg2, arg3, arg4, arg5, arg6 interface{}) *gomock.Call { +func (mr *MockTestRunQueryMockRecorder) LoadTestRunKeys(arg0, arg1, arg2, arg3, arg4, arg5, arg6, arg7 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LoadTestRunKeys", reflect.TypeOf((*MockTestRunQuery)(nil).LoadTestRunKeys), arg0, arg1, arg2, arg3, arg4, arg5, arg6) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LoadTestRunKeys", reflect.TypeOf((*MockTestRunQuery)(nil).LoadTestRunKeys), arg0, arg1, arg2, arg3, arg4, arg5, arg6, arg7) } // LoadTestRuns mocks base method. -func (m *MockTestRunQuery) LoadTestRuns(arg0 []shared.ProductSpec, arg1 mapset.Set, arg2 []string, arg3, arg4 *time.Time, arg5, arg6 *int) (shared.TestRunsByProduct, error) { +func (m *MockTestRunQuery) LoadTestRuns(arg0 []shared.ProductSpec, arg1 mapset.Set, arg2 []string, arg3, arg4 *time.Time, arg5, arg6 *int, arg7 *shared.QueryOptions) (shared.TestRunsByProduct, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "LoadTestRuns", arg0, arg1, arg2, arg3, arg4, arg5, arg6) + ret := m.ctrl.Call(m, "LoadTestRuns", arg0, arg1, arg2, arg3, arg4, arg5, arg6, arg7) ret0, _ := ret[0].(shared.TestRunsByProduct) ret1, _ := ret[1].(error) return ret0, ret1 } // LoadTestRuns indicates an expected call of LoadTestRuns. -func (mr *MockTestRunQueryMockRecorder) LoadTestRuns(arg0, arg1, arg2, arg3, arg4, arg5, arg6 interface{}) *gomock.Call { +func (mr *MockTestRunQueryMockRecorder) LoadTestRuns(arg0, arg1, arg2, arg3, arg4, arg5, arg6, arg7 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LoadTestRuns", reflect.TypeOf((*MockTestRunQuery)(nil).LoadTestRuns), arg0, arg1, arg2, arg3, arg4, arg5, arg6) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LoadTestRuns", reflect.TypeOf((*MockTestRunQuery)(nil).LoadTestRuns), arg0, arg1, arg2, arg3, arg4, arg5, arg6, arg7) } // LoadTestRunsByKeys mocks base method. diff --git a/shared/test_bad_ranges.go b/shared/test_bad_ranges.go new file mode 100644 index 00000000000..ffcd44636d6 --- /dev/null +++ b/shared/test_bad_ranges.go @@ -0,0 +1,120 @@ +package shared + +import "time" + +// InvalidTestRange contains the date information for when a test range should not be included. +type InvalidTestRange struct { + // start is the beginning of when the test run should be excluded. + // The value is inclusive of the range. + start time.Time + // end is the end of when the test run should be excluded. + // The value is exclusive of the range. + end time.Time +} + +// IsWithinRange determines if the input is within the range. +// It logic for the method is: +// +// start <= input < end +// +// This is similar to the existing logic in advanceDateToSkipBadDataIfNecessary [1]. +// [1] https://github.com/web-platform-tests/results-analysis/blob/bb5c86533956a65a506cd0c7202ab6fe1bf1d67f/bad-ranges.js +func (r InvalidTestRange) IsWithinRange(input time.Time) bool { + return !input.Before(r.start) && input.Before(r.end) +} + +var stableBadRanges = []InvalidTestRange{ + // This was some form of Safari outage, undiagnosed but a clear erroneous + // spike in failure rates. + { + start: time.Date(2019, time.February, 6, 0, 0, 0, 0, time.UTC), + end: time.Date(2019, time.March, 4, 0, 0, 0, 0, time.UTC), + }, + // This was a safaridriver outage, resolved by + // https://github.com/web-platform-tests/wpt/pull/18585 + { + start: time.Date(2019, time.June, 27, 0, 0, 0, 0, time.UTC), + end: time.Date(2019, time.August, 23, 0, 0, 0, 0, time.UTC), + }, + // This was a general outage due to the Taskcluster Checks migration. + { + start: time.Date(2020, time.July, 8, 0, 0, 0, 0, time.UTC), + end: time.Date(2020, time.July, 16, 0, 0, 0, 0, time.UTC), + }, + // This was a Firefox outage which produced only partial test results. + { + start: time.Date(2020, time.July, 21, 0, 0, 0, 0, time.UTC), + end: time.Date(2020, time.August, 15, 0, 0, 0, 0, time.UTC), + }, + // This was a regression from https://github.com/web-platform-tests/wpt/pull/29089, + // fixed by https://github.com/web-platform-tests/wpt/pull/32540 + { + start: time.Date(2022, time.January, 25, 0, 0, 0, 0, time.UTC), + end: time.Date(2022, time.January, 27, 0, 0, 0, 0, time.UTC), + }, + // This was a very much incomplete Safari run. + { + start: time.Date(2023, time.July, 17, 0, 0, 0, 0, time.UTC), + end: time.Date(2023, time.July, 18, 0, 0, 0, 0, time.UTC), + }, + // Safari got a lot of broken screenshots. + // https://bugs.webkit.org/show_bug.cgi?id=262078 + { + start: time.Date(2023, time.September, 20, 0, 0, 0, 0, time.UTC), + end: time.Date(2023, time.September, 21, 0, 0, 0, 0, time.UTC), + }, +} + +var experimentalBadRanges = []InvalidTestRange{ + // This was a safaridriver outage, resolved by + // https://github.com/web-platform-tests/wpt/pull/18585 + { + start: time.Date(2019, time.June, 27, 0, 0, 0, 0, time.UTC), + end: time.Date(2019, time.August, 23, 0, 0, 0, 0, time.UTC), + }, + // Bad Firefox run: + // https://wpt.fyi/results/?diff&filter=ADC&run_id=387040002&run_id=404070001 + { + start: time.Date(2019, time.December, 25, 0, 0, 0, 0, time.UTC), + end: time.Date(2019, time.December, 26, 0, 0, 0, 0, time.UTC), + }, + // This was a general outage due to the Taskcluster Checks migration. + { + start: time.Date(2020, time.July, 8, 0, 0, 0, 0, time.UTC), + end: time.Date(2020, time.July, 16, 0, 0, 0, 0, time.UTC), + }, + // Bad Chrome run: + // https://wpt.fyi/results/?diff&filter=ADC&run_id=622910001&run_id=634430001 + { + start: time.Date(2020, time.July, 31, 0, 0, 0, 0, time.UTC), + end: time.Date(2020, time.August, 1, 0, 0, 0, 0, time.UTC), + }, + // Something went wrong with the Firefox run on this date. + { + start: time.Date(2021, time.March, 8, 0, 0, 0, 0, time.UTC), + end: time.Date(2021, time.March, 9, 0, 0, 0, 0, time.UTC), + }, + // This was a regression from https://github.com/web-platform-tests/wpt/pull/29089, + // fixed by https://github.com/web-platform-tests/wpt/pull/32540 + { + start: time.Date(2022, time.January, 25, 0, 0, 0, 0, time.UTC), + end: time.Date(2022, time.January, 27, 0, 0, 0, 0, time.UTC), + }, + // These were very much incomplete Safari runs. + { + start: time.Date(2023, time.September, 2, 0, 0, 0, 0, time.UTC), + end: time.Date(2023, time.September, 3, 0, 0, 0, 0, time.UTC), + }, + { + start: time.Date(2023, time.September, 11, 0, 0, 0, 0, time.UTC), + end: time.Date(2023, time.September, 12, 0, 0, 0, 0, time.UTC), + }, + { + start: time.Date(2023, time.September, 20, 0, 0, 0, 0, time.UTC), + end: time.Date(2023, time.September, 21, 0, 0, 0, 0, time.UTC), + }, + { + start: time.Date(2023, time.September, 22, 0, 0, 0, 0, time.UTC), + end: time.Date(2023, time.September, 23, 0, 0, 0, 0, time.UTC), + }, +} diff --git a/shared/test_bad_ranges_test.go b/shared/test_bad_ranges_test.go new file mode 100644 index 00000000000..bfe4f0da02b --- /dev/null +++ b/shared/test_bad_ranges_test.go @@ -0,0 +1,86 @@ +//go:build small +// +build small + +package shared + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func checkStartTimeBeforeEndTime(t *testing.T, input []InvalidTestRange) { + // A range is valid if: + // - the start time is less than (and not equal to) the end. + for _, testRange := range input { + assert.Less(t, testRange.start, testRange.end) + } + +} + +func TestValidateRanges(t *testing.T) { + // Ensure the values in the range are valid ranges. + checkStartTimeBeforeEndTime(t, stableBadRanges) + checkStartTimeBeforeEndTime(t, experimentalBadRanges) +} + +func TestIsWithinRange(t *testing.T) { + testCases := []struct { + name string + testRange InvalidTestRange + input time.Time + expectedResult bool + }{ + { + name: "before start", + testRange: InvalidTestRange{ + start: time.Date(2024, time.January, 1, 0, 0, 0, 0, time.UTC), + end: time.Date(2024, time.January, 2, 0, 0, 0, 0, time.UTC), + }, + input: time.Date(2023, time.December, 1, 0, 0, 0, 0, time.UTC), + expectedResult: false, + }, + { + name: "after end", + testRange: InvalidTestRange{ + start: time.Date(2024, time.January, 1, 0, 0, 0, 0, time.UTC), + end: time.Date(2024, time.January, 2, 0, 0, 0, 0, time.UTC), + }, + input: time.Date(2024, time.January, 3, 0, 0, 0, 0, time.UTC), + expectedResult: false, + }, + { + name: "within range", + testRange: InvalidTestRange{ + start: time.Date(2024, time.January, 1, 0, 0, 0, 0, time.UTC), + end: time.Date(2024, time.January, 2, 0, 0, 0, 0, time.UTC), + }, + input: time.Date(2024, time.January, 1, 1, 0, 0, 0, time.UTC), + expectedResult: true, + }, + { + name: "edge of start (which is inclusive)", + testRange: InvalidTestRange{ + start: time.Date(2024, time.January, 1, 0, 0, 0, 0, time.UTC), + end: time.Date(2024, time.January, 2, 0, 0, 0, 0, time.UTC), + }, + input: time.Date(2024, time.January, 1, 0, 0, 0, 0, time.UTC), + expectedResult: true, + }, + { + name: "edge of end (which is exclusive)", + testRange: InvalidTestRange{ + start: time.Date(2024, time.January, 1, 0, 0, 0, 0, time.UTC), + end: time.Date(2024, time.January, 2, 0, 0, 0, 0, time.UTC), + }, + input: time.Date(2024, time.January, 2, 0, 0, 0, 0, time.UTC), + expectedResult: false, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.expectedResult, tc.testRange.IsWithinRange(tc.input)) + }) + } +} diff --git a/shared/test_run_filter.go b/shared/test_run_filter.go index 7098e05bd3a..cd8f9c487e8 100644 --- a/shared/test_run_filter.go +++ b/shared/test_run_filter.go @@ -44,15 +44,16 @@ func (s SHAs) ShortSHAs() []string { // TestRunFilter represents the ways TestRun entities can be filtered in // the webapp and api. type TestRunFilter struct { - SHAs SHAs `json:"shas,omitempty"` - Labels mapset.Set `json:"labels,omitempty"` - Aligned *bool `json:"aligned,omitempty"` - From *time.Time `json:"from,omitempty"` - To *time.Time `json:"to,omitempty"` - MaxCount *int `json:"maxcount,omitempty"` - Offset *int `json:"offset,omitempty"` // Used for paginating with MaxCount. - Products ProductSpecs `json:"products,omitempty"` - View *string `json:"view,omitempty"` + SHAs SHAs `json:"shas,omitempty"` + Labels mapset.Set `json:"labels,omitempty"` + Aligned *bool `json:"aligned,omitempty"` + From *time.Time `json:"from,omitempty"` + To *time.Time `json:"to,omitempty"` + MaxCount *int `json:"maxcount,omitempty"` + Offset *int `json:"offset,omitempty"` // Used for paginating with MaxCount. + Products ProductSpecs `json:"products,omitempty"` + View *string `json:"view,omitempty"` + QueryOpts *QueryOptions `json:"option,omitempty"` } type testRunFilterNoCustomMarshalling TestRunFilter @@ -90,7 +91,8 @@ func (filter TestRunFilter) IsDefaultQuery() bool { (filter.From == nil) && (filter.MaxCount == nil || *filter.MaxCount == 1) && (len(filter.Products) < 1) && - (filter.View == nil) + (filter.View == nil) && + (filter.QueryOpts == nil) } // OrDefault returns the current filter, or, if it is a default query, returns @@ -196,6 +198,9 @@ func (filter TestRunFilter) ToQuery() (q url.Values) { if filter.View != nil { q.Set("view", *filter.View) } + if filter.QueryOpts != nil { + filter.QueryOpts.AppendToURLValues(&q) + } return q } diff --git a/shared/test_run_filter_test.go b/shared/test_run_filter_test.go index 7e94c37a87e..2636626c0e9 100644 --- a/shared/test_run_filter_test.go +++ b/shared/test_run_filter_test.go @@ -1,3 +1,4 @@ +//go:build small // +build small // Copyright 2018 The WPT Dashboard Project. All rights reserved. @@ -98,14 +99,65 @@ func TestTestRunFilter_NextPage_FromAndMax(t *testing.T) { }, nextPage) } +func TestTestRunFilter_NextPage_FromAndMax_QueryOptions(t *testing.T) { + // Use UTC to avoid DST craziness. + now := time.Now().UTC() + aWeekAgo := now.AddDate(0, 0, -7) + oneHundred := 100 + options := QueryOptions{ + ExcludeBadRanges: true, + } + // Edge-case: We ask for N runs after a timestamp, but < N runs occurred in + // that time range. This should return the earlier time range. + filter := TestRunFilter{ + From: &aWeekAgo, + To: &now, + MaxCount: &oneHundred, + QueryOpts: &options, + } + chrome, _ := ParseProductSpec("chrome") + loadedRuns := TestRunsByProduct{ + ProductTestRuns{ + Product: chrome, + TestRuns: make(TestRuns, 1), + }, + } + twoWeeksAgo := aWeekAgo.AddDate(0, 0, -7) + aWeekAgoMinusAMilli := aWeekAgo.Add(-time.Millisecond) + nextPage := filter.NextPage(loadedRuns) + assert.Equal(t, &TestRunFilter{ + From: &twoWeeksAgo, + To: &aWeekAgoMinusAMilli, + MaxCount: &oneHundred, + QueryOpts: &options, + }, nextPage) + + // Common case: We ask for N runs after a timestamp, and N runs are returned. + // This should return the next N in the same time range. + one := 1 + filter.MaxCount = &one + nextPage = filter.NextPage(loadedRuns) + assert.Equal(t, &TestRunFilter{ + From: &aWeekAgo, + To: &now, + MaxCount: &one, + Offset: &one, + QueryOpts: &options, + }, nextPage) +} + func TestTestRunFilter_JSONRoundTrip(t *testing.T) { one := 1 chrome, _ := ParseProductSpec("chrome[experimental]") + options := QueryOptions{ + ExcludeBadRanges: true, + } page := TestRunFilter{ - MaxCount: &one, - Offset: &one, - Labels: mapset.NewSet(MasterLabel), - Products: ProductSpecs{chrome}, + MaxCount: &one, + Offset: &one, + Labels: mapset.NewSet(MasterLabel), + Products: ProductSpecs{chrome}, + QueryOpts: &options, } // Test a JSON roundtrip. @@ -117,4 +169,5 @@ func TestTestRunFilter_JSONRoundTrip(t *testing.T) { assert.EqualValues(t, &one, jsonRoundTrip.MaxCount) assert.EqualValues(t, &one, jsonRoundTrip.Offset) assert.Contains(t, ToStringSlice(jsonRoundTrip.Labels), MasterLabel) + assert.Equal(t, options, *jsonRoundTrip.QueryOpts) } diff --git a/shared/test_run_query.go b/shared/test_run_query.go index 45584007eaa..44b1248ecce 100644 --- a/shared/test_run_query.go +++ b/shared/test_run_query.go @@ -30,7 +30,8 @@ type TestRunQuery interface { from *time.Time, to *time.Time, limit, - offset *int) (result TestRunsByProduct, err error) + offset *int, + queryOpts *QueryOptions) (result TestRunsByProduct, err error) // LoadTestRunKeys loads the keys for the TestRun entities for the given parameters. // It is encapsulated because we cannot run single queries with multiple inequality @@ -42,7 +43,8 @@ type TestRunQuery interface { from *time.Time, to *time.Time, limit *int, - offset *int) (result KeysByProduct, err error) + offset *int, + queryOpts *QueryOptions) (result KeysByProduct, err error) // LoadTestRunsByKeys loads test runs by keys and sets their IDs. LoadTestRunsByKeys(KeysByProduct) (result TestRunsByProduct, err error) @@ -76,12 +78,13 @@ func (t testRunQueryImpl) LoadTestRuns( from *time.Time, to *time.Time, limit, - offset *int) (result TestRunsByProduct, err error) { + offset *int, + queryOpts *QueryOptions) (result TestRunsByProduct, err error) { if len(products) == 0 { return nil, errNoProducts } - keys, err := t.LoadTestRunKeys(products, labels, revisions, from, to, limit, offset) + keys, err := t.LoadTestRunKeys(products, labels, revisions, from, to, limit, offset, queryOpts) if err != nil { return nil, err } @@ -115,7 +118,8 @@ func (t testRunQueryImpl) LoadTestRunKeys( from *time.Time, to *time.Time, limit *int, - offset *int) (result KeysByProduct, err error) { + offset *int, + queryOpts *QueryOptions) (result KeysByProduct, err error) { log := GetLogger(t.store.Context()) result = make(KeysByProduct, len(products)) baseQuery := t.store.NewQuery("TestRun") diff --git a/shared/test_run_query_medium_test.go b/shared/test_run_query_medium_test.go index 91740a2401a..6ca25387e9d 100644 --- a/shared/test_run_query_medium_test.go +++ b/shared/test_run_query_medium_test.go @@ -1,3 +1,4 @@ +//go:build medium // +build medium package shared_test @@ -37,7 +38,7 @@ func TestLoadTestRuns(t *testing.T) { key, _ = store.Put(key, &testRun) chrome, _ := shared.ParseProductSpec("chrome") - loaded, err := store.TestRunQuery().LoadTestRuns(shared.ProductSpecs{chrome}, nil, nil, nil, nil, nil, nil) + loaded, err := store.TestRunQuery().LoadTestRuns(shared.ProductSpecs{chrome}, nil, nil, nil, nil, nil, nil, nil) allRuns := loaded.AllRuns() assert.Nil(t, err) assert.Equal(t, 1, len(allRuns)) @@ -65,7 +66,7 @@ func TestLoadTestRunsBySHAs(t *testing.T) { } q := store.TestRunQuery() - runsByProduct, err := q.LoadTestRuns(shared.GetDefaultProducts(), nil, shared.SHAs{"1111111111", "333333333"}, nil, nil, nil, nil) + runsByProduct, err := q.LoadTestRuns(shared.GetDefaultProducts(), nil, shared.SHAs{"1111111111", "333333333"}, nil, nil, nil, nil, nil) runs := runsByProduct.AllRuns() assert.Nil(t, err) assert.Len(t, runs, 2) @@ -75,7 +76,7 @@ func TestLoadTestRunsBySHAs(t *testing.T) { assert.Equal(t, "1111111111", runs[0].Revision) assert.Equal(t, "3333333333", runs[1].Revision) - runsByProduct, err = q.LoadTestRuns(shared.GetDefaultProducts(), nil, shared.SHAs{"11111", "33333"}, nil, nil, nil, nil) + runsByProduct, err = q.LoadTestRuns(shared.GetDefaultProducts(), nil, shared.SHAs{"11111", "33333"}, nil, nil, nil, nil, nil) runs = runsByProduct.AllRuns() assert.Nil(t, err) assert.Len(t, runs, 2) @@ -161,7 +162,7 @@ func TestLoadTestRuns_Experimental_Only(t *testing.T) { labels := mapset.NewSet() labels.Add("experimental") ten := 10 - loaded, err := store.TestRunQuery().LoadTestRuns(products, labels, nil, nil, nil, &ten, nil) + loaded, err := store.TestRunQuery().LoadTestRuns(products, labels, nil, nil, nil, &ten, nil, nil) allRuns := loaded.AllRuns() assert.Nil(t, err) assert.Equal(t, 2, len(allRuns)) @@ -203,7 +204,7 @@ func TestLoadTestRuns_LabelinProductSpec(t *testing.T) { products := make([]shared.ProductSpec, 1) products[0].BrowserName = "chrome" products[0].Labels = mapset.NewSetWith("foo") - loaded, err := store.TestRunQuery().LoadTestRuns(products, nil, nil, nil, nil, nil, nil) + loaded, err := store.TestRunQuery().LoadTestRuns(products, nil, nil, nil, nil, nil, nil, nil) allRuns := loaded.AllRuns() assert.Nil(t, err) assert.Equal(t, 1, len(allRuns)) @@ -246,7 +247,7 @@ func TestLoadTestRuns_SHAinProductSpec(t *testing.T) { products := make([]shared.ProductSpec, 1) products[0].BrowserName = "chrome" products[0].Revision = strings.Repeat("1", 10) - loaded, err := store.TestRunQuery().LoadTestRuns(products, nil, nil, nil, nil, nil, nil) + loaded, err := store.TestRunQuery().LoadTestRuns(products, nil, nil, nil, nil, nil, nil, nil) assert.Nil(t, err) allRuns := loaded.AllRuns() assert.Equal(t, 1, len(allRuns)) @@ -254,21 +255,21 @@ func TestLoadTestRuns_SHAinProductSpec(t *testing.T) { // Partial SHA products[0].Revision = "11111" - loaded, err = store.TestRunQuery().LoadTestRuns(products, nil, nil, nil, nil, nil, nil) + loaded, err = store.TestRunQuery().LoadTestRuns(products, nil, nil, nil, nil, nil, nil, nil) allRuns = loaded.AllRuns() assert.Equal(t, 1, len(allRuns)) assert.Equal(t, "1111111111", allRuns[0].Revision) // Partial SHA, Browser version products[0].BrowserVersion = "63" - loaded, err = store.TestRunQuery().LoadTestRuns(products, nil, nil, nil, nil, nil, nil) + loaded, err = store.TestRunQuery().LoadTestRuns(products, nil, nil, nil, nil, nil, nil, nil) allRuns = loaded.AllRuns() assert.Equal(t, 1, len(allRuns)) assert.Equal(t, "1111111111", allRuns[0].Revision) // Partial SHA, Exact version products[0].BrowserVersion = "63.1.1.1" - loaded, err = store.TestRunQuery().LoadTestRuns(products, nil, nil, nil, nil, nil, nil) + loaded, err = store.TestRunQuery().LoadTestRuns(products, nil, nil, nil, nil, nil, nil, nil) allRuns = loaded.AllRuns() assert.Equal(t, 1, len(allRuns)) assert.Equal(t, "1111111111", allRuns[0].Revision) @@ -309,7 +310,7 @@ func TestLoadTestRuns_Ordering(t *testing.T) { } chrome, _ := shared.ParseProductSpec("chrome") - loaded, err := store.TestRunQuery().LoadTestRuns(shared.ProductSpecs{chrome}, nil, nil, nil, nil, nil, nil) + loaded, err := store.TestRunQuery().LoadTestRuns(shared.ProductSpecs{chrome}, nil, nil, nil, nil, nil, nil, nil) assert.Nil(t, err) allRuns := loaded.AllRuns() assert.Equal(t, 2, len(allRuns)) @@ -354,7 +355,7 @@ func TestLoadTestRuns_From(t *testing.T) { } chrome, _ := shared.ParseProductSpec("chrome") - loaded, err := store.TestRunQuery().LoadTestRuns(shared.ProductSpecs{chrome}, nil, nil, &yesterday, nil, nil, nil) + loaded, err := store.TestRunQuery().LoadTestRuns(shared.ProductSpecs{chrome}, nil, nil, &yesterday, nil, nil, nil, nil) assert.Nil(t, err) allRuns := loaded.AllRuns() assert.Equal(t, 1, len(allRuns)) @@ -396,7 +397,7 @@ func TestLoadTestRuns_To(t *testing.T) { } chrome, _ := shared.ParseProductSpec("chrome") - loaded, err := store.TestRunQuery().LoadTestRuns(shared.ProductSpecs{chrome}, nil, nil, nil, &now, nil, nil) + loaded, err := store.TestRunQuery().LoadTestRuns(shared.ProductSpecs{chrome}, nil, nil, nil, &now, nil, nil, nil) assert.Nil(t, err) allRuns := loaded.AllRuns() assert.Equal(t, 1, len(allRuns))