Skip to content

Commit

Permalink
Attempting to increase reliability of some Pester tests (microsoft#265)
Browse files Browse the repository at this point in the history
We're seeing some inconsistent failures in some of the Pester tests.

The hypothesis is that GitHub may need a little bit more time after the creation of objects before performing certain operations on them (like renaming repos), or may need more time after deleting them before it will successfully return a 404 on a successive Get request.

I have added a number of `Start-Sleep`'s throughout the test codebase wherever we've encountered inconsistent failures, and that appears to have resolved the problem.  We may need to continue to do more of these if additional ones pop up.

The duration of the sleep itself is controlled by `$script:defaultSleepSecondsForReliability` which is defined in `Tests/Common.ps1`.

Long term, I have opened microsoft#267 which poses the idea of switching over to mocking out `Invoke-WebRequest` for the majority of the tests, and instead focus on validating the data that it's sending matches the expected values per the API documentation, and having just a limited number of tests that do actual end-to-end testing.

Fixes microsoft#264
  • Loading branch information
HowardWolosky authored Jul 18, 2020
1 parent 8e55f5a commit f406cc5
Show file tree
Hide file tree
Showing 8 changed files with 251 additions and 6 deletions.
4 changes: 4 additions & 0 deletions Tests/Common.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,10 @@ function Initialize-CommonTestSetup
Set-GitHubConfiguration -LogRequestBody # Make it easier to debug UT failures
Set-GitHubConfiguration -MultiRequestProgressThreshold 0 # Status corrupts the raw CI logs for Linux and Mac, and makes runs take slightly longer.
Set-GitHubConfiguration -DisableUpdateCheck # The update check is unnecessary during tests.

# The number of seconds to sleep after performing some operations to ensure that successive
# API calls properly reflect previously updated state.
$script:defaultSleepSecondsForReliability = 3
}

Initialize-CommonTestSetup
41 changes: 41 additions & 0 deletions Tests/GitHubAssignees.tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,22 @@ try

Context 'Adding and removing an assignee via parameters' {
$issue = $repo | New-GitHubIssue -Title "Test issue"

# The CI build has been unreliable with this test.
# Adding a short sleep to ensure successive queries reflect updated state.
Start-Sleep -Seconds $script:defaultSleepSecondsForReliability

It 'Should have no assignees when created' {
$issue.assignee.login | Should -BeNullOrEmpty
$issue.assignees | Should -BeNullOrEmpty
}

$updatedIssue = Add-GitHubAssignee -OwnerName $script:ownerName -RepositoryName $repo.name -Issue $issue.number -Assignee $owner.login

# The CI build has been unreliable with this test.
# Adding a short sleep to ensure successive queries reflect updated state.
Start-Sleep -Seconds $script:defaultSleepSecondsForReliability

It 'Should have returned the same issue' {
$updatedIssue.number | Should -Be $issue.number
}
Expand Down Expand Up @@ -144,12 +154,22 @@ try

Context 'Adding an assignee with the repo on the pipeline' {
$issue = $repo | New-GitHubIssue -Title "Test issue"

# The CI build has been unreliable with this test.
# Adding a short sleep to ensure successive queries reflect updated state.
Start-Sleep -Seconds $script:defaultSleepSecondsForReliability

It 'Should have no assignees when created' {
$issue.assignee.login | Should -BeNullOrEmpty
$issue.assignees | Should -BeNullOrEmpty
}

$updatedIssue = $repo | Add-GitHubAssignee -Issue $issue.number -Assignee $owner.login

# The CI build has been unreliable with this test.
# Adding a short sleep to ensure successive queries reflect updated state.
Start-Sleep -Seconds $script:defaultSleepSecondsForReliability

It 'Should have returned the same issue' {
$updatedIssue.number | Should -Be $issue.number
}
Expand Down Expand Up @@ -181,12 +201,22 @@ try

Context 'Adding an assignee with the issue on the pipeline' {
$issue = $repo | New-GitHubIssue -Title "Test issue"

# The CI build has been unreliable with this test.
# Adding a short sleep to ensure successive queries reflect updated state.
Start-Sleep -Seconds $script:defaultSleepSecondsForReliability

It 'Should have no assignees when created' {
$issue.assignee.login | Should -BeNullOrEmpty
$issue.assignees | Should -BeNullOrEmpty
}

$updatedIssue = $issue | Add-GitHubAssignee -Assignee $owner.login

# The CI build has been unreliable with this test.
# Adding a short sleep to ensure successive queries reflect updated state.
Start-Sleep -Seconds $script:defaultSleepSecondsForReliability

It 'Should have returned the same issue' {
$updatedIssue.number | Should -Be $issue.number
}
Expand Down Expand Up @@ -218,12 +248,22 @@ try

Context 'Adding an assignee with the assignee user object on the pipeline' {
$issue = $repo | New-GitHubIssue -Title "Test issue"

# The CI build has been unreliable with this test.
# Adding a short sleep to ensure successive queries reflect updated state.
Start-Sleep -Seconds $script:defaultSleepSecondsForReliability

It 'Should have no assignees when created' {
$issue.assignee.login | Should -BeNullOrEmpty
$issue.assignees | Should -BeNullOrEmpty
}

$updatedIssue = $owner | Add-GitHubAssignee -OwnerName $script:ownerName -RepositoryName $repo.name -Issue $issue.number

# The CI build has been unreliable with this test.
# Adding a short sleep to ensure successive queries reflect updated state.
Start-Sleep -Seconds $script:defaultSleepSecondsForReliability

It 'Should have returned the same issue' {
$updatedIssue.number | Should -Be $issue.number
}
Expand All @@ -239,6 +279,7 @@ try
}

$updatedIssue = $owner | Remove-GitHubAssignee -OwnerName $script:ownerName -RepositoryName $repo.name -Issue $issue.number -Force

It 'Should have returned the same issue' {
$updatedIssue.number | Should -Be $issue.number
}
Expand Down
8 changes: 8 additions & 0 deletions Tests/GitHubContents.tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ try
BeforeAll {
# AutoInit will create a readme with the GUID of the repo name
$repo = New-GitHubRepository -RepositoryName ($repoGuid) -AutoInit

# The CI build has been unreliable with this test.
# Adding a short sleep to ensure successive queries reflect updated state.
Start-Sleep -Seconds $script:defaultSleepSecondsForReliability
}

AfterAll {
Expand Down Expand Up @@ -260,6 +264,10 @@ try
$repoName = [Guid]::NewGuid().Guid

$repo = New-GitHubRepository -RepositoryName $repoName -AutoInit

# The CI build has been unreliable with this test.
# Adding a short sleep to ensure successive queries reflect updated state.
Start-Sleep -Seconds $script:defaultSleepSecondsForReliability
}

Context 'When setting new file content' {
Expand Down
9 changes: 8 additions & 1 deletion Tests/GitHubIssues.tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,9 @@ try
for ($i = 0; $i -lt 4; $i++)
{
$newIssues += New-GitHubIssue -OwnerName $script:ownerName -RepositoryName $repo.name -Title ([Guid]::NewGuid().Guid)
Start-Sleep -Seconds 1 # Needed to ensure that there is a unique creation timestamp between issues

# Needed to ensure that there is a unique creation timestamp between issues
Start-Sleep -Seconds $script:defaultSleepSecondsForReliability
}

$newIssues[0] = Set-GitHubIssue -OwnerName $script:ownerName -RepositoryName $repo.name -Issue $newIssues[0].number -State Closed
Expand Down Expand Up @@ -547,6 +549,11 @@ try
}

Lock-GitHubIssue -OwnerName $script:OwnerName -RepositoryName $repo.name -Issue $issue.number

# The CI build has been unreliable with this test.
# Adding a short sleep to ensure successive queries reflect updated state.
Start-Sleep -Seconds $script:defaultSleepSecondsForReliability

$timeline = @(Get-GitHubIssueTimeline -OwnerName $script:OwnerName -RepositoryName $repo.name -Issue $issue.number)
It 'Should have an event now' {
$timeline.Count | Should -Be 1
Expand Down
65 changes: 65 additions & 0 deletions Tests/GitHubLabels.tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ try
$repositoryName = [Guid]::NewGuid().Guid
$repo = New-GitHubRepository -RepositoryName $repositoryName

# The CI build has been unreliable with this test.
# Adding a short sleep to ensure successive queries reflect updated state.
Start-Sleep -Seconds $script:defaultSleepSecondsForReliability

Initialize-GitHubLabel -OwnerName $script:ownerName -RepositoryName $repositoryName -Label $defaultLabels
}

Expand Down Expand Up @@ -205,6 +209,10 @@ try
BeforeAll {
$repositoryName = [Guid]::NewGuid().Guid
$repo = New-GitHubRepository -RepositoryName $repositoryName

# The CI build has been unreliable with this test.
# Adding a short sleep to ensure successive queries reflect updated state.
Start-Sleep -Seconds $script:defaultSleepSecondsForReliability
}

AfterAll {
Expand Down Expand Up @@ -319,6 +327,10 @@ try
BeforeAll {
$repositoryName = [Guid]::NewGuid().Guid
$repo = New-GitHubRepository -RepositoryName $repositoryName

# The CI build has been unreliable with this test.
# Adding a short sleep to ensure successive queries reflect updated state.
Start-Sleep -Seconds $script:defaultSleepSecondsForReliability
}

AfterAll {
Expand All @@ -327,6 +339,11 @@ try

Context 'Removing a label with parameters' {
$label = $repo | New-GitHubLabel -Label 'test' -Color 'CCCCCC'

# The CI build has been unreliable with this test.
# Adding a short sleep to ensure successive queries reflect updated state.
Start-Sleep -Seconds $script:defaultSleepSecondsForReliability

Remove-GitHubLabel -OwnerName $script:ownerName -RepositoryName $repositoryName -Label $label.name -Force

It 'Should be gone after being removed by parameter' {
Expand All @@ -336,6 +353,11 @@ try

Context 'Removing a label with the repo on the pipeline' {
$label = $repo | New-GitHubLabel -Label 'test' -Color 'CCCCCC'

# The CI build has been unreliable with this test.
# Adding a short sleep to ensure successive queries reflect updated state.
Start-Sleep -Seconds $script:defaultSleepSecondsForReliability

$repo | Remove-GitHubLabel -Label $label.name -Confirm:$false

It 'Should be gone after being removed by parameter' {
Expand All @@ -345,6 +367,11 @@ try

Context 'Removing a label with the name on the pipeline' {
$label = $repo | New-GitHubLabel -Label 'test' -Color 'CCCCCC'

# The CI build has been unreliable with this test.
# Adding a short sleep to ensure successive queries reflect updated state.
Start-Sleep -Seconds $script:defaultSleepSecondsForReliability

$label.name | Remove-GitHubLabel -OwnerName $script:ownerName -RepositoryName $repositoryName -Force

It 'Should be gone after being removed by parameter' {
Expand All @@ -354,6 +381,11 @@ try

Context 'Removing a label with the label object on the pipeline' {
$label = $repo | New-GitHubLabel -Label 'test' -Color 'CCCCCC'

# The CI build has been unreliable with this test.
# Adding a short sleep to ensure successive queries reflect updated state.
Start-Sleep -Seconds $script:defaultSleepSecondsForReliability

$label | Remove-GitHubLabel -Force

It 'Should be gone after being removed by parameter' {
Expand All @@ -366,6 +398,10 @@ try
BeforeAll {
$repositoryName = [Guid]::NewGuid().Guid
$repo = New-GitHubRepository -RepositoryName $repositoryName

# The CI build has been unreliable with this test.
# Adding a short sleep to ensure successive queries reflect updated state.
Start-Sleep -Seconds $script:defaultSleepSecondsForReliability
}

AfterAll {
Expand Down Expand Up @@ -543,6 +579,10 @@ try
BeforeAll {
$repositoryName = [Guid]::NewGuid().Guid
$repo = New-GitHubRepository -RepositoryName $repositoryName

# The CI build has been unreliable with this test.
# Adding a short sleep to ensure successive queries reflect updated state.
Start-Sleep -Seconds $script:defaultSleepSecondsForReliability
}

AfterAll {
Expand Down Expand Up @@ -617,6 +657,11 @@ try
BeforeAll {
$repositoryName = [Guid]::NewGuid().Guid
$repo = New-GitHubRepository -RepositoryName $repositoryName

# The CI build has been unreliable with this test.
# Adding a short sleep to ensure successive queries reflect updated state.
Start-Sleep -Seconds $script:defaultSleepSecondsForReliability

$repo | Initialize-GitHubLabel -Label $defaultLabels
}

Expand Down Expand Up @@ -845,6 +890,11 @@ try
BeforeAll {
$repositoryName = [Guid]::NewGuid().Guid
$repo = New-GitHubRepository -RepositoryName $repositoryName

# The CI build has been unreliable with this test.
# Adding a short sleep to ensure successive queries reflect updated state.
Start-Sleep -Seconds $script:defaultSleepSecondsForReliability

$repo | Initialize-GitHubLabel -Label $defaultLabels
}

Expand Down Expand Up @@ -911,6 +961,11 @@ try
BeforeAll {
$repositoryName = [Guid]::NewGuid().Guid
$repo = New-GitHubRepository -RepositoryName $repositoryName

# The CI build has been unreliable with this test.
# Adding a short sleep to ensure successive queries reflect updated state.
Start-Sleep -Seconds $script:defaultSleepSecondsForReliability

$repo | Initialize-GitHubLabel -Label $defaultLabels
}

Expand Down Expand Up @@ -1078,6 +1133,11 @@ try
BeforeAll {
$repositoryName = [Guid]::NewGuid().Guid
$repo = New-GitHubRepository -RepositoryName $repositoryName

# The CI build has been unreliable with this test.
# Adding a short sleep to ensure successive queries reflect updated state.
Start-Sleep -Seconds $script:defaultSleepSecondsForReliability

$repo | Initialize-GitHubLabel -Label $defaultLabels
}

Expand Down Expand Up @@ -1227,6 +1287,11 @@ try
BeforeAll {
$repositoryName = [Guid]::NewGuid().Guid
$repo = New-GitHubRepository -RepositoryName $repositoryName

# The CI build has been unreliable with this test.
# Adding a short sleep to ensure successive queries reflect updated state.
Start-Sleep -Seconds $script:defaultSleepSecondsForReliability

$repo | Initialize-GitHubLabel -Label $defaultLabels

$milestone = $repo | New-GitHubMilestone -Title 'test milestone'
Expand Down
8 changes: 8 additions & 0 deletions Tests/GitHubProjects.tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -611,6 +611,10 @@ try
$project = New-GitHubProject -OwnerName $script:ownerName -RepositoryName $repo.name -ProjectName $defaultRepoProject -Description $defaultRepoProjectDesc
$null = Remove-GitHubProject -Project $project.id -Confirm:$false
It 'Project should be removed' {
# The CI build has been unreliable with this test.
# Adding a short sleep to ensure successive queries reflect updated state.
Start-Sleep -Seconds $script:defaultSleepSecondsForReliability

{Get-GitHubProject -Project $project.id} | Should -Throw
}
}
Expand All @@ -619,6 +623,10 @@ try
$project = $repo | New-GitHubProject -ProjectName $defaultRepoProject -Description $defaultRepoProjectDesc
$project | Remove-GitHubProject -Force
It 'Project should be removed' {
# The CI build has been unreliable with this test.
# Adding a short sleep to ensure successive queries reflect updated state.
Start-Sleep -Seconds $script:defaultSleepSecondsForReliability

{$project | Get-GitHubProject} | Should -Throw
}
}
Expand Down
Loading

0 comments on commit f406cc5

Please sign in to comment.