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

Support discouraged but allowed "$id" values #80

Merged
merged 1 commit into from
Apr 17, 2023

Conversation

handrews
Copy link
Contributor

Fixes #69: Non-normalized URIs and (prior to draft-next) URis with empty fragments are allowed. The official test suite does not pick up these issues, so additional tests have been included.

@codecov-commenter
Copy link

codecov-commenter commented Mar 20, 2023

Codecov Report

Patch coverage: 85.18% and project coverage change: -0.01 ⚠️

Comparison is base (599be33) 92.49% compared to head (e4e8d7d) 92.49%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #80      +/-   ##
==========================================
- Coverage   92.49%   92.49%   -0.01%     
==========================================
  Files          21       22       +1     
  Lines        1999     2024      +25     
  Branches      424      428       +4     
==========================================
+ Hits         1849     1872      +23     
- Misses         96       98       +2     
  Partials       54       54              
Impacted Files Coverage Δ
jschon/vocabulary/future.py 76.47% <76.47%> (ø)
jschon/catalog/_next.py 100.00% <100.00%> (ø)
jschon/jsonschema.py 90.90% <100.00%> (+0.24%) ⬆️
jschon/vocabulary/core.py 84.29% <100.00%> (ø)

... and 1 file with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

tests/test_vocabulary.py Outdated Show resolved Hide resolved
tests/test_vocabulary.py Outdated Show resolved Hide resolved
tests/test_vocabulary.py Outdated Show resolved Hide resolved
tests/test_vocabulary.py Outdated Show resolved Hide resolved
tests/test_vocabulary.py Outdated Show resolved Hide resolved
tests/test_vocabulary.py Outdated Show resolved Hide resolved
jschon/vocabulary/core.py Outdated Show resolved Hide resolved
jschon/vocabulary/core.py Outdated Show resolved Hide resolved
jschon/vocabulary/core.py Outdated Show resolved Hide resolved
jschon/jsonschema.py Show resolved Hide resolved
@handrews
Copy link
Contributor Author

OK I think I got everything, but of course let me know if I missed something.

@handrews
Copy link
Contributor Author

The coverage failure is mostly due to the draft-next tests not being run now that the draft-next version is separate. If they were (see PRs #97 and #98), the patch coverage for the new class would be the same as for the old one.

@marksparkza
Copy link
Owner

Thanks, just a couple of things: a change log entry seems to have snuck into 0.10.0, and just a reminder to double quote the strings in JSONSchema._bootstrap

@handrews
Copy link
Contributor Author

Thanks, just a couple of things: a change log entry seems to have snuck into 0.10.0
I could have sworn I fixed that! 🤦

and just a reminder to double quote the strings in JSONSchema._bootstrap
I had completely missed that comment the first time around, thanks for the explanation. I'll try to remember that - I default to single quotes as a hangover from Perl where single quotes turn off string interpolation for better performance.

That should be everything now.

Non-normalized URIs and (prior to draft-next) URis with empty
fragments are allowed.  The official test suite does not pick
up these issues, so additional tests have been included.
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.

"$id" is (slightly) non-compliant (hidden by errors in JSON Schema Test Suite)
3 participants