E522 Style fixes for Select-Xml tests by ThreeFive-O · Pull Request #9037 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]

Skip to content

Style fixes for Select-Xml tests#9037

Merged
adityapatwardhan merged 3 commits intoPowerShell:masterfrom
ThreeFive-O:Style-SelectXml
Mar 5, 2019
Merged

Style fixes for Select-Xml tests#9037
adityapatwardhan merged 3 commits intoPowerShell:masterfrom
ThreeFive-O:Style-SelectXml

Conversation

@ThreeFive-O
Copy link
Contributor
@ThreeFive-O ThreeFive-O commented Mar 3, 2019

PR Summary

This PR includes:

  • Move Select-Xml tests from two different files into one.
  • Update test descriptions.
  • Update Pester syntax where applicable

PR Context

Tests for a specific cmdlet should be in the corresponding test file.

PR Checklist

@ThreeFive-O ThreeFive-O marked this pull request as ready for review March 4, 2019 07:56
Signed-off-by: ThreeFive-O <threefive-o.github@outlook.com>
@iSazonov iSazonov added the CL-Test Indicates that a PR should be marked as a test change in the Change Log label Mar 4, 2019
@adityapatwardhan
Copy link
Member

@ThreeFive-O Thanks for this PR! Could you update the tests to use the new recommended way of testing error?

@ThreeFive-O
Copy link
Contributor Author

@adityapatwardhan I've found the following sentence in the Pester guideline under "Pester Do and Don't" you linked

  1. Prefer Try-Catch for expected errors and check $_.fullyQualifiedErrorId (don't use should throw).

Is this still referring to a time, where Pester didn't support the syntax Should -Throw -ErrorId? If so, I guess we should update the guideline accordingly.

@adityapatwardhan
Copy link
Member

Yes, that sentence should be removed. Please submit that change in a new PR if you can.

@adityapatwardhan adityapatwardhan changed the title [feature] Style fixes for Select-Xml tests Style fixes for Select-Xml tests Mar 5, 2019
@adityapatwardhan adityapatwardhan merged commit d56e501 into PowerShell:master Mar 5, 2019
@ThreeFive-O ThreeFive-O deleted the Style-SelectXml branch March 5, 2019 23:46
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.

3 participants

0