Updated background invocation `&` operator to use -WorkingDirectory when starting a new job by ayousuf23 · Pull Request #16180 · PowerShell/PowerShell

ayousuf23 · 2021-09-30T23:01:18Z

PR Summary

Updated the background invocation & operator to use -WorkingDirectory parameter when starting a new job.

PR Context

Previously, Set-Location was appended to the script block of Start-Job in order to set the working directory of a new job invoked by the background invocation operator &. Now that Start-Job accepts a -WorkingDirectory parameter that sets the working directory of the new job, it makes sense to update the code for using the background invocation operator to use the new way of setting the working directory of a new job.

PR Checklist

Ilya (iSazonov) · 2021-10-01T17:49:45Z

ayousuf23 Please add test(s) for a scenario you enhance.

Anam Navied (anamnavi) · 2021-10-05T19:24:26Z

ayousuf23 thanks for creating this PR, I like that you've added a thorough summary and context for this PR as this is really helpful :) the code looks good but having tests will just help us verify it works as expected, this is something we can talk about in the Semester of Code office hours this week, thanks again for creating this!

ayousuf23 · 2021-10-13T23:38:01Z

I accidentally erased my previous commit from this PR. I will add the previous commit.

…ing directory of new job and added a testto ensure background job script block does not use Set-Location to set working directory

ayousuf23 · 2021-10-14T00:07:28Z

Right now, the build is failing on my computer. I think this is because of other files (not the ones I changed). Ilya (@iSazonov) I added a test to ensure Set-Location is not added to the background job's script block to set the working directory. Do you think this is enough?

Ilya (iSazonov) · 2021-10-14T03:44:09Z

I added a test to ensure Set-Location is not added to the background job's script block to set the working directory. Do you think this is enough?

I see old code and new code to set location to PWD. I think we already have tests for Set-Location -WorkingDirectory and we should have the same tests for background operator too but I don't know whether we have them. Such test could change current location, run a background script block and check that current location is changed in the script block too.

Ilya (iSazonov) · 2021-10-14T03:46:13Z

Right now, the build is failing on my computer. I think this is because of other files (not the ones I changed)

Please create new branch on a commit before #15177, apply your commits and check if test fails still exist.

/cc Dongbo Wang (@daxian-dbw)

ayousuf23 · 2021-10-21T23:12:59Z

Ilya (@iSazonov) Thank you for your suggestion. I will add another test.

…ackground job's working directory

ayousuf23 · 2021-10-23T23:43:35Z

Ilya (@iSazonov) I added a test to ensure changing the working directory also changes a new background job's working directory.

Ilya (iSazonov) · 2021-10-24T08:09:31Z

ayousuf23 Please look test fails.

ayousuf23 · 2021-10-25T22:17:34Z

I merged the master branch with ayousuf23/issue-10515 to see if that fixes the errors. I looked at the previous test logs and it doesn't seem like the errors are related to the code I added.

Ilya (iSazonov) · 2021-10-26T07:28:23Z

it doesn't seem like the errors are related to the code I added.

Master always stable - we never merge a commit if there are fail tests.
You can open sources of the tests and do its code manually, debug and see what is wrong.

ayousuf23 · 2021-10-26T16:27:51Z

Ilya (@iSazonov) Thank you for your suggestion. I’ll try and get the failing tests fixed this week :)

ayousuf23 · 2021-10-28T23:24:06Z

Ilya (@iSazonov) I think I fixed the issue. I am waiting for the tests to run in order to be sure.

ayousuf23 · 2021-10-28T23:59:21Z

Ilya (@iSazonov) The tests passed :)

Ilya (iSazonov) · 2021-10-29T05:25:57Z

+            $j = (get-variable -value ExecutionContext).SessionState.PSVariable.Get("MyInvocation").Value.MyCommand.ScriptBlock & 
+            (Receive-Job -Wait $j).ToString() | Should -BeExactly "(get-variable -value ExecutionContext).SessionState.PSVariable.Get(`"MyInvocation`").Value.MyCommand.ScriptBlock"


Suggested change

$j = (get-variable -value ExecutionContext).SessionState.PSVariable.Get("MyInvocation").Value.MyCommand.ScriptBlock &

(Receive-Job -Wait $j).ToString() | Should -BeExactly "(get-variable -value ExecutionContext).SessionState.PSVariable.Get(`"MyInvocation`").Value.MyCommand.ScriptBlock"

$j = (Get-Variable -Value ExecutionContext).SessionState.PSVariable.Get("MyInvocation").Value.MyCommand.ScriptBlock &

