Skip to content

Conversation

@HofmeisterAn
Copy link
Collaborator

What does this PR do?

This PR fixes a few minor issues that came up after merging other PRs and tidies up some small inconsistencies that slipped through.

  • Updates the Toxiproxy NuGet dependency to a version compatible with .NET (not just the legacy framework). Unfortunately, both available implementations seem a bit outdated and inactive.
  • Removes unnecessary BOMs and converts files to UTF-8 encoding.
  • Updates the .NET feature in the Dev Containers configuration.

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 Nov 9, 2025
@netlify
Copy link

netlify bot commented Nov 9, 2025

Deploy Preview for testcontainers-dotnet ready!

Name Link
🔨 Latest commit d4d9cd2
🔍 Latest deploy log https://app.netlify.com/projects/testcontainers-dotnet/deploys/6910c9ed6d2f9100084c1b41
😎 Deploy Preview https://deploy-preview-1568--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 Nov 9, 2025

Summary by CodeRabbit

  • Chores

    • Updated .NET development container environment to version 2.4.0
    • Updated package dependencies for Toxiproxy integration
    • Corrected file encoding and cleaned up project files
  • Tests

    • Enhanced Toxiproxy test suite with asynchronous API operations

Walkthrough

Updates include bumping the .NET devcontainer feature, replacing the Toxiproxy NuGet package, converting synchronous Toxiproxy test calls to async, removing BOM characters from Mosquitto files, and a small Playwright docs example tweak.

Changes

Cohort / File(s) Summary
Devcontainer Configuration
.devcontainer/devcontainer.json
Bumped dotnet feature from 2.2.2 to 2.4.0.
Package Dependencies
Directory.Packages.props, tests/Testcontainers.Toxiproxy.Tests/Testcontainers.Toxiproxy.Tests.csproj
Replaced Toxiproxy.Net 2.0.1 with ToxiproxyNetCore 1.0.40.
Toxiproxy Tests (async migration)
tests/Testcontainers.Toxiproxy.Tests/ToxiproxyContainerTest.cs
Replaced synchronous calls with async: client.Add(...)await client.AddAsync(...).ConfigureAwait(true) and proxy.Add(...)await proxy.AddAsync(...).ConfigureAwait(true).
Spell/dictionary updates
Testcontainers.dic, Testcontainers.sln.DotSettings
Added "toxiproxy" to dictionaries.
Playwright docs
docs/modules/playwright.md
Changed local container declaration from var to IContainer and reference from playwrightContainer to _playwrightContainer.
Mosquitto BOM cleanup
src/Testcontainers.Mosquitto/MosquittoBuilder.cs, src/Testcontainers.Mosquitto/MosquittoConfiguration.cs, src/Testcontainers.Mosquitto/MosquittoContainer.cs, src/Testcontainers.Mosquitto/Testcontainers.Mosquitto.csproj, tests/Testcontainers.Mosquitto.Tests/Testcontainers.Mosquitto.Tests.csproj
Removed leading BOM/hidden characters from file headers.

Sequence Diagram(s)

sequenceDiagram
    participant Test as Test (ToxiproxyContainerTest)
    participant Client as ToxiproxyClient
    participant Proxy as ProxyContainer

    Note over Test,Client: Async proxy creation flow (updated)
    Test->>Client: AddAsync(proxy)
    activate Client
    Client-->>Test: Task<Proxy> (completed)
    deactivate Client

    Note over Test,Proxy: Async toxic application (updated)
    Test->>Proxy: AddAsync(latencyToxic)
    activate Proxy
    Proxy-->>Test: Task<Toxic> (completed)
    deactivate Proxy
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Verify test methods are properly marked async and that ConfigureAwait usage is intentional.
  • Confirm API compatibility between Toxiproxy.Net and ToxiproxyNetCore for all usages.
  • Quick checks on devcontainer bump and Playwright example compilation.
  • Inspect Mosquitto BOM removals for no unintended whitespace/encoding regressions.

Possibly related PRs

Suggested reviewers

  • hamidjahad

Poem

🐰
I hopped through bytes to clear the fluff,
BOMs be gone — that header's enough,
Toxiproxy donned a fresher name,
Async awaits to play the game,
Containers hum, the tests take flight.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly and specifically describes the main change: updating the Toxiproxy NuGet dependency to a .NET-compatible version, which aligns with the primary objective evident in the changeset.
Description check ✅ Passed The PR description covers the key changes (Toxiproxy update, BOM removal, Dev Container update) and explains the rationale, though the 'Why is it important?' section is incomplete with only a dash.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/switch-to-toxiproxynetcore-nuget

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1d66977 and d4d9cd2.

