Skip to content

Conversation

@PaulHigin
Copy link
Contributor

PR Summary

Added fix to remove unneeded validation attribute for Arguments parameter, and made tests work with strict mode.

PR Checklist

…dation attribute for Arguments parameter, and made test work with strict mode
Copy link
Member

@SteveL-MSFT SteveL-MSFT left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@dantraMSFT dantraMSFT left a comment

Choose a reason for hiding this comment

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

LGTM

}

$numThreadJobs = (Get-Job | where PSJobTypeName -eq "ThreadJob").Count
$numThreadJobs = @(Get-Job | where PSJobTypeName -eq "ThreadJob").Count
Copy link
Member

Choose a reason for hiding this comment

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

Pester 4 syntax has -HaveCount. This can be converted to:

@(Get-Job | where-object { PSJobTypeName -eq "ThreadJob"}) | Should -HaveCount 0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Will change.

}

$numThreadJobs = (Get-Job | where PSJobTypeName -eq "ThreadJob").Count
$numThreadJobs = @(Get-Job | where PSJobTypeName -eq "ThreadJob").Count
Copy link
Member

Choose a reason for hiding this comment

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

Do not use alias where -> Where-Object

$null = $job1,$job2,$job3,$job4 | Receive-Job -Wait -AutoRemoveJob

(Get-Job | where PSJobTypeName -eq "ThreadJob").Count | Should -Be 0
@(Get-Job | where PSJobTypeName -eq "ThreadJob").Count | Should -Be 0
Copy link
Member

Choose a reason for hiding this comment

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

Please use -HaveCount.

$null = $job1,$job2,$job3,$job4 | Receive-Job -Wait -AutoRemoveJob

(Get-Job | where PSJobTypeName -eq "ThreadJob").Count | Should -Be 0
@(Get-Job | where PSJobTypeName -eq "ThreadJob").Count | Should -Be 0
Copy link
Member

Choose a reason for hiding this comment

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

where -> Where-Object


$numRunningThreadJobs = (Get-Job | where { ($_.PSJobTypeName -eq "ThreadJob") -and ($_.State -eq "Running") }).Count
$numQueuedThreadJobs = (Get-Job | where { ($_.PSJobTypeName -eq "ThreadJob") -and ($_.State -eq "NotStarted") }).Count
$numRunningThreadJobs = @(Get-Job | where { ($_.PSJobTypeName -eq "ThreadJob") -and ($_.State -eq "Running") }).Count
Copy link
Member

Choose a reason for hiding this comment

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

where -> Where-Object

Copy link
Member

@adityapatwardhan adityapatwardhan left a comment

Choose a reason for hiding this comment

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

Approved with one comment about an alias where


$numThreadJobs = (Get-Job | where PSJobTypeName -eq "ThreadJob").Count
$numThreadJobs | Should -Be 0
Get-Job | where PSJobTypeName -eq "ThreadJob" | Should -HaveCount 0
Copy link
Member

Choose a reason for hiding this comment

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

Missed one where here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it. Thanks.

@TravisEz13 TravisEz13 merged commit a213667 into PowerShell:master Aug 15, 2018
@PaulHigin PaulHigin deleted the update-threadjob-version branch August 15, 2018 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-Maintainers-Build specific to affecting the build

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants