test(arch): add NetArchTest suite enforcing the dependency rule in VSA#52
Merged
Merged
Conversation
WalkthroughThis PR adds an architecture test project that uses NetArchTest.Rules (1.3.2) to assert Domain layers across services (Catalog, Order, Payment, Shipping) have no compiled dependencies on infrastructural namespaces or their service's Infrastructure/Features layers. ChangesArchitecture Dependency Testing
🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/NextAurora.ArchitectureTests/DependencyRuleTests.cs`:
- Around line 43-74: Add a companion test that asserts the Application and
Features namespaces don't reference Infrastructure: create a new theory (mirror
the existing Domain_depends_on_nothing_infrastructural) e.g.
Application_and_Features_do_not_depend_on_infrastructure with the same
InlineData anchors; build forbidden =
ForbiddenInDomain.Append($"{service}.Infrastructure").ToArray(); then use
Types.InAssembly(domainAnchor.Assembly).That().ResideInNamespace($"{service}.Application").Or().ResideInNamespace($"{service}.Features").ShouldNot().HaveDependencyOnAny(forbidden).GetResult()
and assert result.IsSuccessful.Should().BeTrue(...) with a failure message
listing result.FailingTypeNames so offenders are reported.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 019fbfad-3184-4e3e-b838-117d227f4935
📒 Files selected for processing (4)
Directory.Packages.propsNextAurora.slnxtests/NextAurora.ArchitectureTests/DependencyRuleTests.cstests/NextAurora.ArchitectureTests/NextAurora.ArchitectureTests.csproj
Comment on lines
+43
to
+74
| [Theory] | ||
| [InlineData(typeof(CatalogService.Domain.Product), "CatalogService")] | ||
| [InlineData(typeof(OrderService.Domain.Order), "OrderService")] | ||
| [InlineData(typeof(PaymentService.Domain.Payment), "PaymentService")] | ||
| [InlineData(typeof(ShippingService.Domain.Shipment), "ShippingService")] | ||
| public void Domain_depends_on_nothing_infrastructural(Type domainAnchor, string service) | ||
| { | ||
| // ARRANGE — the anchor type pins the service's assembly; we scope the rule to that | ||
| // service's Domain namespace. The forbidden set is the shared infrastructural concerns | ||
| // PLUS the service's own Infrastructure + Features namespaces — Domain is the inner | ||
| // circle, so a Domain→Infrastructure or Domain→Features reference inverts the dependency | ||
| // rule. Justified Domain dependencies (NextAurora.Contracts DTOs/events, BCL) are not | ||
| // in the list and stay allowed. | ||
| var forbidden = ForbiddenInDomain | ||
| .Append($"{service}.Infrastructure") | ||
| .Append($"{service}.Features") | ||
| .ToArray(); | ||
|
|
||
| // ACT — assert no type in {service}.Domain has a compiled dependency on any forbidden | ||
| // namespace. This is the same boundary a 4-project split enforces via project | ||
| // references, expressed as a CI-gated test in a single-project VSA codebase. | ||
| var result = Types.InAssembly(domainAnchor.Assembly) | ||
| .That().ResideInNamespace($"{service}.Domain") | ||
| .ShouldNot().HaveDependencyOnAny(forbidden) | ||
| .GetResult(); | ||
|
|
||
| // ASSERT — green codifies the decoupled state; a failure names the offending Domain | ||
| // types so the inversion is obvious in CI output. | ||
| result.IsSuccessful.Should().BeTrue( | ||
| because: $"{service}.Domain must stay free of infrastructure — offending types: " + | ||
| string.Join(", ", result.FailingTypeNames ?? [])); | ||
| } |
There was a problem hiding this comment.
Missing architecture check for Application/Features → Infrastructure coupling.
Line [65] scopes enforcement to {service}.Domain only. The suite still needs a companion rule asserting the Application/Features layer does not reference {service}.Infrastructure.
➕ Suggested follow-up test
+ [Theory]
+ [InlineData(typeof(CatalogService.Domain.Product), "CatalogService")]
+ [InlineData(typeof(OrderService.Domain.Order), "OrderService")]
+ [InlineData(typeof(PaymentService.Domain.Payment), "PaymentService")]
+ [InlineData(typeof(ShippingService.Domain.Shipment), "ShippingService")]
+ public void Application_does_not_depend_on_its_own_infrastructure(Type serviceAnchor, string service)
+ {
+ // ARRANGE — in this repo’s VSA shape, Features acts as the application layer.
+ var forbidden = new[] { $"{service}.Infrastructure" };
+
+ // ACT — enforce Application/Features → Infrastructure is forbidden.
+ var result = Types.InAssembly(serviceAnchor.Assembly)
+ .That().ResideInNamespace($"{service}.Features")
+ .ShouldNot().HaveDependencyOnAny(forbidden)
+ .GetResult();
+
+ // ASSERT — failing type names make the inversion obvious in CI output.
+ result.IsSuccessful.Should().BeTrue(
+ because: $"{service}.Features must not depend on {service}.Infrastructure — offending types: " +
+ string.Join(", ", result.FailingTypeNames ?? []));
+ }As per coding guidelines CLAUDE.md referenced content requires enforcing architectural invariants that “Domain → nothing” and “Domain and Application layers must never reference Infrastructure.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/NextAurora.ArchitectureTests/DependencyRuleTests.cs` around lines 43 -
74, Add a companion test that asserts the Application and Features namespaces
don't reference Infrastructure: create a new theory (mirror the existing
Domain_depends_on_nothing_infrastructural) e.g.
Application_and_Features_do_not_depend_on_infrastructure with the same
InlineData anchors; build forbidden =
ForbiddenInDomain.Append($"{service}.Infrastructure").ToArray(); then use
Types.InAssembly(domainAnchor.Assembly).That().ResideInNamespace($"{service}.Application").Or().ResideInNamespace($"{service}.Features").ShouldNot().HaveDependencyOnAny(forbidden).GetResult()
and assert result.IsSuccessful.Should().BeTrue(...) with a failure message
listing result.FailingTypeNames so offenders are reported.
This was referenced May 30, 2026
Owner
Author
|
@coderabbitai — declining the Application/Features → Infrastructure suggestion. The proposed test would fail on the current codebase because
Every Feature handler in the repo imports The Domain layer IS supposed to know nothing about infrastructure — that's what this PR enforces deterministically via NetArchTest. The application layer's coupling to data infrastructure is part of the simplicity-refactor's deliberate choice (the previous Clean Architecture attempt in CatalogService was retired specifically because the layer split wasn't earning its keep at this scale). See:
The arch-test rung correctly covers Domain → no infrastructure; that's the rule that genuinely applies here. |
|
Tip For best results, initiate chat on the files or code changes.
🐇 (noted and filed away) ✏️ Learnings added
🧠 Learnings used |
First implementation of the "architecture tests" rung from the convention → architecture tests → project split enforcement spectrum (CLAUDE.md "Promotion signal" + docs/vsa-vs-clean-architecture.md). Proves the thesis those docs argue: Clean's dependency rule can be enforced deterministically in NextAurora's single-project VSA shape WITHOUT the 4-project split — at the namespace level, failing CI on violation. What landed: - New test project tests/NextAurora.ArchitectureTests (NetArchTest.Rules 1.3.2, added to Directory.Packages.props; registered in NextAurora.slnx). References all five services so the tests can inspect their assemblies. - DependencyRuleTests: a theory asserting each service's Domain namespace (Catalog/Order/Payment/Shipping) has no compiled dependency on EF Core, ASP.NET, Wolverine, Dapper, Npgsql, Microsoft.Data.SqlClient, Microsoft.Extensions.Caching, or its own Infrastructure/Features. That's the same boundary a 4-project layout enforces via project references — expressed as a CI-gated test in one project. - NotificationService intentionally excluded: it's the "no aggregate" service with no Domain folder, so there's no domain boundary to assert. 4/4 green — codifies the already-decoupled state (the Domain folders import only NextAurora.Contracts DTOs + BCL; the EF/Wolverine/Infrastructure mentions in Domain are all in XML-doc comments, which NetArchTest ignores since it inspects compiled IL). This moves the dependency rule from convention + architecture-reviewer + CodeRabbit (review-enforced) to *also* test-enforced (deterministic). Per docs/vsa-vs-clean-architecture.md, review still covers the judgment calls a static test can't express (e.g. the no-repository-wrapper ban) — the layers are complementary. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
8cba0d4 to
20fca93
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/NextAurora.ArchitectureTests/DependencyRuleTests.cs`:
- Line 48: Rename the test method Domain_depends_on_nothing_infrastructural to
follow the MethodName_Condition_ExpectedResult() convention (for example
DomainNamespace_WhenInspected_HasNoInfrastructureDependency) so it matches the
repository's unit-test naming pattern; update the method declaration in the
DependencyRuleTests class and any references or test attributes that refer to
Domain_depends_on_nothing_infrastructural to use the new name.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 72b28ad5-8869-47ba-b241-65891ed8cab6
📒 Files selected for processing (4)
Directory.Packages.propsNextAurora.slnxtests/NextAurora.ArchitectureTests/DependencyRuleTests.cstests/NextAurora.ArchitectureTests/NextAurora.ArchitectureTests.csproj
| [InlineData(typeof(OrderService.Domain.Order), "OrderService")] | ||
| [InlineData(typeof(PaymentService.Domain.Payment), "PaymentService")] | ||
| [InlineData(typeof(ShippingService.Domain.Shipment), "ShippingService")] | ||
| public void Domain_depends_on_nothing_infrastructural(Type domainAnchor, string service) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Test name deviates from the repo's MethodName_Condition_ExpectedResult() convention.
Domain_depends_on_nothing_infrastructural is descriptive BDD-style but doesn't follow the mandated unit-test naming pattern. Consider e.g. DomainNamespace_WhenInspected_HasNoInfrastructureDependency for consistency with the rest of the suite.
As per coding guidelines: "Use MethodName_Condition_ExpectedResult() naming pattern for all unit tests".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/NextAurora.ArchitectureTests/DependencyRuleTests.cs` at line 48, Rename
the test method Domain_depends_on_nothing_infrastructural to follow the
MethodName_Condition_ExpectedResult() convention (for example
DomainNamespace_WhenInspected_HasNoInfrastructureDependency) so it matches the
repository's unit-test naming pattern; update the method declaration in the
DependencyRuleTests class and any references or test attributes that refer to
Domain_depends_on_nothing_infrastructural to use the new name.
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
1 participant
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.
Summary
First implementation of the "architecture tests" rung from the
convention → architecture tests → project splitenforcement spectrum (CLAUDE.md "Promotion signal" +docs/vsa-vs-clean-architecture.md). It proves the thesis those docs argue: Clean's dependency rule can be enforced deterministically in NextAurora's single-project VSA shape without the 4-project split — at the namespace level, failing CI on violation.What landed
tests/NextAurora.ArchitectureTests(NetArchTest.Rules 1.3.2, added toDirectory.Packages.props; registered inNextAurora.slnx). References all five services so the tests can inspect their assemblies.DependencyRuleTests— a theory asserting each service'sDomainnamespace (Catalog/Order/Payment/Shipping) has no compiled dependency on EF Core, ASP.NET, Wolverine, Dapper, Npgsql,Microsoft.Data.SqlClient,Microsoft.Extensions.Caching, or its ownInfrastructure/Features. Same boundary a 4-project layout enforces via project references — as a CI-gated test in one project.Domainfolder), so there's no domain boundary to assert.Result
4/4 green. Codifies the already-decoupled state — the
Domainfolders import onlyNextAurora.ContractsDTOs + BCL; the EF/Wolverine/Infrastructure mentions in Domain are all in XML-doc comments, which NetArchTest ignores (it inspects compiled IL, not text).Why it matters
Moves the dependency rule from convention + architecture-reviewer + CodeRabbit (review-enforced) to also test-enforced (deterministic). Per
docs/vsa-vs-clean-architecture.md, review still covers the judgment calls a static test can't express — e.g. the no-repository-wrapper ban — so the layers are complementary, not redundant.Scope note
Deliberately focused on the Domain-isolation rule (the core dependency-rule assertion). Possible follow-ups, not in this PR: cross-service boundary checks (no service references another's assembly) and a Features→Domain direction assertion. Kept tight + guaranteed-green for the first arch-test PR.
Test plan
dotnet teston the arch-test project — 4/4 passdotnet buildclean (1 pre-existing benchmark warning, unrelated)NextAurora.slnx)🤖 Generated with Claude Code
Summary by CodeRabbit
Tests
Chores