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 GHC 9.2/9.4 and bytesting 0.11 #9

Merged
merged 5 commits into from
Sep 13, 2022
Merged

Support GHC 9.2/9.4 and bytesting 0.11 #9

merged 5 commits into from
Sep 13, 2022

Conversation

dcoutts
Copy link
Member

@dcoutts dcoutts commented Sep 13, 2022

This is a rebased and extended version of PR #8 from @erikd.

I've had to make it a new PR since it's now a branch in this repo, rather than Erik's repo fork.

In particular it fixes the broken test that Erik identified.

Lets see what CI says now.

erikd and others added 5 commits September 13, 2022 11:27
Canonical JSON is not a proper subset of RFC 7159.

So for the property 'prop_aeson_canonical', where we check that
everything produced as canoncal JSON can be parsed by Aeson (which we
assume correctly implements RFC 7159), we have to tweak things to keep
us within the common subset of canoncal JSON and RFC 7159.
Specifically, canoncal JSON only escapes \ and ", and does not escape
any other non-printable characters.

So the tweak is to just omit non-printable characters from all strings
in this test. Thus we test only within the common subset.
@dcoutts
Copy link
Member Author

dcoutts commented Sep 13, 2022

@erikd if you're wondering about the fix to do with the escape chars, see the comment on the patch and see:
https://wiki.laptop.org/go/Canonical_JSON

Yes, sadly Canonical JSON is not a proper subset of RFC 7159, and that test was improperly assuming it was (or possibly the QC ASCII string generator didn't previously generate control chars less than char 32, but now does).

@dcoutts
Copy link
Member Author

dcoutts commented Sep 13, 2022

Yay, CI all green. Thanks @erikd for setting that up.

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.

2 participants