Skip to content

Add option to generate LCM from tests#5178

Open
sisuresh wants to merge 14 commits intostellar:masterfrom
sisuresh:lcm-gen-tests
Open

Add option to generate LCM from tests#5178
sisuresh wants to merge 14 commits intostellar:masterfrom
sisuresh:lcm-gen-tests

Conversation

@sisuresh
Copy link
Copy Markdown
Contributor

@sisuresh sisuresh commented Mar 12, 2026

Description

Resolves #5125

Tests using TESTDB_IN_MEMORY mode apply transactions directly via tx->apply() instead of closeLedger(), so their transactions don't appear in captured LCM — only empty ledgers are recorded. Fixing applyTx to use closeLedger when capturing LCM breaks a lot of tests, so this PR skips LCM capture for tests that use TESTDB_IN_MEMORY, and adds a TODO to address this later.

Checklist

  • Reviewed the contributing document
  • Rebased on top of master (no merge commits)
  • Ran clang-format v8.0.0 (via make format or the Visual Studio extension)
  • Compiles
  • Ran all tests
  • If change impacts performance, include supporting evidence per the performance document

@sisuresh sisuresh marked this pull request as ready for review March 12, 2026 20:32
Copilot AI review requested due to automatic review settings March 12, 2026 20:32
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds an opt-in test runner mode to automatically capture and persist LedgerCloseMeta (LCM) emitted during ledger closes, primarily to support downstream meta consumers and deterministic fixtures.

Changes:

  • Introduces --capture-lcm in the test runner, accumulating LCM during closeLedger/closeLedgerOn and writing stream-framed XDR files per leaf Catch2 section/test-case.
  • Updates LedgerManagerImpl (tests-only) to retain the most-recent LedgerCloseMeta (with ledger header) for inspection/capture.
  • Tags a curated set of tests with [gen-lcm] and fixes several test ledger-close loops to advance from LCL+1 (to align with stricter assertions).

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/transactions/test/SorobanTxTestUtils.cpp Adds logging include and fixes namespace comment.
src/transactions/test/InvokeHostFunctionTests.cpp Adds [gen-lcm] tags; fixes several loops to close ledgers starting at LCL+1.
src/transactions/test/FrozenLedgerKeysTests.cpp Adds [gen-lcm] tags to selected tests.
src/test/TxTests.h Declares global accumulated-LCM helpers for tests.
src/test/TxTests.cpp Implements accumulated-LCM storage; captures LCM during ledger closes; adds stricter ledger sequence assertion.
src/test/TestUtils.cpp Appends LCM after Soroban network config upgrades when capture is enabled.
src/test/test.h Declares isLcmCaptureEnabled() for test helpers to query capture mode.
src/test/test.cpp Implements --capture-lcm, per-section capture bookkeeping, and file output under test-lcm/.
src/ledger/LedgerManagerImpl.cpp Moves/stabilizes tests-only mLastLedgerCloseMeta capture and ensures it includes the final ledger header.
docs/software/commands.md Documents --capture-lcm, expected output layout, and [gen-lcm] usage.

You can also share your feedback on Copilot code review. Take the survey.

sisuresh and others added 12 commits March 27, 2026 15:45
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Replace __ section separator with - to reduce file path length for
Windows compatibility.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Convert direct tx->apply() calls to closeLedger() across test files so
that LedgerCloseMeta is properly generated when running with
--capture-lcm. This ensures transactions are included in the captured
metadata rather than being applied outside the ledger close pipeline.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@sisuresh sisuresh requested a review from dmkozh March 31, 2026 01:59
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.

Generate complete LedgerCloseMetas in our test cases

2 participants