[Feature] Add Mailpit module#1727
Open
0xced wants to merge 2 commits into
Open
Conversation
✅ Deploy Preview for testcontainers-dotnet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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 labels: 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
Comment on lines
+82
to
+89
| return allowInsecure | ||
| ? builder | ||
| .WithEnvironment("MP_SMTP_AUTH_ALLOW_INSECURE", "1") | ||
| : builder | ||
| // https://mailpit.axllent.org/docs/configuration/certificates/#auto-generate-self-signed-certificates | ||
| .WithEnvironment("MP_SMTP_TLS_CERT", "sans:localhost") | ||
| .WithEnvironment("MP_SMTP_TLS_KEY", "sans:localhost"); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Handle mutually exclusive environment variables on subsequent calls.
If WithSmtpAuthCredentials is called multiple times on the same builder instance with different values for allowInsecure, the environment variables from the previous calls will leak into the new builder state because .WithEnvironment adds or overwrites keys, but does not remove existing ones.
This could result in both MP_SMTP_AUTH_ALLOW_INSECURE and TLS certificates being configured simultaneously. You should explicitly clear the mutually exclusive keys (or defer environment variable resolution to the Build phase using the properties stored in MailpitConfiguration).
🤖 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 `@src/Testcontainers.Mailpit/MailpitBuilder.cs` around lines 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.
Comment on lines
+13
to
+18
| public MailpitConfiguration(NetworkCredential smtpAuthCredentials = null, bool smtpAuthAllowInsecure = true, uint maxMessages = 100) | ||
| { | ||
| SmtpAuthCredentials = smtpAuthCredentials; | ||
| SmtpAuthAllowInsecure = smtpAuthAllowInsecure; | ||
| MaxMessages = maxMessages; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use nullable types for configuration properties.
In Testcontainers, configuration properties are merged using BuildConfiguration.Combine(old, new). Since Combine checks against default(T), non-nullable types fail to merge correctly when explicitly set to their default values (e.g., false for bool, 0 for uint). This means users won't be able to explicitly disable auto-deletion (by setting it to 0) or enforce STARTTLS (by setting it to false), as those values will be treated as unset and overwritten by defaults.
src/Testcontainers.Mailpit/MailpitConfiguration.cs#L13-L18: Change thesmtpAuthAllowInsecureandmaxMessagesconstructor parameters tobool?anduint?(withnullas the default value) so explicit assignments are preserved.src/Testcontainers.Mailpit/MailpitConfiguration.cs#L63-L77: Change theSmtpAuthAllowInsecureandMaxMessagesproperties tobool?anduint?to support the updated constructor.
📍 Affects 1 file
src/Testcontainers.Mailpit/MailpitConfiguration.cs#L13-L18(this comment)src/Testcontainers.Mailpit/MailpitConfiguration.cs#L63-L77
🤖 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 `@src/Testcontainers.Mailpit/MailpitConfiguration.cs` around lines 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.
0xced
force-pushed
the
feature/add-mailpit
branch
from
July 16, 2026 14:16
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
0xced
force-pushed
the
feature/add-mailpit
branch
from
July 17, 2026 07:14
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.
0xced
force-pushed
the
feature/add-mailpit
branch
from
July 17, 2026 13:04
75e1e0e to
2405430
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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