Skip to content

chore: Don't create a static field in a generic class #1555

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

Jump to bottom
Merged
merged 6 commits into from
Oct 18, 2025
Merged

chore: Don't create a static field in a generic class #1555

merged 6 commits into from
Oct 18, 2025

Conversation

@HofmeisterAn
Copy link
Collaborator

@HofmeisterAn HofmeisterAn commented Oct 17, 2025 •
edited
Loading

What does this PR do?

This PR removes a minor code smell. It's generally not recommended to create static fields or properties in a generic class. Typically, a wrapper class is used instead to avoid creating multiple instances.

Edit: While looking into the flaky behavior, I noticed it's a known issue. When the port forwarding container starts, Testcontainers automatically adds the extra host to the builder configuration, which causes the hash to change. The PR pins the extra host to a fixed value. This seems to be a general limitation with reuse.

Why is it important?

-

Related issues

-

@HofmeisterAn HofmeisterAn added the chore A change that doesn't impact the existing functionality, e.g. internal refactorings or cleanups label Oct 17, 2025
@HofmeisterAn HofmeisterAn marked this pull request as ready for review October 17, 2025 14:53
@netlify
Copy link

netlify bot commented Oct 17, 2025 •
edited
Loading

✅ Deploy Preview for testcontainers-dotnet ready!

Name Link
🔨 Latest commit 9cf3543
🔍 Latest deploy log https://app.netlify.com/projects/testcontainers-dotnet/deploys/68f362af60d298000807da68
😎 Deploy Preview https://deploy-preview-1555--testcontainers-dotnet.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link

coderabbitai bot commented Oct 17, 2025 •
edited
Loading

Summary by CodeRabbit

  • Chores

    • Improved internal JSON serialization configuration architecture.
    • Refactored container resource hashing to use centralized serialization settings.

  • Tests

    • Updated test naming and structure for enhanced clarity.
    • Refined container wait strategies in test suite.
    • Improved test setup to better reflect real-world scenarios.

Walkthrough

Adds an internal shared JsonSerializerOptions (with JsonOrderedKeysConverter) and switches ResourceConfiguration.GetReuseHash to use it; reformats a label initializer; renames and adjusts two reuse-hash tests (one now asserts a known Base64 SHA‑1).

Changes

Cohort / File(s) Summary
JSON serialization config
src/Testcontainers/Configurations/Commons/DefaultJsonSerializerOptions.cs		 New internal static class DefaultJsonSerializerOptions exposing public static JsonSerializerOptions Instance { get; }, initialized in a static constructor which adds JsonOrderedKeysConverter to Instance.Converters.
Resource configuration
src/Testcontainers/Configurations/Commons/ResourceConfiguration.cs		 Removed a local static JsonSerializerOptions and its static ctor; GetReuseHash now serializes using DefaultJsonSerializerOptions.Instance.
Label initializer formatting
src/Testcontainers/Configurations/Commons/JsonIgnoreRuntimeResourceLabels.cs		 Reformatted IgnoreLabels initialization from single-line to multi-line trailing-comma style; no behavioral change.
Tests: reuse-hash behavior & names
tests/Testcontainers.Platform.Linux.Tests/ReusableResourceTest.cs		 Renamed tests: ForSameConfigurationCreatedInDifferentOrder → ForKnownConfiguration, ForGivenConfiguration → ForSameConfigurationInDifferentOrder; one test now asserts a fixed Base64 SHA‑1 for a known configuration, the other verifies order-insensitive hash equality; test builder init behavior adjusted.
Wait strategy test
tests/Testcontainers.Platform.Linux.Tests/WaitStrategyModeTest.cs		 Replaced log-message wait with a custom IWaitUntil implementation that waits for container Exited state; removed related message/entrypoint setup.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor Caller as ResourceConfiguration.GetReuseHash
    participant DefaultOpts as DefaultJsonSerializerOptions.Instance
    participant JsonSerializer as System.Text.Json
    participant JsonConverter as JsonOrderedKeysConverter
    participant Hasher as SHA1/Base64

    Caller->>DefaultOpts: Request configured JsonSerializerOptions
    DefaultOpts-->>Caller: Return Instance (with JsonOrderedKeysConverter)
    Caller->>JsonSerializer: Serialize(payload, Instance)
    JsonSerializer->>JsonConverter: Apply ordered-keys conversion (deterministic)
    JsonConverter-->>JsonSerializer: Ordered JSON
    JsonSerializer-->>Caller: JSON bytes/string
    Caller->>Hasher: Compute SHA-1 -> Base64
    Hasher-->>Caller: Reuse-hash
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐇 I sorted keys with gentle paws,

