FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Add tests for #1504 PSUseUsingScopeModifierInNewRunspaces by o-l-a-v · Pull Request #2005 · PowerShell/PSScriptAnalyzer · GitHub

/ PSScriptAnalyzer Public

Add tests for #1504 PSUseUsingScopeModifierInNewRunspaces - #2005

Draft
Olav Rønnestad Birkeland (o-l-a-v) wants to merge 6 commits into
PowerShell:mainfrom
o-l-a-v:test-for-issue-1504
ConversationCommits (6)ChecksFiles changed
Draft

Add tests for #1504 PSUseUsingScopeModifierInNewRunspaces#2005
Olav Rønnestad Birkeland (o-l-a-v) wants to merge 6 commits into
PowerShell:mainfrom
o-l-a-v:test-for-issue-1504

Conversation

Olav Rønnestad Birkeland (o-l-a-v) commented May 27, 2024

Copy link
Copy Markdown
Contributor

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 trigger PSUseUsingScopeModifierInNewRunspaces, which I added to test:

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

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

PR Checklist

Christoph Bergmeister (bergmeister) approved these changes Feb 17, 2025
View reviewed changes

Christoph Bergmeister (bergmeister) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

Comment thread Tests/Rules/UseUsingScopeModifierInNewRunspaces.tests.ps1
}
}
'@
ScriptBlock = '{

Christoph Bergmeister (bergmeister) Feb 17, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

thanks for making it more readable :-)

Andy Jordan (andyleejordan) commented Mar 14, 2025

Copy link
Copy Markdown
Member

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

Olav Rønnestad Birkeland (o-l-a-v) commented Mar 17, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

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

Christoph Bergmeister (bergmeister) commented Mar 17, 2025

Copy link
Copy Markdown
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

Christoph Bergmeister (bergmeister) marked this pull request as draft March 17, 2025 15:29

Olav Rønnestad Birkeland (o-l-a-v) commented Mar 17, 2025

Copy link
Copy Markdown
Contributor 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.

Andy Jordan (andyleejordan) commented Mar 17, 2025

Copy link
Copy Markdown
Member

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

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Christoph Bergmeister (bergmeister) bergmeister approved these changes
Andy Jordan (andyleejordan) Awaiting requested review from andyleejordan
Copilot code review Copilot
Copilot will automatically review once the pull request is marked ready for review

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

Back | FazBrowse Home | New Git URL