-
Notifications
You must be signed in to change notification settings - Fork 15
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
Update to cardano-8.31.0.0
#435
Conversation
3512456
to
4f68120
Compare
…-parameters-update command
91896a7
to
df0b442
Compare
df0b442
to
f3116b2
Compare
f3116b2
to
48b087b
Compare
48b087b
to
294cb74
Compare
hprop_roundtrip_Value_parse_renderPretty :: Property | ||
hprop_roundtrip_Value_parse_renderPretty = | ||
-- TODO enable these tests after switching completely to ledger types | ||
disable_hprop_roundtrip_Value_parse_renderPretty :: Property |
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.
Why can't we enable them now?
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.
The tests fail like when Value
is completely empty and the generators are now capable of generating completely empty values:
✗ Test.Cli.MultiAssetParsing.hprop_roundtrip_Value_parse_render failed at test/cardano-cli-test/Test/Cli/MultiAssetParsing.hs:23:5
after 25 tests.
shrink path: 25:
┏━━ test/cardano-cli-test/Test/Cli/MultiAssetParsing.hs ━━━
18 ┃ hprop_roundtrip_Value_parse_render :: Property
19 ┃ hprop_roundtrip_Value_parse_render =
20 ┃ property $ do
21 ┃ ledgerValue <- forAll $ genValueDefault MaryEraOnwardsConway
┃ │ MaryValue 0 (MultiAsset (fromList []))
22 ┃ let value = fromLedgerValue ShelleyBasedEraConway ledgerValue
23 ┃ tripping
┃ ^^^^^^^^
┃ │ ━━━ Original ━━━
┃ │ Right (valueFromList [])
┃ │ ━━━ Intermediate ━━━
┃ │ ""
┃ │ ━━━ Roundtrip ━━━
┃ │ Left (line 1, column 1):
┃ │ unexpected end of input
┃ │ expecting multi-asset value expression
24 ┃ value
25 ┃ renderValue
26 ┃ (Parsec.parse parseValue "" . Text.unpack)
This failure can be reproduced by running:
> recheckAt (Seed 15168243033552485215 8658252526254707155) "25:" Test.Cli.MultiAssetParsing.hprop_roundtrip_Value_parse_render
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 don't think we particularly care about empty Value
, but for completeness, I think it good if the roundtrip worked for this edge case. This can be fixed in cardano-api
.
But since we're going to delete Value
anyway, it probably isn't worth the effort.
In fact, I will delete these tests because they don't belong here: They test nothing in cardano-cli
. See the import list for verification.
These tests should instead be moved to cardano-api
.
Changelog
Context
Fixes #388
How to trust this PR
Highlight important bits of the PR that will make the review faster. If there are commands the reviewer can run to observe the new behavior, describe them.
Checklist