From d431996ddc486f212c986caf5ceacd044bf47564 Mon Sep 17 00:00:00 2001 From: Davide Grilli Date: Tue, 3 Nov 2026 11:45:43 +0100 Subject: [PATCH] docs: describe the extended test strategy ADR 0033, ADR 0032 update, README test section, agent rules and changelog entry. --- AGENTS.md | 3 +- CHANGELOG.md | 23 +++++++++++++++ CLAUDE.md | 3 +- README.md | 11 +++++-- docs/adr/0032-github-actions-ci.md | 3 +- docs/adr/0033-test-strategy-beyond-core.md | 34 ++++++++++++++++++++++ docs/adr/README.md | 1 + 7 files changed, 73 insertions(+), 5 deletions(-) create mode 100644 docs/adr/0033-test-strategy-beyond-core.md diff --git a/AGENTS.md b/AGENTS.md index a0cc76f..c275b57 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -50,7 +50,8 @@ docker/ reproducible release builds (build.sh + pinned Dockerfiles) - **Test** (headless, primary verification layer): `dotnet test` - Single test: `dotnet test --filter "FullyQualifiedName~TestName"` - Property-based tests (CsCheck, `PropertyTests.cs`) run in the same command, ~30s - - Coverage: `dotnet test tests/PalladiumWallet.Tests --collect:"XPlat Code Coverage"` (coverlet, Cobertura XML) + - Coverage: `dotnet test tests/PalladiumWallet.Tests --settings tests/coverage.runsettings --collect:"XPlat Code Coverage"` (coverlet, Cobertura XML); CI gates `Core` with `tests/check-coverage.py` (95% lines / 88% branches) + - ViewModel tests are headless (`Avalonia.Headless` via `UiSession`, xunit 2) and CLI tests invoke the real entry point in-process: tests under `tests/PalladiumWallet.Tests/App/` that touch process-wide state (`AppPaths`, `Loc`, `Console`, `MainWindowViewModel.UpdateCheck`/`ProfileOverride`) go in xUnit collection `"AppPaths"`; never let a test reach GitHub or a real indexing server - Test tree mirrors `src/Core` (`Chain/ Crypto/ Net/ Spv/ Storage/ Wallet/`) — put new tests in the folder matching the code under test - Network/SPV code (`ElectrumClient`, `WalletSynchronizer`, `TransactionInspector`, TOFU pinning): test against the in-process fake server `tests/PalladiumWallet.Tests/Net/FakeElectrumServer.cs` (loopback TCP + optional TLS, per-method handlers, call counters) — extend that, don't mock the client (it isn't an interface, by design) - Two `internal` test seams (via `InternalsVisibleTo`) sandbox the remaining externals: `UpdateChecker.CheckAsync` takes an optional `HttpMessageHandler` (never hit GitHub from a test); `AppPaths.BootstrapDirOverride`/`PortableBaseOverride`/`DefaultRootOverride` redirect the machine-global path locations (tests using them share xUnit collection `"AppPaths"` because that state is static) diff --git a/CHANGELOG.md b/CHANGELOG.md index 46cbc5f..e669745 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,29 @@ Technical changelog for PalladiumWallet. Format loosely follows by subsystem rather than strictly by date, since `0.9.0` is the first release and covers the full history from the initial commit. +## [Unreleased] + +### Fixed + +- `ElectrumClient`: after a protocol error (e.g. a malformed line from the server) the read + loop died but the client kept reporting `IsConnected == true`, and any later request waited + forever for a response that could never arrive. The read loop now marks the connection dead + before failing the pending requests, and `RequestAsync` fails fast with `IOException` on a + dead connection. + +### Testing + +- Headless ViewModel tests (Avalonia.Headless, no display): wizard flows, password/lock handling, + seed and private-key reveal gating, contacts, settings, sync and send against the fake server. +- In-process end-to-end CLI tests (create/restore/info/sync/send against the fake server). +- Localization consistency tests (7 languages, placeholders, keys used in sources exist). +- Error-path tests for `Core` (empty header range during anchoring, send-all limits, `SyncCache` + roundtrip, derivation edge cases, corrupted config), concurrency tests (in-flight cap, disposal + under load, wallet-lock contention). +- Coverage settings (`tests/coverage.runsettings`) and a coverage gate in CI. +- Test seams: `MainWindowViewModel.UpdateCheck` and `MainWindowViewModel.ProfileOverride` + (internal) so ViewModel tests never reach GitHub or a real indexing server. + ## [1.1.0] — 2026-07-19 Adds a pure address-only watch-only mode (Core + CLI + App wizard + Send diff --git a/CLAUDE.md b/CLAUDE.md index 926a728..61028a3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -50,7 +50,8 @@ docker/ reproducible release builds (build.sh + pinned Dockerfiles) - **Test** (headless, primary verification layer): `dotnet test` - Single test: `dotnet test --filter "FullyQualifiedName~TestName"` - Property-based tests (CsCheck, `PropertyTests.cs`) run in the same command, ~30s - - Coverage: `dotnet test tests/PalladiumWallet.Tests --collect:"XPlat Code Coverage"` (coverlet, Cobertura XML) + - Coverage: `dotnet test tests/PalladiumWallet.Tests --settings tests/coverage.runsettings --collect:"XPlat Code Coverage"` (coverlet, Cobertura XML); CI gates `Core` with `tests/check-coverage.py` (95% lines / 88% branches) + - ViewModel tests are headless (`Avalonia.Headless` via `UiSession`, xunit 2) and CLI tests invoke the real entry point in-process: tests under `tests/PalladiumWallet.Tests/App/` that touch process-wide state (`AppPaths`, `Loc`, `Console`, `MainWindowViewModel.UpdateCheck`/`ProfileOverride`) go in xUnit collection `"AppPaths"`; never let a test reach GitHub or a real indexing server - Test tree mirrors `src/Core` (`Chain/ Crypto/ Net/ Spv/ Storage/ Wallet/`) — put new tests in the folder matching the code under test - Network/SPV code (`ElectrumClient`, `WalletSynchronizer`, `TransactionInspector`, TOFU pinning): test against the in-process fake server `tests/PalladiumWallet.Tests/Net/FakeElectrumServer.cs` (loopback TCP + optional TLS, per-method handlers, call counters) — extend that, don't mock the client (it isn't an interface, by design) - Two `internal` test seams (via `InternalsVisibleTo`) sandbox the remaining externals: `UpdateChecker.CheckAsync` takes an optional `HttpMessageHandler` (never hit GitHub from a test); `AppPaths.BootstrapDirOverride`/`PortableBaseOverride`/`DefaultRootOverride` redirect the machine-global path locations (tests using them share xUnit collection `"AppPaths"` because that state is static) diff --git a/README.md b/README.md index c3dcf72..3f0a243 100644 --- a/README.md +++ b/README.md @@ -141,9 +141,15 @@ Run only the tests in one project: dotnet test tests/PalladiumWallet.Tests ``` -Measure code coverage (the [coverlet](https://github.com/coverlet-coverage/coverlet) collector is already referenced; the report is a Cobertura XML under `TestResults/`): +Measure code coverage (the [coverlet](https://github.com/coverlet-coverage/coverlet) collector is already referenced; the report is a Cobertura XML under `TestResults/`). `tests/coverage.runsettings` excludes the test tooling, and `tests/check-coverage.py` applies the same gate as CI: ```bash -dotnet test tests/PalladiumWallet.Tests --collect:"XPlat Code Coverage" +dotnet test tests/PalladiumWallet.Tests --settings tests/coverage.runsettings --collect:"XPlat Code Coverage" +python3 tests/check-coverage.py TestResults/*/coverage.cobertura.xml PalladiumWallet.Core:95:88 +``` + +Run the tests against a real indexing server (opt-in, skipped when the variable is unset): +```bash +PALLADIUM_TEST_SERVERS=host:50002 PALLADIUM_TEST_SSL=1 dotnet test --filter LiveServer ``` > Cross-implementation tests compare addresses, txids and PSBTs against reference golden vectors: a different address or txid is a blocking bug. @@ -159,6 +165,7 @@ The tests mirror the `Core` layout (`tests/PalladiumWallet.Tests//`): | `Spv/` | Scripthash (vectors computed independently in Python), Merkle proofs (Bitcoin block 100000 + random trees), header parsing and PoW target validation, and the full **`WalletSynchronizer`**: gap-limit scanning, UTXO/history reconstruction, unconfirmed/immature balances, busy-retry, disk-cache reuse — including the paths where a lying server fails Merkle verification or serves a header chain that does not anchor to a hardcoded checkpoint, and the sync must abort | | `Net/` | JSON-RPC transport (pipelining, error mapping, notifications, disconnection, cancellation, oversized responses), typed protocol wrappers, peer discovery/persistence, TLS trust-on-first-use pinning end-to-end (pin, match, mismatch, reset), the update check end-to-end via a stubbed HTTP transport (newer/equal/older tags, HTTP errors, malformed JSON, offline — all best-effort to null) | | `Storage/` | AES-GCM encryption (roundtrip, tampering, fresh salt/nonce), wallet document schema/versioning, atomic saves, single-instance lock, data-path resolution with its full precedence chain (override → portable → pointer file → default) | +| `App/` | Headless ViewModel tests (Avalonia.Headless): wizard flows, password and file-lock handling, seed and private-key reveal gating, contacts, settings, sync and send against the fake server; in-process end-to-end CLI tests; localization consistency (7 languages, placeholders, keys used in sources exist) | | `Wallet/` | Transaction building and signing (spendability rules, coinbase maturity, dust change, multi-UTXO selection, all standard destination types, watch-only PSBT flow, a golden txid for the PSBT signing path, standardness-policy rejection of absurd fees), transaction detail assembly (fees, mine/theirs attribution, RBF, coinbase), amount parsing/formatting, corrupted-wallet-file rejection | Network-facing code is tested against an **in-process fake ElectrumX server** diff --git a/docs/adr/0032-github-actions-ci.md b/docs/adr/0032-github-actions-ci.md index dacacbd..e364331 100644 --- a/docs/adr/0032-github-actions-ci.md +++ b/docs/adr/0032-github-actions-ci.md @@ -15,6 +15,7 @@ The suite (unit, property-based, fuzz-corpus replay, in-process fake-server test - **test**: restore, build (Release) and `dotnet test` of `tests/PalladiumWallet.Tests` on `ubuntu-latest` and `windows-latest`, uploading the TRX and coverage results as artifacts. Only the test project and its references are built, because the Android head needs a workload and SDK that CI does not provision. - **fuzz-smoke**: one job per parser target, running `--random 20000` mutations; any escaping exception fails the job. - **docs**: `docs/check-docs.sh`. +- A **coverage gate** on the Ubuntu test job fails the run if `Core` drops below the thresholds in the workflow (see 0033). The pipeline is offline: `LiveServerTests` does nothing without `PALLADIUM_TEST_SERVERS`, so no third-party indexing server is contacted. Coverage-guided fuzzing (afl++) and release builds stay manual. @@ -30,4 +31,4 @@ The pipeline is offline: `LiveServerTests` does nothing without `PALLADIUM_TEST_ - `.github/workflows/ci.yml` - `docs/check-docs.sh` - `tests/PalladiumWallet.Fuzz/Program.cs` -- Related: 0025, 0026, 0027 +- Related: 0025, 0026, 0027, 0033 diff --git a/docs/adr/0033-test-strategy-beyond-core.md b/docs/adr/0033-test-strategy-beyond-core.md new file mode 100644 index 0000000..e866502 --- /dev/null +++ b/docs/adr/0033-test-strategy-beyond-core.md @@ -0,0 +1,34 @@ +# 0033. Test the UI layer and CLI headlessly and gate coverage + +- **Status:** Accepted +- **Area:** Testing +- **Recorded:** 2026-11-03 + +## Context + +`Core` was covered at about 99% of lines, but `App` (ViewModels, localization) and `Cli` had no tests at all, although they hold logic that moves funds and handles secrets: the send flow, password and lock handling, seed and private-key reveal, the wizard. Nothing stopped coverage from silently dropping. + +## Decision + +- **ViewModels** are tested headlessly with `Avalonia.Headless` (no window, no GPU). The helper package `Avalonia.Headless.XUnit` is not used because it requires xunit v3 and the suite is on xunit 2; `UiSession` runs each test body on a shared headless session instead. Tests are sandboxed: the data root is a temp folder, the saved server points at the in-process `FakeElectrumServer`, and two internal seams keep them offline: `MainWindowViewModel.UpdateCheck` (stubs the GitHub check) and `MainWindowViewModel.ProfileOverride` (removes the real bootstrap servers). +- **CLI** is tested end to end by invoking the real entry point in-process with a captured console, against the fake server (create, restore, info, sync, send). +- **Localization** has consistency tests: every key has all seven languages, placeholders match across languages, and every key used in XAML or C# exists. +- **Tests that touch process-wide state** (`AppPaths`, `Loc`, `Console`, the seams above) share the xUnit collection `AppPaths` so they run serially. +- **Coverage** is measured with `tests/coverage.runsettings` (test tooling excluded). CI fails if `Core` drops below 95% of lines or 88% of branches (`tests/check-coverage.py`); `App` and `Cli` are reported but not yet gated. + +## Consequences + +- Regressions in the wizard, reveal gating, lock handling and send preparation are caught without a display. +- Writing the tests found a real defect in `ElectrumClient` (requests hung forever after a protocol error); it is fixed and covered. +- `MainView` code-behind (window-bound) and the Android head remain untested. +- Headless tests run background timers; the test seams must stay in place for the life of the test process so that leftover ViewModels cannot reach the network. + +## References + +- `tests/PalladiumWallet.Tests/App/HeadlessSetup.cs` +- `tests/PalladiumWallet.Tests/App/VmTestBase.cs` +- `tests/PalladiumWallet.Tests/App/CliTests.cs` +- `tests/PalladiumWallet.Tests/App/LocalizationTests.cs` +- `tests/coverage.runsettings` +- `tests/check-coverage.py` +- Related: 0025, 0026, 0032 diff --git a/docs/adr/README.md b/docs/adr/README.md index 63e506e..085f6ab 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -36,6 +36,7 @@ One ADR per feature or significant design decision. Format: `template.md`. Workf | 0030 | [Single-source versioning with a mandatory changelog entry](0030-single-source-versioning-and-changelog.md) | Distribution | Accepted | | 0031 | [Explicitly defer multisig, Lightning, Tor and hardware wallets](0031-defer-multisig-lightning-and-hardware-wallets.md) | Scope | Accepted | | 0032 | [Run tests, fuzz smoke and docs checks in GitHub Actions](0032-github-actions-ci.md) | Testing | Accepted | +| 0033 | [Test the UI layer and CLI headlessly, gate coverage](0033-test-strategy-beyond-core.md) | Testing | Accepted | ADRs `0002` onwards were written retroactively on 2026-11-03 from the code, comments and changelog. If one misstates the original reasoning, fix it in a pull request.