Skip to content

Conversation

@ThreeFive-O
Copy link
Contributor

@ThreeFive-O ThreeFive-O commented Feb 10, 2019

PR Summary

Fixes #3950

This PR contains a cross-platform SMTP server mockup which allows to run the Send-MailMessage tests on the PowerShell CI system (Azure DevOps pipeline) as well as on local development machines for all supported platforms.

This PR includes:

  • Use of netDumbster (see PR Context for more information) as a cross-platform SMTP server mockup
  • Refactoring of Send-MailMessage tests with new Pester syntax
  • Easier extensibility for further tests (Send-MailMessage cmdlets requires further test coverage)
  • Added RequireSudoOnUnix tag as Linux and macOS on CI requires sudo for privileged port 25 (SMTP)
  • Marked tests for named piping of objects into Send-MailMessage cmdlet as skipped, since this behaviour is not supported as of yet (see issue Send-MailMessage incorrectly handling ValueFromPipelineByPropertyName parameters #7591)

PR Context

Before this PR the Send-MailMessage tests run only on local Linux development machines when a local SMTP server is set up and running.

This is rather inconvenient for testing because:

  • Test code is only meant to be run under Linux
  • One has to set up its own SMTP server
  • Tests are not supported on CI system (Azure DevOps pipeline) for PowerShell

The Azure DevOps pipeline VMs don't have any local SMTP server set up. Therefore the tests for Send-MailMessage are skipped during CI checks.

This PR will provide a way to run the tests on the Azure DevOps pipeline for Windows, Linux and macOS as well on local development machines.

PR Checklist

@ThreeFive-O ThreeFive-O changed the title WIP: Enable cross-platform Send-MailMessage tests for CI Enable cross-platform Send-MailMessage tests for CI Feb 10, 2019
@iSazonov iSazonov added the CL-Test Indicates that a PR should be marked as a test change in the Change Log label Feb 11, 2019
Copy link
Collaborator

@iSazonov iSazonov left a comment

Choose a reason for hiding this comment

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

LGTM with one comment.

@ThreeFive-O ThreeFive-O mentioned this pull request Feb 12, 2019
11 tasks
@iSazonov
Copy link
Collaborator

@adityapatwardhan Please look the PR.

@adityapatwardhan
Copy link
Member

@ThreeFive-O The changes looks great, just added a comment for improving performance slightly. Everything else looks good.

Use NuGet V3 API as package source;
@TravisEz13
Copy link
Member

@adityapatwardhan Please review #9031 . I think we should still take this PR

@ThreeFive-O
Copy link
Contributor Author

@adityapatwardhan and @TravisEz13 Not sure why my CLA status is still not reported, but all other checks have passed.

@TravisEz13
Copy link
Member

@ThreeFive-O No worries, we got status on this commit: 0686809

@adityapatwardhan adityapatwardhan merged commit a2c9482 into PowerShell:master Mar 5, 2019
@ThreeFive-O ThreeFive-O deleted the Fix3950 branch March 5, 2019 23:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create cross-platform tests for Send-MailMessage

4 participants