Skip to content

Commit f05c3fd

Browse files
radicalCopilot
andcommitted
test(ci): cover SelectTests enforce/audit contract and Layer 1 git edges
The selector engine (TestSelector) had 49 acceptance tests, but the boundary that actually gates CI in enforce mode -- Selection.Run's GITHUB_OUTPUT contract, change resolution, and the Layer 1 graph integration -- had almost no coverage. That boundary is the sole gate once ENFORCE_SELECTION flips to true, so an untested run_* mistranslation or under-selection would silently skip tests or jobs. Add tests that each pin a concrete regression, using synthetic maps (no coupling to eng/test-trigger-map.yml): SelectTestsCliTests (run_* + matrix contract): - audit forces every run_* true even on a subset; enforce emits real per-job values and the job: -> run_ name mapping; every map job appears as a key (including group-only and derived-only jobs); ALL via a path rule runs everything with no restriction props. - no --from/--changed-files/--force-all throws; --from/--to is a two-dot diff (diverging git repo so a three-dot regression drops a file); --from alone diffs the working tree; --changed-files trims whitespace/blank lines. - docs-only PR selects nothing -> empty OverrideProjectToBuild (build nothing, the intended outcome); a production project name in the selection produces no override item. GraphAffectedProjectsTests (Layer 1 git/import edges): - a change to an imported Directory.Build.props fans out to importers (ImportPaths index); empty change set returns empty; deepest-directory attribution prefers a nested project over its parent; a cross-project git rename attributes both old and new owners (first test on the git diff path). SelectTestsLayer1IntegrationTests (new): Selection.Run end-to-end with Layer 1 enabled -- a production-source change flows through the reverse closure into the enforce-mode props. Serialize the env-mutating classes into the existing GraphAffectedProjects collection. GITHUB_OUTPUT/GITHUB_STEP_SUMMARY are process-global env vars, so running these classes in parallel let one test's temp dir be deleted while another wrote its side-channel files -- a latent flaky-test hazard for any future Selection.Run test. Delete docs/ci/test-trigger-selector-test-plan.md: its Layer 1 section stubbed a dotnet-affected executable and affected.json parsing that no longer exist (Layer 1 is now the in-process GraphAffectedProjects graph). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent ab4a6cf commit f05c3fd

4 files changed

Lines changed: 749 additions & 51 deletions

File tree

tests/Infrastructure.Tests/TestTriggerMap/GraphAffectedProjectsTests.cs

Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using Aspire.SelectTests;
5+
using System.Diagnostics;
56
using Xunit;
67

78
namespace Infrastructure.Tests.TestTriggerMap;
@@ -107,6 +108,75 @@ public void FileOutsideEveryProjectDirSelectsNothing()
107108
Assert.Empty(affected);
108109
}
109110

111+
// Failure mode: a change to a repo build file imported by every project (Directory.Build.props,
112+
// and by the same mechanism eng/Versions.props etc., captured via ProjectInstance.ImportPaths) is
113+
// not attributed to the projects that import it, so a global build/version change silently runs no
114+
// tests. It must fan out to every importing project. (The file lives at the repo root, under no
115+
// project dir, so ONLY the ImportPaths index — not the containment fallback — can catch it.)
116+
[Fact]
117+
public void ChangeToImportedBuildPropsAffectsImportingProjects()
118+
{
119+
using var repo = new GraphFixture();
120+
121+
var affected = repo.Compute("Directory.Build.props");
122+
123+
Assert.Contains("Core", affected);
124+
Assert.Contains("Mid", affected);
125+
Assert.Contains("AppTests", affected);
126+
Assert.Contains("Other", affected);
127+
}
128+
129+
// Failure mode: an empty diff (e.g. a PR whose changed-file set resolved to nothing) builds the
130+
// graph and/or throws instead of cheaply returning "nothing affected". It must short-circuit to an
131+
// empty set so the selector falls through to Layer 2 alone.
132+
[Fact]
133+
public void EmptyChangeSetSelectsNothing()
134+
{
135+
using var repo = new GraphFixture();
136+
137+
var affected = repo.Compute();
138+
139+
Assert.Empty(affected);
140+
}
141+
142+
// Failure mode: a deleted/unmodeled file under a project that is itself nested inside another
143+
// project's directory is attributed to the OUTER (parent-dir) project, over-selecting the parent's
144+
// dependents and missing the real owner. The longest-directory-first containment fallback must pick
145+
// the deepest (most specific) owning project. Nested is isolated, so only it should be affected.
146+
[Fact]
147+
public void DeletedFileInNestedProjectDirAttributedToDeepestProject()
148+
{
149+
using var repo = new GraphFixture();
150+
151+
// Ghost.cs never existed and sits under Core/Nested (a project nested below Core/).
152+
var affected = repo.Compute("Core/Nested/Ghost.cs");
153+
154+
Assert.Contains("Nested", affected);
155+
Assert.DoesNotContain("Core", affected);
156+
Assert.DoesNotContain("Mid", affected);
157+
Assert.DoesNotContain("AppTests", affected);
158+
}
159+
160+
// Failure mode: a cross-project rename (git -M reports one record "R<sim> <old> <new>") is parsed
161+
// as only one path, so the project that LOST the file (old side) is not marked changed and its
162+
// dependents' tests are silently skipped. data.txt is not a declared item, so neither side touches
163+
// a .csproj — the attribution comes purely from parsing BOTH paths of the rename record plus the
164+
// directory-containment fallback. Exercises the git diff path (GetChangedPathsFromGit), which the
165+
// changed-files fixtures never reach.
166+
[Fact]
167+
public void CrossProjectRenameAttributesBothOldAndNewOwners()
168+
{
169+
using var repo = new GitGraphFixture();
170+
171+
var affected = repo.RenameAcrossProjectsAndCompute("Core/data.txt", "Other/data.txt");
172+
173+
// Old owner (Core) and its dependents, plus the new owner (Other).
174+
Assert.Contains("Core", affected);
175+
Assert.Contains("Mid", affected);
176+
Assert.Contains("AppTests", affected);
177+
Assert.Contains("Other", affected);
178+
}
179+
110180
/// <summary>
111181
/// Creates a disposable temp directory containing a minimal but real MSBuild project graph plus an
112182
/// <c>Aspire.slnx</c>, and runs <see cref="GraphAffectedProjects.Compute"/> against it using a
@@ -141,13 +211,19 @@ public GraphFixture()
141211
Write("Other/Other.cs", "namespace Other; public class O { }");
142212
WriteProject("Other/Other.csproj", compiles: ["Other.cs", @"..\Shared\Linked.cs"], references: []);
143213

