feat: Add Mailpit module - #1727
Conversation
✅ Deploy Preview for testcontainers-dotnet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a Mailpit container module with configurable SMTP authentication, message retention, mapped endpoints, integration tests, solution registration, centralized package versioning, and module documentation. ChangesMailpit container module
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Test
participant MailpitFixture
participant MailpitSMTP
participant MailpitAPI
Test->>MailpitFixture: Send MimeMessage
MailpitFixture->>MailpitSMTP: Submit message over SMTP
MailpitSMTP-->>MailpitFixture: Return message ID
MailpitFixture->>MailpitAPI: Request message by ID
MailpitAPI-->>MailpitFixture: Return message JSON
MailpitFixture-->>Test: Return parsed message
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
tests/Testcontainers.Mailpit.Tests/MailpitContainerTest.cs (1)
46-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a substring check for the certificate issuer to prevent cross-platform test flakiness.
The ordering of Relative Distinguished Names (RDNs) in the
Issuerstring can vary depending on the underlying cryptographic library used by the OS (e.g., OpenSSL on Linux might format it asO=..., CN=...instead ofCN=..., O=...). Consider using.Contains()instead of an exact match to ensure the test passes reliably across all platforms.♻️ Proposed refactor
using var smtpClient = new SmtpClient(); - smtpClient.ServerCertificateValidationCallback = (_, certificate, _, _) => certificate?.Issuer == "CN=localhost, O=Mailpit self-signed certificate"; + smtpClient.ServerCertificateValidationCallback = (_, certificate, _, _) => certificate?.Issuer.Contains("Mailpit self-signed certificate") == true;🤖 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 `@tests/Testcontainers.Mailpit.Tests/MailpitContainerTest.cs` around lines 46 - 47, Update the ServerCertificateValidationCallback in MailpitContainerTest to validate the certificate issuer using a substring check for the expected Mailpit self-signed certificate identity, rather than requiring an exact RDN ordering. Preserve the existing null-certificate rejection behavior.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@src/Testcontainers.Mailpit/.editorconfig`:
- Line 1: Remove the root = true setting from the module-level .editorconfig so
it inherits the repository-level formatting configuration. Keep any other
module-specific editor settings unchanged.
In `@src/Testcontainers.Mailpit/MailpitBuilder.cs`:
- Around line 82-89: Update WithSmtpAuthCredentials in MailpitBuilder so each
call removes or clears the previously configured mutually exclusive SMTP
environment variables before applying the settings for the current allowInsecure
value. Ensure the resulting builder state contains either
MP_SMTP_AUTH_ALLOW_INSECURE or the TLS certificate/key variables, never both,
when calls use different values.
- Around line 71-76: Update WithSmtpAuthCredentials to validate credentials is
non-null before accessing credentials.UserName, throwing the appropriate
argument-null exception for a null parameter while preserving the existing colon
validation for non-null credentials.
In `@src/Testcontainers.Mailpit/MailpitConfiguration.cs`:
- Around line 13-18: Update src/Testcontainers.Mailpit/MailpitConfiguration.cs
lines 13-18 so the smtpAuthAllowInsecure and maxMessages parameters use nullable
bool? and uint? types with null defaults, preserving explicit false and 0 values
during BuildConfiguration.Combine. Update lines 63-77 so the
SmtpAuthAllowInsecure and MaxMessages properties use matching nullable types.
---
Nitpick comments:
In `@tests/Testcontainers.Mailpit.Tests/MailpitContainerTest.cs`:
- Around line 46-47: Update the ServerCertificateValidationCallback in
MailpitContainerTest to validate the certificate issuer using a substring check
for the expected Mailpit self-signed certificate identity, rather than requiring
an exact RDN ordering. Preserve the existing null-certificate rejection
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 87037c8f-c9d0-4468-ad82-3a105cd86ee5
📒 Files selected for processing (16)
Directory.Packages.propsTestcontainers.slnTestcontainers.slnxdocs/modules/index.mdsrc/Testcontainers.Mailpit/.editorconfigsrc/Testcontainers.Mailpit/MailpitBuilder.cssrc/Testcontainers.Mailpit/MailpitConfiguration.cssrc/Testcontainers.Mailpit/MailpitContainer.cssrc/Testcontainers.Mailpit/Testcontainers.Mailpit.csprojsrc/Testcontainers.Mailpit/Usings.cstests/Testcontainers.Mailpit.Tests/.editorconfigtests/Testcontainers.Mailpit.Tests/.runs-ontests/Testcontainers.Mailpit.Tests/Dockerfiletests/Testcontainers.Mailpit.Tests/MailpitContainerTest.cstests/Testcontainers.Mailpit.Tests/Testcontainers.Mailpit.Tests.csprojtests/Testcontainers.Mailpit.Tests/Usings.cs
8a591aa to
c46dbb7
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/Testcontainers.Mailpit.Tests/MailpitContainerTest.cs (1)
54-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCleanly disconnect the SMTP client.
MailKit's
SmtpClientdoes not send aQUITcommand uponDispose(). It is recommended to explicitly disconnect before disposal to ensure a clean termination of the connection with the server.♻️ Proposed refactor
var result = await smtpClient.SendAsync(message, cancellationToken); + await smtpClient.DisconnectAsync(true, cancellationToken); var messageId = QueuedMessage().Match(result).Groups[1].Value; Assert.NotEmpty(messageId);🤖 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 `@tests/Testcontainers.Mailpit.Tests/MailpitContainerTest.cs` around lines 54 - 58, Update the SMTP client cleanup in the test flow containing SendAsync and QueuedMessage to explicitly disconnect the MailKit SmtpClient before disposal, ensuring the disconnect occurs after sending completes and regardless of success or failure.
🤖 Prompt for all review comments with 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.
Nitpick comments:
In `@tests/Testcontainers.Mailpit.Tests/MailpitContainerTest.cs`:
- Around line 54-58: Update the SMTP client cleanup in the test flow containing
SendAsync and QueuedMessage to explicitly disconnect the MailKit SmtpClient
before disposal, ensuring the disconnect occurs after sending completes and
regardless of success or failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f6d5db1f-8924-4d50-be9a-5009162ba9a4
📒 Files selected for processing (15)
Directory.Packages.propsTestcontainers.slnxdocs/modules/index.mdsrc/Testcontainers.Mailpit/.editorconfigsrc/Testcontainers.Mailpit/MailpitBuilder.cssrc/Testcontainers.Mailpit/MailpitConfiguration.cssrc/Testcontainers.Mailpit/MailpitContainer.cssrc/Testcontainers.Mailpit/Testcontainers.Mailpit.csprojsrc/Testcontainers.Mailpit/Usings.cstests/Testcontainers.Mailpit.Tests/.editorconfigtests/Testcontainers.Mailpit.Tests/.runs-ontests/Testcontainers.Mailpit.Tests/Dockerfiletests/Testcontainers.Mailpit.Tests/MailpitContainerTest.cstests/Testcontainers.Mailpit.Tests/Testcontainers.Mailpit.Tests.csprojtests/Testcontainers.Mailpit.Tests/Usings.cs
🚧 Files skipped from review as they are similar to previous changes (11)
- tests/Testcontainers.Mailpit.Tests/.editorconfig
- src/Testcontainers.Mailpit/Usings.cs
- src/Testcontainers.Mailpit/.editorconfig
- tests/Testcontainers.Mailpit.Tests/Usings.cs
- Testcontainers.slnx
- src/Testcontainers.Mailpit/MailpitContainer.cs
- src/Testcontainers.Mailpit/Testcontainers.Mailpit.csproj
- tests/Testcontainers.Mailpit.Tests/Testcontainers.Mailpit.Tests.csproj
- src/Testcontainers.Mailpit/MailpitConfiguration.cs
- docs/modules/index.md
- src/Testcontainers.Mailpit/MailpitBuilder.cs
c46dbb7 to
75e1e0e
Compare
Also make sure that Mailpit can be used without authentication if not configured and improve SMTP authentication configuration by letting the user choose whether to use a self-signed certificate or to allow insecure PLAIN and LOGIN SMTP authentication.
75e1e0e to
2405430
Compare
|
It would be really nice to get this functionality for Testcontainers, great job! If you need help on this PR, please let me know. |
HofmeisterAn
left a comment
There was a problem hiding this comment.
Thanks both to you.
What does this PR do?
This pull request resurrects #1134 to work with the current Testcontainers API.
Attribution to @feslima is preserved in cc70a2f. Then some refinements have been applied in commit 2405430.
Tests have also been added to cover all possible configurations.
Why is it important?
See #1134.
Also, having mail in the module name is much better than Papercut when you are looking for an email related container. 😁
Related issues
Summary by CodeRabbit
New Features
Documentation
Tests