From b47d587f0e8b1530d10aa2a4bd1a7dc349fc8611 Mon Sep 17 00:00:00 2001 From: Staffan Gustafsson Date: Wed, 30 Jan 2019 00:30:36 +0100 Subject: [PATCH 1/6] Fixing issue where help progress was left rendered even when the get-help cmdlet had completed. --- .../help/HelpCommands.cs | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/src/System.Management.Automation/help/HelpCommands.cs b/src/System.Management.Automation/help/HelpCommands.cs index ab269c2c944..f410f5199b0 100644 --- a/src/System.Management.Automation/help/HelpCommands.cs +++ b/src/System.Management.Automation/help/HelpCommands.cs @@ -240,9 +240,10 @@ protected override void BeginProcessing() /// protected override void ProcessRecord() { + HelpSystem helpSystem = this.Context.HelpSystem; try { - this.Context.HelpSystem.OnProgress += new HelpSystem.HelpProgressHandler(HelpSystem_OnProgress); + helpSystem.OnProgress += new HelpSystem.HelpProgressHandler(HelpSystem_OnProgress); bool failed = false; HelpCategory helpCategory = ToHelpCategory(Category, ref failed); @@ -268,7 +269,7 @@ protected override void ProcessRecord() // the idea is to use yield statement in the help lookup to speed up // perceived user experience....So HelpSystem.GetHelp returns an // IEnumerable.. - IEnumerable helpInfos = this.Context.HelpSystem.GetHelp(helpRequest); + IEnumerable helpInfos = helpSystem.GetHelp(helpRequest); // HelpCommand acts differently when there is just one help object and when // there are more than one object...so handling this behavior through // some variables. @@ -319,13 +320,13 @@ protected override void ProcessRecord() // show errors only if there is no wildcard search or VerboseHelpErrors is true. if (((countOfHelpInfos == 0) && (!WildcardPattern.ContainsWildcardCharacters(helpRequest.Target))) - || this.Context.HelpSystem.VerboseHelpErrors) + || helpSystem.VerboseHelpErrors) { // Check if there is any error happened. If yes, // pipe out errors. - if (this.Context.HelpSystem.LastErrors.Count > 0) + if (helpSystem.LastErrors.Count > 0) { - foreach (ErrorRecord errorRecord in this.Context.HelpSystem.LastErrors) + foreach (ErrorRecord errorRecord in helpSystem.LastErrors) { WriteError(errorRecord); } @@ -334,9 +335,10 @@ protected override void ProcessRecord() } finally { - this.Context.HelpSystem.OnProgress -= new HelpSystem.HelpProgressHandler(HelpSystem_OnProgress); + helpSystem.OnProgress -= new HelpSystem.HelpProgressHandler(HelpSystem_OnProgress); + HelpSystem_OnComplete(); // finally clear the ScriptBlockAst -> Token[] cache - this.Context.HelpSystem.ClearScriptBlockTokenCache(); + helpSystem.ClearScriptBlockTokenCache(); } } @@ -657,9 +659,20 @@ private void LaunchOnlineHelp(Uri uriToLaunch) private void HelpSystem_OnProgress(object sender, HelpProgressInfo arg) { - ProgressRecord record = new ProgressRecord(0, this.CommandInfo.Name, arg.Activity); + var record = new ProgressRecord(0, this.CommandInfo.Name, arg.Activity) + { + PercentComplete = arg.PercentComplete + }; + + WriteProgress(record); + } - record.PercentComplete = arg.PercentComplete; + private void HelpSystem_OnComplete() + { + var record = new ProgressRecord(0, this.CommandInfo.Name, "Completed") + { + RecordType = ProgressRecordType.Completed + }; WriteProgress(record); } From 9b286be1f78714bcd5af568fa3f2580bf3150f97 Mon Sep 17 00:00:00 2001 From: Staffan Gustafsson Date: Wed, 6 Feb 2019 13:41:19 +0100 Subject: [PATCH 2/6] Adding test for progress competion --- test/powershell/engine/Help/HelpSystem.Tests.ps1 | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/test/powershell/engine/Help/HelpSystem.Tests.ps1 b/test/powershell/engine/Help/HelpSystem.Tests.ps1 index f1cf27f8b5c..60bc1078ac1 100644 --- a/test/powershell/engine/Help/HelpSystem.Tests.ps1 +++ b/test/powershell/engine/Help/HelpSystem.Tests.ps1 @@ -16,6 +16,8 @@ $script:cmdletsToSkip = @( "Disable-ExperimentalFeature" ) +$PSDefaultParameterValues["Update-Help:UICulture"] = "en-US" + function UpdateHelpFromLocalContentPath { param ([string]$ModuleName, [string] $Scope = 'CurrentUser') @@ -96,7 +98,15 @@ Describe "Validate that get-help works for CurrentUserScope" -Tags @('CI') { } } -Describe "Validate that get-help works for AllUsers Scope" -Tags @('Feature','RequireAdminOnWindows', 'RequireSudoOnUnix') { +Describe "Testing Get-Help Progress" -Tags @('CI') { + It "Last ProgressRecord should be Completed" { + $j = Start-Job { Get-Help DoesNotExist } + $j | Wait-Job + $j.ChildJobs[0].Progress[-1].RecordType | Should -Be ([System.Management.Automation.ProgressRecordType]::Completed) + } +} + +Describe "Validate that get-help works for AllUsers Scope" -Tags @('Feature', 'RequireAdminOnWindows', 'RequireSudoOnUnix') { BeforeAll { $SavedProgressPreference = $ProgressPreference $ProgressPreference = "SilentlyContinue" @@ -507,7 +517,7 @@ Describe "Help failure cases" -Tags Feature { It "An error is returned for a topic that doesn't exist: " -TestCases @( @{ command = "help" }, @{ command = "get-help" } - ){ + ) { param($command) { & $command foobar -ErrorAction Stop } | Should -Throw -ErrorId "HelpNotFound,Microsoft.PowerShell.Commands.GetHelpCommand" From d1fdabefce3fa7d0b98529d36631f4e4effff7d8 Mon Sep 17 00:00:00 2001 From: Staffan Gustafsson Date: Thu, 7 Feb 2019 13:07:31 +0100 Subject: [PATCH 3/6] Tag test `Feature` instead of `CI` --- test/powershell/engine/Help/HelpSystem.Tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/powershell/engine/Help/HelpSystem.Tests.ps1 b/test/powershell/engine/Help/HelpSystem.Tests.ps1 index 60bc1078ac1..88a8bbe2266 100644 --- a/test/powershell/engine/Help/HelpSystem.Tests.ps1 +++ b/test/powershell/engine/Help/HelpSystem.Tests.ps1 @@ -98,7 +98,7 @@ Describe "Validate that get-help works for CurrentUserScope" -Tags @('CI') { } } -Describe "Testing Get-Help Progress" -Tags @('CI') { +Describe "Testing Get-Help Progress" -Tags @('Feature') { It "Last ProgressRecord should be Completed" { $j = Start-Job { Get-Help DoesNotExist } $j | Wait-Job From bc5d085585d4a48f578f807830ca3a95f3e954d6 Mon Sep 17 00:00:00 2001 From: Staffan Gustafsson Date: Thu, 7 Feb 2019 19:07:21 +0100 Subject: [PATCH 4/6] Adding newline before comment --- src/System.Management.Automation/help/HelpCommands.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/System.Management.Automation/help/HelpCommands.cs b/src/System.Management.Automation/help/HelpCommands.cs index f410f5199b0..dcf76088a8d 100644 --- a/src/System.Management.Automation/help/HelpCommands.cs +++ b/src/System.Management.Automation/help/HelpCommands.cs @@ -337,6 +337,7 @@ protected override void ProcessRecord() { helpSystem.OnProgress -= new HelpSystem.HelpProgressHandler(HelpSystem_OnProgress); HelpSystem_OnComplete(); + // finally clear the ScriptBlockAst -> Token[] cache helpSystem.ClearScriptBlockTokenCache(); } From 898ab3da2bb9b44389ef99a7245478b8501b751e Mon Sep 17 00:00:00 2001 From: Staffan Gustafsson Date: Thu, 7 Mar 2019 16:14:39 +0100 Subject: [PATCH 5/6] Addressing Aditya's comments --- test/powershell/engine/Help/HelpSystem.Tests.ps1 | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/test/powershell/engine/Help/HelpSystem.Tests.ps1 b/test/powershell/engine/Help/HelpSystem.Tests.ps1 index 88a8bbe2266..e0ca30e0094 100644 --- a/test/powershell/engine/Help/HelpSystem.Tests.ps1 +++ b/test/powershell/engine/Help/HelpSystem.Tests.ps1 @@ -16,8 +16,6 @@ $script:cmdletsToSkip = @( "Disable-ExperimentalFeature" ) -$PSDefaultParameterValues["Update-Help:UICulture"] = "en-US" - function UpdateHelpFromLocalContentPath { param ([string]$ModuleName, [string] $Scope = 'CurrentUser') @@ -100,9 +98,14 @@ Describe "Validate that get-help works for CurrentUserScope" -Tags @('CI') { Describe "Testing Get-Help Progress" -Tags @('Feature') { It "Last ProgressRecord should be Completed" { - $j = Start-Job { Get-Help DoesNotExist } - $j | Wait-Job - $j.ChildJobs[0].Progress[-1].RecordType | Should -Be ([System.Management.Automation.ProgressRecordType]::Completed) + try { + $j = Start-Job { Get-Help DoesNotExist } + $j | Wait-Job + $j.ChildJobs[0].Progress[-1].RecordType | Should -Be ([System.Management.Automation.ProgressRecordType]::Completed) + } + finally { + $j | Remove-Job + } } } From 7054058846fa40d23a5995e38c4bb6e39abc44d4 Mon Sep 17 00:00:00 2001 From: Aditya Patwardhan Date: Mon, 11 Mar 2019 10:35:32 -0700 Subject: [PATCH 6/6] [Feature]