-
Notifications
You must be signed in to change notification settings - Fork 13
*.Tests.ps1 files with no Describe blocks get executed #16
Comments
This extension depends on the PowerShell Preview extension. Do you have that running? Does the Integrated Console start? |
Of course. Just opened VSCode, had intergrated console running and enabled your extension. It crashes shortly as soon as I go to tests. I open Dev console and those are the errors I see:
|
I'll take a look! Thanks! |
@PrzemyslawKlys do you have Pester 5 installed? |
In the Output pane under "Pester Explorer Log" in the drop down can you share what's in there? |
NOTE: I just published 0.0.6 which should have a bit more polish. Maybe it'll work better for you. |
I have no pester explorer log in the output pane:
PS C:\Support\GitHub\GpoZaurr> get-module -ListAvailable pester
Directory: C:\Program Files\WindowsPowerShell\Modules
ModuleType Version Name ExportedCommands
---------- ------- ---- ----------------
Script 5.1.0 Pester {Invoke-Pester, Describe, Context, It...}
Script 3.4.0 Pester {Describe, Context, It, Should...} |
@PrzemyslawKlys can you double check there's no output? I've noticed that the drop down to select the output scrolls and found Pester Test Explorer towards the bottom. |
I have opened it, and then let it crash and was able to get logs
|
Now I'm thinking, is the behavior good that it actually on first start executes my tests? Should it maybe prompt if you want to enable that? |
So the tests shouldn't be running... and I don't see how they could be possibly running... very strange. Do you have autorun turned on at all for Test Explorer extensions maybe? In any case, there's a couple problems here... we'll get through them... I'm made a small change that will make sure that won't kill the whole extension. Lets see what that fixes for you. |
(That was released like... 30s ago so it may take a bit to show up) |
I've installed Test Extension just because you asked me to (to install your extension). Never configured it. Executes on start:
|
ah... I have a suspicion |
I noticed before that on Windows I had issues with quotes not being escaped... I've now fixed every single instance of that released in 0.0.8. That would explain why your tests were running because the file wasn't being treated as a string but rather a script file that needs to be executed... Let's see |
Yes, doesn't crash now
Tests are there, tests explorer shows no tests. |
yep. I messed it up. ok :) once sec |
Ok 0.0.9 is out. I had thought I tested my original change but I was running old code... oops... I've tested this on Windows with 5.1 so I'm feeling pretty confident here. |
Started crashing again
So we're back to before. Again it seems like it's executing... |
ugh... I don't get how we have such different experiences... but I've added logging that will print out the exact script that I run. Let's see what that gets. |
Log:
Log2:
|
@PrzemyslawKlys can you run that script and tell me what you get? $Path = @(
'c:\Support\GitHub\GpoZaurr\GPOZaurr.Tests.ps1'
'c:\Support\GitHub\GpoZaurr\Tests\GPOPermissions.Tests.ps1'
'c:\Support\GitHub\GpoZaurr\Tests\GPOOwners.Tests.ps1'
)
Import-Module Pester -MinimumVersion 5.0.0 -ErrorAction Stop
function Discover-Test
{
[CmdletBinding()]
param(
[Parameter(Mandatory)]
[String[]] $Path,
[String[]] $ExcludePath
)
& (Get-Module Pester) {
param (
$Path,
$ExcludePath,
$SessionState)
Reset-TestSuiteState
# to avoid Describe thinking that we run in interactive mode
$invokedViaInvokePester = $true
$files = Find-File -Path $Path -ExcludePath $ExcludePath -Extension $PesterPreference.Run.TestExtension.Value
$containers = foreach ($f in $files) {
New-BlockContainerObject -File (Get-Item $f)
}
Find-Test -BlockContainer $containers -SessionState $SessionState } -Path $Path -ExcludePath $ExcludePath -SessionState $PSCmdlet.SessionState
}
function New-SuiteObject ($Block) {
[PSCustomObject]@{
type = 'suite'
id = $Block.ScriptBlock.File + ';' + $Block.ScriptBlock.StartPosition.StartLine
file = $Block.ScriptBlock.File
line = $Block.ScriptBlock.StartPosition.StartLine - 1
label = $Block.Name
children = [Collections.Generic.List[Object]]@()
}
}
function New-TestObject ($Test) {
[PSCustomObject]@{
type = 'test'
id = $Test.ScriptBlock.File + ';' + $Test.ScriptBlock.StartPosition.StartLine
file = $Test.ScriptBlock.File
line = $Test.ScriptBlock.StartPosition.StartLine - 1
label = $Test.Name
}
}
function fold ($children, $Block) {
foreach ($b in $Block.Blocks) {
$o = (New-SuiteObject $b)
$children.Add($o)
fold $o.children $b
}
foreach ($t in $Block.Tests) {
$children.Add((New-TestObject $t))
}
}
$found = Discover-Test -Path $Path
# whole suite
$suite = [PSCustomObject]@{
Blocks = [Collections.Generic.List[Object]] $found
Tests = [Collections.Generic.List[Object]]@()
}
$testSuiteInfo = [PSCustomObject]@{
type = 'suite'
id = 'root'
label = 'Pester'
children = [Collections.Generic.List[Object]]@()
}
foreach ($file in $found) {
$fileSuite = [PSCustomObject]@{
type = 'suite'
id = $file.BlockContainer.Item.FullName
file = $file.BlockContainer.Item.FullName
label = $file.BlockContainer.Item.Name
children = [Collections.Generic.List[Object]]@()
}
$testSuiteInfo.children.Add($fileSuite)
fold $fileSuite.children $file
}
$testSuiteInfo | ConvertTo-Json -Depth 100 All this is suppose to do is give a JSON representation of what tests you have. |
I may have to use EncodedCommand if running the script by itself works |
It fully executes my Tests. Keep in mind that:
|
As soon as I renamed the root -> Everything showed up. So it seems you execute the first file (which you shouldn't I guess). That's the content of it: https://github.com/EvotecIT/GPOZaurr/blob/master/GPOZaurr.Tests.ps1 Here are remaining tests: https://github.com/EvotecIT/GPOZaurr/tree/master/Tests |
I see... ok now I understand. So perhaps I can check to see if there is a I think it's unusual that you have a @nohwnd, do you have any thoughts on this? |
So the problem is really in Pester since we run internal cmdlets of Pester: Import-Module Pester -MinimumVersion 5.0.0 -ErrorAction Stop
function Discover-Test
{
[CmdletBinding()]
param(
[Parameter(Mandatory)]
[String[]] $Path,
[String[]] $ExcludePath
)
# THIS WILL RUN INSIDE OF PESTER SCOPE
& (Get-Module Pester) {
param (
$Path,
$ExcludePath,
$SessionState)
Reset-TestSuiteState
# to avoid Describe thinking that we run in interactive mode
$invokedViaInvokePester = $true
$files = Find-File -Path $Path -ExcludePath $ExcludePath -Extension $PesterPreference.Run.TestExtension.Value
$containers = foreach ($f in $files) {
New-BlockContainerObject -File (Get-Item $f)
}
Find-Test -BlockContainer $containers -SessionState $SessionState } -Path $Path -ExcludePath $ExcludePath -SessionState $PSCmdlet.SessionState
}
Discover-Test -Path 'c:\Support\GitHub\GpoZaurr\GPOZaurr.Tests.ps1' The question is... what part actually invokes your script? We have some suspects: Import-Module Pester -MinimumVersion 5.0.0 -ErrorAction Stop
function Discover-Test
{
[CmdletBinding()]
param(
[Parameter(Mandatory)]
[String[]] $Path,
[String[]] $ExcludePath
)
& (Get-Module Pester) {
param (
$Path,
$ExcludePath,
$SessionState)
Reset-TestSuiteState
# to avoid Describe thinking that we run in interactive mode
$invokedViaInvokePester = $true
# SUSPECT 1
$files = Find-File -Path $Path -ExcludePath $ExcludePath -Extension $PesterPreference.Run.TestExtension.Value
# SUSPECT 2
$containers = foreach ($f in $files) {
New-BlockContainerObject -File (Get-Item $f)
}
# SUSPECT 3
Find-Test -BlockContainer $containers -SessionState $SessionState } -Path $Path -ExcludePath $ExcludePath -SessionState $PSCmdlet.SessionState
}
Discover-Test -Path 'c:\Support\GitHub\GpoZaurr\GPOZaurr.Tests.ps1' Can you do this for me? I want you to run ^ that script but:
Once we figure out which cmdlet is to blame, we can work with Pester maintainers on what should be done. |
Suspect number 3 executes my code. |
I wanted to make sure I took the time to respond to this. I sympathize with you and think that it's fair to ask for what you're asking for... however, the work involved would be quite substantial. The root of the problem is that PowerShell is a dynamic language first, then static language 2nd What do I mean by this? PowerShell module's are imported with The question Pester is asking is... "What tests do I have in this file?" and fortunately or not this is done at runtime. Take the following valid example (that you should never do!): $collection = @(1,2,3,4,5)
foreach ($item in $collection) {
Describe "DescribeName" {
It "ItName" {
$item
}
}
}
When I run
5 tests. Same deal with the discovery script: > Discover-Test -Path /Users/tyleonha/Code/Misc/PowerShell/foo.Tests.ps1 | % Blocks | measure
Count : 5 You and I both know... it's 5 because of the foreach loop... but let's say the So now we arrive at the issue you're seeing. You have
I hope that makes sense. The reality is, I don't think Pester can fix this in an elegant way where everyone is happy. If Pester scans the file for a & "Describe" {
1 | Should -Be 1
} Or some other way to dynamically execute a Function... and that would break. The easiest path forward would be for you to move to a model where you change The second easiest path forward would be for me to maybe allow you to specify files you want to ignore in the discovery. The hardest way forward would be for Pester to somehow know better than to run your script. |
I understand all that. But:
I mean I can disable your plugin, I can go and change my approach for 30+ projects - but you can't simply assume proper naming convention and then say "oh well - it's by design". It will not be just me. It may as well be 50 other people that won't know what's happening - and worst it will auto-execute whatever is in there - and we can't be sure what is in Tests files. I don't want to make your life harder - just think that for the product to be safe & usable you have to do more thinking about what the user may do wrong - and try to prevent it. You do have the ability to do so with your extensive knowledge - just time + effort ;-) |
I agree. Captured that here #18
I can see 3 approaches to fixing this. Both not good enough: 1. "Read in the script as a string and search for
|
My tests in Tests folder are pretty direct: I can execute each test separately and it works. My execution script is mostly used when I want to execute all tests or is directed for use in Azure. It doesn't do anything fancy - simply check if modules that are required by the script are there, and if not it will install them. In other words - it's basically preparing the environment for test execution, eventually doing It's typically not required for the execution of tests - as I expect all modules to be already there.
Of course - that's just me. The safest option would be to do a "prescan" but let users choose what they believe holds the tests - but you would need to provide "description" of what it should be - because I could see people just selecting everything. I do understand the complexity of this. Just want to make sure no accidental damage is done. |
How about this. I will do a very very basic search of "Is the word 'Describe' in this file?"... if it's not, then I will skip over that file. This should unblock you, at least, and although it leaves an edge case or 2, hopefully, if this actually affects someone, they will open an issue here for a further discussion... |
Ok 0.0.11 has been released. Please give it a go when you have a sec |
Much better now.
Not sure if it's by design - or requires work. |
What would rather it say/do?
🎉
As in it didn't work? Can you get those logs for me in this case?
Yeah so again, this is basically running |
|
|
So if I run it in test explorer - it updates it correctly when picking test by test. When doing the same thing but for global (on the image marked as 3) it doesn't update anything. |
@PrzemyslawKlys latest version has new feature to set the test root: "pesterExplorer.testRootDirectory": "Tests" This should allow:
|
Cool. Worked, and marked tests as done correctly. |
Awesome! You're unblocked 😁 please share the extension with your friends ;) |
Maybe there should be a default set to Tests -> or some autodetection? I don't think I ever seen a project that puts tests in root? |
Yeah that's fair. 0.0.13 will now discover "test", "tests", "Test", "Tests" folders and use the first one it finds. |
Hi,
Just tried to run your extension. It doesn't go well.
The text was updated successfully, but these errors were encountered: