Skip to content

Start to re-organize test hierarchy: ImpSpec#5803

Merged
lehins merged 3 commits into
masterfrom
lehins/reorganize-test-hierarchy
May 12, 2026
Merged

Start to re-organize test hierarchy: ImpSpec#5803
lehins merged 3 commits into
masterfrom
lehins/reorganize-test-hierarchy

Conversation

@lehins

@lehins lehins commented May 6, 2026

Copy link
Copy Markdown
Collaborator

Description

This PR introduces a class base approach to enforcing common types of tests to be executed for all eras.

This approach also makes EraSpecificSpec unnecessary, since it further expands the same concept.

For now it only applies this approach to ImpSpec, but other will follow in subsequent PRs
Furthermore, it changes ImpSpec tests into consistent structure.

Checklist

  • Commits in meaningful sequence and with useful messages.
  • Tests added or updated when needed.
  • CHANGELOG.md files updated for packages with externally visible changes.
    NOTE: New section is never added with the code changes. (See RELEASING.md).
  • Versions updated in .cabal and CHANGELOG.md files when necessary, according to the
    versioning process.
  • Version bounds in .cabal files updated when necessary.
    NOTE: If bounds change in a cabal file, that package itself must have a version increase. (See RELEASING.md).
  • Code formatted (use scripts/fourmolize.sh).
  • Cabal files formatted (use scripts/cabal-format.sh).
  • CDDL files are up to date (use scripts/gen-cddl.sh)
  • hie.yaml updated (use scripts/gen-hie.sh).
  • Self-reviewed the diff.

@koslambrou

Copy link
Copy Markdown
Contributor

How should we handle tests that are only applied starting from some era? For example, any plutus related tests should start from Alonzo. Should we have specialized typeclasses such as AlonzoBasedEraSpec which specialized functions like ledgerAlonzoBasedEraMain?

@lehins lehins force-pushed the lehins/reorganize-test-hierarchy branch from b0445ed to ba81b08 Compare May 7, 2026 09:38
@lehins lehins changed the title Re-organize test hierarchy Start to re-organize test hierarchy: ImpSpec May 7, 2026
@lehins lehins marked this pull request as ready for review May 7, 2026 10:18
@lehins lehins requested a review from a team as a code owner May 7, 2026 10:18
@lehins lehins force-pushed the lehins/reorganize-test-hierarchy branch from 615523d to 4c9b2c2 Compare May 7, 2026 10:19
@lehins lehins requested a review from koslambrou May 7, 2026 10:19
@lehins

lehins commented May 7, 2026

Copy link
Copy Markdown
Collaborator Author

How should we handle tests that are only applied starting from some era? For example, any plutus related tests should start from Alonzo. Should we have specialized typeclasses such as AlonzoBasedEraSpec which specialized functions like ledgerAlonzoBasedEraMain?

This question can be answered only in specific context of what type of test it is. This PR, is only concerned with ImpSpec, organization for other types of tests is still coming. As far as your work in #5799, you should not be needing anything like this, because your PR is only suppose to be concerned with exampleTx.

Comment thread eras/allegra/impl/test/Main.hs
Comment thread libs/cardano-ledger-core/testlib/Test/Cardano/Ledger/Era.hs

@teodanciu teodanciu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Love it! Much easier to understand which tests get called, at a small price of a few lines repeated.

So the pattern seems to be: eraImpSpec implementations have:

  • an "opt-in" part - the <Era>.<era>EraSpecificSpec ones, and an
  • "auto-inheritable" part via Imp.spec - which is defined as the Imp.spec of the previous era plus the new stuff that is to be inherited in future eras.

Looks good to me, just a few minor remarks.

Comment thread eras/allegra/impl/test/Main.hs
Comment thread eras/shelley/impl/testlib/Test/Cardano/Ledger/Shelley/Imp.hs
Comment thread eras/shelley/impl/testlib/Test/Cardano/Ledger/Shelley/Imp.hs
Comment thread eras/allegra/impl/testlib/Test/Cardano/Ledger/Allegra/Imp.hs
Comment thread eras/allegra/impl/test/Main.hs
Comment thread eras/allegra/impl/test/Main.hs
Comment thread eras/mary/impl/test/Main.hs
Comment thread eras/mary/impl/test/Main.hs
@lehins lehins force-pushed the lehins/reorganize-test-hierarchy branch 2 times, most recently from 6a494fd to 667c4cd Compare May 12, 2026 11:50
@lehins lehins force-pushed the lehins/reorganize-test-hierarchy branch from 667c4cd to 9cbe932 Compare May 12, 2026 14:25
@lehins lehins enabled auto-merge May 12, 2026 15:53
@lehins lehins merged commit 8c025b9 into master May 12, 2026
50 of 51 checks passed
@lehins lehins deleted the lehins/reorganize-test-hierarchy branch May 12, 2026 16:25
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.

5 participants