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

dotnet test on .NET Framework test suite requires runtime identifier? #4469

Closed
thomhurst opened this issue Dec 29, 2024 · 29 comments
Closed
Assignees
Labels
Discussion External: Other This is caused by external issue, the external issue needs to be solved first.

Comments

@thomhurst
Copy link
Contributor

Is this anything to do with testing platform?

I've just added .NET Standard support to TUnit, and if I do a normal dotnet test it doesn't run unless I pass in a runtime identifier too.

Original issue is here: thomhurst/TUnit#1465

dotnet test:

PS C:\Users\Tom\Downloads\repro1465\repro1465> dotnet test
Restore complete (1.7s)
  repro1465 failed with 1 error(s) (0.4s)
    C:\Program Files\dotnet\sdk\9.0.101\Sdks\Microsoft.NET.Sdk\targets\Microsoft.PackageDependencyResolution.targets(266,5): error NETSDK1047: Assets file 'C:\Users\Tom\Downloads\repro1465\repro1465\obj\project.assets.json' doesn't have a target for 'net472/win-x86'. Ensure that restore has run and that you have included 'net472' in the TargetFrameworks for your project. You may also need to include 'win-x86' in your project's RuntimeIdentifiers.

dotnet test -r win-x86:

PS C:\Users\Tom\Downloads\repro1465\repro1465> dotnet test -r win-x86
Restore complete (0.8s)
  repro1465 succeeded (7.2s) → bin\Debug\net472\win-x86\repro1465.exe
  repro1465 test succeeded (1.4s)

Test summary: total: 1, failed: 0, succeeded: 1, skipped: 0, duration: 1.3s
@Evangelink
Copy link
Member

Looking at that issue now

@Evangelink
Copy link
Member

Evangelink commented Jan 7, 2025

@thomhurst could you please confirm the following behaviors:

  1. From @AArnott sample, doing dotnet build fails with the same issue
  2. Building from VS and then running dotnet test --no-build results in green run of tests

It would seems to indicate this is a msbuild/SDK issue on building (which we call when doing dotnet test).

@baronfel @JanKrivanek @YuliiaKovalova is it something you are aware of?

@baronfel
Copy link
Member

baronfel commented Jan 7, 2025

Got a binlog of what happens when you call MSBuild @Evangelink?

@Evangelink
Copy link
Member

Here you go: msbuild.binlog.txt

@thomhurst
Copy link
Contributor Author

You're right. It happens on the build. dotnet build causes it too:

PS C:\Users\Tom\Downloads\repro1465\repro1465> dotnet build
Restore complete (0.9s)
  repro1465 failed with 1 error(s) (0.4s)
    C:\Program Files\dotnet\sdk\9.0.101\Sdks\Microsoft.NET.Sdk\targets\Microsoft.PackageDependencyResolution.targets(266,5): error NETSDK1047: Assets file 'C:\Users\Tom\Downloads\repro1465\repro1465\obj\project.assets.json' doesn't have a target for 'net472/win-x86'. Ensure that restore has run and that you have included 'net472' in the TargetFrameworks for your project. You may also need to include 'win-x86' in your project's RuntimeIdentifiers.

Build failed with 1 error(s) in 6.4s

I managed to build in VS and then run on the CLI with --no-build like you suggested and that works fine:

PS C:\Users\Tom\Downloads\repro1465\repro1465> dotnet test --no-build
  repro1465 test succeeded (2.4s)

Test summary: total: 1, failed: 0, succeeded: 1, skipped: 0, duration: 1.7s
Build succeeded in 3.0s

@thomhurst
Copy link
Contributor Author

I'm not well versed in the SDK and how msbuild does stuff, but is the problem that the ResolvePackageAssets task is using a win-x86 runtime identifier parameter?

Image

@JanKrivanek
Copy link
Member

Unfortunately it's not easily trackable from the binlog where that inital value comes from

@YuliiaKovalova - this is another example where dotnet/msbuild#11106 will help a ton!

@JanKrivanek
Copy link
Member

But it seems to go from here:

https://github.com/dotnet/sdk/blob/094238ce2d4f69fbfdf0fdf862ef714b3dd027ea/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.RuntimeIdentifierInference.targets#L63

While not being used during Restore.

@marcpopMSFT - could you suggest who should look at this?

@marcpopMSFT
Copy link

This is specific to TUNIT correct? From the binlog, it appears that HasRuntimeOutput is true for the build but not for the restore which is why a RID is getting set for the build but not project assets json gets created during the restore. This is happening because OutputType is ending up as an exe in the build but not during the restore.

The restore downloads the tunit package which ends up setting the OutputType to exe after the restore phase in TUNIT.Engine.props.

@rainersigwald @dsplaisted any suggestions how tunit can work around this? Do we need a property they can disable the implicit RID inference for netfx apps? They could set HasRuntimeOutput to false but I don't know what other side effects that'll have.

@Evangelink
Copy link
Member

Evangelink commented Jan 9, 2025

@thomhurst I confirm that setting <OutputType>Exe</OutputType> directly in the project fixes the issue. So it's neither a dotnet test nor a MTP issue which is good.

@Evangelink Evangelink added Discussion External: Other This is caused by external issue, the external issue needs to be solved first. and removed Needs: Triage 🔍 labels Jan 9, 2025
@thomhurst
Copy link
Contributor Author

