Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jcscottiii/good runs WIP #3677

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion api/checks/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ func loadRunsToCompare(ctx context.Context, filter shared.TestRunFilter) (
filter.To,
&one,
nil,
nil,
)
if err != nil {
return nil, nil, err
Expand Down Expand Up @@ -176,6 +177,7 @@ func loadPRRun(ctx context.Context, filter shared.TestRunFilter, extraLabel stri
nil,
&one,
nil,
nil,
)
run := runs.First()
if err != nil {
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion api/checks/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down
2 changes: 1 addition & 1 deletion api/query/cache/backfill/backfill.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions api/query/cache/backfill/backfill_medium_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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{
Expand Down
2 changes: 1 addition & 1 deletion api/query/cache/backfill/backfill_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
2 changes: 1 addition & 1 deletion api/query/cache/poll/poll.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion api/query/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion api/query/query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion api/results_redirect_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
1 change: 1 addition & 0 deletions api/shas.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions api/test_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion api/test_runs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
35 changes: 35 additions & 0 deletions shared/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 != "" {
Expand Down Expand Up @@ -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
}

Expand Down
58 changes: 58 additions & 0 deletions shared/params_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//go:build small
// +build small

// Copyright 2017 The WPT Dashboard Project. All rights reserved.
Expand All @@ -8,6 +9,7 @@ package shared

import (
"bytes"
"errors"
"fmt"
"io/ioutil"
"net/http"
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
})
}
}
2 changes: 1 addition & 1 deletion shared/run_diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
16 changes: 8 additions & 8 deletions shared/sharedtest/test_run_query_mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading