Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Add tests for #1504 PSUseUsingScopeModifierInNewRunspaces#2005

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
o-l-a-v wants to merge6 commits intoPowerShell:main
base:main
Choose a base branch
Loading
fromo-l-a-v:test-for-issue-1504

Conversation

o-l-a-v
Copy link

PR Summary

Wrote two tests for issue#1504. The issue is not fixed yet, but I had two scenarios that could easily be added to the relevant test file.

Repro to wrongfully triggerPSUseUsingScopeModifierInNewRunspaces, which I added to test:

# Microsoft.PowerShell.Core \ Start-JobStart-Job-ScriptBlock {Param($Foo)$Foo}-ArgumentList'Bar'|Receive-Job-Wait-AutoRemoveJob# Microsoft.PowerShell.ThreadJobStart-ThreadJob-ScriptBlock {Param($Foo)$Foo}-ArgumentList'Bar'|Receive-Job-Wait-AutoRemoveJob

PR Checklist

Copy link
Collaborator

@bergmeisterbergmeister left a comment

Choose a reason for hiding this comment

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

updated branch to run new CI, looks good otherwise, always happy to have more tests :-)

}
}
'@
ScriptBlock = '{
Copy link
Collaborator

Choose a reason for hiding this comment

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

thanks for making it more readable :-)

@andyleejordan
Copy link
Member

Are these essentially testing that the extant bug exists, and should fail when we fix the bug (meaning we'll flip their assertion)?

@o-l-a-v
Copy link
Author

o-l-a-v commentedMar 17, 2025•
edited
Loading

Sorry for the late reply. I wrote the rules with the goal of them failing with current behavior.

@bergmeister
Copy link
Collaborator

Sorry for the late reply. I wrote the rules with the goal of them failing with current behavior.

Gotcha, makes sense now. Do you plan to fix the issue as part of this PR? Changing it to draft for now then

o-l-a-v reacted with thumbs up emoji

@bergmeisterbergmeister marked this pull request as draftMarch 17, 2025 15:29
@o-l-a-v
Copy link
Author

Gotcha, makes sense now. Do you plan to fix the issue as part of this PR? Changing it to draft for now then

No. I just had two very concrete examples that I wanted to add somewhere for when this maybe gets fixed in the future.

bergmeister and andyleejordan reacted with thumbs up emoji

@andyleejordan
Copy link
Member

Got it, will leave open as a draft PR then. Thanks!

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@bergmeisterbergmeisterbergmeister approved these changes

@andyleejordanandyleejordanAwaiting requested review from andyleejordanandyleejordan is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3 participants
@o-l-a-v@andyleejordan@bergmeister
[8]ページ先頭
©2009-2025 Movatter.jp