Skip to content

Remove the event handler that was causing breakpoint changes to be erroneously replicated to the host runspace debugger#10503

Merged
2 commits merged intoPowerShell:masterfrom
KirkMunro:issue-10167-fix
Sep 10, 2019
Merged

Remove the event handler that was causing breakpoint changes to be erroneously replicated to the host runspace debugger#10503
2 commits merged intoPowerShell:masterfrom
KirkMunro:issue-10167-fix

Conversation

@KirkMunro
Copy link
Copy Markdown
Contributor

PR Summary

Fixes #10167.

Prior to this fix, if you created, modified, or removed a breakpoint in a nested debugger or inside of a job debugger, whatever change you made would be applied to the breakpoint with the corresponding breakpoint ID in the host runspace debugger. This fix removes the event handler that was causing breakpoint changes to be erroneously replicated to the host runspace debugger.

PR Context

See discussion with @PaulHigin in #10167.

PR Checklist

@KirkMunro KirkMunro requested a review from PaulHigin as a code owner September 9, 2019 17:25
Copy link
Copy Markdown
Contributor

@PaulHigin PaulHigin left a comment

Choose a reason for hiding this comment

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

LGTM

@iSazonov iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Sep 10, 2019
@iSazonov iSazonov added this to the 7.0.0-preview.4 milestone Sep 10, 2019
@TravisEz13 TravisEz13 changed the title Fix #10167 by removing HandleBreakpointUpdated event handler that should not have been there in the first place Remove the event handler that was causing breakpoint changes to be erroneously replicated to the host runspace debugger Sep 10, 2019
# Ensure that breakpoints were not created in the default runspace.
# Prior to this issue being fixed, breakpoints with the same id
# would be created or updated in the default runspace.
$host.Runspace.Debugger.GetBreakpoints() | Should -BeNullOrEmpty
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

test cases should be independent.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

But this is not blocking.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@TravisEz13 A question about this for my own knowledge: by independent do you mean that the specific test for that issue should be in a different Describe or Context invocation? Or do you mean that the comments shouldn't be there, because they infer a dependency between that test and the issue it resolves?

@TravisEz13 TravisEz13 added the AutoMerge informs the bot to automerge the PR label Sep 10, 2019
@ghost
Copy link
Copy Markdown

ghost commented Sep 10, 2019

Hello @TravisEz13!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit 096a78f into PowerShell:master Sep 10, 2019
@KirkMunro KirkMunro deleted the issue-10167-fix branch September 10, 2019 23:24
TravisEz13 pushed a commit to TravisEz13/PowerShell that referenced this pull request Sep 14, 2019
…roneously replicated to the host runspace debugger (PowerShell#10503)

* fix PowerShell#10167

* Update test/powershell/SDK/Breakpoint.Tests.ps1

Co-Authored-By: Ilya <[email protected]>
@ghost
Copy link
Copy Markdown

ghost commented Sep 19, 2019

🎉v7.0.0-preview.4 has been released which incorporates this pull request.:tada:

Handy links:

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AutoMerge informs the bot to automerge the PR CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Updating breakpoints in a nested debugger or in a job debugger results in breakpoints with the same id in the parent being overwritten

4 participants