8000 Fixing issue with help progress by powercode · Pull Request #8788 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]

Skip to content

Fixing issue with help progress#8788

Merged
adityapatwardhan merged 6 commits intoPowerShell:masterfrom
powercode:HelpProgress
Mar 11, 2019
Merged

Fixing issue with help progress#8788
adityapatwardhan merged 6 commits intoPowerShell:masterfrom
powercode:HelpProgress

Conversation

@powercode
Copy link
Collaborator
@powercode powercode commented Jan 29, 2019

The Get-Help cmdlet never calls WriteProgress with a RecordType set to Completed, so the progress is left being rendered even after the Get-Help cmdlet has completed.

The PR is adding a write progress call with recordtype set to Completed.

It also cleans up lots of chains to this.Context.HelpSystem by extracting a variable.

PR Summary

PR Context

PR Checklist

@adityapatwardhan
Copy link
Member

@powercode Love the change! Could you add some tests? Something like the following.

Describe "Testing Get-Help Progress" {
 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)
}}

Copy link
Collaborator
@iSazonov iSazonov left a comment

Choose a reason for hiding this comment

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

LGTM with one style comment.

@iSazonov iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Feb 7, 2019
@iSazonov
Copy link
Collaborator
iSazonov commented Feb 8, 2019

Reopen to restart all CIs (no response).

@iSazonov iSazonov closed this Feb 8, 2019
@iSazonov iSazonov reopened this Feb 8, 2019
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)
Copy link
Member

Choose a reason for hiding this comment

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

Please add a Remove-Job $j

Copy link
Collaborator

Choose a reason for hiding this comment

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

It should be in try-finally.

@adityapatwardhan
Copy link
Member

@powercode Pushed an empty commit to execute Feature tests. The new tests are categorized as Feature.

@adityapatwardhan adityapatwardhan merged commit 0ebbdc1 into PowerShell:master Mar 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

0