From 078758e0e3a01f8f1e19cfeff8f0b7e3d2e49882 Mon Sep 17 00:00:00 2001 From: Your Date: Fri, 19 Jun 2026 23:02:44 +0200 Subject: [PATCH] test(pdf): #204 extract PdfManager filename-suffix insertion to pure method + 6 tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract the per-back / FacesOnly PDF filename-suffix contract from GenerateBackFirstOneDocPerBack (inlined three times: the LastIndexOf('.') split computed once, then two Substring interpolations) into a pure, deterministic InsertSuffixBeforeExtension method and pin it with 6 unit tests (8 executions incl. a 3-case Theory). The contract: a deck's base name (e.g. "Cards.pdf") gets a suffix inserted just before its FINAL dot, producing "Cards-1.pdf" (per-back, 1-based counter) and "Cards-FacesOnly.pdf" (back-less cards). This lives in a method already flagged by a "BUGFIX CORRIGÉ" comment. The extraction preserves the original Substring(0, LastIndexOf('.')) / Substring(LastIndexOf('.')) split EXACTLY — including its behavior on a dotless name: LastIndexOf returns -1 and Substring(0, -1) throws ArgumentOutOfRangeException. That throw is the existing fail-loud contract (call sites always pass an extension-bearing base name); the method does NOT silently coerce a dotless name (e.g. via Path.GetFileNameWithoutExtension), which would be a behavior change. Output-neutral: the call sites produce the exact same filenames as before. Full suite green: 374 passed / 0 failed / 5 skipped (no regression). Co-Authored-By: Claude Opus 4.6 --- .../PdfManagerFilenameSuffixContractTests.cs | 137 ++++++++++++++++++ .../WebBasedGenerator/PdfManager.cs | 31 +++- 2 files changed, 163 insertions(+), 5 deletions(-) create mode 100644 Generation/Converters/Argumentum.AssetConverter.Tests/WebBasedGenerator/PdfManagerFilenameSuffixContractTests.cs diff --git a/Generation/Converters/Argumentum.AssetConverter.Tests/WebBasedGenerator/PdfManagerFilenameSuffixContractTests.cs b/Generation/Converters/Argumentum.AssetConverter.Tests/WebBasedGenerator/PdfManagerFilenameSuffixContractTests.cs new file mode 100644 index 00000000..30548067 --- /dev/null +++ b/Generation/Converters/Argumentum.AssetConverter.Tests/WebBasedGenerator/PdfManagerFilenameSuffixContractTests.cs @@ -0,0 +1,137 @@ +using System; +using Argumentum.AssetConverter; +using FluentAssertions; +using Xunit; + +namespace Argumentum.AssetConverter.Tests.WebBasedGenerator +{ + /// + /// Contract pin for — #204 secondary + /// (cont. po-2024): the per-back / FacesOnly PDF filename-suffix contract. + /// + /// emits one PDF per distinct back art, plus + /// an extra -FacesOnly PDF for back-less cards. Each output filename takes the deck's base + /// name and inserts a suffix just before its FINAL dot: Cards.pdf + "-1" → + /// Cards-1.pdf; Cards.pdf + "-FacesOnly" → Cards-FacesOnly.pdf. The suffix + /// carries its own leading separator. + /// + /// This was previously inlined three times (the LastIndexOf('.') split computed once, then + /// two Substring interpolations) in a method already flagged by a BUGFIX CORRIGÉ + /// comment. Extracted output-neutral into so the + /// naming contract is unit-testable in isolation. The extraction preserves the original + /// Substring(0, LastIndexOf('.')) / Substring(LastIndexOf('.')) split EXACTLY — + /// including its behavior on a dotless name (LastIndexOf returns -1, and + /// Substring(0, -1) throws ). That throw is the + /// existing contract, not a bug to fix silently: call sites always pass an extension-bearing base + /// name, and a future caller passing a dotless name should fail loud exactly as before. + /// + /// A regression here (inserting at the FIRST dot, dropping the extension, swapping the + /// counter to 0-based, or naively switching to Path.GetFileNameWithoutExtension — which + /// would change behavior on dotless names) silently produces wrongly-named PDFs that overwrite + /// each other or land in the wrong slot, caught only by inspecting the output directory. + /// + public class PdfManagerFilenameSuffixContractTests + { + // ───────────────────────────────────────────────────────────────────────────── + // (1) THE HEADLINE — a standard extension-bearing base name gets the suffix inserted + // before the FINAL dot. The extension is preserved verbatim. This is the per-back + // counter case (suffix "-1") and the FacesOnly case (suffix "-FacesOnly"). + // ───────────────────────────────────────────────────────────────────────────── + + [Fact] + public void StandardExtension_SuffixBeforeFinalDot_ExtensionPreserved() + { + // "Cards.pdf" + "-1" → "Cards-1.pdf". The ".pdf" stays attached; only the stem is extended. + PdfManager.InsertSuffixBeforeExtension("Cards.pdf", "-1") + .Should().Be("Cards-1.pdf", + "the suffix is inserted before the FINAL dot and the extension is preserved — " + + "the per-back PDF #1 for the 'Cards' deck."); + } + + [Fact] + public void FacesOnlySuffix_SameSplit() + { + // The FacesOnly variant uses the same split, just a different (fixed) suffix string. + PdfManager.InsertSuffixBeforeExtension("Cards.pdf", "-FacesOnly") + .Should().Be("Cards-FacesOnly.pdf", + "the FacesOnly PDF uses the identical stem/extension split as the per-back PDFs."); + } + + // ───────────────────────────────────────────────────────────────────────────── + // (2) Per-back COUNTER — the suffix embeds the 1-based back index. The caller formats the + // counter as $"-{backIndex + 1}" (the loop variable is 0-based; the filename is 1-based). + // The method itself is counter-agnostic — it inserts whatever suffix string it receives — + // so these cases pin the END-TO-END caller contract: backIndex 0→suffix "-1", 1→"-2", etc. + // ───────────────────────────────────────────────────────────────────────────── + + [Theory] + [InlineData(0, "TarotCards-1.pdf")] // first back (0-based loop var 0 → filename "-1") + [InlineData(1, "TarotCards-2.pdf")] // second back (0-based loop var 1 → filename "-2") + [InlineData(9, "TarotCards-10.pdf")] // tenth back (0-based loop var 9 → filename "-10") + public void PerBackCounter_OneBased_LoopVarPlusOne(int loopVar, string expected) + { + // The caller passes $"-{loopVar + 1}" — the 0-based loop variable shifted to a 1-based + // filename. A caller off-by-one (passing $"-{loopVar}") would produce "TarotCards-0.pdf" + // for the first back, which this assertion rejects. + PdfManager.InsertSuffixBeforeExtension("TarotCards.pdf", $"-{loopVar + 1}") + .Should().Be(expected, + $"the per-back suffix is the 0-based loop variable + 1: loop var {loopVar} → " + + $"filename \"{loopVar + 1}\"."); + } + + // ───────────────────────────────────────────────────────────────────────────── + // (3) FINAL dot, not the first — a base name with multiple dots inserts before the LAST one, + // so the earlier dots stay in the stem. "Cards.v2.pdf" + "-1" → "Cards.v2-1.pdf" (the + // ".pdf" is the extension; "v2" stays in the stem). A regression that split at the FIRST + // dot would yield "Cards-1.v2.pdf" — wrong extension, wrong stem. + // ───────────────────────────────────────────────────────────────────────────── + + [Fact] + public void MultipleDots_SplitsAtFinalDot_KeepsInnerDotsInStem() + { + PdfManager.InsertSuffixBeforeExtension("Cards.v2.pdf", "-1") + .Should().Be("Cards.v2-1.pdf", + "the split is at the LAST dot, so inner dots stay in the stem and only the true " + + "extension '.pdf' is preserved. A first-dot split would wrongly yield 'Cards-1.v2.pdf'."); + } + + // ───────────────────────────────────────────────────────────────────────────── + // (4) Two-dot / dotless-extension variants — a name ending in a dot, or with a dot only as + // the extension separator with an empty stem. Edge cases of the LastIndexOf('.') split. + // ───────────────────────────────────────────────────────────────────────────── + + [Fact] + public void DotOnlyAsExtension_EmptyStem_SuffixBeforeDot() + { + // ".pdf" (stem empty, LastIndexOf('.') == 0) + "-1" → "-1.pdf". Degenerate but deterministic. + PdfManager.InsertSuffixBeforeExtension(".pdf", "-1") + .Should().Be("-1.pdf", + "when the stem is empty (the dot is at index 0), the suffix is inserted before it " + + "and the extension '.pdf' is still preserved."); + } + + // ───────────────────────────────────────────────────────────────────────────── + // (5) DOTLESS NAME FAILS LOUD — the existing contract. LastIndexOf('.') returns -1, and + // Substring(0, -1) throws ArgumentOutOfRangeException. This is NOT a bug to fix silently: + // call sites always pass an extension-bearing base name, and switching to + // Path.GetFileNameWithoutExtension would SILENTLY change behavior (producing "Cards-1" + // instead of throwing). Pinning the throw keeps the contract fail-loud for a future caller + // that mistakenly passes a dotless name. + // ───────────────────────────────────────────────────────────────────────────── + + [Fact] + public void DotlessName_ThrowsArgumentOutOfRange_FailsLoud() + { + // "Cards" (no dot) → LastIndexOf('.') == -1 → Substring(0, -1) throws. This is the existing + // behavior, preserved output-neutral. A naive "fix" using Path.GetFileNameWithoutExtension + // would return "Cards-1" (no extension) instead — a silent behavior change this test rejects. + Action act = () => PdfManager.InsertSuffixBeforeExtension("Cards", "-1"); + + act.Should().Throw( + "a dotless base name has no extension split point — LastIndexOf('.') returns -1 and " + + "Substring(0, -1) throws. This is the existing fail-loud contract: call sites always " + + "pass an extension-bearing name, and the method must NOT silently coerce a dotless name " + + "(e.g. via Path.GetFileNameWithoutExtension), which would change behavior."); + } + } +} diff --git a/Generation/Converters/Argumentum.AssetConverter/WebBasedGenerator/PdfManager.cs b/Generation/Converters/Argumentum.AssetConverter/WebBasedGenerator/PdfManager.cs index d53d54d6..6072542a 100644 --- a/Generation/Converters/Argumentum.AssetConverter/WebBasedGenerator/PdfManager.cs +++ b/Generation/Converters/Argumentum.AssetConverter/WebBasedGenerator/PdfManager.cs @@ -91,11 +91,33 @@ public static IEnumerable OrderImagesForAlternateFaceAndBack(IEnumerable } } + /// + /// Inserts into just before its + /// FINAL dot, producing e.g. Cards-1.pdf from Cards.pdf + "-1", or + /// Cards-FacesOnly.pdf from Cards.pdf + "-FacesOnly". The suffix INCLUDES + /// its own leading separator (the call sites pass "-N" / "-FacesOnly"). + /// + /// Extracted output-neutral from so the per-back + /// and FacesOnly naming contract is unit-testable in isolation. The implementation preserves + /// the original baseName.Substring(0, LastIndexOf('.')) / Substring(LastIndexOf('.')) + /// split EXACTLY — including its current behavior when there is no dot: LastIndexOf + /// returns -1, and Substring(0, -1) throws . + /// That throw is the existing contract (call sites always pass an extension-bearing base name); + /// this method does NOT silently "fix" it, so a future caller passing a dotless name fails loud + /// exactly as before. A regression here (inserting at the FIRST dot, dropping the extension, or + /// off-by-one on the per-back counter) silently produces wrongly-named PDFs that overwrite each + /// other or land in the wrong slot — caught only by inspecting the output directory. + /// + public static string InsertSuffixBeforeExtension(string baseFileName, string suffix) + { + var indexInsert = baseFileName.LastIndexOf('.'); + return baseFileName.Substring(0, indexInsert) + suffix + baseFileName.Substring(indexInsert); + } + public void GenerateBackFirstOneDocPerBack(string baseName, List cardImages, bool overwriteExistingDocs) { var targetFiles = new List<(string fileName, Func documentImages)>(); - var indexInsert = baseName.LastIndexOf('.'); - + // BUGFIX CORRIGÉ: Partitionner les cartes avec/sans dos au lieu de filtrer var cardsWithBack = cardImages.Where(card => !string.IsNullOrEmpty(card.Back)).ToList(); var cardsWithoutBack = cardImages.Where(card => string.IsNullOrEmpty(card.Back)).ToList(); @@ -124,8 +146,7 @@ public void GenerateBackFirstOneDocPerBack(string baseName, List car return collec; }; - var newName = - $"{baseName.Substring(0, indexInsert)}-{backIndex + 1}{baseName.Substring(indexInsert)}"; + var newName = InsertSuffixBeforeExtension(baseName, $"-{backIndex + 1}"); targetFiles.Add((newName, collecBuilderBF)); } @@ -138,7 +159,7 @@ public void GenerateBackFirstOneDocPerBack(string baseName, List car return collec; }; - var facesOnlyName = $"{baseName.Substring(0, indexInsert)}-FacesOnly{baseName.Substring(indexInsert)}"; + var facesOnlyName = InsertSuffixBeforeExtension(baseName, "-FacesOnly"); targetFiles.Add((facesOnlyName, collecBuilderFacesOnly)); AnsiConsole.MarkupLine($"[cyan]INFO: Creating additional 'FacesOnly' PDF for {cardsWithoutBack.Count} cards without back[/]"); }