Enable integration tests to run in order - Fixes #184

[johlju]

• Add opt-in parameter RunTestInOrder for the helper function Invoke-AppveyorTestScriptTask
  which enables running integration tests in order (issue Running integration test that are dependent on another integration test #184).

---

This change is 

[codecov-io]

Codecov Report

Merging #186 into dev will increase coverage by 0.72%.
The diff coverage is 25.45%.

@@            Coverage Diff             @@
##              dev     #186      +/-   ##
==========================================
+ Coverage   19.66%   20.38%   +0.72%     
==========================================
  Files           4        4              
  Lines         412      466      +54     
==========================================
+ Hits           81       95      +14     
- Misses        331      371      +40

Impacted Files	Coverage Δ
AppVeyor.psm1	0% <0%> (ø)	⬆️
TestHelper.psm1	35.65% <100%> (+4.17%)	⬆️

---

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 75ef028...e8aabac. Read the comment docs.

[johlju]

This is tested here; https://ci.appveyor.com/project/johlju/xsqlserver/build/6.0.854.0#L43

[johlju]

@kwirkykat When trying to fix a bug over at xSQLServer (dsccommunity/SqlServerDsc#774), I hit another problem with the integration test that I think I can fix with a little tweak to this PR. Instead of running the integration tests before the unit tests, I need the integration tests to run after the unit tests.

It has to do with our mocking of the assembly. Integration tests load the SMO assembly resulting in that our mock of SMO fails to load with Add-Type.

I let you know when this is ready to review again!

[johlju]

@kwirkykat Okay, so fixed to the new order, ready for review again. So when you have a chance, please review this one 😄

Added one test, but I will get back to adding more tests once things quite down over at xSQLServer.

[johlju]

Forgot to add this test link. Tested OK here (see order of list of tests)
https://ci.appveyor.com/project/johlju/xsqlserver/build/6.0.891.0#L42

Please ignore that the tests are failing, has nothing to do with this change.

[johlju]

@PlagueHO Maybe you can do an initial review this one to help out @kwirkykat a bit?

[PlagueHO]

Hi @johlju - I've had a look through this and it has occurred to me there might be another solution to this: Create a custom decorator.

With a custom decorator you could add this to the beginning of the *.config.ps1. E.g.

[DSCIntegrationTestOrder(1)]

You could then still use your Get-DscIntegrationTestOrderNumber cmdlet to pull the order out of the *.config.ps1 file, but it would work regardless of if a hashtable existed. You could use the AST to extract this more easily too I think.

To support the custom decorator you'd need to declare the Custom attribute class for the DSCIntegrationTestOrder attribute somewhere in the DSCResource.tests code:

Class DSCIntegrationTestOrder : Attribute {
    [string]$Order

    DSCIntegrationTestOrder([int]$Order)
    {
        $this.Order = $Order
    }
}

Note: The above code would probably need to be changed to support WMF4.

This sort of method might be extended in future to support other metadata we'd need to include in tests.

What do you think?

---

Reviewed 1 of 3 files at r1, 1 of 1 files at r2.
Review status: 2 of 4 files reviewed at latest revision, 9 unresolved discussions, some commit checks failed.

---

README.md, line 338 at r3 (raw file):

## Run integration tests in order

This is only available for resource modules that are using the "Default"-model,

Non-blocking: Could we use something like shared AppVeyor module model rather than Default-model?

---

README.md, line 340 at r3 (raw file):

This is only available for resource modules that are using the "Default"-model,
meaning those resource modules that are calling the helper function
`Invoke-AppveyorTestScriptTask` either without the parameter 'Type', or has

Non-blocking: Perhaps put parameter in back ticks with a dash to make it clear that is is a parameter -Type. Same on next line.

---

README.md, line 343 at r3 (raw file):

assigned the value 'Default' to parameter `Type`.

>**Note:** Resource modules using the "Harness"-model must add this functionality

Non-blocking: Could add an example (e.g. SharePointDsc) to make it clear what a "Harness"-model is?

---

README.md, line 350 at r3 (raw file):

`-RunTestInOrder`.

Also, in each integration test configuration file ('*.config.ps1'), there must be

Many integration test resources actually pass the Hash table in using a -ConfigurationData parameter (e.g https://github.com/PowerShell/xStorage/blob/dev/Tests/Integration/MSFT_xDiskAccessPath.Integration.Tests.ps1#L60). It might be worth pointing out that in these cases a hash table will need to be added to the config regardless.

---

README.md, line 351 at r3 (raw file):

Also, in each integration test configuration file ('*.config.ps1'), there must be
a hash table containing a name-value pair named 'DscIntegrationTestOrder' and be

Non-blocking: Should name-value be key-value?

---

README.md, line 369 at r3 (raw file):

is always run as one of the first tests.

>**Note:** It is not necessary for the name-value pair 'DscIntegrationTestOrder' to be in the

Non-blocking: Should name-value be key-value?

---

Tests/Unit/TestHelper.Tests.ps1, line 18 at r3 (raw file):

                '@()' | `
                    Out-File $filePath_NoHashTable

Can we change to named parameter?

---

Tests/Unit/TestHelper.Tests.ps1, line 21 at r3 (raw file):

                '@{}' | `
                    Out-File $filePath_EmptyHashTable

Can we change to named parameter?

---

Tests/Unit/TestHelper.Tests.ps1, line 24 at r3 (raw file):

                '@{DscIntegrationTestOrder=2}' | `
                    Out-File $filePath_HashTableWithCorrectValue

Can we change to named parameter?

---

Comments from Reviewable

[PlagueHO]

Oops- should be:

Class DSCIntegrationTestOrder : Attribute {
    [int]$Order

    DSCIntegrationTestOrder([int]$Order)
    {
        $this.Order = $Order
    }
}

---

Review status: 2 of 4 files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.

---

Comments from Reviewable

[johlju]

Update 2017-08-23: Changed the class name. Suggestions welcomed.

@PlagueHO your suggestion using a custom attribute is so cool! I built a prototype. It's fully working, and should support PowerShell 4.0? Should I switch to this?

Configuration script:

[Microsoft.DscResourceKit.IntegrationTest(OrderNumber = 2)]
param()

configuration MSFT_xSQLServerSetup_InstallSqlEngineAsSystem_Config
{
    # ... Configuration code
}

Custom attribute (Microsoft.DscResourceKit.cs):

// Custom Attribute for determining the order a test should run in.
// Used to decorate a DSC configuration file with for example [IntegrationTest(OrderNumber = 1)].

using System;

namespace Microsoft.DscResourceKit
{
    // See the attribute guidelines at http://go.microsoft.com/fwlink/?LinkId=85236
    [System.AttributeUsage(System.AttributeTargets.All, Inherited = false, AllowMultiple = true)]
    sealed class IntegrationTest : System.Attribute
    {
        readonly int orderNumber;

        public IntegrationTest(int orderNumber)
        {
            this.orderNumber = orderNumber;
        }

        public int OrderNumber
        {
            get { return orderNumber; }
        }
    }
}

Get-DscIntegrationTestOrderNumber helper function:

function Get-DscIntegrationTestOrderNumber
{
    [CmdletBinding()]
    [OutputType([System.UInt32])]
    param
    (
        [Parameter()]
        [ValidateNotNullOrEmpty()]
        [String]
        $Path
    )

    <#
        Will always return $null if the attribute 'IntegrationTest'
        is not found with the named attribute argument 'OrderNumber'.
    #>
    $returnValue = $null

    # Change WarningAction so it not output a warning for the sealed class.
    Add-Type -Path (Join-Path -Path $PSScriptRoot -ChildPath 'Microsoft.DscResourceKit.cs') -WarningAction SilentlyContinue

    $scriptBlockAst = [System.Management.Automation.Language.Parser]::ParseFile($Path, [ref] $null, [ref] $null)

    $findDSCIntegrationTestOrderAttributeFilter = {
        $args[0] -is [System.Management.Automation.Language.AttributeAst] `
        -and (
            $args[0].TypeName.FullName -eq 'IntegrationTest' `
            -or $args[0].TypeName.FullName -eq 'Microsoft.DscResourceKit.IntegrationTest'
        )
    }

    # Get IntegrationTest attribute in the file if it exist.
    [System.Management.Automation.Language.Ast[]] $dscIntegrationTestOrderAttributeAst = `
        $scriptBlockAst.Find($findDSCIntegrationTestOrderAttributeFilter, $true)

    if ($dscIntegrationTestOrderAttributeAst)
    {
        $findOrderNumberNamedAttributeArgumentFilter = {
            $args[0] -is [System.Management.Automation.Language.NamedAttributeArgumentAst] `
            -and $args[0].ArgumentName -eq 'OrderNumber'
        }

        [System.Management.Automation.Language.Ast[]] $orderNumberNamedAttributeArgumentAst = `
            $dscIntegrationTestOrderAttributeAst.Find($findOrderNumberNamedAttributeArgumentFilter, $true)

        if ($orderNumberNamedAttributeArgumentAst)
        {
            $returnValue = $orderNumberNamedAttributeArgumentAst.Argument.Value
        }
    }

    return $returnValue
}

[johlju]

@PlagueHO I refactored this according to your suggestion. It improved the usage a lot. I tried to get PowerShell 4.0 compatibility, I hope the method I used will work with PowerShell 4.0. I have not tested that, I have no machine with PowerShell 4.0 available.
Please suggest other namespace, attribute, named attribute arguments names if there are better.

Tested OK here; https://ci.appveyor.com/project/johlju/xsqlserver/build/6.0.924.0#L43

---

Review status: 0 of 5 files reviewed at latest revision, 9 unresolved discussions.

---

README.md, line 338 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Non-blocking: Could we use something like shared AppVeyor module model rather than Default-model?

Done.

---

README.md, line 340 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Non-blocking: Perhaps put parameter in back ticks with a dash to make it clear that is is a parameter -Type. Same on next line.

Done.

---

README.md, line 343 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Non-blocking: Could add an example (e.g. SharePointDsc) to make it clear what a "Harness"-model is?

Done.

---

README.md, line 350 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Many integration test resources actually pass the Hash table in using a -ConfigurationData parameter (e.g https://github.com/PowerShell/xStorage/blob/dev/Tests/Integration/MSFT_xDiskAccessPath.Integration.Tests.ps1#L60). It might be worth pointing out that in these cases a hash table will need to be added to the config regardless.

No longer relevant.

---

README.md, line 351 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Non-blocking: Should name-value be key-value?

No longer relevant.

---

README.md, line 369 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Non-blocking: Should name-value be key-value?

No longer relevant.

---

Tests/Unit/TestHelper.Tests.ps1, line 18 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can we change to named parameter?

Done.

---

Tests/Unit/TestHelper.Tests.ps1, line 21 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can we change to named parameter?

Done.

---

Tests/Unit/TestHelper.Tests.ps1, line 24 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can we change to named parameter?

Done.

---

Comments from Reviewable

[PlagueHO]

Fantastic stuff @johlju - really awesome as always. I'm just traveling at the moment but will be able to review on Friday night. But really awesome and this is very cool!

[johlju]

Thanks! 😄 This will help a lot in xSQLServer, hopefully we can improve on this for other stuff to :)

Please let me know if we should rename the names of the namespace, attribute etc. I had poor imagination when I tried to figure what would be the best names in the long run 😄

[PlagueHO]

Only some minor spelling/grammar things! Looking great! 😁

---

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

---

AppVeyor.psm1, line 312 at r5 (raw file):

                $integrationTestConfigurationFiles = Get-ChildItem @getChildItemParameters

                # In each file, search for existens of key value pair 'DscIntegrationTestOrder'.

This comment needs to be updated to match new attribute method

---

README.md, line 351 at r5 (raw file):

Also, each integration test configuration file ('*.config.ps1') must be decorated
with a attribute 'IntegrationTest' containing a name attribute argument

with an attribute

Also, could the attribute full name be used here and perhaps in back ticks? E.g.

with an attribute Microsoft.DscResourceKit.IntegrationTest ..

---

README.md, line 359 at r5 (raw file):

value 1 will be run before integration tests with value 2. If an integration test
does not have a assigned order, it will be run unordered after all ordered tests
has been run.

have been run.

---

README.md, line 366 at r5 (raw file):

and the integration test file is named 'MSFT_xSQLServerSetup.Integration.Tests.ps1'.

Example how the configuration file could look like to make sure a integration test

make sure an integration test

---

TestHelper.psm1, line 1258 at r5 (raw file):

<#
    .SYNOPSIS
        Returns the integration test order number if it exist in the attribute

if it exists in the attribute

---

TestHelper.psm1, line 1259 at r5 (raw file):

    .SYNOPSIS
        Returns the integration test order number if it exist in the attribute
        'IntegrationTest' with the named attribute argument 'OrderNumber'.

Maybe include the full name of the attribute Microsoft.DscResourceKit.IntegrationTest

---

TestHelper.psm1, line 1279 at r5 (raw file):

    <#
        Will always return $null if the attribute 'IntegrationTest'

Use the full name? Microsoft.DscResourceKit.IntegrationTest

---

TestHelper.psm1, line 1284 at r5 (raw file):

    $returnValue = $null

    # Change WarningAction so it not output a warning for the sealed class.

so it does not output

---

Comments from Reviewable

[johlju]

I changed a variable that also used the old attribute name.

---

Review status: 2 of 5 files reviewed at latest revision, 8 unresolved discussions.

---

AppVeyor.psm1, line 312 at r5 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

This comment needs to be updated to match new attribute method

Done.

---

README.md, line 351 at r5 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

with an attribute

Also, could the attribute full name be used here and perhaps in back ticks? E.g.

with an attribute Microsoft.DscResourceKit.IntegrationTest ..

Done.

---

README.md, line 359 at r5 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

have been run.

Done.

---

README.md, line 366 at r5 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

make sure an integration test

Done.

---

TestHelper.psm1, line 1258 at r5 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

if it exists in the attribute

Done.

---

TestHelper.psm1, line 1259 at r5 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Maybe include the full name of the attribute Microsoft.DscResourceKit.IntegrationTest

Done.

---

TestHelper.psm1, line 1279 at r5 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Use the full name? Microsoft.DscResourceKit.IntegrationTest

Done.

---

TestHelper.psm1, line 1284 at r5 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

so it does not output

Done.

---

Comments from Reviewable

[PlagueHO]

Nice job @johlju !

@kwirkykat - any chance of a merge if you're OK with this?

---

Reviewed 3 of 3 files at r6.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[johlju]

@kwirkykat do you have a chance to look this over? Would love to get this merged so I can continue my work over at xSQLServer, which depends on this.