Skip to content

Commit

Permalink
Add structured search atom for filtering by results that differ (#1463)
Browse files Browse the repository at this point in the history
* Add is:different search atom

* Docs and another test

* Apply suggestions from code review

Co-Authored-By: Philip Jägenstedt <philip@foolip.org>

* Always return 1
  • Loading branch information
lukebjerring authored Sep 5, 2019
1 parent 0891fdb commit af72dd6
Show file tree
Hide file tree
Showing 8 changed files with 219 additions and 4 deletions.
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
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.
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

0 comments on commit af72dd6

Please sign in to comment.