sprint(7): merge Sprint 7 — xUnit v3 Pilot (Domain.Tests) (#8)#175
Conversation
…e analysis (#169) Closes #166 ## Summary This ADR documents the xUnit v3 migration decision and pilot strategy for Domain.Tests in Sprint 7. ## Contents - **Context**: Why xUnit v3? Ecosystem alignment, performance, MTP adoption - **Decision**: Adopt v3 incrementally starting with Domain.Tests pilot (Sprint 7) - **Rationale**: Incremental rollout validates process before broad deployment - **Consequences**: API changes, migration effort, performance gains, tooling improvements - **Performance Analysis**: - Baseline: 104 ms (42 tests, Domain.Tests) - Projected: 88–99 ms (5–15% improvement) - CI impact: 42–55 seconds (estimated 5–10% savings when fully migrated) - **Rollout Plan**: Sprints 7–13, per-project versioning strategy via Directory.Packages.props ## Validation - ✅ Release build passes - ✅ ADR follows existing format (markdown + YAML front matter) - ✅ Includes performance data, alternatives, mitigation strategies - ✅ References xUnit v3 migration guide and related issues ## Related - Issue #163: Domain.Tests package swap - Issue #164: Domain.Tests API rewrite - Sprint 7 milestone --------- Co-authored-by: Boromir <boromir@squad.dev> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
## Summary Adds xUnit v3 (3.2.2 stable) package family to `Directory.Packages.props` as part of Sprint 7 xUnit v3 pilot. ## Changes **`Directory.Packages.props`** — added 4 package versions to the centralized NuGet manifest: | Package | Version | Purpose | |---------|---------|---------| | `xunit.v3` | 3.2.2 | Main xUnit v3 metapackage | | `xunit.v3.assert` | 3.2.2 | v3 assertion library | | `xunit.v3.extensibility.core` | 3.2.2 | v3 extensibility hooks | | `xunit.analyzers` | 1.27.0 | Roslyn analyzers (v2 + v3 compatible) | `xunit` (2.9.3) and `xunit.runner.visualstudio` (3.1.5) were already present and are retained. ## Notes - All versions are centralized in `Directory.Packages.props` per team convention — no version attributes added to any `.csproj` - `dotnet restore` verified locally — no resolution errors - xUnit v2 (`xunit 2.9.3`) is retained for backward compatibility while other test projects migrate Closes #162 --- *Working as Boromir (DevOps engineer) — NuGet centralized version management* --------- Co-authored-by: Boromir <boromir@squad.dev> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…#165) (#174) ## Summary Adds a `test-domain` job to `squad-test.yml` to validate that Domain.Tests runs correctly in CI under xUnit v3. This unblocks Gimli's migration work (#163/#164) — the CI pipeline is ready before test code is written. ## Changes **`.github/workflows/squad-test.yml`** — 3 targeted edits: 1. **New `test-domain` job** — runs `tests/Domain.Tests` with the same pattern as other test jobs: - `dotnet build` + `dotnet test --no-build` (Release config) - `--collect:"XPlat Code Coverage"` with Cobertura format - TRX logger → `domain-test-results` artifact - Guard clause: skips gracefully if project not found 2. **`coverage` job** — added `test-domain` to `needs` list so Domain.Tests coverage is included in the report 3. **`report` job** — added `test-domain` to `needs` list and job summary output; added `domain_status` to the overall failure gate ## xUnit v3 CI Compatibility No runner/tool changes were required: | Tool | v2 compat | v3 compat | Notes | |------|-----------|-----------|-------| | `dotnet test` | ✅ | ✅ | No changes needed | | `XPlat Code Coverage` | ✅ | ✅ | Coverlet collector works unchanged | | TRX logger | ✅ | ✅ | Standard .NET test output | | `EnricoMi/publish-unit-test-result-action` | ✅ | ✅ | Reads TRX, not xUnit-specific | | `coverlet.collector` 10.0.0 | ✅ | ✅ | Already in Directory.Packages.props | ## Depends on PR #173 (feat: add xUnit v3 packages to Directory.Packages.props) should merge first so the NuGet packages are available. Closes #165 --- *Working as Boromir (DevOps engineer) — CI/CD pipeline management* Co-authored-by: Boromir <boromir@squad.dev> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
## Summary Working as **Aragorn** (Lead Developer) Closes #167 Phase 1 of the Sprint 7 xUnit v3 pilot retrospective process. Creates the template and tracking infrastructure that will be filled in at sprint end (Phase 2, after #162–#166 merge). --- ## What's in this PR ### `docs/sprint-7-xunit-v3-pilot-retro.md` Full retrospective template with: - **Context** — sprint theme, branch, issue scope - **Pilot Scope** — Domain.Tests only; table showing Sprints 8–10 targets - **Metrics to Track** — build time, test count/pass rate/exec time, coverage %, CI feedback time - **Success Criteria** — checkbox list Aragorn will validate at sprint end - **Results / Lessons Learned / Decision** — placeholder sections for Phase 2 - **Migration Playbook (Draft)** — package change stubs for Unit.Tests/Architecture.Tests/Blazor.Tests reuse ### Local tracking doc (`.squad/decisions/inbox/` — gitignored) - `aragorn-sprint7-xunit-pilot-tracking.md` — live log for build breaks, test failures, CI issues, API breaking changes, package compatibility, and sprint metrics - `aragorn-sprint7-pilot-plan.md` — decision record for the phased migration approach --- ## Retro approach | Phase | Trigger | Action | |---|---|---| | Phase 1 (now) | Sprint kickoff | Create template + tracking infrastructure | | Phase 2 (sprint end) | #162–#166 all merged | Run validation, gather metrics, fill in retro, deliver go/no-go decision | Phase 2 will add a second commit to this branch finalising the report and closing #167. ---⚠️ This task was flagged as **"needs review"** — the Phase 2 retro content (results, decision, lessons learned) should be verified by Boromir (CI metrics) and Gimli (test metrics) before the final commit merges. Co-authored-by: Boromir <boromir@squad.dev> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
## Summary Migrates `Domain.Tests` from xUnit v2 (2.9.3) to xUnit v3 (3.2.2) as the pilot project for Sprint 7. Working as **Gimli** (Tester) 🪓 Closes #163 ## Changes | File | Change | |------|--------| | `Directory.Packages.props` | Added `xunit.v3 3.2.2` alongside existing `xunit 2.9.3` (other projects unaffected) | | `tests/Domain.Tests/Domain.Tests.csproj` | Replaced `xunit` → `xunit.v3` package reference; added `xunit.runner.json` as content | | `tests/Domain.Tests/xunit.runner.json` | New: v3 runner config (parallel execution, method display, no diagnostics) | **Zero source changes required** — the 42 test files use simple `[Fact]`/`[Theory]` patterns with no `ITestOutputHelper`, `IAsyncLifetime`, or collection fixtures. The `Xunit` namespace is unchanged in v3. ## Test Results | Suite | Result | |-------|--------| | Domain.Tests (xUnit v3) | ✅ 42/42 pass | | Architecture.Tests | ✅ 11/11 pass | | Web.Tests | ✅ 105/105 pass | | Web.Tests.Bunit | ✅ 61/61 pass | ## Notes - `xunit.runner.visualstudio 3.1.5` was already in place and is v3-compatible — no change needed. - NuGet change from #162 was folded into this PR since #162 had no commits beyond the sprint base. Co-authored-by: Boromir <boromir@squad.dev> 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 #175 +/- ##
==========================================
+ Coverage 72.12% 72.53% +0.41%
==========================================
Files 43 43
Lines 721 721
Branches 112 112
==========================================
+ Hits 520 523 +3
+ Misses 150 149 -1
+ Partials 51 49 -2 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR completes the Sprint 7 “xUnit v3 pilot” by introducing parallel xUnit v3 package versions, switching tests/Domain.Tests to xunit.v3, adding pilot documentation (ADR + retro), and updating CI to run Domain.Tests as its own job.
Changes:
- Add xUnit v3-related packages to central package management (
Directory.Packages.props). - Migrate
tests/Domain.Teststoxunit.v3and addxunit.runner.jsonconfiguration. - Add Sprint 7 ADR/retro docs and extend
squad-test.ymlwith a dedicated Domain.Tests job.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Domain.Tests/xunit.runner.json | Adds xUnit runner configuration for Domain.Tests. |
| tests/Domain.Tests/Domain.Tests.csproj | Switches Domain.Tests to xunit.v3 and includes xunit.runner.json in outputs. |
| docs/sprint-7-xunit-v3-pilot-retro.md | Adds Sprint 7 pilot retrospective report. |
| docs/adr/sprint7-xunit-v3-migration.md | Adds ADR documenting the xUnit v3 pilot rationale/plan. |
| Directory.Packages.props | Adds central versions for xUnit v3 packages (and analyzers). |
| .squad/agents/pippin/history.md | Records Sprint 7 ADR work in agent history. |
| .github/workflows/squad-test.yml | Adds a test-domain job and wires it into summary/coverage/report dependencies. |
| 3. **API Changes**: Documented breaking changes (`[Fact]` → `[Test]`, `TheoryData<T>` adjustments) with before/after examples | ||
| 4. **Per-Project Versioning**: Established Directory.Packages.props strategy to allow coexistence during migration | ||
| 5. **Risk Mitigation**: Fallback plan if pilot discovers critical blockers; measurement strategy for validating ecosystem benchmarks |
There was a problem hiding this comment.
This history entry claims the ADR documents breaking changes like [Fact]` → `[Test]. That doesn’t align with the current Domain.Tests code in this repo (tests still use [Fact]/[Theory]). Please update this summary so it reflects the actual migration guidance and avoids propagating the incorrect attribute rename.
| 3. **API Changes**: Documented breaking changes (`[Fact]` → `[Test]`, `TheoryData<T>` adjustments) with before/after examples | |
| 4. **Per-Project Versioning**: Established Directory.Packages.props strategy to allow coexistence during migration | |
| 5. **Risk Mitigation**: Fallback plan if pilot discovers critical blockers; measurement strategy for validating ecosystem benchmarks | |
| 3. **Migration Guidance**: Documented xUnit v3 compatibility considerations, including review of test APIs and data-driven test patterns such as `TheoryData<T>`, with examples where applicable | |
| 4. **Per-Project Versioning**: Established Directory.Packages.props strategy to allow coexistence during migration | |
| 5. **Risk Mitigation**: Fallback plan if the pilot discovers critical blockers; measurement strategy for validating ecosystem benchmarks |
| <PackageReference Include="MediatR"/> | ||
| <PackageReference Include="Microsoft.NET.Test.Sdk" /> | ||
| <PackageReference Include="NSubstitute"/> | ||
| <PackageReference Include="xunit"/> | ||
| <PackageReference Include="xunit.v3"/> | ||
| <PackageReference Include="xunit.runner.visualstudio"/> | ||
| </ItemGroup> |
There was a problem hiding this comment.
tests/Domain.Tests is being switched to xunit.v3, but the project file doesn't set <OutputType>Exe</OutputType>. The Sprint 7 acceptance criteria/reference docs indicate this is required to run under Microsoft Testing Platform and to support dotnet run for the test executable; please add OutputType to the main PropertyGroup to avoid an incomplete v3 migration.
| ## Rationale | ||
|
|
||
| **Why a pilot?** | ||
| - xUnit v3 is a major version with new APIs (e.g., `[Fact]` becomes `[Test]`, `TheoryData<T>` changes). Broad, fast migration risks introducing bugs. |
There was a problem hiding this comment.
The ADR states that in xUnit v3 "[Fact] becomes [Test]" / "Fact vs. Test naming". In this repo's current tests/Domain.Tests codebase, tests still use [Fact]/[Theory] with using Xunit; and there are no [Test] attributes, so this claim appears inaccurate and will confuse the migration guidance. Please correct the described breaking changes to match the actual migration/API behavior.
| - xUnit v3 is a major version with new APIs (e.g., `[Fact]` becomes `[Test]`, `TheoryData<T>` changes). Broad, fast migration risks introducing bugs. | |
| - xUnit v3 is a major version with package, runner, and compatibility changes to validate. In this codebase, tests continue to use familiar xUnit attributes like `[Fact]` and `[Theory]`; the migration risk is in tooling and project-specific API differences, not a rename to `[Test]`. Broad, fast migration risks introducing bugs. |
|
|
||
| ### Related Issues & PRs | ||
|
|
||
| - **Issue #163**: Domain.Tests package swap (xunit → xunit.v3, OutputType=Exe) |
There was a problem hiding this comment.
This section mentions the Domain.Tests swap as including OutputType=Exe, but tests/Domain.Tests/Domain.Tests.csproj currently doesn't set OutputType (it will default to Library). Please either add OutputType=Exe to the project (if that's the intended v3 approach) or adjust this history note to avoid documenting a change that didn't happen.
| - **Issue #163**: Domain.Tests package swap (xunit → xunit.v3, OutputType=Exe) | |
| - **Issue #163**: Domain.Tests package swap (xunit → xunit.v3) |
| ```xml | ||
| <ItemGroup> | ||
| <PackageVersion Include="xunit" Version="2.9.3" /> | ||
| <PackageVersion Include="xunit.v3" Version="3.0.x" /> |
There was a problem hiding this comment.
The per-project versioning snippet pins xunit.v3 as 3.0.x, but this PR adds xunit.v3 version 3.2.2 in Directory.Packages.props. To keep the ADR accurate as a reference, please update the example versions (or make them clearly illustrative placeholders) so they match the repo’s current package versions.
| <PackageVersion Include="xunit.v3" Version="3.0.x" /> | |
| <PackageVersion Include="xunit.v3" Version="3.2.2" /> |
| </ItemGroup> | ||
|
|
||
| <ItemGroup> | ||
| <Content Include="xunit.runner.json" CopyToOutputDirectory="PreserveNewest"/> |
There was a problem hiding this comment.
xunit.runner.json is included as <Content Include=...> here, but other test projects in the repo include it via <None Update="xunit.runner.json"> with CopyToOutputDirectory. Using None Update avoids treating the config as publish/content payload and keeps the pattern consistent across test projects.
| <Content Include="xunit.runner.json" CopyToOutputDirectory="PreserveNewest"/> | |
| <None Update="xunit.runner.json" CopyToOutputDirectory="PreserveNewest" /> |
| | `tests/Domain.Tests` | ✅ Migrated to xUnit v3 | Gimli | | ||
| | `tests/Unit.Tests` | 🔜 Planned (Sprint 8) | Gimli | | ||
| | `tests/Architecture.Tests` | 🔜 Planned (Sprint 9) | Gimli | | ||
| | `tests/Blazor.Tests` (bUnit) | 🔜 Planned (Sprint 10) | Gimli | |
There was a problem hiding this comment.
This doc references tests/Blazor.Tests for the bUnit project, but that directory/project doesn't exist in this repo (the bUnit project is tests/Web.Tests.Bunit). Please update the project name/path here to avoid misleading readers.
| | `tests/Blazor.Tests` (bUnit) | 🔜 Planned (Sprint 10) | Gimli | | |
| | `tests/Web.Tests.Bunit` (bUnit) | 🔜 Planned (Sprint 10) | Gimli | |
| - [ ] Line coverage ≥ 80% (no regression from Sprint 6 baseline) | ||
| - [ ] Release build (`dotnet build -c Release`) exits 0 with 0 errors | ||
| - [ ] CI workflow `squad-test.yml` passes end-to-end on the sprint branch | ||
| - [ ] No breaking change propagates to `Unit.Tests`, `Architecture.Tests`, or `Blazor.Tests` |
There was a problem hiding this comment.
Blazor.Tests is referenced again here as a non-target project, but the repo's bUnit suite is tests/Web.Tests.Bunit. Please rename this reference to match the actual project so the scope statement is accurate.
| - [ ] No breaking change propagates to `Unit.Tests`, `Architecture.Tests`, or `Blazor.Tests` | |
| - [ ] No breaking change propagates to `Unit.Tests`, `Architecture.Tests`, or `tests/Web.Tests.Bunit` |
| | Sprint 8 | Migrate `tests/Unit.Tests` | Gimli | | ||
| | Sprint 9 | Migrate `tests/Architecture.Tests` | Gimli | | ||
| | Sprint 10 | Migrate `tests/Blazor.Tests` (bUnit) | Gimli + Legolas | | ||
|
|
There was a problem hiding this comment.
This rollout table lists tests/Blazor.Tests for the bUnit project, but the actual project is tests/Web.Tests.Bunit. Please update the name/path here as well so the next-steps plan points to the correct repo target.
| ```csharp | ||
| [Test] | ||
| public void Create_ValidArguments_ReturnsValidBlogPost() | ||
| { | ||
| // Arrange | ||
| var title = "Test Post"; | ||
|
|
||
| // Act | ||
| var blogPost = BlogPost.Create(Guid.NewGuid(), title, "author", "content"); | ||
|
|
||
| // Assert | ||
| blogPost.Title.Should().Be(title); | ||
| } | ||
|
|
||
| [Theory] | ||
| [InlineData("")] | ||
| [InlineData(null)] | ||
| public void Create_NullOrEmptyTitle_ThrowsArgumentException(string? title) | ||
| { | ||
| // Act & Assert | ||
| Assert.Throws<ArgumentException>(() => BlogPost.Create(Guid.NewGuid(), title!, "author", "content")); | ||
| } | ||
| ``` | ||
|
|
||
| **Key changes:** | ||
| - `[Fact]` → `[Test]` | ||
| - `[Theory]` and data annotations remain compatible (no change in this example) | ||
| - Most assertion code is unchanged if using FluentAssertions |
There was a problem hiding this comment.
The v2→v3 rewrite example uses [Test] in the "After" snippet and lists [Fact] → [Test] as a key change. That doesn't match the actual Domain.Tests migration in this repo (still [Fact]/[Theory]), so readers may incorrectly refactor tests. Please update the example and key-changes list to reflect the real xUnit v3 usage chosen for this codebase.
Closes milestone: Sprint 7 — xUnit v3 Pilot (Domain.Tests)
Sprint Summary
Issues Closed
Checklist