Skip to content
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

Attach to VS automatically #3197

Merged
merged 5 commits into from
Nov 29, 2021
Merged

Conversation

nohwnd
Copy link
Member

@nohwnd nohwnd commented Nov 26, 2021

Description

Add a tool to attach sub-processes to an instance of Visual Studio, so we can run vstest.console via wrapper and get it attached, or similarly run testhost, and get it automatically attached to VS.

This tool needs to be built as net472, because netcoreapp2.1 and netcoreapp3.1 don't support the used api. So the same executable is re-used for everything. This has the added benefit of not packing it into the nugets we create.

Adds VSTEST_RUNNER_DEBUG_ATTACHVS, VSTEST_HOST_DEBUG_ATTACHVS, VSTEST_DATACOLLECTOR_DEBUG_ATTACHVS environment variables, which take value 0, 1, and <number>.

0 disables attaching to VS and is the default, 1 enables it and a parent process of Visual Studio is looked up, or the first Visual Studio that started.

Any number larger than 1 is considered a PID of the process to be inspected, and same logic is used. The current process and all it's parents are checked, and if none of them is VS, then the first VS that started is used. You can use this to define exactly which VS should we attach to.

VSTEST_DEBUG_NOBP is also added to prevent stopping on breakpoints after attaching.

The AttachVS tool is located in 3 ways, by looking at VSTEST_DEBUG_ATTACHVS_PATH env variable, by searching PATH, and by walking up the directory tree, and looking for special path (this is to locate the tool when you start VS, without and additional env variables). Our build script also adds the correct path to PATH. This allows you to debug both stuff from artifacts, as well as arbitrary dotnet test run.

For general use outside of our repo, put AttachVS to your PATH, or specify VSTEST_DEBUG_ATTACHVS_PATH.

The tool can be observed using DebugView++, logs are written to Trace.

@nohwnd nohwnd changed the title Attach vs debugger on debug Attach to VS automatically Nov 26, 2021
@nohwnd
Copy link
Member Author

nohwnd commented Nov 26, 2021

Our build script also adds the correct path to PATH.

Actually no. Build.cmd is generally used, so the build will run in a sub-process, and won't affect the current session. And to set it to user context we need admin, and I don't want to do that. So just add it to your PATH I guess?

Or you can run the ./scripts/build.ps1 directly and that sets that environment correctly.

Copy link
Contributor

@MarcoRossignoli MarcoRossignoli left a comment

Choose a reason for hiding this comment

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

LL(ovely)TM

<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFrameworks>net472</TargetFrameworks>
<LangVersion>preview</LangVersion>
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: looks off

Copy link
Member Author

Choose a reason for hiding this comment

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

not sure why, but lately pressing format in .csproj totally wrecks the file...

src/datacollector/DataCollectorMain.cs Outdated Show resolved Hide resolved
@MarcoRossignoli
Copy link
Contributor

It's great, maybe follow-up with docs update

@nohwnd nohwnd merged commit 83de0ad into microsoft:main Nov 29, 2021
src/AttachVS/AttachVs.cs Show resolved Hide resolved
src/AttachVS/AttachVs.cs Show resolved Hide resolved
src/AttachVS/AttachVs.cs Show resolved Hide resolved
src/AttachVS/AttachVs.cs Show resolved Hide resolved

return isVs;
}
catch
Copy link
Member

Choose a reason for hiding this comment

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

Why would the above try block throw anything ? Looks like we're only doing a string comparison in there and some tracing and we made sure to check process is not null first.

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right. It's probably pointless now.

src/AttachVS/AttachVs.cs Show resolved Hide resolved
Elanis pushed a commit to Elanis/portfolio that referenced this pull request Dec 13, 2022
Bumps [Microsoft.NET.Test.Sdk](https://github.com/microsoft/vstest) from 17.0.0 to 17.1.0.
<details>
<summary>Release notes</summary>

*Sourced from [Microsoft.NET.Test.Sdk's releases](https://github.com/microsoft/vstest/releases).*

> ## v17.1.0
> See the release notes [here](https://github.com/microsoft/vstest-docs/blob/main/docs/releases.md#1710).
>
> ## v17.1.0-release-20220113-05
> See the release notes [here](https://github.com/microsoft/vstest-docs/blob/master/docs/releases.md#1710-release-20220113-05).
>
> ## v17.1.0-preview-20211130-02
> See the release notes [here](https://github.com/microsoft/vstest-docs/blob/master/docs/releases.md#1710-preview-20211130-02).
>
> ## v17.1.0-preview-20211109-03
> See the release notes [here](https://github.com/microsoft/vstest-docs/blob/master/docs/releases.md#1710-preview-20211109-03).
</details>
<details>
<summary>Commits</summary>

- [`fcc0e3a`](microsoft/vstest@fcc0e3a) Added support for TestAdapterLoadingStrategy. ([#3374](microsoft/vstest#3374))
- [`d3c6439`](microsoft/vstest@d3c6439) Fix architecture retrival ([#3251](microsoft/vstest#3251))
- [`4d9f0fb`](microsoft/vstest@4d9f0fb) [rel/17.1] Update dependencies from devdiv/DevDiv/vs-code-coverage ([#3221](microsoft/vstest#3221))
- [`d72bcfa`](microsoft/vstest@d72bcfa) [rel/17.1] Update dependencies from devdiv/DevDiv/vs-code-coverage ([#3216](microsoft/vstest#3216))
- [`c9489b4`](microsoft/vstest@c9489b4) External dependencies updated ([#3204](microsoft/vstest#3204))
- [`e90972e`](microsoft/vstest@e90972e) [main] Update dependencies from dotnet/arcade devdiv/DevDiv/vs-code-coverage ...
- [`9cf8a92`](microsoft/vstest@9cf8a92) AttachVS PR comments fixed ([#3201](microsoft/vstest#3201))
- [`8b41e37`](microsoft/vstest@8b41e37) Update dependencies from https://dev.azure.com/devdiv/DevDiv/_git/vs-code-cov...
- [`83de0ad`](microsoft/vstest@83de0ad) Attach to VS automatically ([#3197](microsoft/vstest#3197))
- [`d36b7d8`](microsoft/vstest@d36b7d8) Update dependencies from https://dev.azure.com/devdiv/DevDiv/_git/vs-code-cov...
- Additional commits viewable in [compare view](microsoft/vstest@v17.0.0...v17.1.0)
</details>

<br />

Reviewed-on: https://gitea.dysnomia.studio/elanis/portfolio/pulls/26
Co-authored-by: elanis <elanis@noreply.example.org>
Co-committed-by: elanis <elanis@noreply.example.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants