@KirkMunro Is it possible to split the PR on some small ones? The PR is so large that we can wait too long for approval.

@KirkMunro Is it possible to split the PR on some small ones? The PR is so large that we can wait too long for approval.

That sounds like quite a bit more work, with little tangible value; however, a "review map" might help. Here is a break down of the changes in each file, with like changes grouped together to allow them to be reviewed more easily. Between these extra details and being able to mark files as viewed in the GitHub UI, I think it should be much easier for others to review this PR.

Cleanup of changes done in @TylerLeonhardt's other PR that this supercedes

src/Microsoft.PowerShell.Commands.Utility/commands/utility/New-PSBreakpoint.cs
src/Microsoft.PowerShell.Commands.Utility/commands/utility/EnableDisableRunspaceDebugCommand.cs

• simple removal of changes added by @TylerLeonhardt in PR New New-PSBreakpoint cmdlet & new -Breakpoint parameter for Debug-Runspace #8923; instead of
  creating breakpoints using New-PSBreakpoint and passing breakpoints into
  Enable-RunspaceDebug, breakpoints are managed on a runspace using the
  original set of *-PSBreakpoint cmdlets.

src/Microsoft.PowerShell.Commands.Utility/commands/utility/PSBreakpointCreationBase.cs

• simple removal of previous refactoring work that was done in the same PR

src/Modules/Unix/Microsoft.PowerShell.Utility/Microsoft.PowerShell.Utility.psd1
src/Modules/Windows/Microsoft.PowerShell.Utility/Microsoft.PowerShell.Utility.psd1

