Add test for PublishSingleFile+PublishAot #34948
Conversation
ghost
added
|
cc @dotnet/ilc-contrib |
| var rid = buildProperties["NETCoreSdkPortableRuntimeIdentifier"]; | ||
| var publishCommand = new PublishCommand(testAsset); | ||
| publishCommand | ||
| .Execute($"/p:RuntimeIdentifier={rid}", "/p:SelfContained=true") |
There was a problem hiding this comment.
Is /p:RuntimeIdentifier={rid} and /p:SelfContained=true still needed?
There was a problem hiding this comment.
That's a good question. I think the warning has been removed, so this might be fine. I'll try it and see.
| .Execute().Should().Pass() | ||
| .And.HaveStdOutContaining("Hello World"); | ||
| } | ||
|
|
||
| [RequiresMSBuildVersionTheory("17.8.0")] | ||
| [MemberData(nameof(Net7Plus), MemberType = typeof(PublishTestUtils))] |
There was a problem hiding this comment.
The new test should have this attribute as well - it can only run on .NET 7 +, right, or maybe even newer??
There was a problem hiding this comment.
Is there a situation where we run the unit tests on different versions? I don't really understand how we would expect that to work.
There was a problem hiding this comment.
Reason one - consistency - if the other test has it, then why is this one different?
Reason two - my understanding is that to maintain SDK back compat, we can run the unit tests against older TFMs to test for it - and thus tests which can only work on a given TFM need to be marked accordingly.
But I know next to nothing about this.
There was a problem hiding this comment.
@rainersigwald Could you shed some light on this? When should we be using RequiresMSBuildVersion? These tests are core-SDK only, so I don't think desktop MSBuild is even a consideration.
There was a problem hiding this comment.
AFAIK RequiresMSBuildVersion means "when building using msbuild.exe this uses features that are new to the named version". Since msbuild.exe is delivered to the test runners through their VS install, it's generally not up to date with the version embedded in the SDK--but Microsoft-internal folks generally are, if they're running VS Internal Preview. So that gets some coverage + lights up coverage automatically after the VS on the image updates.
In general, if the PR tests pass I don't think you need the attribute
These tests are core-SDK only, so I don't think desktop MSBuild is even a consideration.
Really? Based on looking at the test I would have expected it to work driven by either build engine.
There was a problem hiding this comment.
They probably work fine, I just don't particularly want to support desktop MSBuild if I don't have to. Reduces version swing.
No description provided.