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

User/adstep/uwp net9 support #4817

Closed
wants to merge 33 commits into from

Conversation

adstep
Copy link
Contributor

@adstep adstep commented Jan 29, 2025

This PR updates (#3848) with the latest change from main.

#3848 includes the initial work to support UWP apps using .NET 9 (see here).
This tackles the extensions assembly:

  • Multi-target net8.0-10.0.26100.0 (we can bump to .NET 9 once it goes GA)
  • Define WINDOWS_UWP on this target
  • Pack this .dll into net8.0-windows10.0.26100.0/uwp
  • Update .targets to reference it if UseUwpTools is set (ie. this is a UWP app using .NET 9)

Directory.Build.props Outdated Show resolved Hide resolved
Directory.Build.props Outdated Show resolved Hide resolved
eng/verify-nupkgs.ps1 Outdated Show resolved Hide resolved

<ItemGroup>
<ProjectCapability Include="TestContainer" />
<PackageReference Include="Microsoft.TestPlatform" Version="17.13.0-dev" />
Copy link
Member

Choose a reason for hiding this comment

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

Is there another PR that touches VSTest? If so, we will need to later update the sample with a version available on NuGet instead of a locally built version

Copy link
Contributor Author

@adstep adstep Jan 29, 2025

Choose a reason for hiding this comment

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

Yea I had some changes in vstest to remove subscribing to the process, which doesn't work in UWP. @Evangelink is working on a PR for that. I'll circle back and update these to the public version when that gets published. And also minimize what's needed.

Sergio originally had which I think should be sufficient (plus removing the dev dependency again):

    <PackageReference Include="Microsoft.Windows.SDK.CPP" Version="10.0.26100.2454" />

	<!-- Do not use MSTest metapackage as we don't want to use Microsoft.NET.Test.Sdk -->
	<PackageReference Include="MSTest.TestFramework" Version="3.7.0-dev" />
	<PackageReference Include="MSTest.TestAdapter" Version="3.7.0-dev" />
	<PackageReference Include="MSTest.Analyzers" Version="3.7.0-dev" />
	<!-- Instead we want to rely on Microsoft.TestPlatform.TestHost to support msix -->
	<PackageReference Include="Microsoft.TestPlatform.TestHost" Version="17.12.0" />

Copy link
Member

Choose a reason for hiding this comment

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

@adstep I don't recall if you did some changes to VSTest or not.

Copy link
Contributor Author

@adstep adstep Jan 29, 2025

Choose a reason for hiding this comment

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

@Evangelink I had to remove UnitTestClient from subscribing to Parent process' ExitCallback. ProcessHelper throws in the UWP setting because it executes in a sandbox that doesn't have permissions to access process info.

The old VSTest framework that works with legacy UWP just had ProcessHelper as a complete no-op for all the methods.

If you remove the ExitCallabck then it leaves the UnitTestClient open to hanging potentially - but it got the tests running for me. I wasn't sure if the old VSTest framework did something diifferent for legacy UWP.

Comment on lines 35 to 36
- UWP 10.0.16299
- UWP 10.0.17763 with .NET 9
Copy link
Member

Choose a reason for hiding this comment

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

nit: Indentation is different

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm struggling to fix this in my editor. If you can send a workspace commit I'll update it.

<file src="net9.0\Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.dll" target="build\net9.0\" />
<file src="net9.0-windows10.0.17763.0\Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.dll" target="build\net9.0\uwp" />
Copy link
Member

Choose a reason for hiding this comment

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

How are we differentiating net9.0 UWP vs net9.0 WinUI? They both use the same TFM right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe the plan was to use UseWinUI and UseUwpTools properties.

src/Adapter/MSTest.TestAdapter/MSTest.TestAdapter.csproj Outdated Show resolved Hide resolved
src/Adapter/MSTest.TestAdapter/MSTest.TestAdapter.csproj Outdated Show resolved Hide resolved
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
<None Update="build\netfx-netcore-netstandard\MSTest.TestAdapter.targets">
<None Update="build\common\MSTest.TestAdapter.targets">
Copy link
Member

Choose a reason for hiding this comment

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

Let me done some of these fixes in a separate PR.

src/Adapter/MSTest.TestAdapter/MSTest.TestAdapter.nuspec Outdated Show resolved Hide resolved
@Evangelink Evangelink self-assigned this Jan 29, 2025
Evangelink and others added 4 commits January 29, 2025 20:57
# Conflicts:
#	src/Adapter/MSTest.TestAdapter/MSTest.TestAdapter.csproj
@Evangelink
Copy link
Member

Closing this one in favor of the initial from @Sergio0694

@Evangelink Evangelink closed this Jan 30, 2025
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.

5 participants