Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed InModuleScope from xNetworkAdapter tests #147

Merged
merged 1 commit into from
Sep 28, 2016
Merged

Removed InModuleScope from xNetworkAdapter tests #147

merged 1 commit into from
Sep 28, 2016

Conversation

narrieta
Copy link
Contributor

I was reviewing the comments in PowerShell/DscResources#175. @PlagueHO mentions issues trying to use the variables at the top of the script that hold the mock definitions.

Submitting one possible way to deal with those issues. My concern wrt InModuleScope is not as much about this particular test, but about using it in our test templates to write the entire test code.

@PlagueHO Let me know if you hit other issues with InModuleScope and I can look into them in the next few days.

Thanks

@kwirkykat kwirkykat added the needs review The pull request needs a code review. label Sep 27, 2016
@PlagueHO
Copy link
Member

@narrieta - that is some great work! This was extremely helpful. I very much appreciate your work on this.

When I get a chance I'll submit a PR to DSCResources to add this methodology to TestGuidelines.md.

Oh and :LGTM:


Review status: 0 of 1 files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@PlagueHO
Copy link
Member

Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@narrieta narrieta merged commit 993fe8b into dsccommunity:dev Sep 28, 2016
@vors vors removed the needs review The pull request needs a code review. label Sep 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants