Skip to content

Conversation

@ThreeFive-O
Copy link
Contributor

@ThreeFive-O ThreeFive-O commented Sep 15, 2018

PR Summary

Use newer Pester syntax on Send-MailMessage cmdlet tests:

  • Use Pester test cases
  • Add logic to store current mailbox content before running tests
  • Refactoring of code

PR Checklist

Use Pester test cases
Add logic to store current mailbox content if in use
Refactoring
{
return $false
}
}
Copy link
Member

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.

Copy link
Contributor

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 ?

@adityapatwardhan
Copy link
Member

@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.

@ThreeFive-O
Copy link
Contributor Author

@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.
It's the same logic as the old test. Do you have any information if the PowerShell-CI-linux hosts a local Postfix SMTP server?

@adityapatwardhan
Copy link
Member

@ThreeFive-O You can go to logs and see that both tests are marked as pending:

2018-09-15T19:08:37.4876307Z Describing Send-MailMessage
2018-09-15T19:08:37.5576199Z ??

If there are not running on Linux-CI then maybe add more logging with Write-Verbose -Verbose for the reason on skipping.

@TravisEz13 would the following work on Linux-CI hosts?

$tc = New-Object -TypeName System.Net.Sockets.TcpClient -ArgumentList "localhost", 25

@TravisEz13
Copy link
Member

It would only work if a mail host has been installed.

@ThreeFive-O
Copy link
Contributor Author

I think that explains why the test is marked as pending. The daily build of the master branch is also marked as pending:

2018-09-28T10:15:30.5317964Z   Describing Basic Send-MailMessage tests
2018-09-28T10:15:30.6222472Z ??

@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.

@anmenaga
Copy link

@TravisEz13 @adityapatwardhan please provide your feedback on questions in the last post.

@TravisEz13
Copy link
Member

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.

@stale
Copy link

stale bot commented Nov 11, 2018

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.
Thank you for your contributions.
Community members are welcome to grab these works.

@stale stale bot added the Stale label Nov 11, 2018
@stale
Copy link

stale bot commented Nov 21, 2018

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.
Thanks again for your contribution.
Community members are welcome to grab these works.

@stale stale bot closed this Nov 21, 2018
@ThreeFive-O ThreeFive-O deleted the SendMailMessageTests-1 branch February 12, 2019 12:35
@ThreeFive-O
Copy link
Contributor Author

Superseded by PR #8859

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants