Merge pull request #17 from KevinMarquette/develop !deploy

Integrated PSScriptAnalyzer into Pester tests and corrected any discovered issues
KevinMarquette · Feb 8, 2017 · 40b2bb3 · 40b2bb3
2 parents e550213 + ff03394
commit 40b2bb3
Show file tree

Hide file tree

Showing 18 changed files with 83 additions and 62 deletions.
diff --git a/PSGraph/PSGraph.psd1 b/PSGraph/PSGraph.psd1
@@ -72,7 +72,7 @@ Description = 'Builds graphs using GraphViz'
 FunctionsToExport = @('Edge','Export-PSGraph','Graph','Inline','Install-GraphViz','Node','Rank','SubGraph')
 
 # Cmdlets to export from this module, for best performance, do not use wildcards and do not delete the entry, use an empty array if there are no cmdlets to export.
-CmdletsToExport = '*'
+CmdletsToExport = @()
 
 # Variables to export from this module
 VariablesToExport = '*'

diff --git a/PSGraph/Private/Format-Value.ps1 b/PSGraph/Private/Format-Value.ps1
@@ -11,7 +11,7 @@ function Format-Value
 
     begin
     {
-        if($Script:CustomFormat -eq $null)
+        if( $null -eq $Script:CustomFormat )
         {
             Set-NodeFormatScript
         }

diff --git a/PSGraph/Private/Get-GraphVizArguments.ps1 → PSGraph/Private/Get-GraphVizArgument.ps1 b/PSGraph/Private/Get-GraphVizArguments.ps1 → PSGraph/Private/Get-GraphVizArgument.ps1
@@ -1,11 +1,11 @@
-function Get-GraphVizArguments
+function Get-GraphVizArgument
 {
     <#
         .Description
         Takes an array and converts it to commandline arguments for GraphViz
 
         .Example
-        Get-GraphVizArguments -InputObject @{OutputFormat='jpg'}
+        Get-GraphVizArgument -InputObject @{OutputFormat='jpg'}
 
         .Notes
         If no destination is provided, it will set the auto name flag.
@@ -26,8 +26,8 @@ function Get-GraphVizArguments
     {        
         if($InputObject -ne $null)
         {
-            $InputObject = Update-DefaultArguments -InputObject $InputObject  
-            $arguments = Get-TranslatedArguments -InputObject $InputObject  
+            $InputObject = Update-DefaultArgument -InputObject $InputObject  
+            $arguments = Get-TranslatedArgument -InputObject $InputObject  
         }
 
         return $arguments

diff --git a/PSGraph/Private/Get-Indent.ps1 b/PSGraph/Private/Get-Indent.ps1
@@ -4,7 +4,7 @@ function Get-Indent
     param($depth=$script:indent)
     process
     {
-        if($depth -eq $null -or $depth -lt 0)
+        if( $null -eq $depth -or $depth -lt 0 )
         {
             $depth = 0
         }

diff --git a/PSGraph/Private/Get-TranslatedArguments.ps1 → PSGraph/Private/Get-TranslatedArgument.ps1 b/PSGraph/Private/Get-TranslatedArguments.ps1 → PSGraph/Private/Get-TranslatedArgument.ps1
@@ -1,13 +1,12 @@
-function Get-TranslatedArguments($InputObject)
+function Get-TranslatedArgument($InputObject)
 {
     $paramLookup = Get-ArgumentLookUpTable
-    $arguments = @()
 
     Write-Verbose 'Walking parameter mapping'
     foreach($key in $InputObject.keys)
     {
         Write-Debug $key
-        if($key -ne $null -and $paramLookup.ContainsKey($key))
+        if( $null -ne $key -and $paramLookup.ContainsKey($key))
         {
             $newArgument = $paramLookup[$key]
             if($newArgument -like '*{0}*')

diff --git a/PSGraph/Private/Update-DefaultArguments.ps1 → PSGraph/Private/Update-DefaultArgument.ps1 b/PSGraph/Private/Update-DefaultArguments.ps1 → PSGraph/Private/Update-DefaultArgument.ps1
@@ -1,5 +1,6 @@
-function Update-DefaultArguments
+function Update-DefaultArgument
 {
+     [Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSUseShouldProcessForStateChangingFunctions","")]
     [cmdletbinding()]
     param($inputObject)
 

diff --git a/PSGraph/Public/Edge.ps1 b/PSGraph/Public/Edge.ps1
@@ -128,7 +128,7 @@ function Edge
         { #Deducing the pattern 'edge @{}' as default edge definition
              node 'edge' -attributes $Node[0]
         }
-        if($Node -ne $null)
+        elseif($null -ne $Node )
         { # Used when scripted properties are specified
             foreach($item in $Node)
             {            
@@ -141,12 +141,12 @@ function Edge
         } 
         else
         {
-            if ($Attributes -ne $null -and [string]::IsNullOrEmpty($LiteralAttribute))
+            if ($null -ne $Attributes -and [string]::IsNullOrEmpty($LiteralAttribute))
             {
                 $GraphVizAttribute = ConvertTo-GraphVizAttribute -Attributes $Attributes
             }        
 
-            if ($To -ne $null)
+            if ($null -ne $To)
             { # If we have a target array, cross multiply results
                 foreach($sNode in $From)
                 {                    

diff --git a/PSGraph/Public/Export-PSGraph.ps1 b/PSGraph/Public/Export-PSGraph.ps1
@@ -30,7 +30,7 @@ function Export-PSGraph
         It checks the piped data for file paths. If it can't find a file, it assumes it is graph data.
         This may give unexpected errors when the file does not exist.
     #>
-
+    [Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSAvoidUsingInvokeExpression","")]
     [cmdletbinding()]
     param(
         # The GraphViz file to process or contents of the graph in Dot notation
@@ -84,7 +84,7 @@ function Export-PSGraph
         # Select only items with 'dot' BaseName and use first one
         $graphViz = Resolve-Path -path $GraphVizPath -ErrorAction SilentlyContinue | Get-Item | Where-Object BaseName -eq 'dot' | Select-Object -First 1
 
-        if($graphViz -eq $null)
+        if( $null -eq $graphViz )
         {
             throw "Could not find GraphViz installed on this system. Please run 'Install-GraphViz' to install the needed binaries and libraries. This module just a wrapper around GraphViz and is looking for it in your program files folder. Optionally pass a path to your dot.exe file with the GraphVizPath parameter"
         }
@@ -96,16 +96,16 @@ function Export-PSGraph
     process
     {     
 
-        if($Source -ne $null)
+        if( $null -ne $Source )
         {
             # if $Source is a list of files, process each one
             $fileList = Resolve-Path -Path $Source -ea 0
-            if($fileList -ne $null)
+            if( $null -ne $fileList )
             {
-                foreach($file in $fileList )
+                foreach( $file in $fileList )
                 {     
                     Write-Verbose "Generating graph from '$($file.path)'"
-                    $arguments = Get-GraphVizArguments -InputObject $PSBoundParameters
+                    $arguments = Get-GraphVizArgument -InputObject $PSBoundParameters
                     & $graphViz @($arguments + $file.path)
                 }
             } 
@@ -129,7 +129,7 @@ function Export-PSGraph
                 $file = [System.IO.Path]::GetRandomFileName()               
                 $PSBoundParameters["DestinationPath"] = Join-Path $env:temp "$file.$OutputFormat"            
             }
-            $arguments = Get-GraphVizArguments $PSBoundParameters
+            $arguments = Get-GraphVizArgument $PSBoundParameters
              Write-Verbose " Arguments: $($arguments -join ' ')"
 
             $standardInput.ToString() | & $graphViz @($arguments)

diff --git a/PSGraph/Public/Graph.ps1 b/PSGraph/Public/Graph.ps1
@@ -20,7 +20,9 @@ function Graph
         .Notes
         The output is a string so it can be saved to a variable or piped to other commands
     #>
+    [Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSAvoidDefaultValueForMandatoryParameter","")]
     [cmdletbinding(DefaultParameterSetName='Default')]
+    [OutputType([string])]
     param(
 
         # Name or ID of the graph

diff --git a/PSGraph/Public/Node.ps1 b/PSGraph/Public/Node.ps1
@@ -27,6 +27,7 @@ function Node
         I had conflits trying to alias Get-Node to node, so I droped the verb from the name.
         If you have subgraphs, it works best to define the node inside the subgraph before giving it an edge
     #>
+    [Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSAvoidDefaultValueForMandatoryParameter","")]
     [cmdletbinding()]
     param(
         # The name of the node

diff --git a/PSGraph/Public/Rank.ps1 b/PSGraph/Public/Rank.ps1
@@ -59,13 +59,13 @@ function Rank
     process
     {
         $itemList = New-Object System.Collections.Queue
-        if($Nodes -ne $null)
+        if($null -ne $Nodes)
         {
-            $Nodes | %{$_} | %{$itemList.Enqueue($_)}
+            $Nodes | ForEach-Object{$_} | ForEach-Object{$itemList.Enqueue($_)}
         }
-        if($AdditionalNodes -ne $null)
+        if($null -ne $AdditionalNodes)
         {
-            $AdditionalNodes | %{$_} | %{$_} | %{$itemList.Enqueue($_)}
+            $AdditionalNodes | ForEach-Object{$_} | ForEach-Object{$_} | ForEach-Object{$itemList.Enqueue($_)}
         }
 
         $Values += foreach($item in $itemList)

diff --git a/PSGraph/Public/Set-NodeFormatScript.ps1 b/PSGraph/Public/Set-NodeFormatScript.ps1
@@ -10,12 +10,17 @@ function Set-NodeFormatScript
         .Notes
         This can be used if different datasets are not consistent.
     #>
-    [cmdletbinding()]
+    [cmdletbinding(SupportsShouldProcess)]
     param(
 
         # The Scriptblock used to process every node value
         [ScriptBlock]
         $ScriptBlock = {$_}
     )
-    $Script:CustomFormat = $ScriptBlock
+
+    if($PSCmdlet.ShouldProcess('Change default code id format function'))
+    {
+         $Script:CustomFormat = $ScriptBlock
+    }
+
 }
diff --git a/PSGraph/Public/SubGraph.ps1 b/PSGraph/Public/SubGraph.ps1
@@ -18,7 +18,7 @@ function SubGraph
         This is just like the graph or digraph, except the name must match cluster_#
         The numbering must start at 0 and work up or the processor will fail.
     #>
-
+    [Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSAvoidDefaultValueForMandatoryParameter","")]
     [cmdletbinding(DefaultParameterSetName='Default')]
     param(
         # Numeric ID of subgraph starting at 0
@@ -27,7 +27,7 @@ function SubGraph
             Position = 0
         )]
         [int]
-        $ID = 0,
+        $ID,
 
         # The commands to execute inside the subgraph
         [Parameter(
@@ -53,7 +53,7 @@ function SubGraph
             ParameterSetName='Attributes'
         )]
         [hashtable]
-        $Attributes = @{}      
+        $Attributes = @{}
     )
 
     process

diff --git a/Tests/Help.Tests.ps1 b/Tests/Help.Tests.ps1
@@ -20,8 +20,11 @@ Describe "Help tests for $moduleName" -Tags Build {
             }
             foreach($parameter in $node.parameters.parameter)
             {
-                it "parameter $($parameter.name) has a description" {
-                    $parameter.Description.text | Should Not BeNullOrEmpty
+                if($parameter -notmatch 'whatif|confirm')
+                {
+                    it "parameter $($parameter.name) has a description" {
+                        $parameter.Description.text | Should Not BeNullOrEmpty
+                    }
                 }
             }
         }

diff --git a/Tests/PrivateFunctions.Tests.ps1 b/Tests/PrivateFunctions.Tests.ps1
@@ -41,18 +41,18 @@ InModuleScope -ModuleName PSGraph {
         }
 
 
-        Context "Get-GraphVizArguments" {
+        Context "Get-GraphVizArgument" {
 
             It "Does not throw an error" {
-                {Get-GraphVizArguments} | Should Not Throw
+                {Get-GraphVizArgument} | Should Not Throw
             }
 
             It "Should not throw an error with empty hashtable" {
-                {Get-GraphVizArguments @{}} | Should Not Throw
+                {Get-GraphVizArgument @{}} | Should Not Throw
             }
 
             It "Should not throw an error with hashtable" {
-                {Get-GraphVizArguments @{OutputFormat='png'}} | Should Not Throw
+                {Get-GraphVizArgument @{OutputFormat='png'}} | Should Not Throw
             }
         }  
 
@@ -75,18 +75,18 @@ InModuleScope -ModuleName PSGraph {
         Context "Get-TranslatedArguments" {
 
             It "Does not throw an error" {
-                {Get-TranslatedArguments} | Should Not Throw
+                {Get-TranslatedArgument} | Should Not Throw
             }
             It "Translates DestinationPath" {
-                Get-TranslatedArguments @{DestinationPath='test.png'} | Should be '-otest.png'
-                Get-TranslatedArguments @{DestinationPath='test.png'} | Should not be '-o test.png'
+                Get-TranslatedArgument @{DestinationPath='test.png'} | Should be '-otest.png'
+                Get-TranslatedArgument @{DestinationPath='test.png'} | Should not be '-o test.png'
             }
         }  
 
-        Context "Update-DefaultArguments" {
+        Context "Update-DefaultArgument" {
 
             It "Does not throw an error" {
-                {Update-DefaultArguments @{}} | Should Not Throw
+                {Update-DefaultArgument @{}} | Should Not Throw
             }
         }
 

diff --git a/Tests/Project.Tests.ps1 b/Tests/Project.Tests.ps1
@@ -4,19 +4,34 @@ $moduleName = Split-Path $moduleRoot -Leaf
 
 Describe "General project validation: $moduleName" -Tags Build {
 
-    $scripts = Get-ChildItem $projectRoot -Include *.ps1,*.psm1,*.psd1 -Recurse
+    Context "Valid Powershell" {
+        $scripts = Get-ChildItem $projectRoot -Include *.ps1,*.psm1,*.psd1 -Recurse | where fullname -notmatch 'classes'
+        # TestCases are splatted to the script so we need hashtables
+        $testCase = $scripts | Foreach-Object{@{file=$_}}   
 
-    # TestCases are splatted to the script so we need hashtables
-    $testCase = $scripts | Foreach-Object{@{file=$_}}         
-    It "Script <file> should be valid powershell" -TestCases $testCase {
-        param($file)
+        It "Script <file> should be valid powershell" -TestCases $testCase {
+            param($file)
 
-        $file.fullname | Should Exist
+            $file.fullname | Should Exist
 
-        $contents = Get-Content -Path $file.fullname -ErrorAction Stop
-        $errors = $null
-        $null = [System.Management.Automation.PSParser]::Tokenize($contents, [ref]$errors)
-        $errors.Count | Should Be 0
+            $contents = Get-Content -Path $file.fullname -ErrorAction Stop
+            $errors = $null
+            $null = [System.Management.Automation.PSParser]::Tokenize($contents, [ref]$errors)
+            $errors.Count | Should Be 0
+        }
+    }
+
+    Context "ScriptAnalyzer" {
+
+        $scripts = Get-ChildItem $moduleRoot -Include *.ps1,*.psm1,*.psd1 -Recurse | where fullname -notmatch 'classes'
+        $testCase = $scripts | Foreach-Object{@{file=$_}} 
+
+        it "Script <file> should pass ScriptAnalyzer rules" -TestCases $testCase {
+            param($file)
+
+            $file.fullname | Should Exist 
+            Invoke-ScriptAnalyzer $file| Should BeNullOrEmpty
+        }
     }
 
     It "Module '$moduleName' can import cleanly" {

diff --git a/build.ps1 b/build.ps1
@@ -11,9 +11,9 @@ param ($Task = 'Default')
 # Grab nuget bits, install modules, set build variables, start build.
 Get-PackageProvider -Name NuGet -ForceBootstrap | Out-Null
 
-Install-Module Psake, PSDeploy, BuildHelpers -force
+Install-Module Psake, PSDeploy, BuildHelpers, PSScriptAnalyzer -force
 Install-Module Pester -Force -SkipPublisherCheck 
-Import-Module Psake, BuildHelpers
+Import-Module Psake, BuildHelpers, PSScriptAnalyzer
 
 Set-BuildEnvironment
 

diff --git a/psake.ps1 b/psake.ps1
@@ -28,14 +28,12 @@ Task Init {
     "Build System Details:"
     Get-Item ENV:BH* | Format-List
     "`n"
-    "Check for node conflict"
-    Get-Command node | Select-Object *
 }
 
 Task UnitTests -Depends Init {
     $lines
     'Running quick unit tests to fail early if there is an error'
-    $TestResults = Invoke-Pester -Path $ProjectRoot\Tests\*unit* -PassThru -Tag Build -Show Failed
+    $TestResults = Invoke-Pester -Path $ProjectRoot\Tests\*unit* -PassThru -Tag Build 
 
     if($TestResults.FailedCount -gt 0)
     {
@@ -49,17 +47,14 @@ Task Test -Depends UnitTests  {
     "`n`tSTATUS: Testing with PowerShell $PSVersion"
 
     # Gather test results. Store them in a variable and file
-    $TestResults = Invoke-Pester -Path $ProjectRoot\Tests -PassThru -OutputFormat NUnitXml -OutputFile "$ProjectRoot\$TestFile" -Tag Build -Show Failed
+    $TestResults = Invoke-Pester -Path $ProjectRoot\Tests -PassThru -OutputFormat NUnitXml -OutputFile "$ProjectRoot\$TestFile" -Tag Build
 
     # In Appveyor?  Upload our tests! #Abstract this into a function?
     If($ENV:BHBuildSystem -eq 'AppVeyor')
     {
         "Uploading $ProjectRoot\$TestFile to AppVeyor"
         "JobID: $env:APPVEYOR_JOB_ID"
-        (New-Object 'System.Net.WebClient').UploadFile(
-            "https://ci.appveyor.com/api/testresults/nunit/$($env:APPVEYOR_JOB_ID)",
-            (Resolve-Path "$ProjectRoot\$TestFile" )
-        )
+        (New-Object 'System.Net.WebClient').UploadFile("https://ci.appveyor.com/api/testresults/nunit/$($env:APPVEYOR_JOB_ID)", (Resolve-Path "$ProjectRoot\$TestFile"))
     }
 
     Remove-Item "$ProjectRoot\$TestFile" -Force -ErrorAction SilentlyContinue
@@ -93,7 +88,7 @@ Task Build -Depends Test {
     $version = [version]::New($version.Major,$version.Minor,$version.Build,$env:BHBuildNumber)
     Write-Host "Using version: $version"
 
-    Update-Metadata -Path $env:BHPSModuleManifest -PropertyName ModuleVersion -Value $Version
+    Update-Metadata -Path $env:BHPSModuleManifest -PropertyName ModuleVersion -Value $version
 }
 
 Task Deploy -Depends Build {