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

Add structured search atom for filtering by results that differ #1463

Merged
merged 4 commits into from
Sep 5, 2019
Merged
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
24 changes: 24 additions & 0 deletions api/query/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,19 @@ or, negation,

Where `[product]` is a product specification (e.g. `safari`, `chrome-69`).

### Meta qualities

> BETA: This feature is under development and may change without warning.

Filters the results to values which possess/exhibit a given quality.

is:[quality]

#### `is:different`

Filters to rows where there is more than one resulting status for a test
across the runs.

### And-conjuction

[query1] and [query2] [and ...]
Expand Down Expand Up @@ -173,3 +186,14 @@ Search untriaged issues -
Search triaged issues -

chrome:pass and link:bugs.chromium.org

#### is

`is` query atoms perform a search for tests that possess some meta quality.

{"is": "different"}



See [#meta-qualities](Meta qualities) above for more information on other
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's just different now, but is flaky one you want to add in the future?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep :)

meta qualities than `"different"`.
52 changes: 52 additions & 0 deletions api/query/atoms.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,23 @@ func (l AbstractLink) BindToRuns(runs ...shared.TestRun) ConcreteQuery {
}
}

// MetadataQuality represents the root of an "is" query, which asserts known
// metadata qualities to the results
type MetadataQuality int

const (
// MetadataQualityUnknown is a placeholder for unrecognized values.
MetadataQualityUnknown MetadataQuality = 0
// MetadataQualityDifferent represents an is:different atom.
// "different" ensures that one or more results differs from the other results.
MetadataQualityDifferent MetadataQuality = 1
)

// BindToRuns for MetadataQuality is a no-op; it is independent of test runs.
func (q MetadataQuality) BindToRuns(runs ...shared.TestRun) ConcreteQuery {
return q
}

// TestStatusEq is a query atom that matches tests where the test status/result
// from at least one test run matches the given status value, optionally filtered
// to a specific browser name.
Expand Down Expand Up @@ -656,6 +673,36 @@ func (l *AbstractLink) UnmarshalJSON(b []byte) error {
return nil
}

// UnmarshalJSON for MetadataQuality attempts to interpret a query atom as
// {"is":<metadata quality>}.
func (q *MetadataQuality) UnmarshalJSON(b []byte) error {
var data map[string]*json.RawMessage
err := json.Unmarshal(b, &data)
if err != nil {
return err
}
is, ok := data["is"]
if !ok {
return errors.New(`Missing "is" pattern property: "is"`)
}
var quality string
if err := json.Unmarshal(*is, &quality); err != nil {
return errors.New(`"is" property is not a string`)
}

*q, err = MetadataQualityFromString(quality)
return err
}

// MetadataQualityFromString returns the enum value for the given string.
func MetadataQualityFromString(quality string) (MetadataQuality, error) {
switch quality {
case "different":
return MetadataQualityDifferent, nil
}
return MetadataQualityUnknown, fmt.Errorf(`Unknown "is" quality "%s"`, quality)
}

func unmarshalQ(b []byte) (AbstractQuery, error) {
var tnp TestNamePattern
err := json.Unmarshal(b, &tnp)
Expand Down Expand Up @@ -717,5 +764,10 @@ func unmarshalQ(b []byte) (AbstractQuery, error) {
if err == nil {
return l, nil
}
var i MetadataQuality
err = json.Unmarshal(b, &i)
if err == nil {
return i, nil
}
return nil, errors.New(`Failed to parse query fragment as test name pattern, test status constraint, negation, disjunction, conjunction, sequential or count`)
}
47 changes: 43 additions & 4 deletions api/query/atoms_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,25 @@ func TestStructuredQuery_link(t *testing.T) {
}}, rq)
}

