Skip to content

Introduce roundtrip testing - #207

Merged
michaelmior merged 1 commit into
h2non:masterfrom
kurtmckee:test-roundtrips
Jan 28, 2026
Merged

Introduce roundtrip testing#207
michaelmior merged 1 commit into
h2non:masterfrom
kurtmckee:test-roundtrips

Conversation

@kurtmckee

@kurtmckee kurtmckee commented Jan 4, 2026

Copy link
Copy Markdown

While working on #206, I kept encountering bugs and missing features in the existing implementation's JsonPath string serialization and equality methods that hampered comparison and roundtrip testing. I'm therefore submitting this PR to fix bugs in the existing implementation.

This PR introduces the following changes:

  • Add hypothesis as a testing tool
  • Add roundtrip testing: given an arbitrary string that can be parsed, it should be possible to parse the string, serialize the JsonPath instance back to a string, and re-parse that string back to the same JsonPath instance.

I had hypothesis generate ~10,000,000 examples cumulatively, which identified a number of issues in the current implementation:


Added

  • Support equality checking of Operation instances
  • Support string serialization of Union and Intersect instances

Fixed

  • Fix string serialization throughout the library to enforce roundtrip parsing consistency.
    • Fields are more conservatively enclosed in quotion marks.
      This fixes serialization and re-parsing of "00", '%', '0@' and "&'".
    • Operation instances can now be serialized.
      This fixes serialization of 0-@ and A -A.
    • SortedThis instances can now be serialized and re-parsed.
      This fixes serialization of 0[/0].
    • Child precedence is now preserved using parentheses during serialization.
      This ensures that serialized strings like a..b[c] serialize and re-parse identically.
  • Fix parsing and string serialization of numeric-only identifiers.
    This fixes parsing of 10, which was parsed as two separate fields.
  • Fix equality checks for SortedThis instances.

@kurtmckee
kurtmckee marked this pull request as draft January 4, 2026 21:44
@kurtmckee
kurtmckee marked this pull request as ready for review January 4, 2026 23:48

@michaelmior michaelmior left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, just one minor suggestion.

Comment thread jsonpath_ng/jsonpath.py
@kurtmckee

Copy link
Copy Markdown
Author

@michaelmior Is there anything else that needs to be addressed or considered before this can be merged?

@michaelmior

Copy link
Copy Markdown
Collaborator

@kurtmckee Nope! Go for it :) (I guess a minor merge conflict changelog needs to be resolved, but obviously not a big deal.)

@kurtmckee

Copy link
Copy Markdown
Author

I don't have merge permissions on the project. 👍

I'll fix the merge conflict straight away, though!

@kurtmckee

Copy link
Copy Markdown
Author

Rebased on master and force-pushed to resolve merge conflicts.

I also fixed a typo in the CHANGELOG (the word "pickly" -> "pickle") and removed trailing whitespace.

@michaelmior
michaelmior merged commit e00121b into h2non:master Jan 28, 2026
7 checks passed
@michaelmior

Copy link
Copy Markdown
Collaborator

Merged! Thanks :)

@kurtmckee
kurtmckee deleted the test-roundtrips branch January 28, 2026 19:35
@lantrix

lantrix commented Feb 2, 2026

Copy link
Copy Markdown

Any chance of a release to now remove your use of the deprecated ply@3.11 package?

https://security.snyk.io/vuln/SNYK-PYTHON-PLY-15046763
https://github.com/bohmiiidd/Undocumented-RCE-in-PLY

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants