Parse serialized test cases in shrink viewer tool by nbloomf · Pull Request #22 · tweag/cardano-node

nbloomf · 2026-02-17T21:11:55Z

Description

Depends on tweag/ouroboros-consensus-testing#31 and tweag/ouroboros-consensus-testing#56

This patch adds to the shrink viewer tool added in #17 the ability to parse a serialized test case file. It also updates the test runner to generate compatible JSON.

Serialized test cases do not directly represent minimal failing examples. Because it is super important that the viewer is accurate and creating values of type GenesisTest is pretty intricate, we instead store a QCGen (seed) and a shrink index and reuse the real generator of test cases.

There's a handful of todos in the test case runner that are waiting on the testgen tool; these are marked with my handle.

Depends on #28, closes tweag/cardano-conformance-testing-of-consensus#79. One necessary feature of our testing strategy is the ability to represent individual test cases in a form that can be stored, transmitted, and parsed. This PR adds the ability to render test cases as JSON. Most instances are handwritten (rather than derived via Generic) to avoid leaking details about the internal representation, since serialized test cases may persist. See also tweag/cardano-node#22 ``` cabal test ouroboros-consensus-diffusion:consensus-test --test-options='-p "/JSON Serialization/"' ```

dpulls · 2026-03-03T21:10:11Z

🎉 All dependencies have been resolved !

isovector

Great start, thanks! Left some comments

isovector · 2026-03-20T15:56:18Z

          let isGlobalSuccess =
                testRes == TestSuccess &&
                 (isNothing inputIndex || inputIndex == Just mempty)
+              testVersion = Serialize.TestVersion 1


Let's pull this off the test suite too, and verify that the one in our testfile agrees with it

Wonderig, you mean adding a TestSuiteData field for it?

In general, how should we go about when the work depends on upstream changes? My take would be to just make then in a new branch on the other package and work it out locally, but then the question remains of how reviewers could attempt to compile. For example we could change the source-repository-package to point to the corresponding revision. Or just trust the submitter managed to compile locally and the changes the PR depends on will eventually land.

Looks like it didn't happen, but there was supposed to be a test version attached to each conformance test, which we could cross reference to ensure that the serialized version is talking to the same version. Maybe sync with @ninioArtillero about the best place to get this data to line up.

isovector · 2026-03-20T15:59:22Z

+  rawKey <- case rawJson of
+    Aeson.Object o -> case Aeson.lookup "key" o of
+      Just (Aeson.String k) -> pure k
+      _ -> throwError "Malformed JSON: missing string field \"key\""
+    _ -> throwError "Malformed JSON: must be an object"
+  testKey <- case parseKeyName (T.unpack rawKey) of
+    Just (k :: GenesisTestKey) -> pure k
+    Nothing -> throwError $ "Unrecognized test case key: " <> T.unpack rawKey
+
+  reifiedTestCase :: Serialize.ReifiedTestCase GenesisTestKey Serialize.BlockRep
+    <- case eitherDecode input of
+      Right ok -> pure ok
+      Left err -> throwError $ "Input decoding error: " <> err


Can we put this logic into a canonical FromJSON instance?

isovector · 2026-03-20T16:01:39Z

                      , errors
                      , mtl
                      , optparse-applicative
-                      , ouroboros-consensus-cardano


I'm pretty sure we need this one.

I'm in favor of keeping redundant imports in general... just for convenience. This could easily be cleaned-up when editing for upstream submission.

isovector · 2026-03-20T16:05:53Z

