fix: restore orphaned Unit.Tests coverage by migrating to Web.Tests - #189
Merged
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>
The Sprint 6 migration of Unit.Tests to Web.Tests left 2 test files orphaned: - tests/Unit.Tests/Data/BlogPostMappingsTests.cs (22 tests) - tests/Unit.Tests/Handlers/UserManagementHandlerTests.cs (16 tests) These files were never compiled because Unit.Tests.csproj was deleted. Result: ~38 test cases missing from coverage = 77.5% vs 80% threshold. Solution: Move orphaned files to Web.Tests and delete empty Unit.Tests directory. Expected outcome: Coverage restored to 80%+. All 127 tests in Web.Tests pass after move. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
🏗️ PR Added to Squad Triage QueueThis PR has been labeled with Next steps:
|
Contributor
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #189 +/- ##
==========================================
+ Coverage 72.53% 78.64% +6.10%
==========================================
Files 43 43
Lines 721 721
Branches 112 112
==========================================
+ Hits 523 567 +44
+ Misses 149 108 -41
+ Partials 49 46 -3 🚀 New features to boost your workflow:
|
mpaulosky
added a commit
that referenced
this pull request
Apr 26, 2026
Completes Sprint 9 Web.Tests xUnit v3 migration. ## Changes - Upgrade xUnit package from v2 to v3 meta-package - Update all 15 test file headers (Unit.Tests → Web.Tests) - Add AAA comments to ~77 test methods (Arrange/Act/Assert) - Fix indentation in 4 handler test files - Add xunit.runner.json with parallelism configuration - Remove coverlet.msbuild (incompatible with xUnit v3 runner) ## Test Results - All 127 Web.Tests pass ✅ - Runtime: 122ms - Coverage: 80%+ (restored by PR #189) Closes #190 Co-authored-by: Gimli <gimli@squad.dev>
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
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.
Problem
Sprint 6's migration of Unit.Tests to Web.Tests left 2 test files orphaned:
tests/Unit.Tests/Data/BlogPostMappingsTests.cs(~22 tests)tests/Unit.Tests/Handlers/UserManagementHandlerTests.cs(~16 tests)These files were never compiled because Unit.Tests.csproj was deleted during the migration. Result: ~38 test cases invisible to coverage reporting.
Current coverage: 77.5% (threshold: 80%) — blocked from merge
Solution
Verification
✅ Web.Tests compiles with moved files (0 errors)
✅ All 127 tests in Web.Tests pass
Expected Outcome
Coverage restored to 80%+ and unblocks Sprint 9