Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Add tests for Dsc configuration compilation on Windows#5011

Merged
TravisEz13 merged 11 commits into
PowerShell:masterPowerShell/PowerShell:masterfrom
Indhukrishna:insivara/AddDscCompilationTests/1710Indhukrishna/PowerShell:insivara/AddDscCompilationTests/1710Copy head branch name to clipboard
Oct 11, 2017
Merged

Add tests for Dsc configuration compilation on Windows#5011
TravisEz13 merged 11 commits into
PowerShell:masterPowerShell/PowerShell:masterfrom
Indhukrishna:insivara/AddDscCompilationTests/1710Indhukrishna/PowerShell:insivara/AddDscCompilationTests/1710Copy head branch name to clipboard

Conversation

@Indhukrishna

Copy link
Copy Markdown
Contributor

This PR adds tests for testing Dsc Configuration compilation on Windows. #4570

@Indhukrishna Indhukrishna added the Area-DSC Desired State Configuration issues label Oct 4, 2017
@SteveL-MSFT SteveL-MSFT requested a review from TravisEz13 October 4, 2017 21:44
@ArieHein

ArieHein commented Oct 8, 2017

Copy link
Copy Markdown

for the sake of my OCD, please align the "Ensure" with "Destination Path" and align all three resources at the same column position. :)

@Indhukrishna

Copy link
Copy Markdown
Contributor Author

@ArieHein Thanks for the feedback. I opened it in VSCode and ISE and fixed it. But in the Github UI, I can see that the indentation is still messed up. How do I fix this properly?

}

DSCTestConfig
"@).Invoke()

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.

It might be good to put this in the form of {[Scr...Invoke()}|should not throw and/or verify the expected file gets created.

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.

I've submitted a PR to your branch to make this change: Indhukrishna#1
I'll make the change directly, once the tests pass if you would like me to.

@TravisEz13 TravisEz13 Oct 10, 2017

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.

Tests have passed on Linux and Windows. I'm going to push to your branch.

}

DSCTestConfig
"@).Invoke()

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.

same comment


DSCTestConfig
"@).Invoke()
"@).Invoke() | Should Not Throw;

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.

these tests should fail... a script block should be piped to should not throw

"@).Invoke()
"@).Invoke()

Test-Path -Path .\DSCTestConfig\localhost.mof | Should Be $True

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.

to verify a file exists, please use `` | should exist` , it results it better errors

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.

Updated

@TravisEz13 TravisEz13 dismissed their stale review October 10, 2017 21:33

issues resolved


".\DSCTestConfig\localhost.mof" | Should Exist

Remove-Item -Force -Recurse -Path DSCTestConfig

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.

The remove should be in an AfterEach or the finally or a try{}finally{} otherwise it won't run if the test fails. Might be easier to just use the test drive...

@TravisEz13 TravisEz13 Oct 10, 2017

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.

I've submitted a PR to resolve this: Indhukrishna#2 . I'll merge it if the tests pass.

@TravisEz13 TravisEz13 dismissed their stale review October 10, 2017 22:58

issues resolved

@TravisEz13 TravisEz13 merged commit 2a9cd72 into PowerShell:master Oct 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-DSC Desired State Configuration issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.