Fix multi-result behavior across all versions of PowerShell (fixes CI…

… UT's on all platforms) (#199) Tests were failing on Mac and Linux, but not Windows ([recent test run](https://dev.azure.com/ms/PowerShellForGitHub/_build/results?buildId=83887&view=logs&j=0da5d1d9-276d-5173-c4c4-9d4d4ed14fdb)). That's because Windows CI was running against PoSh 5.x while Linux and Mac were running on PoSh 7.x. There's a slight difference in behavior for how those two treat arrays. The real root cause for this was the behavior of `Invoke-GHRestMethodMultipleResult`. When creating `$finalResult`, it was always blindly adding the result to the existing array: https://github.com/microsoft/PowerShellForGitHub/blob/587e2042621091c79cc06be2aa9cc6ea836561f4/GitHubCore.ps1#L648 `...` https://github.com/microsoft/PowerShellForGitHub/blob/587e2042621091c79cc06be2aa9cc6ea836561f4/GitHubCore.ps1#L670 Oddly enough, this created a difference in behavior between PowerShell versions when making the result an array on the caller side. Now I ensure that I don't add anything to `$finalResult` unless there's actually a value. With that change, we can now be sure that when we grab the result as an array, it'll be appropriately empty or populated (and not populated with a single `$null` entry, thus making `Count` 1, erroneously). I removed the attempt to force the results to be an array, because this is pointless. PowerShell will always unwrap an array of 0 or 1 in a return result. If you want to ensure that a result is always an array, you have to [wrap the result in an object](https://stackoverflow.com/a/60330501) or you have to do wrap the result in an array on the caller side. https://github.com/microsoft/PowerShellForGitHub/blob/587e2042621091c79cc06be2aa9cc6ea836561f4/GitHubCore.ps1#L684-L685 I also normalized some naming in all of the tests, so that when we're getting back a singular result (by querying for a specific item) that we use a singular variable name, and a plural variable name otherwise. With this change, we should now be passing CI on all OS platforms and across PowerShell 4+. Resolves #198
microsoft · May 30, 2020 · bcd0a56 · bcd0a56
1 parent 587e204
commit bcd0a56
Show file tree

Hide file tree

Showing 12 changed files with 175 additions and 152 deletions.
diff --git a/GitHubCore.ps1 b/GitHubCore.ps1
@@ -667,7 +667,11 @@ function Invoke-GHRestMethodMultipleResult
             }
 
             $result = Invoke-GHRestMethod @params
-            $finalResult += $result.result
+            if ($null -ne $result.result)
+            {
+                $finalResult += $result.result
+            }
+
             $nextLink = $result.nextLink
             $currentDescription = "$Description (getting additional results)"
         }
@@ -681,8 +685,7 @@ function Invoke-GHRestMethodMultipleResult
             Set-TelemetryEvent -EventName $TelemetryEventName -Properties $TelemetryProperties -Metrics $telemetryMetrics
         }
 
-        # Ensure we're always returning our results as an array, even if there is a single result.
-        return @($finalResult)
+        return $finalResult
     }
     catch
     {

diff --git a/Tests/GitHubAnalytics.tests.ps1 b/Tests/GitHubAnalytics.tests.ps1
@@ -16,10 +16,10 @@ try
         $repo = New-GitHubRepository -RepositoryName ([Guid]::NewGuid().Guid) -AutoInit
 
         Context 'When initially created, there are no issues' {
-            $issues = Get-GitHubIssue -Uri $repo.svn_url
+            $issues = @(Get-GitHubIssue -Uri $repo.svn_url)
 
             It 'Should return expected number of issues' {
-                @($issues).Count | Should be 0
+                $issues.Count | Should be 0
             }
         }
 
@@ -34,29 +34,29 @@ try
             $newIssues[0] = Update-GitHubIssue -OwnerName $script:ownerName -RepositoryName $repo.name -Issue $newIssues[0].number -State Closed
             $newIssues[-1] = Update-GitHubIssue -OwnerName $script:ownerName -RepositoryName $repo.name -Issue $newIssues[-1].number -State Closed
 
-            $issues = Get-GitHubIssue -Uri $repo.svn_url
+            $issues = @(Get-GitHubIssue -Uri $repo.svn_url)
             It 'Should return only open issues' {
-                @($issues).Count | Should be 2
+                $issues.Count | Should be 2
             }
 
-            $issues = Get-GitHubIssue -Uri $repo.svn_url -State All
+            $issues = @(Get-GitHubIssue -Uri $repo.svn_url -State All)
             It 'Should return all issues' {
-                @($issues).Count | Should be 4
+                $issues.Count | Should be 4
             }
 
             $createdOnOrAfterDate = Get-Date -Date $newIssues[0].created_at
             $createdOnOrBeforeDate = Get-Date -Date $newIssues[2].created_at
-            $issues = (Get-GitHubIssue -Uri $repo.svn_url) | Where-Object { ($_.created_at -ge $createdOnOrAfterDate) -and ($_.created_at -le $createdOnOrBeforeDate) }
+            $issues = @((Get-GitHubIssue -Uri $repo.svn_url) | Where-Object { ($_.created_at -ge $createdOnOrAfterDate) -and ($_.created_at -le $createdOnOrBeforeDate) })
 
             It 'Smart object date conversion works for comparing dates' {
-                @($issues).Count | Should be 2
+                $issues.Count | Should be 2
             }
 
             $createdDate = Get-Date -Date $newIssues[1].created_at
-            $issues = Get-GitHubIssue -Uri $repo.svn_url -State All | Where-Object { ($_.created_at -ge $createdDate) -and ($_.state -eq 'closed') }
+            $issues = @(Get-GitHubIssue -Uri $repo.svn_url -State All | Where-Object { ($_.created_at -ge $createdDate) -and ($_.state -eq 'closed') })
 
             It 'Able to filter based on date and state' {
-                @($issues).Count | Should be 1
+                $issues.Count | Should be 1
             }
         }
 
