-
Notifications
You must be signed in to change notification settings - Fork 732
Introduce new AnyShelleyBasedEra type and simplify CDDL tests. #5072
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
Conversation
] | ||
prop_roundtrip_TxWitness_Cddl :: Property | ||
prop_roundtrip_TxWitness_Cddl = H.property $ do | ||
AnyShelleyBasedEra era <- H.forAll $ Gen.element [AnyShelleyBasedEra ShelleyBasedEraShelley .. AnyShelleyBasedEra ShelleyBasedEraAlonzo] |
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.
minBound
?
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.
Now using both minBound
and maxBound
.
cardano-api/src/Cardano/Api/Eras.hs
Outdated
"Alonzo" -> pure $ AnyShelleyBasedEra ShelleyBasedEraAlonzo | ||
"Babbage" -> pure $ AnyShelleyBasedEra ShelleyBasedEraBabbage | ||
"Conway" -> pure $ AnyShelleyBasedEra ShelleyBasedEraConway | ||
wrong -> fail $ "Failed to parse unknown era: " <> Text.unpack wrong |
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 overlaps with:
instance FromJSON AnyCardanoEra where
parseJSON = withText "AnyCardanoEra"
$ \case
"Byron" -> pure $ AnyCardanoEra ByronEra
"Shelley" -> pure $ AnyCardanoEra ShelleyEra
"Allegra" -> pure $ AnyCardanoEra AllegraEra
"Mary" -> pure $ AnyCardanoEra MaryEra
"Alonzo" -> pure $ AnyCardanoEra AlonzoEra
"Babbage" -> pure $ AnyCardanoEra BabbageEra
"Conway" -> pure $ AnyCardanoEra ConwayEra
wrong -> fail $ "Failed to parse unknown era: " <> Text.unpack wrong
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.
There's overlap, but this instance can't accept Byron.
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.
I've added property tests to ensure the cardano and shelley based eras always match except for Byron.
@@ -365,6 +383,62 @@ instance IsShelleyBasedEra BabbageEra where | |||
instance IsShelleyBasedEra ConwayEra where | |||
shelleyBasedEra = ShelleyBasedEraConway | |||
|
|||
data AnyShelleyBasedEra where | |||
AnyShelleyBasedEra |
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.
What problem is the introduction of this GADT solving?
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.
e70d2f2
to
e374b40
Compare
e374b40
to
8e5c3bf
Compare
f7252d4
to
5b2c997
Compare
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.
two minor comments, happy to approve after
af398c6
to
0b151b3
Compare
@@ -0,0 +1,52 @@ | |||
{-# LANGUAGE TypeApplications #-} |
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.
Space between pragma and module
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.
Fixed. Thanks!
0b151b3
to
1150fb9
Compare
…onvert tests into property tests
1150fb9
to
09a6d21
Compare
Introduce new
AnyShelleyBasedEra
type and simplify CDDL tests.We inline roundtrip functions that are rarely used and convert test groups to property tests for consistency.