-
-
Notifications
You must be signed in to change notification settings - Fork 49
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
Results do not match other implementations #28
Comments
Thanks. I'll update Oj to more closely follow the consensus. As a side note, on Safari the header labels on your table all blended together making it unreadable for the golang entries. |
You might also be interested in the JSONPath draft development (https://github.com/ietf-wg-jsonpath/draft-ietf-jsonpath-jsonpath) and the slack channel to drive a common interpretation of JSONPath (https://join.slack.com/t/jsonpath-standard/shared_invite/zt-es623y9z-~JOiqJTylBXSttT5n_xaPQ). |
Thanks |
I wasn't successful running the commands outlined in the README.md but I'm sure I can get something figured out with the selector and document files. Is there any way to get a listing of the consensus results without running all the tests? |
Currently the Docker build seems broken, I'll push a fix. I mostly run natively on OSX, so the Docker build is a bit behind, sadly.
Yes, and using this is definitely encouraged: https://github.com/cburgmer/json-path-comparison/blob/master/regression_suite/regression_suite.yaml This gets updated manually every few weeks when the versions of dependencies are bumped. |
I am using OSX as well so that would help. I think I can drive a test with just the consensus file so that is great. Thanks. |
The consensus branch has the fixes in it. Is it possible to recheck with that branch? |
This one is still failing locally:
There are further selectors I have omitted from the initial issue, as they are not wrong by the consensus, just unsupported. I'm posting them now for completeness:
|
Odd that I'll look at the others as well. Thanks for all the test cases. |
Support for |
As for |
It doesn't look like there was a final agreement. Personally I prefer to support simple operations in scripts over allowing That leaves the |
I am still not able to run the tests by following the README.md instructions but I did notice that the Golang targets use go v1.13 which is 2 minor versions out of date and more than a year old. The current version is v1.15. |
It seems the error depends on the JSON payload to surface:
However
|
Thanks. That should help. I'll run that way to check as well. |
I think the latest in the consensus branch should pass all but the key with a dash in it but I don't plan on changing that one. |
Anyway, let me know if it looks good and I'll make a release. |
Looks good to me! I suggest you look at the other queries as well, even those lacking consensus, as there are a few cases you might also want to support. Take for example: https://cburgmer.github.io/json-path-comparison/#array_slice_with_open_start_and_negative_step. Once you release I can re-run and publish the comparison. |
I think I have the slices covered. I had the end being inclusive and the consensus was for exclusive. That covered a majority of the differences. I'll release and take another look. BTW, the headers are readable now. |
Version v1.4.0 has been released. |
I'll submit a merge request for the update along with a few other changes to main.go and go.mod. You can choose to ignore of merge. I won't take offence in either case. |
Based on the results I think this issue can be closed. You agree? Unrelated to closing the issue, have you considered scoring each implementation and displaying that somewhere? Maybe 1 point for consensus, .5 for a majority, .25 for minority, and zero for errors or not matching the consensus? I just picked those values on the spur of the moment. The OjG README.md now has a link to your site. |
Yes, happy to close.
Yes and no. I believe it's complex :)
Awesome. I hope we can help our users make better decisions when adopting a certain implementation. |
But feel free to raise an issue over at the comparison project so we can discuss with others whether there's a good alternative. Also, my answer might have not caught you initial point. What is your goal for the scoring? |
The goal was a kind of overview or summary of the very long list. I'll create an issue and see how the conversation goes. |
The following queries provide results that do not match those of other implementations of JSONPath
(compare https://cburgmer.github.io/json-path-comparison/):
$[1:3]
Input:
Expected output:
Actual output:
$[0:5]
Input:
Expected output:
Actual output:
NOT_FOUND
$[1:10]
Input:
Expected output:
Actual output:
NOT_FOUND
$[-4:-4]
Input:
Expected output:
Actual output:
$[-4:-3]
Input:
Expected output:
Actual output:
$[-4:2]
Input:
Expected output:
Actual output:
$[-4:3]
Input:
Expected output:
Actual output:
$[:2]
Input:
Expected output:
Actual output:
$[-4:]
Input:
Expected output:
Actual output:
NOT_FOUND
$[0:3:1]
Input:
Expected output:
Actual output:
$[0:4:2]
Input:
Expected output:
Actual output:
$[]
Input:
Expected output:
Actual output:
$..[1].key
Input:
Expected output (in any order as no consensus on ordering exists):
Actual output:
$..key
Input:
Expected output (in any order as no consensus on ordering exists):
Actual output:
$.store..price
Input:
Expected output (in any order as no consensus on ordering exists):
Actual output:
$[?(1==1)]
Input:
Error:
For reference, the output was generated by the program in https://github.com/cburgmer/json-path-comparison/tree/master/implementations/Golang_github.com-ohler55-ojg.
The text was updated successfully, but these errors were encountered: