The Test target derives a project's results directory in two places

[phmatray]

Problem / motivation

build/Build.cs's Test target derives a project's results directory in two places.

The intended single home is a local function, and its comment says so in as many words:

// One home for the path, because the run and the guard below have to agree on it: if
// they ever computed it differently the guard would inspect a directory nothing wrote
// to, and fail every run for a reason that has nothing to do with the reporters.
AbsolutePath ResultsDirectoryFor(Project project) => TestResultsDirectory / project.Name;

The "did not complete" branch of the failure summary rebuilds the same formula by hand instead:

failed.Select(name => $"{name} (reports, if any, in {TestResultsDirectory / name})")

So the invariant the comment claims — one home — is not actually held. It is held for the run and
the guard, which are the two the comment names, and quietly broken for the third caller.

The impact is latent, not current. Today both expressions produce the same string, so nothing is
wrong on disk or on screen. It becomes wrong the moment ResultsDirectoryFor changes shape — sanitise
the project name, include the TFM, nest by configuration — all plausible now that three suites run
(FormCraft.UnitTests, FormCraft.ForMudBlazor.UnitTests, FormCraft.ForFluentUI.UnitTests). At that
point the runner and the report guard still agree with each other, while the crash summary points a
reader at a directory that does not exist — and the reader reaching for it is, by definition, someone
already debugging a suite that died.

Nothing covers this branch, so the drift would be silent: it only renders when a suite fails to
complete, which no test exercises.

This is the same drift-between-two-copies shape that #276 (PR #325) removed on the promise-vs-guard
axis — the Test target used to state the promised report kinds in .Produces and restate them in the
guard, and now reads one ReporterBackedReports list twice. That change left the path axis standing.

Proposed solution

Give the path exactly one definition and route all three callers through it.

The mechanical obstacle is that failed is a List<string> of project names, while
ResultsDirectoryFor takes a Project — which is why the failure branch hand-rolled the path in the
first place. Hold Project instances in failed instead, and the third caller can use the same local
function the other two do:

var failed = new List<Project>();
…
failed.Add(project);
…
.Where(project => !failed.Contains(project))
…
failed.Select(project => $"{project.Name} (reports, if any, in {ResultsDirectoryFor(project)})")

ResultsDirectoryFor itself is untouched, so the Ci/ guard that pins
TestResultsDirectory / project.Name — described there as "the behaviour this whole issue turns on"
for #256 — keeps matching unchanged.

Alternatives considered

• Make the local function take a name (ResultsDirectoryFor(string projectName)) and pass
  project.Name at the other two call sites. Also gives one formula, but it rewrites the text that
  three TestReportingTests assertions match — including
  build.ShouldMatch(@"TestResultsDirectory\s*/\s*project\.Name"), the Test reports are named by runner and timestamp, so a downloaded artifact cannot be attributed to a suite #256 pin. More guard churn for
  the same result. Note C# forbids overloading a local function, so "add an overload" is not on the
  table without promoting it to a method.
• Look the Project up by name in the failure branch
  (testProjects.First(p => p.Name == name)). Keeps failed as strings but is strictly worse to read
  and adds a lookup that can throw.
• Leave it and add a comment saying the two must be kept in sync. This is what the current code
  effectively does, and it is the thing The .html test report is promised but unenforced, so half the artifact can vanish silently #276 argued against: a comment is inert prose, and the
  invariant survives only until someone edits one side.

Area

Build/CI infrastructure — build/Build.cs (Test target's failure summary) and the guards in
FormCraft.UnitTests/Ci/. No library code.

Related: #276 (the same drift shape, promise-vs-guard axis, just closed via PR #325), #256 (added
the per-project results directory and ResultsDirectoryFor), #231 (the declared-but-unenforced defect
this family of guards exists for).

🧠 Brainstorm

Problem / context