@@ -88,18 +88,18 @@ try
             $issueCounts = $issueCounts | Sort-Object -Property Count -Descending
 
             It 'Should return expected number of issues for each repository' {
-                @($issueCounts[0].Count) | Should be 3
-                @($issueCounts[1].Count) | Should be 0
+                $issueCounts[0].Count | Should be 3
+                $issueCounts[1].Count | Should be 0
             }
 
             It 'Should return expected repository names' {
-                @($issueCounts[0].Uri) | Should be ($repo1.svn_url)
-                @($issueCounts[1].Uri) | Should be ($repo2.svn_url)
+                $issueCounts[0].Uri | Should be $repo1.svn_url
+                $issueCounts[1].Uri | Should be $repo2.svn_url
             }
         }
 
-        $null = Remove-GitHubRepository -Uri ($repo1.svn_url)
-        $null = Remove-GitHubRepository -Uri ($repo2.svn_url)
+        $null = Remove-GitHubRepository -Uri $repo1.svn_url
+        $null = Remove-GitHubRepository -Uri $repo2.svn_url
     }
 
 
@@ -186,10 +186,10 @@ try
             $null = New-GitHubRepository -RepositoryName $repositoryName -AutoInit
             $repositoryUrl = "https://github.com/$script:ownerName/$repositoryName"
 
-            $collaborators = Get-GitHubRepositoryCollaborator -Uri $repositoryUrl
+            $collaborators = @(Get-GitHubRepositoryCollaborator -Uri $repositoryUrl)
 
             It 'Should return expected number of collaborators' {
-                @($collaborators).Count | Should be 1
+                $collaborators.Count | Should be 1
             }
 
             $null = Remove-GitHubRepository -OwnerName $script:ownerName -RepositoryName $repositoryName
@@ -201,10 +201,10 @@ try
         $null = New-GitHubRepository -RepositoryName $repositoryName -AutoInit
         $repositoryUrl = "https://github.com/$script:ownerName/$repositoryName"
 
-        $contributors = Get-GitHubRepositoryContributor -Uri $repositoryUrl -IncludeStatistics
+        $contributors = @(Get-GitHubRepositoryContributor -Uri $repositoryUrl -IncludeStatistics)
 
         It 'Should return expected number of contributors' {
-            @($contributors).Count | Should be 1
+            $contributors.Count | Should be 1
         }
 
         $null = Remove-GitHubRepository -OwnerName $script:ownerName -RepositoryName $repositoryName