+{-# LANGUAGE RecordWildCards #-}
+{-# LANGUAGE TypeOperators #-}
+
+{-# OPTIONS_GHC -Wno-orphans #-}


Do we have orphans?

isovector · 2026-03-20T16:06:44Z

+    -- QuickCheck's default size is 30, which we adjust to get the initial test case.
+    genesisTest = QC.unGen generator seed (ctMaxSize conformanceTest 30)
+
+  pure $ makeTestCase genesisTest


I don't understand what's happening here. I think you can just expand out the shrink index.

We store the block tree and point schedule in the json, but the full GenesisTest has a bunch of other parameters that aren't included. I think the two options are to either store the extra data and build the genesis test, or store the seed and regenerate it.

In that case I really don't understand what's happening. Please write a big comment describing the situation.

isovector · 2026-03-20T16:10:55Z

Does this actually parse any test files? Would be nice instead of hardcoding in a call to genchains.

nbloomf · 2026-03-20T17:04:48Z

Does this actually parse any test files? Would be nice instead of hardcoding in a call to genchains.

Yes; getInputTestCase reads/decodes the test case and is used in main. I touched on this in another reply but the reason why genChains makes an appearance is that GenesisTest has more stuff in it than the blockchain and point schedule, and to ensure the shrunk test case is correct (and because who knows how all those parameters affect test case generation) this deterministically regenerates it just like the tests do. I figure it's less fragile to recreate from the seed and test key than from 10 genesis test parameters, although that is arguable.

ninioArtillero

This is coming nicely! Left some comments.

ninioArtillero · 2026-03-20T15:25:48Z

 shrinkGenesisTest _ = []

-data TestResult = TestSuccess | TestFailure deriving (Eq, Ord, Show, Enum, Bounded)
+data TestResult


I think now that we are storing the seed in a test failure a docstring is called for

ninioArtillero · 2026-03-20T19:32:15Z

          let isGlobalSuccess =
                testRes == TestSuccess &&
                 (isNothing inputIndex || inputIndex == Just mempty)
+              testVersion = Serialize.TestVersion 1


Wonderig, you mean adding a TestSuiteData field for it?

In general, how should we go about when the work depends on upstream changes? My take would be to just make then in a new branch on the other package and work it out locally, but then the question remains of how reviewers could attempt to compile. For example we could change the source-repository-package to point to the corresponding revision. Or just trust the submitter managed to compile locally and the changes the PR depends on will eventually land.

ninioArtillero · 2026-03-20T19:35:20Z

                      , errors
                      , mtl
                      , optparse-applicative
-                      , ouroboros-consensus-cardano


I'm in favor of keeping redundant imports in general... just for convenience. This could easily be cleaned-up when editing for upstream submission.

ninioArtillero · 2026-03-20T19:40:32Z

-  let bytes = encodePretty testCase
+writeOutputTestCase outputPath (TestCase reifiedTestCase testCase _) = do
+  let
+    Serialize.ReifiedTestCase {..} = reifiedTestCase


Avoid using RecordWildCards per the style guide (Damian also explicitly asked for this on our upstream PR). NamedFieldPuns is kosher BTW.

ninioArtillero · 2026-03-20T20:00:14Z

 import           System.Environment (getArgs)
 import           System.IO (hPutStr, hPutStrLn, stderr)
+import           Test.Consensus.Genesis.Setup (ConformanceTest(..))
+import           Test.Consensus.Genesis.Tests (GenesisTestKey, testSuite)


Just flagging that GenesisTestKey does not contain all the tests: we are missing the SmokeTestKey. This is not relevant to this PR though.

As a note, we are still to define another Key type to gather all keys.

BTW, now such key is found under TestSuite.All

ninioArtillero · 2026-03-20T20:10:34Z

  , optMode         :: Mode
  } deriving (Eq, Show)

 data Mode


Please add some documentation to this and the following type. Alternatively, in this case, it might suffice to give a more specific name.

nbloomf self-assigned this Feb 17, 2026

This was referenced Feb 17, 2026

Update shrinkview to parse testcases tweag/cardano-conformance-testing-of-consensus#81

Open

Add test case serialization tweag/ouroboros-consensus-testing#31

Merged

nbloomf force-pushed the add/parse-test-cases-in-viewer branch from 7ff5c82 to f5dbed7 Compare March 20, 2026 00:31

nbloomf changed the base branch from conformance-testing to ninioArtillero/list-classes March 20, 2026 04:35

nbloomf force-pushed the add/parse-test-cases-in-viewer branch from c7bd2df to c7219d5 Compare March 20, 2026 04:40

nbloomf marked this pull request as ready for review March 20, 2026 04:45

nbloomf requested review from isovector and ninioArtillero March 20, 2026 04:45

ninioArtillero added the enhancement New feature or request label Mar 20, 2026

isovector requested changes Mar 20, 2026

View reviewed changes

ninioArtillero reviewed Mar 20, 2026

View reviewed changes

ninioArtillero force-pushed the ninioArtillero/list-classes branch 2 times, most recently from 0abdc51 to 7135d6f Compare March 24, 2026 18:59

Parse serialized test cases in viewer tool

a6805d5

nbloomf force-pushed the add/parse-test-cases-in-viewer branch from 6a04c07 to a6805d5 Compare March 30, 2026 15:30

nbloomf changed the base branch from ninioArtillero/list-classes to conformance-testing March 30, 2026 15:45

Uh oh!

Conversation

nbloomf commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

dpulls Bot commented Mar 3, 2026

Uh oh!

isovector left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

isovector commented Mar 20, 2026

Uh oh!

nbloomf commented Mar 20, 2026

Uh oh!

ninioArtillero left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nbloomf commented Feb 17, 2026 •

edited

Loading