sprint(8): merge Sprint 8 — xUnit v3 Pilot (Architecture.Tests) (#9) - #188
sprint(8): merge Sprint 8 — xUnit v3 Pilot (Architecture.Tests) (#9)#188mpaulosky wants to merge 5 commits into
Conversation
## Summary Closes #176 Working as **Boromir** (DevOps Engineer) Mirrors the Sprint 7 Domain.Tests xUnit v3 pilot by upgrading Architecture.Tests from xunit 2.x to xunit.v3 3.2.2. ## Changes - **`tests/Architecture.Tests/Architecture.Tests.csproj`** - Replaced `<PackageReference Include="xunit"/>` with `<PackageReference Include="xunit.v3"/>` (version 3.2.2 centralized in `Directory.Packages.props`) - Added `<PackageReference Include="xunit.analyzers"/>` (version 1.27.0 centralized) - Added `xunit.runner.json` as `Content` item (matching Domain.Tests pattern) - **`tests/Architecture.Tests/xunit.runner.json`** — new file matching Domain.Tests Sprint 7 configuration ## Sprint 7 Pilot Context `Directory.Packages.props` already contained `xunit.v3 3.2.2`, `xunit.v3.assert`, `xunit.v3.extensibility.core`, and `xunit.analyzers 1.27.0` from the Sprint 7 Domain.Tests pilot. This PR wires up Architecture.Tests to use those centralized entries. ## NuGet Policy Compliance ✅ All versions remain in `Directory.Packages.props` — no version pins in `.csproj` ## Local Validation - `dotnet build`: ✅ 0 errors - `dotnet test tests/Architecture.Tests`: ✅ 11/11 passed - `dotnet test tests/Domain.Tests`: ✅ 42/42 passed (pilot unaffected) Co-authored-by: Boromir <boromir@squad.dev> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
## Summary Closes #177 Working as **Boromir** (DevOps Engineer) Validates CI passes cleanly with xUnit v3 packages in Architecture.Tests (from #176) and Domain.Tests (Sprint 7 pilot). ## CI Validation Findings ### Local Build & Test Results | Step | Result | Notes | |------|--------|-------| | `dotnet restore` | ✅ PASS | All packages resolved | | `dotnet build --configuration Release` | ✅ PASS | 0 errors, 306 pre-existing warnings (CA2007/CA1707 in integration tests — not xunit-related, pre-existing) | | `dotnet test tests/Architecture.Tests` | ✅ PASS | 11/11 tests passed | | `dotnet test tests/Domain.Tests` | ✅ PASS | 42/42 tests passed | ### Workflow Assessment | Workflow | Behaviour | Status | |----------|-----------|--------| | `squad-ci.yml` | Build-only gate on PR to dev/main | ✅ No changes needed | | `squad-preview.yml` | Full test suite on push to dev |⚠️ Domain.Tests was missing — **added** | ### Workflow Change **`squad-preview.yml`** — Added `dotnet test tests/Domain.Tests` to the test matrix. The Sprint 7 Domain.Tests pilot was missing from CI gate despite being the reference implementation. Architecture.Tests was already present and now validates with xunit.v3. ## No Adjustments Needed xUnit v3 3.2.2 works drop-in with `Microsoft.NET.Test.Sdk 18.5.0` and `xunit.runner.visualstudio 3.1.5` (already centralized in `Directory.Packages.props`). No runner configuration changes required beyond the `xunit.runner.json` added in #176.⚠️ This task was flagged as "needs review" — please have a squad member review before merging (CI workflow changes affect all contributors). --------- Co-authored-by: Boromir <boromir@squad.dev> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
## 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: | File | Tests | Change | |------|-------|--------| | `DomainLayerTests.cs` | 4 | AAA comments + extract `assembly` variable for clean Arrange/Act split | | `VsaLayerTests.cs` | 3 | AAA comments + extract `domainAssembly` variable | | `ThemeLayerTests.cs` | 2 | AAA comments | | `CachingLayerTests.cs` | 2 | AAA comments | **Total: 11 tests migrated** ## xUnit v3 API Observations - `[Fact]` attribute is **identical** in xUnit v2 and v3 — no attribute changes required - No async/await patterns in architecture tests — no lifecycle changes needed - No `IClassFixture` / `ICollectionFixture` — stateless tests, no collection changes needed - FluentAssertions `.Should()` works unchanged with xUnit v3 - Parallelization via `xunit.runner.json` was already configured in Wave 1 (#182) ## Pattern Reference (Sprint 7 Domain.Tests Pilot) Followed the same approach as the Sprint 7 Domain.Tests migration: - xUnit v3 meta-package via `<PackageReference Include="xunit.v3"/>` ✅ (Wave 1) - `xunit.runner.json` with `parallelizeAssembly: true` ✅ (Wave 1) - Gimli Rule #3: AAA comments on all test methods ✅ (this PR) - Gimli Rule #6: File headers present on all files ✅ (pre-existing) ## Test Results ``` Passed! - Failed: 0, Passed: 11, Skipped: 0, Total: 11 — Architecture.Tests.dll Passed! - Failed: 0, Passed: 42, Skipped: 0, Total: 42 — Domain.Tests.dll ``` Zero failures. Ready to merge to `sprint/8-xunit-v3-pilot`. Co-authored-by: Boromir <boromir@squad.dev> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
## 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>
…181) - Creates docs/sprint-8-xunit-v3-pilot-retro.md with full retrospective - Covers Wave 1-3 delivery, what went well, improvements, learnings - Documents 11 arch tests + 42 domain tests passing (53+ combined) - Records key decisions from Boromir and Gimli inbox entries - Includes xUnit v3 rollout recommendations for Sprint 9+ - Updates Aragorn history.md with Sprint 8 learnings Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🏗️ PR Added to Squad Triage QueueThis PR has been labeled with Next steps:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #188 +/- ##
=======================================
Coverage 72.53% 72.53%
=======================================
Files 43 43
Lines 721 721
Branches 112 112
=======================================
Hits 523 523
Misses 149 149
Partials 49 49 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Closes out Sprint 8 by finalizing the xUnit v3 pilot migration for Architecture.Tests, updating CI gating to include Domain.Tests, and adding sprint documentation/learnings artifacts.
Changes:
- Migrate
tests/Architecture.Teststo xUnit v3 packages/config (includingxunit.runner.json) and apply small test readability refactors (AAA comments/variable extraction). - Update
squad-preview.ymlto runDomain.Testsin the dev-branch preview gate alongside existing test projects. - Add Sprint 8 retrospective doc and append Sprint 8 learnings to Squad agent history logs.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Architecture.Tests/Architecture.Tests.csproj | Switches Architecture.Tests to xUnit v3 (+ analyzers) and copies xunit.runner.json to output. |
| tests/Architecture.Tests/xunit.runner.json | Adds runner configuration enabling parallelization and display options for xUnit v3. |
| tests/Architecture.Tests/VsaLayerTests.cs | Refactors tests for AAA-style comments and minor variable renaming/extraction. |
| tests/Architecture.Tests/ThemeLayerTests.cs | Adds AAA-style comments for consistency/readability. |
| tests/Architecture.Tests/DomainLayerTests.cs | Extracts assembly variables and adds AAA-style comments to clarify intent. |
| tests/Architecture.Tests/CachingLayerTests.cs | Adds AAA-style comments and keeps existing architectural assertions. |
| .github/workflows/squad-preview.yml | Adds dotnet test tests/Domain.Tests to the preview validation gate. |
| docs/sprint-8-xunit-v3-pilot-retro.md | Adds Sprint 8 retrospective document for the Architecture.Tests xUnit v3 pilot. |
| .squad/agents/gimli/history.md | Appends Sprint 8 Wave 2 migration notes/learnings. |
| .squad/agents/boromir/history.md | Appends Sprint 7/8 packaging + CI learnings for xUnit v3 rollout. |
| .squad/agents/aragorn/history.md | Appends Sprint 7/8 rollout strategy and metrics learnings. |
| 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() | ||
| .ResideInNamespace("MyBlog.Web.Features.BlogPosts") | ||
| .ShouldNot() | ||
| .HaveDependencyOnAny("MyBlog.Web.Features.UserManagement") | ||
| .GetResult(); |
There was a problem hiding this comment.
Features_Should_Not_Reference_Each_Other (and the comment "and vice versa") implies enforcing both directions, but the test only checks that MyBlog.Web.Features.BlogPosts does not depend on ...UserManagement. Either add the reciprocal assertion (UserManagement should not depend on BlogPosts) or rename/update the test/comment to match the single-direction rule being enforced.
| ``` | ||
|
|
||
| Versions are centralized in `Directory.Packages.props` (3.2.2, 1.27.0, 3.1.1 respectively). | ||
| No individual `.csproj` version pins. See `.squad/decisions/inbox/boromir-xunit-v3-package-strategy.md`. |
There was a problem hiding this comment.
This retrospective references decision files under .squad/decisions/inbox/..., but that directory is gitignored, so readers of the repo won't be able to find these documents. Consider linking to the canonical .squad/decisions/decisions.md entries instead, or include the referenced decision docs in a tracked location.
| No individual `.csproj` version pins. See `.squad/decisions/inbox/boromir-xunit-v3-package-strategy.md`. | |
| No individual `.csproj` version pins. See the relevant xUnit v3 package strategy | |
| entry in [`.squad/decisions/decisions.md`](../.squad/decisions/decisions.md). |
| | Issue | Wave | Owner | Status | Description | | ||
| |---|---|---|---|---| | ||
| | #176 | 1a | Boromir | ✅ Done | Add xUnit v3 packages to Architecture.Tests | | ||
| | #177 | 1b | Boromir | ✅ Done | CI validation for xUnit v3 Architecture.Tests | | ||
| | #178 | 2a | Gimli | ✅ Done | Migrate Architecture.Tests to xUnit v3 | | ||
| | #179 | 2b | Gimli | ✅ Done | Validate — no test failures after migration | | ||
| | #180 | 3a | Pippin | 🔄 In Progress | ADR documentation | | ||
| | #181 | 3b | Aragorn | 🔄 In Progress | Sprint retrospective (this document) | |
There was a problem hiding this comment.
In the sprint delivery summary table, issues #180 and #181 are marked as "In Progress", but this PR description states the ADR and retrospective deliverables are complete and this document itself represents #181. Update the status values (and/or accompanying text) so the retrospective doesn't contradict the sprint-close PR summary.
|
|
||
| The three-wave dependency model performed exactly as intended: | ||
|
|
||
| ``` |
There was a problem hiding this comment.
The wave dependency chain code block uses an unlabeled fenced block. If the markdown validators require specifying a language for fenced code blocks, consider using a language hint like text for this block.
| ``` | |
| ```text |
| ### Learnings | ||
| 1. For MyBlog test coverage, a `ConfigureAwait(false)` cleanup in a MediatR pipeline behavior is non-observable unless tests explicitly assert synchronization-context behavior. | ||
| 2. Scope discipline matters here: do not churn nearby tests for unrelated convention gaps when the requested review is only about async continuation configuration. No newline at end of file | ||
| 2. Scope discipline matters here: do not churn nearby tests for unrelated convention gaps when the requested review is only about async continuation configuration. |
There was a problem hiding this comment.
Missing a blank line between the numbered list and the following ## heading. As written, many Markdown renderers treat the heading as part of list item 2, which breaks document structure.
| 2. Scope discipline matters here: do not churn nearby tests for unrelated convention gaps when the requested review is only about async continuation configuration. | |
| 2. Scope discipline matters here: do not churn nearby tests for unrelated convention gaps when the requested review is only about async continuation configuration. |
|
Sprint 8 already merged to dev via alternate merge path. All 6 issues resolved, all PRs (#182-187) integrated successfully. |
Pull request was closed
…emote branch prune (#237) ## Summary Working as Ralph (Meta) Closes #236 ## Changes - **Merged PR #235** — squash merged with all 23 CI checks passing - **Pruned 6 stale remote branches** (Sprint 6–8 orphans, all associated issues/PRs closed since April 2026): - `origin/squad/140-domain-servicedefaults-ca-warnings` (PR #156) - `origin/squad/153-web-infrastructure-warnings` (PR #157) - `origin/squad/154-webtests-bunit-warnings` (PR #158) - `origin/squad/155-test-assembly-ca1014` (PR #159) - `origin/squad/164-domain-tests-xunit-v3-fixes` (PR #171) - `origin/sprint/8-xunit-v3-pilot` (PR #188) - **Updated `identity/now.md`**: Sprint 16 ready, remote hygiene milestone noted - **Updated `ralph/history.md`**: 2026-05-06 follow-up session log appended ## Verification - Pre-commit gate: ✅ 0 markdownlint errors - Pre-push gate (build + tests + integration): ✅ all green Co-authored-by: Boromir <boromir@squad.dev> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sprint Close
This PR closes Sprint 8 — xUnit v3 Pilot (Architecture.Tests) milestone.
Summary
Deliverables
✅ xUnit v3 packages integrated into Architecture.Tests
✅ All 11 Architecture.Tests tests migrated and passing
✅ CI validation complete with bonus Domain.Tests gate fix
✅ ADR documentation for Architecture.Tests rollout
✅ Sprint 8 retrospective with Wave model analysis
Wave Model Results
Metrics
Next
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com