test+refactor: services to 100/100; cover REST Host.Extensions + listener lifecycles; delete unused factories#19
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR significantly boosts test coverage across the Paperless stack but introduces several regressions and maintainability concerns that should be addressed before merging. Most notably, the code contradicts the PR description regarding 'DocumentErrors.StorageUnavailable', which has been deleted despite being labeled as a replacement for other removed components.
Furthermore, 'ReportProcessor.cs' contains performance bottlenecks due to repeated schema compilation and logical flaws in exception handling that render certain catch blocks unreachable. Codacy reports the PR is not up to standards, primarily due to a massive increase in code clones (107) and several paged-result variables being deconstructed but never asserted in tests. High-risk areas include the 'ReportProcessor' and 'GenAiResultListenerTests', which require refactoring to reduce duplication and improve error visibility.
About this PR
- Systemic duplication detected: 107 new clones were identified by Codacy, particularly within 'GenAiResultListenerTests'. Consider refactoring shared setup and assertion logic into private helpers to improve maintainability.
- There is a contradiction in the PR documentation: the summary states that 'DocumentErrors.StorageUnavailable' supersedes 'RichProblemDetailsFactory', but the actual diff shows 'StorageUnavailable' was also deleted. This leaves a gap in how storage-specific failures are mapped.
Test suggestions
- ContractViolationException factory methods correctly populate error types and build multi-error messages.
- TypedErrorOrAsyncExtensions correctly maps ErrorOr results to OK, NoContent, and NotFound status codes.
- GenAiResultListener and OcrResultListener log start/stop events and exit loops gracefully on cancellation.
- SearchIndexService.InitializeAsync handles concurrent first-time callers using a thread-safe double-check lock.
- ReportProcessor correctly handles dates that satisfy XML schema but fail DateOnly parsing (e.g., timezone suffixes).
- DocumentService rethrows unknown storage exceptions while mapping known state transition failures to validation errors.
- ServiceCollectionExtensions properly manages MinIO bucket creation, including handling 'already exists' race conditions.
- Missing verification of paging state ('hasMore' flags) in DocumentRepositoryIntegrationTests.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Missing verification of paging state ('hasMore' flags) in DocumentRepositoryIntegrationTests.
Low confidence findings
- The PR description notes that the flake in 'MultipleDocuments_SearchCorrectly' remains unresolved. This should be tracked or addressed to ensure CI stability.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
|
|
||
| // Act | ||
| (List<Document> results, bool hasMore) = await _repository | ||
| (List<Document> results, bool hasMore) = await Repository |
There was a problem hiding this comment.
🟡 MEDIUM RISK
The deconstructed variable 'hasMore' is currently unused. Consider adding an assertion to verify the paging state or use a discard if it is intentionally ignored.
| // XmlSerializer.Deserialize wraps both XmlException (well-formedness) | ||
| // and XmlSchemaException (validation) as InvalidOperationException, so a | ||
| // separate `catch (XmlException)` branch is unreachable. | ||
| return ReportErrors.InvalidSchema(ex.Message); |
There was a problem hiding this comment.
🟡 MEDIUM RISK
Suggestion: Improve error reporting by including InnerException.Message to provide actionable feedback for XML/schema violations. Additionally, rephrase the comment on line 129 to avoid using code-like syntax (keywords like 'catch') which triggers linter warnings for commented-out code.
| // separate `catch (XmlException)` branch is unreachable. | ||
| return ReportErrors.InvalidSchema(ex.Message); | ||
| } | ||
| catch (XmlSchemaException ex) |
There was a problem hiding this comment.
⚪ LOW RISK
This catch block for XmlSchemaException is unreachable. When using XmlSerializer.Deserialize, schema validation errors are wrapped in an InvalidOperationException, which is already handled at line 125.
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughPR expands test coverage across error result conversion, listener lifecycle management, service configuration, and search concurrency. Consolidates error factory helpers ( ChangesTest Coverage Expansion and Error Factory Consolidation
Sequence Diagram(s)No diagram required: changes are primarily test additions and error factory consolidation without new feature flows or significant control-flow interactions. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| CodeStyle | 3 minor |
🟢 Metrics 136 complexity · 93 duplication
Metric Results Complexity 136 Duplication 93
AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Pull request overview
This PR is a coverage-focused push across PaperlessREST and PaperlessServices, primarily adding unit/integration tests to exercise previously uncovered branches, plus a small set of production refactors and dead-code deletions to simplify unreachable paths.
Changes:
- Expanded unit + integration test suites to cover listener lifecycles, DI registration, error-to-typed-result conversions, report processing edge cases, and search-index initialization concurrency.
- Refactored
TypedErrorOrAsyncExtensionsusage (and some endpoints) to avoidValueTaskanalyzer pitfalls while keeping the same endpoint result contracts. - Deleted unused production code paths (e.g., old ProblemDetails factory and unused error factories) and tightened a few resource-lifetime patterns in tests.
Reviewed changes
Copilot reviewed 46 out of 46 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| Pipeline/Components/ITest.cs | Updates xUnit MTP namespace filters for Unit/Integration targets. |
| PaperlessUI.Blazor/Components/Pages/Weather.razor | Switches to TimeProvider and cryptographic RNG in the sample page. |
| PaperlessServices.Tests/Unit/ServiceCollectionExtensionsTests.cs | Adds unit coverage for PaperlessServices DI extension methods and option parsing branches. |
| PaperlessServices.Tests/Unit/SearchIndexServiceTests.cs | Improves resilience-path coverage and resource cleanup for ES settings in unit tests. |
| PaperlessServices.Tests/Unit/OcrWorkerTests.cs | Ensures worker instances are disposed in tests. |
| PaperlessServices.Tests/Unit/OcrProcessorTests.cs | Updates deterministic IDs/time usage patterns in OCR processor tests. |
| PaperlessServices.Tests/Integration/WorkerTestBase.cs | Adjusts MinIO client setup/disposal and time usage in integration fixture base. |
| PaperlessServices.Tests/Integration/SearchIndexIntegrationTests.cs | Updates naming/time usage and improves clarity of test constants. |
| PaperlessServices.Tests/Integration/SearchIndexConcurrencyTests.cs | Adds integration coverage for SearchIndexService.InitializeAsync concurrency branches. |
| PaperlessServices.Tests/Integration/OcrIntegrationTests.cs | Updates time usage; minor constant naming tweaks. |
| PaperlessServices.Tests/Integration/FakeTextSummarizer.cs | Loosens input validation in fake summarizer to support integration flows. |
| PaperlessServices.Tests/GlobalUsings.cs | Adds System.Diagnostics.CodeAnalysis for shared suppressions. |
| PaperlessREST/Host/Extensions/TypedErrorOrAsyncExtensions.cs | Adds ErrorOr<T> / ErrorOr<Deleted> extension overloads and delegates async overloads to them. |
| PaperlessREST/Host/Extensions/ServiceCollectionExtensions.cs | Inlines MinIO/Elasticsearch client construction in DI registration. |
| PaperlessREST/Host/Extensions/RichProblemDetailsFactory.cs | Deletes unused ProblemDetails factory/metadata extensions. |
| PaperlessREST/Features/DocumentManagement/Presentation/Endpoints/DocumentEndpoints.cs | Adjusts endpoints to await then map ErrorOr<T> (aligning with updated extensions/analyzers). |
| PaperlessREST/Features/DocumentManagement/Application/DocumentErrors.cs | Removes unused error factories to reduce dead code. |
| PaperlessREST/Features/BatchProcessing/Application/ReportProcessor.cs | Disposes schema reader explicitly; removes unreachable XML exception catch with rationale. |
| PaperlessREST/Features/BatchProcessing/Application/ReportErrors.cs | Removes unused InvalidXml and deletes unused BatchErrors class. |
| PaperlessREST/Configuration/MinioOptions.cs | Tightens endpoint parsing and removes now-unused CreateClient() helper. |
| PaperlessREST/Configuration/ElasticsearchOptions.cs | Removes now-unused CreateClient() helper. |
| PaperlessREST.Tests/Unit/UploadDocumentRequestDtoTests.cs | Adds coverage for record with/copy-constructor behavior. |
| PaperlessREST.Tests/Unit/TypedErrorOrAsyncExtensionsTests.cs | Adds comprehensive tests for typed-result conversion and contract violation paths. |
| PaperlessREST.Tests/Unit/ServiceCollectionExtensionsTests.cs | Adds unit coverage for REST DI helpers (bucket ensure, recurring jobs, env helpers). |
| PaperlessREST.Tests/Unit/ReportProcessorTests.cs | Adds coverage for schema-valid-but-parse-invalid date format edge case. |
| PaperlessREST.Tests/Unit/OpenApiMetadataExtensionsTests.cs | Adds tests to validate OpenAPI metadata extension behavior. |
| PaperlessREST.Tests/Unit/OcrResultListenerTests.cs | Disposes listeners in tests and switches time usage to TimeProvider. |
| PaperlessREST.Tests/Unit/MappingTests.cs | Uses TimeProvider time instead of DateTimeOffset.UtcNow. |
| PaperlessREST.Tests/Unit/ListenerLifecycleTests.cs | Adds execution/lifecycle coverage for listeners, including cancellation and exception paths. |
| PaperlessREST.Tests/Unit/GlobalExceptionHandlerTests.cs | Minor activity/time constant cleanup and deterministic fixed time usage. |
| PaperlessREST.Tests/Unit/GenAiResultListenerTests.cs | Disposes listeners in tests and switches event time to TimeProvider. |
| PaperlessREST.Tests/Unit/ExceptionHandlerTests.cs | Uses a more specific exception type in test setup. |
| PaperlessREST.Tests/Unit/EndpointsTests.cs | Uses TimeProvider for result timestamps; renames cached builders for style consistency. |
| PaperlessREST.Tests/Unit/DocumentTests.cs | Uses static-field naming convention and TimeProvider-consistent timestamps. |
| PaperlessREST.Tests/Unit/DocumentServiceTests.cs | Adds additional exception/transition-path coverage and switches time to TimeProvider. |
| PaperlessREST.Tests/Unit/ContractViolationExceptionTests.cs | Adds unit coverage for contract-violation exception factories/diagnostics. |
| PaperlessREST.Tests/Unit/BatchOrchestratorTests.cs | Uses TimeProvider.System in mock returns to avoid direct UtcNow. |
| PaperlessREST.Tests/Integration/SharedRestContainerFixture.cs | Refactors factory setup into subclass and improves disposal/nullability handling. |
| PaperlessREST.Tests/Integration/DocumentRepositoryIntegrationTests.cs | Improves nullability initialization safety and switches time usage to TimeProvider. |
| PaperlessREST.Tests/Integration/DocumentEndpointTests.cs | Disposes multipart content; adds CA2000 justification and tightens ownership transfer handling. |
| PaperlessREST.Tests/Integration/DocumentAccessRepositoryIntegrationTests.cs | Improves nullability initialization safety and switches date/time usage to TimeProvider. |
| PaperlessREST.Tests/Integration/BatchOrchestratorIntegrationTests.cs | Uses static token field naming and switches date/time usage to TimeProvider. |
| PaperlessREST.Tests/GlobalUsings.cs | Adds System.Diagnostics.CodeAnalysis for shared suppressions. |
| PaperlessREST.Tests/DocumentBuilder.cs | Uses TimeProvider.System instead of UtcNow for builder timestamps. |
| global.json | Bumps ANcpLua.NET.Sdk{,.Web,.Test} to 3.4.29. |
| Directory.Build.props | Removes the warnings carve-out file. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // ═══════════════════════════════════════════════════════════════════════════ | ||
| // ValueTask<ErrorOr<T>> Extensions | ||
| // ═══════════════════════════════════════════════════════════════════════════ | ||
|
|
||
| extension<T>(ValueTask<ErrorOr<T>> task) | ||
| extension<T>(ErrorOr<T> result) | ||
| { |
| var content = new MultipartFormDataContent(); | ||
| try | ||
| { | ||
| var fileContent = new ByteArrayContent(pdfBytes); |
| using MinioClient minioClient = new(); | ||
| minioClient | ||
| .WithEndpoint(minioEndpoint) | ||
| .WithCredentials(_minio.GetAccessKey(), _minio.GetSecretKey()) | ||
| .Build(); | ||
|
|
||
| await minioClient.MakeBucketAsync(new MakeBucketArgs().WithBucket(_bucketName)); |
| using MinioClient minioClient = new(); | ||
| minioClient | ||
| .WithEndpoint(minioEndpoint) | ||
| .WithCredentials(_minio.GetAccessKey(), _minio.GetSecretKey()) | ||
| .Build(); | ||
| await minioClient.MakeBucketAsync(new MakeBucketArgs().WithBucket(_bucketName)); |
…ensions — superseded by DocumentErrors
…o callers) Verified by grep across PaperlessREST + PaperlessServices that these factories have zero production references outside their own files: DocumentErrors (deleted): - StorageTimeout, StorageServerError, StorageConnectionFailed: only used in DocumentServiceErrorMappingTests via the inline Error.Unexpected(...) calls in TryMapStorageException, not via the factory methods. The string codes are duplicated between the factory and the inline calls; tests assert against the inline codes. - StorageUnavailable, SearchUnavailable, StorageFailed, DeleteFailed, InvalidStateTransition, MessageBrokerUnavailable (both overloads): only referenced in XML doc comments in DocumentEndpoints.cs, never invoked. BatchErrors (deleted entirely): - PathRequired, InvalidPath, PathsNotDistinct, InvalidTimeZone: zero references anywhere. BatchOptions validation is handled inline in ServiceCollectionExtensions via AddOptionsWithValidateOnStart. Per CLAUDE.md: 'delete a file outright when the codebase is healthier without it.' Brings DocumentErrors.cs to 100% (only Used factories remain) and removes BatchErrors as dead code.
…ionExtensions
InitializeAsync had two uncovered branches inside the lock that the existing
SearchIndexIntegrationTests could not exercise because the shared fixture
populates the static `s_initializedIndices` cache before the first test runs.
Drives them via two new integration tests that mint a fresh
`test-{Guid.NewGuid():N}` index per test and construct their own
SearchIndexService (DI gives the fixture-bound singleton):
- Branch (b): two concurrent IndexDocumentAsync calls on a unique index race
the outer ContainsKey check, the semaphore serializes them, and the second
caller hits the inner ContainsKey and short-circuits. Proved by asserting
the "Created Elasticsearch index" log fires exactly once via FakeLogger.
- Branch (c): pre-create the index out-of-band, then assert InitializeAsync
finds Exists == true and emits no create-log. The document is still
indexed successfully against the pre-existing index.
A third test pins the "don't fake it" contract for null createdAt: the
field is absent from the persisted _source rather than substituted with
processedAt.
PaperlessServices/Host/Extensions/ServiceCollectionExtensions.cs's no-arg
AddOcrServices() and AddGenAiServices(IConfiguration) were unreachable from
the integration fixture (which uses the IConfiguration overload). A new
unit-test class covers them in isolation, plus the MinIO endpoint
parsing branches (schemeless host:port vs. full URI) and UseSsl=true.
…rrorOr, OpenApi, ServiceCollection ContractViolationException + ContractViolationDiagnostics + ErrorDetail (was 0%): - Every factory: ForNotFoundOnly, ForValidationOnly, ForNotFoundOrConflict, ForCrudOperation, For (custom params). - BuildMessage single-error and three-error branches (verifies the literal "(+ 2 more error(s))" suffix). - GetDiagnostics round-trip including AllErrors ordering and metadata population (present + null). - Record equality holds when array-typed members reference the same instance, and fails when arrays differ (documents synthesized equality's reference semantics on T[]). - with-expressions for both records. - CallerMemberName default for ForNotFoundOnly. TypedErrorOrAsyncExtensions (was 34.6% line / 15.8% branch): - Sync ErrorOr<T>.ToOkOr404: success, NotFound, ContractViolation on non-NotFound. - Sync ErrorOr<Deleted>.ToNoContentOr404: success, NotFound, ContractViolation on non-NotFound. - Task<ErrorOr<T>>/Task<ErrorOr<Deleted>> overloads (delegating to the ValueTask state machines exercise both layers). - ToAcceptedAtRouteOrProblem: success path, Validation grouping by Code, Failure → 500 (kebab-case URN + camelCase metadata extensions), Unexpected → 503 with retryAfter default 30 / metadata override / RetryAfter-key exclusion, unhandled ErrorType → ContractViolation with [Validation, Failure, Unexpected]. - Null / empty / populated Metadata variants for both Failure and Unexpected. - ToKebabCase exercised via assertions on document.-storage-failed, plain.-failure, document.-storage-unavailable, i-pad-or-not, simple (covers leading-lowercase no-dash + internal-uppercase dash branches). OpenApiMetadataExtensions (was 75%): - ProducesNotFound, ProducesConflict, ProducesServiceUnavailable, ProducesGetByIdErrors, ProducesDeleteErrors(canConflict=false/true), ProducesWriteErrors, ProducesDocumentUploadErrors — assert via IEndpointRouteBuilder.DataSources materialization so the Finally callbacks actually run and metadata is observable. - ProducesDeleteErrors returns the same builder instance for chaining. ServiceCollectionExtensions EnsureStorageBucketAsync (was 42.86%): - Bucket exists → MakeBucket never called. - Bucket missing → MakeBucket invoked + LogInformation fires. - Race condition → ArgumentException "already owned" swallowed + LogDebug "already exists" fires. - ArgumentException without "already owned" rethrows. ServiceCollectionExtensions RegisterRecurringJobs: - AddOrUpdate called with JobId, cron, and timezone from BatchOptions. - LogInformation includes JobId/cron/tz. ServiceCollectionExtensions property accessors: - Minio, MinioOpts, BatchOpts, DbFactory return the registered instances. WebApplication.IsDev — Development=true / Production=false. Remaining: MapEndpoints' if(app.IsDev) branch (lines 34–36, 42–43) covers five lines that require a fully-wired WebApplicationFactory in Development environment (Hangfire dashboard, Scalar, OpenAPI). The integration tests spin Test environment by design. Leaving these uncovered for now; they are host-only wiring and exercised in dev runtime.
… delete unreachable XmlException branch
DocumentService.cs gaps closed (100% line/branch):
- UploadDocumentAsync: added test for unknown storage exception type that
TryMapStorageException returns null for; asserts the original exception
propagates uncaught (covers the `throw;` re-raise branch).
- ProcessOcrResultAsync: added two tests for the transitionResult.IsError
short-circuit (lines 148-151): one with an already-Completed document
(Document.CannotComplete), one with an already-Failed document
(Document.CannotFail). Asserts MockBehavior.Strict on the repository to
prove UpdateAsync is NOT called when the state transition fails.
ReportProcessor.cs gaps closed:
- Added ProcessAsync_DateWithTimezone test: '2024-01-15+02:00' satisfies
xs:date schema validation but fails DateOnly.TryParseExact('yyyy-MM-dd'),
exercising the InvalidDate factory at lines 100-103.
- DELETED the `catch (XmlException ex)` branch (lines 125-127) as
unreachable: XmlSerializer.Deserialize wraps both XmlException and
XmlSchemaException as InvalidOperationException before the catch chain
sees them. Verified by writing a test against empty/malformed content and
observing it always hits InvalidOperationException → InvalidSchema.
- DELETED the corresponding ReportErrors.InvalidXml factory (sole caller
was the deleted catch block).
DTOs.cs (UploadDocumentRequest) gap closed:
- New UploadDocumentRequestDtoTests.cs covers the synthesized record
copy-constructor used by 'with' expressions (was the 50% uncovered
half; the File property is exercised by every upload test).
All 350 tests in PaperlessREST.Tests pass.
…ons(true)
The existing SearchIndexServiceTests use ThrowExceptions(false), which
routes transport failures through LogIndexResult's invalid-response
warn path rather than the catch block at lines 57-61. Production wires
the ElasticsearchClient with .ThrowExceptions() (true), so the catch
IS exercised in production but was dead under the current test setup.
Adds IndexDocumentAsync_WhenClientThrows_LogsWarningAndSwallowsException:
constructs a fresh client with ThrowExceptions(true) against the
unreachable host and asserts the catch-block log line ("Failed to
index document...") fires with the underlying TransportException
attached, while IndexDocumentAsync still completes without throwing.
This is the path that protects OCR processing when Elasticsearch is
genuinely unavailable in production.
…mDetails, Hangfire, OpenApi, ApiExplorer lambdas
Drives PaperlessREST/Host/Extensions/ServiceCollectionExtensions.cs from 41/46
(89.1%) to 46/46 (100% line + 100% branch) on the CI-aligned dotcov metric.
Adds 9 facts to ServiceCollectionExtensionsTests.cs covering the previously-
uncovered configuration lambda bodies that ASP.NET Core only invokes through
its options pipeline:
- MapEndpoints_WhenIsDev_RegistersDevelopmentOnlyRoutes — IsDev=true branch
(MapOpenApi + Scalar + Hangfire dashboard) read off IEndpointRouteBuilder.DataSources
- MapEndpoints_WhenNotDev_OmitsDevelopmentOnlyRoutes — IsDev=false branch
- MapEndpoints_WhenIsDev_ScalarConfigureCallback_SetsTitleServersAndTheme —
reflects into the Scalar request-delegate's captured configure Action since
Scalar defers options to HTTP-request time
- AddDependencies_ProblemDetailsCustomization_PopulatesTraceIdAndInstanceFromHttpContextWhenNoActivity
— Activity.Current == null branch, asserts fallback to HttpContext.TraceIdentifier
- AddDependencies_ProblemDetailsCustomization_UsesActivityIdWhenAvailable —
Activity.Current != null branch, asserts Activity.Id wins over TraceIdentifier
- AddDependencies_HangfireServerOptions_SetWorkerCountAndServerName —
invokes only the BackgroundJobServerHostedService factory (to avoid resolving
RabbitMQ listeners that would dial localhost), asserts WorkerCount == ProcessorCount
and ServerName == "{MachineName}-{32hex GUID}"
- AddDependencies_OpenApiCreateSchemaReferenceId_ReturnsNullForEnumAndDefaultForOther
— enum → null, POCO → OpenApiOptions.CreateDefaultSchemaReferenceId default
- AddDependencies_OpenApiDocumentTransformer_SetsTitleVersionAndDescription —
extracts DelegateOpenApiDocumentTransformer._documentTransformer and invokes it
on a fresh OpenApiDocument; asserts exact "Paperless OCR API" / "v1" / description
- AddDependencies_ApiExplorerOptions_SetsGroupNameFormatAndSubstituteApiVersionInUrl
— asserts exact "'v'VVV" + SubstituteApiVersionInUrl == true
CreateWiredBuilder helper wires AddDependencies against in-memory IConfiguration
and swaps PostgreSQL JobStorage for Hangfire MemoryStorage so IHostedService
resolution does not require a running database. GetInlineProblemDetailsConfigure
and GetOpenApiConfigure introspect the ServiceCollection for the production
ConfigureNamedOptions<T>.Action so the actual production lambda is exercised
rather than a copy.
No production code touched. UnitTests: +9 facts, suite remains green.
…k (L20-22)
Drives PaperlessREST/Features/EventProcessing/Presentation/GenAiResultListener.cs
from 58/61 (95.1%) to 60/61 (98.4%) on the CI-aligned dotcov metric. The state
machine <ExecuteAsync>d__5 moves from line-rate 88% / branch-rate 50% to
line-rate 96% / branch-rate 100%.
Adds one fact + one helper iterator to ListenerLifecycleTests.cs:
GenAi_ExecuteAsync_TokenCancelledBetweenYields_BodyBreakCheckFires hits the
`if (stoppingToken.IsCancellationRequested) { break; }` block inside the
await-foreach. The pre-existing StoppingTokenCancelled test cannot reach it —
cancellation through the [EnumeratorCancellation] token short-circuits the
iterator before the loop body re-enters, so the body's IsCancellationRequested
check never fires. The new YieldAfterCancel<T> iterator deliberately ignores
[EnumeratorCancellation], yields the second event after the test cancels the
CTS, and forces the body-internal check to trip + break. Asserts:
- DocumentService.UpdateDocumentSummaryAsync for event #2 → Times.Never
- AckAsync → Times.Once
- SseStream.Publish(e2) → Times.Never
- "GenAI Result Listener stopped" Information log present (clean break, not throw)
- No Error-level logs (proves it was a break, not a generic-catch rethrow)
Remaining uncovered line: <ExecuteAsync>d__5 L34 (closing brace of
`catch (OperationInterruptedException) when (...no queue...)`). That brace is
the leave-instruction sequence point for a catch body whose last statement is
`await Task.Delay(Timeout.Infinite, stoppingToken)`. Task.Delay with Infinite
has no normal-return path — every reachable case throws OperationCanceledException
out of the catch — so the leave target at L34 is unreachable Roslyn-emitted
state-machine noise. Documented as phantom; not chased.
No production code touched.
PR #16 added RichProblemDetailsFactoryTests.cs (exercises types deleted in efceded) and BatchAndReportErrorsTests.cs (exercises BatchErrors deleted in 4a1aa98 + ReportErrors.InvalidXml deleted in e97ff02). - Delete RichProblemDetailsFactoryTests.cs entirely — every symbol it references is gone (RichProblemDetailsFactory, ErrorMetadataExtensions). - Trim BatchAndReportErrorsTests.cs to the four surviving ReportErrors factories (FileNotFound, InvalidSchema, InvalidDate, InvalidGuid). The surface is the only test coverage of ReportErrors.* so it stays.
d8ea6e1 to
700d0e4
Compare
Summary
Drives backend coverage on the CI gate metric (
./build.sh Coverage→dotcov report Artifacts/coverage --exclude-generated) from 97.3% (917/942) at PR #16 to 99.8% (893/895) at branch tip.PaperlessServicesreaches 100% (218/218);PaperlessRESTreaches 99.7% (675/677) — only two phantom Roslyn IL braces remain. Branch coverage is 100% (430/430). PaperlessREST.Tests: 220 → 303 (+83); PaperlessServices.Tests: 60 → 67 (+7); 370 unit tests total, all green.d44c6c1RichProblemDetailsFactory/ErrorMetadataExtensions— superseded byDocumentErrorsf5a1e99DocumentErrors/BatchErrorsfactories (no callers)2a3b9c2ExecuteAsynclifecycle branchesdb342fdSearchIndexServiceconcurrency +ServiceCollectionExtensions(Services side)794d2adHost.Extensionsto 100% —ContractViolationException,TypedErrorOrAsyncExtensions,OpenApiMetadataExtensions,ServiceCollectionExtensionspartialf2515b2DocumentService/ReportProcessor/UploadRequestgaps; delete unreachableXmlExceptioncatch branch (caught by enclosingInvalidOperationException)19957f1SearchIndexServicecatch block viaThrowExceptions(true)2df1f95ServiceCollectionExtensionsto 100% — IsDev, ProblemDetails, Hangfire, OpenApi, ApiExplorer lambdasd8ea6e1Failure modes newly tested
ProblemDetailsCustomizationwithActivity.Current == null(fallback toHttpContext.TraceIdentifier);OpenApi.CreateSchemaReferenceIdon enum vs POCO;MapEndpointsIsDev=true vs IsDev=false;Document.MarkAsCompletedalready-completed / already-failed transitionsMinioClient.MakeBucketAsyncthrowing non-"already owned"ArgumentExceptionrethrows;ElasticsearchClientconfigured withThrowExceptions(true)forSearchIndexServicecatch arm; storage exception mapping inDocumentService.UploadDocumentAsync(TimeoutException, HttpRequestException 5xx, IOException+SocketException)ErrorOrvariantsDocumentService.ProcessOcrResultAsyncreturningDocument.StateTransition*errors;DocumentService.UpdateDocumentSummaryAsyncreturningDocument.NotFound;ReportProcessorreturningInvalidSchema/InvalidDate/InvalidGuid/FileNotFoundif (stoppingToken.IsCancellationRequested) break;via a deliberately non-cooperative iterator (YieldAfterCancel<T>that ignores[EnumeratorCancellation])SearchIndexService.InitializeAsyncdouble-caller race — second caller enters, finds the index already in the dictionary inside the lock, and returns early (no secondIndices.CreateAsynccall)ProblemDetailsContextcarriestrace_id+instance; ApiExplorerGroupNameFormat == "'v'VVV"andSubstituteApiVersionInUrl == true; OpenApi document transformer sets exact Title/Version/Description stringsProduction code deleted as unreachable / unused
PaperlessREST/Host/Extensions/RichProblemDetailsFactory.cs+ErrorMetadataExtensions.cs— superseded byDocumentErrorsfactories; no remaining callersPaperlessREST/Features/DocumentManagement/Application/DocumentErrors.csandBatchProcessing/Application/BatchErrors.cs— old factories removed;Document*Errors/ReportErrorsconsolidatedReportProcessor.cs—catch (XmlException)branch removed;XmlSerializer.Deserializealready wraps bothXmlException(well-formedness) andXmlSchemaException(validation) asInvalidOperationException, so the explicitXmlExceptionarm was unreachableFiles still <100% (both phantom Roslyn IL sequence points)
PaperlessREST/Features/EventProcessing/Presentation/GenAiResultListener.cs<ExecuteAsync>d__5L34 is the closing brace ofcatch (OperationInterruptedException) when (...no queue...). Its body's last statement isawait Task.Delay(Timeout.Infinite, stoppingToken), which has no normal-return path — every cancellation case throws OCE out of the catch — so theleavetarget at L34 is unreachable. Eliminating it would require weakening the cancellation semantics; out of scope.PaperlessREST/Features/BatchProcessing/Application/ReportProcessor.cs<ParseAndValidateXmlAsync>d__11L120 is the closing brace of thetryblock, also the implicit dispose-scope terminator forawait using FileSystemStream stream. All five reachable return paths inside the try (L97 InvalidSchema, L103 InvalidDate, L110 empty docs, L116 InvalidGuid, L119 happy) are covered with hits=1; the JIT elides the visit to the outer try-}when every exit is an explicitreturn. No new test input can reach a sixth exit.Final session (commits
2df1f95,d8ea6e1) — specificallyTwo commits added on top of the prior 7. Coverage delta on the dotcov metric: 886/895 (99.0%) → 893/895 (99.8%), +7 lines.
2df1f95closed theServiceCollectionExtensions89.1% → 100% gap (5 lines: 4 lambda bodies + theIsDev=truebranch). Tests reflect into ASP.NET Core'sConfigureNamedOptions<T>.Action, Scalar's request-delegate-captured options Action, Hangfire'sBackgroundJobServerHostedService._optionsfield, and OpenAPI'sDelegateOpenApiDocumentTransformer._documentTransformerto invoke each production lambda directly.MemoryStorageswap avoids dialing Postgres.d8ea6e1closed the GenAi listener 95.1% → 98.4% gap (2 lines).YieldAfterCancel<T>deliberately ignores[EnumeratorCancellation]to force the body-internal cancel check to trip after the iterator hands the next event back into a cancelled foreach.Test plan
./build.sh UnitTests→ 303 + 67 = 370 passed./build.sh IntegrationTests→ green./build.sh Coverage→ green;dotcov report Artifacts/coverage --exclude-generated→ 893/895 (99.8%) line, 100% branchBuild & Test (backend)job green on push🤖 Generated with Claude Code