test: ToJSON/ToSchema instances agree #626
Merged
Conversation
This file contains 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
brprice
commented
Aug 9, 2022
brprice
force-pushed
the
brprice/openapi-test
branch
2 times, most recently
from
f311ab3
to
b9b6198
Compare
brprice
force-pushed
the
brprice/split-test-library
branch
from
dc0ce93
to
e28b192
Compare
brprice
force-pushed
the
brprice/openapi-test
branch
from
b9b6198
to
236523f
Compare
brprice
commented
Aug 9, 2022
Comment on lines
+26
to
+38
| -- Suitable for deriving via, when the ToJSON instance is via PrimerJSON | ||
| instance | ||
| (Typeable a, Generic a, GToSchema (Rep a), Typeable os, Typeable ks, AesonOptions os) => | ||
| ToSchema (CustomJSON (os :: ks) a) | ||
| where | ||
| declareNamedSchema _ = genericDeclareNamedSchema (fromAesonOptions (aesonOptions @os)) (Proxy @a) |
There was a problem hiding this comment.
I wonder whether there is a library for this? I am a bit nervous writing it myself as am not entirely convinced it is correct (although the tests we are currently adding should catch that)
brprice
force-pushed
the
brprice/openapi-test
branch
from
4bc2cb6
to
c17a3ca
Compare
brprice
commented
Aug 9, 2022
brprice
commented
Aug 9, 2022
brprice
commented
Aug 9, 2022
brprice
force-pushed
the
brprice/openapi-test
branch
7 times, most recently
from
d242bf1
to
dc0a2ab
Compare
brprice
commented
Aug 10, 2022
brprice
commented
Aug 10, 2022
brprice
force-pushed
the
brprice/split-test-library
branch
5 times, most recently
from
8ef0fb1
to
0492962
Compare
brprice
force-pushed
the
brprice/openapi-test
branch
from
80eb725
to
b3dee7f
Compare
dhess
approved these changes
Aug 13, 2022
Whoops, misclick. |
georgefst
approved these changes
Aug 15, 2022
Unfortunately cabal-fmt does not yet support references to cabal sublibraries, breaking the source code checks. Support has been implemented, so hopefully we will be able to revert this commit when the next release of cabal-fmt is made. (This commit is preparation for adding some tests to primer-service that will require primer:primer-hedgehog.)
We bump the cabal-version to enable use of sublibrary references.
Instead of importing modules just for common types (e.g. Text), we simply pull in Foreword, which includes most commonly used identifiers.
Note that the old instance (via `Name`) was nonsense: `GlobalName`s and `Name`s are not serialised the same (one is a record, the other is a string)! The instance compiled even though `GlobalName` is not `Coercible` with `Name` since GHC only requires that the corresponding methods of the two instances are `Coercible. Since `ToSchema a` only uses `a` via `Proxy`, this is true for any choices of `a`.
brprice
force-pushed
the
brprice/openapi-test
branch
from
b3dee7f
to
8c6ce4f
Compare
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.
We test all json encodings agree with their declared schema. This is done by manually listing each type to test, so needs to be updated if we serialize other types in the future.