From ad8c4d4b9c51a17be83295b5536ef8e88996882e Mon Sep 17 00:00:00 2001 From: Brandon Date: Tue, 15 Sep 2026 15:17:44 -0400 Subject: [PATCH] Make ArchitectureTests read the resolved dependency graph They called GetReferencedAssemblies(), which omits references the code never uses, so they passed vacuously (#54). They now read the test run's .deps.json, which records every project's declared dependencies. Verified by injecting a ProjectReference from RulesKernel to RulesKernel.Analyzers and one from RegulatoryProbe to RulesKernel.Randomness: all three affected tests fail. Closes #54 Co-Authored-By: Claude Opus 5 --- docs/architecture.md | 11 ++-- probes/RegulatoryProbe.Tests/ProbeTests.cs | 8 +++ .../ArchitectureTests.cs | 50 +++++++++++---- tests/RulesKernel.Tests/ArchitectureTests.cs | 64 ++++++++++++++----- tools/repo-checks.py | 8 +-- 5 files changed, 104 insertions(+), 37 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index ca7612e..729b5a6 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -28,11 +28,12 @@ declared `ProjectReference` graph out of the csproj files, and it fails on any p present on disk but absent from the declared graph — an undeclared project must not silently escape enforcement. -The per-assembly `ArchitectureTests` are a second net and are **not yet load-bearing**: the -C# compiler omits assembly references a compilation does not actually use, so while no -assembly consumes another, `GetReferencedAssemblies()` returns nothing and those assertions -pass vacuously. They begin catching real violations as soon as code crosses an assembly -boundary. Until then the declared-graph check is the enforcement. +The per-assembly `ArchitectureTests` are a second net, read from the build rather than from +text. They read the `.deps.json` the SDK writes beside each test assembly, which records every +project in the resolved graph and what it depends on, whether or not the code uses it yet. They +used to call `GetReferencedAssemblies()` instead. That passed vacuously, because the compiler +drops references the code never uses (#54). A `ProjectReference` from `RulesKernel` to a +sibling fails them today, before any code calls across it. `RulesKernel.Testing` lives under `tests/` because it is test-support, not a layer of an engine. It is packaged anyway, because an engine built on this kernel needs the same diff --git a/probes/RegulatoryProbe.Tests/ProbeTests.cs b/probes/RegulatoryProbe.Tests/ProbeTests.cs index 236e8f5..78c0cd3 100644 --- a/probes/RegulatoryProbe.Tests/ProbeTests.cs +++ b/probes/RegulatoryProbe.Tests/ProbeTests.cs @@ -59,6 +59,14 @@ public void This_assembly_references_the_kernel_and_never_the_randomness_package Assert.Contains("RulesKernel", referenced); Assert.DoesNotContain("RulesKernel.Randomness", referenced); + + // The compiler drops a reference the code never uses, so the line above cannot see a + // declared dependency nothing has called yet (issue #54). The build's dependency graph + // can: it lists every package and project this test run resolved. + string depsJson = System.IO.File.ReadAllText(System.IO.Path.Combine( + AppContext.BaseDirectory, typeof(DeferralLimitEngine).Assembly.GetName().Name + ".deps.json")); + Assert.DoesNotContain("\"RulesKernel.Randomness/", depsJson, StringComparison.Ordinal); + Assert.Contains("\"RulesKernel/", depsJson, StringComparison.Ordinal); } // ------------------------------------------------------------------ the temporal axis diff --git a/tests/RulesKernel.Randomness.Tests/ArchitectureTests.cs b/tests/RulesKernel.Randomness.Tests/ArchitectureTests.cs index 125bf7d..d12da8e 100644 --- a/tests/RulesKernel.Randomness.Tests/ArchitectureTests.cs +++ b/tests/RulesKernel.Randomness.Tests/ArchitectureTests.cs @@ -1,26 +1,52 @@ using System; +using System.Collections.Generic; +using System.IO; using System.Linq; +using System.Text.Json; namespace RulesKernel.Randomness.Tests; /// -/// RulesKernel.Randomness sits directly on the kernel and nothing else. See the -/// note in the kernel's own architecture tests on why this direction only. +/// RulesKernel.Randomness sits directly on the kernel and nothing else, and +/// RulesKernel.Testing directly on RulesKernel.Randomness. Read from this test +/// run's .deps.json, which records declared project dependencies whether or not the code +/// uses them yet; see the kernel's own ArchitectureTests for why (issue #54). /// public sealed class ArchitectureTests { - private static readonly string[] Allowed = ["RulesKernel"]; + [Theory] + [InlineData("RulesKernel", new string[0])] + [InlineData("RulesKernel.Randomness", new[] { "RulesKernel" })] + [InlineData("RulesKernel.Testing", new[] { "RulesKernel.Randomness" })] + public void Each_layer_depends_only_on_the_layer_below(string project, string[] allowed) + { + var graph = OfThisTestRun(); + + Assert.True(graph.ContainsKey(project), $"{project} is not in this test run's dependency graph"); + Assert.Empty(graph[project].Except(allowed, StringComparer.Ordinal)); + } - [Fact] - public void Randomness_references_only_the_kernel() + private static Dictionary OfThisTestRun() { - string[] referenced = AssemblyMarker.Assembly - .GetReferencedAssemblies() - .Select(a => a.Name ?? string.Empty) - .Where(n => n.StartsWith("RulesKernel", StringComparison.Ordinal)) - .OrderBy(n => n, StringComparer.Ordinal) - .ToArray(); + string path = Path.Combine(AppContext.BaseDirectory, typeof(ArchitectureTests).Assembly.GetName().Name + ".deps.json"); + using var document = JsonDocument.Parse(File.ReadAllText(path)); + JsonElement target = document.RootElement.GetProperty("targets").EnumerateObject().Single().Value; + + var graph = new Dictionary(StringComparer.Ordinal); + foreach (JsonProperty library in target.EnumerateObject()) + { + string name = library.Name.Split('/')[0]; + if (name.StartsWith("RulesKernel", StringComparison.Ordinal)) + { + graph[name] = library.Value.TryGetProperty("dependencies", out JsonElement dependencies) + ? dependencies.EnumerateObject() + .Select(dependency => dependency.Name) + .Where(dependency => dependency.StartsWith("RulesKernel", StringComparison.Ordinal)) + .ToArray() + : []; + } + } - Assert.Empty(referenced.Except(Allowed, StringComparer.Ordinal)); + return graph; } } diff --git a/tests/RulesKernel.Tests/ArchitectureTests.cs b/tests/RulesKernel.Tests/ArchitectureTests.cs index b2476ae..8f125f4 100644 --- a/tests/RulesKernel.Tests/ArchitectureTests.cs +++ b/tests/RulesKernel.Tests/ArchitectureTests.cs @@ -1,33 +1,65 @@ using System; +using System.Collections.Generic; +using System.IO; using System.Linq; +using System.Text.Json; namespace RulesKernel.Tests; /// -/// Mechanical enforcement of the layering in docs/architecture.md. RulesKernel is -/// the floor: it may reference no other assembly in this repository at all. +/// Mechanical enforcement of the layering in docs/architecture.md, read from the build's own +/// dependency graph. RulesKernel is the floor: it may depend on no other assembly in +/// this repository at all. /// /// -/// This assertion is one-directional on purpose. The C# compiler omits assembly references -/// a compilation does not actually use, so "RulesKernel references X" cannot be asserted -/// until code here consumes X. What can always be asserted -- and what matters -- is that -/// nothing leaks upward. The declared ProjectReference graph is checked separately -/// and exactly by tools/repo-checks.py --only layering, which reads the csproj files -/// rather than compiled output, and that check is the load-bearing one. +/// These tests used to ask the compiled assembly for its referenced assemblies, and passed +/// vacuously: the C# compiler omits a reference the code does not use, so a +/// ProjectReference nobody had called yet was invisible (issue #54). The +/// .deps.json the SDK writes beside this test assembly records every project in the +/// graph and what each declares, used or not. This is a second net under +/// tools/repo-checks.py --only layering, not a copy of it: that check reads project and +/// build files as text, and this one reads what MSBuild actually resolved, so a reference +/// either one misreads the other still sees. /// /// public sealed class ArchitectureTests { [Fact] - public void Kernel_references_no_other_RulesKernel_assembly() + public void Kernel_depends_on_no_other_RulesKernel_project() { - string[] referenced = AssemblyMarker.Assembly - .GetReferencedAssemblies() - .Select(a => a.Name ?? string.Empty) - .Where(n => n.StartsWith("RulesKernel", StringComparison.Ordinal)) - .OrderBy(n => n, StringComparer.Ordinal) - .ToArray(); + var graph = ProjectGraph.OfThisTestRun(); - Assert.Empty(referenced); + Assert.True(graph.ContainsKey("RulesKernel"), "the kernel is not in this test run's dependency graph"); + Assert.Empty(graph["RulesKernel"]); + } +} + +/// The RulesKernel* projects in a test run's .deps.json, and what each depends on. +internal static class ProjectGraph +{ + public static IReadOnlyDictionary OfThisTestRun() + { + string path = Path.Combine(AppContext.BaseDirectory, typeof(ProjectGraph).Assembly.GetName().Name + ".deps.json"); + using var document = JsonDocument.Parse(File.ReadAllText(path)); + JsonElement target = document.RootElement.GetProperty("targets").EnumerateObject().Single().Value; + + var graph = new Dictionary(StringComparer.Ordinal); + foreach (JsonProperty library in target.EnumerateObject()) + { + string name = library.Name.Split('/')[0]; + if (!name.StartsWith("RulesKernel", StringComparison.Ordinal)) + { + continue; + } + + graph[name] = library.Value.TryGetProperty("dependencies", out JsonElement dependencies) + ? dependencies.EnumerateObject() + .Select(dependency => dependency.Name) + .Where(dependency => dependency.StartsWith("RulesKernel", StringComparison.Ordinal)) + .ToArray() + : []; + } + + return graph; } } diff --git a/tools/repo-checks.py b/tools/repo-checks.py index 4a40819..0cd7ef6 100755 --- a/tools/repo-checks.py +++ b/tools/repo-checks.py @@ -785,10 +785,10 @@ def _includes(pattern: re.Pattern[str], text: str) -> set[str]: def check_layering(root: Path) -> CheckResult: """The declared project-dependency graph must match the architecture, exactly. - This reads the csproj files rather than compiled output, which makes it exact: the C# - compiler omits references a compilation does not actually use, so the reflective - ArchitectureTests pass vacuously until real code crosses a boundary. This check bites - the moment a reference is written, including on an empty project. + This reads the csproj files rather than compiled output, so it bites the moment a + reference is written, including on an empty project. The ArchitectureTests read the + resolved graph from each test run's .deps.json and bite at the same moment; the two + disagree only when one of them misreads the build, which is the point of having both. Three edges count as a dependency, not one: * ProjectReference, in either quoting style;