Skip to content

Add attachDotnetDebugger debug option #3903

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 16 commits into from
Apr 11, 2023
Merged

Add attachDotnetDebugger debug option #3903

merged 16 commits into from
Apr 11, 2023

Conversation

JustinGrote
Copy link
Collaborator

@JustinGrote JustinGrote commented Apr 1, 2022

Latest VSIX for dotnet debugger

PR Summary

Adds an option to attach the omnisharp C# debugger for binary module projects, enabling mixed debugging for Powershell Binary Modules.

Demo.mp4

The attach task runs as a child task to the PowerShell debugging session and is managed via its lifecycle.
image

Related Changes

As part of this PR I also refactored/rearranged the debug config resolution/validation/mutation steps, to occur at the proper stages of functions that were called. I added tests for what I changed.

WIP Checklist

  • Cleanup on Temporary PSIC stop
  • Multi-Root Workspace Validation
  • Generalize to allow specification of a config name

PR Checklist

Note: Tick the boxes below that apply to this pull request by putting an x between the square brackets.
Please mark anything not applicable to this PR NA.

  • PR has a meaningful title
  • Summarized changes
  • PR has tests
  • This PR is ready to merge and is not work in progress
    • If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready

@JustinGrote
Copy link
Collaborator Author

This PR is still active, and I likely will try to fix it before PSConfEU as it will dovetail well into our binary module presentation.

@JustinGrote
Copy link
Collaborator Author

@andschwa picking this back up again. Would you prefer me to add the functionality in PR increments since it requires an explicit debug option to activate for easier review, or do you want this to be a big ol' PR with all batteries included?

@andyleejordan
Copy link
Member

Uhh just how big do you think one big PR will be? Certainly less overhead to do it in one go.

@JustinGrote
Copy link
Collaborator Author

Uhh just how big do you think one big PR will be? Certainly less overhead to do it in one go.

Not super huge, see the WIP items. Maybe a couple hundred lines at most.

@JustinGrote
Copy link
Collaborator Author

JustinGrote commented Mar 30, 2023

@andschwa this PR now works great locally for me, but uh, any idea how to troubleshoot the CI problem here?
image

EDIT: It helps to rebase to the latest main :)

@JustinGrote JustinGrote marked this pull request as ready for review March 30, 2023 04:05
@JustinGrote JustinGrote requested a review from a team March 30, 2023 04:05
@JustinGrote JustinGrote marked this pull request as draft March 30, 2023 04:05
@JustinGrote JustinGrote marked this pull request as ready for review March 30, 2023 04:06
@JustinGrote
Copy link
Collaborator Author

JustinGrote commented Mar 30, 2023

@andschwa I may need some help with getting the tests up and running, I can't for the life of me get it to work either in codespaces or locally.

EDIT: Not sure what I did but I completely blew away all of node_modules, etc. and did an npm ci and things seem to be OK again.

@andyleejordan
Copy link
Member

@JustinGrote ping me when you want a review! I should be on Discord too.

@JustinGrote JustinGrote changed the title WIP: Add attachDotnetDebugger debug option Add attachDotnetDebugger debug option Mar 30, 2023
@JustinGrote
Copy link
Collaborator Author

@andschwa implementation code is good for review, we'll just need to dream up some tests.

@JustinGrote
Copy link
Collaborator Author

Here is an extension build for anyone wanting to play with it. You will need to rename .zip to .vsix and then use "Install Extension from VSIX" in the VSCode command palette.
powershell-2023.3.2-binaryModuleDebug.zip

@nohwnd
Copy link
Contributor

nohwnd commented Apr 3, 2023

Or specify the extension on commandline: code --install-extension (Resolve-Path "~\Downloads\powershell-2023.3.2-binaryModuleDebug.vsix")

@andyleejordan andyleejordan added the Issue-Enhancement A feature request (enhancement). label Apr 3, 2023
@JustinGrote
Copy link
Collaborator Author

Rebased to latest main

Copy link
Member

@andyleejordan andyleejordan left a comment

Choose a reason for hiding this comment

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

Looks really good, just some changes!

@JustinGrote
Copy link
Collaborator Author

@andschwa this is ready for review. End to End tests for the binary module debugging were added and it tests both that it starts correctly and that a C# breakpoint is hit in a hybrid module.

@JustinGrote JustinGrote disabled auto-merge April 10, 2023 16:20
Extract processId Fetch

Cleanup legacy bad rebase

Revert breakout config