• removed New-PSBreakpoint reference
• changed experimental feature name/description to match what it actually does now
  (this was previously missing from the Windows version of the file, and would have been
  added in List experimental feature for NewPSBreakpoint in Module manifest for Windows #9907 if this PR didn't supercede it)

src/System.Management.Automation/engine/InitialSessionState.cs

• removed New-PSBreakpoint reference

test/powershell/engine/Basic/DefaultCommands.Tests.ps1

• removed New-PSBreakpoint reference

test/powershell/Modules/Microsoft.PowerShell.Utility/Enable-RunspaceDebug.Tests.ps1

• removed this file as it was based on the other PR that this supercedes

Changes for Pester tests

test/powershell/Modules/Microsoft.PowerShell.Utility/RunspaceBreakpointManagement.Tests.ps1

• added tests for runspace breakpoint management (both local and remote runspaces)

test/tools/TestMetadata.json

• updated to match new experimental feature name and test file

Changes to support -Runspace parameter in *-PSBreakpoint commands

src/Microsoft.PowerShell.Commands.Utility/commands/utility/RunspaceParameter.cs

• defined a transformation attribute that converts integers, GUIDs and strings into
  runspaces; this is for use in the -Runspace parameter added to the *-PSBreakpoint
  cmdlets updated below (it keeps the number of parameter sets low in those commands
  while still supporting multiple ways to reference a runspace)

src/Microsoft.PowerShell.Commands.Utility/commands/utility/PSBreakpointCommandBase.cs
src/Microsoft.PowerShell.Commands.Utility/commands/utility/PSBreakpointAccessorCommandBase.cs
src/Microsoft.PowerShell.Commands.Utility/commands/utility/Get-PSBreakpoint.cs
src/Microsoft.PowerShell.Commands.Utility/commands/utility/Set-PSBreakpoint.cs
src/Microsoft.PowerShell.Commands.Utility/commands/utility/PSBreakpointUpdaterCommandBase.cs
src/Microsoft.PowerShell.Commands.Utility/commands/utility/Disable-PSBreakpoint.cs
src/Microsoft.PowerShell.Commands.Utility/commands/utility/Enable-PSBreakpoint.cs
src/Microsoft.PowerShell.Commands.Utility/commands/utility/Remove-PSBreakpoint.cs

• added -Runspace parameter to each *-PSBreakpoint cmdlet
• standardized parameter set names across this set of cmdlets
• refactored *-PSBreakpoint cmdlets into class hierarchy, as follows:

                         PSBreakpointCommandBase
                            /              \
  PSBreakpointAccessorCommandBase        PSBreakpointUpdaterCommandBase
               |                                       |
   Get/SetPSBreakpointCommand        Disable/Enable/RemovePSBreakpointCommand
  

Changes to support managing breakpoints in debugger via the SDK

src/System.Management.Automation/engine/debugger/Breakpoint.cs

• fixed minor issue in display of breakpoint that was hit when attaching
  the debugger to a remote runspace
• updated RemoveSelf method to return a bool if a breakpoint was removed

src/System.Management.Automation/engine/debugger/debugger.cs

• added SetCommandBreakpoint, SetLineBreakpoint, SetVariableBreakpoint,
  RemoveBreakpoint, EnableBreakpoint, and DisableBreakpoint public methods
  in Debugger class
• added DebugJob method that allows you to attach a debugger to a job
  without invoking a BreakAll
• changed return type of AddCommandBreakpoint, AddLineBreakpoint and
  AddVariableBreakpoint on ScriptDebugger class to be specific to the type
  of breakpoint they add
• removed NewBreakpoint methods on ScriptDebugger class (these have been
  superceded by the SetBreakpoint public methods)
• updated Remove*Breakpoint methods to return a bool indicating whether or
  not a breakpoint was removed
• defined overrides in ScriptDebugger for the *Breakpoint methods that were
  added to the base class, and grouped them together (this involved replacing
  some methods that were previously internal)
• moved a few methods into proper groupings according to existing regions
• defined overrides in NestedDebugger for the *Breakpoint methods that were
  added to the base class
• added internal virtual function names for breakpoint functions that can now
  be invoked in a remote runspace

src/System.Management.Automation/engine/remoting/client/remoterunspace.cs

• added overrides to remote debugger for the *Breakpoint methods that were
  added to the base class
• added internal helper method to handle invoking those methods remotely,
  capturing their results, and processing any errors

src/System.Management.Automation/engine/remoting/client/Job.cs

• added IsFinished property to simplify code elsewhere that checks if a job
  is finished
• added new Breakpoint methods to JobDebugger class and grouped them together

src/System.Management.Automation/engine/server/ServerRunspacePoolDriver.cs

• added support for the new *Breakpoint virtual functions to the
  PreProcessDebuggerCommand method
• added overrides for *Breakpoint methods to ServerRemoteDebugger class
• added override for new DebugJob overload method to ServerRemoteDebugger class

src/System.Management.Automation/engine/serialization.cs

• added RehydrationFlags.NullValueOk to Script and Action properties of
  breakpoints (this ensures they are set to null instead of empty when rehydrated,
  which matches how they are set before they are dehydrated)

src/System.Management.Automation/engine/hostifaces/PSDataCollection.cs

• added AddRange method to simplify adding a collection to a collection

Addition of -NoInitialBreak parameter to Debug-Job and Debug-Runspace, plus refactoring

src/Microsoft.PowerShell.Commands.Utility/resources/Debugger.resx
src/System.Management.Automation/resources/RemotingErrorIdStrings.resx
src/System.Management.Automation/resources/DebuggerStrings.resx

• moved a bunch of resource strings into src/System.Management.Automation/resources/DebuggerStrings.resx
  as part of the refactoring of DebugRunspaceCommand and DebugJobCommand under
  a common base class
• added some resource strings

src/System.Management.Automation/engine/remoting/commands/PSRemoteDebugCmdlet.cs
src/Microsoft.PowerShell.Commands.Utility/commands/utility/DebugRunspaceCommand.cs
src/System.Management.Automation/engine/remoting/commands/DebugJob.cs

• added -NoInitialBreak parameter to Debug-Runspace and Debug-Job cmdlets
• moved common code out of the cmdlet classes into new PSRemoteDebugCmdlet class
• refactored these two cmdlets to eliminate unintended differences in the UX they
  provided into a class hierarchy, as follows:

             PSRemoteDebugCmdlet
               /              \
  DebugJobCommand        DebugRunspaceCommand
  

I'll try to take a look at this later this week, but may not get to it until the following week. Since this is such a large change I will mark it for committee review.

@PaulHigin FYI for when you do get to look at this:

I'm experiencing an intermittent issue with my RunspaceBreakpointManagement.Tests.ps1 Pester test series where setting a breakpoint in a remote runspace (i.e. one associated with a background job) gets stuck. On my system, when I don't have my VS 2019 debugger connected to the PowerShell session process, I can very often reproduce my intermittent issue the second time that I manually invoke that Pester test file in a single PowerShell session. All tests pass in the first run, but the second run hangs at a certain point (the moment the first MS-PSRDP call is made. Digging in to this on my system, I believe it's getting stuck between the moment it invokes the virtual __Set-PSBreakpoint function in a nested PowerShell instance, which should set the breakpoint on the remote runspace. The local process seems to be waiting for the pipeline to finish, and the debugger in the remote (job) process seems to be waiting for a client to connect to it, so this seems to have to do with the MS-PSRDP. If you wait long enough (30 minutes), the Pester test eventually fails (probably due to a timeout) with a PSRemotingTransportException.

In the PR that this supercedes, @TylerLeonhardt switched many of the collections to their concurrent equivalent, and he updated the breakpoint ID increment to be thread safe as well. Those changes were preserved through this PR work.

I know little about how the MS-PSRDP plumbing works other than what I learned while working on this PR, so I wanted to let you know what I ran into so that you could have a look when you get to this review work because so far I can't figure out the root cause.

Phase 2 of splitting this up: PR #10492, that includes the *-PSBreakpoint cmdlet changes.

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

@KirkMunro Please resolve merge conflicts.

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 15 days. It will be closed if no further activity occurs within 10 days of this comment.