test(visual): fail loud on cold-start across Pdf*Tests (#957 residu ii) - #966
Merged
Conversation
#963 (f7875f6) closed the cold-start faux-vert for VisualQaHarness. The same EnsureTarget() pattern carried across ~20 tests in PdfDimensionTests, PdfSnapshotTests, and PdfContentTests — each one a pass-on-nothing: a missing Target/ made the test green without asserting anything. Same correctif as #963: the silent `return;` removed, replaced by an explicit Assert.Fail("..."). A subtraction, not a counterweight — no [Fact(Skip)] (static in xUnit, would kill the tests where they work), no continue-on-error, no fabricated coverage. Secondary guards that verified nothing (missing lang dir, 0 PDFs, pdf==null, sizes.Count<2, missing expected file) get the same treatment, so a *partial* Target/ (some languages missing, a CardSet absent) can't pass silently either — those were the subtler faux-verts. CI scope verified: VisualTests.csproj is in the .sln (compiled by `dotnet build`), but build.yml's Test step targets ONLY Argumentum.AssetConverter.Tests .csproj, so these tests do NOT run in CI. Assert.Fail therefore fails locally (run the pipeline first), not in the release gate. Scope: test code only (~+30/-10 across 3 files). 0 prod write, 0 CSV, 0 OWL. Build: 0 Avertissement(s) 0 Erreur(s) on VisualTests.csproj. Co-Authored-By: Claude-Code <noreply@anthropic.com>
This was referenced Jul 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Closes the remainder of the faux-vert class that #963 (
f7875f68) opened for one harness. ai-01's body in #963 notes thatEnsureTarget()inPdfDimensionTests/PdfSnapshotTests/PdfContentTests(~20 tests) carries the identical cold-start pattern and calls for the same correctif. This PR de-gates it.The pattern: every test began
if (!EnsureTarget()) return;whereEnsureTarget()returnedfalseon a missingTarget/with only an_output.WriteLine("Skipped…"). A missingTarget/→ the test returned green without asserting anything. Pass-on-nothing.The fix — a subtraction, not a counterweight
Identical to #963:
private bool EnsureTarget()→private void EnsureTarget(); the silentreturn falsebody becomesAssert.Fail("…require a generated Target/ — run the pipeline first. This test verified nothing.").if (!EnsureTarget()) return;call site →EnsureTarget();.Target/(one language missing, a CardSet absent, the expected file not there) was passing silently too:if (pdfs.Count == 0) return;→Assert.Failif (!Directory.Exists(dir)) return;→Assert.Failif (pdfs.Length == 0) return;→Assert.Failif (pdf == null) return;→Assert.Failif (sizes.Count < 2) return;→Assert.Failif (!File.Exists(pdfPath)) { WriteLine; return; }→Assert.Fail(PdfSnapshotTests)No
[Fact(Skip)](static in xUnit 2.9.3 v2 — would kill the tests where they actually work). No continue-on-error. No fabricated coverage. EachAssert.Failmessage names what was missing and ends "test verified nothing" so the intent is unmistakable in the runner output.CI scope — verified (the DoD ask)
Argumentum.AssetConverter.VisualTests.csprojis inArgumentum Converters.sln→ it is compiled bydotnet build(and by CI's build step)..github/workflows/build.ymlTest step runsdotnet test "Generation/Converters/Argumentum.AssetConverter.Tests/Argumentum.AssetConverter.Tests.csproj"— targets onlyTests.csproj, not VisualTests.Impact: these
Assert.Fails fire locally (developer / po-2023 running the pipeline then the visual suite), not in the release gate. CI stays green regardless. This matches #963's finding.Files
PdfDimensionTests.csPdfSnapshotTests.csPdfContentTests.cs~20 tests total. Test code only.
Build / test
Build (compile proof):
Behaviour proof (one filtered test,
--no-build): this machine'sTarget/exists but has no generated PDFs (noDocuments/density-0/) — the exact cold-start/partial state the fix targets.These 4 failures ARE the fix working, not a regression. Before this PR, the same 4 variants passed green and silent (
Target/present →EnsureTarget()returned true →if (pdfs.Count == 0) return;skipped quietly). After, they fail loud at the exact line added here (line 77) with a message ending "test verified nothing". TheEnsureTarget()Assert path itself is the identical pattern already shipped in #963 (f7875f68, merged & working); on this machineTargetRootexists so that branch isn't reached — the secondary guards are, and they fire correctly.A fully-green
dotnet testover the whole VisualTests suite is assertable only by a machine that has run the generation pipeline (a populatedTarget/{lang}/Documents/density-0/with PDFs) — a heavy lane run, out of scope for this test-code-only PR.Scope
Test code only (~+30/−10, 3 files). 0 prod write, 0 CSV, 0 OWL, 0 CardPen. Base:
ff80107b(ai-01 dispatch).DoD (mirror #963)
return;removed, replaced by explicitAssert.Fail("…")— all 3 files[Fact(Skip)], no continue-on-error, no fabricated coverage🤖 po-2024