Throw when providing null or empty array to -ForEach (#2537)

* Add Run.FailOnNullOrEmptyForEach option * Add override option to It Describe and Context * Throw on empty or null ForEach * Add tests * Fix tests with empty ForEach * Update configuration option description
pester · Jul 10, 2024 · 5f5251d · 5f5251d
1 parent 1fbcde8
commit 5f5251d
Show file tree

Hide file tree

Showing 10 changed files with 250 additions and 47 deletions.
diff --git a/src/csharp/Pester/RunConfiguration.cs b/src/csharp/Pester/RunConfiguration.cs
@@ -32,6 +32,7 @@ public class RunConfiguration : ConfigurationSection
         private BoolOption _passThru;
         private BoolOption _skipRun;
         private StringOption _skipRemainingOnFailure;
+        private BoolOption _failOnNullOrEmptyForEach;
 
         public static RunConfiguration Default { get { return new RunConfiguration(); } }
         public static RunConfiguration ShallowClone(RunConfiguration configuration)
@@ -53,6 +54,7 @@ public RunConfiguration(IDictionary configuration) : this()
                 configuration.AssignValueIfNotNull<bool>(nameof(PassThru), v => PassThru = v);
                 configuration.AssignValueIfNotNull<bool>(nameof(SkipRun), v => SkipRun = v);
                 configuration.AssignObjectIfNotNull<string>(nameof(SkipRemainingOnFailure), v => SkipRemainingOnFailure = v);
+                configuration.AssignValueIfNotNull<bool>(nameof(FailOnNullOrEmptyForEach ), v => FailOnNullOrEmptyForEach  = v);
             }
         }
 
@@ -68,6 +70,7 @@ public RunConfiguration(IDictionary configuration) : this()
             PassThru = new BoolOption("Return result object to the pipeline after finishing the test run.", false);
             SkipRun = new BoolOption("Runs the discovery phase but skips run. Use it with PassThru to get object populated with all tests.", false);
             SkipRemainingOnFailure = new StringOption("Skips remaining tests after failure for selected scope, options are None, Run, Container and Block.", "None");
+            FailOnNullOrEmptyForEach  = new BoolOption("Fails discovery when -ForEach is provided $null or @() in a block or test. Can be overriden for a specific Describe/Context/It using -AllowNullOrEmptyForEach.", true);
         }
 
         public StringArrayOption Path
@@ -229,5 +232,21 @@ public StringOption SkipRemainingOnFailure
                 }
             }
         }
+
+        public BoolOption FailOnNullOrEmptyForEach
+        {
+            get { return _failOnNullOrEmptyForEach; }
+            set
+            {
+                if (_failOnNullOrEmptyForEach == null)
+                {
+                    _failOnNullOrEmptyForEach = value;
+                }
+                else
+                {
+                    _failOnNullOrEmptyForEach = new BoolOption(_failOnNullOrEmptyForEach, value.Value);
+                }
+            }
+        }
     }
 }
diff --git a/src/en-US/about_PesterConfiguration.help.txt b/src/en-US/about_PesterConfiguration.help.txt
@@ -69,6 +69,10 @@ SECTIONS AND OPTIONS
         Type: string
         Default value: 'None'
 
+        FailOnNullOrEmptyForEach: Fails discovery when -ForEach is provided $null or @() in a block or test. Can be overriden for a specific Describe/Context/It using -AllowNullOrEmptyForEach.
+        Type: bool
+        Default value: $true
+
     Filter:
         Tag: Tags of Describe, Context or It to be run.
         Type: string[]

diff --git a/src/functions/Context.ps1 b/src/functions/Context.ps1
@@ -26,6 +26,10 @@
     Use this parameter to explicitly mark the block to be skipped. This is preferable to temporarily
     commenting out a block, because it remains listed in the output.
 
+    .PARAMETER AllowNullOrEmptyForEach
+    Allows empty or null values for -ForEach when Run.FailOnNullOrEmptyForEach is enabled.
+    This might be excepted in certain scenarios like using external data.
+
     .PARAMETER ForEach
     Allows data driven tests to be written.
     Takes an array of data and generates one block for each item in the array, and makes the item