thomhurst commented Jan 9, 2025

@thomhurst I confirm that setting <OutputType>Exe</OutputType> directly in the project fixes the issue. So it's neither a dotnet test nor a MTP issue which is good.

Ah that's good!
I've got a buildTransitive props file in my NuGet package that should be setting that though: https://nuget.info/packages/TUnit.Engine/0.6.52

But it looks like it's in an empty nested folder...

Not sure why as it should be pulling through the TargetFramework

<PackagePath>buildTransitive/$(TargetFramework)/</PackagePath>

@thomhurst
Copy link
Contributor Author

Anyway - This is a me problem. Sorry for the red herring. Thanks for the help everyone!

@baronfel
Copy link
Member

baronfel commented Jan 9, 2025

@thomhurst just confirming - that structure would 100% interfere with the auto-loading behaviors of NuGet. Check for double-slashes in the PackagePath for the items you're bundling?

@rainersigwald
Copy link
Member

The restore downloads the tunit package which ends up setting the OutputType to exe after the restore phase in TUNIT.Engine.props.

This is the core of it--I think the other test packages require that the project itself set OutputType. If it's set in the import, it will be set for "real" post-restore builds, but not the restore itself.

@dsplaisted
Copy link
Member

Yes, props and targets files in packages should not be setting any properties which would modify the input to Restore, which includes the OutputType.

@Evangelink
Copy link
Member

The restore downloads the tunit package which ends up setting the OutputType to exe after the restore phase in TUNIT.Engine.props.

This is the core of it--I think the other test packages require that the project itself set OutputType. If it's set in the import, it will be set for "real" post-restore builds, but not the restore itself.

One reason why custom MSBuild SDKs are good. When using it, we can actually set the output type in the props and everything works fine while for a normal package we have this error.

@thomhurst
Copy link
Contributor Author

thomhurst commented Jan 9, 2025

Yes, props and targets files in packages should not be setting any properties which would modify the input to Restore, which includes the OutputType.

Weird. This has been in TUnit for ages and doesn't seem to affect .NET (Core) builds?

Only for .NET Framework has it not worked.

I'd really like to be able to abstract away all the properties from users to simplify things for them.

@AArnott
Copy link
Member

AArnott commented Jan 10, 2025

I'd really like to be able to abstract away all the properties from users to simplify things for them.

@thomhurst for what it's worth IMO some properties are better left in the project. I wouldn't typically expect a PackageReference to change something so fundamental for the project as its OutputType property.

@Evangelink
Copy link
Member

@AArnott I think @bradwilson is doing the same for xUnit. I do tend to think it's more expected for test because with the new platform each test app has to be an executable so it feels boilerplate. But again we saw similar problem in MSTest and that's why we went with output type set by user for normal nuget and automatically set when using MSTest.Sdk custom MSBuild SDK.

Fyi I'll be having some discussions next week for the possibility to have some generic test SDK linked to the new platform.

@rainersigwald
Copy link
Member

doing the same for xUnit. I do tend to think it's more expected for test because with the new platform each test app has to be an executable so it feels boilerplate.

I'd recommend that the NuGet package validate the exe requirement but not try to impose it (for this reason that it does other stuff).

@thomhurst
Copy link
Contributor Author

doing the same for xUnit. I do tend to think it's more expected for test because with the new platform each test app has to be an executable so it feels boilerplate.

I'd recommend that the NuGet package validate the exe requirement but not try to impose it (for this reason that it does other stuff).

How would I do this?

@rainersigwald
Copy link
Member

Something like (untested)

<Target Name="ValidateExeProject"
        BeforeTargets="BeforeBuild"
        Condition="'$(BuildingProject)' == 'true'">
    <Error Condition="'$(OutputType)' != 'Exe'"
           Text="TUnit test projects must have OutputType Exe as of version XXX" />
</Target>

@bradwilson
Copy link

I'm happy to do this for xUnit.net as well, but I have no idea what '$(BuildingProject)' == 'true' is. I can't find it documented anywhere, and when set as a condition my target never triggers.

My temptation is to use '$(TargetFramework)' != '' instead.

@thomhurst
Copy link
Contributor Author

thomhurst commented Jan 10, 2025

Echoing brad. Also does this go in the props/targets file too?

@bradwilson
Copy link

Also, Microsoft.NET.Test.Sdk unilaterally sets <OutputType>Exe</OutputType> for netcoreapp3.1, which actually doesn't work properly (presumably for the reasons mentioned here), but also still ends up bypassing the notification message.

This puts things in a weird position: you'll get a notification for .NET Framework projects, but not .NET projects, and also the .NET projects aren't actually building executables despite trying to hard-set <OutputType>Exe</OutputType>. Kind of the worst case situation.

@thomhurst
Copy link
Contributor Author

Im struggling with props files on net framework ATM.

Net core I put files in build transitive and it works great I'm trying just build for net framework and not getting the results I expect.

@bradwilson
Copy link

bradwilson commented Jan 10, 2025

Edit: I'm able to fix the lack of AppHost by injecting <UseAppHost>true</UseAppHost>. I'll also print out a message if this is set wrong (for example, overridden in the project file).

@bradwilson
Copy link

v3 1.0.2-pre.3 has the updates above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion External: Other This is caused by external issue, the external issue needs to be solved first.
Projects
None yet
Development

No branches or pull requests

9 participants