From d50988052261284fcf202db401eb78bdd7a08f29 Mon Sep 17 00:00:00 2001 From: Olaf Monien Date: Wed, 22 Jul 2026 01:13:37 +0200 Subject: [PATCH 01/15] Document cleanup roadmap: split PR #90 and DUnitX-first. Record that AEAD architecture must be separated from ChaCha features, and that a full DUnit to DUnitX migration is the prerequisite before that work. --- Docs/Cleanup-Roadmap.md | 151 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 151 insertions(+) create mode 100644 Docs/Cleanup-Roadmap.md diff --git a/Docs/Cleanup-Roadmap.md b/Docs/Cleanup-Roadmap.md new file mode 100644 index 0000000..5abe3fb --- /dev/null +++ b/Docs/Cleanup-Roadmap.md @@ -0,0 +1,151 @@ +# DEC Cleanup Roadmap + +Working branch: **`Cleanup_OM`** (PRs against **`development`**). + +This document records agreed technical decisions and the planned order of work. +It is intentionally a planning artefact, not a design for individual pull requests. + +--- + +## 1. Context: PR #90 (ChaCha20-Poly1305) + +Open contribution: [PR #90](https://github.com/MHumm/DelphiEncryptionCompendium/pull/90) +(`mikerabat` → base branch `development_chacha20poly1305`). + +The PR currently mixes **two independent concerns**: + +| Concern | What it contains | Risk if merged as-is | +|---|---|---| +| **A. Architecture / AEAD base rewrite** | Slimmer `TAuthenticatedCipherModesBase` lifecycle (`InitAuth`, `UpdateWithEncDecBuf`, `FinalizeMAC` / `FinalizeAEAD`, multi-call encode/decode); large GCM rewrite for stream/chunk support (#87); shared wiring for authenticated modes | Touches core cipher-mode contracts; can break GCM/CCM and existing tests; needs careful review | +| **B. ChaCha features** | `TCipher_ChaCha20`, `TCipher_XChaCha20`, Poly1305 mode (`cmPoly1305`, `DECCipherModesPoly1305`), CPU feature detection, SSE/AVX paths, AES-NI, Wycheproof/RFC tests | Large feature surface, but can be evaluated as an algorithm addition **once** the base API is stable | + +Related extras in the same PR (also not “pure ChaCha”): AES-NI assembler, `DECCPUSupport`, `DECOptions.inc` ASM define changes, 64-bit util/ASM fixes. + +### Decision (binding) + +**The architecture rework must be completely separated from the ChaCha feature work.** + +Consequences: + +1. **Do not** merge PR #90 as a single unit. +2. Split (or re-land) into at least: + - **PR / package 1 — AEAD architecture:** base class + GCM multi-call/stream behaviour (+ only the minimum CCM adaptations required for the new interface). No ChaCha/XChaCha/Poly1305 algorithm code. + - **PR / package 2 — ChaCha20 / XChaCha20 / Poly1305 AEAD:** algorithms, mode wiring, SIMD options, dedicated tests — built **on top of** the settled architecture from package 1. +3. Optional further splits (if review load requires it): AES-NI, CPU support, options/ASM fixes as separate changes. +4. Until that split exists, treat PR #90 as a **reference implementation / donor branch**, not as the integration path. + +Rationale: architecture changes redefine how authenticated modes work for the whole library. Reviewing them together with a new cipher family makes regressions harder to attribute and forces an all-or-nothing review of ~8k LOC. + +--- + +## 2. Prerequisite: complete DUnit → DUnitX migration + +**Before** tackling the AEAD architecture split (and before large cipher-mode refactors), the unit-test suite must be **fully migrated to DUnitX**. + +### 2.1 Current state (as of `development` / start of `Cleanup_OM`) + +| Item | Status | +|---|---| +| Classic DUnit project | `Unit Tests/DECDUnitTestSuite.dpr` (+ `.dproj`) — uses `TestFramework` | +| DUnitX project | `Unit Tests/DECDUnitXTestSuite.dpr` (+ `.dproj`) | +| Shared switch | `Unit Tests/Tests/TestDefines.inc` — `{.$DEFINE DUnitX}` (**off by default**) | +| Dual-stack tests | Most units under `Unit Tests/Tests/*.pas` use `{$IFDEF DUnitX}` for uses / `[TestFixture]` / registration, and still fall back to classic `TestFramework` / DUnit runners | +| Compatibility layer | Several units import `DUnitX.DUnitCompatibility` when DUnitX is on — transitional, not end state | +| Coverage tooling | `Unit Tests/CodeCoverage/` historically oriented around the DUnit-era suite | + +The suite is therefore **dual-stack**: one codebase, two frameworks, compile-time selected. That is useful historically, but it: + +- doubles project/maintenance surface, +- encourages DUnit-shaped APIs (`TTestCase`, compatibility shims) instead of idiomatic DUnitX, +- complicates CI (“which runner is authoritative?”), +- will fight larger AEAD refactors that need clear, single-runner green/red signals. + +### 2.2 Target state + +| Item | Target | +|---|---| +| Framework | **DUnitX only** | +| Classic DUnit project | Removed (or left unmaintained only if a temporary deprecation notice is required — prefer removal) | +| `TestDefines.inc` dual mode | Removed or reduced to non-framework defines; no DUnit fallback | +| Test units | Idiomatic DUnitX: `[TestFixture]`, `[Test]`, `Assert.*` / DUnitX asserts, `TDUnitX.RegisterTestFixture` (or attribute discovery as chosen consistently) | +| `DUnitX.DUnitCompatibility` | Eliminated once call sites no longer need it | +| Default / CI runner | `DECDUnitXTestSuite` (console; optional GUI/TestInsight kept only if still useful) | +| Documentation | README / CONTRIBUTING mention DUnitX only | + +Success criteria: + +1. All existing tests compile and run under DUnitX without `TestFramework` / classic DUnit packages. +2. No remaining `{$IFDEF DUnitX}` dual paths in test units (except temporary WIP on a sub-branch). +3. One documented way to run the full suite from IDE and command line. +4. Code coverage (if kept) wired to the DUnitX suite. + +### 2.3 Suggested migration steps (implementation order) + +These steps are guidance for a later implementation PR (or series of PRs on `Cleanup_OM`): + +1. **Baseline:** enable `DUnitX` in `TestDefines.inc`, run full `DECDUnitXTestSuite`, record pass/fail inventory. +2. **Project cleanup:** make DUnitX the only suite in the group project; stop shipping dual search paths for DUnit. +3. ** mechanize unit-by-unit:** drop `TestFramework` uses and `{$ELSE}` DUnit branches; keep behaviour identical. +4. **Remove compatibility layer:** replace remaining DUnit-compatible assert/test-case patterns with native DUnitX. +5. **Delete** `DECDUnitTestSuite` (and obsolete ModelSupport / DUnit-only assets if unused). +6. **Update docs** (readme “Has it been tested?”, CONTRIBUTING) and any coverage scripts. +7. **Optional:** align layout/naming later (`tests/` etc.) — out of scope for the framework migration itself. + +Do **not** mix this migration with AEAD architecture or ChaCha feature work in the same PR. + +--- + +## 3. Agreed overall work order + +``` +Cleanup_OM (and follow-up PRs → development) +│ +├─ 0. Repo hygiene (started) +│ e.g. DelphiStandards .gitignore +│ +├─ 1. DUnit → DUnitX migration (complete) ← next major technical step +│ Single test runner, no dual-stack +│ +├─ 2. AEAD architecture split (from PR #90 concern A) +│ Base + GCM stream/multi-call only; review as core API change +│ +├─ 3. ChaCha / XChaCha / Poly1305 (from PR #90 concern B) +│ On top of settled AEAD base; algorithms + tests +│ +└─ 4. Optional: AES-NI / CPU support / options fixes + As separate reviewable packages if not absorbed earlier +``` + +### Why this order + +1. **Tests first:** architecture and cipher merges need a single, trustworthy automated suite. Dual DUnit/DUnitX weakens that signal. +2. **Architecture before features:** ChaCha-as-AEAD depends on (or must not re-introduce) the multi-call authenticated-mode model; shipping ChaCha on the old GCM-only shape and then rewriting the base again is wasted motion. +3. **Separation of review:** maintainers can accept/reject AEAD API changes without blocking or rubber-stamping a large SIMD cipher contribution. + +--- + +## 4. Out of scope for this document + +- Detailed AEAD class design (belongs in a design note when step 2 starts). +- Accept/reject decision on individual PR #90 commits. +- Delphi style / layout alignment with external house standards beyond what is already done (e.g. `.gitignore`). +- FPC/Lazarus policy (may interact with ASM defines from the donor PR; track separately). + +--- + +## 5. References + +| Item | Location | +|---|---| +| Working branch | `Cleanup_OM` | +| Integration target | `development` | +| Donor PR | https://github.com/MHumm/DelphiEncryptionCompendium/pull/90 | +| Donor branch (origin target) | `development_chacha20poly1305` | +| Donor head (implementation) | `mikerabat/DelphiEncryptionCompendium` branch `development` | +| DUnit project | `Unit Tests/DECDUnitTestSuite.*` | +| DUnitX project | `Unit Tests/DECDUnitXTestSuite.*` | +| Dual-stack switch | `Unit Tests/Tests/TestDefines.inc` | + +--- + +*Document created as part of cleanup planning on `Cleanup_OM`. Update this file when a step completes or a decision changes.* From 711d525e678516f44e74312616f6bf425d710116 Mon Sep 17 00:00:00 2001 From: Olaf Monien Date: Wed, 22 Jul 2026 13:13:34 +0200 Subject: [PATCH 02/15] Document DUnitX migration plan; keep DUnit for parity comparison. Inventory dual suites, sense-check existing tests, and record that DECDUnitTestSuite stays until a later removal PR. --- Docs/Cleanup-Roadmap.md | 70 ++-- Docs/plans/2026-07-22-dunitx-migration.md | 415 ++++++++++++++++++++++ 2 files changed, 446 insertions(+), 39 deletions(-) create mode 100644 Docs/plans/2026-07-22-dunitx-migration.md diff --git a/Docs/Cleanup-Roadmap.md b/Docs/Cleanup-Roadmap.md index 5abe3fb..8760764 100644 --- a/Docs/Cleanup-Roadmap.md +++ b/Docs/Cleanup-Roadmap.md @@ -38,58 +38,50 @@ Rationale: architecture changes redefine how authenticated modes work for the wh --- -## 2. Prerequisite: complete DUnit → DUnitX migration +## 2. Prerequisite: DUnit → DUnitX migration (DUnit kept for comparison) -**Before** tackling the AEAD architecture split (and before large cipher-mode refactors), the unit-test suite must be **fully migrated to DUnitX**. +**Before** tackling the AEAD architecture split (and before large cipher-mode refactors), DUnitX must become a **complete, parity-proven** runner for the whole suite. -### 2.1 Current state (as of `development` / start of `Cleanup_OM`) +### 2.1 Binding constraint: keep DUnit temporarily + +The classic **DUnit suite stays** for the time being so we can **compare results** (same tests, both runners) and prove the migration did not drop or alter behaviour. + +| Phase | DUnit (`DECDUnitTestSuite`) | DUnitX (`DECDUnitXTestSuite`) | +|---|---|---| +| **Now / migration branch** | Keep; baseline for comparison | Bring to full parity; make authoritative for new work | +| **Later (separate PR)** | Remove after explicit decision | Only runner; drop dual-stack / compatibility | + +Detailed plan: **[`Docs/plans/2026-07-22-dunitx-migration.md`](plans/2026-07-22-dunitx-migration.md)** +Branch: `Cleanup_OM-DUnitX-migration` + +### 2.2 Current state (as of `development` / start of cleanup) | Item | Status | |---|---| | Classic DUnit project | `Unit Tests/DECDUnitTestSuite.dpr` (+ `.dproj`) — uses `TestFramework` | -| DUnitX project | `Unit Tests/DECDUnitXTestSuite.dpr` (+ `.dproj`) | -| Shared switch | `Unit Tests/Tests/TestDefines.inc` — `{.$DEFINE DUnitX}` (**off by default**) | -| Dual-stack tests | Most units under `Unit Tests/Tests/*.pas` use `{$IFDEF DUnitX}` for uses / `[TestFixture]` / registration, and still fall back to classic `TestFramework` / DUnit runners | -| Compatibility layer | Several units import `DUnitX.DUnitCompatibility` when DUnitX is on — transitional, not end state | -| Coverage tooling | `Unit Tests/CodeCoverage/` historically oriented around the DUnit-era suite | +| DUnitX project | `Unit Tests/DECDUnitXTestSuite.dpr` (+ `.dproj`) — **incomplete** vs DUnit (missing CCM, ZIP helper, AEAD common test data) | +| Shared switch | `Unit Tests/Tests/TestDefines.inc` — `{.$DEFINE DUnitX}` (**off by default**); DUnitX dproj supplies define | +| Dual-stack tests | Most units use `{$IFDEF DUnitX}` + `DUnitX.DUnitCompatibility` vs classic `TestFramework` | +| Sense check | Existing tests are largely **meaningful** (vectors, regressions); weak spots documented in the plan, not migration blockers | -The suite is therefore **dual-stack**: one codebase, two frameworks, compile-time selected. That is useful historically, but it: +### 2.3 Target state after migration branch (DUnit still present) -- doubles project/maintenance surface, -- encourages DUnit-shaped APIs (`TTestCase`, compatibility shims) instead of idiomatic DUnitX, -- complicates CI (“which runner is authoritative?”), -- will fight larger AEAD refactors that need clear, single-runner green/red signals. +| Item | Target | +|---|---| +| DUnitX project | **Full** unit list matching DUnit | +| Parity | No DUnitX-only failures; fixture sets aligned | +| DUnit project | Still builds and runs (comparison) | +| Assert style | May still use `Check*` via `DUnitCompatibility` | +| Dual `IFDEF` | Still allowed | -### 2.2 Target state +### 2.4 Later target (after comparison period — not this branch) | Item | Target | |---|---| | Framework | **DUnitX only** | -| Classic DUnit project | Removed (or left unmaintained only if a temporary deprecation notice is required — prefer removal) | -| `TestDefines.inc` dual mode | Removed or reduced to non-framework defines; no DUnit fallback | -| Test units | Idiomatic DUnitX: `[TestFixture]`, `[Test]`, `Assert.*` / DUnitX asserts, `TDUnitX.RegisterTestFixture` (or attribute discovery as chosen consistently) | -| `DUnitX.DUnitCompatibility` | Eliminated once call sites no longer need it | -| Default / CI runner | `DECDUnitXTestSuite` (console; optional GUI/TestInsight kept only if still useful) | -| Documentation | README / CONTRIBUTING mention DUnitX only | - -Success criteria: - -1. All existing tests compile and run under DUnitX without `TestFramework` / classic DUnit packages. -2. No remaining `{$IFDEF DUnitX}` dual paths in test units (except temporary WIP on a sub-branch). -3. One documented way to run the full suite from IDE and command line. -4. Code coverage (if kept) wired to the DUnitX suite. - -### 2.3 Suggested migration steps (implementation order) - -These steps are guidance for a later implementation PR (or series of PRs on `Cleanup_OM`): - -1. **Baseline:** enable `DUnitX` in `TestDefines.inc`, run full `DECDUnitXTestSuite`, record pass/fail inventory. -2. **Project cleanup:** make DUnitX the only suite in the group project; stop shipping dual search paths for DUnit. -3. ** mechanize unit-by-unit:** drop `TestFramework` uses and `{$ELSE}` DUnit branches; keep behaviour identical. -4. **Remove compatibility layer:** replace remaining DUnit-compatible assert/test-case patterns with native DUnitX. -5. **Delete** `DECDUnitTestSuite` (and obsolete ModelSupport / DUnit-only assets if unused). -6. **Update docs** (readme “Has it been tested?”, CONTRIBUTING) and any coverage scripts. -7. **Optional:** align layout/naming later (`tests/` etc.) — out of scope for the framework migration itself. +| DUnit project | Removed | +| Native asserts | `Assert.*`; drop `DUnitCompatibility` | +| Docs / CI | DUnitX only | Do **not** mix this migration with AEAD architecture or ChaCha feature work in the same PR. diff --git a/Docs/plans/2026-07-22-dunitx-migration.md b/Docs/plans/2026-07-22-dunitx-migration.md new file mode 100644 index 0000000..48395fa --- /dev/null +++ b/Docs/plans/2026-07-22-dunitx-migration.md @@ -0,0 +1,415 @@ +# DUnit → DUnitX Migration Plan + +> **For agentic workers:** Implement task-by-task on branch `Cleanup_OM-DUnitX-migration`. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make DUnitX the complete, authoritative unit-test runner for DEC while temporarily keeping the classic DUnit suite for parity comparison. + +**Architecture:** Keep the dual-project layout during migration. Bring the DUnitX project to full parity with DUnit (all units, all fixtures, same assertions via `DUnitX.DUnitCompatibility` first). Prove green/green equivalence. Only after that, optionally modernize asserts / drop compatibility — **without** deleting DUnit yet. + +**Tech Stack:** Delphi DUnitX (`DUnitX.TestFramework`, console + NUnit XML logger), existing DUnit (`TestFramework`, GUI/Text runners), shared test units under `Unit Tests/Tests/`. + +**Branch:** `Cleanup_OM-DUnitX-migration` (from `Cleanup_OM`) +**PR target (later):** `Cleanup_OM` → eventually `development` + +## Global Constraints + +- **Keep DUnit for now.** `DECDUnitTestSuite` stays buildable and runnable as a comparison baseline until we explicitly decide to remove it (separate decision, not part of the first migration PR). +- **Do not change production crypto code** in this migration unless a test reveals a real bug; then fix in a separate commit with a clear message. +- **Do not mix** AEAD architecture or ChaCha (PR #90) into this work. +- Prefer **behaviour-preserving** migration: same tests, same vectors, same pass/fail meaning. +- Project language for commits/docs in this package: **English** (matches DEC upstream). +- Encoding for any touched `.pas`: preserve existing style; do not mass-reformat. + +--- + +## 0. Audit summary (pre-migration) + +### 0.1 Projects + +| Project | Role today | Keep? | +|---|---|---| +| `Unit Tests/DECDUnitTestSuite.dpr` | Classic DUnit (GUI/console, TestInsight optional) | **Yes, temporarily** — comparison baseline | +| `Unit Tests/DECDUnitXTestSuite.dpr` | DUnitX console runner | **Yes** — target authoritative suite | +| `Unit Tests/Tests/TestDefines.inc` | `{.$DEFINE DUnitX}` off by default | Will enable for DUnitX path; DUnit path remains without define | + +### 0.2 Coverage gap (critical) + +DUnitX project is **behind** DUnit. Missing from `DECDUnitXTestSuite.dpr`: + +| Unit | In DUnit? | In DUnitX? | Notes | +|---|---|---|---| +| `TestDECCipherModesCCM.pas` | Yes | **No** | CCM mode + NIST-style vectors | +| `TestDECZIPHelper.pas` | Yes | **No** | ZIP crypto algorithm helper | +| `AuthenticatedCiphersCommonTestData.pas` | Yes | **No** | Shared data (not a fixture; needed by CCM/GCM) | + +All other test units appear in both projects. + +### 0.3 Dual-stack pattern (current) + +Almost every fixture unit does: + +```pascal +{$INCLUDE TestDefines.inc} +uses + {$IFDEF DUnitX} + DUnitX.TestFramework, DUnitX.DUnitCompatibility, + {$ELSE} + TestFramework, + {$ENDIF} + ... +type + {$IFDEF DUnitX} [TestFixture] {$ENDIF} + TMyTests = class(TTestCase) + published + procedure TestSomething; + end; + +initialization + {$IFDEF DUnitX} + TDUnitX.RegisterTestFixture(...); + {$ELSE} + RegisterTests(...); + {$ENDIF} +``` + +Assertions are overwhelmingly **DUnit-style** (`CheckEquals` ~1393, `CheckException` ~87, `CheckNotEquals` ~39). Native `Assert.*` is rare (~2× `Assert.Fail`). +→ Migration phase 1 keeps `DUnitX.DUnitCompatibility` so bodies stay unchanged. + +`DECDUnitXTestSuite.dpr` sets `runner.FailsOnNoAsserts := False` — empty tests would not fail. Leave as-is until parity is proven; consider enabling later as a quality gate. + +### 0.4 Sense check: do the DUnit tests make sense? + +#### Keep / meaningful (vast majority) + +| Area | Unit(s) | Verdict | +|---|---|---| +| Base registry | `TestDECBaseClass` | Real class list / identity / short name tests | +| Formats | `TestDECFormat`, `TestDECFormatBase` | Encode/decode + registry; exception helpers for invalid names/ids | +| Hashes | `TestDECHash`, `TestDECHashSHA3`, `TestDECHashKDF`, `TestDECHashMAC` | Large vector suites; SHA3 uses NIST files under `Unit Tests/Data/`; HMAC/PBKDF2 incl. bug #46 regression | +| Ciphers | `TestDECCipher` | Per-algorithm fixtures + key/IV edge cases | +| Modes (classic) | `TestDECCipherModes` | ECBx, OFB8/x, CFB8/x, CFS8/x, CBCx, CTSx; also touches GCM/CCM for auth-API failure paths | +| GCM | `TestDECCipherModesGCM` | NIST-style vectors, auth failure, stream/chunk notes | +| CCM | `TestDECCipherModesCCM` | Dedicated CCM suite (must be on DUnitX) | +| Paddings | `TestDECCipherPaddings` | Positive/negative padding remove; dual Fail/Assert.Fail already | +| Formats API | `TestDECCipherFormats` | Bytes/stream/string encode-decode matrix | +| Util | `TestDECUtil` | Bit ops, protect buffer/stream/string, `IsEqual`, Shannon entropy | +| Random | `TestDECRandom` | **Deterministic** sequences with fixed seed — sensible regression, not “true RNG quality” (documented in unit header) | +| ZIP | `TestDECZIPHelper` | Class resolution + unknown algorithm exceptions | +| Infrastructure | `TestDECTestDataContainer`, `AuthenticatedCiphersCommonTestData` | Support, not empty “fake” suites | + +#### Intentional “empty” code (not abandoned tests) + +- `TestDECHash.pas`: stub overrides (`DoInit`/`DoDone`/`DoTransform` “Empty on purpose”) on **test doubles** (`TDECHashIncrement8`, abstract-error probes) — **OK**. +- Empty `SetUp`/`TearDown` where only class methods are tested — **OK**. + +#### Weak or incomplete (document; fix only if cheap / separate commit) + +| Issue | Where | Assessment | +|---|---|---| +| `Check(true)` after successful CCM IV init loop | `TestDECCipherModesCCM` | Only proves “no exception”; weak. Prefer no assert needed if exception is the failure mode, or assert mode/state. **Do not block migration.** | +| CCM large-stream tests commented out | `TestDECCipherModesCCM` | Incomplete vs GCM; track for AEAD work later, not DUnitX migration | +| GCM stream chunking TODOs / dead commented code | `TestDECCipherModesGCM` | Cleanup optional; behaviour tests still present | +| `TestDECCipherModes` uses mostly `TCipher_Null` + short strings | Mode wiring regression, not full AES-CBC NIST vectors | **Still useful** for mode dispatch; real crypto vectors live in cipher/GCM/CCM units | +| File encode/decode tests commented | `TestDECCipherFormats` | Optional future; not required for migration | +| SHA3 TODOs (naming / comments) | `TestDECHashSHA3` | Cosmetic / follow-up | +| Cipher TODOs (“Should be specified via FTestData?”) | `TestDECCipher` | Design debt; tests still assert | + +**Overall:** The suite is **substantively meaningful** as a regression pack. No large blocks of skeleton tests that should be deleted before migration. Weak spots are known and secondary. + +### 0.5 Comparison protocol (DUnit kept on purpose) + +While both runners exist: + +1. Build and run **DUnit** suite (`DECDUnitTestSuite`, console if possible) → save log / failure list. +2. Build and run **DUnitX** suite (`DECDUnitXTestSuite`) → NUnit XML + console. +3. **Parity rule:** same fixtures must pass on both; any DUnitX-only failure is a migration bug until proven otherwise. +4. New tests added during/after migration go into shared units and must be registered for **both** runners until DUnit is retired. + +--- + +## File map (what will change) + +| Path | Action | +|---|---| +| `Unit Tests/Tests/TestDefines.inc` | Document dual use; keep `DUnitX` define for DUnitX builds | +| `Unit Tests/DECDUnitXTestSuite.dpr` | Add missing units; ensure include/define story is reliable | +| `Unit Tests/DECDUnitXTestSuite.dproj` | Mirror unit list / search paths / `DUnitX` define | +| `Unit Tests/DECDUnitTestSuite.dpr` / `.dproj` | **Keep**; only touch if registration/shared unit needs both | +| `Unit Tests/Tests/*.pas` | Ensure every fixture registers under both `IFDEF` branches; remove dead dual bugs only | +| `Docs/Cleanup-Roadmap.md` | Link to this plan; note “DUnit retained for comparison” | +| `readme.md` (optional later) | Mention both runners until DUnit removal | + +--- + +## Task 1: Baseline both runners + +**Files:** none (measurement only), or add `Docs/plans/dunitx-parity-log.md` if useful + +- [ ] **Step 1: Confirm branch** + +```bash +git branch --show-current +# expect: Cleanup_OM-DUnitX-migration +``` + +- [ ] **Step 2: Build DUnit suite (Win32 Debug preferred)** + +Use project `Unit Tests/DECDUnitTestSuite.dproj` with Delphi MSBuild / IDE / `DelphiBuildDPROJ.ps1` if available in environment. + +Expected: successful compile. + +- [ ] **Step 3: Run DUnit suite** + +Prefer console: define `CONSOLE_TESTRUNNER` if needed, or run GUI and export results. + +Record: total tests, failures, errors, ignored. + +- [ ] **Step 4: Build DUnitX suite as-is (before fixes)** + +`Unit Tests/DECDUnitXTestSuite.dproj` — ensure `DUnitX` is defined in the project (dproj already has `DUnitX;DEBUG` etc.). +Also ensure `TestDefines.inc` define is **on** for DUnitX builds (today default is **off** — see Task 2). + +- [ ] **Step 5: Run DUnitX suite as-is** + +Record baseline. Expect possible missing CCM/ZIP coverage even if green. + +- [ ] **Step 6: Commit only if you added a parity log file; otherwise no commit** + +--- + +## Task 2: Make `DUnitX` define reliable for the DUnitX project + +**Problem:** `TestDefines.inc` has `{.$DEFINE DUnitX}` commented out. DUnitX dpr includes it, but dproj also defines `DUnitX`. Relying on both is confusing; DUnit must **not** define `DUnitX`. + +**Files:** +- Modify: `Unit Tests/Tests/TestDefines.inc` +- Modify: `Unit Tests/DECDUnitXTestSuite.dpr` (comments) +- Verify: `Unit Tests/DECDUnitTestSuite.dproj` has **no** `DUnitX` in `DCC_Define` + +- [ ] **Step 1: Document the switch in `TestDefines.inc`** + +Keep default **off** so opening test units under the DUnit project still compiles as DUnit. Rely on **project-level** `DCC_Define=DUnitX` for the DUnitX project (already present in dproj). Update the comment to say: + +```pascal +/// +/// When DUnitX is defined (typically via DECDUnitXTestSuite project options), +/// unit tests compile against DUnitX + DUnitCompatibility. +/// When undefined (DECDUnitTestSuite), classic DUnit TestFramework is used. +/// Do not enable the define here permanently — that would break the DUnit project. +/// +{.$DEFINE DUnitX} +``` + +- [ ] **Step 2: Verify DUnitX dproj defines `DUnitX` for all configs used** + +Debug/Release (and GUI if present) must include `DUnitX` in `DCC_Define`. + +- [ ] **Step 3: Verify DUnit dproj does not define `DUnitX`** + +- [ ] **Step 4: Rebuild both projects** + +Expected: both compile. + +- [ ] **Step 5: Commit** + +```bash +git add "Unit Tests/Tests/TestDefines.inc" "Unit Tests/DECDUnitXTestSuite.dpr" +git commit -m "Clarify DUnit vs DUnitX define ownership for dual-suite builds." +``` + +--- + +## Task 3: Close the DUnitX project gap (CCM, ZIP, shared data) + +**Files:** +- Modify: `Unit Tests/DECDUnitXTestSuite.dpr` +- Modify: `Unit Tests/DECDUnitXTestSuite.dproj` (add units the IDE/MSBuild need) +- Verify: `TestDECCipherModesCCM.pas`, `TestDECZIPHelper.pas`, `AuthenticatedCiphersCommonTestData.pas` already have dual-stack `IFDEF` registration + +- [ ] **Step 1: Add to `DECDUnitXTestSuite.dpr` uses clause** (order flexible; keep near related units): + +```pascal + TestDECCipherModesGCM in 'Tests\TestDECCipherModesGCM.pas', + TestDECCipherModesCCM in 'Tests\TestDECCipherModesCCM.pas', + TestDECCipherPaddings in 'Tests\TestDECCipherPaddings.pas', + TestDECZIPHelper in 'Tests\TestDECZIPHelper.pas', + AuthenticatedCiphersCommonTestData in 'Tests\AuthenticatedCiphersCommonTestData.pas'; +``` + +(Adjust if some lines already exist — avoid duplicates.) + +- [ ] **Step 2: Add the same units to `DECDUnitXTestSuite.dproj`** + +Prefer IDE “add unit” or careful `DCCReference` entries matching existing style. + +- [ ] **Step 3: Confirm CCM/ZIP units register fixtures under `{$IFDEF DUnitX}`** + +Each should call `TDUnitX.RegisterTestFixture(...)` in `initialization`. + +- [ ] **Step 4: Build + run DUnitX** + +Expected: CCM and ZIP tests appear and run. + +- [ ] **Step 5: Run DUnit again (sanity — should be unchanged)** + +- [ ] **Step 6: Commit** + +```bash +git add "Unit Tests/DECDUnitXTestSuite.dpr" "Unit Tests/DECDUnitXTestSuite.dproj" +git commit -m "Include CCM, ZIP helper, and AEAD test data in DUnitX suite." +``` + +--- + +## Task 4: Fixture registration audit (every unit dual-clean) + +**Files:** all `Unit Tests/Tests/Test*.pas` with fixtures + +- [ ] **Step 1: Inventory registration** + +For each fixture unit, ensure: + +| Branch | Required | +|---|---| +| `{$IFDEF DUnitX}` | `[TestFixture]` on classes + `TDUnitX.RegisterTestFixture` (or documented RTTI-only discovery — currently suite uses both `UseRTTI := True` **and** explicit register; keep explicit register for consistency) | +| `{$ELSE}` | `RegisterTest` / `RegisterTests` as today | + +- [ ] **Step 2: Fix any unit that only registers one side** + +Known OK from audit: most units already dual-register. Re-check after CCM/ZIP inclusion. + +- [ ] **Step 3: `TestDECUtil` fixtures** + +Methods are not all named `Test*` (e.g. `ReverseBits32`) but are `published` — DUnit picks them up; DUnitX via `TTestCase` compatibility should too. Run suite and confirm counts roughly match. + +- [ ] **Step 4: Commit only if fixes were needed** + +```bash +git commit -m "Fix dual-stack test registration gaps for DUnitX parity." +``` + +--- + +## Task 5: Parity run (pass/fail comparison) + +**Files:** optional log under `Docs/plans/` + +- [ ] **Step 1: Run full DUnit suite, capture output** + +- [ ] **Step 2: Run full DUnitX suite, capture console + NUnit XML** + +- [ ] **Step 3: Compare** + +| Check | Criterion | +|---|---| +| Failures on DUnitX not on DUnit | **Blocker** — fix migration | +| Failures on both | Product/test bug — fix or document, not “DUnitX issue” | +| Tests only on one runner | **Blocker** — registration/project gap | +| Count mismatch | Investigate (`TestUtil` naming, ignored tests, RTTI double-registration) | + +Watch for **double execution** if both RTTI and `RegisterTestFixture` register the same class twice under DUnitX. If duplicates appear, set `runner.UseRTTI := False` **or** remove redundant registration — pick one strategy suite-wide (prefer **explicit RegisterTestFixture + UseRTTI False** for predictability). + +- [ ] **Step 4: Fix DUnitX runner if double-registration** + +In `DECDUnitXTestSuite.dpr`: + +```pascal +runner.UseRTTI := False; // fixtures registered explicitly in unit initialization +``` + +Re-run parity. + +- [ ] **Step 5: Commit runner tweak + any registration fixes** + +```bash +git commit -m "Stabilize DUnitX discovery to match DUnit fixture set." +``` + +--- + +## Task 6: Small sense-check cleanups (only if zero behaviour risk) + +Do **not** expand scope. Optional in same branch if parity is green: + +- [ ] Replace `Check(true)` in CCM IV success path with a real assertion or remove and rely on exception — **only** if still equivalent. +- [ ] Delete clearly dead commented stream tests **or** leave with a single `// Deferred: multi-call CCM streams (AEAD roadmap)` note — do not implement streams here. +- [ ] Ensure `TestDECCipherPaddings` does not need both `Fail` and `Assert.Fail` under the same `IFDEF` in a broken way (read both branches). + +- [ ] **Commit only if something landed** + +```bash +git commit -m "Minor test clarity fixes found during DUnitX parity review." +``` + +--- + +## Task 7: Documentation + +**Files:** +- Modify: `Docs/Cleanup-Roadmap.md` (link + “DUnit retained for comparison”) +- This plan: mark tasks done as work proceeds +- Optional: short note in `readme.md` under “Has it been tested?” + +- [ ] **Step 1: Update Cleanup-Roadmap section 2** + +State explicitly: + +- DUnitX is the target authoritative runner. +- DUnit suite remains until parity is trusted and a later PR removes it. +- Link to `Docs/plans/2026-07-22-dunitx-migration.md`. + +- [ ] **Step 2: Commit** + +```bash +git add Docs/Cleanup-Roadmap.md Docs/plans/2026-07-22-dunitx-migration.md +git commit -m "Document DUnitX migration plan and dual-suite comparison policy." +``` + +--- + +## Task 8: Done criteria for this branch (mergeable to `Cleanup_OM`) + +Migration branch is **complete enough to merge** when: + +1. DUnitX project includes **all** test units DUnit has (incl. CCM, ZIP, shared AEAD data). +2. Both suites **compile** on the supported Delphi version used in this environment. +3. Parity run: **no DUnitX-only failures**; fixture sets aligned (no missing suites). +4. DUnit project **still present and green** (comparison baseline). +5. Docs updated; no ChaCha/AEAD drive-by changes. +6. `DUnitX.DUnitCompatibility` may still be used — **OK** for this phase. + +**Explicitly deferred (later PRs):** + +- Remove `DECDUnitTestSuite` and all `{$ELSE}` DUnit branches. +- Replace `Check*` with native `Assert.*` and drop `DUnitCompatibility`. +- Enable `FailsOnNoAsserts := True`. +- Rework weak tests / implement CCM stream tests. +- Folder rename `Unit Tests` → `tests`. + +--- + +## Suggested commit series (summary) + +1. Clarify define ownership +2. Add missing units to DUnitX project +3. Registration / UseRTTI stabilization +4. Optional minor test clarity +5. Docs + +--- + +## References + +| Item | Path / link | +|---|---| +| Roadmap | `Docs/Cleanup-Roadmap.md` | +| DUnit project | `Unit Tests/DECDUnitTestSuite.*` | +| DUnitX project | `Unit Tests/DECDUnitXTestSuite.*` | +| Define switch | `Unit Tests/Tests/TestDefines.inc` | +| NIST/GCM/SHA3 data | `Unit Tests/Data/` | +| Collector branch | `Cleanup_OM` | +| This feature branch | `Cleanup_OM-DUnitX-migration` | + +--- + +*Plan written after full dual-suite inventory and sense-check of existing DUnit tests. DUnit retention for comparison is a binding constraint until a later removal PR.* From 01e8c39f84bd9f6be2f506daf0c77f91302bfd64 Mon Sep 17 00:00:00 2001 From: Olaf Monien Date: Wed, 22 Jul 2026 13:17:57 +0200 Subject: [PATCH 03/15] Document DUnitX DPR best-practice audit against official examples. Compare DECDUnitXTestSuite.dpr to VSoft D12/D13 templates; fold runner hardening into Task 3 of the migration plan. --- Docs/plans/2026-07-22-dunitx-migration.md | 162 ++++++++++++++++++++-- 1 file changed, 149 insertions(+), 13 deletions(-) diff --git a/Docs/plans/2026-07-22-dunitx-migration.md b/Docs/plans/2026-07-22-dunitx-migration.md index 48395fa..b350283 100644 --- a/Docs/plans/2026-07-22-dunitx-migration.md +++ b/Docs/plans/2026-07-22-dunitx-migration.md @@ -127,13 +127,135 @@ While both runners exist: --- +## 0.6 DUnitX project (`.dpr`) best-practice audit + +Reference implementation: official **VSoftTechnologies/DUnitX** examples +[`Examples/DUnitXExamples_D12Athens.dpr`](https://github.com/VSoftTechnologies/DUnitX/blob/master/Examples/DUnitXExamples_D12Athens.dpr) / +[`Examples/DUnitXExamples_D13.dpr`](https://github.com/VSoftTechnologies/DUnitX/blob/master/Examples/DUnitXExamples_D13.dpr) +(plus framework notes on `{$STRONGLINKTYPES ON}` + manual `RegisterTestFixture` in `DUnitX.Examples.General.pas`). + +### Checklist: current `DECDUnitXTestSuite.dpr` vs best practice + +| Topic | Best practice (official / CI-ready) | DEC today | Verdict | +|---|---|---|---| +| **Console app type** | `{$IFNDEF TESTINSIGHT}{$APPTYPE CONSOLE}{$ENDIF}` | Same idea, wrapped in extra `{$IFNDEF GUI}` | OK; GUI path half-dead | +| **`{$STRONGLINKTYPES ON}`** | Required so RTTI/link keeps fixtures when using attributes/RTTI | Present | **OK** | +| **Command line** | `TDUnitX.CheckCommandLine` before run | Present | **OK** | +| **Runner** | `runner := TDUnitX.CreateRunner` | Present | **OK** | +| **UseRTTI** | `True` **or** explicit register only — not both without care | `UseRTTI := True` **and** units call `RegisterTestFixture` | **Risk of double registration** → prefer **one** strategy (plan Task 5: explicit + `UseRTTI := False`) | +| **FailsOnNoAsserts** | Examples leave `False`; quality gate often wants `True` | `False` | Keep `False` until parity; later enable | +| **Exit code** | Must set non-zero on failure for CI | Sets `ExitCode := EXIT_ERRORS` if `not results.AllPassed` | **Better than official sample** (sample often omits this) | +| **Exception path exit code** | Unhandled exception should fail process | `except` only `Writeln` — **ExitCode often stays 0** | **Gap** | +| **Console logger** | Honour `TDUnitX.Options.ConsoleMode` (Off / Quiet / full); Quiet flag from mode | Always `TDUnitXConsoleLogger.Create(true)` | **Gap** — ignores CLI/options | +| **NUnit XML logger** | Official: only under `{$IFDEF CI}`; always-on XML is also fine for local+CI | Always on via `TDUnitX.Options.XMLOutputFile` | Acceptable; document path | +| **JUnit XML** | Optional (`DUnitX.Loggers.XML.JUnit`) for some CI | Not used | Optional later | +| **CI behaviour** | `{$IFDEF CI}`: `ConsoleMode := Off`, no pause, XML on | Only `{$IFNDEF CI}` pause block; no CI console off | **Partial** | +| **Interactive pause** | Official sets `ExitBehavior := Pause` when not CI | Only pauses if already Pause | **Weaker interactive UX** | +| **TestInsight** | `{$IFDEF TESTINSIGHT}` → `TestInsight.DUnitX.RunRegisteredTests` (unit in uses) | Runtime probe `IsTestInsightRunning` + `TestInsight.Client`; compile path incomplete without defines | **Legacy pattern** — modernise to official ifdef | +| **IDE safety** | Comment: *keep comment here to protect the following conditional from being removed by the IDE when adding a unit* before `{$IFNDEF TESTINSIGHT} var` | Missing | **Gap** (Delphi 12+ known to mangle dpr) | +| **Memory leaks** | `ReportMemoryLeaksOnShutdown := True` common for test exes | Not in DUnitX dpr (DUnit suite has it) | **Gap** (nice-to-have) | +| **Timing** | Optional `TStopWatch` around `Execute` | Absent | Optional | +| **Dead GUI scaffolding** | Single console (+ TestInsight) is enough | Top `GUI`/`MobileGUI` defines, commented GUI runner, dproj configs GUI/MobileGUI | **Noise** — remove or finish, don’t leave half | +| **Complete uses list** | Every fixture unit + support units | Missing CCM, ZIP, AEAD common data | **Critical gap** (Task 3) | +| **`TestDefines.inc`** | Project `DCC_Define=DUnitX` is source of truth | File says “must enable define in inc” (outdated comment) | **Docs/define ownership** (Task 2) | +| **dproj `DUnitX` define** | All configs that run this suite | Debug/Release have `DUnitX`; GUI config may not | **Verify every config** | + +### Target shape for `DECDUnitXTestSuite.dpr` (migration target) + +Minimal structure aligned with DUnitX D13 example **plus** DEC needs (exit code, always-on XML optional, full unit list): + +```pascal +program DECDUnitXTestSuite; + +{$IFNDEF TESTINSIGHT} +{$APPTYPE CONSOLE} +{$ENDIF} +{$STRONGLINKTYPES ON} + +uses + System.SysUtils, + {$IFDEF TESTINSIGHT} + TestInsight.DUnitX, + {$ENDIF} + DUnitX.Loggers.Console, + DUnitX.Loggers.Xml.NUnit, + DUnitX.TestFramework, + // ... all TestDEC* units + AuthenticatedCiphersCommonTestData ... + ; + +{ keep comment here to protect the following conditional from being removed by the IDE when adding a unit } +{$IFNDEF TESTINSIGHT} +var + runner: ITestRunner; + results: IRunResults; + logger: ITestLogger; + nunitLogger: ITestLogger; +{$ENDIF} +begin +{$IFDEF TESTINSIGHT} + TestInsight.DUnitX.RunRegisteredTests; +{$ELSE} + try + ReportMemoryLeaksOnShutdown := True; + TDUnitX.CheckCommandLine; + + runner := TDUnitX.CreateRunner; + // Fixtures register in unit initialization — avoid double discovery: + runner.UseRTTI := False; + runner.FailsOnNoAsserts := False; // True after parity period + + {$IFDEF CI} + TDUnitX.Options.ConsoleMode := TDunitXConsoleMode.Off; + {$ELSE} + // TDUnitX.Options.ExitBehavior := TDUnitXExitBehavior.Pause; // optional interactive + {$ENDIF} + + if TDUnitX.Options.ConsoleMode <> TDunitXConsoleMode.Off then + begin + logger := TDUnitXConsoleLogger.Create( + TDUnitX.Options.ConsoleMode = TDunitXConsoleMode.Quiet); + runner.AddLogger(logger); + end; + + // NUnit XML for CI and local regression comparison + nunitLogger := TDUnitXXMLNUnitFileLogger.Create(TDUnitX.Options.XMLOutputFile); + runner.AddLogger(nunitLogger); + + results := runner.Execute; + if not results.AllPassed then + System.ExitCode := EXIT_ERRORS; + + {$IFNDEF CI} + if TDUnitX.Options.ExitBehavior = TDUnitXExitBehavior.Pause then + begin + System.Write('Done.. press key to quit.'); + System.Readln; + end; + {$ENDIF} + except + on E: Exception do + begin + System.Writeln(E.ClassName, ': ', E.Message); + System.ExitCode := 1; + end; + end; +{$ENDIF} +end. +``` + +### Implementation note + +Bringing the dpr to this shape is part of **Tasks 2–5** (define ownership, unit list, registration/UseRTTI, parity). Do not rewrite the dpr in isolation without re-running both suites for comparison. + +--- + ## File map (what will change) | Path | Action | |---|---| | `Unit Tests/Tests/TestDefines.inc` | Document dual use; keep `DUnitX` define for DUnitX builds | -| `Unit Tests/DECDUnitXTestSuite.dpr` | Add missing units; ensure include/define story is reliable | -| `Unit Tests/DECDUnitXTestSuite.dproj` | Mirror unit list / search paths / `DUnitX` define | +| `Unit Tests/DECDUnitXTestSuite.dpr` | Add missing units; align with §0.6 best-practice target shape | +| `Unit Tests/DECDUnitXTestSuite.dproj` | Mirror unit list / search paths / `DUnitX` define on **all** suite configs | | `Unit Tests/DECDUnitTestSuite.dpr` / `.dproj` | **Keep**; only touch if registration/shared unit needs both | | `Unit Tests/Tests/*.pas` | Ensure every fixture registers under both `IFDEF` branches; remove dead dual bugs only | | `Docs/Cleanup-Roadmap.md` | Link to this plan; note “DUnit retained for comparison” | @@ -219,11 +341,11 @@ git commit -m "Clarify DUnit vs DUnitX define ownership for dual-suite builds." --- -## Task 3: Close the DUnitX project gap (CCM, ZIP, shared data) +## Task 3: Close the DUnitX project gap + harden the `.dpr` (best practice) **Files:** -- Modify: `Unit Tests/DECDUnitXTestSuite.dpr` -- Modify: `Unit Tests/DECDUnitXTestSuite.dproj` (add units the IDE/MSBuild need) +- Modify: `Unit Tests/DECDUnitXTestSuite.dpr` (unit list **and** §0.6 runner shape) +- Modify: `Unit Tests/DECDUnitXTestSuite.dproj` (units + ensure `DUnitX` on all configs used for this suite) - Verify: `TestDECCipherModesCCM.pas`, `TestDECZIPHelper.pas`, `AuthenticatedCiphersCommonTestData.pas` already have dual-stack `IFDEF` registration - [ ] **Step 1: Add to `DECDUnitXTestSuite.dpr` uses clause** (order flexible; keep near related units): @@ -238,25 +360,39 @@ git commit -m "Clarify DUnit vs DUnitX define ownership for dual-suite builds." (Adjust if some lines already exist — avoid duplicates.) -- [ ] **Step 2: Add the same units to `DECDUnitXTestSuite.dproj`** +- [ ] **Step 2: Align runner bootstrap with §0.6 target shape** + +Required in this task (not deferred): + +1. IDE protection comment before conditional `var` block +2. `UseRTTI := False` if fixtures use explicit `RegisterTestFixture` (confirm after Task 4) — if Task 4 not done yet, set after registration audit; default recommendation: **False + explicit** +3. Console logger respects `TDUnitX.Options.ConsoleMode` +4. `except` sets `System.ExitCode := 1` +5. `ReportMemoryLeaksOnShutdown := True` +6. Remove or quarantine dead GUI/`MobileGUI` scaffolding in the dpr (commented GUI runner, unused top defines) +7. Modern TestInsight path: `TestInsight.DUnitX.RunRegisteredTests` under `{$IFDEF TESTINSIGHT}` (drop runtime `IsTestInsightRunning` if it conflicts) + +Keep: `CheckCommandLine`, NUnit XML logger, non-zero exit on `not AllPassed`, DUnit suite untouched. + +- [ ] **Step 3: Add the same units to `DECDUnitXTestSuite.dproj`** -Prefer IDE “add unit” or careful `DCCReference` entries matching existing style. +Prefer IDE “add unit” or careful `DCCReference` entries matching existing style. Ensure `DCC_Define` includes `DUnitX` for Debug/Release (and TestInsight if that config builds the same sources). -- [ ] **Step 3: Confirm CCM/ZIP units register fixtures under `{$IFDEF DUnitX}`** +- [ ] **Step 4: Confirm CCM/ZIP units register fixtures under `{$IFDEF DUnitX}`** Each should call `TDUnitX.RegisterTestFixture(...)` in `initialization`. -- [ ] **Step 4: Build + run DUnitX** +- [ ] **Step 5: Build + run DUnitX** -Expected: CCM and ZIP tests appear and run. +Expected: CCM and ZIP tests appear and run; process exit code non-zero on failure. -- [ ] **Step 5: Run DUnit again (sanity — should be unchanged)** +- [ ] **Step 6: Run DUnit again (sanity — should be unchanged)** -- [ ] **Step 6: Commit** +- [ ] **Step 7: Commit** ```bash git add "Unit Tests/DECDUnitXTestSuite.dpr" "Unit Tests/DECDUnitXTestSuite.dproj" -git commit -m "Include CCM, ZIP helper, and AEAD test data in DUnitX suite." +git commit -m "Complete DUnitX suite units and align DPR with DUnitX best practices." ``` --- From f3f16a8c5f4ac20bd273180b09f1cf1cebda6298 Mon Sep 17 00:00:00 2001 From: Olaf Monien Date: Wed, 22 Jul 2026 13:23:54 +0200 Subject: [PATCH 04/15] Task 1: baseline DUnit and DUnitX runners (parity log) Record Win32 Console DUnit (1425 run, 12 failures) and Debug DUnitX (1457 found, 15 failures) baselines, coverage gap (CCM/ZIP), and environment notes for the DUnitX migration. --- Docs/plans/dunitx-parity-log.md | 118 ++++++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) create mode 100644 Docs/plans/dunitx-parity-log.md diff --git a/Docs/plans/dunitx-parity-log.md b/Docs/plans/dunitx-parity-log.md new file mode 100644 index 0000000..e640d69 --- /dev/null +++ b/Docs/plans/dunitx-parity-log.md @@ -0,0 +1,118 @@ +# DUnit / DUnitX Parity Log + +Baseline measurement for DUnit → DUnitX migration (Task 1). +Branch: `Cleanup_OM-DUnitX-migration` +Date: 2026-07-22 +Host: Delphi 13 Florence (BDS 37.0), Win32 + +## How to reproduce + +1. Build library: `Source\DEC60.dproj` Config=Debug Platform=Win32, `/p:ProductVersion=37.0` +2. Build DUnit console: `Unit Tests\DECDUnitTestSuite.dproj` Config=**Console** Platform=Win32 +3. Build DUnitX: `Unit Tests\DECDUnitXTestSuite.dproj` Config=**Debug** Platform=Win32 + - On this machine DUnitX Win32 DCUs live under + `C:\Users\Public\Documents\Embarcadero\Studio\37.0\Bpl\Win32\Debug` + (not under `$(BDS)\lib\win32\…`). Pass them on the unit search path together with + `Compiled\DCU_IDE37.0_Win32_Debug` when building from MSBuild. +4. **CWD must be the EXE output directory** so relative data paths + `..\..\Unit Tests\Data\…` resolve to the repo’s `Unit Tests\Data\`. + Running from `Unit Tests\` resolves to `X:\Unit Tests\Data\` and fails massively. +5. DUnitX CLI: `DECDUnitXTestSuite.exe -exit:Continue -xml:\dunitx-results.xml` + +Logs (local, not committed): `.superpowers/sdd/baseline-logs/` + +--- + +## Project inventory + +| Project | Role | Config used | Output | +|---|---|---|---| +| `DECDUnitTestSuite` | Classic DUnit (keep for comparison) | Console (defines `CONSOLE_TESTRUNNER`) | `Compiled\BIN_IDE37.0_Win32_Console\` | +| `DECDUnitXTestSuite` | DUnitX console | Debug (defines `DUnitX;DEBUG`) | `Compiled\BIN_IDE37.0_Win32_Debug\` | +| `Tests\TestDefines.inc` | `{.$DEFINE DUnitX}` **off** by default | DUnitX project supplies define via dproj | — | + +### Units in DUnit only (coverage gap) + +| Unit | Approx. tests (from DUnit log) | +|---|---| +| `TestDECCipherModesCCM.pas` | 16 (`TestTDECCCM`) | +| `TestDECZIPHelper.pas` | 3 (`TestZIPHelpers`) | +| `AuthenticatedCiphersCommonTestData.pas` | support only (no fixture) | + +All other test units are referenced by both projects. + +--- + +## Baseline results (2026-07-22) + +### DUnit — Console / Win32 + +| Metric | Value | +|---|---| +| Run | **1425** | +| Failures | **12** | +| Errors | **0** | +| Ignored | (not reported separately) | +| Duration | ~1:15 | +| Exit code | 0 (DUnit TextTestRunner does not fail the process on test failures) | + +### DUnitX — Debug / Win32 (as-is, before migration fixes) + +| Metric | Value | +|---|---| +| Found | **1457** | +| Passed | **1442** | +| Failed | **15** | +| Errors | **0** | +| Ignored | **0** | +| Exit code | 1 (`EXIT_ERRORS` when not all passed) | + +> Count gap (1457 vs 1425): DUnitX reports **more** cases despite missing CCM+ZIP (~19 tests). Likely RTTI discovery / registration differences under `runner.UseRTTI := True` plus dual-stack registration. Not investigated further in Task 1. + +--- + +## Failure lists + +### Shared failures (same assertion intent on both runners) — 12 + +| # | Fixture / test | Notes | +|---|---|---| +| 1–7 | `TestTHash_Keccak_224` — `TestCalcStream`, `TestCalcStreamRawByteString`, `TestCalcStreamNoDone`, `TestCalcStreamNoDoneMulti`, `TestCalcBuffer`, `TestCalcBytes`, `TestCalcRawByteString` | Index 1 digest mismatch | +| 8 | `TestTHash_Keccak_224.TestCalcUnicodeString` | Index 1 empty expected vs non-empty | +| 9 | `TestTHash_Keccak_256.TestCalcUnicodeString` | Index 4 mismatch (input `<02>`) | +| 10 | `TestTHash_Keccak_384.TestCalcUnicodeString` | Index 4 mismatch | +| 11 | `TestTHash_Keccak_512.TestCalcUnicodeString` | Index 4 mismatch | +| 12 | `TestTDECGCM.TestEncodeStreamChunked` | Auth tag mismatch (Set 105) | + +These are **pre-existing product/test issues**, not migration bugs. Track separately from DUnitX parity work. + +### DUnitX-only failures — 3 + +| # | Fixture / test | DUnit result | Notes | +|---|---|---|---| +| 13 | `TestTISO10126Padding.TestAddPadding_RawByteString` | **Pass** (22 patterns) | ISO 10126 uses random pad bytes; possible non-determinism or DUnitX compatibility assert difference | +| 14 | `TestTISO10126Padding.TestAddPadding_Bytes` | **Pass** (22 patterns) | Same as above | +| 15 | `TestTHash_BCrypt.TestIsPasswordHash` | **Pass** | Expected `False`, got `True` under DUnitX only — investigate in migration | + +### DUnit-only coverage (not run on DUnitX yet) + +All **16** CCM + **3** ZIP tests **passed** on DUnit in this baseline. They are absent from `DECDUnitXTestSuite.dpr` (Task 3+). + +--- + +## Build notes / environment quirks + +1. **No** `build-scripts/DelphiBuildDPROJ.ps1` in this repo; used `rsvars.bat` + `msbuild` (Studio 37.0). +2. Library must be built first so `Compiled\DCU_IDE37.0_Win32_Debug` exists (test dproj search path points there, not at `Source\`). +3. DUnitX first compile without extra path: `F2613 Unit 'DUnitX.Loggers.Console' not found`. Fixed by adding Bpl Win32 Debug DCU dir. +4. Overriding only `DCC_UnitSearchPath` on the MSBuild command line **replaces** the dproj’s DEC DCU path — include **both** DEC DCU and DUnitX DCU paths. +5. `TestDefines.inc` still has `{.$DEFINE DUnitX}` commented; project define `DUnitX` in dproj is what enables the dual-stack branch for DUnitX builds today (Task 2 will harden this). + +--- + +## Parity rule (for later tasks) + +- Same fixtures must pass on both runners. +- DUnitX-only failures (ISO10126×2, BCrypt IsPasswordHash) are **migration-priority** until proven environmental. +- Shared 12 failures are baseline noise; do not block adding CCM/ZIP to DUnitX, but re-check they remain the same set after registration fixes. +- After Task 3 (add missing units), expect DUnitX total ≥ DUnit total and CCM/ZIP green. From 94a50d3c9848c3c264b22400de17c3244fa4df2c Mon Sep 17 00:00:00 2001 From: Olaf Monien Date: Wed, 22 Jul 2026 13:29:26 +0200 Subject: [PATCH 05/15] Clarify DUnit vs DUnitX define ownership for dual-suite builds. --- Unit Tests/DECDUnitXTestSuite.dpr | 6 +++--- Unit Tests/Tests/TestDefines.inc | 8 +++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/Unit Tests/DECDUnitXTestSuite.dpr b/Unit Tests/DECDUnitXTestSuite.dpr index 086f4bb..8e70792 100644 --- a/Unit Tests/DECDUnitXTestSuite.dpr +++ b/Unit Tests/DECDUnitXTestSuite.dpr @@ -3,9 +3,9 @@ {.$DEFINE MobileGUI} program DECDUnitXTestSuite; -// In order to run DEC Unit tests via DUnitX framework the $DEFINE DUnitX define -// in TestDefines.inc must be enabled, as it makes all the unit test units DUnitX -// compatible +// DUnitX is enabled by this project's DCC_Define (Debug/Release/GUI/…), not by +// uncommenting TestDefines.inc — leaving the inc define off keeps DECDUnitTestSuite +// on classic DUnit when the same test units are opened/built under that project. {$INCLUDE Tests\TestDefines.inc} {$IFNDEF GUI} diff --git a/Unit Tests/Tests/TestDefines.inc b/Unit Tests/Tests/TestDefines.inc index 97eb9c2..b0026ff 100644 --- a/Unit Tests/Tests/TestDefines.inc +++ b/Unit Tests/Tests/TestDefines.inc @@ -1,5 +1,7 @@ /// -/// When enabled the Unit tests can be run via DUnitX test framework but no -/// longer via DUnit test framework. +/// When DUnitX is defined (typically via DECDUnitXTestSuite project options), +/// unit tests compile against DUnitX + DUnitCompatibility. +/// When undefined (DECDUnitTestSuite), classic DUnit TestFramework is used. +/// Do not enable the define here permanently — that would break the DUnit project. /// -{.$DEFINE DUnitX} \ No newline at end of file +{.$DEFINE DUnitX} From c51d70406894862f595fe232a724e6180266236a Mon Sep 17 00:00:00 2001 From: Olaf Monien Date: Wed, 22 Jul 2026 13:35:43 +0200 Subject: [PATCH 06/15] Complete DUnitX suite units and align DPR with DUnitX best practices. --- Unit Tests/DECDUnitXTestSuite.dpr | 122 ++++++++++++---------------- Unit Tests/DECDUnitXTestSuite.dproj | 3 + 2 files changed, 54 insertions(+), 71 deletions(-) diff --git a/Unit Tests/DECDUnitXTestSuite.dpr b/Unit Tests/DECDUnitXTestSuite.dpr index 8e70792..bc857f5 100644 --- a/Unit Tests/DECDUnitXTestSuite.dpr +++ b/Unit Tests/DECDUnitXTestSuite.dpr @@ -1,25 +1,20 @@ -{$UNDEF GUI} -{.$DEFINE GUI} -{.$DEFINE MobileGUI} -program DECDUnitXTestSuite; +program DECDUnitXTestSuite; -// DUnitX is enabled by this project's DCC_Define (Debug/Release/GUI/…), not by +// DUnitX is enabled by this project's DCC_Define (Debug/Release/…), not by // uncommenting TestDefines.inc — leaving the inc define off keeps DECDUnitTestSuite // on classic DUnit when the same test units are opened/built under that project. {$INCLUDE Tests\TestDefines.inc} -{$IFNDEF GUI} - {$IFNDEF TESTINSIGHT} - {$APPTYPE CONSOLE} - {$ENDIF} +{$IFNDEF TESTINSIGHT} +{$APPTYPE CONSOLE} {$ENDIF} - {$STRONGLINKTYPES ON} + uses System.SysUtils, {$IFDEF TESTINSIGHT} - TestInsight.Client, - {$ENDIF } + TestInsight.DUnitX, + {$ENDIF} DUnitX.Loggers.Console, DUnitX.Loggers.Xml.NUnit, DUnitX.TestFramework, @@ -38,79 +33,64 @@ uses TestDECHashMAC in 'Tests\TestDECHashMAC.pas', TestDECHashSHA3 in 'Tests\TestDECHashSHA3.pas', TestDECCipherModesGCM in 'Tests\TestDECCipherModesGCM.pas', - TestDECCipherPaddings in 'Tests\TestDECCipherPaddings.pas'; + TestDECCipherModesCCM in 'Tests\TestDECCipherModesCCM.pas', + TestDECCipherPaddings in 'Tests\TestDECCipherPaddings.pas', + TestDECZIPHelper in 'Tests\TestDECZIPHelper.pas', + AuthenticatedCiphersCommonTestData in 'Tests\AuthenticatedCiphersCommonTestData.pas'; -function IsTestInsightRunning: Boolean; -{$IFDEF TESTINSIGHT} +{ keep comment here to protect the following conditional from being removed by the IDE when adding a unit } +{$IFNDEF TESTINSIGHT} var - client: ITestInsightClient; + runner: ITestRunner; + results: IRunResults; + logger: ITestLogger; + nunitLogger: ITestLogger; +{$ENDIF} begin - client := TTestInsightRestClient.Create; - client.StartedTesting(0); - Result := not client.HasError; -end; +{$IFDEF TESTINSIGHT} + TestInsight.DUnitX.RunRegisteredTests; {$ELSE} -begin - result := false; -end; -{$ENDIF} + try + ReportMemoryLeaksOnShutdown := True; + TDUnitX.CheckCommandLine; -var - runner : ITestRunner; - results : IRunResults; - logger : ITestLogger; - nunitLogger : ITestLogger; -begin + runner := TDUnitX.CreateRunner; + // Fixtures register in unit initialization — avoid double discovery: + runner.UseRTTI := False; + runner.FailsOnNoAsserts := False; // True after parity period -//{$IFDEF GUI} -// // DUnitX.Loggers.GUIX.GUIXTestRunner.Run.Execute; -//// DUnitX.Loggers.GUIX.GUIXTestRunner.Run; -// DUnitX.Loggers.GUI.VCL.Run; -// exit; -//{$ENDIF} + {$IFDEF CI} + TDUnitX.Options.ConsoleMode := TDunitXConsoleMode.Off; + {$ENDIF} - try - if IsTestInsightRunning then - {$IFDEF TESTINSIGHT} - TestInsight.DUnitX.RunRegisteredTests - {$ENDIF} - else + if TDUnitX.Options.ConsoleMode <> TDunitXConsoleMode.Off then begin - //Check command line options, will exit if invalid - TDUnitX.CheckCommandLine; - //Create the test runner - runner := TDUnitX.CreateRunner; - //Tell the runner to use RTTI to find Fixtures - runner.UseRTTI := True; - //tell the runner how we will log things - //Log to the console window -// {$IFDEF GUI} -// logger := TGUIXTestRunner.Create(nil); -// {$ELSE} - logger := TDUnitXConsoleLogger.Create(true); -// {$ENDIF} + logger := TDUnitXConsoleLogger.Create( + TDUnitX.Options.ConsoleMode = TDunitXConsoleMode.Quiet); runner.AddLogger(logger); - //Generate an NUnit compatible XML File - nunitLogger := TDUnitXXMLNUnitFileLogger.Create(TDUnitX.Options.XMLOutputFile); - runner.AddLogger(nunitLogger); - runner.FailsOnNoAsserts := False; //When true, Assertions must be made during tests; + end; + + // NUnit XML for CI and local regression comparison + nunitLogger := TDUnitXXMLNUnitFileLogger.Create(TDUnitX.Options.XMLOutputFile); + runner.AddLogger(nunitLogger); - //Run tests - results := runner.Execute; - if not results.AllPassed then - System.ExitCode := EXIT_ERRORS; + results := runner.Execute; + if not results.AllPassed then + System.ExitCode := EXIT_ERRORS; - {$IFNDEF CI} - //We don't want this happening when running under CI. - if TDUnitX.Options.ExitBehavior = TDUnitXExitBehavior.Pause then - begin - System.Write('Done.. press key to quit.'); - System.Readln; - end; - {$ENDIF} + {$IFNDEF CI} + if TDUnitX.Options.ExitBehavior = TDUnitXExitBehavior.Pause then + begin + System.Write('Done.. press key to quit.'); + System.Readln; end; + {$ENDIF} except on E: Exception do + begin System.Writeln(E.ClassName, ': ', E.Message); + System.ExitCode := 1; + end; end; +{$ENDIF} end. diff --git a/Unit Tests/DECDUnitXTestSuite.dproj b/Unit Tests/DECDUnitXTestSuite.dproj index a0e4579..32b4662 100644 --- a/Unit Tests/DECDUnitXTestSuite.dproj +++ b/Unit Tests/DECDUnitXTestSuite.dproj @@ -197,7 +197,10 @@ + + + Base From 00d9e0e02a7d8cf5902162e6b612cc6121c0b0f5 Mon Sep 17 00:00:00 2001 From: Olaf Monien Date: Wed, 22 Jul 2026 13:44:17 +0200 Subject: [PATCH 07/15] Fix dual-stack test registration gaps for DUnitX parity. --- Unit Tests/Tests/TestDECFormat.pas | 2 ++ Unit Tests/Tests/TestDECRandom.pas | 1 + Unit Tests/Tests/TestDECUtil.pas | 4 ++++ Unit Tests/Tests/TestDECZIPHelper.pas | 1 + 4 files changed, 8 insertions(+) diff --git a/Unit Tests/Tests/TestDECFormat.pas b/Unit Tests/Tests/TestDECFormat.pas index 0f72b74..64a9cb8 100644 --- a/Unit Tests/Tests/TestDECFormat.pas +++ b/Unit Tests/Tests/TestDECFormat.pas @@ -3039,6 +3039,7 @@ initialization TDUnitX.RegisterTestFixture(TestTFormat_BigEndian16); TDUnitX.RegisterTestFixture(TestTFormat_BigEndian32); TDUnitX.RegisterTestFixture(TestTFormat_BigEndian64); + TDUnitX.RegisterTestFixture(TestTFormat_UTF8); TDUnitX.RegisterTestFixture(TestTFormat_UTF16); {$ELSE} RegisterTests('DECFormat', [//TestTFormat, @@ -3054,6 +3055,7 @@ initialization TestTFormat_ESCAPE.Suite, TestTFormat_BigEndian16.Suite, TestTFormat_BigEndian32.Suite, + TestTFormat_BigEndian64.Suite, TestTFormat_UTF8.Suite, TestTFormat_UTF16.Suite]); {$ENDIF} diff --git a/Unit Tests/Tests/TestDECRandom.pas b/Unit Tests/Tests/TestDECRandom.pas index 146ca3f..fc57e72 100644 --- a/Unit Tests/Tests/TestDECRandom.pas +++ b/Unit Tests/Tests/TestDECRandom.pas @@ -41,6 +41,7 @@ interface /// default ones afterwards, which should be done to enable repeated test /// runs in the same session. /// + {$IFDEF DUnitX} [TestFixture] {$ENDIF} TTestRandom = class(TTestCase) private RandomNumbers: TBytes; diff --git a/Unit Tests/Tests/TestDECUtil.pas b/Unit Tests/Tests/TestDECUtil.pas index 295682d..2fe4d98 100644 --- a/Unit Tests/Tests/TestDECUtil.pas +++ b/Unit Tests/Tests/TestDECUtil.pas @@ -33,6 +33,7 @@ interface DECUtil; type + {$IFDEF DUnitX} [TestFixture] {$ENDIF} TTestBitTwiddling = class(TTestCase) published procedure ReverseBits32; @@ -45,6 +46,7 @@ TTestBitTwiddling = class(TTestCase) procedure XORBuffers; end; + {$IFDEF DUnitX} [TestFixture] {$ENDIF} TTestBufferProtection = class(TTestCase) published procedure ProtectBuffer; @@ -70,6 +72,7 @@ TTestBufferProtection = class(TTestCase) procedure BytesToString_StringToBytes; end; + {$IFDEF DUnitX} [TestFixture] {$ENDIF} TTestIsEqual = class(TTestCase) published procedure IsEqualsNormal; @@ -77,6 +80,7 @@ TTestIsEqual = class(TTestCase) procedure IsEqualsZeroLength; end; + {$IFDEF DUnitX} [TestFixture] {$ENDIF} TTestMist = class(TTestCase) published procedure TestShannonEntropy; diff --git a/Unit Tests/Tests/TestDECZIPHelper.pas b/Unit Tests/Tests/TestDECZIPHelper.pas index 031c538..1b77290 100644 --- a/Unit Tests/Tests/TestDECZIPHelper.pas +++ b/Unit Tests/Tests/TestDECZIPHelper.pas @@ -36,6 +36,7 @@ interface /// /// Test cases for the various helper functions /// + {$IFDEF DUnitX} [TestFixture] {$ENDIF} TestZIPHelpers = class(TTestCase) strict private /// From dfc8034b7bf62bdff505c9caf2d94f57af4851a7 Mon Sep 17 00:00:00 2001 From: Olaf Monien Date: Wed, 22 Jul 2026 13:54:23 +0200 Subject: [PATCH 08/15] Document Task 5 DUnit/DUnitX pass-fail parity comparison. --- Docs/plans/dunitx-parity-log.md | 99 +++++++++++++++++++++++++++++---- 1 file changed, 89 insertions(+), 10 deletions(-) diff --git a/Docs/plans/dunitx-parity-log.md b/Docs/plans/dunitx-parity-log.md index e640d69..697851b 100644 --- a/Docs/plans/dunitx-parity-log.md +++ b/Docs/plans/dunitx-parity-log.md @@ -31,15 +31,17 @@ Logs (local, not committed): `.superpowers/sdd/baseline-logs/` | `DECDUnitXTestSuite` | DUnitX console | Debug (defines `DUnitX;DEBUG`) | `Compiled\BIN_IDE37.0_Win32_Debug\` | | `Tests\TestDefines.inc` | `{.$DEFINE DUnitX}` **off** by default | DUnitX project supplies define via dproj | — | -### Units in DUnit only (coverage gap) +### Units coverage (updated Task 3+) -| Unit | Approx. tests (from DUnit log) | +All test units are referenced by both projects, including: + +| Unit | Role | |---|---| -| `TestDECCipherModesCCM.pas` | 16 (`TestTDECCCM`) | -| `TestDECZIPHelper.pas` | 3 (`TestZIPHelpers`) | +| `TestDECCipherModesCCM.pas` | CCM fixture (`TestTDECCCM`) — both runners | +| `TestDECZIPHelper.pas` | ZIP helpers (`TestZIPHelpers`) — both runners | | `AuthenticatedCiphersCommonTestData.pas` | support only (no fixture) | -All other test units are referenced by both projects. +Task 1 gap (CCM + ZIP missing from DUnitX) is **closed**. --- @@ -107,12 +109,89 @@ All **16** CCM + **3** ZIP tests **passed** on DUnit in this baseline. They are 3. DUnitX first compile without extra path: `F2613 Unit 'DUnitX.Loggers.Console' not found`. Fixed by adding Bpl Win32 Debug DCU dir. 4. Overriding only `DCC_UnitSearchPath` on the MSBuild command line **replaces** the dproj’s DEC DCU path — include **both** DEC DCU and DUnitX DCU paths. 5. `TestDefines.inc` still has `{.$DEFINE DUnitX}` commented; project define `DUnitX` in dproj is what enables the dual-stack branch for DUnitX builds today (Task 2 will harden this). +6. **Shared DCU dir:** both test projects write fixture DCUs to `Compiled\DCU_IDE37.0_Win32__Demos`. When switching DUnitX → DUnit, delete `Test*.dcu` (and `AuthenticatedCiphers*.dcu`) from that folder first, or the DUnit build fails looking for `DUnitX.Attributes`. +7. **Run CWD:** always the EXE output directory so `..\..\Unit Tests\Data\…` resolves correctly. + +--- + +## Task 5 parity run (2026-07-22) — pass/fail comparison + +Rebuilt and re-ran both suites on `Cleanup_OM-DUnitX-migration` after Task 4 dual-registration fixes. +`runner.UseRTTI := False`; fixtures registered only via `TDUnitX.RegisterTestFixture` in unit `initialization`. + +Local logs: `.superpowers/sdd/baseline-logs/task5-dunit-run.txt`, `task5-dunitx-run.txt`, `task5-dunitx-nunit.xml`. + +### Counts + +| Suite | Config | Found/Run | Passed | Failed | Errors | Exit | +|---|---|---:|---:|---:|---:|---:| +| **DUnit** Task 5 | Console Win32 | **1436** | 1424 | **12** | 0 | 0 | +| **DUnitX** Task 5 | Debug Win32 | **1476** | **1461** | **15** | 0 | 1 | +| DUnit Task 1 baseline | Console Win32 | 1425 | — | 12 | 0 | — | +| DUnitX Task 1 baseline | Debug Win32 | 1457 | 1442 | 15 | 0 | 1 | + +NUnit XML: `total=1476 failures=15 errors=0` (matches console summary). + +Coverage: CCM + ZIP present on **both** runners (gap from Task 1 closed in Task 3). No unit-level “tests only on one runner” blocker. + +### Count gap (1476 − 1436 = **40**) — explained + +Not fixture double-registration (UseRTTI is False; NUnit shows **122** unique fixtures). + +DUnitX discovers **redeclared published methods** twice when a descendant re-publishes a base `published` method (base RTTI entry + override entry). Classic DUnit `.Suite` registers each method name once. + +| Pattern | Extra cases | Example | +|---|---:|---| +| Hash leaf redeclares `TestIsPasswordHash` (base `THash_TestBase` already has it) | 38 | `TestTHash_MD2` … `TestTHash_Shake256` | +| `TestTISO10126Padding` overrides `TestAddPadding_*` | 2 | `TestAddPadding_RawByteString`, `TestAddPadding_Bytes` | +| **Total extras** | **40** | 1476 = 1436 + 40 | + +Confirmed in NUnit XML: same fixture path lists the same `test-case` name twice (often one Success + one Failure when base vs override assertions differ). + +### Failure classification + +#### Shared failures (both runners) — **12** — product/test baseline, not migration + +| # | Fixture / test | Notes | +|---|---|---| +| 1–7 | `TestTHash_Keccak_224` — `TestCalcStream`, `TestCalcStreamRawByteString`, `TestCalcStreamNoDone`, `TestCalcStreamNoDoneMulti`, `TestCalcBuffer`, `TestCalcBytes`, `TestCalcRawByteString` | Index 1 digest mismatch | +| 8 | `TestTHash_Keccak_224.TestCalcUnicodeString` | Index 1 empty expected vs non-empty | +| 9 | `TestTHash_Keccak_256.TestCalcUnicodeString` | Index 4 (`Input: <02>`) | +| 10 | `TestTHash_Keccak_384.TestCalcUnicodeString` | Index 4 | +| 11 | `TestTHash_Keccak_512.TestCalcUnicodeString` | Index 4 | +| 12 | `TestTDECGCM.TestEncodeStreamChunked` | Auth tag mismatch Set 105 | + +#### DUnitX-only failures — **3** — method double-discovery (not product crypto) + +| # | Fixture / test | DUnit | Cause | +|---|---|---|---| +| 13 | `TestTISO10126Padding.TestAddPadding_RawByteString` | Pass | DUnitX also invokes **base** `TestTPaddingBase` implementation (exact byte compare); random pad bytes fail. Override with `RemoveRandomPadding` still **passes**. | +| 14 | `TestTISO10126Padding.TestAddPadding_Bytes` | Pass | Same as above | +| 15 | `TestTHash_BCrypt.TestIsPasswordHash` | Pass | DUnitX also invokes **base** `THash_TestBase.TestIsPasswordHash` (`CheckEquals(false, …)`). Override expecting `True` still **passes**. | + +These three are **migration / DUnitX discovery issues**, not random flakiness and not missing registration. They are **blockers for strict pass-set parity** until the published override pattern is adjusted (e.g. stop re-publishing inherited tests; use a single published method per name; or attribute-only `[Test]` on leaves). + +#### DUnit-only failures — **0** + +No failures that appear only on DUnit. + +### Runner / registration status + +| Check | Result | +|---|---| +| `runner.UseRTTI` | **False** (explicit fixtures only) — no change needed in Task 5 | +| RTTI + RegisterTestFixture double fixture set | **Not observed** | +| Redeclared published method double execution | **Yes — 40 cases** (see above) | +| Units missing from one project | **None** (CCM/ZIP dual-listed) | + +No runner source tweak was required in Task 5. --- -## Parity rule (for later tasks) +## Parity rule (updated Task 5) -- Same fixtures must pass on both runners. -- DUnitX-only failures (ISO10126×2, BCrypt IsPasswordHash) are **migration-priority** until proven environmental. -- Shared 12 failures are baseline noise; do not block adding CCM/ZIP to DUnitX, but re-check they remain the same set after registration fixes. -- After Task 3 (add missing units), expect DUnitX total ≥ DUnit total and CCM/ZIP green. +- Same fixtures must be registered on both runners (done Tasks 3–4). +- **Shared** failures = product/test debt; do not treat as DUnitX migration defects. +- **DUnitX-only** failures today are the ISO10126×2 + BCrypt `TestIsPasswordHash` trio, explained by redeclared published methods. Fix that discovery pattern before calling pass/fail parity complete. +- Count: expect DUnitX ≥ DUnit; residual +40 is the redeclared-method effect, not missing tests. +- Keep cleaning shared `__Demos` test DCUs when alternating runners. From f8206b5e874b64621cc7ac6e71c8d0746e340015 Mon Sep 17 00:00:00 2001 From: Olaf Monien Date: Wed, 22 Jul 2026 14:07:43 +0200 Subject: [PATCH 09/15] Fix DUnitX-only failures from redeclared published test methods. Use non-published virtual hooks (NormalizeAddPaddingResult, ExpectedIsPasswordHash) so ISO10126 and BCrypt keep correct assertions without DUnitX double-discovery. --- Unit Tests/Tests/TestDECCipherPaddings.pas | 76 +++++++++------------- Unit Tests/Tests/TestDECHash.pas | 23 +++++-- 2 files changed, 48 insertions(+), 51 deletions(-) diff --git a/Unit Tests/Tests/TestDECCipherPaddings.pas b/Unit Tests/Tests/TestDECCipherPaddings.pas index fa6618a..b5f80bf 100644 --- a/Unit Tests/Tests/TestDECCipherPaddings.pas +++ b/Unit Tests/Tests/TestDECCipherPaddings.pas @@ -52,11 +52,20 @@ TestTPaddingBase = class(TTestCase) FValidTestData: TArray; FNegativeTestData: TArray; FValidRemoveTestData: TArray; + /// + /// Normalize add-padding output for equality checks. Default is identity. + /// ISO 10126 overrides to strip random pad bytes marked as '?' in the pattern. + /// + /// + /// Keep comparison logic here (not as published test overrides) so DUnitX does + /// not double-discover base + leaf published methods of the same name. + /// + function NormalizeAddPaddingResult(const AValue, APattern: RawByteString): RawByteString; virtual; published - procedure TestAddPadding_RawByteString; virtual; + procedure TestAddPadding_RawByteString; procedure TestRemovePadding_RawByteString; virtual; procedure TestRemovePadding_RawByteStringExceptions; virtual; - procedure TestAddPadding_Bytes; virtual; + procedure TestAddPadding_Bytes; procedure TestRemovePadding_Bytes; virtual; procedure TestRemovePadding_BytesExceptions; virtual; procedure TestHasValidPadding_Bytes; virtual; @@ -94,13 +103,11 @@ TestTANSI_X9_23Padding = class(TestTPaddingBase) /// {$IFDEF DUnitX} [TestFixture] {$ENDIF} TestTISO10126Padding = class(TestTPaddingBase) - protected + strict protected function RemoveRandomPadding(const Res, Pattern: RawByteString): RawByteString; + function NormalizeAddPaddingResult(const AValue, APattern: RawByteString): RawByteString; override; public procedure SetUp; override; - published - procedure TestAddPadding_RawByteString; override; - procedure TestAddPadding_Bytes; override; end; /// @@ -116,6 +123,12 @@ implementation { TestTPaddingBase } +function TestTPaddingBase.NormalizeAddPaddingResult(const AValue, APattern: RawByteString): RawByteString; +begin + // Default: exact match (APattern is used by ISO 10126 to mask random pad bytes) + Result := AValue; +end; + procedure TestTPaddingBase.TestAddPadding_RawByteString; var I : integer; @@ -126,8 +139,9 @@ procedure TestTPaddingBase.TestAddPadding_RawByteString; Res := FPaddingClass.AddPadding(FValidTestData[I].InputData, FValidTestData[I].BlockSize); - CheckEquals(Res, - FValidTestData[I].OutputData, + CheckEquals(NormalizeAddPaddingResult(Res, FValidTestData[I].OutputData), + NormalizeAddPaddingResult(FValidTestData[I].OutputData, + FValidTestData[I].OutputData), 'Valid test data set ' + I.ToString + ' failed'); end; @@ -144,8 +158,10 @@ procedure TestTPaddingBase.TestAddPadding_Bytes; Res := FPaddingClass.AddPadding(DECUtil.RawStringToBytes(FValidTestData[I].InputData), FValidTestData[I].BlockSize); - CheckEquals(DECUtil.BytesToRawString(Res), - FValidTestData[I].OutputData, + CheckEquals(NormalizeAddPaddingResult(DECUtil.BytesToRawString(Res), + FValidTestData[I].OutputData), + NormalizeAddPaddingResult(FValidTestData[I].OutputData, + FValidTestData[I].OutputData), 'Valid test data set ' + I.ToString + ' failed'); end; @@ -739,44 +755,10 @@ function TestTISO10126Padding.RemoveRandomPadding(const Res, Pattern: RawByteStr Result := Result + Res[c]; end; -procedure TestTISO10126Padding.TestAddPadding_RawByteString; -var - I : integer; - Res : RawByteString; +function TestTISO10126Padding.NormalizeAddPaddingResult(const AValue, APattern: RawByteString): RawByteString; begin - for I := Low(FValidTestData) to High(FValidTestData) do - begin - Res := FPaddingClass.AddPadding(FValidTestData[I].InputData, - FValidTestData[I].BlockSize); - - Res := RemoveRandomPadding(Res, FValidTestData[I].OutputData); - CheckEquals(Res, - RemoveRandomPadding(FValidTestData[I].OutputData, - FValidTestData[I].OutputData), - 'Valid test data set ' + I.ToString + ' failed'); - end; - - Status(length(FValidTestData).ToString + ' test pattern passed'); -end; - -procedure TestTISO10126Padding.TestAddPadding_Bytes; -var - I : integer; - Res : TBytes; -begin - for I := Low(FValidTestData) to High(FValidTestData) do - begin - Res := FPaddingClass.AddPadding(DECUtil.RawStringToBytes(FValidTestData[I].InputData), - FValidTestData[I].BlockSize); - - CheckEquals(RemoveRandomPadding(DECUtil.BytesToRawString(Res), - FValidTestData[I].OutputData), - RemoveRandomPadding(FValidTestData[I].OutputData, - FValidTestData[I].OutputData), - 'Valid test data set ' + I.ToString + ' failed'); - end; - - Status(length(FValidTestData).ToString + ' test pattern passed'); + // ISO 10126 fills pad bytes with random data; expected patterns use '?' as wildcards + Result := RemoveRandomPadding(AValue, APattern); end; { TestTISO7816Padding } diff --git a/Unit Tests/Tests/TestDECHash.pas b/Unit Tests/Tests/TestDECHash.pas index 41792cc..2e27ff4 100644 --- a/Unit Tests/Tests/TestDECHash.pas +++ b/Unit Tests/Tests/TestDECHash.pas @@ -177,6 +177,16 @@ THash_TestBase = class(TTestCase) /// be set in this class. /// procedure ConfigHashClass(HashClass: TDECHash; IdxTestData:Integer); virtual; + /// + /// Expected value of FHash.IsPasswordHash for this fixture. + /// Password-hash leaves (e.g. BCrypt) override to True. + /// + /// + /// Using a non-published virtual hook avoids redeclaring published + /// TestIsPasswordHash on leaves, which DUnitX would run twice + /// (base RTTI entry + leaf re-publish) and fail for password hashes. + /// + function ExpectedIsPasswordHash: Boolean; virtual; public procedure SetUp; override; procedure TearDown; override; @@ -724,6 +734,7 @@ TBCryptBSDTestData = record function SplitTestVector(const Vector: string):TBCryptBSDTestData; protected procedure ConfigHashClass(aHashClass: TDECHash; aIdxTestData:Integer); override; + function ExpectedIsPasswordHash: Boolean; override; public procedure SetUp; override; procedure DoTestCostFactorTooShortException; @@ -735,7 +746,6 @@ TBCryptBSDTestData = record published procedure TestDigestSize; procedure TestBlockSize; - procedure TestIsPasswordHash; procedure TestClassByName; procedure TestIdentity; procedure TestMaximumSaltLength; @@ -5576,9 +5586,14 @@ procedure THash_TestBase.TestGetPaddingByte; CheckEquals(0, FHash.PaddingByte, 'Default padding byte is wrong'); end; +function THash_TestBase.ExpectedIsPasswordHash: Boolean; +begin + Result := False; +end; + procedure THash_TestBase.TestIsPasswordHash; begin - CheckEquals(false, FHash.IsPasswordHash); + CheckEquals(ExpectedIsPasswordHash, FHash.IsPasswordHash); end; procedure THash_TestBase.TestIsPasswordHashBase; @@ -6453,9 +6468,9 @@ procedure TestTHash_BCrypt.TestIdentity; CheckEquals($9CA55338, FHash.Identity); end; -procedure TestTHash_BCrypt.TestIsPasswordHash; +function TestTHash_BCrypt.ExpectedIsPasswordHash: Boolean; begin - CheckEquals(true, FHash.IsPasswordHash); + Result := True; end; procedure TestTHash_BCrypt.TestIsValidPasswordFalseString; From 6009c800a66b81b67039773d28742a7fc6b4bf5b Mon Sep 17 00:00:00 2001 From: Olaf Monien Date: Wed, 22 Jul 2026 14:07:43 +0200 Subject: [PATCH 10/15] Minor test clarity fixes found during DUnitX parity review. --- Unit Tests/Tests/TestDECCipherModesCCM.pas | 41 +++++++--------------- 1 file changed, 12 insertions(+), 29 deletions(-) diff --git a/Unit Tests/Tests/TestDECCipherModesCCM.pas b/Unit Tests/Tests/TestDECCipherModesCCM.pas index 26fb507..7a8d728 100644 --- a/Unit Tests/Tests/TestDECCipherModesCCM.pas +++ b/Unit Tests/Tests/TestDECCipherModesCCM.pas @@ -77,8 +77,7 @@ TestTDECCCM = class(TTestCase) procedure TestInitFailureIVTooLong; procedure TestInitFailureIVTooShort; procedure TestEncodeStream; -// procedure TestEncodeLargeStream; -// procedure TestEncodeStreamChunked; + // Deferred: multi-call CCM streams (AEAD roadmap) procedure TestGetDataToAuthenticate; procedure TestSetDataToAuthenticate; procedure TestSetAuthenticationBitLengths; @@ -197,7 +196,7 @@ procedure TestTDECCCM.TestDecode; string(TestData.CT) + ' Act. PT: ' + StringOf(TFormat_HexL.Encode(DecryptData))); - // Additional Authentication Data prfen + // Additional Authentication Data pr�fen CheckEquals(string(TestData.TagResult), StringOf(TFormat_HexL.Encode(FCipherAES.CalculatedAuthenticationResult)), 'Authentication tag wrong for key ' + @@ -254,13 +253,16 @@ procedure TestTDECCCM.TestInitIV; begin Key := [1, 2, 3, 4, 5, 6, 7, 8]; + // Legal CCM nonce lengths are 7..13; Init raising would fail the test for i := 7 to 13 do begin SetLength(IV, i); FillChar(IV[0], length(IV), $FF); FCipherAES.Init(Key, IV, $FF, pmNone); - Check(true); + // Real assertion: Init must leave the cipher in CCM mode (no exception = length accepted) + Check(FCipherAES.Mode = TCipherMode.cmCCM, + 'Mode must remain CCM after Init with IV length ' + i.ToString); end; end; @@ -315,7 +317,7 @@ procedure TestTDECCCM.TestEncode; string(TestData.CT) + ' Act.: ' + EncrDataStr); - // Additional Authentication Data prfen + // Additional Authentication Data pr�fen CheckEquals(string(TestData.TagResult), StringOf(TFormat_HexL.Encode(FCipherAES.CalculatedAuthenticationResult)), 'Authentication tag wrong for Key ' + @@ -419,7 +421,7 @@ procedure TestTDECCCM.TestDecodeStream; string(TestData.CT) + ' Act.: ' + StringOf(TFormat_HexL.Encode(DecryptData))); - // Additional Authentication Data prfen + // Additional Authentication Data pr�fen CheckEquals(string(TestData.TagResult), StringOf(TFormat_HexL.Encode(FCipherAES.CalculatedAuthenticationResult)), 'Authentication tag wrong for key ' + @@ -444,13 +446,6 @@ procedure TestTDECCCM.DoTestAuthenticationBitLengthWrong; FCipherAES.AuthenticationResultBitLength := FTestBitLength; end; -//procedure TestTDECCCM.TestEncodeStreamChunked; -//begin -// // Use cipher block size as max chunk size -// DoTestEncodeStream_LoadAndTestCAVSData( -// Max(FCipherAES.Context.BlockSize, FCipherAES.Context.BufferSize)); -//end; -// procedure TestTDECCCM.DoTestEncodeStream_LoadAndTestCAVSData(const aMaxChunkSize: Int64); var @@ -463,21 +458,9 @@ procedure TestTDECCCM.DoTestEncodeStream_LoadAndTestCAVSData(const DoTestEncodeStream_TestSingleSet(curSetIndex, 0, aMaxChunkSize); end; end; -// -//procedure TestTDECCCM.TestEncodeLargeStream; -//begin -// // There is only one record in test data set atm, so need to allow -// // incomplete load -// FTestDataLoader.LoadFile('..\..\Unit Tests\Data\gcmEncryptExtIV256_large.rsp', -// FTestDataList, True); -// Status('Encode large stream using chunking'); -// CheckEquals(8192, StreamBufferSize, 'Might need to update data set to have enough data!'); -//{ TODO : Auskommentierten Code entfernen } -//// Assert(StreamBufferSize = 8192, 'Might need to update data set to have enough data!'); -// DoTestEncodeStream_TestSingleSet(0, 0, StreamBufferSize); -// Status('Encode large stream without chunking'); -// DoTestEncodeStream_TestSingleSet(0, 0, -1); -//end; + +// Deferred: multi-call CCM streams (AEAD roadmap) — former TestEncodeStreamChunked / +// TestEncodeLargeStream bodies intentionally not re-enabled here. procedure TestTDECCCM.DoTestEncodeStream_TestSingleSet(const aSetIndex, aDataIndex: Integer; const aMaxChunkSize: Int64 = -1); @@ -540,7 +523,7 @@ procedure TestTDECCCM.DoTestEncodeStream_TestSingleSet(const aSetIndex, string(TestData.AAD) + ' Act.: ' + StringOf(TFormat_HexL.Encode(FCipherAES.DataToAuthenticate))); - // Additional Authentication Data prfen + // Additional Authentication Data pr�fen CheckEquals(string(TestData.TagResult), StringOf(TFormat_HexL.Encode(FCipherAES.CalculatedAuthenticationResult)), 'Authentication tag wrong for Set ' + aSetIndex.ToString + ' and Data ' + aDataIndex.ToString + From 83d20febba5e6210496957741ea312502a19e3aa Mon Sep 17 00:00:00 2001 From: Olaf Monien Date: Wed, 22 Jul 2026 14:08:07 +0200 Subject: [PATCH 11/15] Document Task 6 fail-set parity after DUnitX-only failure fixes. --- Docs/plans/dunitx-parity-log.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/Docs/plans/dunitx-parity-log.md b/Docs/plans/dunitx-parity-log.md index 697851b..0012bed 100644 --- a/Docs/plans/dunitx-parity-log.md +++ b/Docs/plans/dunitx-parity-log.md @@ -195,3 +195,16 @@ No runner source tweak was required in Task 5. - **DUnitX-only** failures today are the ISO10126×2 + BCrypt `TestIsPasswordHash` trio, explained by redeclared published methods. Fix that discovery pattern before calling pass/fail parity complete. - Count: expect DUnitX ≥ DUnit; residual +40 is the redeclared-method effect, not missing tests. - Keep cleaning shared `__Demos` test DCUs when alternating runners. + +--- + +## Task 6 run (2026-07-22) — DUnitX-only failures fixed + +After non-published virtual hooks for ISO10126 padding compare and BCrypt `ExpectedIsPasswordHash` (see Task 6 report): + +| Suite | Found/Run | Failed | DUnitX-only fails | +|---|---:|---:|---:| +| DUnit Console | 1436 | **12** | — | +| DUnitX Debug | **1473** | **12** | **0** | + +Fail-sets match (12 shared Keccak/GCM). Count gap 37 = remaining redundant hash-leaf `TestIsPasswordHash` redeclares (no longer fail). From 39aaddac14c0c7d9b07b1eda5f760658f862b05c Mon Sep 17 00:00:00 2001 From: Olaf Monien Date: Wed, 22 Jul 2026 14:10:41 +0200 Subject: [PATCH 12/15] Document DUnitX migration plan and dual-suite comparison policy. --- Docs/Cleanup-Roadmap.md | 46 +++++++-------- Docs/plans/2026-07-22-dunitx-migration.md | 72 ++++++++++++----------- readme.md | 5 ++ 3 files changed, 64 insertions(+), 59 deletions(-) diff --git a/Docs/Cleanup-Roadmap.md b/Docs/Cleanup-Roadmap.md index 8760764..022e423 100644 --- a/Docs/Cleanup-Roadmap.md +++ b/Docs/Cleanup-Roadmap.md @@ -42,39 +42,32 @@ Rationale: architecture changes redefine how authenticated modes work for the wh **Before** tackling the AEAD architecture split (and before large cipher-mode refactors), DUnitX must become a **complete, parity-proven** runner for the whole suite. -### 2.1 Binding constraint: keep DUnit temporarily - -The classic **DUnit suite stays** for the time being so we can **compare results** (same tests, both runners) and prove the migration did not drop or alter behaviour. +**Status (branch `Cleanup_OM-DUnitX-migration`, 2026-07-22):** migration work is **done for this phase**. DUnitX is the **target authoritative runner**. The classic **DUnit suite remains** until parity is trusted longer-term and a **later PR** removes it (not part of this migration). | Phase | DUnit (`DECDUnitTestSuite`) | DUnitX (`DECDUnitXTestSuite`) | |---|---|---| -| **Now / migration branch** | Keep; baseline for comparison | Bring to full parity; make authoritative for new work | +| **Now (migration branch complete)** | Kept; comparison baseline | Complete unit list; parity-proven fail-set; preferred for new work | | **Later (separate PR)** | Remove after explicit decision | Only runner; drop dual-stack / compatibility | Detailed plan: **[`Docs/plans/2026-07-22-dunitx-migration.md`](plans/2026-07-22-dunitx-migration.md)** -Branch: `Cleanup_OM-DUnitX-migration` +Parity log: **[`Docs/plans/dunitx-parity-log.md`](plans/dunitx-parity-log.md)** -### 2.2 Current state (as of `development` / start of cleanup) +### 2.1 Binding constraint: keep DUnit temporarily -| Item | Status | -|---|---| -| Classic DUnit project | `Unit Tests/DECDUnitTestSuite.dpr` (+ `.dproj`) — uses `TestFramework` | -| DUnitX project | `Unit Tests/DECDUnitXTestSuite.dpr` (+ `.dproj`) — **incomplete** vs DUnit (missing CCM, ZIP helper, AEAD common test data) | -| Shared switch | `Unit Tests/Tests/TestDefines.inc` — `{.$DEFINE DUnitX}` (**off by default**); DUnitX dproj supplies define | -| Dual-stack tests | Most units use `{$IFDEF DUnitX}` + `DUnitX.DUnitCompatibility` vs classic `TestFramework` | -| Sense check | Existing tests are largely **meaningful** (vectors, regressions); weak spots documented in the plan, not migration blockers | +The classic **DUnit suite stays** for the time being so we can **compare results** (same tests, both runners) and prove the migration did not drop or alter behaviour. -### 2.3 Target state after migration branch (DUnit still present) +### 2.2 Achieved state after migration branch -| Item | Target | +| Item | Status | |---|---| -| DUnitX project | **Full** unit list matching DUnit | -| Parity | No DUnitX-only failures; fixture sets aligned | -| DUnit project | Still builds and runs (comparison) | -| Assert style | May still use `Check*` via `DUnitCompatibility` | -| Dual `IFDEF` | Still allowed | +| Classic DUnit project | `Unit Tests/DECDUnitTestSuite.*` — still builds/runs (comparison) | +| DUnitX project | **Full** unit list matching DUnit, including **CCM**, **ZIP**, AEAD common test data; DPR hardened (explicit registration, `UseRTTI := False`, exit codes, console mode) | +| Shared switch | `Unit Tests/Tests/TestDefines.inc` — `{.$DEFINE DUnitX}` **off by default**; DUnitX dproj supplies the define | +| Dual-stack tests | Fixtures register under both `{$IFDEF DUnitX}` and classic DUnit | +| Assert style | Still `Check*` via `DUnitX.DUnitCompatibility` (native `Assert.*` deferred) | +| **Fail-set parity** | Both suites: **12 shared failures** (Keccak digest vectors + GCM chunked stream tag — product/test debt, not migration). **0 DUnitX-only failures** | -### 2.4 Later target (after comparison period — not this branch) +### 2.3 Later target (after comparison period — not this branch) | Item | Target | |---|---| @@ -82,6 +75,7 @@ Branch: `Cleanup_OM-DUnitX-migration` | DUnit project | Removed | | Native asserts | `Assert.*`; drop `DUnitCompatibility` | | Docs / CI | DUnitX only | +| Shared 12 failures | Fix as product/test work (Keccak / GCM), independent of runner choice | Do **not** mix this migration with AEAD architecture or ChaCha feature work in the same PR. @@ -95,8 +89,9 @@ Cleanup_OM (and follow-up PRs → development) ├─ 0. Repo hygiene (started) │ e.g. DelphiStandards .gitignore │ -├─ 1. DUnit → DUnitX migration (complete) ← next major technical step -│ Single test runner, no dual-stack +├─ 1. DUnit → DUnitX migration ← done on Cleanup_OM-DUnitX-migration +│ DUnitX complete + fail-set parity; DUnit retained for comparison +│ (single runner / remove DUnit = later PR) │ ├─ 2. AEAD architecture split (from PR #90 concern A) │ Base + GCM stream/multi-call only; review as core API change @@ -137,7 +132,10 @@ Cleanup_OM (and follow-up PRs → development) | DUnit project | `Unit Tests/DECDUnitTestSuite.*` | | DUnitX project | `Unit Tests/DECDUnitXTestSuite.*` | | Dual-stack switch | `Unit Tests/Tests/TestDefines.inc` | +| DUnitX migration plan | `Docs/plans/2026-07-22-dunitx-migration.md` | +| DUnit / DUnitX parity log | `Docs/plans/dunitx-parity-log.md` | +| Migration branch | `Cleanup_OM-DUnitX-migration` | --- -*Document created as part of cleanup planning on `Cleanup_OM`. Update this file when a step completes or a decision changes.* +*Document created as part of cleanup planning on `Cleanup_OM`. Updated 2026-07-22 for completed DUnitX migration phase (DUnit still retained for comparison).* diff --git a/Docs/plans/2026-07-22-dunitx-migration.md b/Docs/plans/2026-07-22-dunitx-migration.md index b350283..23cbd51 100644 --- a/Docs/plans/2026-07-22-dunitx-migration.md +++ b/Docs/plans/2026-07-22-dunitx-migration.md @@ -1,6 +1,6 @@ # DUnit → DUnitX Migration Plan -> **For agentic workers:** Implement task-by-task on branch `Cleanup_OM-DUnitX-migration`. Steps use checkbox (`- [ ]`) syntax for tracking. +> **For agentic workers:** Implement task-by-task on branch `Cleanup_OM-DUnitX-migration`. Steps use checkbox (`- [x]`) syntax for tracking. **Goal:** Make DUnitX the complete, authoritative unit-test runner for DEC while temporarily keeping the classic DUnit suite for parity comparison. @@ -9,7 +9,9 @@ **Tech Stack:** Delphi DUnitX (`DUnitX.TestFramework`, console + NUnit XML logger), existing DUnit (`TestFramework`, GUI/Text runners), shared test units under `Unit Tests/Tests/`. **Branch:** `Cleanup_OM-DUnitX-migration` (from `Cleanup_OM`) -**PR target (later):** `Cleanup_OM` → eventually `development` +**PR target (later):** `Cleanup_OM` → eventually `development` + +**Status (2026-07-22):** Tasks 1–7 complete. DUnitX has full unit coverage (incl. CCM/ZIP), hardened DPR, and **fail-set parity** with DUnit (12 shared Keccak/GCM failures; 0 DUnitX-only). DUnit retained for comparison. See `Docs/plans/dunitx-parity-log.md`. ## Global Constraints @@ -267,35 +269,35 @@ Bringing the dpr to this shape is part of **Tasks 2–5** (define ownership, uni **Files:** none (measurement only), or add `Docs/plans/dunitx-parity-log.md` if useful -- [ ] **Step 1: Confirm branch** +- [x] **Step 1: Confirm branch** ```bash git branch --show-current # expect: Cleanup_OM-DUnitX-migration ``` -- [ ] **Step 2: Build DUnit suite (Win32 Debug preferred)** +- [x] **Step 2: Build DUnit suite (Win32 Debug preferred)** Use project `Unit Tests/DECDUnitTestSuite.dproj` with Delphi MSBuild / IDE / `DelphiBuildDPROJ.ps1` if available in environment. Expected: successful compile. -- [ ] **Step 3: Run DUnit suite** +- [x] **Step 3: Run DUnit suite** Prefer console: define `CONSOLE_TESTRUNNER` if needed, or run GUI and export results. Record: total tests, failures, errors, ignored. -- [ ] **Step 4: Build DUnitX suite as-is (before fixes)** +- [x] **Step 4: Build DUnitX suite as-is (before fixes)** `Unit Tests/DECDUnitXTestSuite.dproj` — ensure `DUnitX` is defined in the project (dproj already has `DUnitX;DEBUG` etc.). Also ensure `TestDefines.inc` define is **on** for DUnitX builds (today default is **off** — see Task 2). -- [ ] **Step 5: Run DUnitX suite as-is** +- [x] **Step 5: Run DUnitX suite as-is** Record baseline. Expect possible missing CCM/ZIP coverage even if green. -- [ ] **Step 6: Commit only if you added a parity log file; otherwise no commit** +- [x] **Step 6: Commit only if you added a parity log file; otherwise no commit** --- @@ -308,7 +310,7 @@ Record baseline. Expect possible missing CCM/ZIP coverage even if green. - Modify: `Unit Tests/DECDUnitXTestSuite.dpr` (comments) - Verify: `Unit Tests/DECDUnitTestSuite.dproj` has **no** `DUnitX` in `DCC_Define` -- [ ] **Step 1: Document the switch in `TestDefines.inc`** +- [x] **Step 1: Document the switch in `TestDefines.inc`** Keep default **off** so opening test units under the DUnit project still compiles as DUnit. Rely on **project-level** `DCC_Define=DUnitX` for the DUnitX project (already present in dproj). Update the comment to say: @@ -322,17 +324,17 @@ Keep default **off** so opening test units under the DUnit project still compile {.$DEFINE DUnitX} ``` -- [ ] **Step 2: Verify DUnitX dproj defines `DUnitX` for all configs used** +- [x] **Step 2: Verify DUnitX dproj defines `DUnitX` for all configs used** Debug/Release (and GUI if present) must include `DUnitX` in `DCC_Define`. -- [ ] **Step 3: Verify DUnit dproj does not define `DUnitX`** +- [x] **Step 3: Verify DUnit dproj does not define `DUnitX`** -- [ ] **Step 4: Rebuild both projects** +- [x] **Step 4: Rebuild both projects** Expected: both compile. -- [ ] **Step 5: Commit** +- [x] **Step 5: Commit** ```bash git add "Unit Tests/Tests/TestDefines.inc" "Unit Tests/DECDUnitXTestSuite.dpr" @@ -348,7 +350,7 @@ git commit -m "Clarify DUnit vs DUnitX define ownership for dual-suite builds." - Modify: `Unit Tests/DECDUnitXTestSuite.dproj` (units + ensure `DUnitX` on all configs used for this suite) - Verify: `TestDECCipherModesCCM.pas`, `TestDECZIPHelper.pas`, `AuthenticatedCiphersCommonTestData.pas` already have dual-stack `IFDEF` registration -- [ ] **Step 1: Add to `DECDUnitXTestSuite.dpr` uses clause** (order flexible; keep near related units): +- [x] **Step 1: Add to `DECDUnitXTestSuite.dpr` uses clause** (order flexible; keep near related units): ```pascal TestDECCipherModesGCM in 'Tests\TestDECCipherModesGCM.pas', @@ -360,7 +362,7 @@ git commit -m "Clarify DUnit vs DUnitX define ownership for dual-suite builds." (Adjust if some lines already exist — avoid duplicates.) -- [ ] **Step 2: Align runner bootstrap with §0.6 target shape** +- [x] **Step 2: Align runner bootstrap with §0.6 target shape** Required in this task (not deferred): @@ -374,21 +376,21 @@ Required in this task (not deferred): Keep: `CheckCommandLine`, NUnit XML logger, non-zero exit on `not AllPassed`, DUnit suite untouched. -- [ ] **Step 3: Add the same units to `DECDUnitXTestSuite.dproj`** +- [x] **Step 3: Add the same units to `DECDUnitXTestSuite.dproj`** Prefer IDE “add unit” or careful `DCCReference` entries matching existing style. Ensure `DCC_Define` includes `DUnitX` for Debug/Release (and TestInsight if that config builds the same sources). -- [ ] **Step 4: Confirm CCM/ZIP units register fixtures under `{$IFDEF DUnitX}`** +- [x] **Step 4: Confirm CCM/ZIP units register fixtures under `{$IFDEF DUnitX}`** Each should call `TDUnitX.RegisterTestFixture(...)` in `initialization`. -- [ ] **Step 5: Build + run DUnitX** +- [x] **Step 5: Build + run DUnitX** Expected: CCM and ZIP tests appear and run; process exit code non-zero on failure. -- [ ] **Step 6: Run DUnit again (sanity — should be unchanged)** +- [x] **Step 6: Run DUnit again (sanity — should be unchanged)** -- [ ] **Step 7: Commit** +- [x] **Step 7: Commit** ```bash git add "Unit Tests/DECDUnitXTestSuite.dpr" "Unit Tests/DECDUnitXTestSuite.dproj" @@ -401,7 +403,7 @@ git commit -m "Complete DUnitX suite units and align DPR with DUnitX best practi **Files:** all `Unit Tests/Tests/Test*.pas` with fixtures -- [ ] **Step 1: Inventory registration** +- [x] **Step 1: Inventory registration** For each fixture unit, ensure: @@ -410,15 +412,15 @@ For each fixture unit, ensure: | `{$IFDEF DUnitX}` | `[TestFixture]` on classes + `TDUnitX.RegisterTestFixture` (or documented RTTI-only discovery — currently suite uses both `UseRTTI := True` **and** explicit register; keep explicit register for consistency) | | `{$ELSE}` | `RegisterTest` / `RegisterTests` as today | -- [ ] **Step 2: Fix any unit that only registers one side** +- [x] **Step 2: Fix any unit that only registers one side** Known OK from audit: most units already dual-register. Re-check after CCM/ZIP inclusion. -- [ ] **Step 3: `TestDECUtil` fixtures** +- [x] **Step 3: `TestDECUtil` fixtures** Methods are not all named `Test*` (e.g. `ReverseBits32`) but are `published` — DUnit picks them up; DUnitX via `TTestCase` compatibility should too. Run suite and confirm counts roughly match. -- [ ] **Step 4: Commit only if fixes were needed** +- [x] **Step 4: Commit only if fixes were needed** ```bash git commit -m "Fix dual-stack test registration gaps for DUnitX parity." @@ -430,11 +432,11 @@ git commit -m "Fix dual-stack test registration gaps for DUnitX parity." **Files:** optional log under `Docs/plans/` -- [ ] **Step 1: Run full DUnit suite, capture output** +- [x] **Step 1: Run full DUnit suite, capture output** -- [ ] **Step 2: Run full DUnitX suite, capture console + NUnit XML** +- [x] **Step 2: Run full DUnitX suite, capture console + NUnit XML** -- [ ] **Step 3: Compare** +- [x] **Step 3: Compare** | Check | Criterion | |---|---| @@ -445,7 +447,7 @@ git commit -m "Fix dual-stack test registration gaps for DUnitX parity." Watch for **double execution** if both RTTI and `RegisterTestFixture` register the same class twice under DUnitX. If duplicates appear, set `runner.UseRTTI := False` **or** remove redundant registration — pick one strategy suite-wide (prefer **explicit RegisterTestFixture + UseRTTI False** for predictability). -- [ ] **Step 4: Fix DUnitX runner if double-registration** +- [x] **Step 4: Fix DUnitX runner if double-registration** In `DECDUnitXTestSuite.dpr`: @@ -455,7 +457,7 @@ runner.UseRTTI := False; // fixtures registered explicitly in unit initializatio Re-run parity. -- [ ] **Step 5: Commit runner tweak + any registration fixes** +- [x] **Step 5: Commit runner tweak + any registration fixes** ```bash git commit -m "Stabilize DUnitX discovery to match DUnit fixture set." @@ -467,11 +469,11 @@ git commit -m "Stabilize DUnitX discovery to match DUnit fixture set." Do **not** expand scope. Optional in same branch if parity is green: -- [ ] Replace `Check(true)` in CCM IV success path with a real assertion or remove and rely on exception — **only** if still equivalent. -- [ ] Delete clearly dead commented stream tests **or** leave with a single `// Deferred: multi-call CCM streams (AEAD roadmap)` note — do not implement streams here. -- [ ] Ensure `TestDECCipherPaddings` does not need both `Fail` and `Assert.Fail` under the same `IFDEF` in a broken way (read both branches). +- [x] Replace `Check(true)` in CCM IV success path with a real assertion or remove and rely on exception — **only** if still equivalent. +- [x] Delete clearly dead commented stream tests **or** leave with a single `// Deferred: multi-call CCM streams (AEAD roadmap)` note — do not implement streams here. +- [x] Ensure `TestDECCipherPaddings` does not need both `Fail` and `Assert.Fail` under the same `IFDEF` in a broken way (read both branches). -- [ ] **Commit only if something landed** +- [x] **Commit only if something landed** ```bash git commit -m "Minor test clarity fixes found during DUnitX parity review." @@ -486,7 +488,7 @@ git commit -m "Minor test clarity fixes found during DUnitX parity review." - This plan: mark tasks done as work proceeds - Optional: short note in `readme.md` under “Has it been tested?” -- [ ] **Step 1: Update Cleanup-Roadmap section 2** +- [x] **Step 1: Update Cleanup-Roadmap section 2** State explicitly: @@ -494,7 +496,7 @@ State explicitly: - DUnit suite remains until parity is trusted and a later PR removes it. - Link to `Docs/plans/2026-07-22-dunitx-migration.md`. -- [ ] **Step 2: Commit** +- [x] **Step 2: Commit** ```bash git add Docs/Cleanup-Roadmap.md Docs/plans/2026-07-22-dunitx-migration.md diff --git a/readme.md b/readme.md index a1cb892..1e66abc 100644 --- a/readme.md +++ b/readme.md @@ -58,6 +58,11 @@ need to work out how to implement these tests and maybe look for test data. Why don't you help out by researching useful test data for those few tests? We're talking at block chaining mode tests for the ciphers specifically. +DUnitX (`Unit Tests/DECDUnitXTestSuite`) is the preferred / authoritative runner; +the classic DUnit suite remains for comparison until a later removal. Both currently +share the same known product/test failures (Keccak and GCM chunked stream); see +`Docs/plans/dunitx-parity-log.md` and `Docs/Cleanup-Roadmap.md` §2. + In DEC 6.2 the unit tests for the hash classes were looked at and where not already used original test data vectors (as far as we could find them - for most we could) have been added to improve test coverage. V6.3 added further synthesized tests for From 98abde0a77f4f5ee11381804cb881a10a0f97970 Mon Sep 17 00:00:00 2001 From: Olaf Monien Date: Wed, 22 Jul 2026 14:13:06 +0200 Subject: [PATCH 13/15] Mark Task 8 done criteria complete; branch mergeable to Cleanup_OM. --- Docs/plans/2026-07-22-dunitx-migration.md | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/Docs/plans/2026-07-22-dunitx-migration.md b/Docs/plans/2026-07-22-dunitx-migration.md index 23cbd51..338af1b 100644 --- a/Docs/plans/2026-07-22-dunitx-migration.md +++ b/Docs/plans/2026-07-22-dunitx-migration.md @@ -11,7 +11,7 @@ **Branch:** `Cleanup_OM-DUnitX-migration` (from `Cleanup_OM`) **PR target (later):** `Cleanup_OM` → eventually `development` -**Status (2026-07-22):** Tasks 1–7 complete. DUnitX has full unit coverage (incl. CCM/ZIP), hardened DPR, and **fail-set parity** with DUnit (12 shared Keccak/GCM failures; 0 DUnitX-only). DUnit retained for comparison. See `Docs/plans/dunitx-parity-log.md`. +**Status (2026-07-22):** Tasks 1–8 complete — branch **mergeable** to `Cleanup_OM`. DUnitX has full unit coverage (incl. CCM/ZIP), hardened DPR, and **fail-set parity** with DUnit (12 shared Keccak/GCM failures; 0 DUnitX-only). DUnit retained for comparison. See `Docs/plans/dunitx-parity-log.md`. ## Global Constraints @@ -509,12 +509,14 @@ git commit -m "Document DUnitX migration plan and dual-suite comparison policy." Migration branch is **complete enough to merge** when: -1. DUnitX project includes **all** test units DUnit has (incl. CCM, ZIP, shared AEAD data). -2. Both suites **compile** on the supported Delphi version used in this environment. -3. Parity run: **no DUnitX-only failures**; fixture sets aligned (no missing suites). -4. DUnit project **still present and green** (comparison baseline). -5. Docs updated; no ChaCha/AEAD drive-by changes. -6. `DUnitX.DUnitCompatibility` may still be used — **OK** for this phase. +- [x] **1.** DUnitX project includes **all** test units DUnit has (incl. CCM, ZIP, shared AEAD data). +- [x] **2.** Both suites **compile** on the supported Delphi version used in this environment. +- [x] **3.** Parity run: **no DUnitX-only failures**; fixture sets aligned (no missing suites). +- [x] **4.** DUnit project **still present and green** (comparison baseline — known 12 shared fails). +- [x] **5.** Docs updated; no ChaCha/AEAD drive-by changes. +- [x] **6.** `DUnitX.DUnitCompatibility` may still be used — **OK** for this phase. + +Verified 2026-07-22 on `Cleanup_OM-DUnitX-migration` (Task 6 logs + source/git audit; full suite re-run optional). Report: `.superpowers/sdd/task-8-report.md`. **Explicitly deferred (later PRs):** From c9a566e927e6206ceb88e0dd0370cb26394c4652 Mon Sep 17 00:00:00 2001 From: Olaf Monien Date: Wed, 22 Jul 2026 16:14:47 +0200 Subject: [PATCH 14/15] Record decision to keep hash-leaf TestIsPasswordHash redeclares. They only affect DUnitX case counts, not fail-set parity; no cleanup needed. --- Docs/plans/dunitx-parity-log.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Docs/plans/dunitx-parity-log.md b/Docs/plans/dunitx-parity-log.md index 0012bed..e313461 100644 --- a/Docs/plans/dunitx-parity-log.md +++ b/Docs/plans/dunitx-parity-log.md @@ -208,3 +208,12 @@ After non-published virtual hooks for ISO10126 padding compare and BCrypt `Expec | DUnitX Debug | **1473** | **12** | **0** | Fail-sets match (12 shared Keccak/GCM). Count gap 37 = remaining redundant hash-leaf `TestIsPasswordHash` redeclares (no longer fail). + +### Decision: leave remaining hash-leaf redeclares as-is + +**Accepted (2026-07-22):** Do **not** remove the ~37 non-password hash leaf +`TestIsPasswordHash` redeclares. They only inflate the DUnitX case count +(base RTTI + leaf re-publish); both invocations pass with the same meaning as +the base (`IsPasswordHash = False`). Fail-set parity is already achieved; no +product or migration defect remains. Optional hygiene only — explicitly out of +scope unless someone wants exact count parity later. From 401874040a26502232c57869fe4872e5b4401fce Mon Sep 17 00:00:00 2001 From: Olaf Monien Date: Wed, 22 Jul 2026 16:33:34 +0200 Subject: [PATCH 15/15] Address CodeRabbit review on DUnitX PR. Define TESTINSIGHT for the TestInsight build config, and reject ISO 10126 padding results whose length does not match the expected pattern before masking random pad bytes. --- Unit Tests/DECDUnitXTestSuite.dproj | 4 ++++ Unit Tests/Tests/TestDECCipherPaddings.pas | 7 +++++++ 2 files changed, 11 insertions(+) diff --git a/Unit Tests/DECDUnitXTestSuite.dproj b/Unit Tests/DECDUnitXTestSuite.dproj index 32b4662..e85c60c 100644 --- a/Unit Tests/DECDUnitXTestSuite.dproj +++ b/Unit Tests/DECDUnitXTestSuite.dproj @@ -169,6 +169,10 @@ MobileGUI;$(DCC_Define) + + + TESTINSIGHT;$(DCC_Define) + 1033 diff --git a/Unit Tests/Tests/TestDECCipherPaddings.pas b/Unit Tests/Tests/TestDECCipherPaddings.pas index b5f80bf..2672e89 100644 --- a/Unit Tests/Tests/TestDECCipherPaddings.pas +++ b/Unit Tests/Tests/TestDECCipherPaddings.pas @@ -757,6 +757,13 @@ function TestTISO10126Padding.RemoveRandomPadding(const Res, Pattern: RawByteStr function TestTISO10126Padding.NormalizeAddPaddingResult(const AValue, APattern: RawByteString): RawByteString; begin + // Length must match first: masking only looks at Pattern indices and would + // truncate a longer result or read past a shorter one, hiding real failures. + if Length(AValue) <> Length(APattern) then + begin + Result := AValue; + Exit; + end; // ISO 10126 fills pad bytes with random data; expected patterns use '?' as wildcards Result := RemoveRandomPadding(AValue, APattern); end;