You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Root cause of CI failure: ES 9.4.1 changed startup behaviour in a way incompatible with Testcontainers' ElasticsearchConfiguration.TlsEnabled detection, causing a 1-hour hang in SharedContainerFixture.InitializeAsync (run #25952721854).
Reverted ES image to 9.1.3 (was working before the bump).
Added xpack.security.http.ssl.enabled=false so the built-in ES wait strategy probes HTTP, not HTTPS.
Fixed latent DisposeAsync NRE: _host is null! until assigned inside InitializeAsync; if StartAsync throws first, DisposeAsync was crashing on _host.StopAsync().
Changes
Commit
What
86b123f
xpack.security.http.ssl.enabled=false — ES built-in wait uses HTTP
…h wait uses HTTP
Testcontainers.Elasticsearch 4.11's built-in wait strategy probes ES via
`new HttpWaitStrategy().UsingTls(configuration.TlsEnabled)`. TlsEnabled is
the AND of two env vars: `xpack.security.enabled` AND
`xpack.security.http.ssl.enabled`, both explicitly "false". The fixture
only set the first, so TlsEnabled returned true, the wait probed HTTPS,
ES (security disabled) only answered plain HTTP, and the probe never
satisfied — every PaperlessServices integration test failed with
`System.TimeoutException` after Testcontainers' 1-hour default.
Adding the second env var makes TlsEnabled return false, the wait
probes HTTP, and the fixture completes in seconds.
Verified locally on ES 9.4.1 / MinIO 2025-09-07 / RabbitMQ 4.3.0 with
Testcontainers 4.11 — 13/13 integration tests pass in 57s (vs. hanging
forever before).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…llection serializes
xUnit v3's parallelAlgorithm: aggressive submits every test to the
parallel thread pool simultaneously, ignoring collection boundaries —
even tests sharing `[Collection(SharedContainerCollection.Name)]` end
up running in parallel against the same Elasticsearch instance.
Result: OcrIntegrationTests.ProcessMultipleDocuments_Concurrently
spam-indexes documents while
SearchIndexIntegrationTests.MultipleDocuments_SearchCorrectly is
polling for its own write to become visible. On a slow CI disk, the
write/refresh cycle stalls and the poll's 10s cap is reached — the
test fails deterministically with TaskCanceledException at exactly
10s 047ms (visible in run 25955174937).
Conservative scheduling keeps tests in the SharedContainer collection
sequential, eliminating the cross-test ES write contention. Local
13/13 integration tests now run in 37s (down from 57s under
aggressive — less contention even when nothing fails).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… slow single call
The polling helper used a single 10s linked-token across every SearchAsync
call. On GitHub-hosted runners the first search after index creation can
spend several seconds priming Lucene query caches even after the
preceding Refresh.True has returned — eating the budget and surfacing as
the deterministic 10s 047ms TaskCanceledException at line 90 of
SearchIndexIntegrationTests.MultipleDocuments_SearchCorrectly.
Two changes:
- bump the default overall timeout from 10s → 30s. CI runners are slower
than local dev machines and 10s was set when the suite was running on
beefier hardware. 30s is still tight for a happy-path search.
- catch OperationCanceledException inside the loop and exit cleanly so
the caller's final attempt (with the caller's own token) decides
pass/fail. Previously a cancelled poll surfaced as
TaskCanceledException at the helper boundary, hiding whether the doc
was actually missing or just not yet visible.
Local 5/5 SearchIndexIntegrationTests pass in 41s on ES 9.4.1.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…sync polls
SearchIndexService writes documents with Refresh.True (`?refresh=true`),
which the Elastic 9.x docs describe as forcing a refresh of the affected
shards before the index call returns. In practice, on GitHub-hosted
ubuntu-latest runners the per-document refresh has been observed to not
fully propagate before the first SearchAsync hits the index — the doc is
visible to a real-time GET (used by WaitForDocumentAsync, which passes
on CI) but invisible to _search (used by WaitForSearchResultsAsync,
which fails).
Calling client.Indices.RefreshAsync at the start of the polling helper
forces an explicit index-level refresh. It's idempotent: on local dev
machines where the per-document refresh already settled, this is a fast
no-op; on a slow CI runner it converts a deterministic empty-result
flake (MultipleDocuments_SearchCorrectly was failing at exactly
10s 047ms with TaskCanceledException, then post-timeout-bump at 30s
with "collection is empty") into a passing search.
The refresh failure is caught and ignored — the polling loop below is
the actual correctness boundary; the refresh is best-effort warmup.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
SearchIndexService.IndexDocumentAsync built the IndexRequestDescriptor
without an explicit `.Index(...)`, so writes used whatever DefaultIndex
the injected ElasticsearchClient was configured with — not the index
the service's own ElasticsearchOptions point to.
For production this is a no-op (one client, one default, same value as
options.Value.DefaultIndex). It is *not* a no-op when a test builds a
SearchIndexService SUT with its own options but reuses the shared
fixture's ElasticsearchClient. The SUT's InitializeAsync correctly
creates its per-test index with the keyword/text/date mapping; then
IndexAsync silently writes to the *fixture's* default index instead.
Concretely: alphabetical run order is
GenAiIntegrationTests → OcrIntegrationTests →
SearchIndexConcurrencyTests → SearchIndexIntegrationTests …
SearchIndexConcurrencyTests' SUT was the first writer to the fixture
index. ES auto-created it with dynamic mapping (`id` as text+keyword
subfield) before SearchIndexIntegrationTests.MultipleDocuments_Search…
got a chance to call its own InitializeAsync — and that one then
short-circuited on ExistsAsync==true, never applying the proper
`p.Keyword("id")` mapping. The downstream
`Filter(f => f.Term(t => t.Field("id").Value(helloId.ToString())))`
ran against a tokenised text field and matched nothing, surfacing as
"Expected collection to contain a single item, but the collection is
empty" at line 90.
Explicit `.Index(options.Value.DefaultIndex)` routes writes back to the
service-owned index. Knock-on: the secondary assertion in
SearchIndexConcurrencyTests.InitializeAsync_WhenIndexPreCreated_Skips…
relied on the old behaviour — the doc landing in the fixture index so
fixture.WaitForDocumentAsync could find it. With the fix, the doc now
lives in the SUT's pre-created index, so that assertion now queries
the SUT's index directly via ElasticClient.GetAsync (still verifying
the InitializeAsync-skips-create branch, just against the correct
index).
Local 13/13 PaperlessServices integration tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bumping to 9.4.1 regressed PaperlessServices integration tests from
12/13 passing to 0/26 passing with a 1h hang (some combination of the
new image's startup signature or default settings vs. our wait
strategy / xpack.security flags).
Locally on macOS+OrbStack, ES 9.1.3 plus the in-flight test fixes
(xpack.security.http.ssl.enabled=false, 30s search-poll budget,
explicit pre-wait refresh, conservative SharedContainer collection
serialization, explicit options.Value.DefaultIndex in IndexAsync)
runs the full integration suite green:
PaperlessREST.Tests: 59/59 passed (1m 18s)
PaperlessServices.Tests: 13/13 passed (45s) — includes the
previously-flaky MultipleDocuments_SearchCorrectly
Keep the other version bumps (Testcontainers 4.11, Elastic client 9.4,
EF Core 10.0.8 etc.) — they were not implicated.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…c fails
If StartAsync times out before _host is assigned, DisposeAsync throws NRE
on _host.StopAsync(), polluting test output with a cascade failure.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📜 Recent review details⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
**/*.cs: Do not qualify event, field, method, and property access with 'this.' in C# code
Use predefined types (like int, string) instead of BCL types (like Int32, String) in C#
Always use parentheses for clarity in arithmetic and relational binary operators in C#
Require explicit accessibility modifiers for non-interface members in C#
Prefer coalesce expressions, collection initializers, null propagation, and object initializers in C#
Prefer auto-properties and compound assignments in C#
Prefer conditional expressions over assignment and return statements in C#
Mark fields as readonly when possible in C#
Treat all unused parameters as code quality issues in C#
Use explicit types instead of 'var' in C# unless type is apparent
Prefer expression-bodied members for accessors, indexers, lambdas, methods, and properties in C#
Use pattern matching instead of 'as' with null checks and 'is' with cast checks in C#
Prefer switch expressions over traditional switch statements in C#
Use conditional delegate calls in C# to avoid null reference exceptions
Prefer static local functions over instance local functions in C#
Use modifier order: public, private, protected, internal, file, static, extern, new, virtual, abstract, sealed, override, readonly, unsafe, required, volatile, async in C#
Always use braces for code blocks in C#
Prefer file-scoped namespaces in C#
Prefer implicit object creation when type is apparent in C#
Prefer index and range operators in C#
Place 'using' directives outside of namespace declarations in C#
Always place opening brace on new line before catch, else, finally, and members in C#
Indent block contents and case contents in C#
Add space after comma, before and after binary operators, and after colon in inheritance clause in C#
Do not add space after dot, cast, or before open square brackets in C#
Name interfaces with 'I' prefix using PascalCase in C#
Name types (classes, structs, enums) using PascalCase in C#
Name non-field members (properties, events...
C#/TS/Python: Fail changes that introduce sync-over-async, unobserved fire-and-forget work, missing CancellationToken/AbortSignal propagation on public/internal async boundaries when the callee exposes a token/signal accepting overload, sleeps for synchronization, or resource disposal paths that can drop in-flight work. Pass missing propagation only when the available public dependency API exposes no cancellation-capable overload.
PaperlessServices/**/*.cs: Use ErrorOr result types for service layer methods instead of throwing exceptions
Use Microsoft.Extensions.Http.Resilience (Polly v8) for external API calls (e.g., Gemini); configure retry and circuit-breaker policies
Fixed missing field initialization in document search indexing
Chores
Updated Elasticsearch container version in test environments
Adjusted test execution strategy to conservative parallelization for improved stability
Enhanced test infrastructure with improved timeout handling and index management
Walkthrough
This PR updates Elasticsearch test infrastructure across six files: downgrading the container image from 9.4.1 to 9.1.3, disabling HTTP SSL in test containers, hardening test isolation by querying specific indexes, reworking polling with longer timeouts and cancellation safety, explicitly targeting indexes in the search service, and switching test parallelization from aggressive to conservative.
Changes
Elasticsearch integration stability and test correctness
Layer / File(s)
Summary
Elasticsearch container configuration and SSL alignment .env.test, PaperlessREST.Tests/Integration/SharedRestContainerFixture.cs, PaperlessServices.Tests/Integration/WorkerTestBase.cs
Elasticsearch Docker image pinned to 9.1.3 across environment config and test fixtures; HTTP SSL explicitly disabled to prevent Testcontainers wait-strategy TLS probing mismatches.
Test isolation and polling reliability hardening PaperlessServices.Tests/Integration/SearchIndexConcurrencyTests.cs, PaperlessServices.Tests/Integration/WorkerTestBase.cs
Test verification queries the specific isolated index via ElasticClient.GetAsync instead of fixture's shared index; WaitForSearchResultsAsync timeout increased to 30s with linked cancellation tokens, preemptive index refresh, and structured exception handling; DisposeAsync guarded against incomplete initialization.
Search service explicit index targeting PaperlessServices/Features/OcrProcessing/Infrastructure/Search/SearchIndexService.cs
IndexDocumentAsync explicitly sets target index via options.Value.DefaultIndex and initializes previously missing summary field in document payload.
Test execution parallelization strategy
Layer / File(s)
Summary
Test parallelization algorithm adjustment PaperlessServices.Tests/xunit.runner.json
parallelAlgorithm changed from "aggressive" to "conservative"; other runner settings unchanged.
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
ANcpLua/Paperless#23: Overlaps directly with this PR's updates to WorkerTestBase.cs regarding HTTP SSL disabling and WaitForSearchResultsAsync polling refactoring.
ANcpLua/Paperless#24: Overlaps with the test isolation fix to query the correct isolated index and the search service's explicit index targeting via options.Value.DefaultIndex.
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR stabilizes Elasticsearch-backed integration tests by pinning the Elasticsearch image, adjusting test runner behavior, improving search polling, and fixing a host disposal null path.
Changes:
Downgrades test Elasticsearch image defaults from 9.4.1 to 9.1.3 across test fixtures and .env.test.
Makes search indexing explicitly target the configured default index.
Improves PaperlessServices integration fixture reliability with HTTP SSL config, safer disposal, and longer/refresh-backed search polling.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
The PR successfully addresses several stability issues in the Elasticsearch integration suite, including a version revert to 9.1.3 and improvements to wait strategies. Codacy analysis indicates the changes are up to standards, with a net reduction in code clones. However, a production logic fix (explicitly setting the index name in SearchIndexService) is bundled with these test infrastructure changes, which complicates regression tracking. Additionally, while the DisposeAsync logic has been improved with a null-check for the host, it remains partially vulnerable to NullReferenceExceptions if other resources like RabbitMQ fail to initialize, potentially masking the root cause of CI failures.
About this PR
This PR bundles a production logic fix (explicitly passing the index name in SearchIndexService) with test infrastructure stabilization. It is generally recommended to separate production bug fixes from test refactors to ensure cleaner rollback paths and better visibility in change logs.
Test suggestions
Verify that SearchIndexService correctly uses the configured DefaultIndex when indexing documents.
Verify that WaitForSearchResultsAsync handles slow CI environments via its 30s timeout and manual refresh.
Verify that DisposeAsync does not throw a NullReferenceException if the test host or container resources fail to initialize.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify that DisposeAsync does not throw a NullReferenceException if the test host or container resources fail to initialize.
The reason will be displayed to describe this comment to others. Learn more.
🟡 MEDIUM RISK
Suggestion: Improve the resilience of the teardown process to ensure CI logs clearly reflect setup failures.
Wrap the host shutdown in a try-catch block so that if StopAsync fails, the execution still proceeds to container disposal.
Use null-conditional access for all disposable fields (e.g., _rabbit?.DisposeAsync()) to prevent NullReferenceExceptions during teardown if the test failed during the initialization of these specific components.
Closing: origin/main already has all fixes (ES 9.1.3, xpack.security.http.ssl.enabled=false, defensive DisposeAsync with best-effort catches) via PRs #23 and #24. This PR would regress DisposeAsync error-handling relative to main.
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
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.
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
Summary
ElasticsearchConfiguration.TlsEnableddetection, causing a 1-hour hang inSharedContainerFixture.InitializeAsync(run #25952721854).9.1.3(was working before the bump).xpack.security.http.ssl.enabled=falseso the built-in ES wait strategy probes HTTP, not HTTPS.DisposeAsyncNRE:_hostisnull!until assigned insideInitializeAsync; ifStartAsyncthrows first,DisposeAsyncwas crashing on_host.StopAsync().Changes
86b123fxpack.security.http.ssl.enabled=false— ES built-in wait uses HTTPbe2e216parallelAlgorithm: conservative— serialises SharedContainer collectiond2a65eeWaitForSearchResultsAsync30 s budget + ES indexRefreshbefore polls6ae05bcWaitForSearchResultsAsync910431boptions.Value.DefaultIndexinSearchIndexService.IndexAsync29584ce6eed1dcDisposeAsyncguard for_hostTest plan
PaperlessServices.Tests(integration): 13/13 passed locally in 44 s🤖 Generated with Claude Code