Add dotnetDebuggerConfigName info

TODO the workspace folder support

Fix typo

Cleanup flow and breakout temporary console to dedicated functions

Add handlers to cleanup dotnet attach

Fix linting issues

Refactor one-time event disposal using variable scope demonry

Clarify the handlers

Remove Disposable (lint fix)

Add initial test

Lint fix

Fix description to use correct and more specific terminology.

Co-authored-by: Andy Jordan <[email protected]>

Apply suggestions from @andschwa review

Fixed E2E Debug Testing

Remove Only

Add initial Debug test scaffolding

Remove "only"

Move validation logic to resolve functions and implement first resolve test

Rearrange methods with public first and implementation details last, in order of implemented interfaces. Add more tests

Fixed positioning of resolvesubstitutedvariables, now the public methods happen in the order they are called

Populate more tests

Some more scaffoldng and a broken test

Add optional attach port to runTests and remove only *again*

Fix wrong argv for port

Add check for PSES Binaries

Adjust StubInterface to use Partial to preserve intellisense

More test fixes

Fix test names to be consistent with other tests

More specific error checks and implement structuredClone

Add structuredclone to dev dependencies

Revert accidental commit of new build tasks

Update package.json

Co-authored-by: Andy Jordan <[email protected]>

Enable ESModuleInterop and fix related issues

Updated structuredclone import to ES6 syntax

Add ESModuleInterop to tsconfig.json

More test adds and add newlines between Arrange Act Assert

More test scaffolding

ESModuleInterop fixes

Bump package-lock

More esModuleInterop fixes

Tests TESTS TESTS TESTS TESTS

Flaky Test Check

Move testing default folder to examples

Add binary module example and C# extension fetching

Add precompiled binarymodule so that it doesnt have to be built for testing

Fix pid for ProcessID

Add first binary module E2E test

Suppress first run messages in examples folder

Remove Only

Add createDebugAdapterDescriptor test

Remove Only

Move working dir back to test (was breaking settings tests) and add dummy csproj for C# activation

Move dummy to mocks folder

Reload window for CI

Fix assertion order in binary module test

Skip Mac on binary module script test for now

Fix bug where stdout is null if CLI install fails

Extract CSharp extension install to separate function for DAMP clarity

Add test if C# extension was found

Remove imports

More MacOS troubleshooting (7 minute cycles between tests is hell)

More troubleshooting lines

Add hack to change cwd to make resolveCliArgs happy

Found out the electron extensions run in a whole separate process on MacOS, handle that

Re-enable resolveCliArgs detection problem fix

Remove error reference

Try without window reload

Remove Async from InstallCSharpExtension

Remove more async

Wait for C# extension to load rather than just reloading the window

More binary module test scaffolding

End to End Tests Done! 🎉🎉🎉

Spoke too soon, find out why CI breakpoint test fails (probably because it might be headless)

Add undefined checks

Build test binary module locally to avoid absolute PDB path issues

MacOS why you gotta be such a PIA

Remove dummy csproj, maybe MacOS search was a flake

Try reload window on mac

Move MacOS C# install to before tests to avoid test duplications

Fix reload loop on mac

Remove reload from initial

Add some debugging to try to find macos issue

Move event registration to before startDebugging

Try increasing timeout for MacOS

Add WaitEvent helper function

Try MacOS Hot Install Again

Retry: RunCode test seems to be flaky

Change exec to spawn to make CodeQL happy

Revert "Change exec to spawn to make CodeQL happy"

This reverts commit d3aea33.
@JustinGrote
Copy link
Collaborator Author

JustinGrote commented Apr 11, 2023

Doing the "hot install" of the extension seems to be flaky and locking up the extension host on 2022 PS7 for some reason on occasion, I may just move the C# initialization to the beginning prior to starting the tests in the first place.
image

Copy link
Member

@andyleejordan andyleejordan left a comment

Choose a reason for hiding this comment

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

👏

@JustinGrote JustinGrote enabled auto-merge (squash) April 11, 2023 20:18
@JustinGrote JustinGrote merged commit c990d7f into main Apr 11, 2023
@JustinGrote JustinGrote deleted the binaryModuleDebug branch April 11, 2023 20:18
JustinGrote added a commit that referenced this pull request Apr 14, 2023
Fixes dotnetDebuggerConfigName-option in launch config when using new attachDotnetDebugger feature introduced in #3903. Always threw config not found-error when dotnetDebuggerConfigName was set.

Co-authored-by: Justin Grote <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants