-
Notifications
You must be signed in to change notification settings - Fork 8
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
Proptests for the query
parser
#51
Labels
acceptance: go ahead
Reviewed, implementation can start
area: proptest
Improvements to proptest coverage
help wanted
External contributions welcome
type: reliability
Tests, code quality
Milestone
Comments
V0ldek
added
help wanted
External contributions welcome
acceptance: go ahead
Reviewed, implementation can start
type: reliability
Tests, code quality
labels
Nov 21, 2022
V0ldek
added
area: parser
area: proptest
Improvements to proptest coverage
acceptance: go ahead
Reviewed, implementation can start
and removed
acceptance: go ahead
Reviewed, implementation can start
labels
Nov 23, 2022
3 tasks
V0ldek
added a commit
that referenced
this issue
Apr 10, 2023
Added reliability proptests for query parsers and updated the docs for the grammar. Ref: #51
V0ldek
added a commit
that referenced
this issue
May 24, 2023
V0ldek
added a commit
that referenced
this issue
May 24, 2023
3 tasks
V0ldek
added a commit
that referenced
this issue
May 25, 2023
V0ldek
added a commit
that referenced
this issue
May 25, 2023
github-actions
bot
removed
the
acceptance: go ahead
Reviewed, implementation can start
label
May 25, 2023
Tagging @V0ldek for notifications |
V0ldek
added
acceptance: go ahead
Reviewed, implementation can start
and removed
acceptance: triage
Waiting for owner's input
labels
Aug 7, 2023
We have very good proptests and a fuzzer for valid queries. Adding proptests for error cases might be way too hard and not bring that much benefit. Snapshot testing for errors would be better - at least one test for each error kind in |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
acceptance: go ahead
Reviewed, implementation can start
area: proptest
Improvements to proptest coverage
help wanted
External contributions welcome
type: reliability
Tests, code quality
Is your feature request related to a problem? Please describe.
We should use the
proptest
crate to comprehensively test the query parser.Describe the solution you'd like
As usual for proptests, this is a bit of a complex design space.
It's relatively easy to create proptests for correct query strings – just generate an arbitrary sequence of selectors and stringify them. It might be good to start with that as a single PR first.
Query strings that should result in an error are a bit more tricky. There are many things that can cause a parsing error, and just generating random garbage won't give too good a coverage. We can start by generating a sequence of valid selectors interspersed with "error" segments, and then figuring out smart kinds of "error" – things like three-dot sequences, root selectors inside unescaped labels...
As usual for proptests, we need to do it iteratively. Start with simpler tests and then consequently add more complex cases.
Additional context
I'm not sure what the acceptance criteria for this issue are, so we're just gonna wing it – we'll close it when we feel sufficiently happy with the test suite for something labelled as "1.0.0".
The text was updated successfully, but these errors were encountered: