IntersectMBO / ouroboros-consensus

Implementation of a Consensus Layer for the Ouroboros family of protocols
https://ouroboros-consensus.cardano.intersectmbo.org
Apache License 2.0
34 stars 23 forks source link

Improve golden tests infrastructure #694

Open edsko opened 4 years ago

edsko commented 4 years ago

At the moment, we have a manually constructed examples, leading to the possibility of lots of missed parts of the encoding, and not all tests live where they ought to.

We should:

  1. Introduce a generic infrastructure (probably in cardano-base) for constructing canonical examples:
class CanonicalExamples a where
  canonicalExamples :: Proxy a -> [a]

which should have a default generic implementation: for a sum type just construct canonical examples for each of the constructors, and for a product type just construct a single canonical example for all components.

  1. Manual instances should be provided for (a) recursive datatypes (the above strategy would otherwise give an infinite number of examples), (b) the standard types (tuples, maybe, lists, ..), and (c) types where the decoder establishes specific invariants. As an example of category (c), suppose we have a data type with three fields a, b, and c, where the encoder only encodes a and b, and the coder reconstructs c from a and b. An arbitrary example would fail a roundtrip test, as the two c values would not match, and so this will require a manual instance. Writing such a manual instance can piggy back on canonical examples for standard types (i.e., it could just ask for a canonical example of an a, b tuple).

  2. Instances should be provided for all the ledger types, in cardano-ledger for Byron and in cardano-ledger-specs for Shelley, and these repos should be equipped with golden tests for these canonical examples.

  3. In ouroboros-consensus, we should provide instances for our types, not duplicating any of the work done in the ledger, and have golden tests for our types. Some of those tests will necessarily involve types from the ledger (i.e., ExtLedgerState), that is not problematic.

edsko commented 4 years ago

Marking this as medium priority, despite it being a testing ticket, since we already had one occurrence of an unintended change in the binary format slipping in.

edsko commented 4 years ago

Implementation note: instances for base types such as Bool, Int, etc should probably only give a single value, otherwise we'll end up with combinatorial explosion and way too many examples.

kderme commented 4 years ago

Introduce a generic infrastructure (probably in cardano-base)

cardano-base or cardano-prelude?

mrBliss commented 4 years ago

In cardano-prelude, see https://github.com/input-output-hk/cardano-prelude/blob/master/src/Cardano/Prelude/GHC/Heap/NormalForm/Classy.hs

edsko commented 4 years ago

Yes, sorry.