[CTC 2] Port genesis tests into new TestSuite data structure#1915
[CTC 2] Port genesis tests into new TestSuite data structure#1915
TestSuite data structure#1915Conversation
ninioArtillero
force-pushed
the
ctc/test-suites
branch
2 times, most recently
from
f31b469 to
5e10769
Compare
ninioArtillero
requested review from
bladyjoker,
dnadales,
fraser-iohk,
geo2a,
jasagredo and
nfrisby
as code owners
ninioArtillero
changed the title
TestSuite data structureTestSuite data structure
ninioArtillero
changed the title
TestSuite data structureTestSuite data structure
|
Added the |
ninioArtillero
force-pushed
the
ctc/test-suites
branch
from
5e10769 to
c753abd
Compare
dnadales
left a comment
dnadales
left a comment
There was a problem hiding this comment.
My biggest concern is the use of SmallKey and the complexity that it brings vs the actual safety it provides.
ninioArtillero
force-pushed
the
ctc/test-suites
branch
from
73ac17d to
06e4280
Compare
ninioArtillero
force-pushed
the
ctc/test-suites
branch
3 times, most recently
from
6b502ca to
8f68873
Compare
|
@dnadales The pending comments have been resolved and the proposed changes done, most notably:
I did also proposed changing the Furthermore, the history has been cleaned-up back to a single commit, tests pass locally, a CI error from a missing |
ninioArtillero
force-pushed
the
ctc/test-suites
branch
from
8f68873 to
eb5ca89
Compare
ninioArtillero
force-pushed
the
ctc/test-suites
branch
from
eb5ca89 to
e35d8ae
Compare
jasagredo
left a comment
jasagredo
left a comment
There was a problem hiding this comment.
The transformation looks consistent. There is a bunch of code to reify the test-suite which looks fine and the mkConformanceTest is basically a re-shaping of what was there before. I think this looks good, however I have a bunch of comments I would like to clarify or suggest, but I am open to debate them.
| data TestKey | ||
| = WithNoAdversariesAndOneScheduleForAllPeers | ||
| | WithNoAdversariesAndOneSchedulePerHonestPeer | ||
| | WithAdversariesAndOneScheduleForAllPeers | ||
| | WithAdversariesAndOneSchedulePerHonestPeer | ||
| deriving stock (Eq, Ord, Generic) | ||
| deriving SmallKey via Generically TestKey |
There was a problem hiding this comment.
It might be interesting to create a TH splice that defines this datatype by grouping all the functions with a specific pattern like
test_withNoAdversariesAndOneScheduleForAllPeers :: ...
test_withNoAdversariesAndOneSchedulePerHonestPeer :: ...
return []
tests = $mkTestsBut that would be a nice to have and I don't even know how to do this myself.
There was a problem hiding this comment.
I don't know how to do it either 😅 , and can just wonder about how much complexity would it introduce. Such an automation seems pretty desirable, but the cognitive burden of a naming convention seems to be a slight improvement over that of adding a constructor per test (and have the compiler ensure the test is included) IMO.
There was a problem hiding this comment.
I can advise on doing this later down the line if we're so inclined
ninioArtillero
force-pushed
the
ctc/test-suites
branch
from
e35d8ae to
20dc6cd
Compare
The new `ConformanceTest` record contains fields for all data formerly used to run a test via `forAllGenesisTest` helper, plus other fields that pertain to its evaluation as a test property on a`TestTree`. A value of this type is defined for each point-schedule/genesis test. All such `ConformanceTest`s are arranged in a `TestSuite`s, a data structure introduced with the goal of eventually exporting them as part of a new sublibrary for the Conformance Testing of Consensus harness (see https://github.com/tweag/cardano-conformance-testing-of-consensus). This data structure is designed to optimize single test lookups, while retaining the necessary structure to be compiled back to a `TestTree`, so that the introduced changes preserve the semantics of the `ouroboros-consensus:test:consensus-diffusion-test` test suite. To accomplish this, a new data type is introduced in each module as a `key` for the locally defined `TestSuite`, such that each test in the module corresponds to a unique value (nullary constructor) of this type. This means that including a new test in a `TestSuite` requires the extension of such key type by introducing a new data constructor. These `key` types are aggregated into higher level key types to reify the nested grouping of the original tasty `TestTree`s. It is by means of `mkTestSuite` and `at` that a higher level `TestSuite` can be defined. The `SmallKey key` constraint is needed for their exhaustive construction, via `newTestSuite` and `mkTestSuite`; as implemented, a `TestSuite` is a total map. Co-authored-by: Sandy Maguire <sandy.maguire@tweag.io>
ninioArtillero
force-pushed
the
ctc/test-suites
branch
from
20dc6cd to
39c9a75
Compare
Backports from IntersectMBO/ouroboros-consensus#1915 - Documentation updates + fixes - `newtypes` for adjustment fields of `ConformanceTest`: `AdjustTestCount` and `AdjustMaxSize` - Introduced `getAllKeys` function to hide the `SmallKey` class method `allKeys` - Blacklisted more big type `SmallKey` instances (`Int`s, `Word`s and `Text`). - Refactored `SmallKey` tests to extend case variety - Recovered the former `TestTree` structure inside the `CSJ.testSuite` - Added `grouping` combinator to `TestSuite.hs` - Changed snake_case `ConformanceTest`s names to camelCase - Removed use of `RecordWildCards` in `Genesis.Setup.hs` - Added version bound to `monoidal-containers`
5 participants
The new
ConformanceTestrecord contains fields for all data formerly used to run a test viaforAllGenesisTesthelper, plus other fields that adjust its evaluation as a test property on aTestTree. It is worth noting that aGenesisTestis more general than what its name suggests; it is the type representing the "Node-vs-Environment" or point schedule tests.All such
ConformanceTests are arranged inTestSuites, a data structure introduced with the goal of eventually exporting them as part of a new sublibrary for the Conformance Testing of Consensus (CTC) harness. This data structure is designed to optimize single test lookups, while retaining the necessary structure to be compiled back to aTestTree, in such a way that the introduced changes preserve the semantics of theouroboros-consensus:test:consensus-diffusion-testtest suite. Those,TestSuites lay out an interface between the internal property test execution and the up-coming CTC harness.To accomplish this, a new
TestKeydata type is introduced in each module as akeyfor the locally definedTestSuite, such that each test in the module corresponds to a unique value (nullary constructor) of this type. This means that including a new test in aTestSuiterequires the extension ofTestKeyby e.g. introducing a new data constructor. TheseTestKeytypes are aggregated into higher level key types (sums of other test key unary constructors) to reify the nested grouping of the original tastyTestTrees by combining them usingmkTestSuite.The new
SmallKeyconstraint is morally equivalent toUniverse.Class.Finitefrom theuniverse-basepackage, but includes a set of measures to prevent large finite types from instantiating it. The reason for this is that itsallKeysmethod is used for the exhaustive construction ofTestSuites vianewTestSuiteandmkTestSuite; as implemented, aTestSuiteis a total map.