Skip to content

fix(tests): unblock CI by stopping Testcontainers' ES wait from probing HTTPS #23

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ANcpLua merged 4 commits into from
May 16, 2026
Merged

fix(tests): unblock CI by stopping Testcontainers' ES wait from probing HTTPS #23

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
86b123f
fix(tests): set xpack.security.http.ssl.enabled=false so Elasticsearc…
ANcpLua May 16, 2026
be2e216
fix(tests): use conservative parallel algorithm so SharedContainer co…
ANcpLua May 16, 2026
d2a65ee
fix(tests): give WaitForSearchResultsAsync a 30s budget and survive a…
ANcpLua May 16, 2026
6ae05bc
fix(tests): explicitly refresh the index before WaitForSearchResultsA…
ANcpLua May 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 50 additions & 10 deletions PaperlessServices.Tests/Integration/WorkerTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ public class SharedContainerFixture : IAsyncLifetime
Environment.GetEnvironmentVariable("ELASTIC_IMAGE") ?? DefaultElasticsearchImage)
.WithEnvironment("discovery.type", "single-node")
.WithEnvironment("xpack.security.enabled", "false")
// Required so Testcontainers' ElasticsearchConfiguration.TlsEnabled evaluates to false
// (it AND-s xpack.security.enabled with xpack.security.http.ssl.enabled). Without this,
// the built-in wait strategy probes HTTPS while ES listens on plain HTTP, and hangs.
.WithEnvironment("xpack.security.http.ssl.enabled", "false")
Comment on lines +35 to +38
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify Testcontainers.Elasticsearch TlsEnabled logic

# Search Testcontainers .NET repository for ElasticsearchConfiguration implementation
rg -nP --type=cs -A10 -B2 'class ElasticsearchConfiguration|TlsEnabled.*get' --iglob '*elasticsearch*'

Repository: ANcpLua/Paperless

Length of output: 43

🌐 Web query:

Testcontainers.Elasticsearch 4.11 ElasticsearchConfiguration TlsEnabled implementation

💡 Result:

In Testcontainers for.NET (including version 4.11.0), there is no explicit property named TlsEnabled within an ElasticsearchConfiguration class [1][2]. The Testcontainers Elasticsearch module automatically configures the container to use HTTPS by default for modern Elasticsearch versions (8.x and later), which are secure by default [2][3]. Because the container runs with TLS enabled,.NET clients attempting to connect will typically fail unless the client is configured to trust the self-signed certificate generated by the container [2]. To handle this in your.NET application, you should: 1. Create an ElasticsearchClientSettings instance [2]. 2. Set the ServerCertificateValidationCallback delegate to CertificateValidations.AllowAll to bypass certificate validation for your test environment [2]. Example implementation: var clientSettings = new ElasticsearchClientSettings(new Uri(_elasticsearchContainer.GetConnectionString)); clientSettings.ServerCertificateValidationCallback(CertificateValidations.AllowAll); var client = new ElasticsearchClient(clientSettings); If you need to disable security (and thus TLS) for specific testing scenarios, you can typically do so by modifying the environment variables of the container (e.g., setting xpack.security.enabled to false), though this is generally discouraged in favor of proper certificate handling [3][4][5]. Recent updates to the.NET module have improved support for handling these configurations, including returning appropriate HTTP/HTTPS connection strings based on the security state [4].

Citations:

Remove the inaccurate comment about ElasticsearchConfiguration.TlsEnabled and provide a correct explanation.

The comment references ElasticsearchConfiguration.TlsEnabled and describes an "AND" logic between two environment variables that does not match Testcontainers.Elasticsearch 4.11 implementation. TlsEnabled is not an explicit property that evaluates based on those flags. The environment variables set here control Elasticsearch security configuration directly. Replace the comment with a simpler, accurate explanation:

// xpack.security.http.ssl.enabled=false disables HTTPS for Elasticsearch.
// Without this, the built-in wait strategy probes HTTPS while ES listens on HTTP (due to xpack.security.enabled=false), causing hangs.
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@PaperlessServices.Tests/Integration/WorkerTestBase.cs` around lines 35 - 38,
Remove the inaccurate reference to ElasticsearchConfiguration.TlsEnabled and
replace the existing comment above the
.WithEnvironment("xpack.security.http.ssl.enabled", "false") call with a
concise, accurate explanation that setting xpack.security.http.ssl.enabled=false
disables HTTPS for Elasticsearch and that without it the wait strategy may probe
HTTPS while Elasticsearch is listening on HTTP (when
xpack.security.enabled=false), which causes the startup hang; keep the
.WithEnvironment("xpack.security.http.ssl.enabled", "false") line unchanged.

.WithEnvironment("ES_JAVA_OPTS", "-Xms512m -Xmx512m")
.Build();

Expand Down Expand Up @@ -188,27 +192,63 @@ public async Task<SearchResponse<T>> WaitForSearchResultsAsync<T>(
TimeSpan? timeout = null,
TimeSpan? pollInterval = null)
{
timeout ??= TimeSpan.FromSeconds(10);
// 30s overall budget: GitHub-hosted runners are markedly slower than local
// dev machines and the first SearchAsync after index creation can spend
// several seconds priming query caches even after Refresh.True returns.
timeout ??= TimeSpan.FromSeconds(30);
Comment on lines +195 to +198
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Timeout contract is no longer enforced after loop exit.

The method states a 30s overall budget, but the final SearchAsync uses only cancellationToken, so it can run past the budget and recreate CI tail latency/hangs under ES slowness.

Proposed fix (preserve “final response” semantics without exceeding budget) 
 public async Task<SearchResponse<T>> WaitForSearchResultsAsync<T>(
 	Action<SearchRequestDescriptor<T>> configureSearch,
 	CancellationToken cancellationToken,
 	TimeSpan? timeout = null,
 	TimeSpan? pollInterval = null)
 {
 	// 30s overall budget: GitHub-hosted runners are markedly slower than local
 	// dev machines and the first SearchAsync after index creation can spend
 	// several seconds priming query caches even after Refresh.True returns.
 	timeout ??= TimeSpan.FromSeconds(30);
 	pollInterval ??= TimeSpan.FromMilliseconds(100);

 	ElasticsearchClient client = Services.GetRequiredService<ElasticsearchClient>();
 	using CancellationTokenSource overallCts = new(timeout.Value);
 	using CancellationTokenSource overallLinked =
 		CancellationTokenSource.CreateLinkedTokenSource(overallCts.Token, cancellationToken);
+	SearchResponse<T>? lastResponse = null;

 	while (!overallLinked.Token.IsCancellationRequested)
 	{
 		try
 		{
 			SearchResponse<T> response = await client.SearchAsync<T>(configureSearch, overallLinked.Token);
+			lastResponse = response;

 			if (response.Documents.Count > 0)
 			{
 				return response;
 			}
 		}
 		catch (OperationCanceledException) when (overallLinked.Token.IsCancellationRequested)
 		{
 			break;
 		}

 		try
 		{
 			await Task.Delay(pollInterval.Value, overallLinked.Token);
 		}
 		catch (OperationCanceledException)
 		{
 			break;
 		}
 	}

-	// Final attempt with the caller's token only so the assertion sees real
-	// "found nothing" data rather than a TaskCanceledException at the wait boundary.
-	return await client.SearchAsync<T>(configureSearch, cancellationToken);
+	// Return the most recent real response if available, keeping the overall budget strict.
+	if (lastResponse is not null)
+	{
+		return lastResponse;
+	}
+
+	// If cancellation came from caller, preserve that behavior.
+	cancellationToken.ThrowIfCancellationRequested();
+	throw new TimeoutException("Timed out waiting for Elasticsearch search results.");
 }

Also applies to: 232-234

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@PaperlessServices.Tests/Integration/WorkerTestBase.cs` around lines 195 -
198, The final SearchAsync call can run beyond the intended overall timeout
because it only uses cancellationToken; fix by enforcing the timeout for the
entire method: create a CancellationTokenSource with the computed timeout
(timeout ??= TimeSpan.FromSeconds(30)) and link it with the incoming
cancellationToken (e.g., using CancellationTokenSource.CreateLinkedTokenSource),
then pass the linked token into the final SearchAsync calls instead of plain
cancellationToken; apply the same change to the other occurrences mentioned (the
similar SearchAsync at the 232–234 region) to preserve the "final response"
semantics while ensuring the 30s budget is honored.

