cardano-ledger icon indicating copy to clipboard operation
cardano-ledger copied to clipboard

Clean up `RoundTrip` tests

Open Soupstraw opened this issue 3 months ago • 1 comments

In Test.Cardano.Ledger.Binary.RoundTrip there is a lot of code duplication due to having "plain" and "annotated" versions for each roundtripping function. It seems that over time these functions have diverged quite a bit. As a part of #3025 I was trying to improve the error messages, but I had to make changes in both versions of the roundtripping properties.

This lead me to try out a potential solution for reducing duplication which involved passing an AnnotatorEvidence type around and then matching on that in places where the logic has to be different depending on whether we are round-tripping an annotated type or a plain type. It seemed to work in most part and I got rid of a lot of the *Ann* functions, but I ran into trouble with the embedTrip function, because the annotated version had diverged so much that it was very difficult to factor out the common logic in both functions.

I think it's a good idea to re-implement the roundtripping logic using ImpSpec. Since the cardano-ledger-binary package can't depend on cardano-ledger-core, we'll have to figure a way to use ImpTestM in cardano-ledger-binary.

Soupstraw avatar Sep 24 '25 10:09 Soupstraw

I think it's a good idea to re-implement the roundtripping logic using ImpSpec

ImpSpec should not be necessary for round trip testing. There is no large state that we need to deal with, as such ImpSpec would add unnecessary complexity

lehins avatar Sep 25 '25 18:09 lehins