Skip to content

Add more tests for SmtpClient #114690

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.

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

Merged
merged 5 commits into from
Apr 22, 2025
Merged

Add more tests for SmtpClient #114690

merged 5 commits into from
Apr 22, 2025

Conversation

rzikm
Copy link
Member

@rzikm rzikm commented Apr 15, 2025

This PR adds a number of functional tests for the SmptClient. The idea is to map out the current behavior of all 3 ways of sending messages:

  • synchronously via Send
  • asynchronously via void-returning SendAsync (notification returned via SendCompleted)
  • asynchronously via Task-based SendMailAsync

This will give us more confidence for potential future modernization of the code base.

Combining with Unit tests (untouched in this PR), this increases the line coverage from 67% to 75% and branch coverage from 59% to 63%. Lines of interest that are not yet covered seem to be mostly exception propagation in the asynchronous Begin/End method chains and would require targeted test for each step of the chain to cover fully.

@ghost ghost added the area-System.Net label Apr 15, 2025
@rzikm rzikm marked this pull request as ready for review April 15, 2025 13:32
@Copilot Copilot AI review requested due to automatic review settings April 15, 2025 13:32
@rzikm rzikm requested a review from a team April 15, 2025 13:32
Copy link
Contributor

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.

Files not reviewed (1)
  • src/libraries/System.Net.Mail/tests/Functional/System.Net.Mail.Functional.Tests.csproj: Language not supported

Copy link
Member

@rokonec rokonec left a comment

Choose a reason for hiding this comment

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

Very nice. Thanks

Some tests failures from checks seems to be related to given changes.

@@ -201,13 +247,32 @@ await SendMessageAsync(

switch (command.ToUpper())
{
case "STARTTLS":
Copy link
Member

Choose a reason for hiding this comment

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

❤️

@rzikm rzikm merged commit aec77dd into dotnet:main Apr 22, 2025
77 of 85 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators May 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants