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

Fix using [ScriptBlock]::Create with InModuleScope. #1146

Merged
merged 3 commits into from
Nov 22, 2018

Conversation

sethvs
Copy link
Contributor

@sethvs sethvs commented Nov 14, 2018

1. General summary of the pull request

We use ScriptBlock with InModuleScope.

Now we can use it like this:

Describe "{}" {
    $ScriptBlock = {Write-Output -InputObject 'This is a ScriptBlock'}
    Import-Module -Name PowerShellGet
    InModuleScope -ModuleName PowerShellGet -ScriptBlock $ScriptBlock
}

the result is:

Describing {}

But if we need to construct ScriptBlock dynamicly by using [ScriptBlock]::Create method

Describe "[ScriptBlock]::Create" {
    $ScriptBlockText = "Write-Output -InputObject 'This is a ScriptBlock'"
    $ScriptBlock = [ScriptBlock]::Create($ScriptBlockText)
    Import-Module -Name PowerShellGet
    InModuleScope -ModuleName PowerShellGet -ScriptBlock $ScriptBlock
}

the result will be:

Describing [ScriptBlock]::Create
  [-] Error occurred in Describe block 49ms
    ParameterBindingValidationException: Cannot bind argument to parameter 'SessionStateInternal' because it is null.

This is because, when InModuleScope function gets $originalScriptBlockScope of the ScriptBlock

$originalScriptBlockScope = Get-ScriptBlockScope -ScriptBlock $ScriptBlock

the result will be $null.

It seems, that ScriptBlock created with Create method doesn't have SessionStateInternal property.
For example this code returns null:

$ScriptBlockText = "Write-Output -InputObject 'This is a ScriptBlock'"
$ScriptBlock = [ScriptBlock]::Create($ScriptBlockText)
$flags = [System.Reflection.BindingFlags]'Instance,NonPublic'
[scriptblock].GetProperty('SessionStateInternal', $flags).GetValue($ScriptBlock, $null)

So, when InModuleScope function tries to return the $originalScriptBlockScope to the ScriptBlock

Set-ScriptBlockScope -ScriptBlock $ScriptBlock -SessionStateInternal $originalScriptBlockScope

the Set-ScriptBlockScope throws an error, because -SessionStateInternal parameter cannot be null.

This PS allows -SessionStateInternal parameter to accept null, so that ScriptBlock will have its original SessionStateInternal back, even if it is $null.

@nohwnd
Copy link
Member

nohwnd commented Nov 14, 2018

@sethvs nice catch. Could you add tests for it as well please? 🙂

@nohwnd nohwnd added the Bug label Nov 14, 2018
@sethvs
Copy link
Contributor Author

sethvs commented Nov 20, 2018

Sorry for delay :)

@@ -40,6 +40,15 @@ Describe "Executing test code inside a module" {
}
}

It "Can use ScriptBlock inside the module scope" {
$ScriptBlockOne = { Write-Output "I am a ScriptBlockOne" }
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 see any need to mix the two tests cases together. Could you split them to Can execute bound ScriptBlock inside the module scope ({} makes scriptblock bound to the current session) and Can execute unbound ScriptBlock in module scope please? :)

@sethvs
Copy link
Contributor Author

sethvs commented Nov 21, 2018

Is it OK with Should swallow test output without -PassThru test in PowerShell v2?

@nohwnd
Copy link
Member

nohwnd commented Nov 21, 2018

@sethvs sorry I don't get what you are asking. Could you rephrase please?

@sethvs
Copy link
Contributor Author

sethvs commented Nov 21, 2018

Publish Status to GitHub (Pester) check is failed on test with name Should swallow test output without -PassThru when it was run on PowerShell v2.
Just want to say, I haven't touched this test :)

@nohwnd
Copy link
Member

nohwnd commented Nov 21, 2018

@sethvs Now I get it. There is some ongoing problem with the build server, that makes some of the tests fail, I am
not sure if it's the problem of the test or the server. Plus it's really hard to investigate when the server waits 3 hours to run the build. I tried running it again to see if the error stays the same.

@nohwnd nohwnd merged commit 231077f into pester:master Nov 22, 2018
@nohwnd
Copy link
Member

nohwnd commented Nov 22, 2018

@sethvs Merged, thanks ! 👍

@sethvs sethvs deleted the setScriptBlockScope branch November 22, 2018 11:30
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.

2 participants