pollInterval ??= TimeSpan.FromMilliseconds(100);

ElasticsearchClient client = Services.GetRequiredService<ElasticsearchClient>();
using CancellationTokenSource cts = new(timeout.Value);
using CancellationTokenSource linked =
CancellationTokenSource.CreateLinkedTokenSource(cts.Token, cancellationToken);
using CancellationTokenSource overallCts = new(timeout.Value);
using CancellationTokenSource overallLinked =
CancellationTokenSource.CreateLinkedTokenSource(overallCts.Token, cancellationToken);
Comment on lines +195 to +204

// Force an index-level refresh up front. SearchIndexService writes documents
// with Refresh.True (`?refresh=true`), which is supposed to guarantee
// immediate searchability — but on slow CI disks the per-document refresh
// is observed to not always propagate before the first SearchAsync. The
// explicit Indices.RefreshAsync here is defensive and idempotent: locally
// it's a no-op (everything's already refreshed), on CI it converts an
// invisible flake into a passing search.
try
{
await client.Indices.RefreshAsync(
r => r.Indices(client.ElasticsearchClientSettings.DefaultIndex),
overallLinked.Token);
}
catch (OperationCanceledException) when (overallLinked.Token.IsCancellationRequested)
{
// Fall through to the final attempt below.
}

while (!linked.Token.IsCancellationRequested)
while (!overallLinked.Token.IsCancellationRequested)
{
SearchResponse<T> response = await client.SearchAsync<T>(configureSearch, linked.Token);
try
{
SearchResponse<T> response = await client.SearchAsync<T>(configureSearch, overallLinked.Token);

if (response.Documents.Count > 0)
if (response.Documents.Count > 0)
{
return response;
}
}
catch (OperationCanceledException) when (overallLinked.Token.IsCancellationRequested)
{
return response;
break;
}

await Task.Delay(pollInterval.Value, linked.Token);
try
{
await Task.Delay(pollInterval.Value, overallLinked.Token);
}
catch (OperationCanceledException)
{
break;
}
}

// Final attempt
// Final attempt with the caller's token only so the assertion sees real
// "found nothing" data rather than a TaskCanceledException at the wait boundary.
return await client.SearchAsync<T>(configureSearch, cancellationToken);
Comment on lines +250 to 252
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Honor polling timeout on final Elasticsearch search

When the 30s budget expires during SearchAsync, the new OperationCanceledException handler breaks out of the loop and then issues a final SearchAsync with only the caller token. If the caller token is still active, this extra request is no longer bounded by the polling timeout and can block for the transport/request timeout, so a method documented as waiting "until results are found or timeout occurs" can run much longer than timeout in failing CI scenarios.

Useful? React with 👍 / 👎.

}

Expand Down
2 changes: 1 addition & 1 deletion PaperlessServices.Tests/xunit.runner.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"methodDisplay": "classAndMethod",
"methodDisplayOptions": "replaceUnderscoreWithSpace,useOperatorMonikers",
"parallelizeTestCollections": true,
"parallelAlgorithm": "aggressive",
"parallelAlgorithm": "conservative",
"maxParallelThreads": "4x",
Comment on lines 6 to 8
"stopOnFail": false,
"failSkips": false,
Expand Down
Loading