Issue# 114 - VesterTemplate.Tests.ps1 Performance Updates

[midacts]

I added jpsider's suggestion

• _Move the check for null items in the foreach ($scope) loop.

• Only add valid ScopeObj's

• if (($ScopeObj.InventoryList -ne $NULL) -and ($ScopeObj.TestFiles -ne $NULL)){
• $Final += $ScopeObj
• }
• This will only add populated values to the new collection._

I did not add the other suggestions. I think the way where you get the object and loop through each test is how it is currently.

I'm not sure that we need these lines anymore:

    # The parent folder must be one of these names, to help with $Object scoping below
    # If adding here, also needs to be added to the switch below
    If ('vCenter|Datacenter|Cluster|DSCluster|Host|VM|Network' -notmatch $Scope) {
        Write-Warning "Skipping test $TestName. Use -Verbose for more details"
        Write-Verbose 'Test files should be in a folder with one of the following names:'
        Write-Verbose 'vCenter / Datacenter / Cluster / Host / VM / Network'
        Write-Verbose 'This helps Vester determine which inventory object(s) to use during the test.'
        # Use continue to skip this test and go to the next loop iteration
        continue
    }

and

    If (($Final | Where-Object { $_.Scope -eq $Scope }).InventoryList -eq $null) {
        Write-Verbose "No objects found in scope $Scope, skipping tests"
        # Use continue to skip this test and go to the next loop iteration
        continue
    }    

mainly because of lines 51-53

[midacts]

All the suggested changes have went through and I have tested them. Let me know if you guys see anything else.

Thanks again for your review and suggestions.

[midacts]

That was a good find (I always miss stuff like that).

[brianbunke]

First, thanks to everyone for this effort! I definitely apologize for this sitting so long. It's a very visible change, so I wanted to give it a test drive myself, and I'm just getting to that now.

I haven't reviewed every change to the code yet, but I have one suggestion that I'm adding to my new review.

In the PR as is, the many Get- calls are resolved (yay!), but it suffers from a different speed problem. Within each test, a new Describe block is created for every single object, and then the "is the config value null?" check is performed. This makes null value tests take far longer, because if a single VM test's config value is null, it still has to create a new Describe block and then skip for every single VM you have.

[brianbunke]

I should note, the combination of changes in this PR (including the suggestion in my review) resulted in my benchmark test just now going from 150 seconds down to 65.

Seems pretty good 👍

[brianbunke]

I believe the ForEach ($Object in $Inventory) { should move below the $null check at line 108. This PR right now, lines 92-108:

• for each Vester test,
• for each VMware inventory object,
• create a unique Describe block,
• and check if the $cfg value for this test is $null

What matches current behavior, and what I believe to be correct:

• for each Vester test,
• create one Describe block,
• check if the $cfg value for this test is $null,
• and then loop through each VMware inventory object

Moving the Describe block & $cfg value check up really speeds up tests where there is a $null value. I see this in action when many VM hardening tests run against >100 VMs.

[midacts]

I added the requested changes. Let me know if you see anything else that needs to change

[brianbunke]

Kudos to @Midacts for sticking with this, and much love to @jpsider and @jonneedham for the reviews. As far as I'm concerned, Vester just doubled in speed. 🍾

:shipit:

[midacts]

No problem. I am glad we were all able to work together to find the best solution. Glad to be a part of the team.