Skip to content
This repository was archived by the owner on Jun 14, 2024. It is now read-only.

Update environment tests to meet Pester 4.0.0 standards - #133

Merged
Daniel Scott-Raynsford (PlagueHO) merged 3 commits into
PowerShell:devfrom
PlagueHO:Issue-129
Jan 25, 2019
Merged

Update environment tests to meet Pester 4.0.0 standards#133
Daniel Scott-Raynsford (PlagueHO) merged 3 commits into
PowerShell:devfrom
PlagueHO:Issue-129

Conversation

@PlagueHO

@PlagueHO Daniel Scott-Raynsford (PlagueHO) commented Jan 24, 2019 •
edited by kwirkykat
Loading

Copy link
Copy Markdown
Contributor

Pull Request (PR) description

This PR updates the tests for the Environment resource to meet Pester 4.0.0 standards and corrects a couple of style issues.

This Pull Request (PR) fixes the following issues

Task list

  • Added an entry under the Unreleased section of the change log in the README.md.
    Entry should say what was changed, and how that affects users (if applicable).
  • Resource documentation added/updated in README.md.
  • Resource parameter descriptions added/updated in README.md, schema.mof
    and comment-based help.
  • Comment-based help added/updated.
  • Localization strings added/updated in all localization files as appropriate.
  • Examples appropriately added/updated.
  • Unit tests added/updated. See DSC Resource Testing Guidelines.
  • Integration tests added/updated (where possible). See DSC Resource Testing Guidelines.
  • New/changed code adheres to DSC Resource Style Guidelines and Best Practices.

Johan Ljunggren (@johlju) - would you mind reviewing when you have time?

This change is Reviewable

@PlagueHO Daniel Scott-Raynsford (PlagueHO) added the needs review The pull request needs a code review. label Jan 24, 2019
@codecov-io

Codecov (codecov-io) commented Jan 24, 2019

Copy link
Copy Markdown

Codecov Report

Merging #133 into dev will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@         Coverage Diff         @@
##            dev   #133   +/-   ##
===================================
  Coverage    83%    83%           
===================================
  Files        19     19           
  Lines      2760   2760           
  Branches      4      4           
===================================
  Hits       2305   2305           
  Misses      451    451           
  Partials      4      4

mhendric
Mike Hendrickson (mhendric) approved these changes Jan 24, 2019
View reviewed changes

@mhendric Mike Hendrickson (mhendric) left a comment

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.

Reviewed 4 of 5 files at r1, 1 of 1 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on Daniel Scott-Raynsford (@PlagueHO))

Tests/Integration/MSFT_EnvironmentResource.config.ps1, line 34 at r2 (raw file):

$Target = ('Process', 'Machine')

Non blocking. It's not explicitly stated in the style guidelines, but all examples in 'Correct Format for Arrays' show an @ symbol before the parenthesis. Up to you whether you want to update this to match the examples.

@mhendric Mike Hendrickson (mhendric) removed the needs review The pull request needs a code review. label Jan 24, 2019
@PlagueHO Daniel Scott-Raynsford (PlagueHO) added the needs review The pull request needs a code review. label Jan 25, 2019
PlagueHO
Daniel Scott-Raynsford (PlagueHO) commented Jan 25, 2019
View reviewed changes

@PlagueHO Daniel Scott-Raynsford (PlagueHO) left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

All done. Thanks Mike Hendrickson (@mhendric)

Reviewable status: 4 of 5 files reviewed, 1 unresolved discussion (waiting on Mike Hendrickson (@mhendric))

Tests/Integration/MSFT_EnvironmentResource.config.ps1, line 34 at r2 (raw file):

Previously, mhendric (Mike Hendrickson) wrote…
$Target = ('Process', 'Machine')

Non blocking. It's not explicitly stated in the style guidelines, but all examples in 'Correct Format for Arrays' show an @ symbol before the parenthesis. Up to you whether you want to update this to match the examples.

Good catch! This is correct it should have an @()

Done.

mhendric
Mike Hendrickson (mhendric) approved these changes Jan 25, 2019
View reviewed changes

@mhendric Mike Hendrickson (mhendric) left a comment

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.

:lgtm:

Reviewed 1 of 1 files at r3.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on Mike Hendrickson (@mhendric))

mhendric
Mike Hendrickson (mhendric) approved these changes Jan 25, 2019
View reviewed changes

@mhendric Mike Hendrickson (mhendric) left a comment

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.

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@mhendric Mike Hendrickson (mhendric) added ready for merge The pull request was approved by the community and is ready to be merged by a maintainer. and removed needs review The pull request needs a code review. labels Jan 25, 2019
@PlagueHO

Daniel Scott-Raynsford (PlagueHO) commented Jan 25, 2019 •
edited by kwirkykat
Loading

Copy link
Copy Markdown
Contributor Author

Thanks Mike Hendrickson (@mhendric) - feel free to merge my branches after :lgtm: if you want 😁. I'm good either way.

@PlagueHO
Daniel Scott-Raynsford (PlagueHO) merged commit 90301bd into PowerShell:dev Jan 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer
@mhendric Mike Hendrickson (mhendric) mhendric approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

ready for merge The pull request was approved by the community and is ready to be merged by a maintainer.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@PlagueHO @codecov-io @mhendric @SteveL-MSFT