func TestStructuredQuery_is(t *testing.T) {
var rq RunQuery
err := json.Unmarshal([]byte(`{
"run_ids": [0, 1, 2],
"query": {
"exists": [{
"is": "different"
}]
}
}`), &rq)
assert.Nil(t, err)
assert.Equal(t, RunQuery{
RunIDs: []int64{0, 1, 2},
AbstractQuery: AbstractExists{[]AbstractQuery{
MetadataQualityDifferent,
}},
}, rq)
}

func TestStructuredQuery_combinedlink(t *testing.T) {
var rq RunQuery
err := json.Unmarshal([]byte(`{
Expand Down Expand Up @@ -664,8 +683,7 @@ func TestStructuredQuery_bindCount(t *testing.T) {
Where: TestStatusEq{Status: 1},
}

runs := shared.TestRuns{}
runs = shared.TestRuns{
runs := shared.TestRuns{
{
ID: int64(0),
ProductAtRevision: e.ProductAtRevision,
Expand Down Expand Up @@ -696,8 +714,7 @@ func TestStructuredQuery_bindLink(t *testing.T) {
Pattern: "bugs.bar",
}

runs := shared.TestRuns{}
runs = shared.TestRuns{
runs := shared.TestRuns{
{
ID: int64(0),
ProductAtRevision: e.ProductAtRevision,
Expand Down Expand Up @@ -727,6 +744,28 @@ func TestStructuredQuery_bindLink(t *testing.T) {
assert.Equal(t, expect, q.BindToRuns(runs...))
}

func TestStructuredQuery_bindIs(t *testing.T) {
defer func(url string) {
shared.MetadataArchiveURL = url
}(shared.MetadataArchiveURL)

e := shared.ParseProductSpecUnsafe("chrome")
f := shared.ParseProductSpecUnsafe("safari")
q := MetadataQualityDifferent

runs := shared.TestRuns{
{
ID: int64(0),
ProductAtRevision: e.ProductAtRevision,
},
{
ID: int64(1),
ProductAtRevision: f.ProductAtRevision,
},
}
assert.Equal(t, q, q.BindToRuns(runs...))
}

func TestStructuredQuery_bindAnd(t *testing.T) {
p := shared.ParseProductSpecUnsafe("edge")
q := AbstractAnd{
Expand Down
18 changes: 18 additions & 0 deletions api/query/cache/index/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"sync"

log "github.com/Hexcles/logrus"
mapset "github.com/deckarep/golang-set"
"github.com/web-platform-tests/wpt.fyi/api/query"
"github.com/web-platform-tests/wpt.fyi/shared"
)
Expand Down Expand Up @@ -72,6 +73,12 @@ type Link struct {
metadata map[string][]string
}

// MetadataQuality is a query.MetadataQuality bound to an in-memory index.
type MetadataQuality struct {
index
quality query.MetadataQuality
}

// And is a query.And bound to an in-memory index.
type And struct {
index
Expand Down Expand Up @@ -189,6 +196,15 @@ func (l Link) Filter(t TestID) bool {
return false
}

// Filter interprets a MetadataQuality as a filter function over TestIDs.
func (q MetadataQuality) Filter(t TestID) bool {
set := mapset.NewSet()
for _, result := range q.runResults {
set.Add(result.GetResult(t))
}
return set.Cardinality() > 1
}

// Filter interprets an And as a filter function over TestIDs.
func (a And) Filter(t TestID) bool {
args := a.args
Expand Down Expand Up @@ -243,6 +259,8 @@ func newFilter(idx index, q query.ConcreteQuery) (filter, error) {
return Count{idx, v.Count, fs}, nil
case query.Link:
return Link{idx, v.Pattern, v.Metadata}, nil
case query.MetadataQuality:
return MetadataQuality{idx, v}, nil
case query.And:
fs, err := filters(idx, v.Args)
if err != nil {
Expand Down
60 changes: 60 additions & 0 deletions api/query/cache/index/index_filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,66 @@ func TestBindExecute_Link(t *testing.T) {
assert.Equal(t, expectedResult, srs[0])
}

func TestBindExecute_Is(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
loader := NewMockReportLoader(ctrl)
idx, err := NewShardedWPTIndex(loader, testNumShards)
assert.Nil(t, err)

runs := mockTestRuns(loader, idx, []testRunData{
testRunData{
shared.TestRun{ID: 1},
&metrics.TestResultsReport{
Results: []*metrics.TestResults{
&metrics.TestResults{
Test: "/a/b/c",
Status: "PASS",
},
&metrics.TestResults{
Test: "/d/e/f",
Status: "FAIL",
},
},
},
},
testRunData{
shared.TestRun{ID: 2},
&metrics.TestResultsReport{
Results: []*metrics.TestResults{
&metrics.TestResults{
Test: "/a/b/c",
Status: "PASS",
},
&metrics.TestResults{
Test: "/d/e/f",
Status: "PASS",
},
},
},
},
})

quality := query.MetadataQualityDifferent
plan, err := idx.Bind(runs, quality)
assert.Nil(t, err)

res := plan.Execute(runs, query.AggregationOpts{})
srs, ok := res.([]shared.SearchResult)
assert.True(t, ok)

assert.Equal(t, 1, len(srs))
expectedResult := shared.SearchResult{
Test: "/d/e/f", // /a/b/c was the same, /d/e/f differed.
LegacyStatus: []shared.LegacySearchRunResult{
shared.LegacySearchRunResult{Passes: 0, Total: 1},
shared.LegacySearchRunResult{Passes: 1, Total: 1},
},
}

assert.Equal(t, expectedResult, srs[0])
}

func TestBindExecute_LinkNoMatchingPattern(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
Expand Down
6 changes: 6 additions & 0 deletions api/query/concrete_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,12 @@ func (Link) Size() int { return 1 }
// Size of Count is the sum of the sizes of its constituent ConcretQuery instances.
func (c Count) Size() int { return size(c.Args) }

// Size of Is depends on the quality.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are these Size() methods for, what happens if they return the wrong value?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "size" of the query being performed; not sure exactly when it's used. @mdittmer could answer that.

func (q MetadataQuality) Size() int {
// Currently only 'Different' supported, which is one set comparison per row.
return 1
}

// Size of Or is the sum of the sizes of its constituent ConcretQuery instances.
func (o Or) Size() int { return size(o.Args) }

Expand Down
10 changes: 10 additions & 0 deletions webapp/components/test-search.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ const QUERY_GRAMMAR = ohm.grammar(`
Fragment
= not Fragment -- not
| linkExp
| isExp
| statusExp
| subtestExp
| pathExp
Expand All @@ -101,6 +102,9 @@ const QUERY_GRAMMAR = ohm.grammar(`
linkExp
= caseInsensitive<"link"> ":" nameFragment

isExp
= caseInsensitive<"is"> ":" metadataQualityLiteral

patternExp = nameFragment

productSpec = browserName ("-" browserVersion)?
Expand All @@ -113,6 +117,9 @@ const QUERY_GRAMMAR = ohm.grammar(`
statusLiteral
= ${statuses.map(s => 'caseInsensitive<"' + s + '">').join('\n |')}

metadataQualityLiteral
= caseInsensitive<"different">

nameFragment
= basicNameFragment -- basic
| quotemark complexNameFragment quotemark -- quoted
Expand Down Expand Up @@ -222,6 +229,9 @@ const QUERY_SEMANTICS = QUERY_GRAMMAR.createSemantics().addOperation('eval', {
status: {not: r.eval()},
};
},
isExp: (l, colon, r) => {
return { is: r.eval() };
},
subtestExp: (l, colon, r) => {
return { subtest: r.eval() };
},
Expand Down
6 changes: 6 additions & 0 deletions webapp/components/test/test-search.html
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,12 @@
});
});

test('is', () => {
assertQueryParse('is:different', {
exists: [{ is: 'different' }]
});
});

test('count', () => {
assertQueryParse('count:5(status:PASS or status:OK)', {
exists: [{
Expand Down