Replace Id with GroupId in Block and Test objects (#2364)

* Replace Block/Test Id with GroupId to be explicit * Remove old tests * Use line:column as group id --------- Co-authored-by: Jakub Jareš <me@jakubjares.com>
pester · Apr 19, 2024 · 9d44623 · 9d44623
1 parent fa6f0b2
commit 9d44623
Show file tree

Hide file tree

Showing 9 changed files with 39 additions and 231 deletions.
diff --git a/src/Pester.Runtime.ps1 b/src/Pester.Runtime.ps1
@@ -167,21 +167,22 @@ function New-ParametrizedBlock {
         [Parameter(Mandatory = $true)]
         [ScriptBlock] $ScriptBlock,
         [int] $StartLine = $MyInvocation.ScriptLineNumber,
+        [int] $StartColumn = $MyInvocation.OffsetInLine,
         [String[]] $Tag = @(),
         [HashTable] $FrameworkData = @{ },
         [Switch] $Focus,
         [Switch] $Skip,
         $Data
     )
 
-    # using the position of Describe/Context as Id to group data-generated blocks. Should be unique enough because it only needs to be unique for the current block, so the way to break this would be to inline multiple blocks with ForEach, but that is unlikely to happen. When it happens just use StartLine:StartPosition
+    # using the position of Describe/Context as Id to group data-generated blocks. Should be unique enough because it only needs to be unique for the current block
     # TODO: Id is used by NUnit2.5 and 3 testresults to group. A better way to solve this?
-    $id = $StartLine
+    $groupId = "${StartLine}:${StartColumn}"
 
     foreach ($d in @($Data)) {
         # shallow clone to give every block it's own copy
         $fmwData = $FrameworkData.Clone()
-        New-Block -Id $id -Name $Name -ScriptBlock $ScriptBlock -StartLine $StartLine -Tag $Tag -FrameworkData $fmwData -Focus:$Focus -Skip:$Skip -Data $d
+        New-Block -GroupId $groupId -Name $Name -ScriptBlock $ScriptBlock -StartLine $StartLine -Tag $Tag -FrameworkData $fmwData -Focus:$Focus -Skip:$Skip -Data $d
     }
 }
 
@@ -197,7 +198,7 @@ function New-Block {
         [String[]] $Tag = @(),
         [HashTable] $FrameworkData = @{ },
         [Switch] $Focus,
-        [String] $Id,
+        [String] $GroupId,
         [Switch] $Skip,
         $Data
     )
@@ -235,7 +236,7 @@ function New-Block {
     $block.StartLine = $StartLine
     $block.FrameworkData = $FrameworkData
     $block.Focus = $Focus
-    $block.Id = $Id
+    $block.GroupId = $GroupId
     $block.Skip = $Skip
     $block.Data = $Data
 
@@ -482,7 +483,7 @@ function New-Test {
         [int] $StartLine = $MyInvocation.ScriptLineNumber,
         [String[]] $Tag = @(),
         $Data,
-        [String] $Id,
+        [String] $GroupId,
         [Switch] $Focus,
         [Switch] $Skip
     )
@@ -504,7 +505,7 @@ function New-Test {
     }
 
     $test = [Pester.Test]::Create()
-    $test.Id = $Id
+    $test.GroupId = $GroupId
     $test.ScriptBlock = $ScriptBlock
     $test.Name = $Name
     # using the non-expanded name as default to fallback to it if we don't
@@ -2645,18 +2646,19 @@ function New-ParametrizedTest () {
         [Parameter(Mandatory = $true, Position = 1)]
         [ScriptBlock] $ScriptBlock,
         [int] $StartLine = $MyInvocation.ScriptLineNumber,
+        [int] $StartColumn = $MyInvocation.OffsetInLine,
         [String[]] $Tag = @(),
         # do not use [hashtable[]] because that throws away the order if user uses [ordered] hashtable
         [object[]] $Data,
         [Switch] $Focus,
         [Switch] $Skip
     )
 
-    # using the position of It as Id for the the test so we can join multiple testcases together, this should be unique enough because it only needs to be unique for the current block, so the way to break this would be to inline multiple tests, but that is unlikely to happen. When it happens just use StartLine:StartPosition
+    # using the position of It as Id for the the test so we can join multiple testcases together, this should be unique enough because it only needs to be unique for the current block.
     # TODO: Id is used by NUnit2.5 and 3 testresults to group. A better way to solve this?
-    $id = $StartLine
+    $groupId = "${StartLine}:${StartColumn}"
     foreach ($d in $Data) {
-        New-Test -Id $id -Name $Name -Tag $Tag -ScriptBlock $ScriptBlock -StartLine $StartLine -Data $d -Focus:$Focus -Skip:$Skip
+        New-Test -GroupId $groupId -Name $Name -Tag $Tag -ScriptBlock $ScriptBlock -StartLine $StartLine -Data $d -Focus:$Focus -Skip:$Skip
     }
 }
 

