Update to use DSCResource.test Shared Code - Fixes #177

[PlagueHO]

This PR changes:

• Converted to use AppVeyor.psm1 in DSCResource.Tests repository.
• Converted to use Example and Markdown tests in DSCResource.Tests repository.

This also fixes #177

---

This change is 

[PlagueHO]

@tysonjhayes - I have updated this PR with the new MetaTestOptIn.json file to opt in to the Markdown and Example tests. Everything should be good to go now.

[TravisEz13]

missing newline at end of file

[PlagueHO]

Fixed.

LGTM

[johlju]

Reviewed 10 of 11 files at r1, 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 6 unresolved discussions.

---

appveyor.yml, line 12 at r2 (raw file):

        $mainModuleFolder = "Modules\$moduleName"
        $harnessModulePath = "Tests\$($moduleName).TestHarness.psm1"
        $harnessFunctionName = "Invoke-$($moduleName)Test"

Question: Couldn't we make this function generic in the future, the same name for all resources that uses Harness? Like Invoke-HarnessTest or just plain Invoke-Test.
Or is there a point to having the name in the function likeInvoke-xNetworkingTest?

---

.vscode/RunAllTests.ps1, line 2 at r2 (raw file):

Import-Module (Join-Path $PSScriptRoot "..\Tests\xNetworking.TestHarness.psm1" -Resolve)
$dscTestsPath = Join-Path -Path $PSScriptRoot `

This one expects the DscResource.Tests to already been cloned? What if it isn't?

---

Tests/xNetworking.TestHarness.psm1, line 6 at r2 (raw file):

    param
    (
        [System.String] $TestResultsFile,

Could we format these two parameters like

[Parameter()] 
[System.String]
$TestResultsFile

---

Tests/xNetworking.TestHarness.psm1, line 13 at r2 (raw file):

    Write-Verbose -Message 'Commencing all xNetworking tests'

    $repoDir = Join-Path -Path $PSScriptRoot -ChildPath "..\" -Resolve

Single quotes

---

Tests/xNetworking.TestHarness.psm1, line 17 at r2 (raw file):

    $testCoverageFiles = @()
    Get-ChildItem -Path "$repoDir\modules\xNetworking\DSCResources\**\*.psm1" -Recurse | ForEach-Object {
        if ($_.FullName -notlike '*\DSCResource.Tests\*') {

Could this happen? Shouldn't DSCResource.Tests always end up outside DSCResources folder? If not, then code in RunAllTests.ps1 won't work. :)

---

Comments from Reviewable

[PlagueHO]

@johlju - regarding the name of the name of the harness invoke call. I fully agree - a consistent name would be much better here. I can change xNetworking and xStorage.

The question will be down to @BrianFarnhill: Brian are you ok with changing the name of your Invoke-SharePointDscTests function to a generic name?

I'll look at the rest of the comments tomorrow! Thanks for reviewing @johlju

[PlagueHO]

@johlju - I've been thinking about this and I think we should default the name of the Harness function to Invoke-HarnessTest. If @BrianFarnhill wants to use the alternate name then we can just allow the parameter to be passed. That way we can use the default in all our repos but still support SharePointDsc.

I'll submit a PR with the changes to DSCREsource.tests and also to xNetworking and xStorage.

[PlagueHO]

Review status: 8 of 11 files reviewed at latest revision, 6 unresolved discussions.

---

appveyor.yml, line 12 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Question: Couldn't we make this function generic in the future, the same name for all resources that uses Harness? Like Invoke-HarnessTest or just plain Invoke-Test.
Or is there a point to having the name in the function likeInvoke-xNetworkingTest?

Done. I've submitted the PR to DSCResource.Tests. Once that is through this will start working. Note, I used:
Invoke-TestHarness.

---

*[.vscode/RunAllTests.ps1, line 2 at r2](https://reviewable.io:443/reviews/powershell/xnetworking/186#-KcqD2uxc1rMKO6WTMQm:-KcvSk4X31vwM9oiMP50:b-896fix) ([raw file](https://github.com/powershell/xnetworking/blob/b77c26836170d4307b11f054b5020f1d2b067959/.vscode/RunAllTests.ps1#L2)):*
<details><summary><i>Previously, johlju (Johan Ljunggren) wrote…</i></summary><blockquote>

This one expects the DscResource.Tests to already been cloned? What if it isn't?
</blockquote></details>

Done.

---

*[Tests/TestHarness.psm1, line 6 at r2](https://reviewable.io:443/reviews/powershell/xnetworking/186#-KcqELwOezXrRaYJxbKJ:-KcvSm4HHImogXM0cK0E:b-896fix) ([raw file](https://github.com/powershell/xnetworking/blob/b77c26836170d4307b11f054b5020f1d2b067959/Tests/TestHarness.psm1#L6)):*
<details><summary><i>Previously, johlju (Johan Ljunggren) wrote…</i></summary><blockquote>

Could we format these two parameters like

[Parameter()]
[System.String]
$TestResultsFile

</blockquote></details>

Done.

---

*[Tests/TestHarness.psm1, line 13 at r2](https://reviewable.io:443/reviews/powershell/xnetworking/186#-KcqEbfibrXB2r9IhDc8:-KcvSn1s31L72vLo5JSN:b-896fix) ([raw file](https://github.com/powershell/xnetworking/blob/b77c26836170d4307b11f054b5020f1d2b067959/Tests/TestHarness.psm1#L13)):*
<details><summary><i>Previously, johlju (Johan Ljunggren) wrote…</i></summary><blockquote>

Single quotes
</blockquote></details>

Done.

---

*[Tests/TestHarness.psm1, line 17 at r2](https://reviewable.io:443/reviews/powershell/xnetworking/186#-KcqErkhJTgmFWbbjEKh:-KcvSnZ0iyl2-ToD1t3Q:b-2gd7o5) ([raw file](https://github.com/powershell/xnetworking/blob/b77c26836170d4307b11f054b5020f1d2b067959/Tests/TestHarness.psm1#L17)):*
<details><summary><i>Previously, johlju (Johan Ljunggren) wrote…</i></summary><blockquote>

Could this happen? Shouldn't DSCResource.Tests always end up outside DSCResources folder? If not, then code in RunAllTests.ps1 won't work. :)
</blockquote></details>

It can happen. You can still run individual sets of tests on each repo by changing to the folder containing the specific tests and executing them which will clone into the DSCResources folder. So I think is better to be safe than sorry here.

---


*Comments from [Reviewable](https://reviewable.io:443/reviews/powershell/xnetworking/186)*
<!-- Sent from Reviewable.io -->

[TravisEz13]

Reviewed 4 of 4 files at r3.
Review status: all files reviewed at latest revision, 4 unresolved discussions.

---

Comments from Reviewable

[codecov-io]

Codecov Report

❗ No coverage uploaded for pull request base (dev@660d0c5). Click here to learn what that means.
The diff coverage is n/a.

@@         Coverage Diff          @@
##             dev   #186   +/-   ##
====================================
  Coverage       ?    95%           
====================================
  Files          ?     15           
  Lines          ?   1113           
  Branches       ?      0           
====================================
  Hits           ?   1068           
  Misses         ?     45           
  Partials       ?      0

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 660d0c5...8c8dbfd. Read the comment docs.

[TravisEz13]

95% code coverage impressive. You can explore the code coverage per file in the branch view.

[johlju]

Reviewed 3 of 4 files at r3, 4 of 4 files at r4.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

.codecov.yml, line 20 at r4 (raw file):

      default:
        # Set the overall project code coverage requirement to 70%
        target: 70

Minor: Maybe raise target on this repo?

---

Comments from Reviewable

[TravisEz13]

Our HQRM guidelines are for 70%. There is evidence that higher targets are not always better (although most of those assumed integration tests were not included in the goal.) I would not raise it above 80.

Currently, the other option (threshold) is to prevent regression. I currently have it saying that a 5% regression is allowed. It would be better to lower this to a lower number.

[johlju]

@TravisEz13 I'm good with 70% if that is the guideline. But I thought we could use this check to fail if a new resource got the limit under 95% (in this repos case).

[johlju]

Aha I see! I did not see the threshold value bit in your text until now. Then I understand how it works! Yes maybe it would be good to lower it. Maybe @PlagueHO need to trim the value in.

[johlju]

---

Review status: all files reviewed at latest revision, all discussions resolved, all commit checks successful.

---

Comments from Reviewable

[PlagueHO]

@johlju , @TravisEz13 - I'm ok with lowering the threshold to a lower value. We could try say 2% and see how that goes. @tysonjhayes ?

[TravisEz13]

@PlagueHO That sounds good. We should just remember to treat it with reason. It's all very new.

[tysonjhayes]

Wouldn't lowering the threshold value increase the amount of code coverage needed? So new resources would have to meet the 95+/- 2% code coverage needed before they could be submitted? Seems like a tall order.

[TravisEz13]

Threshold says that a PR that is -threshold% will be marked as failing. So yes, it would increase the requirement. But don't we want to decrease code coverage regression? You can still merge if these fail as well.

[BrianFarnhill]

@PlagueHO Sorry for being late to the party here, had a busy week. To answer the question of me, nah I don't mind using a generic name at all for the test suit. I'm happy to play nice with others on this one, no biggy at all.

[PlagueHO]

No worries @BrianFarnhill mate! I actually changed it so you can use a generic name or override it with your own one by specifying parameter values (AppVeyor.yml is smaller if you use generic).

[PlagueHO]

@tysonjhayes , @TravisEz13 - perhaps we should leave xNetworking threshold as is for now and see how things go. I guess the whole codecov thing is a bit new (at least to these repos) so we might want to monitor for a bit.

[TravisEz13]

@PlagueHO It is probably better to be a bit cautious in the beginning. This is why I set 70% project requirement and 5% threshold. It seemed to give a good amount of wiggle room while telling people when they met the project goals.

[PlagueHO]

Ok - I think we should just leave it as is and merge it. @TravisEz13, @tysonjhayes - are you happy for me to merge?

[PlagueHO]

@johlju, I think there is still one unreviewed file...

---

Review status: 12 of 13 files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable