8000 Fix the CI build break by daxian-dbw · Pull Request #2806 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]
Skip to content

Fix the CI build break #2806

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Nov 30, 2016
Merged

Fix the CI build break #2806

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ea464fc
Revert the timer change introduced in #2640 because it causes race co…
daxian-dbw Nov 29, 2016
fc92d12
Temporarily disable Update-Help and Save-Help tests in CI builds
daxian-dbw Nov 30, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 2 additions & 42 deletions src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHostUserInterfaceProgress.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
using System;
using System.Management.Automation;
using Dbg = System.Management.Automation.Diagnostics;
using System.Threading;


namespace Microsoft.PowerShell
Expand All @@ -29,12 +28,6 @@ class ConsoleHostUserInterface : System.Management.Automation.Host.PSHostUserInt
// destroy the data structures representing outstanding progress records
// take down and destroy the progress display

if (_progPaneUpdateTimer != null)
{
// Stop update 'ProgressPane' and destroy timer
_progPaneUpdateTimer.Dispose();
_progPaneUpdateTimer = null;
}
if (_progPane != null)
{
Dbg.Assert(_pendingProgress != null, "How can you have a progress pane and no backing data structure?");
Expand Down Expand Up @@ -71,41 +64,11 @@ class ConsoleHostUserInterface : System.Management.Automation.Host.PSHostUserInt

if (_progPane == null)
{
// This is the first time we've received a progress record.
// Create a progress pane,
// then show it,
// then create and start timer to update it.
// This is the first time we've received a progress record. Create a progress pane, then show it.

_progPane = new ProgressPane(this);

if (_progPaneUpdateTimer == null && _progPane != null)
{
_progPane.Show(_pendingProgress);
_progPaneUpdateTimer = new Timer( new TimerCallback(ProgressPaneUpdateTimerElapsed), null, UpdateTimerThreshold, Timeout.Infinite);
}
}
}



/// <summary>
///
/// TimerCallback for _progPaneUpdateTimer to update 'ProgressPane' and restart the timer.
///
/// </summary>

private
void
ProgressPaneUpdateTimerElapsed(object sender)
{
if (_progPane != null)
{
_progPane.Show(_pendingProgress);
}
if (_progPaneUpdateTimer != null)
{
_progPaneUpdateTimer.Change(UpdateTimerThreshold, Timeout.Infinite);
}
_progPane.Show(_pendingProgress);
}


Expand Down Expand Up @@ -205,9 +168,6 @@ class ConsoleHostUserInterface : System.Management.Automation.Host.PSHostUserInt

private ProgressPane _progPane = null;
private PendingProgress _pendingProgress = null;
// The timer update 'ProgressPane' every 'UpdateTimerThreshold' milliseconds
private Timer _progPaneUpdateTimer;
private const int UpdateTimerThreshold = 100;
}
} // namespace

Expand Down
2 changes: 1 addition & 1 deletion test/powershell/Modules/Microsoft.PowerShell.Utility/Start-Sleep.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ Describe "Start-Sleep DRT Unit Tests" -Tags "CI" {
Start-Sleep -Milliseconds 1000
$dtEnd = [DateTime]::Now
$milliseconds = (New-TimeSpan -Start $dtStart -End $dtEnd).TotalMilliseconds
$milliseconds | Should BeGreaterThan 1000
$milliseconds -ge 1000 | Should Be $true
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$milliseconds -ge 1000 | Should Be $true [](start = 8, length = 40)

not sure why this changed, the pester message is harder to understand this way:
before:

PS> describe a { it b { 1 | should begreaterthan 2 } }
Describing a
 [-] b 819ms
   Expected {1} to be greater than {2}
   at line: 1 in
   1: describe a { it b { 1 | should begreaterthan 2 } }

now

PS> describe a { it b { 1 -gt 2 | should be $true } }
Describing a
 [-] b 57ms
   Expected: {True}
   But was:  {False}
   at line: 1 in
   1: describe a { it b { 1 -gt 2 | should be $true } }

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was changed because I saw this test failed with error expected {1000} to be greater than {1000}. Maybe there is a Should BeGreaterOrEqualThan?

Copy link
Collaborator
@JamesWTruher JamesWTruher Dec 1, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or change to should begreaterthan 999
we know it will be greater than that
Pester doesn't have a BeGreaterOrEqual assertion, but it's a good idea

}

}
Expand Down
14 changes: 8 additions & 6 deletions test/powershell/engine/Help/UpdatableHelpSystem.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,15 +158,16 @@ function RunUpdateHelpTests
{
param (
[string]$tag = "CI",
[switch]$useSourcePath
[switch]$useSourcePath,
[switch]$Pending
)

foreach ($moduleName in $modulesInBox)
{
if ($powershellCoreModules -contains $moduleName)
{

It "Validate Update-Help for module '$moduleName'" {
It "Validate Update-Help for module '$moduleName'" -Pending:$Pending {

# If the help file is already installed, delete it.
Get-ChildItem $testCases[$moduleName].HelpInstallationPath -Include @("about_*.txt","*help.xml") -Recurse -ea SilentlyContinue |
Expand Down Expand Up @@ -209,7 +210,8 @@ function RunUpdateHelpTests
function RunSaveHelpTests
{
param (
[string]$tag = "CI"
[string]$tag = "CI",
[switch]$Pending
)

foreach ($moduleName in $modulesInBox)
Expand All @@ -221,7 +223,7 @@ function RunSaveHelpTests
$saveHelpFolder = Join-Path $TestDrive (Get-Random).ToString()
New-Item $saveHelpFolder -Force -ItemType Directory

It "Validate Save-Help for the '$moduleName' module" {
It "Validate Save-Help for the '$moduleName' module" -Pending:$Pending {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're introducing a new pattern here passing in the $Pending value. My preference is to just have -Pending on the relevant test cases rather than trying to pass it by the switch. One reason is I can search all the tests to see which ones are pending, but with this, I have to look at the code to determine if it's pending or not.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this test, this single It block runs in a loop to test a bunch different modules, so there is no specific test case that you can directly put a -Pending at.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SteveL-MSFT Do you have any further concerns? If not, I'd like to merge this PR to get CI running again

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@daxian-dbw I see, in that case, I guess it's ok. thanks.


if ((Get-UICulture).Name -ne "en-Us")
{
Expand Down Expand Up @@ -266,7 +268,7 @@ function ValidateSaveHelp

Describe "Validate Update-Help from the Web for one PowerShell Core module." -Tags @('CI', 'RequireAdminOnWindows') {

RunUpdateHelpTests -tag "CI"
RunUpdateHelpTests -tag "CI" -Pending
}

Describe "Validate Update-Help from the Web for all PowerShell Core modules." -Tags @('Feature', 'RequireAdminOnWindows') {
Expand All @@ -285,7 +287,7 @@ Describe "Validate Update-Help -SourcePath for all PowerShell Core modules." -Ta
}

Describe "Validate 'Save-Help -DestinationPath for one PowerShell Core modules." -Tags @('CI', 'RequireAdminOnWindows') {
RunSaveHelpTests -tag "CI"
RunSaveHelpTests -tag "CI" -Pending
}

Describe "Validate 'Save-Help -DestinationPath for all PowerShell Core modules." -Tags @('Feature', 'RequireAdminOnWindows') {
Expand Down
0