Skip to content

DUnitX migration: full suite parity, keep DUnit for comparison - #97

Merged
MHumm merged 18 commits into
MHumm:developmentfrom
omonien:pr-dunitx-migration
Aug 21, 2026
Merged

DUnitX migration: full suite parity, keep DUnit for comparison#97
MHumm merged 18 commits into
MHumm:developmentfrom
omonien:pr-dunitx-migration

Conversation

@omonien

@omonien omonien commented Jul 23, 2026

Copy link
Copy Markdown

For Markus — theme: test infrastructure (prerequisite)

This is the foundation for the cleanup work: make DUnitX a complete, parity-proven runner for the whole suite, while keeping classic DUnit as a comparison baseline.

No production crypto algorithms are changed here.

Why this matters for you

Before larger AEAD / cipher-mode work (and before reviewing a split of PR #90), we need a single trustworthy test signal. Today both runners exist; DUnitX was incomplete and had DUnitX-only failures from dual-stack quirks. After this PR:

  • DUnitX covers the same unit set as DUnit (incl. CCM, ZIP helper, AEAD shared test data)
  • Fail-set parity: same failures on both runners (no DUnitX-only reds)
  • Classic DUnit stays until you explicitly decide to remove it later

What changed (high level)

Area Change
DECDUnitXTestSuite Full unit list; DPR aligned with DUnitX practice (explicit fixtures, UseRTTI := False, console/CI exit codes, TestInsight path)
Dual-stack tests Registration gaps fixed; ISO10126 / BCrypt no longer redeclare published methods in a way that double-discovers under DUnitX
Docs Migration plan + parity log; roadmap section on DUnitX
readme.md Notes DUnitX as preferred runner

Parity (Delphi 13 / Win32, at time of work)

Suite Run Failed
DUnit Console 1436 12
DUnitX Debug 1473 12
  • 0 DUnitX-only failures after fixes
  • The same 12 shared failures were pre-existing product/test debt (Keccak vectors + GCM stream chunked) — fixed in separate PRs
  • Residual +37 DUnitX case count from hash-leaf TestIsPasswordHash redeclares — accepted (count noise only, no fail impact)

Out of scope (intentionally later)

  • Removing the classic DUnit project
  • Switching asserts from Check* / DUnitCompatibility to native Assert.*
  • Fixing Keccak / GCM product bugs (own PRs)

Suggested review focus

  1. DPR / project wiring for DUnitX — acceptable for CI and local runs?
  2. Test-only API hooks (NormalizeAddPaddingResult, ExpectedIsPasswordHash) — OK as dual-stack bridge?
  3. Decision to keep DUnit temporarily — documented; removal is a separate future PR

Risk

Low for library users (tests + docs). Medium for maintainers who still open the DUnit project (still valid).

Dependency note

This branch includes Docs/Cleanup-Roadmap.md (updated for the DUnitX phase). If PR #96 (roadmap only) is already merged, GitHub should auto-merge the common docs with little conflict; otherwise this PR alone is self-contained.

Test plan

  • Build Source\DEC60.dproj Debug Win32
  • Build + run DECDUnitTestSuite Console Win32
  • Build + run DECDUnitXTestSuite Debug Win32
  • Spot-check: CCM and ZIP fixtures present under DUnitX

Details: Docs/plans/2026-07-22-dunitx-migration.md, Docs/plans/dunitx-parity-log.md

@omonien omonien mentioned this pull request Jul 23, 2026
Open
omonien added 15 commits July 27, 2026 11:16
@omonien
Document cleanup roadmap: split PR MHumm#90 and DUnitX-first.
001f668 
Record that AEAD architecture must be separated from ChaCha features,
and that a full DUnit to DUnitX migration is the prerequisite before
that work.
@omonien
Document DUnitX migration plan; keep DUnit for parity comparison.
1b1b32f 
Inventory dual suites, sense-check existing tests, and record that
DECDUnitTestSuite stays until a later removal PR.
@omonien
Document DUnitX DPR best-practice audit against official examples.
dae2172 
Compare DECDUnitXTestSuite.dpr to VSoft D12/D13 templates; fold runner
hardening into Task 3 of the migration plan.
@omonien
Task 1: baseline DUnit and DUnitX runners (parity log)
8edef9d 
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.
@omonien
Clarify DUnit vs DUnitX define ownership for dual-suite builds.
e57bf63
@omonien
Complete DUnitX suite units and align DPR with DUnitX best practices.
ac0e061
@omonien
Fix dual-stack test registration gaps for DUnitX parity.
87cd62d
@omonien
Document Task 5 DUnit/DUnitX pass-fail parity comparison.
b6ef397
@omonien
Fix DUnitX-only failures from redeclared published test methods.
2128549 
Use non-published virtual hooks (NormalizeAddPaddingResult, ExpectedIsPasswordHash) so ISO10126 and BCrypt keep correct assertions without DUnitX double-discovery.
@omonien
Minor test clarity fixes found during DUnitX parity review.
54ab976
@omonien
Document Task 6 fail-set parity after DUnitX-only failure fixes.
ee45678
@omonien
Document DUnitX migration plan and dual-suite comparison policy.
486c17f
@omonien
Mark Task 8 done criteria complete; branch mergeable to Cleanup_OM.
7d34e02
@omonien
Record decision to keep hash-leaf TestIsPasswordHash redeclares.
af2bd43 
They only affect DUnitX case counts, not fail-set parity; no cleanup needed.
@omonien
Address CodeRabbit review on DUnitX PR.
a2a57e2 
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.
@omonien
omonien force-pushed the pr-dunitx-migration branch from 4018740 to a2a57e2 Compare July 27, 2026 09:16
@MHumm

MHumm commented Aug 16, 2026

Copy link
Copy Markdown
Owner

Started with evaluation of this one. Will take a bit of time.
I got it now that one has sub targets under the Debug build configuration now.

I didn't fully understand this part of the comment in the .dpr header yet:

// — leaving the inc define off keeps DECDUnitTestSuite
// on classic DUnit when the same test units are opened/built under that project.

@MHumm

MHumm commented Aug 20, 2026

Copy link
Copy Markdown
Owner

I tried to get the DUnitX test project running with TestInsight now, but I have some issues:

  • I can select the TestInsight "sub" buildconfigurationm and run all tests by pressing F9
  • The play etc. buttons in TestInsight's toolbar never get enabled
  • Right clicking on a tree node in TestInsight and selecting to run these node's tests doesn't seem to do anything
  • Some tests show the red message "Während des Tests wurden keine Annahmen vorgenommen".
    For instance this one: procedure TestTPaddingBase.TestRemovePadding_RawByteStringExceptions;
    My suspicion is, that we need to change the implementation of these. They test for occurance of certain exceptions
    and at least what I know from DUnit it is not the way one would implement this in DUnit.

@MHumm

MHumm commented Aug 20, 2026

Copy link
Copy Markdown
Owner

Another small issue: by changing the encoding of TestDECCpherModesCCM.pas from ANSI to UTF8 (which is ok) some3 German umlaut in some comment got replaced by a non printable char. This is in the methods TestTDECCCM.TestDecode, TestTDECCCM.TestEncode, TestTDECCCM.DoTestEncodeStream_TestSingleSet and most likely the comments should be translated into English anyway.

cursoragent and others added 2 commits August 21, 2026 06:28
@cursoragent @omonien
Fix UTF-8 corruption in CCM test AAD comments.
3d430eb 
ANSI→UTF-8 conversion left U+FFFD in four German prüfen comments;
replace with English as requested in PR review.

Co-authored-by: Olaf Monien <omonien@users.noreply.github.com>
@cursoragent @omonien
Add UTF-8 BOM to TestDECCipherModesCCM.pas.
8dbfb99 
Delphi sources in this project require UTF-8 with BOM (EF BB BF).

Co-authored-by: Olaf Monien <omonien@users.noreply.github.com>
This was referenced Aug 21, 2026
Open
Open
@omonien

omonien commented Aug 21, 2026 via email

Copy link
Copy Markdown
Author

@cursoragent @omonien
Clarify DUnitX DPR comment and fix padding exception assertions.
1860146 
Rewrite the DECDUnitXTestSuite header so it is clear that DUnitX comes
from the .dproj defines, not TestDefines.inc. Use WillRaise/CheckException
for padding negative tests so DUnitX records an assertion on the raise path.

Co-authored-by: Olaf Monien <omonien@users.noreply.github.com>
@omonien

omonien commented Aug 21, 2026

Copy link
Copy Markdown
Author

Hallo Markus,

zu den beiden anderen Punkten:

Der Kommentar im .dpr war unglücklich formuliert. DUnitX kommt aus den Compiler-Defines der .dproj (Debug/Release/TestInsight), nicht aus TestDefines.inc. Das Include bleibt ohne DUnitX, damit dieselben Test-Units unter DECDUnitTestSuite weiter klassisches DUnit sind. Ich habe den Text im Header entsprechend klarer gemacht.

Zu TestInsight: F9 kenne ich so wie du. Die Play-/Rechtsklick-Buttons in der TestInsight-Toolbar sind eher ein IDE-/Plugin-Thema; daran habe ich nichts geändert. Den roten Hinweis „keine Annahmen vorgenommen“ (z. B. TestRemovePadding_RawByteStringExceptions) hast du richtig gesehen — die Tests haben die Exception nur geschluckt, ohne Check*/Assert.*. Die beiden Padding-Exception-Tests nutzen jetzt Assert.WillRaise (DUnitX) bzw. CheckException (DUnit).

Viele Grüße
Olaf

@MHumm

MHumm commented Aug 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Ok, schaue ich mir mal an. Ich kenne eigentlich beim TestInsight die Play usw. Buttons auch als blaue Buttons die ich anklicken kann. Ok, jetzt sind die blau.

Rest schaue ich mir mal weiter an. Noch eine Frage: wie ist das mit diesen Mobile Tests?
Ich erinnere mich an eine Jahre zurück liegende Beta oder so wo es von DUnitX auch einen FMX GUI Runner gab, aber EMBT hat scheinbar das Dateiausliefern vermasselt, so dass der für mich gar nicht compilierbar war. Und Consolen Programme für Mobile gibt's trotz meines Jahre alten Requests dafür m.W. immer noch nicht. Ok, Prio ist da nicht so hoch...
Die Debug/GUI Build-Konfiguration scheint ja aber doch alles als Console auszuführen oder liege ich da irgendwie falsch?
Wie ist das mit MobileGUI? Wenn ich mein Android Smartphone oder Tablet verbinde kann ich das dann darauf ausführen? Muss ich wohl mal testen...

@MHumm
MHumm merged commit 2509cff into MHumm:development Aug 21, 2026
@omonien

omonien commented Aug 22, 2026 via email

Copy link
Copy Markdown
Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@omonien @MHumm @cursoragent