-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Updated thread job to version 1.1.2 #7522
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.
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
Updated thread job to version 1.1.2 #7522
Conversation
…dation attribute for Arguments parameter, and made test work with strict mode
SteveL-MSFT
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
dantraMSFT
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Fixed botched merge.
| } | ||
|
|
||
| $numThreadJobs = (Get-Job | where PSJobTypeName -eq "ThreadJob").Count | ||
| $numThreadJobs = @(Get-Job | where PSJobTypeName -eq "ThreadJob").Count |
There was a problem hiding this comment.
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 0There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where -> Where-Object
adityapatwardhan
left a comment
There was a problem hiding this 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missed one where here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. Thanks.
PR Summary
Added fix to remove unneeded validation attribute for Arguments parameter, and made tests work with strict mode.
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:to the beginning of the title and remove the prefix when the PR is ready.[feature]if the change is significant or affects feature tests