feat: Add Toxiproxy module #1454
Conversation
# Conflicts: # Directory.Packages.props # Testcontainers.sln
✅ Deploy Preview for testcontainers-dotnet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
HofmeisterAn
added
module
HofmeisterAn
force-pushed
the
develop
branch
3 times, most recently
from
4900ecd to
8fa5f1b
Compare
Summary by CodeRabbitRelease Notes
WalkthroughAdds a new Testcontainers.Toxiproxy module (projects, builder/configuration/container types), documentation and mkdocs entry, solution and package updates, a new integration test project with latency-toxic tests and CI config, plus a small GrafanaContainer cleanup. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer
participant Builder as ToxiproxyBuilder
participant Container as ToxiproxyContainer
participant Docker as Docker Engine
participant Client as Toxiproxy.Net Client
Dev->>Builder: new ToxiproxyBuilder()
Builder->>Builder: Init() — set image, control port, proxied ports, wait strategy
Dev->>Builder: Configure proxies (name, listen, upstream)
Dev->>Builder: Build()
Builder->>Container: Create(configuration)
Dev->>Container: StartAsync()
Container->>Docker: Start container
Docker-->>Container: Running
Container->>Client: Connect to control port, create proxies
Client-->>Container: Proxies ready
Dev->>Container: Apply toxic / Measure via mapped proxied port
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
src/Testcontainers.Toxiproxy/ToxiproxyBuilder.cs (2)
28-32: Simplify constructor initialization pattern.The current pattern calls a private constructor with an empty list, then reassigns
DockerResourceConfigurationfromInit(). This is confusing becauseInit()returns a builder instance (potentially a new one due to method chaining), and extracting only itsDockerResourceConfigurationis indirect.Consider simplifying to:
public ToxiproxyBuilder() - : this(new ToxiproxyConfiguration(), new List<Proxy>()) + : base(new ToxiproxyConfiguration()) { DockerResourceConfiguration = Init().DockerResourceConfiguration; }This makes the initialization flow clearer by directly calling the base constructor.
88-92: Add validation for proxy configurations.The
Validate()method only checks thatDockerResourceConfigurationis not null. Consider adding validation for the_initialProxieslist to catch configuration errors early:
- Check for duplicate proxy names
- Validate listen and upstream address formats
- Ensure proxy names are not empty
Example validation:
protected override void Validate() { base.Validate(); _ = Guard.Argument(DockerResourceConfiguration, nameof(DockerResourceConfiguration)).NotNull(); + + var duplicateNames = _initialProxies.GroupBy(p => p.Name).Where(g => g.Count() > 1).Select(g => g.Key); + if (duplicateNames.Any()) + { + throw new ArgumentException($"Duplicate proxy names found: {string.Join(", ", duplicateNames)}"); + } + + foreach (var proxy in _initialProxies) + { + _ = Guard.Argument(proxy.Name, nameof(proxy.Name)).NotNull().NotEmpty(); + _ = Guard.Argument(proxy.Listen, nameof(proxy.Listen)).NotNull().NotEmpty(); + _ = Guard.Argument(proxy.Upstream, nameof(proxy.Upstream)).NotNull().NotEmpty(); + } }tests/Testcontainers.Toxiproxy.Tests/ToxiproxyContainerTest.cs (1)
438-445: Note: GetFreePort has a TOCTOU race condition.Between stopping the listener (line 443) and the caller using the returned port, another process could bind to that port. This is a known limitation of the "get free port" pattern and is generally acceptable for tests, though it can occasionally cause flaky test failures in CI environments with high parallelism.
If flakiness becomes an issue, consider allowing the OS to assign ports by using port 0 in bindings where possible, or implementing retry logic around connection attempts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
Directory.Packages.props(1 hunks)Testcontainers.sln(6 hunks)docs/modules/index.md(1 hunks)docs/modules/toxiproxy.md(1 hunks)src/Testcontainers.Toxiproxy/.editorconfig(1 hunks)src/Testcontainers.Toxiproxy/Testcontainers.Toxiproxy.csproj(1 hunks)src/Testcontainers.Toxiproxy/ToxiproxyBuilder.cs(1 hunks)src/Testcontainers.Toxiproxy/ToxiproxyConfiguration.cs(1 hunks)src/Testcontainers.Toxiproxy/ToxiproxyContainer.cs(1 hunks)src/Testcontainers.Toxiproxy/Usings.cs(1 hunks)tests/Testcontainers.Toxiproxy.Tests/.editorconfig(1 hunks)tests/Testcontainers.Toxiproxy.Tests/.runs-on(1 hunks)tests/Testcontainers.Toxiproxy.Tests/Testcontainers.Toxiproxy.Tests.csproj(1 hunks)tests/Testcontainers.Toxiproxy.Tests/ToxiproxyContainerTest.cs(1 hunks)tests/Testcontainers.Toxiproxy.Tests/Usings.cs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-08T09:04:00.026Z
Learnt from: HofmeisterAn
Repo: testcontainers/testcontainers-dotnet PR: 1509
File: tests/Testcontainers.Grafana.Tests/GrafanaContainerTest.cs:45-46
Timestamp: 2025-11-08T09:04:00.026Z
Learning: In xUnit.net test methods in the testcontainers-dotnet project, ConfigureAwait(true) is recommended and correct to use, as it ensures proper synchronization context capture for xUnit's test parallelism management. Do not suggest changing ConfigureAwait(true) to ConfigureAwait(false) in test methods.
Applied to files:
tests/Testcontainers.Toxiproxy.Tests/ToxiproxyContainerTest.cs
🧬 Code graph analysis (5)
src/Testcontainers.Toxiproxy/ToxiproxyContainer.cs (3)
src/Testcontainers.Toxiproxy/ToxiproxyBuilder.cs (10)
PublicAPI(6-112)ToxiproxyContainer(44-48)ToxiproxyBuilder(17-22)ToxiproxyBuilder(28-32)ToxiproxyBuilder(34-38)ToxiproxyBuilder(54-65)ToxiproxyBuilder(74-85)ToxiproxyBuilder(95-98)ToxiproxyBuilder(101-104)ToxiproxyBuilder(107-111)src/Testcontainers.Toxiproxy/ToxiproxyConfiguration.cs (5)
PublicAPI(4-25)ToxiproxyConfiguration(7-9)ToxiproxyConfiguration(11-14)ToxiproxyConfiguration(16-19)ToxiproxyConfiguration(21-24)tests/Testcontainers.Toxiproxy.Tests/ToxiproxyContainerTest.cs (4)
Task(11-14)Task(17-20)Task(326-351)Task(412-436)
src/Testcontainers.Toxiproxy/Usings.cs (1)
src/Testcontainers/Logging.cs (1)
Logging(10-302)
src/Testcontainers.Toxiproxy/ToxiproxyConfiguration.cs (2)
src/Testcontainers.Toxiproxy/ToxiproxyBuilder.cs (1)
PublicAPI(6-112)src/Testcontainers.Toxiproxy/ToxiproxyContainer.cs (1)
PublicAPI(9-62)
src/Testcontainers.Toxiproxy/ToxiproxyBuilder.cs (2)
src/Testcontainers.Toxiproxy/ToxiproxyContainer.cs (2)
PublicAPI(9-62)ToxiproxyContainer(21-26)src/Testcontainers.Toxiproxy/ToxiproxyConfiguration.cs (5)
PublicAPI(4-25)ToxiproxyConfiguration(7-9)ToxiproxyConfiguration(11-14)ToxiproxyConfiguration(16-19)ToxiproxyConfiguration(21-24)
tests/Testcontainers.Toxiproxy.Tests/ToxiproxyContainerTest.cs (2)
src/Testcontainers.Toxiproxy/ToxiproxyBuilder.cs (9)
ToxiproxyContainer(44-48)ToxiproxyBuilder(17-22)ToxiproxyBuilder(28-32)ToxiproxyBuilder(34-38)ToxiproxyBuilder(54-65)ToxiproxyBuilder(74-85)ToxiproxyBuilder(95-98)ToxiproxyBuilder(101-104)ToxiproxyBuilder(107-111)src/Testcontainers.Toxiproxy/ToxiproxyContainer.cs (2)
ToxiproxyContainer(21-26)Task(43-61)
⏰ 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). (7)
- 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: analyze (csharp)
🔇 Additional comments (9)
tests/Testcontainers.Toxiproxy.Tests/.editorconfig (1)
1-1: LGTM!Standard EditorConfig root declaration for the test project.
src/Testcontainers.Toxiproxy/.editorconfig (1)
1-1: LGTM!Standard EditorConfig root declaration for the source project.
tests/Testcontainers.Toxiproxy.Tests/.runs-on (1)
1-1: LGTM!Valid CI runner specification using Ubuntu 24.04 LTS.
src/Testcontainers.Toxiproxy/Usings.cs (1)
1-10: LGTM!Standard global using directives that streamline code across the Toxiproxy module by eliminating repetitive namespace declarations.
src/Testcontainers.Toxiproxy/Testcontainers.Toxiproxy.csproj (1)
1-13: LGTM!Standard project structure for a Testcontainers module with appropriate multi-targeting (net8.0, net9.0, netstandard2.0, netstandard2.1) and proper dependency management through central package versioning.
Directory.Packages.props (1)
90-90: Toxiproxy.Net 2.0.1 is verified as the latest stable version with no known vulnerabilities.The addition of
Toxiproxy.Netversion 2.0.1 is correct and safe. No further action needed.src/Testcontainers.Toxiproxy/ToxiproxyBuilder.cs (1)
9-10: LGTM! Well-structured builder implementation.The core builder logic is well-implemented:
- Constants for image and port provide clear configuration
Init()properly sets up image, port binding, and HTTP-based wait strategy for the/proxiesendpointWithProxy()correctly adds initial proxies to be created on startupMerge()properly handles immutability by creating a new list copy of_initialProxies, avoiding shared mutable stateAlso applies to: 54-65, 74-85, 107-111
tests/Testcontainers.Toxiproxy.Tests/ToxiproxyContainerTest.cs (2)
264-323: LGTM! Excellent integration test patterns.The
LatencyToxic_ShouldIntroduceExpectedDelayandTimeoutToxic_ShouldDropConnectionAfterTimeouttests demonstrate excellent practices:
- Properly create and dispose both TCP listeners and containers
- Test actual network behavior with real latency/timeout measurements
- Use appropriate tolerance ranges for timing assertions (lines 319-322, 408)
- Clean up resources in all code paths (lines 316-317, 406-407)
These tests provide valuable coverage of the Toxiproxy module's core functionality.
Also applies to: 354-409
6-20: LGTM! Well-structured test class with proper lifecycle management.The test class correctly implements xUnit's
IAsyncLifetimepattern for container setup and teardown. The shared container field (line 8) is appropriate since xUnit creates a new test class instance per test method, ensuring test isolation. The basic proxy management tests (lines 23-227) provide good coverage of CRUD operations, error handling, and toxic configuration.Also applies to: 23-227
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/Testcontainers.Toxiproxy/ToxiproxyContainer.cs (1)
7-7: Unused field: consider adding convenience methods.The
_configurationfield is stored but never used. Consider either removing it or adding convenience methods that utilize the configuration to provide better ergonomics.For example:
public Connection GetConnection() { return new Connection(Hostname, GetMappedPublicPort(ToxiproxyBuilder.ToxiproxyControlPort)); } public Client GetClient() { return GetConnection().Client(); }This would simplify test code from:
using var connection = new Connection(_toxiproxyContainer.Hostname, _toxiproxyContainer.GetMappedPublicPort()); var client = connection.Client();to:
var client = _toxiproxyContainer.GetClient();tests/Testcontainers.Toxiproxy.Tests/ToxiproxyContainerTest.cs (1)
102-103: Latency assertions may be brittle in CI environments.The assertions assume specific latency ranges (0-250ms baseline, 1000-1500ms with toxic). These ranges might be too strict for slower CI environments or under heavy load, potentially causing flaky tests.
Consider:
- Widening the ranges slightly
- Or focusing assertions on relative change (e.g.,
Assert.True(latencyWithToxic > latencyWithoutToxic + 900))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
Directory.Packages.props(1 hunks)src/Testcontainers.Toxiproxy/Testcontainers.Toxiproxy.csproj(1 hunks)src/Testcontainers.Toxiproxy/ToxiproxyBuilder.cs(1 hunks)src/Testcontainers.Toxiproxy/ToxiproxyConfiguration.cs(1 hunks)src/Testcontainers.Toxiproxy/ToxiproxyContainer.cs(1 hunks)src/Testcontainers.Toxiproxy/Usings.cs(1 hunks)tests/Testcontainers.Toxiproxy.Tests/Testcontainers.Toxiproxy.Tests.csproj(1 hunks)tests/Testcontainers.Toxiproxy.Tests/ToxiproxyContainerTest.cs(1 hunks)tests/Testcontainers.Toxiproxy.Tests/Usings.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Testcontainers.Toxiproxy/Testcontainers.Toxiproxy.csproj
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-08T09:04:00.026Z
Learnt from: HofmeisterAn
Repo: testcontainers/testcontainers-dotnet PR: 1509
File: tests/Testcontainers.Grafana.Tests/GrafanaContainerTest.cs:45-46
Timestamp: 2025-11-08T09:04:00.026Z
Learning: In xUnit.net test methods in the testcontainers-dotnet project, ConfigureAwait(true) is recommended and correct to use, as it ensures proper synchronization context capture for xUnit's test parallelism management. Do not suggest changing ConfigureAwait(true) to ConfigureAwait(false) in test methods.
Applied to files:
tests/Testcontainers.Toxiproxy.Tests/ToxiproxyContainerTest.cs
🧬 Code graph analysis (4)
src/Testcontainers.Toxiproxy/ToxiproxyContainer.cs (2)
src/Testcontainers.Toxiproxy/ToxiproxyBuilder.cs (2)
PublicAPI(4-80)ToxiproxyContainer(38-42)src/Testcontainers.Toxiproxy/ToxiproxyConfiguration.cs (6)
PublicAPI(4-53)ToxiproxyConfiguration(10-12)ToxiproxyConfiguration(18-22)ToxiproxyConfiguration(28-32)ToxiproxyConfiguration(38-42)ToxiproxyConfiguration(49-52)
tests/Testcontainers.Toxiproxy.Tests/ToxiproxyContainerTest.cs (1)
src/Testcontainers.Toxiproxy/ToxiproxyBuilder.cs (6)
ToxiproxyBuilder(18-22)ToxiproxyBuilder(28-32)ToxiproxyBuilder(45-61)ToxiproxyBuilder(64-67)ToxiproxyBuilder(70-73)ToxiproxyBuilder(76-79)
src/Testcontainers.Toxiproxy/ToxiproxyBuilder.cs (2)
src/Testcontainers.Toxiproxy/ToxiproxyContainer.cs (2)
PublicAPI(4-18)ToxiproxyContainer(13-17)src/Testcontainers.Toxiproxy/ToxiproxyConfiguration.cs (6)
PublicAPI(4-53)ToxiproxyConfiguration(10-12)ToxiproxyConfiguration(18-22)ToxiproxyConfiguration(28-32)ToxiproxyConfiguration(38-42)ToxiproxyConfiguration(49-52)
src/Testcontainers.Toxiproxy/ToxiproxyConfiguration.cs (2)
src/Testcontainers.Toxiproxy/ToxiproxyBuilder.cs (1)
PublicAPI(4-80)src/Testcontainers.Toxiproxy/ToxiproxyContainer.cs (1)
PublicAPI(4-18)
⏰ 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). (7)
- GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.ActiveMq, 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: analyze (csharp)
🔇 Additional comments (5)
Directory.Packages.props (1)
90-90: Package version verified and secure.Toxiproxy.Net 2.0.1 exists on NuGet as the latest available version and has no known security vulnerabilities.
src/Testcontainers.Toxiproxy/ToxiproxyConfiguration.cs (1)
1-53: LGTM!The configuration class follows the standard testcontainers module pattern with proper constructor chaining for immutable configuration propagation.
tests/Testcontainers.Toxiproxy.Tests/ToxiproxyContainerTest.cs (2)
3-44: LGTM!The test class properly implements
IAsyncLifetimefor resource management, ensuring containers and network are disposed. The use ofConfigureAwait(false)in lifecycle methods is appropriate.
64-76: Consider adding proxy readiness check.After creating the proxy at line 69, the test immediately attempts to connect to Redis through it at line 76. There's no verification that the proxy is fully initialized and listening. This could cause intermittent test failures if the proxy takes time to start accepting connections.
Consider adding a brief delay or polling mechanism to verify the proxy is ready:
var redisProxy = client.Add(proxy); // Wait for proxy to be ready await Task.Delay(100).ConfigureAwait(true);Or verify by attempting a connection with retry logic.
src/Testcontainers.Toxiproxy/ToxiproxyBuilder.cs (1)
45-61: LGTM!The
Initmethod correctly configures the Toxiproxy container with:
- Control port binding and readiness check
- Dynamic binding of 32 proxied ports (8666-8697)
- Proper wait strategy polling the
/versionendpoint
HofmeisterAn
left a comment
HofmeisterAn
left a comment
There was a problem hiding this comment.
Finally, I had time to look into this PR, something I've been looking forward to for a while. One thing I noticed is that the Toxiproxy module was implemented with a dependency on Toxiproxy.Net. We generally don't include client dependencies in our modules, so I removed it to align the module with our standards and updated the tests and docs accordingly.
I also added an example that matches the Testcontainers for Java implementation. It would be great to add the other tests as examples too. If you think anything is missing, please feel free to create an issue so we can discuss the changes or missing parts.
Thanks for the initial work on this! BTW awesome that you're in F1 😎. I'm a big fan, just the wrong team 😬.
HofmeisterAn
changed the title
2 participants
What does this PR do?
Adds a new Testcontainers.Toxiproxy module that enables integration testing with Shopify's Toxiproxy.
It provides a ToxiproxyContainer and a ToxiproxyBuilder to simulate various network conditions (latency, timeout, bandwidth limit, etc.) within containerized test environments.
This module uses the Toxiproxy.Net client library to interact with the Toxiproxy HTTP API.
Included tests cover basic proxy creation, toxic addition, and runtime behavior of network faults like latency and timeouts.
Why is it important?
There is currently no native Toxiproxy module in testcontainers-dotnet.
Toxiproxy is a powerful tool for simulating real-world network issues such as slow connections, disconnections, or dropped packets.
How to test this PR.
dotnet test tests/Testcontainers.Toxiproxy.Tests