I spun the hash without a pause,
Tests renamed, a tidy song,
Base64 hums the whole day long,
I hop away — order kept and strong.

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The PR description is incomplete against the repository template. While the "What does this PR do?" section is present and filled, the mandatory "Why is it important?" section contains only "-" and is essentially empty. Additionally, the recommended "How to test this PR" section is not filled. The lack of a rationale explanation leaves reviewers without understanding why this change matters beyond the surface-level code smell removal mentioned in the changeset summary. Overall, the description lacks the completeness expected for a PR of this complexity.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The title "chore: Don't create a static field in a generic class" accurately and concisely describes the main architectural change in this PR. The changeset demonstrates this principle by introducing a wrapper class (DefaultJsonSerializerOptions) and moving the static JsonSerializerOptions field out of a generic class into this separate wrapper, which is then used throughout the codebase. The title clearly communicates the pattern being addressed and the primary refactoring intent, making it easy for reviewers to understand the core purpose of the PR.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch bugfix/remove-code-smell

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@HofmeisterAn HofmeisterAn changed the title fix: Don't create a static field in a generic class chore: Don't create a static field in a generic class Oct 17, 2025
coderabbitai[bot]
coderabbitai bot reviewed Oct 17, 2025
View reviewed changes
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d6cd3dc and 5838d75.

📒 Files selected for processing (4)
  • src/Testcontainers/Configurations/Commons/DefaultJsonSerializerOptions.cs (1 hunks)
  • src/Testcontainers/Configurations/Commons/JsonIgnoreRuntimeResourceLabels.cs (1 hunks)
  • src/Testcontainers/Configurations/Commons/ResourceConfiguration.cs (1 hunks)
  • tests/Testcontainers.Platform.Linux.Tests/ReusableResourceTest.cs (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/Testcontainers/Configurations/Commons/DefaultJsonSerializerOptions.cs (1)
src/Testcontainers/Configurations/Commons/JsonOrderedKeysConverter.cs (1)
  • JsonOrderedKeysConverter (9-32)
src/Testcontainers/Configurations/Commons/ResourceConfiguration.cs (1)
src/Testcontainers/Configurations/Commons/DefaultJsonSerializerOptions.cs (2)
  • DefaultJsonSerializerOptions (5-13)
  • DefaultJsonSerializerOptions (7-10)
src/Testcontainers/Configurations/Commons/JsonIgnoreRuntimeResourceLabels.cs (1)
src/Testcontainers/Clients/TestcontainersClient.cs (3)
  • TestcontainersClient (21-433)
  • TestcontainersClient (47-57)
  • TestcontainersClient (59-75)
tests/Testcontainers.Platform.Linux.Tests/ReusableResourceTest.cs (2)
src/Testcontainers/Configurations/Commons/ResourceConfiguration.cs (1)
  • GetReuseHash (89-101)
src/Testcontainers/Configurations/Commons/IResourceConfiguration.cs (1)
  • GetReuseHash (48-48)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (116)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
  • GitHub Check: ci (Testcontainers.Qdrant, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.PostgreSql, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Pulsar, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.PubSub, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Platform.Windows, windows-2022)
  • GitHub Check: ci (Testcontainers.OpenSearch, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
  • GitHub Check: ci (Testcontainers.Qdrant, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.PostgreSql, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Pulsar, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.PubSub, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Platform.Windows, windows-2022)
  • GitHub Check: ci (Testcontainers.OpenSearch, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
  • GitHub Check: ci (Testcontainers.Qdrant, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.PostgreSql, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Pulsar, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.PubSub, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Platform.Windows, windows-2022)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
  • GitHub Check: ci (Testcontainers.Qdrant, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.PostgreSql, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Pulsar, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.PubSub, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Platform.Windows, windows-2022)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
  • GitHub Check: ci (Testcontainers.Qdrant, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.PostgreSql, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Pulsar, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.PubSub, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Platform.Windows, windows-2022)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
  • GitHub Check: ci (Testcontainers.Qdrant, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.PostgreSql, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Pulsar, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.PubSub, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Platform.Windows, windows-2022)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
  • GitHub Check: ci (Testcontainers.Qdrant, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.PostgreSql, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Pulsar, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.PubSub, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Platform.Windows, windows-2022)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
  • GitHub Check: ci (Testcontainers.Qdrant, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.PostgreSql, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Pulsar, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.PubSub, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Platform.Windows, windows-2022)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
  • GitHub Check: ci (Testcontainers.Qdrant, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.PostgreSql, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Pulsar, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.PubSub, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Platform.Windows, windows-2022)
  • GitHub Check: ci (Testcontainers.CockroachDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
  • GitHub Check: ci (Testcontainers.RabbitMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Qdrant, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.PostgreSql, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Pulsar, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.PubSub, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Platform.Windows, windows-2022)
🔇 Additional comments (4)
src/Testcontainers/Configurations/Commons/JsonIgnoreRuntimeResourceLabels.cs (1)

11-16: LGTM! Formatting improvement enhances readability.

The multi-line format with trailing commas is more maintainable and follows common C# conventions.

src/Testcontainers/Configurations/Commons/ResourceConfiguration.cs (1)

91-91: LGTM! Successfully addresses the static-in-generic code smell.

By using DefaultJsonSerializerOptions.Instance instead of a static field in this generic class, the PR correctly eliminates the issue where each generic instantiation would create its own static field instance.

tests/Testcontainers.Platform.Linux.Tests/ReusableResourceTest.cs (2)

107-146: LGTM! Improved test structure with deterministic hash validation.

The test improvements are excellent:

  • Clearer method name that describes what is being tested
  • Given/When/Then structure enhances readability
  • Hardcoded hash value ensures serialization stability for the reuse feature
  • Comprehensive JSON documentation clarifies the expected payload

The deterministic hash assertion is valuable for detecting unintended serialization changes that would break container reuse functionality.

150-176: LGTM! Well-structured test for order-independent hashing.

The refactored test effectively validates that configuration order doesn't affect the reuse hash:

  • Clear Given/When/Then structure
  • Tests both environment variables and labels in different orders
  • Descriptive naming matches the test's intent

coderabbitai[bot]
coderabbitai bot reviewed Oct 17, 2025
View reviewed changes
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (1)
src/Testcontainers/Configurations/Commons/DefaultJsonSerializerOptions.cs (1)

12-12: Mutability concern previously raised.

The exposed mutable JsonSerializerOptions singleton can be modified by any consumer, potentially affecting all serialization operations. This concern was already raised in a previous review with suggested mitigations (immutability documentation or defensive copying).

🧹 Nitpick comments (1)
src/Testcontainers/Configurations/Commons/DefaultJsonSerializerOptions.cs (1)

12-12: Consider making the Instance property internal for consistency.

The class is internal, but the property is public. While this has no practical effect (the property is effectively internal due to the containing class), making it explicitly internal would improve consistency and clarity.

-    public static JsonSerializerOptions Instance { get; } = new JsonSerializerOptions();
+    internal static JsonSerializerOptions Instance { get; } = new JsonSerializerOptions();
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5838d75 and 6dec98e.

📒 Files selected for processing (1)
  • src/Testcontainers/Configurations/Commons/DefaultJsonSerializerOptions.cs (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/Testcontainers/Configurations/Commons/DefaultJsonSerializerOptions.cs (1)
src/Testcontainers/Configurations/Commons/JsonOrderedKeysConverter.cs (1)
  • JsonOrderedKeysConverter (9-32)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (60)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
🔇 Additional comments (1)
src/Testcontainers/Configurations/Commons/DefaultJsonSerializerOptions.cs (1)

5-13: Good solution to the generic class static field code smell.

Moving the static JsonSerializerOptions from a generic class into this dedicated non-generic wrapper class correctly addresses the PR objective and follows best practices.

The initialization order is correct: the property initializer on line 12 executes before the static constructor body on lines 7-10, ensuring the converter is properly registered.

coderabbitai[bot]
coderabbitai bot reviewed Oct 17, 2025
View reviewed changes
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/Testcontainers/Configurations/Commons/DefaultJsonSerializerOptions.cs (1)

12-13: Consider documenting the shared-instance constraint.

The Instance property exposes a mutable singleton that could be modified by internal consumers. While the internal scope mitigates the risk, adding a brief XML doc comment warning against modification would help prevent future maintainers from inadvertently altering the shared configuration.

Example documentation:

+  /// <summary>
+  /// Shared JSON serializer options for Testcontainers.
+  /// Do not modify; changes affect all consumers.
+  /// </summary>
   public static JsonSerializerOptions Instance { get; }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6dec98e and 4e90748.

📒 Files selected for processing (1)
  • src/Testcontainers/Configurations/Commons/DefaultJsonSerializerOptions.cs (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/Testcontainers/Configurations/Commons/DefaultJsonSerializerOptions.cs (1)
src/Testcontainers/Configurations/Commons/JsonOrderedKeysConverter.cs (1)
  • JsonOrderedKeysConverter (9-32)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (52)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ClickHouse, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ClickHouse, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ClickHouse, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
🔇 Additional comments (1)
src/Testcontainers/Configurations/Commons/DefaultJsonSerializerOptions.cs (1)

5-14: Good refactoring to eliminate the code smell.

The new static class successfully addresses the PR objective by moving the shared JsonSerializerOptions out of a generic class into a dedicated holder. The internal accessibility appropriately limits the scope while allowing reuse across the library.

@HofmeisterAn HofmeisterAn reopened this Oct 17, 2025
coderabbitai[bot]
coderabbitai bot reviewed Oct 17, 2025
View reviewed changes
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4e90748 and a7bcb69.

📒 Files selected for processing (1)
  • tests/Testcontainers.Platform.Linux.Tests/ReusableResourceTest.cs (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/Testcontainers.Platform.Linux.Tests/ReusableResourceTest.cs (2)
src/Testcontainers/Configurations/Commons/ResourceConfiguration.cs (1)
  • GetReuseHash (89-101)
src/Testcontainers/Configurations/Commons/IResourceConfiguration.cs (1)
  • GetReuseHash (48-48)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (70)
  • GitHub Check: ci (Testcontainers.ClickHouse, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CockroachDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Couchbase, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CosmosDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CouchDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Consul, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
  • GitHub Check: ci (Testcontainers.ClickHouse, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CockroachDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Couchbase, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CosmosDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CouchDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Consul, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
  • GitHub Check: ci (Testcontainers.ClickHouse, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CockroachDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Couchbase, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CosmosDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CouchDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Consul, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
  • GitHub Check: ci (Testcontainers.ClickHouse, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CockroachDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Couchbase, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CosmosDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CouchDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Consul, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
  • GitHub Check: ci (Testcontainers.ClickHouse, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CockroachDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Couchbase, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CosmosDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CouchDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Consul, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
  • GitHub Check: ci (Testcontainers.ClickHouse, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CockroachDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Couchbase, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CosmosDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CouchDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Consul, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
  • GitHub Check: ci (Testcontainers.ClickHouse, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CockroachDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Couchbase, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CosmosDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CouchDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Consul, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
  • GitHub Check: ci (Testcontainers.ClickHouse, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CockroachDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Couchbase, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CosmosDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CouchDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Consul, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
  • GitHub Check: ci (Testcontainers.ClickHouse, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CockroachDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Couchbase, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CosmosDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CouchDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Consul, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
  • GitHub Check: ci (Testcontainers.ClickHouse, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CockroachDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Couchbase, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CosmosDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CouchDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Consul, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
🔇 Additional comments (1)
tests/Testcontainers.Platform.Linux.Tests/ReusableResourceTest.cs (1)

107-154: Recognize this brittle test is an intentional regression canary; reframe concerns accordingly.

The hardcoded hash serves as a regression test—it should break when serialization changes, alerting maintainers to unintended modifications of the reuse mechanism. This brittleness is intentional, not a flaw.

The design is well-supported:

  • JsonOrderedKeysConverter (in DefaultJsonSerializerOptions) ensures consistent key ordering, reducing spurious hash changes
  • ForSameConfigurationInDifferentOrder test validates order-independence
  • Git history shows active management: recent commits include "fix: Generate consistent reuse hashes by sorting dictionary keys"

However, valid trade-offs remain:

  • The test requires updating the hardcoded hash whenever the serialization format genuinely changes
  • The WithExtraHost workaround couples the test to port forwarding infrastructure (documented in lines 3-9 as necessary to prevent flakiness)—only the first test includes it; the second test does not, maintaining cleaner isolation

For maintainability: Keep this test as a regression canary, but ensure changes to serialization logic update the hash value deliberately and re-verify the JSON structure in comments remains accurate.

@HofmeisterAn
Copy link
Collaborator Author

HofmeisterAn commented Oct 17, 2025 •
edited
Loading

… the extra hosts are not a dictionary. Overriding Init() or not calling it should work 😬.

coderabbitai[bot]
coderabbitai bot reviewed Oct 18, 2025
View reviewed changes
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
tests/Testcontainers.Platform.Linux.Tests/WaitStrategyModeTest.cs (1)

68-74: Extract duplicated WaitUntil class to shared test helper.

This WaitUntil implementation is duplicated in tests/Testcontainers.Platform.Linux.Tests/PortForwardingTest.cs (lines 120-126). Consider extracting it to a shared test helper class or making it internal in a common test utilities namespace to eliminate duplication and improve maintainability.

Example approach: Create a shared TestWaitStrategies class in a common test utilities namespace:

// In a shared test utilities file
internal static class TestWaitStrategies
{
    internal sealed class WaitUntilExited : IWaitUntil
    {
        public Task<bool> UntilAsync(IContainer container)
        {
            return Task.FromResult(TestcontainersStates.Exited.Equals(container.State));
        }
    }
}

Then reference it from both test files:

-    private sealed class WaitUntil : IWaitUntil
-    {
-        public Task<bool> UntilAsync(IContainer container)
-        {
-            return Task.FromResult(TestcontainersStates.Exited.Equals(container.State));
-        }
-    }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between aa603b3 and 9cf3543.

📒 Files selected for processing (1)
  • tests/Testcontainers.Platform.Linux.Tests/WaitStrategyModeTest.cs (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/Testcontainers.Platform.Linux.Tests/WaitStrategyModeTest.cs (3)
tests/Testcontainers.Platform.Linux.Tests/PortForwardingTest.cs (3)
  • WaitUntil (121-127)
  • Task (109-118)
  • Task (123-126)
src/Testcontainers.MongoDb/MongoDbBuilder.cs (4)
  • Task (161-184)
  • Task (203-212)
  • Task (221-224)
  • Task (227-233)
src/Testcontainers/Configurations/WaitStrategies/WaitStrategy.cs (2)
  • Task (102-105)
  • Task (113-116)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (56)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ClickHouse, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ClickHouse, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ClickHouse, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ClickHouse, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CockroachDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ClickHouse, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Consul, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CockroachDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ClickHouse, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Consul, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CockroachDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ClickHouse, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Consul, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CockroachDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ClickHouse, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Consul, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CockroachDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ClickHouse, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
🔇 Additional comments (1)
tests/Testcontainers.Platform.Linux.Tests/WaitStrategyModeTest.cs (1)

11-11: LGTM: Wait strategy configuration is appropriate.

The custom wait strategy correctly applies the mode configuration and uses the new WaitUntil implementation to wait for container exit.

@HofmeisterAn HofmeisterAn merged commit 1cfac50 into develop Oct 18, 2025
140 checks passed
@HofmeisterAn HofmeisterAn deleted the bugfix/remove-code-smell branch October 18, 2025 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

Assignees

No one assigned

Labels

chore A change that doesn't impact the existing functionality, e.g. internal refactorings or cleanups

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@HofmeisterAn @0xced