Skip to content

Fix flaky test prop_childLookup#24

Merged
nbloomf merged 2 commits intoconformance-testingfrom
fix/shrinkindex-child-test
Mar 5, 2026
Merged

Fix flaky test prop_childLookup#24
nbloomf merged 2 commits intoconformance-testingfrom
fix/shrinkindex-child-test

Conversation

@nbloomf
Copy link
Member

@nbloomf nbloomf commented Mar 4, 2026

Description

Fixes tweag/cardano-conformance-testing-of-consensus#104.

For any given shrinking function f, makeShrinkTree f and f should be coherent in the sense that

ShrinkTree.lookup (ShrinkTree.child n) (makeShrinkTree f x) == Just (f x !! n)

whenever either side is defined and not Nothing. Or more intuitively, makeShrinkTree f x should reify the traces of shrink x when shrink == f. This is important because of the way test cases and shrink indices can persist across test runs.

There is a property test to validate this, prop_childLookup, however it is currently flaky because of edge cases in the generation of arbitrary indices. This presents as a test that randomly fails with either "negative index" or "index too large", neither of which has to do with the property being violated.

This patch tweaks the test to check the entire list of child shrinks rather than just one at a specific index. This is equivalent to the original property, since

as == bs   <==>   and [as!!k == bs!!k | k <- [0..]]

(restricting k on the right so that both lookups exist). This sidesteps the partial function (!!) and also checks more evidence per test case.

Copy link
Collaborator

@ninioArtillero ninioArtillero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for solving this! I like that this refactor documents the generation of the arbitrary shrinker with the new Shrinker explicitly.

After going through this I found that the "negative index" error could be solved by just limiting the range of the Shrinkerlength to be within chooseInt (1,100)... but we indeed want to have the empty shrinker! Besides, the list equality in the property is less ad hoc than checking for equality of a single child. Nice!

Left some suggestions. Feel free to merge when ready.

Co-authored-by: Xavier Góngora <xavier.gongora@tweag.io>
@nbloomf nbloomf marked this pull request as ready for review March 5, 2026 14:35
@nbloomf nbloomf merged commit 03b7c15 into conformance-testing Mar 5, 2026
10 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants