Skip to content

Conversation

@JanKrivanek
Copy link
Member

Fixes #8313

Context

Some of the unit tests are failing when run from VS - caused by the fact that those tests are supposed to execute msbuild and fail to do so due to attempts to interpret the test runner as the dotnet host process

Changes Made

Added helper to infer the proper host to run the msbuild in core
Logic adopted from SDK, with some alternations (removed unnecessary interop, added nullables, some styling fixes)

<Compile Include="..\Shared\UnitTests\MockEngine.cs" />
<Compile Include="..\Shared\UnitTests\MockLogger.cs" />
<Compile Include="..\Shared\UnitTests\ObjectModelHelpers.cs" />
<Compile Include="..\UnitTests.Shared\EnvironmentProvider.cs" />
Copy link
Member

Choose a reason for hiding this comment

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

nit: just see there is Shared\UnitTests and UnitTests.Shared: what is the difference here?

Copy link
Member

Choose a reason for hiding this comment

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

Nice catch!

For historical reasons, there is a bunch of code in MSBuild that is compiled into multiple assemblies (sometimes with slightly different ifdefs). That includes test assemblies. However, the reasons for doing this in non-test assemblies (they aren't great reasons but they are reasons) don't really apply for test, so we introduced a shared assembly for test stuff to build that sort of thing once--but didn't move everything.

Copy link
Member

Choose a reason for hiding this comment

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

Actually it looks like shortly after introducing that assembly, we dropped all references to it (in ~2017).

It's still a good idea.

Copy link
Member

@vlada-shubina vlada-shubina left a comment

Choose a reason for hiding this comment

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

Looks good

@JanKrivanek JanKrivanek added the merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now. label Jan 26, 2023
@JanKrivanek JanKrivanek merged commit bd63317 into dotnet:main Jan 31, 2023
@JanKrivanek JanKrivanek deleted the proto/test-runner-utils branch January 31, 2023 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unit test failures when run from VS

3 participants