-
Notifications
You must be signed in to change notification settings - Fork 8
test: Check hugr json serializations against the schema (again) #2216
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
base: main
Are you sure you want to change the base?
Conversation
329ca37
to
e90dc84
Compare
@@ -10,7 +10,7 @@ setup: | |||
|
|||
# Run the pre-commit checks. | |||
check: | |||
uv run pre-commit run --all-files | |||
HUGR_TEST_SCHEMA=1 uv run pre-commit run --all-files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels quite heavyweight for a pre-commit, but I don't really object
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just the rule ran on just check
. pre-commit
has its own config that doesn't set the envvar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @aborgna-q - Lemme just check why the roundtrip schema validation wasn't catching my change, because this looks like the same check...
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2216 +/- ##
=======================================
Coverage 82.00% 82.01%
=======================================
Files 230 230
Lines 40940 40951 +11
Branches 37041 37052 +11
=======================================
+ Hits 33574 33584 +10
- Misses 5398 5399 +1
Partials 1968 1968
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Yeah, so see https://github.com/CQCL/hugr/actions/runs/15024222682, passing on the last commit (20bd6a7) on branch |
So, should we merge it? |
I think we should understand why these kind of checks don't catch the malformed json's mentioned above; if these aren't schema violations, why not / what are they? Thinking we have schema checks when actually they don't catch what look to me like schema errors, could well be worse than knowing we don't have schema checks.... |
Removed in #2186