Skip to content

Adding test coverage for Alias provider#2264

Merged
daxian-dbw merged 1 commit into
PowerShell:masterfrom
ChrisUbben:PortTestMultiMachineProvidersAlias
Sep 21, 2016
Merged

Adding test coverage for Alias provider#2264
daxian-dbw merged 1 commit into
PowerShell:masterfrom
ChrisUbben:PortTestMultiMachineProvidersAlias

Conversation

@ChrisUbben

@ChrisUbben ChrisUbben commented Sep 14, 2016

Copy link
Copy Markdown

Could you both take a look at this for me? @Francisco-Gamino @JamesWTruher
Thanks!

@msftclas

msftclas commented Sep 14, 2016

Copy link
Copy Markdown

Hi @ChrisUbben, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!


It looks like you're a Microsoft contributor (Christopher Ubben). If you're full-time, we DON'T require a Contribution License Agreement. If you are a vendor, please DO sign the electronic Contribution License Agreement. It will take 2 minutes and there's no faxing! https://cla.microsoft.com.

TTYL, MSBOT;

@daxian-dbw daxian-dbw self-assigned this Sep 14, 2016
daxian-dbw
daxian-dbw requested changes Sep 14, 2016
View reviewed changes
Comment thread test/powershell/Modules/Microsoft.PowerShell.Management/Alias.Tests.ps1 Outdated

@daxian-dbw daxian-dbw Sep 14, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This test is testing alias provider. It should not belong to the Management module. Please move the test file to the appropriate folder.

@lzybkr lzybkr Sep 15, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also fix the spelling of validation.

@ChrisUbben ChrisUbben Sep 19, 2016

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed spelling of validation.
I talked to @JamesWTruher and he said to put these provider tests in Modules/Microsoft.PowerShell.Management/.

@daxian-dbw

daxian-dbw commented Sep 16, 2016

Copy link
Copy Markdown
Member

@Francisco-Gamino could you please review this PR?

@ChrisUbben ChrisUbben closed this Sep 19, 2016
@ChrisUbben ChrisUbben reopened this Sep 19, 2016
@msftclas

msftclas commented Sep 19, 2016

Copy link
Copy Markdown

Hi @ChrisUbben, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!


It looks like you're a Microsoft contributor (Christopher Ubben). If you're full-time, we DON'T require a Contribution License Agreement. If you are a vendor, please DO sign the electronic Contribution License Agreement. It will take 2 minutes and there's no faxing! https://cla.microsoft.com.

TTYL, MSBOT;

daxian-dbw
daxian-dbw requested changes Sep 21, 2016
View reviewed changes
Comment thread test/powershell/Modules/Microsoft.PowerShell.Management/Alias.Tests.ps1 Outdated

@daxian-dbw daxian-dbw Sep 21, 2016 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe add > $null to New-Item? #closed

@ChrisUbben ChrisUbben Sep 21, 2016

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed. Thanks Dongbo!

Comment thread test/powershell/Modules/Microsoft.PowerShell.Management/Alias.Tests.ps1 Outdated

@daxian-dbw daxian-dbw Sep 21, 2016 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please move Remove-Item to a finally block, so that it will be cleaned up even if the above Should Be failed. #closed

@ChrisUbben ChrisUbben Sep 21, 2016

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed. Thanks Dongbo!

daxian-dbw
daxian-dbw approved these changes Sep 21, 2016
View reviewed changes

@daxian-dbw daxian-dbw left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@daxian-dbw
daxian-dbw merged commit 309bf60 into PowerShell:master Sep 21, 2016
@TravisEz13 TravisEz13 added Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept and removed Review - Waiting on Author labels May 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@daxian-dbw daxian-dbw daxian-dbw approved these changes
+1 more reviewer
@lzybkr lzybkr lzybkr left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@daxian-dbw daxian-dbw

Labels

Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ChrisUbben @msftclas @daxian-dbw @lzybkr @TravisEz13