Migrate Architecture.Tests to xUnit v3 (#178) - #184
Conversation
- Add AAA (Arrange/Act/Assert) comments to all 11 test methods - Extract assembly variable in DomainLayerTests for cleaner Arrange/Act split - No xUnit attribute changes needed — [Fact] is identical in v2 and v3 - Follows Sprint 7 Domain.Tests pilot pattern (FluentAssertions + xunit.v3) - All 11 architecture tests pass with xunit.v3 3.2.2 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🏗️ PR Added to Squad Triage QueueThis PR has been labeled with Next steps:
|
## Summary Post-migration validation for Architecture.Tests xUnit v3 migration. No failures found. Closes #179 **Working as Gimli (Tester)** --- ## Findings After applying the xUnit v3 migration in PR #184, all 11 architecture tests pass with zero failures: ``` Passed! - Failed: 0, Passed: 11, Skipped: 0, Total: 11 — Architecture.Tests.dll (72ms) ``` **No fixes required.** The Architecture.Tests migration in Wave 2 is clean. ## Root Cause Analysis xUnit v3 is fully backward-compatible for all patterns used in Architecture.Tests: - `[Fact]` — identical in v2 and v3 ✅ - FluentAssertions `.Should()` — unchanged ✅ - NetArchTest.Rules builder chain — unchanged ✅ - No `IClassFixture` / `ICollectionFixture` patterns in use ✅ - No xUnit lifecycle overrides (`IAsyncLifetime`, `SetupAttribute`) ✅ The only migration work was Gimli Rule #3 compliance (AAA comments), which is purely cosmetic and cannot introduce test failures. ## Deliverables - ✅ Validated: 11 tests pass with xUnit v3 3.2.2 - ✅ Appended Sprint 8 Wave 2 learnings to `.squad/agents/gimli/history.md` - ✅ Drafted `gimli-xunit-v3-migration-pattern.md` decision (in local decisions inbox for Scribe) ## Prerequisite This PR should merge after #184 is merged to `sprint/8-xunit-v3-pilot`. Co-authored-by: Boromir <boromir@squad.dev> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates Architecture.Tests test methods to follow the repo’s xUnit v3 migration conventions (matching the earlier Domain.Tests pilot) without changing the underlying architectural rules being asserted.
Changes:
- Added AAA (Arrange/Act/Assert) comments to all Architecture.Tests test methods.
- Extracted local
Assemblyvariables in domain-related tests to clarify Arrange vs Act separation. - Minor variable renames to align with consistent assertion style (
result.IsSuccessful).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/Architecture.Tests/DomainLayerTests.cs | Adds AAA structure and extracts assembly local for clearer Arrange/Act split. |
| tests/Architecture.Tests/VsaLayerTests.cs | Adds AAA comments, renames locals to result, extracts domainAssembly in one test. |
| tests/Architecture.Tests/ThemeLayerTests.cs | Adds AAA comments around NetArchTest rule execution. |
| tests/Architecture.Tests/CachingLayerTests.cs | Adds AAA comments around caching dependency rules and assertions. |
| public void Features_Should_Not_Reference_Each_Other() | ||
| { | ||
| // BlogPosts slice should not reference UserManagement and vice versa | ||
| var blogPostsResult = Types.InAssembly(WebAssembly) | ||
| // Arrange / Act — BlogPosts slice should not reference UserManagement and vice versa | ||
| var result = Types.InAssembly(WebAssembly) | ||
| .That() |
There was a problem hiding this comment.
Features_Should_Not_Reference_Each_Other (and the new comment text) implies a two-way dependency check, but the current rule only verifies that types in MyBlog.Web.Features.BlogPosts don’t depend on MyBlog.Web.Features.UserManagement. Either add a second assertion for the reverse direction (UserManagement → BlogPosts) or rename/adjust the test name and comment to match the one-way check.
| var result = Types.InAssembly(WebAssembly) | ||
| .That() | ||
| .ResideInNamespace("MyBlog.Web.Components.Theme") | ||
| .Should() | ||
| .ResideInNamespace("MyBlog.Web.Components.Theme") |
There was a problem hiding this comment.
This test is effectively tautological: it filters to types already in MyBlog.Web.Components.Theme and then asserts those types reside in the same namespace, so it can’t fail if the namespace exists. Consider selecting theme components by a different predicate (e.g., name starts with "Theme" / assignable to ComponentBase) and asserting they reside in MyBlog.Web.Components.Theme.
| // Arrange | ||
| var assembly = typeof(BlogPost).Assembly; | ||
|
|
||
| // Act | ||
| var result = Types.InAssembly(assembly) |
There was a problem hiding this comment.
typeof(BlogPost).Assembly is repeated in each test method; consider extracting a single private static readonly Assembly DomainAssembly = typeof(BlogPost).Assembly; (or similar) at the class level to reduce duplication and keep the Arrange sections focused on test-specific setup.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## sprint/8-xunit-v3-pilot #184 +/- ##
==========================================================
Coverage ? 72.53%
==========================================================
Files ? 43
Lines ? 721
Branches ? 112
==========================================================
Hits ? 523
Misses ? 149
Partials ? 49 🚀 New features to boost your workflow:
|
Summary
Migrates Architecture.Tests test methods to xUnit v3 API conventions, following the Sprint 7 Domain.Tests pilot pattern.
Closes #178
Working as Gimli (Tester)
What Changed
All 4 test files updated with proper xUnit v3 migration conventions:
DomainLayerTests.csassemblyvariable for clean Arrange/Act splitVsaLayerTests.csdomainAssemblyvariableThemeLayerTests.csCachingLayerTests.csTotal: 11 tests migrated
xUnit v3 API Observations
[Fact]attribute is identical in xUnit v2 and v3 — no attribute changes requiredIClassFixture/ICollectionFixture— stateless tests, no collection changes needed.Should()works unchanged with xUnit v3xunit.runner.jsonwas already configured in Wave 1 (Add xUnit v3 packages to Architecture.Tests (#176) #182)Pattern Reference (Sprint 7 Domain.Tests Pilot)
Followed the same approach as the Sprint 7 Domain.Tests migration:
<PackageReference Include="xunit.v3"/>✅ (Wave 1)xunit.runner.jsonwithparallelizeAssembly: true✅ (Wave 1)Test Results
Zero failures. Ready to merge to
sprint/8-xunit-v3-pilot.