IntersectMBO / IntersectMBO/cardano-ledger

Clean up `RoundTrip` tests

Open
#5,304 1 comment 0 reactions 0 assignees View on GitHub
:credit_card: technical-debt :detective: testing
Dominant language
Haskell
Stars
295
Forks
179
Avg merge
4d 7h
Merged PRs (30d)
29

Description

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`.

Contributor guide

Open the contributing guide

Research direction

Start in Test.Cardano.Ledger.Binary.RoundTrip and compare the plain and annotated roundtripping functions, especially embedTrip. Investigate how ImpSpec and ImpTestM could be used while keeping cardano-ledger-binary independent of cardano-ledger-core. Done means the duplicated logic is reduced while preserving the roundtrip tests and their improved error messages.

Written by the indexing model from the issue text.

Assessment

Tech stack
haskell
Domain
blockchain, testing
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.