The Test target has accumulated three readers of "where does project X's reports live": the
DotNetTest invocation (--results-directory), the emitted-report guard, and the failure summary's
"did not complete" branch. #256 introduced ResultsDirectoryFor to unify the first two and wrote a
comment explaining why that mattered. The third was added in the same family of changes (#259) and
never routed through it, because it iterates a List<string> rather than the projects themselves.

The cost is not a present bug — the strings are identical today. The cost is that a comment asserting
an invariant is now false, which is worse than having no comment: the next person to change
ResultsDirectoryFor reads "one home for the path" and reasonably stops looking.

Approaches

A. Hold Project in failed. Change the list's element type; the failure branch then calls
ResultsDirectoryFor(project) like everyone else.
Trade-offs: smallest diff at the definition — ResultsDirectoryFor and its TestResultsDirectory / project.Name body are untouched, so the #256 text pin keeps matching. Requires updating exactly one
Ci/ assertion (the completed-projects filter added by #276, which matches
!\w+\.Contains\(\w+\.Name\)), because membership stops being tested on .Name. Also worth a moment's
thought: List<Project>.Contains uses default equality, which for Nuke's Project is reference
identity — correct here since every entry comes from the same testProjects instances, but it should
be stated rather than assumed.

B. Make the local function take a name. ResultsDirectoryFor(string projectName).
Trade-offs: arguably the more natural signature, and it makes the failure branch a one-word change.
But it rewrites the body the guards read, reddening three assertions at once including #256's, and
those guards are the repo's stated defence for this exact file. Same destination, more collateral.

C. Do nothing; document the coupling. Zero risk today.
Trade-offs: leaves a comment that claims an invariant the code does not hold — the failure mode
#231/#276 are both about, at small scale.

Recommendation

A. It reaches one-formula with the least disturbance to the guards that protect this file, and it
leaves the #256 pin — the assertion the per-project layout actually turns on — matching untouched
text. The one guard it does move is the newest one (#276's), which is cheap to re-express and whose
subject ("only projects that completed are asked for reports") is unchanged by the refactor.

Assumptions: failed's entries always come from the testProjects list, so instance identity is a
sound membership test; *.log guarding stays out of scope (that is its own question, see #276's
Alternatives); no behaviour change is intended — identical output for identical runs.

📋 Spec

Goal

build/Build.cs derives a project's results directory in exactly one place, and the Test target's
three readers of that path all go through it — including the "did not complete" branch.

Scope

• build/Build.cs — failed holds Project; the failure summary calls ResultsDirectoryFor.
• FormCraft.UnitTests/Ci/TestReportingTests.cs — the completed-projects assertion is re-expressed for
  the new membership test, and a guard is added so a fourth hand-rolled derivation cannot reappear.

Non-goals

• No change to the emitted-report guard's coverage (The .html test report is promised but unenforced, so half the artifact can vanish silently #276's ReporterBackedReports list stays as-is).
• No guard for *.log — different emitting mechanism, its own question.
• No change to what the failure summary says, only to where it gets the path from.

Behaviour

Output is byte-identical for every run today. The change is structural.

flowchart LR
    R[ResultsDirectoryFor] --> A[--results-directory<br/>on DotNetTest]
    R --> B[emitted-report guard]
    R -.->|missing today| C["did not complete"<br/>summary]
    D[TestResultsDirectory / name<br/>hand-rolled] --> C

Loading

After the change the dashed edge is real and the D node is gone.

Key files

• build/Build.cs — var failed = new List<string>(), failed.Add(project.Name),
  .Where(project => !failed.Contains(project.Name)), and the failed.Select(name => …) summary.
• FormCraft.UnitTests/Ci/TestReportingTests.cs —
  BuildScript_Should_Ask_For_Reports_Only_From_Projects_That_Completed.

Validation

• The full suite stays green; the emitted-report guard behaves identically.
• Forcing a suite to fail still reports it once as "did not complete", naming its directory (ci: give each test project its own results directory (#256) #259's
  property) — and that directory now comes from ResultsDirectoryFor.
• A new Ci/ assertion fails if any TestResultsDirectory / <x> composition reappears outside
  ResultsDirectoryFor and the .Produces promises.

Edge cases

• Membership. List<Project>.Contains is reference equality for Nuke's Project. Every entry
  originates in testProjects, so this holds — but if the implementer prefers, compare on .Name
  explicitly and say so, rather than leaving it implicit.
• The .Produces lines legitimately compose TestResultsDirectory / "**" / report — the new
  no-second-derivation guard must exempt the promises, or it fails on correct code.

Assumptions

Behaviour-preserving refactor; Ci/ guards stay text assertions over Build.cs, consistent with the
existing pattern — no MSBuild or YAML parser is introduced.

🛠️ Implementation plan

For agentic workers: REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (- [ ]) syntax for tracking.

Goal: one definition of a project's results directory, with every reader in the Test target
going through it.

Architecture: build infrastructure. Nuke build/Build.cs drives dotnet test; the Ci/ guard
suite asserts on that file's text so a regression fails a test rather than going unnoticed.

Tech stack: Nuke 10.1.0, .NET SDK pinned by global.json, xUnit v3 on Microsoft.Testing.Platform.

Global Constraints

• Base branch is dev, not main. The PR targets dev.
• Commit identity: Philippe Matray <phmatray@gmail.com>.
• TreatWarningsAsErrors=true is deliberate — never relax it to make a build pass.
• dotnet test --filter is inert under MTP (warning MTP0001) — always run the full suite
  (dotnet test -c Release) and never report a filtered result.
• The emitted-report guard may become stricter, never weaker. Preserve both ci: give each test project its own results directory (#256) #259 properties: the
  guard runs only for projects that completed, and every offender is surfaced in one throw
  naming project + directory + missing kinds.
• ⛔ Do not use Project.GetProperty(...) or anything else that evaluates a csproj with MSBuild
  in-process — it passes locally and fails every CI run (Could not load file or assembly 'NuGet.Frameworks'). Test-project discovery reads the csproj as text for exactly that reason.
• This is a behaviour-preserving refactor: identical output for identical runs.
• CHANGELOG.md is owned by release-please — never hand-edit it.

Task 1: Route the failure summary through ResultsDirectoryFor

Files: modify build/Build.cs (Test target); modify
FormCraft.UnitTests/Ci/TestReportingTests.cs.

Interfaces: failed becomes List<Project>; the "did not complete" entry reads
$"{project.Name} (reports, if any, in {ResultsDirectoryFor(project)})". ResultsDirectoryFor is
unchanged.

• Step 1: Update BuildScript_Should_Ask_For_Reports_Only_From_Projects_That_Completed so its
  regex expresses membership without requiring .Name, and add an assertion that the "did not
  complete" entry gets its directory from ResultsDirectoryFor rather than composing one. Run
  dotnet test -c Release → RED.
• Step 2: In build/Build.cs, change failed to List<Project>, failed.Add(project), and
  .Where(project => !failed.Contains(project)). Note in a comment why instance identity is a
  sound membership test here (entries all originate in testProjects).
• Step 3: Rewrite the "did not complete" summary to ResultsDirectoryFor(project), deleting
  the hand-rolled TestResultsDirectory / name.
• Step 4: Run dotnet test -c Release → GREEN.
• Step 5: Commit: refactor(build): give the results directory one definition (#<this issue>)

Task 2: Make a fourth hand-rolled derivation fail a test

Files: modify FormCraft.UnitTests/Ci/TestReportingTests.cs.

Interfaces: none.

• Step 1: Add a guard asserting no TestResultsDirectory / <x> composition exists outside
  ResultsDirectoryFor's definition and the .Produces promises — the promises legitimately
  compose TestResultsDirectory / "**" / report, so exempt them explicitly. Run
  dotnet test -c Release → GREEN (the code already satisfies it after Task 1).
• Step 2: Prove the new guard is not vacuous: temporarily reintroduce a hand-rolled
  TestResultsDirectory / name somewhere in the target, run dotnet test -c Release, confirm the
  new guard — and only it — fails, then restore.
• Step 3: Run dotnet test -c Release → GREEN.
• Step 4: Commit: test(ci): pin that the results directory has one definition (#<this issue>)

Task 3: Confirm the crash path still reads correctly

Files: none expected — this task verifies Tasks 1-2.

Interfaces: none.

• Step 1: Temporarily force one suite to fail, run ./build.sh Test, and confirm that project
  is still reported once as "did not complete" and not as a missing report (ci: give each test project its own results directory (#256) #259's
  property), with its directory named — then restore.
• Step 2: Confirm a normal ./build.sh Test still succeeds and each
  test-results/<project>/ holds a .trx, an .html and the .log.
• Step 3: Run dotnet test -c Release → GREEN.
• Step 4: Commit (only if anything changed): docs(build): note the single results-directory home (#<this issue>)