@@ -242,15 +242,13 @@ try
     }
 
     Describe 'Getting repositories from organization' {
-        <# Temporary hack due to issues with this test in ADO #> . (Join-Path -Path $moduleRootPath -ChildPath 'Tests\Config\Settings.ps1')
-
-        $original = Get-GitHubRepository -OrganizationName $script:organizationName
+        $original = @(Get-GitHubRepository -OrganizationName $script:organizationName)
 
         $repo = New-GitHubRepository -RepositoryName ([guid]::NewGuid().Guid) -OrganizationName $script:organizationName
-        $current = Get-GitHubRepository -OrganizationName $script:organizationName
+        $current = @(Get-GitHubRepository -OrganizationName $script:organizationName)
 
         It 'Should return expected number of organization repositories' {
-            (@($current).Count - @($original).Count) | Should be 1
+            ($current.Count - $original.Count) | Should be 1
         }
 
         $null = Remove-GitHubRepository -Uri $repo.svn_url
@@ -260,15 +258,15 @@ try
         $repositoryName = [guid]::NewGuid().Guid
         $null = New-GitHubRepository -RepositoryName $repositoryName -AutoInit
 
-        $contributors = Get-GitHubRepositoryContributor -OwnerName $script:ownerName -RepositoryName $repositoryName -IncludeStatistics
+        $contributors = @(Get-GitHubRepositoryContributor -OwnerName $script:ownerName -RepositoryName $repositoryName -IncludeStatistics)
 
         $uniqueContributors = $contributors |
             Select-Object -ExpandProperty author |
             Select-Object -ExpandProperty login -Unique
             Sort-Object
 
         It 'Should return expected number of unique contributors' {
-            @($uniqueContributors).Count | Should be 1
+            $uniqueContributors.Count | Should be 1
         }
 
         $null = Remove-GitHubRepository -OwnerName $script:ownerName -RepositoryName $repositoryName
@@ -298,14 +296,14 @@ try
         $repositoryName = [guid]::NewGuid().Guid
         $null = New-GitHubRepository -RepositoryName $repositoryName -AutoInit
 
-        $branches = Get-GitHubRepositoryBranch -OwnerName $script:ownerName -RepositoryName $repositoryName
+        $branches = @(Get-GitHubRepositoryBranch -OwnerName $script:ownerName -RepositoryName $repositoryName)
 
         It 'Should return expected number of repository branches' {
-            @($branches).Count | Should be 1
+            $branches.Count | Should be 1
         }
 
         It 'Should return the name of the branches' {
-            @($branches[0].name) | Should be "master"
+            $branches[0].name | Should be 'master'
         }
 
         $null = Remove-GitHubRepository -OwnerName $script:ownerName -RepositoryName $repositoryName

diff --git a/Tests/GitHubAssignees.tests.ps1 b/Tests/GitHubAssignees.tests.ps1
@@ -44,7 +44,7 @@ try
         Context 'For adding an assignee to an issue'{
             $assigneeList = @(Get-GitHubAssignee -Uri $repo.svn_url)
             $assigneeUserName = $assigneeList[0].login
-            $assignees = @($assigneeUserName)
+            $assignees = $assigneeUserName
             New-GithubAssignee -Uri $repo.svn_url -Issue $issue.number -Assignee $assignees
             $issue = Get-GitHubIssue -Uri $repo.svn_url -Issue $issue.number
 

diff --git a/Tests/GitHubLabels.tests.ps1 b/Tests/GitHubLabels.tests.ps1
@@ -79,10 +79,10 @@ try
             Set-GitHubLabel -OwnerName $ownerName -RepositoryName $repositoryName -Label $defaultLabels
 
             Context 'When querying for all labels' {
-                $labels = Get-GitHubLabel -OwnerName $ownerName -RepositoryName $repositoryName
+                $labels = @(Get-GitHubLabel -OwnerName $ownerName -RepositoryName $repositoryName)
 
                 It 'Should return expected number of labels' {
-                    $($labels).Count | Should be $:defaultLabels.Count
+                    $labels.Count | Should be $:defaultLabels.Count
                 }
             }
 
@@ -123,17 +123,17 @@ try
 
             $labelName = [Guid]::NewGuid().Guid
             New-GitHubLabel -OwnerName $ownerName -RepositoryName $repositoryName -Name $labelName -Color BBBBBB
-            $labels = Get-GitHubLabel -OwnerName $ownerName -RepositoryName $repositoryName
+            $labels = @(Get-GitHubLabel -OwnerName $ownerName -RepositoryName $repositoryName)
 
             It 'Should return increased number of labels' {
-                $($labels).Count | Should be ($defaultLabels.Count + 1)
+                $labels.Count | Should be ($defaultLabels.Count + 1)
             }
 
             Remove-GitHubLabel -OwnerName $ownerName -RepositoryName $repositoryName -Name $labelName
-            $labels = Get-GitHubLabel -OwnerName $ownerName -RepositoryName $repositoryName
+            $labels = @(Get-GitHubLabel -OwnerName $ownerName -RepositoryName $repositoryName)
 
             It 'Should return expected number of labels' {
-                $($labels).Count | Should be $defaultLabels.Count
+                $labels.Count | Should be $defaultLabels.Count
             }
 
             $null = Remove-GitHubRepository -OwnerName $ownerName -RepositoryName $repositoryName
@@ -187,7 +187,7 @@ try
 
             # Add new label
             New-GitHubLabel -OwnerName $ownerName -RepositoryName $repositoryName -Name $labelName -Color BBBBBB
-            $labels = Get-GitHubLabel -OwnerName $ownerName -RepositoryName $repositoryName
+            $labels = @(Get-GitHubLabel -OwnerName $ownerName -RepositoryName $repositoryName)
 
             # Change color of existing label
             Update-GitHubLabel -OwnerName $ownerName -RepositoryName $repositoryName -Name "bug" -NewName "bug" -Color BBBBBB
@@ -200,10 +200,10 @@ try
             }
 
             Set-GitHubLabel -OwnerName $ownerName -RepositoryName $repositoryName -Label $defaultLabels
-            $labels = Get-GitHubLabel -OwnerName $ownerName -RepositoryName $repositoryName
+            $labels = @(Get-GitHubLabel -OwnerName $ownerName -RepositoryName $repositoryName)
 
             It 'Should return expected number of labels' {
-                $($labels).Count | Should be $defaultLabels.Count
+                $labels.Count | Should be $defaultLabels.Count
                 $bugLabel = $labels | Where-Object {$_.name -eq "bug"}
                 $bugLabel.color | Should be "fc2929"
             }
@@ -269,7 +269,7 @@ try
                 Remove-GitHubIssueLabel -OwnerName $ownerName -RepositoryName $repositoryName -Name "discussion" -Issue $issue.number
                 Remove-GitHubIssueLabel -OwnerName $ownerName -RepositoryName $repositoryName -Name "question" -Issue $issue.number
                 Remove-GitHubIssueLabel -OwnerName $ownerName -RepositoryName $repositoryName -Name "bug" -Issue $issue.number
-                $labelIssues = Get-GitHubLabel -OwnerName $ownerName -RepositoryName $repositoryName -Issue $issue.number
+                $labelIssues = @(Get-GitHubLabel -OwnerName $ownerName -RepositoryName $repositoryName -Issue $issue.number)
 
                 It 'Should have removed three labels from the issue' {
                     $labelIssues.Count | Should be ($defaultLabels.Count - 3)
@@ -278,7 +278,7 @@ try
 
             Context 'For removing all issues'{
                 Remove-GitHubIssueLabel -OwnerName $ownerName -RepositoryName $repositoryName -Issue $issue.number
-                $labelIssues = Get-GitHubLabel -OwnerName $ownerName -RepositoryName $repositoryName -Issue $issue.number
+                $labelIssues = @(Get-GitHubLabel -OwnerName $ownerName -RepositoryName $repositoryName -Issue $issue.number)
 
                 It 'Should have removed all labels from the issue' {
                     $labelIssues.Count | Should be 0
@@ -312,7 +312,7 @@ try
                 $labelIssues.Count | Should be $defaultLabels.Count
             }
 
-            $updatedIssueLabels = @($labelsToAdd[0])
+            $updatedIssueLabels = $labelsToAdd[0]
             $updatedIssue = Update-GitHubIssue -OwnerName $ownerName -RepositoryName $repositoryName -Issue $issue.number -Label $updatedIssueLabels
 
             It 'Should have 1 label after updating the issue' {

diff --git a/Tests/GitHubMilestones.tests.ps1 b/Tests/GitHubMilestones.tests.ps1
@@ -71,7 +71,7 @@ try
         }
 
         Context 'For getting milestones from a repo' {
-            $existingMilestones = @(Get-GitHubMilestone -Uri $repo.svn_url -State Closed)
+            $existingMilestones =@(Get-GitHubMilestone -Uri $repo.svn_url -State Closed)
             $issue = Get-GitHubIssue -Uri $repo.svn_url -Issue $issue.number
 
             It 'Should have the expected number of milestones' {
@@ -110,11 +110,11 @@ try
                 $existingMilestones.Count | Should be 4
             }
 
-            foreach($milestone in $existingMilestones) {
+            foreach ($milestone in $existingMilestones) {
                 Remove-GitHubMilestone -Uri $repo.svn_url -Milestone $milestone.number
             }
 
-            $existingMilestones = @(Get-GitHubMilestone -Uri $repo.svn_url)
+            $existingMilestones = @(Get-GitHubMilestone -Uri $repo.svn_url -State All)
             $issue = Get-GitHubIssue -Uri $repo.svn_url -Issue $issue.number
 
             It 'Should have no milestones' {