📒 Files selected for processing (1)
  • docs/modules/playwright.md (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • docs/modules/playwright.md
⏰ 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.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: analyze (csharp)

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.

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: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bfa1fb9 and 1d66977.

📒 Files selected for processing (12)
  • .devcontainer/devcontainer.json (1 hunks)
  • Directory.Packages.props (1 hunks)
  • Testcontainers.dic (1 hunks)
  • Testcontainers.sln.DotSettings (1 hunks)
  • docs/modules/playwright.md (1 hunks)
  • src/Testcontainers.Mosquitto/MosquittoBuilder.cs (1 hunks)
  • src/Testcontainers.Mosquitto/MosquittoConfiguration.cs (1 hunks)
  • src/Testcontainers.Mosquitto/MosquittoContainer.cs (1 hunks)
  • src/Testcontainers.Mosquitto/Testcontainers.Mosquitto.csproj (1 hunks)
  • tests/Testcontainers.Mosquitto.Tests/Testcontainers.Mosquitto.Tests.csproj (1 hunks)
  • tests/Testcontainers.Toxiproxy.Tests/Testcontainers.Toxiproxy.Tests.csproj (1 hunks)
  • tests/Testcontainers.Toxiproxy.Tests/ToxiproxyContainerTest.cs (2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-08T09:04:00.045Z
Learnt from: HofmeisterAn
Repo: testcontainers/testcontainers-dotnet PR: 1509
File: tests/Testcontainers.Grafana.Tests/GrafanaContainerTest.cs:45-46
Timestamp: 2025-11-08T09:04:00.045Z
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
⏰ 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.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
🔇 Additional comments (11)
Testcontainers.dic (1)

38-38: LGTM!

The addition of "toxiproxy" to the dictionary is consistent with the package update from Toxiproxy.Net to ToxiproxyNetCore.

tests/Testcontainers.Mosquitto.Tests/Testcontainers.Mosquitto.Tests.csproj (1)

1-1: LGTM!

The BOM removal improves file encoding consistency and is part of the broader cleanup mentioned in the PR objectives.

Testcontainers.sln.DotSettings (1)

47-47: LGTM!

The dictionary entry addition is consistent with the Toxiproxy package changes and prevents IDE spell-check warnings.

tests/Testcontainers.Toxiproxy.Tests/ToxiproxyContainerTest.cs (2)

77-78: LGTM!

The conversion to async is correct and aligns with the ToxiproxyNetCore package API. The use of ConfigureAwait(true) is appropriate for xUnit test methods.


110-111: LGTM!

The async conversion is correct and consistent with the proxy addition pattern above.

src/Testcontainers.Mosquitto/MosquittoContainer.cs (1)

1-1: LGTM!

The BOM removal is part of the encoding cleanup mentioned in the PR objectives and improves file consistency.

src/Testcontainers.Mosquitto/Testcontainers.Mosquitto.csproj (1)

1-1: LGTM!

The BOM removal completes the encoding cleanup for the Mosquitto module files.

.devcontainer/devcontainer.json (1)

16-19: Version 2.4.0 verified as the latest stable release.

The .NET devcontainer feature version 2.4.0 is the latest stable version, so the update is current and appropriate. No action required.

src/Testcontainers.Mosquitto/MosquittoConfiguration.cs (1)

1-1: LGTM! BOM character removed.

The removal of the byte order mark improves file encoding consistency with no functional impact.

src/Testcontainers.Mosquitto/MosquittoBuilder.cs (1)

1-1: LGTM! BOM character removed.

Consistent encoding cleanup across the Mosquitto module.

Directory.Packages.props (1)

91-91: Package replacement aligns with .NET compatibility goals.

The centralized switch from Toxiproxy.Net to ToxiproxyNetCore supports the PR's objective of using a .NET-compatible version. However, ensure that all consuming code has been updated to accommodate any API differences between these packages.

This change depends on verification that test code has been properly updated (see comment on the test project file).

@HofmeisterAn HofmeisterAn merged commit 5517556 into develop Nov 9, 2025
81 checks passed
@HofmeisterAn HofmeisterAn deleted the feature/switch-to-toxiproxynetcore-nuget branch November 9, 2025 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants