-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Improve Send-MailMessage tests #7793
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
Conversation
Use Pester test cases Add logic to store current mailbox content if in use Refactoring
| { | ||
| return $false | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a newline between these lines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adityapatwardhan , Are we having CodeFactor only for .cs. Isn't this should be caught by CodeFactor ?
|
@ThreeFive-O It seems that the test is marked as pending on Linux. Is that expected? I would prefer not to have a different name when the test is pending versus when it succeeds. This help in tracking history of tests. |
|
@adityapatwardhan I can't really see the full output of the PowerShell-CI-linux test run from Pester. The test should only be marked as pending, if no SMTP server is available on the CI or if the user password can't be retrieved from /etc/password. |
|
@ThreeFive-O You can go to logs and see that both tests are marked as pending:
If there are not running on Linux-CI then maybe add more logging with @TravisEz13 would the following work on Linux-CI hosts? $tc = New-Object -TypeName System.Net.Sockets.TcpClient -ArgumentList "localhost", 25 |
|
It would only work if a mail host has been installed. |
|
I think that explains why the test is marked as pending. The daily build of the master branch is also marked as pending: @TravisEz13 and @adityapatwardhan How should we proceed? Make another PR which installs Postfix mail server on the PowerShell-CI-Linux? Honestly I'm not very familiar with that. On the other hand I'm working on a solution for #3950. But this is time-consuming, given the fact I want to enhance the test for the cmdlet first. |
|
@TravisEz13 @adityapatwardhan please provide your feedback on questions in the last post. |
|
I would recommend adding a setup script for postfix and adding a step to the linux.yml to install it. Detect if it's there and skip the test if it's not there. This would give us the most flexibility. |
|
This PR has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed if no further activity occurs within 10 days. |
|
This PR has be automatically close because it is stale. If you wish to continue working on the PR, please first update the PR, then reopen it. |
|
Superseded by PR #8859 |
PR Summary
Use newer Pester syntax on Send-MailMessage cmdlet tests:
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:to the beginning of the title and remove the prefix when the PR is ready.[feature]if the change is significant or affects feature tests