feat(tests): migrate Unit.Tests → Web.Tests and remove Unit.Tests project - #152
Conversation
…ject - Add Handle_Success_DoesNotCallInvalidateById to CreateBlogPostHandlerTests - Add HandleGetById_CacheServiceThrows_ReturnsFailResult to EditBlogPostHandlerTests - Remove InternalsVisibleTo(Unit.Tests), add Web.Tests/Bunit/Integration to AssemblyInfo.cs - Remove Unit.Tests from solution file and squad-release.yml workflow - Delete tests/Unit.Tests/ project entirely Closes #151 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 @@
## sprint/6-code-quality #152 +/- ##
=========================================================
- Coverage 77.23% 76.80% -0.43%
=========================================================
Files 43 43
Lines 672 677 +5
Branches 111 112 +1
=========================================================
+ Hits 519 520 +1
- Misses 103 106 +3
- Partials 50 51 +1 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Migrates remaining handler unit tests from the removed tests/Unit.Tests project into tests/Web.Tests, updates internals visibility for the new test assemblies, and removes the redundant Unit.Tests project from the solution and release workflow.
Changes:
- Added missing handler test cases to
tests/Web.Tests(Create + Edit/GetById scenarios). - Updated
src/Webinternals visibility to targetWeb.Tests*assemblies instead ofUnit.Tests. - Removed
Unit.Testsfrom the solution and from the tag release workflow; deleted thetests/Unit.Tests/project.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Web.Tests/Handlers/CreateBlogPostHandlerTests.cs | Adds a cache-invalidation behavior test for create. |
| tests/Web.Tests/Handlers/EditBlogPostHandlerTests.cs | Adds a cache-exception handling test for the GetById query handled by the edit handler. |
| src/Web/Properties/AssemblyInfo.cs | Switches InternalsVisibleTo from Unit.Tests to Web.Tests, Web.Tests.Bunit, Web.Tests.Integration. |
| MyBlog.slnx | Removes tests/Unit.Tests/Unit.Tests.csproj from the solution. |
| .github/workflows/squad-release.yml | Removes Unit.Tests execution steps from release workflow. |
| tests/Unit.Tests/* (deleted) | Deletes the redundant Unit.Tests project and its test files. |
| await _handler.Handle(command, CancellationToken.None); | ||
|
|
||
| // Assert |
There was a problem hiding this comment.
This test only asserts that InvalidateByIdAsync was not called. If the handler returns a failure result (or otherwise misbehaves) but still doesn’t invalidate-by-id, the test would pass. Capture the handler result and assert success (and optionally that InvalidateAllAsync was called) so the test fails on unexpected outcomes.
| await _handler.Handle(command, CancellationToken.None); | |
| // Assert | |
| var result = await _handler.Handle(command, CancellationToken.None); | |
| // Assert | |
| result.Success.Should().BeTrue(); | |
| await _cache.Received(1).InvalidateAllAsync(Arg.Any<CancellationToken>()); |
| - name: Run unit tests | ||
| run: | |
There was a problem hiding this comment.
After removing Unit.Tests, this release workflow now only runs Architecture.Tests. That’s a significant reduction in release validation compared to other workflows (e.g., squad-preview.yml runs Web.Tests and Web.Tests.Integration too). Consider running the remaining relevant test projects here (Web.Tests, Web.Tests.Bunit if applicable, Web.Tests.Integration, Domain.Tests, etc.) so tag releases aren’t created without executing the main test suite. Also note that .github/workflows/squad-insider-release.yml still invokes tests/Unit.Tests/Unit.Tests.csproj, which will fail now that the project is deleted; it should be updated in the same PR to remove/replace those calls.
| - name: Run unit tests | |
| run: | | |
| - name: Run test suite | |
| run: | | |
| dotnet test tests/Domain.Tests --configuration Release --no-build --no-restore \ | |
| --logger "trx;LogFileName=domain-results.trx" | |
| dotnet test tests/Web.Tests --configuration Release --no-build --no-restore \ | |
| --logger "trx;LogFileName=web-results.trx" | |
| dotnet test tests/Web.Tests.Integration --configuration Release --no-build --no-restore \ | |
| --logger "trx;LogFileName=web-integration-results.trx" |
| <Project Path="tests/AppHost.Tests/AppHost.Tests.csproj" /> | ||
| <Project Path="tests/Architecture.Tests/Architecture.Tests.csproj" /> | ||
| <Project Path="tests/Domain.Tests/Domain.Tests.csproj" /> | ||
| <Project Path="tests/Unit.Tests/Unit.Tests.csproj" /> | ||
| <Project Path="tests/Web.Tests/Web.Tests.csproj" /> | ||
| <Project Path="tests/Web.Tests.Bunit/Web.Tests.Bunit.csproj" /> |
There was a problem hiding this comment.
With Unit.Tests removed from the solution, double-check that all unique scenarios previously covered there were migrated into Web.Tests. For example, the deleted Unit.Tests/Handlers/GetBlogPostsHandlerTests.cs included a cache-service-throws path test, but tests/Web.Tests/Handlers/GetBlogPostsHandlerTests.cs currently doesn’t cover the cache service throwing directly (only an exception returned from the fetch path). Consider adding that test to avoid coverage regression before removing the project from the solution.
- CreateBlogPostHandlerTests: Handle_Success_DoesNotCallInvalidateById now captures the result and asserts Success=true and Received(1).InvalidateAllAsync in addition to the existing DidNotReceive.InvalidateByIdAsync assertion - GetBlogPostsHandlerTests: add Handle_CacheServiceThrows_ReturnsFailResult using ThrowsAsync(InvalidOperationException) to cover the cache-throws path - squad-release.yml: replace lone Architecture.Tests run with full suite: Architecture.Tests, Domain.Tests, Web.Tests, Web.Tests.Integration - squad-insider-release.yml: remove three broken Unit.Tests references and replace with same four-project test suite as squad-release.yml Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…d/151-migrate-unit-tests-to-web-tests
Unit.Tests project was removed in PR #152. This entry is now orphaned. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
## Sprint 6: Code Quality **Goal:** Eliminate analyzer warnings, enforce internal visibility on Web feature types, expand test coverage, and migrate Unit.Tests into Web.Tests. --- ## Merged Feature PRs - Closes #137 — PR #144 — fix(quality): rename `ct` → `cancellationToken` in all MediatR handlers - Closes #138 — PR #145 — test: add unit tests for BlogPost, Result, and ValidationBehavior (42 tests) - Closes #139 — PR #146 — test: add unit tests for UserManagementHandler (16 tests) - Closes #140 — PR #147 — fix(quality): make Web feature types `internal` (CA1515, ~28 types) - Closes #141 — PR #148 — test: add unit tests for BlogPostMappings (22 tests) - Closes #142 — PR #149 — fix(quality): add ConfigureAwait(false) and specific exception catches - Closes #143 — PR #150 — fix(quality): address Domain and ServiceDefaults analyzer warnings - Closes #151 — PR #152 — feat(tests): migrate Unit.Tests → Web.Tests and remove Unit.Tests project --- ## Test Summary - **80+ new tests** added across all PRs - **105 Web.Tests passing** (0 failures) - `DynamicProxyGenAssembly2` added to `InternalsVisibleTo` in Web.csproj for NSubstitute support ## Notable Changes - `Unit.Tests` project removed; all tests now live in `Web.Tests` - ~28 Web feature types changed from `public` to `internal` (CA1515 compliance) - All MediatR handler parameters renamed from `ct` to `cancellationToken` for clarity - `ConfigureAwait(false)` applied to all async calls in service layer - Domain and ServiceDefaults analyzer warnings resolved --- ## Checklist - [x] All sprint issues closed (#137–#143, #151) - [x] CI green (0 build errors) - [x] 105 Web.Tests passing - [x] Milestone at 100% --------- Co-authored-by: Boromir <boromir@squad.dev> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Closes #151
Working as Gimli (Test Specialist)
Migrates all valuable Unit.Tests content into Web.Tests and removes the redundant project.
Changes
Handle_Success_DoesNotCallInvalidateByIdtestHandleGetById_CacheServiceThrows_ReturnsFailResulttestInternalsVisibleTo("Unit.Tests")withWeb.Tests,Web.Tests.Bunit,Web.Tests.IntegrationTest Results
All 104 Web.Tests pass (including 2 new test methods).