Skip to content

Add test for PublishSingleFile+PublishAot #34948

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Add test for PublishSingleFile+PublishAot #34948

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
515b11e
Add test for PublishSingleFile+PublishAot
agocke Aug 25, 2023
60702b6
Fix test
agocke Aug 25, 2023
6df6431
Merge branch 'main' into test-singlefile-aot
agocke Aug 30, 2023
f5ec2c7
Merge remote-tracking branch 'upstream/main' into test-singlefile-aot
agocke Sep 25, 2023
cb05255
Use property name instead of flag
agocke Sep 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishAnAotApp.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,32 @@ public GivenThatWeWantToPublishAnAotApp(ITestOutputHelper log) : base(log)
{
}

[Fact]
public void Can_Publish_SingleFile_And_NativeAot()
{
var targetFramework = ToolsetInfo.CurrentTargetFramework;
const string projectName = "SingleFileAndNativeAot";
var project = CreateHelloWorldTestProject(targetFramework, projectName, true);
project.AdditionalProperties["PublishSingleFile"] = "true";
project.AdditionalProperties["PublishAot"] = "true";
var testAsset = _testAssetsManager.CreateTestProject(project);

var publishCommand = new PublishCommand(testAsset);
publishCommand
.Execute("-p:UseCurrentRuntimeIdentifier=true")
.Should().Pass();

var buildProperties = project.GetPropertyValues(testAsset.TestRoot, targetFramework);
var rid = buildProperties["NETCoreSdkPortableRuntimeIdentifier"];

var publishDirectory = publishCommand.GetOutputDirectory(targetFramework, runtimeIdentifier: rid).FullName;
var publishedExe = Path.Combine(publishDirectory, $"{project.Name}{Constants.ExeSuffix}");

var command = new RunExeCommand(Log, publishedExe)
.Execute().Should().Pass()
.And.HaveStdOutContaining("Hello World");
}

[RequiresMSBuildVersionTheory("17.8.0")]
[MemberData(nameof(Net7Plus), MemberType = typeof(PublishTestUtils))]
Copy link
Member

Choose a reason for hiding this comment

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

The new test should have this attribute as well - it can only run on .NET 7 +, right, or maybe even newer??

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

@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.

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

They probably work fine, I just don't particularly want to support desktop MSBuild if I don't have to. Reduces version swing.

public void NativeAot_hw_runs_with_no_warnings_when_PublishAot_is_enabled(string targetFramework)
Expand Down