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.

Sign up for GitHub

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
+214 −0
Merged

Add more tests for selectors #90

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d4f1f96
Add more tests for selectors
Marcono1234 Aug 2, 2024
b83d681
Remove duplicated whitespace tests
Marcono1234 Aug 3, 2024
4e9f117
Extend slice tests for start and step arguments
Marcono1234 Aug 3, 2024
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
21 changes: 21 additions & 0 deletions tests/basic.json
View file
Open in desktop
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 Expand Up @@ -200,6 +210,17 @@
1
]
},
{
"name": "multiple selectors, spaces",
Marcono1234 marked this conversation as resolved.
Show resolved Hide resolved
"selector": "$[ 'a' , 1 ]",
"document": {
"a": 1,
"b": 2
},
"result": [
1
]
},
{
"name": "multiple selectors, index and slice",
"selector": "$[1,5:7]",
Expand Down
48 changes: 48 additions & 0 deletions tests/index_selector.json
View file
Open in desktop
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
73 changes: 73 additions & 0 deletions tests/slice_selector.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,27 @@
5
]
},
{
"name": "slice selector with spaces",
Marcono1234 marked this conversation as resolved.
Show resolved Hide resolved
"selector": "$[ 1 : 6 : 2 ]",
"document": [
0,
1,
2,
3,
4,
5,
6,
7,
8,
9
],
"result": [
1,
3,
5
]
},
{
"name": "slice selector with everything omitted, short form",
"selector": "$[:]",
Expand Down Expand Up @@ -610,6 +631,28 @@
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": "min exact",
"selector": "$[:-9007199254740991:]",
"document": [],
"result": []
},
{
"name": "max exact",
"selector": "$[:9007199254740991:]",
"document": [],
"result": []
},
{
"name": "min exact - 1",
"selector": "$[:-9007199254740992:]",
"invalid_selector": true
},
{
"name": "max exact + 1",
"selector": "$[:9007199254740992:]",
"invalid_selector": true
},
{
"name": "overflowing to value",
"selector": "$[2:231584178474632390847141970017375815706539969331281128078915168015826259279872]",
Expand Down Expand Up @@ -639,6 +682,36 @@
"name": "underflowing step",
"selector": "$[-1:-10:-231584178474632390847141970017375815706539969331281128078915168015826259279872]",
"invalid_selector": true
},
{
"name": "leading 0",
"selector": "$[:01:]",
"invalid_selector": true
},
{
"name": "decimal",
"selector": "$[:1.0:]",
"invalid_selector": true
},
{
"name": "plus",
"selector": "$[:+1:]",
"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.

Same as with the index variant of this test.

"invalid_selector": true
},
{
"name": "-0",
"selector": "$[:-0:]",
"invalid_selector": true
},
{
"name": "leading -0",
"selector": "$[:-01:]",
"invalid_selector": true
}
]
}
Loading