Repository navigation
Close jsondiff input files after loading documents - #254
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues block approval of the input-file cleanup change.
No open issues 🎉
What changed in this PR
This PR fixes input-file cleanup in the jsondiff CLI, releasing file handles after loading documents even when JSON parsing fails.
Changes:
- Uses
ExitStackto close both owned inputs while leaving standard input open. - Adds regression tests for equal and differing documents, malformed JSON in either file, and stdin in either position.
- Preserves existing output and exit-code behavior.
| File | Description |
|---|---|
| tests.py | Adds input cleanup and stdin preservation tests. |
| jsonpatch_cli.py | Closes owned input files after loading, including on parsing errors. |
Review effort: Balanced
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
diff_files() leaves both argparse-opened input files open after reading them. They also remain open if either JSON document fails to parse. This makes repeated in-process calls depend on garbage collection to release descriptors and file locks.
Use ExitStack to close both owned inputs after loading the documents, including on parsing errors. Standard input is left open when either filename is '-'. Output and exit codes are unchanged.
Three tests retain real input handles and check cleanup for equal and differing documents, malformed JSON in either file, and stdin in either position. All six subcases fail on unchanged master and pass with the fix.
Validation on Python 3.12.14: 289 tests and 24 property-based tests pass, the CI Flake8 error check passes, and git diff --check is clean.