Refactor Invoke-Pester and validate configuration early #2317

fflaten · 2023-03-04T22:57:24Z

PR Summary

The code for Invoke-Pester has become huge over the years and hard to read and maintain. This PR splits out parts of the code to plugin-steps and helper-functions.

It also moves configuration validation to the start to avoid longs runs being lost in the end due to a typo.

Extract parameter set migration for Simple and Legacy-set
Call TestResult-export through plugin End-step
Move CodeCoverage processing to plugin-steps Start, RunEnd and End
Validate configuration early to avoid running a long run just to loose it at the end when it throws due to invalid format for coverage, testresult etc. (ex. RuntimeExceptions for invalid output configuration doesn't kill the run #2304)
- Also cleaned up the error message for invalid StringOption-value so they all look consistent.

Fix #2304

PR Checklist

PR has meaningful title
Summary describes changes
PR is ready to be merged
- If not, use the arrow next to Create Pull Request to mark it as a draft. PR can be marked Ready for review when it's ready.
Tests are added/update (if required)
Documentation is updated/added (if required)

src/functions/Coverage.Plugin.ps1

+                try {
+                    $xmlWriter.Close()
+                }
+                catch {


src/functions/Coverage.Plugin.ps1

+                try {
+                    $stringWriter.Close()
+                }
+                catch {


src/functions/TestResults.ps1

fflaten · 2023-03-07T22:50:03Z

Throwing in Start plugin-step creates an ugly and duplicate error atm. Likely due to being thrown through Assert-Success. Might be better to use own function like Resolve-OutputConfiguration

The duplicate was because Assert-Success printed the error while also throwing it. Suggest removing the host message.

Thoughts on where to validate config, especially string-options? Options:

Start plugin-step. Removed the duplicate message, but still a bit verbose due to Assert-Success
Validation-function, ex. Resolve-OutputConfiguration
Validate in assembly for StringOption. Would block third-party plugins from integrating with built-in options like TestResult.OutputFormat, but do we want to support that?
A new ConfigurePlugin/SetupPlugin step invoked without Invoke-PluginStep or a different branch to simplify error. Maybe invoked before Find-File call which can be slow. This might also benefit public plugin API.

fflaten · 2023-03-07T23:36:54Z

src/Pester.Runtime.ps1

+        $rc++
        $ec = 0
        if ($null -ne $r.ErrorRecord -and $r.ErrorRecord.Length -gt 0) {
-            $err += "Result $($rc++):"
            $anyFailed = $true
            foreach ($e in $r.ErrorRecord) {
-                $err += "Error $($ec++):"
+                $err += "$([Environment]::NewLine)Result $rc - Error $((++$ec)):"


Previous message didn't print $ec and $rc (only incremented them) due to missing parantheses.

fflaten · 2023-03-25T12:31:57Z

/azp run

azure-pipelines · 2023-03-25T12:32:06Z

Azure Pipelines successfully started running 1 pipeline(s).

nohwnd · 2023-04-06T07:33:55Z

src/Pester.Runtime.ps1

@@ -1216,7 +1227,7 @@ function Invoke-PluginStep {
    # this is end step, we should run all steps no matter if some failed, and we should run them in opposite direction
    # only do this if there is more than 1, to avoid the "expensive" -like check and reverse
    $isEndStep = 1 -lt $pluginsWithGivenStep.Count -and $Step -like "*End"
-    if (-not $isEndStep) {
+    if ($isEndStep) {


So this did not reverse the steps before, did you see any bugs linked to that?

No bugs I'm aware of. Just not behaving like comment says. We reverse on TearDown-steps as expected, but this currently reverses on everything else than *End.

Not a critical part of the PR, just testing it out while in draft. Will need to revert this or change plugin-order as it (PR-code) now runs CC and nunit-reports before writing test-report to screen. Either way it's confusing/inconsistent atm (pre-PR).

fflaten · 2023-04-15T16:08:17Z

Throwing in Start plugin-step creates an ugly and duplicate error atm. Likely due to being thrown through Assert-Success. Might be better to use own function like Resolve-OutputConfiguration

The duplicate was because Assert-Success printed the error while also throwing it. Suggest removing the host message.

Thoughts on where to validate config, especially string-options? Options:

Start plugin-step. Removed the duplicate message, but still a bit verbose due to Assert-Success

Validation-function, ex. Resolve-OutputConfiguration

Validate in assembly for StringOption. Would block third-party plugins from integrating with built-in options like TestResult.OutputFormat, but do we want to support that?

A new ConfigurePlugin/SetupPlugin step invoked without Invoke-PluginStep or a different branch to simplify error. Maybe invoked before Find-File call which can be slow. This might also benefit public plugin API.

Thoughts on this @nohwnd?

Fix pester#2304

Fixes result count and error count not being printed and updates formats. Also removes the host message because it's duplicated when exception is thrown.

fflaten · 2023-05-14T13:06:34Z

Rebased and updated with validation-functions for now. Can revisit later when looking into public plugins API.

Also cleaned up the error message for invalid StringOption. It now list option and supported values just like CodeCoverage did.

fflaten · 2023-05-14T13:11:12Z

src/Pester.Runtime.ps1

                $err += & $SafeCommands["Out-String"] -InputObject $e
                $err += & $SafeCommands["Out-String"] -InputObject $e.ScriptStackTrace
            }
        }
    }

    if ($anyFailed) {
-        $Message = $Message + ":`n$err"
-        Write-PesterHostMessage -ForegroundColor Red $Message


Revert this or good? Not validating configuration in plugin-step now, so PR is not affected by the duplicate error from Assert-Success. Not sure if it's useful somewhere else

nohwnd · 2023-06-07T08:01:58Z

@fflaten Approved both this and your test output refactoring PRs, they seem like they might run into each other so I was not sure in which order you want them merged, please merge any way you like :)

fflaten · 2023-06-07T09:15:55Z

@nohwnd Thanks! Let's start with this. It will also cause a conflict in the last PR for container data. Can fix both later today. 🙂

fflaten · 2023-06-07T09:40:41Z

Btw. Don't have merge rights, so up to you :)

nohwnd · 2023-06-07T12:41:02Z

@fflaten you do now.

github-advanced-security bot found potential problems Mar 4, 2023

View reviewed changes

fflaten commented Mar 7, 2023

View reviewed changes

nohwnd reviewed Apr 6, 2023

View reviewed changes

fflaten added 17 commits May 14, 2023 10:49

Extract validation of Output-configuraiton

0dace9e

Resolve CodeCoverage config in plugin Start-step

83f60b8

Invoke RunEnd plugin-steps

39654cc

End CodeCoverage analysis in RunEnd-step

c5a337f

Generate Coverage report in plugin End-step

e67fc35

Move TestResult to own plugin

4f66c73

Use List for plugins

efd43a8

Validate all Output-configuration early

1180b5f

Fix pester#2304

Use splatting when creating Coverage plugin

fc8ff72

Extract Simple and Legacy migrations to functions

e97a607

Fix missing newlines

16b5230

Fix broken configuration tests

99b955f

Update Assert-Success error

773bcf3

Fixes result count and error count not being printed and updates formats. Also removes the host message because it's duplicated when exception is thrown.

Validate SkipRemainingOnFailure config in Start

b47c600

Cleanup comments

4a01048

Add helper for invalid stringoption errormessage

21aceb8

Validate plugin configuration early

7520096

fflaten force-pushed the invokepester-refactor branch from c7e716d to 7520096 Compare May 14, 2023 13:06

fflaten commented May 14, 2023

View reviewed changes

fflaten changed the title ~~WIP - Refactoring Invoke-Pester and early configuration-validation~~ Refactor Invoke-Pester and validate configuration early May 14, 2023

fflaten marked this pull request as ready for review May 14, 2023 13:21

fflaten requested a review from nohwnd May 14, 2023 13:21

Refactor Join-And and Join-Or

0ca5d9a

nohwnd approved these changes Jun 7, 2023

View reviewed changes

fflaten merged commit 997002c into pester:main Jun 7, 2023
14 checks passed

fflaten deleted the invokepester-refactor branch June 7, 2023 14:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor Invoke-Pester and validate configuration early #2317

Refactor Invoke-Pester and validate configuration early #2317

fflaten commented Mar 4, 2023 •

edited

fflaten commented Mar 7, 2023 •

edited

fflaten Mar 7, 2023

fflaten commented Mar 25, 2023

azure-pipelines bot commented Mar 25, 2023

nohwnd Apr 6, 2023

fflaten Apr 6, 2023 •

edited

fflaten commented Apr 15, 2023 •

edited

fflaten commented May 14, 2023

fflaten May 14, 2023

nohwnd Jun 7, 2023

nohwnd commented Jun 7, 2023

fflaten commented Jun 7, 2023

fflaten commented Jun 7, 2023

nohwnd commented Jun 7, 2023

Refactor Invoke-Pester and validate configuration early #2317

Refactor Invoke-Pester and validate configuration early #2317

Conversation

fflaten commented Mar 4, 2023 • edited

PR Summary

PR Checklist

fflaten commented Mar 7, 2023 • edited

fflaten Mar 7, 2023

Choose a reason for hiding this comment

fflaten commented Mar 25, 2023

azure-pipelines bot commented Mar 25, 2023

nohwnd Apr 6, 2023

Choose a reason for hiding this comment

fflaten Apr 6, 2023 • edited

Choose a reason for hiding this comment

fflaten commented Apr 15, 2023 • edited

fflaten commented May 14, 2023

fflaten May 14, 2023

Choose a reason for hiding this comment

nohwnd Jun 7, 2023

Choose a reason for hiding this comment

nohwnd commented Jun 7, 2023

fflaten commented Jun 7, 2023

fflaten commented Jun 7, 2023

nohwnd commented Jun 7, 2023

fflaten commented Mar 4, 2023 •

edited

fflaten commented Mar 7, 2023 •

edited

fflaten Apr 6, 2023 •

edited

fflaten commented Apr 15, 2023 •

edited