(Receive-Job -Wait $j).ToString() | Should -BeExactly "(Get-Variable -Value ExecutionContext).SessionState.PSVariable.Get(`"MyInvocation`").Value.MyCommand.ScriptBlock"

ayousuf23 · 2022-01-10T10:15:49Z

Anam Navied (@anamnavi) Sorry for the late response. I added parameter names for the arguments, as you suggested.

ayousuf23 · 2022-01-13T12:22:19Z

Anam Navied (@anamnavi) Ilya (@iSazonov) Should I combine all the commits into one commit?

Ilya (iSazonov) · 2022-01-13T13:43:31Z

Anam Navied (@anamnavi) Ilya (@iSazonov) Should I combine all the commits into one commit?

No, we will do squash in merge time.

Deleted user (ghost) · 2022-01-22T14:00:43Z

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

ayousuf23 · 2022-02-01T12:49:07Z

Ilya (@iSazonov) Is there anything remaining for me to do in order to get the PR merged?

Ilya (iSazonov) · 2022-02-01T13:09:46Z

ayousuf23 We are waiting a review from anybody from MSFT team..

ayousuf23 · 2022-02-01T20:54:00Z

Anam Navied (@anamnavi) Can you please review this PR?

Dongbo Wang (daxian-dbw) · 2022-02-01T23:04:00Z

ayousuf23 Sorry for the delay of response from the team. Can you please mention the number of the issue that you fixed with this PR in the PR description (in the form Fix #xxxxx)? So the issue can be closed automatically when merging this PR.

Anam Navied (@anamnavi) already signed off. I will ping Travis Plunk (@TravisEz13) offline to remind him of this PR.

ayousuf23 · 2022-02-01T23:36:25Z

Dongbo Wang (@daxian-dbw) Thanks for your comment! I updated the PR's description.

Ilya (iSazonov) · 2022-02-02T17:15:14Z

Dongbo Wang (@daxian-dbw) I see the error on Windows CI "##[error]This is a scheduled windows-2016 brownout. The windows-2016 environment is deprecated and will be removed on March 15, 2022. For more details, see actions/runner-images#4312"

Deleted user (ghost) · 2022-02-10T02:00:41Z

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

pull-request-quantifier-deprecated · 2022-02-11T18:42:26Z

This PR has 34 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!

Quantification details

Label      : Extra Small
Size       : +24 -10
Percentile : 13.6%

Total files changed: 2

Change summary by file extension:
.cs : +15 -9
.ps1 : +9 -1

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

Fast and predictable releases to production:
- Optimal size changes are more likely to be reviewed faster with fewer
  iterations.
- Similarity in low PR complexity drives similar review times.
Review quality is likely higher as complexity is lower:
- Bugs are more likely to be detected.
- Code inconsistencies are more likely to be detetcted.
Knowledge sharing is improved within the participants:
- Small portions can be assimilated better.
Better engineering practices are exercised:
- Solving big problems by dividing them in well contained, smaller problems.
- Exercising separation of concerns within the code changes.

What can I do to optimize my changes

Use the PullRequestQuantifier to quantify your PR accurately
- Create a context profile for your repo using the context generator
- Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
- Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
- Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
Change your engineering behaviors
- For PRs that fall outside of the desired spectrum, review the details and check if:
  - Your PR could be split in smaller, self-contained PRs instead
  - Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

One line was added: +1 -0
One line was deleted: +0 -1
One line was modified: +1 -1 (git diff doesn't know about modified, it will
interpret that line like one addition plus one deletion)
Change percentiles: Change characteristics (addition, deletion, modification)
of this PR in relation to all other PRs within the repository.

Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

Deleted user (ghost) · 2022-02-19T02:00:41Z

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

Ilya (iSazonov) · 2022-02-21T18:05:44Z

ayousuf23 Thanks for your contribution!

… when starting a new job (PowerShell#16180)

ghost assigned Travis Plunk (TravisEz13) Sep 30, 2021

ayousuf23 mentioned this pull request Oct 1, 2021

& background invocation operator should -WorkingDirectory when starting a job #10515

Closed

Ilya (iSazonov) requested a review from Aditya Patwardhan (adityapatwardhan) October 1, 2021 17:49

Ilya (iSazonov) added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Oct 1, 2021

James Truher (JamesWTruher) requested review from James Truher (JamesWTruher), alerickson, Anam Navied (anamnavi) and Andy Jordan (andyleejordan) October 2, 2021 17:44

ayousuf23 force-pushed the issue-10515 branch from 3279c97 to a8f79c8 Compare October 13, 2021 23:31

made background invocation operator use -WorkingDirectory to set work…

412fffa

…ing directory of new job and added a testto ensure background job script block does not use Set-Location to set working directory

ayousuf23 force-pushed the issue-10515 branch from a8f79c8 to 412fffa Compare October 14, 2021 00:04

added a test to make sure changing working directory also changes a b…

7112fa1

…ackground job's working directory

Merge remote-tracking branch 'origin' into issue-10515

8b1522a

fixed error when running background jobs

9d4949d

pull-request-quantifier-deprecated Bot added the Extra Small label Oct 28, 2021

Ilya (iSazonov) reviewed Oct 29, 2021

View reviewed changes

ghost removed the Review - Needed The PR is being reviewed label Jan 13, 2022

ghost added the Review - Needed The PR is being reviewed label Jan 22, 2022

ghost removed the Review - Needed The PR is being reviewed label Feb 1, 2022

Add a few new lines to break up the code

5dc5651

Travis Plunk (TravisEz13) approved these changes Feb 2, 2022

View reviewed changes

ghost added the Review - Needed The PR is being reviewed label Feb 10, 2022

Dongbo Wang (daxian-dbw) closed this Feb 11, 2022

Dongbo Wang (daxian-dbw) reopened this Feb 11, 2022

ghost removed the Review - Needed The PR is being reviewed label Feb 11, 2022

ghost added the Review - Needed The PR is being reviewed label Feb 19, 2022

Dongbo Wang (daxian-dbw) assigned Dongbo Wang (daxian-dbw) and unassigned Travis Plunk (TravisEz13) Feb 21, 2022

ghost removed the Review - Needed The PR is being reviewed label Feb 21, 2022

Dongbo Wang (daxian-dbw) merged commit 31c14f2 into PowerShell:master Feb 21, 2022

Dongbo Wang (daxian-dbw) mentioned this pull request Nov 28, 2022

Starting background job in a UNC path with & returns error #18618

Closed

5 tasks done

Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request Aug 6, 2026

Updated background invocation & operator to use -WorkingDirectory…

7a1d32a

… when starting a new job (PowerShell#16180)

		$j = (get-variable -value ExecutionContext).SessionState.PSVariable.Get("MyInvocation").Value.MyCommand.ScriptBlock &
		(Receive-Job -Wait $j).ToString() \| Should -BeExactly "(get-variable -value ExecutionContext).SessionState.PSVariable.Get(`"MyInvocation`").Value.MyCommand.ScriptBlock"

Conversation

ayousuf23 commented Sep 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Context

PR Checklist

Uh oh!

Ilya (iSazonov) commented Oct 1, 2021

Uh oh!

Anam Navied (anamnavi) commented Oct 5, 2021

Uh oh!

ayousuf23 commented Oct 13, 2021

Uh oh!

ayousuf23 commented Oct 14, 2021

Uh oh!

Ilya (iSazonov) commented Oct 14, 2021

Uh oh!

Ilya (iSazonov) commented Oct 14, 2021

Uh oh!

ayousuf23 commented Oct 21, 2021

Uh oh!

ayousuf23 commented Oct 23, 2021

Uh oh!

Ilya (iSazonov) commented Oct 24, 2021

Uh oh!

ayousuf23 commented Oct 25, 2021

Uh oh!

Ilya (iSazonov) commented Oct 26, 2021

Uh oh!

ayousuf23 commented Oct 26, 2021

Uh oh!

ayousuf23 commented Oct 28, 2021

Uh oh!

ayousuf23 commented Oct 28, 2021

Uh oh!

Ilya (iSazonov) Oct 29, 2021

Choose a reason for hiding this comment

Uh oh!

ayousuf23 commented Jan 10, 2022

Uh oh!

ayousuf23 commented Jan 13, 2022

Uh oh!

Ilya (iSazonov) commented Jan 13, 2022

Uh oh!

Deleted user (ghost) commented Jan 22, 2022

Uh oh!

ayousuf23 commented Feb 1, 2022

Uh oh!

Ilya (iSazonov) commented Feb 1, 2022

Uh oh!

ayousuf23 commented Feb 1, 2022

Uh oh!

Dongbo Wang (daxian-dbw) commented Feb 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ayousuf23 commented Feb 1, 2022

Uh oh!

Ilya (iSazonov) commented Feb 2, 2022

Uh oh!

Deleted user (ghost) commented Feb 10, 2022

Uh oh!

pull-request-quantifier-deprecated Bot commented Feb 11, 2022

What can I do to optimize my changes

How to interpret the change counts in git diff output

Uh oh!

Deleted user (ghost) commented Feb 19, 2022

Uh oh!

Ilya (iSazonov) commented Feb 21, 2022

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ayousuf23 commented Sep 30, 2021 •

edited

Loading

Dongbo Wang (daxian-dbw) commented Feb 1, 2022 •

edited

Loading