-
Notifications
You must be signed in to change notification settings - Fork 83
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 all/none root queries #1471
Conversation
} | ||
} | ||
{ | ||
var a AbstractAll |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All / None re-used a/n, and the variables persisting makes for copy-pasta bugs, so I've scoped all the attempts. This (AbstractAll) and AbstractNone are added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thanks for explaining!
|
||
`none` query objects perform a disjunction of all of the runs, in order to ensure | ||
that no single run satisfies all of its queries. `none` queries are a simplification for | ||
`{"not": {"exists": [...] }}` queries. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, makes sense!
for _, run := range runs { | ||
bound := arg.BindToRuns(run) | ||
if _, ok := bound.(True); !ok { // And with True is pointless. | ||
byRun = append(byRun, bound) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What kind of query hits this condition?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a simplification behaviour when binding to runs (e.g. an empty OR becomes false) which this accounts for.
} | ||
} | ||
{ | ||
var a AbstractAll |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thanks for explaining!
@@ -230,6 +230,26 @@ | |||
}); | |||
}); | |||
|
|||
test('all', () => { | |||
assertQueryParse('all(status:!PASS status:!OK)', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good example, I'd like to search for this :)
Force-merging to ignore Travis timeouts. |
Fixes #1467
Description
Adds root query options for
all
andnone
(instead of the default,exists
).Strangely,
count
andseq
get wrapped inexists
. I started to tweak that behaviour, but decided against busying up the PR. Will clean that up another time.