diff --git a/src/csharp/Pester/Block.cs b/src/csharp/Pester/Block.cs
@@ -40,7 +40,9 @@ public Block()
         public List<object> ErrorRecord { get; set; }
         public TimeSpan Duration { get => DiscoveryDuration + FrameworkDuration + UserDuration; }
 
-        public string Id { get; set; }
+        [Obsolete("Id is obsolete and should no longer be used. Use GroupId instead.")]
+        public string Id { get => GroupId; }
+        public string GroupId { get; set; }
         public List<string> Tag { get; set; }
         public bool Focus { get; set; }
         public bool Skip { get; set; }

diff --git a/src/csharp/Pester/Test.cs b/src/csharp/Pester/Test.cs
@@ -40,7 +40,10 @@ public Test()
         public TimeSpan Duration { get => UserDuration + FrameworkDuration; }
 
         public string ItemType { get; private set; }
-        public string Id { get; set; }
+
+        [Obsolete("Id is obsolete and should no longer be used. Use GroupId instead.")]
+        public string Id { get => GroupId; }
+        public string GroupId { get; set; }
         public ScriptBlock ScriptBlock { get; set; }
         public List<string> Tag { get; set; }
         public bool Focus { get; set; }

diff --git a/src/functions/Context.ps1 b/src/functions/Context.ps1
@@ -108,7 +108,7 @@
 
         if ($PSBoundParameters.ContainsKey('ForEach')) {
             if ($null -ne $ForEach -and 0 -lt @($ForEach).Count) {
-                New-ParametrizedBlock -Name $Name -ScriptBlock $Fixture -StartLine $MyInvocation.ScriptLineNumber -Tag $Tag -FrameworkData @{ CommandUsed = 'Context'; WrittenToScreen = $false } -Focus:$Focus -Skip:$Skip -Data $ForEach
+                New-ParametrizedBlock -Name $Name -ScriptBlock $Fixture -StartLine $MyInvocation.ScriptLineNumber -StartColumn $MyInvocation.OffsetInLine -Tag $Tag -FrameworkData @{ CommandUsed = 'Context'; WrittenToScreen = $false } -Focus:$Focus -Skip:$Skip -Data $ForEach
             }
             else {
                 # @() or $null is provided do nothing

diff --git a/src/functions/Describe.ps1 b/src/functions/Describe.ps1
@@ -116,7 +116,7 @@
 
         if ($PSBoundParameters.ContainsKey('ForEach')) {
             if ($null -ne $ForEach -and 0 -lt @($ForEach).Count) {
-                New-ParametrizedBlock -Name $Name -ScriptBlock $Fixture -StartLine $MyInvocation.ScriptLineNumber -Tag $Tag -FrameworkData @{ CommandUsed = 'Describe'; WrittenToScreen = $false } -Focus:$Focus -Skip:$Skip -Data $ForEach
+                New-ParametrizedBlock -Name $Name -ScriptBlock $Fixture -StartLine $MyInvocation.ScriptLineNumber -StartColumn $MyInvocation.OffsetInLine -Tag $Tag -FrameworkData @{ CommandUsed = 'Describe'; WrittenToScreen = $false } -Focus:$Focus -Skip:$Skip -Data $ForEach
             }
             else {
                 # @() or $null is provided do nothing

diff --git a/src/functions/It.ps1 b/src/functions/It.ps1
@@ -164,7 +164,7 @@
 
     if ($PSBoundParameters.ContainsKey('ForEach')) {
         if ($null -ne $ForEach -and 0 -lt @($ForEach).Count) {
-            New-ParametrizedTest -Name $Name -ScriptBlock $Test -StartLine $MyInvocation.ScriptLineNumber -Data $ForEach -Tag $Tag -Focus:$Focus -Skip:$Skip
+            New-ParametrizedTest -Name $Name -ScriptBlock $Test -StartLine $MyInvocation.ScriptLineNumber -StartColumn $MyInvocation.OffsetInLine -Data $ForEach -Tag $Tag -Focus:$Focus -Skip:$Skip
         }
         else {
             # @() or $null is provided do nothing

diff --git a/src/functions/TestResults.NUnit25.ps1 b/src/functions/TestResults.NUnit25.ps1
@@ -118,12 +118,12 @@ function Write-NUnitTestSuiteElements {
     }
 
     $suites = @(
-        # Tests only have Id if parameterized. All other tests are put in group with '' value
-        $Node.Tests | & $SafeCommands['Group-Object'] -Property Id
+        # Tests only have GroupId if parameterized. All other tests are put in group with '' value
+        $Node.Tests | & $SafeCommands['Group-Object'] -Property GroupId
     )
 
     foreach ($suite in $suites) {
-        # TODO: when suite has name it belongs into a test group (test cases that are generated from the same test, based on the provided data) so we want extra level of nesting for them, right now this is encoded as having an Id that is non empty, but this is not ideal, it would be nicer to make it more explicit
+        # When group has name it is a parameterized tests (data-generated using -ForEach/TestCases) so we want extra level of nesting for them
         $testGroupId = $suite.Name
         if ($testGroupId) {
             $parameterizedSuiteInfo = Get-ParameterizedTestSuiteInfo -TestSuiteGroup $suite
@@ -159,11 +159,8 @@ function Write-NUnitTestSuiteElements {
 function Get-ParameterizedTestSuiteInfo {
     param([Microsoft.PowerShell.Commands.GroupInfo] $TestSuiteGroup)
     # this is generating info for a group of tests that were generated from the same test when TestCases are used
-    # I am using the Name from the first test as the name of the test group, even though we are grouping at
-    # the Id of the test (which is the line where the ScriptBlock of that test starts). This allows us to have
-    # unique Id (the line number) and also a readable name
-    # the possible edgecase here is putting $(Get-Date) into the test name, which would prevent us from
-    # grouping the tests together if we used just the name, and not the linenumber (which remains static)
+    # Using the Name from the first test as the name of the test group to make it readable,
+    # even though we are grouping using GroupId of the tests.
     $node = [PSCustomObject] @{
         Path              = $TestSuiteGroup.Group[0].Path
         TotalCount        = 0

diff --git a/src/functions/TestResults.NUnit3.ps1 b/src/functions/TestResults.NUnit3.ps1
@@ -132,14 +132,12 @@ function Write-NUnit3TestSuiteElement {
     }
 
     $blockGroups = @(
-        # Blocks only have Id if parameterized (using -ForEach). All other blocks are put in group with '' value
-        $Node.Blocks | & $SafeCommands['Group-Object'] -Property Id
+        # Blocks only have GroupId if parameterized (using -ForEach). All other blocks are put in group with '' value
+        $Node.Blocks | & $SafeCommands['Group-Object'] -Property GroupId
     )
 
     foreach ($group in $blockGroups) {
-        # TODO: Switch Id to GroupId or something more explicit for identifying data-generated blocks (and tests).
-        # Couldn't use Where-Object Data | Group Name instead of Id because duplicate block and test names are allowed.
-        # When group has name it belongs into a block group (data-generated using -ForEach) so we want extra level of nesting for them
+        # When group has name it is a parameterized block (data-generated using -ForEach) so we want extra level of nesting for them
         $blockGroupId = $group.Name
         if ($blockGroupId) {
             if (@($group.Group.ShouldRun) -notcontains $true) {
@@ -169,14 +167,12 @@ function Write-NUnit3TestSuiteElement {
     }
 
     $testGroups = @(
-        # Tests only have Id if parameterized. All other tests are put in group with '' value
-        $Node.Tests | & $SafeCommands['Group-Object'] -Property Id
+        # Tests only have GroupId if parameterized. All other tests are put in group with '' value
+        $Node.Tests | & $SafeCommands['Group-Object'] -Property GroupId
     )
 
     foreach ($group in $testGroups) {
-        # TODO: when suite has name it belongs into a test group (test cases that are generated from the same test,
-        # based on the provided data) so we want extra level of nesting for them, right now this is encoded as having an Id that is non empty,
-        # but this is not ideal, it would be nicer to make it more explicit
+        # When group has name it is a parameterized tests (data-generated using -ForEach/TestCases) so we want extra level of nesting for them
         $testGroupId = $group.Name
         if ($testGroupId) {
             if (@($group.Group.ShouldRun) -notcontains $true) {
@@ -387,11 +383,8 @@ function Get-NUnit3ParameterizedMethodSuiteInfo {
     param([Microsoft.PowerShell.Commands.GroupInfo] $TestSuiteGroup, [string] $ParentPath)
     # this is generating info for a group of tests that were generated from the same test when TestCases are used
 
-    # Using the Name from the first test as the name of the test group, even though we are grouping at
-    # the Id of the test (which is the line where the ScriptBlock of that test starts). This allows us to have
-    # unique Id (the line number) and also a readable name
-    # the possible edgecase here is putting $(Get-Date) into the test name, which would prevent us from
-    # grouping the tests together if we used just the name, and not the linenumber (which remains static)
+    # Using the Name from the first test as the name of the test group to make it readable,
+    # even though we are grouping using GroupId of the tests.
 
     $sampleTest = $TestSuiteGroup.Group[0]
     $node = [PSCustomObject] @{
@@ -436,11 +429,8 @@ function Get-NUnit3ParameterizedFixtureSuiteInfo {
     param([Microsoft.PowerShell.Commands.GroupInfo] $TestSuiteGroup, [string] $ParentPath)
     # this is generating info for a group of blocks that were generated from the same block when ForEach are used
 
-    # Using the Name from the first block as the name of the block group, even though we are grouping at
-    # the Id of the block (which is the line where the ScriptBlock of that block starts). This allows us to have
-    # unique Id (the line number) and also a readable name
-    # the possible edgecase here is putting $(Get-Date) into the block name, which would prevent us from
-    # grouping the blocks together if we used just the name, and not the linenumber (which remains static)
+    # Using the Name from the first block as the name of the block group to make it readable,
+    # even though we are grouping using GroupId of the blocks.
 
     $sampleBlock = $TestSuiteGroup.Group[0]
     $node = [PSCustomObject] @{
@@ -487,8 +477,8 @@ function Write-NUnit3TestCaseElement {
 
     Write-NUnit3TestCaseAttributes -TestResult $TestResult -ParentPath $ParentPath -XmlWriter $XmlWriter
 
-    # tests with testcases/foreach (has .Id) has tags on ParameterizedMethod-node
-    $includeTags = (-not $TestResult.Id) -and $TestResult.Tag
+    # Tests with testcases/foreach (has .GroupId) has tags on ParameterizedMethod-node
+    $includeTags = (-not $TestResult.GroupId) -and $TestResult.Tag
     $hasData = $TestResult.Data -is [System.Collections.IDictionary] -and $TestResult.Data.Keys.Count -gt 0
 
     if ($includeTags -or $hasData) {