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

[ayousuf23]

PR Summary

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

Fix #10515

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

• PR has a meaningful title
  • Use the present tense and imperative mood when describing your changes
• [ x] Summarized changes
• Make sure all .h, .cpp, .cs, .ps1 and .psm1 files have the correct copyright header
• This PR is ready to merge and is not Work in Progress.
  • If the PR is work in progress, please add the prefix WIP: or [ WIP ] to the beginning of the title (the WIP bot will keep its status check at Pending while the prefix is present) and remove the prefix when the PR is ready.
• Breaking changes
  • None
  • OR
  • Experimental feature(s) needed
    • Experimental feature name(s):
• User-facing changes
  • Not Applicable
  • OR
  • Documentation needed
    • Issue filed:
• Testing - New and feature
  • [] N/A or can only be tested interactively
  • OR
  • [ x] Make sure you've added a new test if existing tests do not effectively test the code changed
• Tooling
  • I have considered the user experience from a tooling perspective and don't believe tooling will be impacted.
  • OR
  • I have considered the user experience from a tooling perspective and opened an issue in the relevant tool repository. This may include:
    • Impact on PowerShell Editor Services which is used in the PowerShell extension for VSCode
      (which runs in a different PS Host).
      • Issue filed:
    • Impact on Completions (both in the console and in editors) - one of PowerShell's most powerful features.
      • Issue filed:
    • Impact on PSScriptAnalyzer (which provides linting & formatting in the editor extensions).
      • Issue filed:
    • Impact on EditorSyntax (which provides syntax highlighting with in VSCode, GitHub, and many other editors).
      • Issue filed:

[iSazonov]

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

[anamnavi]

@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]

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

[ayousuf23]

Right now, the build is failing on my computer. I think this is because of other files (not the ones I changed). @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?

[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?

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.

[iSazonov]

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 @daxian-dbw

[ayousuf23]

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

[ayousuf23]

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

[iSazonov]

@ayousuf23 Please look test fails.

[ayousuf23]

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.

[iSazonov]

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]

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

[ayousuf23]

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

[ayousuf23]

@iSazonov The tests passed :)

[iSazonov]

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]

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

[ayousuf23]

@anamnavi @iSazonov Should I combine all the commits into one commit?

[iSazonov]

@anamnavi @iSazonov Should I combine all the commits into one commit?

No, we will do squash in merge time.

[ghost]

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]

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

[iSazonov]

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

[ayousuf23]

@anamnavi Can you please review this PR?

[daxian-dbw]

@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.

@anamnavi already signed off. I will ping @TravisEz13 offline to remind him of this PR.

[ayousuf23]

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

[iSazonov]

@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"

[ghost]

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]

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.

[ghost]

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

[iSazonov]

@ayousuf23 Thanks for your contribution!