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 more tests for selectors #90

Merged
merged 3 commits into from
Aug 3, 2024
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
10 changes: 10 additions & 0 deletions tests/basic.json
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,16 @@
"selector": "$[0 2]",
"invalid_selector": true
},
{
"name": "selector, leading comma",
"selector": "$[,0]",
"invalid_selector": true
},
{
"name": "selector, trailing comma",
"selector": "$[0,]",
"invalid_selector": true
},
{
"name": "multiple selectors, name and index, array data",
"selector": "$['a',1]",
Expand Down
48 changes: 48 additions & 0 deletions tests/index_selector.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,34 @@
],
"result": []
},
{
"name": "min exact index",
"selector": "$[-9007199254740991]",
"document": [
"first",
"second"
],
"result": []
},
{
"name": "max exact index",
"selector": "$[9007199254740991]",
"document": [
"first",
"second"
],
"result": []
},
{
"name": "min exact index - 1",
"selector": "$[-9007199254740992]",
"invalid_selector": true
},
{
"name": "max exact index + 1",
"selector": "$[9007199254740992]",
"invalid_selector": true
},
{
"name": "overflowing index",
"selector": "$[231584178474632390847141970017375815706539969331281128078915168015826259279872]",
Expand Down Expand Up @@ -85,6 +113,26 @@
"selector": "$[01]",
"invalid_selector": true
},
{
"name": "decimal",
"selector": "$[1.0]",
"invalid_selector": true
},
{
"name": "plus",
"selector": "$[+1]",
Copy link

Choose a reason for hiding this comment

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

Why is this invalid ? in Java, this is valid.

  private def `int`[_: P]: P[Int] = P("0" | (CharIn("+\\-").? ~ `DIGIT1` ~ `DIGIT`.rep)).!
    .map(_.toLong)
    .filter(value => MIN_INTEGER < value && value < MAX_INTEGER)
    .map(num => Math.min(Int.MaxValue, Math.max(Int.MinValue, num)).toInt)

Copy link
Collaborator

Choose a reason for hiding this comment

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

The standard does not allow + for integers.

index-selector      = int                        ; decimal integer

int                 = "0" /
                      (["-"] DIGIT1 *DIGIT)      ; - optional
DIGIT1              = %x31-39                    ; 1-9 non-zero digit

https://ietf-wg-jsonpath.github.io/draft-ietf-jsonpath-base/draft-ietf-jsonpath-base.html#name-syntax-5

Copy link

Choose a reason for hiding this comment

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

Oh, I see. It seems I missed this, but I think + can also be optional, will update my implementation, thanks

"invalid_selector": true
},
{
"name": "minus space",
"selector": "$[- 1]",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think an argument could be made for this to be put in the whitespace tests as well, but I can see it staying here.

Other opinions welcome.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm getting more and more inclined to think we can benefit from tags. I might be able to make an attempt to add tags in a day or two.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, I think a lot of these in particular could be categorized as just "parsing". I don't think that it's practical to have 2^53 items in a JSON file 😆

"invalid_selector": true
},
{
"name": "-0",
"selector": "$[-0]",
"invalid_selector": true
},
{
"name": "leading -0",
"selector": "$[-01]",
Expand Down
156 changes: 156 additions & 0 deletions tests/slice_selector.json
Original file line number Diff line number Diff line change
Expand Up @@ -610,6 +610,72 @@
9
]
},
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it make sense to add similar checks for the "start" and "step" parameters?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed. Let's go ahead and add those tests, then this should be good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have duplicated them now for start and step, please let me know if it is ok like this.

"name": "start, min exact",
"selector": "$[-9007199254740991::]",
"document": [],
"result": []
},
{
"name": "start, max exact",
"selector": "$[9007199254740991::]",
"document": [],
"result": []
},
{
"name": "start, min exact - 1",
"selector": "$[-9007199254740992::]",
"invalid_selector": true
},
{
"name": "start, max exact + 1",
"selector": "$[9007199254740992::]",
"invalid_selector": true
},
{
"name": "end, min exact",
"selector": "$[:-9007199254740991:]",
"document": [],
"result": []
},
{
"name": "end, max exact",
"selector": "$[:9007199254740991:]",
"document": [],
"result": []
},
{
"name": "end, min exact - 1",
"selector": "$[:-9007199254740992:]",
"invalid_selector": true
},
{
"name": "end, max exact + 1",
"selector": "$[:9007199254740992:]",
"invalid_selector": true
},
{
"name": "step, min exact",
"selector": "$[::-9007199254740991]",
"document": [],
"result": []
},
{
"name": "step, max exact",
"selector": "$[::9007199254740991]",
"document": [],
"result": []
},
{
"name": "step, min exact - 1",
"selector": "$[::-9007199254740992]",
"invalid_selector": true
},
{
"name": "step, max exact + 1",
"selector": "$[::9007199254740992]",
"invalid_selector": true
},
{
"name": "overflowing to value",
"selector": "$[2:231584178474632390847141970017375815706539969331281128078915168015826259279872]",
Expand Down Expand Up @@ -639,6 +705,96 @@
"name": "underflowing step",
"selector": "$[-1:-10:-231584178474632390847141970017375815706539969331281128078915168015826259279872]",
"invalid_selector": true
},
{
"name": "start, leading 0",
"selector": "$[01::]",
"invalid_selector": true
},
{
"name": "start, decimal",
"selector": "$[1.0::]",
"invalid_selector": true
},
{
"name": "start, plus",
"selector": "$[+1::]",
"invalid_selector": true
},
{
"name": "start, minus space",
"selector": "$[- 1::]",
"invalid_selector": true
},
{
"name": "start, -0",
"selector": "$[-0::]",
"invalid_selector": true
},
{
"name": "start, leading -0",
"selector": "$[-01::]",
"invalid_selector": true
},
{
"name": "end, leading 0",
"selector": "$[:01:]",
"invalid_selector": true
},
{
"name": "end, decimal",
"selector": "$[:1.0:]",
"invalid_selector": true
},
{
"name": "end, plus",
"selector": "$[:+1:]",
"invalid_selector": true
},
{
"name": "end, minus space",
"selector": "$[:- 1:]",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as with the index variant of this test.

"invalid_selector": true
},
{
"name": "end, -0",
"selector": "$[:-0:]",
"invalid_selector": true
},
{
"name": "end, leading -0",
"selector": "$[:-01:]",
"invalid_selector": true
},
{
"name": "step, leading 0",
"selector": "$[::01]",
"invalid_selector": true
},
{
"name": "step, decimal",
"selector": "$[::1.0]",
"invalid_selector": true
},
{
"name": "step, plus",
"selector": "$[::+1]",
"invalid_selector": true
},
{
"name": "step, minus space",
"selector": "$[::- 1]",
"invalid_selector": true
},
{
"name": "step, -0",
"selector": "$[::-0]",
"invalid_selector": true
},
{
"name": "step, leading -0",
"selector": "$[::-01]",
"invalid_selector": true
}
]
}
Loading