Throw when no test cases passed to -ForEach

[fsackur]

Summary of the feature request

• given that "no test cases" results in no failures
• and given that we don't want to manually review every pipeline run
• and given that scopes can be tricky ;-)
  • then when I pass $null to Context -ForEach (or Describe or It)
  • I should get a failed test
• and (needs discussion)
  • when I pass an empty collection to -ForEach
  • I should get a failed test

How should it work? (optional)

I would like to see this become default in some future version. For now, it's a breaking change, so we need an explicit param (or pester config option?).

# Test-Foo.Tests.ps1
BeforeDiscovery {
    $Module = Import-Module Pester -PassThru
    $TestCases = (1, 2, 3)
}

InModuleScope $Module {
    # This is a bug; $TestCases is not defined in the module scope
    Describe "Adding" -ForEach $TestCases {
        It "Adds <_>" {2 + $_ | Should -BeGreaterThan 2}
    }
}

Current behaviour:

> Invoke-Pester -Output Detailed .\Test-Foo.Tests.ps1
Pester v5.3.1

Starting discovery in 1 files.
Discovery found 0 tests in 10ms.
Running tests.
Tests completed in 9ms
Tests Passed: 0, Failed: 0, Skipped: 0 NotRun: 0

The above output is green - that is the whole problem.

Suggested behaviour:

# Test-Foo.Tests.ps1
Describe "Adding" -ForEach $null -AssertNotNullOrEmpty {
    It "Adds <_>" {2 + $_ | Should -BeGreaterThan 2}
}

> Invoke-Pester -Output Detailed .\Test-Foo.Tests.ps1
Pester v5.3.1

Starting discovery in 1 files.
[-] Discovery in C:\githubdata\WHAM-CLI\Test-Foo.Tests.ps1 failed with:
System.Exception: Expected a value for ForEach, but got $null or empty.
blah
blah
Running tests.

Running tests from '.\Test-Foo.Tests.ps1'
Describing Adding
  [-] Container failed with:
    System.Exception: Expected a value for ForEach, but got $null or empty.
Tests completed in 135ms
Tests Passed: 0, Failed: 1, Skipped: 0 NotRun: 0

Alternatively, and possibly better:

$Config = New-PesterConfiguration
$Config.Should.FailOnEmptyForEach = $true

[fflaten]

Thanks for the feedback.

Empty testcases (count 0) is a legit scenario when using data-driven/generated tests, so failing that would be wrong imo.

If an option would be added, I'd probably vote for the first parameter as it's feels more flexible in data-driven tests.

Non-existing variable references could be caught using Set-StrictMode -Version Latest. It needs to be set inside InModuleScope in your example. Would that work for you?

[johlju]

I refactor unit tests between Pester 4 to Pester 5, and in one instance I by accident saw that one test using ForEach did not run any tests. This was due to a bug in my refactor, I forgot to move a block to the BeforeDiscovery-block so the variable where set, but did not contain any test cases. There could be similar bugs in my refactor that I did not see. So see the benefit with an option to toggle that a ForEach reports an error if there is no tests.

@fsackur In regards to the example in the issue description, as per the warning notice at https://pester.dev/docs/usage/modules, we should avoid wrapping other Pester blocks inside InModuleScope, but instead using InModuleScope only inside Before*, After*and It-blocks. That would have at least solved that particular scenario. 🙂

[nohwnd]

I have no idea why I did not make that the default for -TestCases/-ForEach, I even think it was the default to throw on empty at some point in time. I probably had issues reporting errors from discovery, or I forgot.

Personally I think it is the better option to have it fail by default and have a parameter that allows empty. I would like to make that the default in the next major version of Pester. So how about this:

$Config = New-PesterConfiguration
$Config.Run.FailOnNullOrEmptyForEach = $true

Describe "Adding" -ForEach $null -AllowNullOrEmptyForEach { }
It "Adding" -ForEach $null -AllowNullOrEmptyForEach { }

The FailOnEmptyForEach would be $false in Pester v5, and $true in v6. And if you want to adopt the option early you can simply just enable it. There would be no option to go the other way around.

[johlju]

Works for me to have that as an option in the settings, I would enable it for v5,, and I'm also okay with that it defaults to $true in v6.

[erikgraa]

I met this need during some Pester V4 to Pester V5 conversion. The proposed solution looks great.