Skip to content

Add unit and feature tests for Send-MailMessage#9213

Merged
adityapatwardhan merged 4 commits intoPowerShell:masterfrom
ThreeFive-O:Send-MailMessage_FeatureTests
May 2, 2019
Merged

Add unit and feature tests for Send-MailMessage#9213
adityapatwardhan merged 4 commits intoPowerShell:masterfrom
ThreeFive-O:Send-MailMessage_FeatureTests

Conversation

@ThreeFive-O
Copy link
Copy Markdown
Contributor

@ThreeFive-O ThreeFive-O commented Mar 23, 2019 •
edited
Loading

PR Summary

This PR adds several unit and feature tests for Send-MailMessage cmdlet.

Add unit tests:

  • Cc parameter
  • Bcc parameter
  • To parameter with array

Add feature tests:

  • Port parameter
  • Throw on wrong email address format
  • Free-From email address format
  • Priority parameter
  • BodyAsHtml switch
  • UTF8 Encoding
  • Attachments

PR Context

This PR should check against the behavior of Send-MailMessage before work on a newer implementation of Send-MailMessage cmdlet starts.

PR Checklist

@ThreeFive-O ThreeFive-O marked this pull request as ready for review March 23, 2019 21:28
@iSazonov iSazonov added the CL-Test Indicates that a PR should be marked as a test change in the Change Log label Mar 26, 2019
@iSazonov iSazonov added this to the 6.3.0-preview.1 milestone Mar 26, 2019
@iSazonov iSazonov requested a review from TravisEz13 March 26, 2019 11:58
@TravisEz13 TravisEz13 changed the title [feature] Add unit and feature tests for Send-MailMessage Add unit and feature tests for Send-MailMessage Mar 26, 2019
@TravisEz13
Copy link
Copy Markdown
Member

TravisEz13 commented Mar 26, 2019

Please don't use [feature] in commits, until we are able to remove the old code. All tests are run all the time without this tag now and the tag triggers the old code which was sometimes buggy.

@ThreeFive-O
Copy link
Copy Markdown
Contributor Author

ThreeFive-O commented Mar 26, 2019

Will do, thank you @TravisEz13!

iSazonov
iSazonov reviewed Apr 10, 2019
View reviewed changes
test/powershell/Modules/Microsoft.PowerShell.Utility/Send-MailMessage.Tests.ps1
}
}

Describe "Send-MailMessage Feature Tests" -Tags Feature, RequireSudoOnUnix {
Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov Apr 10, 2019

Choose a reason for hiding this comment

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

@TravisEz13 Make sense to use Feature tag?

Copy link
Copy Markdown
Contributor Author

@ThreeFive-O ThreeFive-O Apr 10, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@iSazonov Feature tags for Pester are still available, but [feature] tag for commit messages in new PRs are now unnecessary, since all tests will be executed for PRs and daily builds on the CI agents.

Copy link
Copy Markdown
Contributor

@RDIL RDIL Apr 10, 2019

Choose a reason for hiding this comment

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

I think Ilya means the -Tags Feature in the code

Copy link
Copy Markdown
Contributor Author

@ThreeFive-O ThreeFive-O Apr 10, 2019

Choose a reason for hiding this comment

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

@RDIL The way I understood it: Are -Tags Feature is still necessary for this kind of scenario. In my opinion it makes sense to use Feature tag, since the scope of the tests is much broader, than just some basic CI tests. Also those tests influence the time to run all test heavily.

Copy link
Copy Markdown
Member

@TravisEz13 TravisEz13 Apr 14, 2019

Choose a reason for hiding this comment

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

They all seem appropriate to me

@adityapatwardhan adityapatwardhan merged commit 0735c95 into PowerShell:master May 2, 2019
@adityapatwardhan
Copy link
Copy Markdown
Member

adityapatwardhan commented May 2, 2019

@ThreeFive-O Thank you for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TravisEz13 TravisEz13 TravisEz13 approved these changes

@adityapatwardhan adityapatwardhan adityapatwardhan approved these changes

@iSazonov iSazonov iSazonov approved these changes

+1 more reviewer

@RDIL RDIL RDIL left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@adityapatwardhan adityapatwardhan

Labels

CL-Test Indicates that a PR should be marked as a test change in the Change Log

Projects

None yet

Milestone

7.0.0-preview.1

Development

Successfully merging this pull request may close these issues.

5 participants

@ThreeFive-O @TravisEz13 @adityapatwardhan @iSazonov @RDIL