@@ -85,6 +89,7 @@
 
         # [Switch] $Focus,
         [Switch] $Skip,
+        [Switch] $AllowNullOrEmptyForEach,
 
         [Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidAssignmentToAutomaticVariable', '', Justification = 'ForEach is not used in Foreach-Object loop')]
         $ForEach
@@ -107,13 +112,15 @@
         }
 
         if ($PSBoundParameters.ContainsKey('ForEach')) {
-            if ($null -ne $ForEach -and 0 -lt @($ForEach).Count) {
-                New-ParametrizedBlock -Name $Name -ScriptBlock $Fixture -StartLine $MyInvocation.ScriptLineNumber -StartColumn $MyInvocation.OffsetInLine -Tag $Tag -FrameworkData @{ CommandUsed = 'Context'; WrittenToScreen = $false } -Focus:$Focus -Skip:$Skip -Data $ForEach
+            if ($null -eq $ForEach -or 0 -eq @($ForEach).Count) {
+                if ($PesterPreference.Run.FailOnNullOrEmptyForEach.Value -and -not $AllowNullOrEmptyForEach) {
+                    throw [System.ArgumentException]::new('Value can not be null or empty array. If this is expected, use -AllowNullOrEmptyForEach', 'ForEach')
+                }
+                # @() or $null is provided and allowed, do nothing
+                return
             }
-            else {
-                # @() or $null is provided do nothing
 
-            }
+            New-ParametrizedBlock -Name $Name -ScriptBlock $Fixture -StartLine $MyInvocation.ScriptLineNumber -StartColumn $MyInvocation.OffsetInLine -Tag $Tag -FrameworkData @{ CommandUsed = 'Context'; WrittenToScreen = $false } -Focus:$Focus -Skip:$Skip -Data $ForEach
         }
         else {
             New-Block -Name $Name -ScriptBlock $Fixture -StartLine $MyInvocation.ScriptLineNumber -Tag $Tag -FrameworkData @{ CommandUsed = 'Context'; WrittenToScreen = $false } -Focus:$Focus -Skip:$Skip

diff --git a/src/functions/Describe.ps1 b/src/functions/Describe.ps1
@@ -28,6 +28,10 @@
     Use this parameter to explicitly mark the block to be skipped. This is preferable to temporarily
     commenting out a block, because it remains listed in the output.
 
+    .PARAMETER AllowNullOrEmptyForEach
+    Allows empty or null values for -ForEach when Run.FailOnNullOrEmptyForEach is enabled.
+    This might be excepted in certain scenarios like using external data.
+
     .PARAMETER ForEach
     Allows data driven tests to be written.
     Takes an array of data and generates one block for each item in the array, and makes the item
@@ -93,6 +97,7 @@
 
         # [Switch] $Focus,
         [Switch] $Skip,
+        [Switch] $AllowNullOrEmptyForEach,
 
         [Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidAssignmentToAutomaticVariable', '', Justification = 'ForEach is not used in Foreach-Object loop')]
         $ForEach
@@ -115,12 +120,15 @@
         }
 
         if ($PSBoundParameters.ContainsKey('ForEach')) {
-            if ($null -ne $ForEach -and 0 -lt @($ForEach).Count) {
-                New-ParametrizedBlock -Name $Name -ScriptBlock $Fixture -StartLine $MyInvocation.ScriptLineNumber -StartColumn $MyInvocation.OffsetInLine -Tag $Tag -FrameworkData @{ CommandUsed = 'Describe'; WrittenToScreen = $false } -Focus:$Focus -Skip:$Skip -Data $ForEach
-            }
-            else {
-                # @() or $null is provided do nothing
+            if ($null -eq $ForEach -or 0 -eq @($ForEach).Count) {
+                if ($PesterPreference.Run.FailOnNullOrEmptyForEach.Value -and -not $AllowNullOrEmptyForEach) {
+                    throw [System.ArgumentException]::new('Value can not be null or empty array. If this is expected, use -AllowNullOrEmptyForEach', 'ForEach')
+                }
+                # @() or $null is provided and allowed, do nothing
+                return
             }
+
+            New-ParametrizedBlock -Name $Name -ScriptBlock $Fixture -StartLine $MyInvocation.ScriptLineNumber -StartColumn $MyInvocation.OffsetInLine -Tag $Tag -FrameworkData @{ CommandUsed = 'Describe'; WrittenToScreen = $false } -Focus:$Focus -Skip:$Skip -Data $ForEach
         }
         else {
             New-Block -Name $Name -ScriptBlock $Fixture -StartLine $MyInvocation.ScriptLineNumber -Tag $Tag -FrameworkData @{ CommandUsed = 'Describe'; WrittenToScreen = $false } -Focus:$Focus -Skip:$Skip

diff --git a/src/functions/It.ps1 b/src/functions/It.ps1
@@ -31,6 +31,10 @@
     Use this parameter to explicitly mark the test to be skipped. This is preferable to temporarily
     commenting out a test, because the test remains listed in the output.
 
+    .PARAMETER AllowNullOrEmptyForEach
+    Allows empty or null values for -ForEach when Run.FailOnNullOrEmptyForEach is enabled.
+    This might be excepted in certain scenarios like using external data.
+
     .PARAMETER ForEach
     (Formerly called TestCases.) Optional array of hashtable (or any IDictionary) objects.
     If this parameter is used, Pester will call the test script block once for each table in
@@ -128,7 +132,8 @@
         [String[]] $Tag,
 
         [Parameter(ParameterSetName = 'Skip')]
-        [Switch] $Skip
+        [Switch] $Skip,
+        [Switch] $AllowNullOrEmptyForEach
 
         # [Parameter(ParameterSetName = 'Skip')]
         # [String] $SkipBecause,
@@ -148,12 +153,15 @@
     }
 
     if ($PSBoundParameters.ContainsKey('ForEach')) {
-        if ($null -ne $ForEach -and 0 -lt @($ForEach).Count) {
-            New-ParametrizedTest -Name $Name -ScriptBlock $Test -StartLine $MyInvocation.ScriptLineNumber -StartColumn $MyInvocation.OffsetInLine -Data $ForEach -Tag $Tag -Focus:$Focus -Skip:$Skip
-        }
-        else {
-            # @() or $null is provided do nothing
+        if ($null -eq $ForEach -or 0 -eq @($ForEach).Count) {
+            if ($PesterPreference.Run.FailOnNullOrEmptyForEach.Value -and -not $AllowNullOrEmptyForEach) {
+                throw [System.ArgumentException]::new('Value can not be null or empty array. If this is expected, use -AllowNullOrEmptyForEach', 'ForEach')
+            }
+            # @() or $null is provided and allowed, do nothing
+            return
         }
+
+        New-ParametrizedTest -Name $Name -ScriptBlock $Test -StartLine $MyInvocation.ScriptLineNumber -StartColumn $MyInvocation.OffsetInLine -Data $ForEach -Tag $Tag -Focus:$Focus -Skip:$Skip
     }
     else {
         New-Test -Name $Name -ScriptBlock $Test -StartLine $MyInvocation.ScriptLineNumber -Tag $Tag -Focus:$Focus -Skip:$Skip

diff --git a/tst/Format2.Tests.ps1 b/tst/Format2.Tests.ps1
@@ -17,8 +17,7 @@ InPesterModuleScope {
 
 
     Describe "Format-Collection2" {
-        It "Formats empty collection to @()" -TestCases @(
-        ) {
+        It "Formats empty collection to @()" {
             Format-Collection2 -Value @() | Verify-Equal "@()"
         }
 

diff --git a/tst/Pester.RSpec.Configuration.ts.ps1 b/tst/Pester.RSpec.Configuration.ts.ps1
@@ -55,6 +55,10 @@ i -PassThru:$PassThru {
             [PesterConfiguration]::Default.Run.SkipRemainingOnFailure.Value | Verify-Equal "None"
         }
 
+        t 'Run.FailOnNullOrEmptyForEach is $true' {
+            [PesterConfiguration]::Default.Run.FailOnNullOrEmptyForEach.Value | Verify-Equal $true
+        }
+
         # Output configuration
         t "Output.Verbosity is Normal" {
             [PesterConfiguration]::Default.Output.Verbosity.Value | Verify-Equal "Normal"