Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reattach TestDrive and TestRegistry after running inner Invoke-Pester #2341

Merged
merged 7 commits into from May 9, 2023

Conversation

fflaten
Copy link
Collaborator

@fflaten fflaten commented Apr 15, 2023

PR Summary

When running Pester inside Pester, it would delete the existing/parent TestDrive/TestRegistry.
This PR detects, stores and reattaches the previous TestDrive/TestRegistry drives before returning to the parent instance.

Also remaps TestDrive/TestRegistry at end of block if missing. This can occur if user code removes it or a inner/nester Invoke-Pester failed and never got to reattach it.

Fix #2147

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/TestRegistry.ps1 Dismissed Show dismissed Hide dismissed
}
else {
# this will inherit to child scopes and affect behavior of ex. TestDrive/TestRegistry
$runningPesterInPester = $true

Check warning

Code scanning / PSScriptAnalyzer

The variable 'runningPesterInPester' is assigned but never used. Warning

The variable 'runningPesterInPester' is assigned but never used.
Copy link
Collaborator Author

@fflaten fflaten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments for questions. Likely merge conflict in #2337.

@@ -638,6 +638,11 @@ function Invoke-Pester {
# this will inherit to child scopes and allow Describe / Context to run directly from a file or command line
$invokedViaInvokePester = $true

if ($null -ne $state) {
# this will inherit to child scopes and affect behavior of ex. TestDrive/TestRegistry
$runningPesterInPester = $true
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we track this (and $invokedViaInvokePester) in $state-prop instead of inheriting the variables?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, why not.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saving for future PR :)

src/Main.ps1 Show resolved Hide resolved
src/functions/TestDrive.ps1 Show resolved Hide resolved
src/functions/TestDrive.ps1 Show resolved Hide resolved
Comment on lines -45 to -47
#publish the global TestDrive variable used in few places within the module
if (-not (& $SafeCommands['Test-Path'] "Variable:Global:$DriveName")) {
& $SafeCommands['New-Variable'] -Name $DriveName -Scope Global -Value $directory
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this intentional to not steal a user variable?
If so, I can revert/change to only override a Pester-created value based on value.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't remember the history, maybe it was originally not mapping TestDrive to a random directory, and inner Pester simply did not map it's own TestDrive, but instead inherited it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it's been there since v2, probably to avoid nested Describe-blocks (which I assume was supported) to overwrite the TestDrive. Similar check for existing PSDrive. Should be safe to remove now.

}

function New-TestDrive ([Switch]$PassThru, [string] $Path) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PSScriptAnalyzer doesn't track parameters properly without a param-block it seems, so changed a few functions. Ex. this has a unused $PassThru switch. Removed since this is internal.

@fflaten fflaten changed the title Keep and reattach parent TestDrive and TestRegistry after running inner Invoke-Pester Reattach parent TestDrive and TestRegistry after running inner Invoke-Pester Apr 16, 2023
@fflaten
Copy link
Collaborator Author

fflaten commented Apr 25, 2023

Marking as ready for review, but might get a merge conflict as mentioned above.

@fflaten fflaten marked this pull request as ready for review April 25, 2023 19:53
@nohwnd
Copy link
Member

nohwnd commented May 6, 2023

Reviewed and approved. Please fix the conflict :)

@fflaten fflaten changed the title Reattach parent TestDrive and TestRegistry after running inner Invoke-Pester Reattach TestDrive and TestRegistry after running inner Invoke-Pester May 7, 2023
@nohwnd nohwnd merged commit 7a101ce into pester:main May 9, 2023
14 checks passed
@fflaten fflaten deleted the nested-testdriveregistry branch May 9, 2023 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TestDrive Cleaning Early with Nested Invoke-Pester
2 participants