214+
// Nested: an isolated project that lives UNDER Core's directory, so the containment
215+
// fallback must prefer it over Core for files under Core/Nested.
216+
Write("Core/Nested/Nested.cs", "namespace Nested; public class N { }");
217+
WriteProject("Core/Nested/Nested.csproj", compiles: ["Nested.cs"], references: []);
218+
144219
Write("Aspire.slnx",
145220
"""
146221
<Solution>
147222
<Project Path="Core/Core.csproj" />
148223
<Project Path="Mid/Mid.csproj" />
149224
<Project Path="AppTests/AppTests.csproj" />
150225
<Project Path="Other/Other.csproj" />
226+
<Project Path="Core/Nested/Nested.csproj" />
151227
</Solution>
152228
""");
153229
}
@@ -190,4 +266,128 @@ private void WriteProject(string relativePath, string[] compiles, string[] refer
190266

191267
public void Dispose() => _temp.Dispose();
192268
}
269+
270+
/// <summary>
271+
/// A real git repo containing the same minimal project graph, used to exercise the git diff path
272+
/// (<c>git diff --name-status -M</c>) — including rename records — that the changed-files fixture
273+
/// cannot reach.
274+
/// </summary>
275+
private sealed class GitGraphFixture : IDisposable
276+
{
277+
private readonly TestTempDirectory _temp = new();
278+
279+
public GitGraphFixture()
280+
{
281+
Write("Directory.Build.props", "<Project />");
282+
Write("Directory.Build.targets", "<Project />");
283+
284+
Write("Shared/Linked.cs", "namespace Shared; public static class Linked { }");
285+
286+
Write("Core/Core.cs", "namespace Core; public class C { }");
287+
WriteProject("Core/Core.csproj", compiles: ["Core.cs", @"..\Shared\Linked.cs"], references: []);
288+
289+
Write("Mid/Mid.cs", "namespace Mid; public class M { }");
290+
WriteProject("Mid/Mid.csproj", compiles: ["Mid.cs"], references: [@"..\Core\Core.csproj"]);
291+
292+
Write("AppTests/AppTests.cs", "namespace AppTests; public class T { }");
293+
WriteProject("AppTests/AppTests.csproj", compiles: ["AppTests.cs"], references: [@"..\Mid\Mid.csproj"]);
294+
295+
Write("Other/Other.cs", "namespace Other; public class O { }");
296+
WriteProject("Other/Other.csproj", compiles: ["Other.cs"], references: []);
297+
298+
// A loose, non-declared file (not a <Compile>/<Content> item). It is attributed purely by
299+
// directory containment, so renaming it touches no .csproj — isolating the rename-record
300+
// parse from any project-file edit.
301+
Write("Core/data.txt", "payload");
302+
303+
Write("Aspire.slnx",
304+
"""
305+
<Solution>
306+
<Project Path="Core/Core.csproj" />
307+
<Project Path="Mid/Mid.csproj" />
308+
<Project Path="AppTests/AppTests.csproj" />
309+
<Project Path="Other/Other.csproj" />
310+
</Solution>
311+
""");
312+
313+
Git("init", "-q", "-b", "main");
314+
Git("config", "user.email", "test@example.com");
315+
Git("config", "user.name", "Test");
316+
Git("config", "commit.gpgsign", "false");
317+
Git("add", "-A");
318+
Git("commit", "-q", "-m", "base");
319+
}
320+
321+
/// <summary>
322+
/// Renames <paramref name="oldRelativePath"/> to <paramref name="newRelativePath"/> in a new
323+
/// commit, then computes the affected set for <c>base..HEAD</c> via the git diff path.
324+
/// </summary>
325+
public IReadOnlyCollection<string> RenameAcrossProjectsAndCompute(string oldRelativePath, string newRelativePath)
326+
{
327+
var baseSha = Git("rev-parse", "HEAD");
328+
329+
var newFull = System.IO.Path.Combine(_temp.Path, newRelativePath.Replace('/', System.IO.Path.DirectorySeparatorChar));
330+
Directory.CreateDirectory(System.IO.Path.GetDirectoryName(newFull)!);
331+
Git("mv", oldRelativePath, newRelativePath);
332+
Git("commit", "-q", "-m", "rename across projects");
333+
334+
return GraphAffectedProjects.Compute(_temp.Path, from: baseSha, to: "HEAD", changedFilesPath: null);
335+
}
336+
337+
private void Write(string relativePath, string contents)
338+
{
339+
var fullPath = System.IO.Path.Combine(_temp.Path, relativePath.Replace('\\', System.IO.Path.DirectorySeparatorChar));
340+
Directory.CreateDirectory(System.IO.Path.GetDirectoryName(fullPath)!);
341+
File.WriteAllText(fullPath, contents);
342+
}
343+
344+
private void WriteProject(string relativePath, string[] compiles, string[] references)
345+
{
346+
var items = string.Join(Environment.NewLine,
347+
compiles.Select(c => $""" <Compile Include="{c}" Link="{System.IO.Path.GetFileName(c)}" />""")
348+
.Concat(references.Select(r => $""" <ProjectReference Include="{r}" />""")));
349+
350+
Write(relativePath,
351+
$"""
352+
<Project Sdk="Microsoft.NET.Sdk">
353+
<PropertyGroup>
354+
<TargetFramework>net10.0</TargetFramework>
355+
<EnableDefaultCompileItems>false</EnableDefaultCompileItems>
356+
</PropertyGroup>
357+
<ItemGroup>
358+
{items}
359+
</ItemGroup>
360+
</Project>
361+
""");
362+
}
363+
364+
private string Git(params string[] args)
365+
{
366+
var psi = new ProcessStartInfo("git")
367+
{
368+
WorkingDirectory = _temp.Path,
369+
RedirectStandardOutput = true,
370+
RedirectStandardError = true,
371+
UseShellExecute = false,
372+
};
373+
psi.ArgumentList.Add("--no-pager");
374+
foreach (var arg in args)
375+
{
376+
psi.ArgumentList.Add(arg);
377+
}
378+
379+
using var process = Process.Start(psi) ?? throw new InvalidOperationException("Failed to start git.");
380+
var stdout = process.StandardOutput.ReadToEnd();
381+
var stderr = process.StandardError.ReadToEnd();
382+
process.WaitForExit();
383+
if (process.ExitCode != 0)
384+
{
385+
throw new InvalidOperationException($"git {string.Join(' ', args)} failed ({process.ExitCode}): {stderr}");
386+
}
387+
388+
return stdout.Trim();
389+
}
390+
391+
public void Dispose() => _temp.Dispose();
392+
}
193393
}

tests/Infrastructure.Tests/TestTriggerMap/SelectTestsAcceptanceTests.cs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -601,6 +601,21 @@ public void UnmatchedAffectedProjectAddsNothing()
601601
Assert.Empty(r.Jobs);
602602
}
603603

604+
// Layer 1 reports the full affected set, which can include tests/ projects that are NOT in the
605+
// runnable matrix (shared fixtures/helpers like a TestFixtures or testproject project). Those names
606+
// are intersected with the matrix before selection, so they must never be selected as a test (only
607+
// an affected_project_rule may reference such a name by name). Failure mode: a non-runnable project
608+
// leaking into the matrix.
609+
[Fact]
610+
public void Layer1AffectedNonMatrixTestNameIsNotSelected()
611+
{
612+
var r = Select([], layer1: ["TestFixtures.Shared", "testproject"]);
613+
614+
Assert.False(r.SelectsAll);
615+
Assert.Empty(r.TestProjects);
616+
Assert.Empty(r.Jobs);
617+
}
618+
604619
// --- H. Real-map invariant smoke (computed from the filesystem; no hardcoded names) ------
605620

606621
[Fact]

0 commit comments

Comments
 (0)