test: raise Web coverage above 80% (issue #244) - #245
Conversation
- Add BlogPostCacheServiceTests covering all 4 methods of BlogPostCacheService: GetOrFetchAllAsync (L1 hit, L2 hit, corrupt L2, full miss), GetOrFetchByIdAsync (L1 hit, L2 hit, corrupt L2, full miss, null fetch), InvalidateAllAsync, InvalidateByIdAsync - Add OperationCanceledException rethrow + unexpected Exception tests to CreateBlogPostHandler, DeleteBlogPostHandler, GetBlogPostsHandler, and EditBlogPostHandler (both Handle methods) - Line coverage: 69.5% → 81.5% (527/646 lines) Closes #244 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
github-actions
Bot
added
the
squad
🏗️ PR Added to Squad Triage QueueThis PR has been labeled with Next steps:
|
There was a problem hiding this comment.
Pull request overview
Raises Web project test coverage above the 80% threshold required by issue #244 by adding focused unit tests around caching behavior and handler exception paths.
Changes:
- Added new unit tests for
BlogPostCacheServicecovering L1/L2 cache hits, corrupt L2 payload fallback, cache misses, and invalidation. - Expanded handler test suites to cover
OperationCanceledExceptionrethrow behavior and the generic “unexpected exception” failure path.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Web.Tests/Infrastructure/Caching/BlogPostCacheServiceTests.cs | New tests covering the two-tier blog post cache behavior (L1/L2/miss/corrupt payload/invalidate). |
| tests/Web.Tests/Handlers/GetBlogPostsHandlerTests.cs | Adds tests for cancellation rethrow and unexpected exception result mapping. |
| tests/Web.Tests/Handlers/EditBlogPostHandlerTests.cs | Adds tests for cancellation rethrow and unexpected exception handling in both edit and get-by-id paths. |
| tests/Web.Tests/Handlers/DeleteBlogPostHandlerTests.cs | Adds tests for cancellation rethrow and unexpected exception result mapping. |
| tests/Web.Tests/Handlers/CreateBlogPostHandlerTests.cs | Adds tests for cancellation rethrow and unexpected exception result mapping. |
| using System.Text.Json; | ||
|
|
||
| using Microsoft.Extensions.Caching.Memory; | ||
| using Microsoft.Extensions.Options; |
| private readonly MemoryCache _realLocalCache = new(new MemoryCacheOptions()); | ||
| private readonly IDistributedCache _distributedCache = Substitute.For<IDistributedCache>(); | ||
| private readonly BlogPostCacheService _sut; | ||
|
|
||
| public BlogPostCacheServiceTests() | ||
| { | ||
| _sut = new BlogPostCacheService(_realLocalCache, _distributedCache); | ||
| } | ||
|
|
||
| public void Dispose() => _realLocalCache.Dispose(); | ||
|
|
||
| private static readonly JsonSerializerOptions JsonOpts = new(JsonSerializerDefaults.Web); | ||
|
|
||
| private static List<BlogPostDto> MakeDtos() => | ||
| [ | ||
| new(Guid.NewGuid(), "Title1", "Content1", "Author1", DateTime.UtcNow, null, false), | ||
| new(Guid.NewGuid(), "Title2", "Content2", "Author2", DateTime.UtcNow, null, true), | ||
| ]; | ||
|
|
||
| // ── GetOrFetchAllAsync ──────────────────────────────────────────────────── | ||
|
|
||
| [Fact] | ||
| public async Task GetOrFetchAllAsync_L1Hit_ReturnsCachedListWithoutDistributedCall() | ||
| { | ||
| // Arrange | ||
| var cachedList = MakeDtos(); | ||
| _realLocalCache.Set(BlogPostCacheKeys.All, cachedList); | ||
|
|
||
| var fetchCalled = false; | ||
| Task<IReadOnlyList<BlogPostDto>> fetch() { fetchCalled = true; return Task.FromResult<IReadOnlyList<BlogPostDto>>([]); } | ||
|
|
||
| // Act | ||
| var result = await _sut.GetOrFetchAllAsync(fetch, CancellationToken.None); | ||
|
|
||
| // Assert | ||
| result.Should().HaveCount(2); |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #245 +/- ##
==========================================
+ Coverage 78.35% 83.01% +4.65%
==========================================
Files 43 43
Lines 730 730
Branches 112 112
==========================================
+ Hits 572 606 +34
+ Misses 112 81 -31
+ Partials 46 43 -3 🚀 New features to boost your workflow:
|
930a138 to
5277b66
Compare
✅ Aragorn — Lead Review: APPROVEDPR #245 — test: raise Web coverage above 80% (issue #244)
Gate checks
Coverage
Coverage increases by 12 percentage points, clearing the 80% threshold required by #244. Copilot automated reviewCopilot flagged two items — neither is a bug or security issue:
Both are cleanup items, not blockers. Ask Gimli to address on the branch before merge. Architecture assessment
Verdict✅ APPROVED (with optional cleanup requested). Structurally sound, architecturally correct, satisfies the coverage gate for #244. Recommended pre-merge: Gimli removes the unused |
⚒️ Gimli — Tester Review: REQUEST CHANGESPR #245 — test: raise Web coverage above 80% (issue #244) What's good — a lot, actuallyThe handler exception-path additions are exactly what was needed:
The coverage improvement is genuine — not gaming. These tests hit real production branches. Blockers🚨 BLOCKER 1 —
|
…erviceTests - Remove unused 'using Microsoft.Extensions.Options;' - Re-indent entire file with tabs to match repo test-file style Closes #245 blocker items Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2035ca2 to
a7767df
Compare
✅ Aragorn — Re-Review: APPROVEDPR #245 — test: raise Web coverage above 80% (issue #244)
Blocker resolution confirmedBoth of Gimli's CHANGES_REQUESTED blockers are resolved in the current HEAD (
Copilot's two inline review threads are both marked outdated — the fix commits superseded the original state that triggered those comments. Gate compliance (re-verified)
Verdict✅ APPROVED — all blockers cleared, all gates green. Ralph: you are clear to squash-merge PR #245. |
mpaulosky
left a comment
mpaulosky
left a comment
There was a problem hiding this comment.
⚒️ Gimli — Tester Re-Review: APPROVED ✅
PR #245 — test: raise Web coverage above 80% (issue #244)
Blocker Resolution
Both blockers from my previous REQUEST CHANGES verdict are fully resolved:
✅ BLOCKER 1 CLEARED — Tab indentation restored
BlogPostCacheServiceTests.cs now has correct tab indentation throughout — class fields, constructor, helper methods, and all 11 test methods. Every line inside the class body is properly indented. Matches project standard.
✅ BLOCKER 2 CLEARED — Unused using removed
using Microsoft.Extensions.Options; is gone. The two remaining usings are:
System.Text.Json— used byJsonSerializer,JsonSerializerOptions,JsonSerializerDefaults✅Microsoft.Extensions.Caching.Memory— used byMemoryCache,MemoryCacheOptions✅
No dead imports. File is clean.
Re-Evaluation of BlogPostCacheServiceTests.cs
Full re-read confirms the file is correct and well-structured:
- File header present and correctly formatted ✅
- File-scoped namespace (
namespace Web.Infrastructure.Caching;) ✅ - AAA comments present on all 11 tests ✅
- Real
MemoryCachefor L1 (correct — avoids theIMemoryCache.Setextension-method mock trap) ✅ - NSubstitute for
IDistributedCache✅ IDisposableimplemented to dispose the real cache ✅JsonSerializerOptions JsonOptsscoped asstatic readonly(efficient) ✅- All four cache tiers tested: L1 hit, L2 hit, corrupt JSON fallback, full miss ✅
- Null DB-result path covered for
GetOrFetchByIdAsync✅ - Invalidation tests verify both L1 removal and distributed cache
RemoveAsync✅ - Handler exception-path tests (all four handlers) correct ✅
Standing Non-Blockers (carry-forward, no action needed now)
- L2 hit tests (
GetOrFetchAllAsync_L2Hit,GetOrFetchByIdAsync_L2Hit) don't assert the fetch delegate was NOT called. Airtight but functional without it. Flag for follow-up. EditBlogPostHandlerEdit path missing a test forUpdateAsyncthrowingInvalidOperationException. Pre-existing gap — tell Aragorn.
Verdict
✅ APPROVED — test review is clear.
Both blockers resolved, CI green (304 tests, 0 failures), coverage 83.01% → well above the 80% gate required by #244. This is ready to merge.
Working as Gimli (Tester)
Summary
Raises Web project line coverage from 69.5% → 81.5% (527/646 lines), clearing the 80% threshold required by issue #244.
Changes
New file
tests/Web.Tests/Infrastructure/Caching/BlogPostCacheServiceTests.cs— 11 tests coveringBlogPostCacheService:GetOrFetchAllAsync: L1 hit, L2 hit, corrupt L2 JSON fallback, full missGetOrFetchByIdAsync: L1 hit, L2 hit, corrupt L2 JSON fallback, full miss, null DB resultInvalidateAllAsync,InvalidateByIdAsyncModified files
Added
OperationCanceledExceptionrethrow and unexpectedExceptionbranch tests to:CreateBlogPostHandlerTestsDeleteBlogPostHandlerTestsGetBlogPostsHandlerTestsEditBlogPostHandlerTests(bothHandleEditandHandleGetByIdpaths)Test Results
All 270 tests pass. Pre-push gate passed.
Coverage
Closes #244