From c3a68ecb8caf3c86a6332d07541ca24ddf1cb646 Mon Sep 17 00:00:00 2001 From: Noah Gilson Date: Tue, 13 Dec 2022 13:45:41 -0800 Subject: [PATCH] Respond to PR feedback on RID Inference for Publish Only --- ...oft.NET.RuntimeIdentifierInference.targets | 16 +++++++----- ...GivenThatWeWantToBuildASelfContainedApp.cs | 13 +++++----- .../GlobalPropertyFlowTests.cs | 26 ++++++++++--------- .../TestAssetsManager.cs | 1 + 4 files changed, 31 insertions(+), 25 deletions(-) diff --git a/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.RuntimeIdentifierInference.targets b/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.RuntimeIdentifierInference.targets index 9acdab5b0041..469c238029e8 100644 --- a/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.RuntimeIdentifierInference.targets +++ b/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.RuntimeIdentifierInference.targets @@ -80,12 +80,14 @@ Copyright (c) .NET Foundation. All rights reserved. '$(RuntimeIdentifier)' == '' and '$(_IsExecutable)' == 'true' and '$(IsTestProject)' != 'true' and '$(IsRidAgnostic)' != 'true' and - ('$(_IsPublishing)' == 'true' or '$(SelfContained)' == 'true') and - ( - '$(SelfContained)' == 'true' or - '$(PublishReadyToRun)' == 'true' or - '$(PublishSingleFile)' == 'true' or - '$(PublishAot)' == 'true' + ( + '$(SelfContained)' == 'true' or + ('$(_IsPublishing)' == 'true' and + ( + '$(PublishReadyToRun)' == 'true' or + '$(PublishSingleFile)' == 'true' or + '$(PublishAot)' == 'true' + ) )">true @@ -184,7 +186,7 @@ Copyright (c) .NET Foundation. All rights reserved. Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp' and '$(HasRuntimeOutput)' == 'true'"> - diff --git a/src/Tests/Microsoft.NET.Build.Tests/GivenThatWeWantToBuildASelfContainedApp.cs b/src/Tests/Microsoft.NET.Build.Tests/GivenThatWeWantToBuildASelfContainedApp.cs index 8ca7b919d710..426ce36dcc6a 100644 --- a/src/Tests/Microsoft.NET.Build.Tests/GivenThatWeWantToBuildASelfContainedApp.cs +++ b/src/Tests/Microsoft.NET.Build.Tests/GivenThatWeWantToBuildASelfContainedApp.cs @@ -405,10 +405,10 @@ public void It_does_not_build_SelfContained_due_to_PublishSelfContained_being_tr } [Theory] - [InlineData("--p:PublishReadyToRun=true")] - [InlineData("-p:PublishSingleFile=true")] - [InlineData("-p:PublishSelfContained=true")] - [InlineData("-p:PublishAot=true")] + [InlineData("PublishReadyToRun")] + [InlineData("PublishSingleFile")] + [InlineData("PublishSelfContained")] + [InlineData("PublishAot")] public void It_builds_without_implicit_rid_with_RuntimeIdentifier_specific_during_publish_only_properties(string property) { var tfm = ToolsetInfo.CurrentTargetFramework; @@ -417,12 +417,13 @@ public void It_builds_without_implicit_rid_with_RuntimeIdentifier_specific_durin IsExe = true, TargetFrameworks = tfm, }; + testProject.AdditionalProperties[property] = "true"; testProject.RecordProperties("RuntimeIdentifier"); - var testAsset = _testAssetsManager.CreateTestProject(testProject, identifier: $"ItBuildsWithoutImplicitRIDOnRIDRequiringPropertiesDuringPublish_{property}"); + var testAsset = _testAssetsManager.CreateTestProject(testProject, identifier: property); var buildCommand = new DotnetBuildCommand(testAsset); buildCommand - .Execute(property) + .Execute() .Should() .Pass(); diff --git a/src/Tests/Microsoft.NET.Build.Tests/GlobalPropertyFlowTests.cs b/src/Tests/Microsoft.NET.Build.Tests/GlobalPropertyFlowTests.cs index 612044e3e83a..8a960e2f52e4 100644 --- a/src/Tests/Microsoft.NET.Build.Tests/GlobalPropertyFlowTests.cs +++ b/src/Tests/Microsoft.NET.Build.Tests/GlobalPropertyFlowTests.cs @@ -86,7 +86,7 @@ public void TestGlobalPropertyFlowToLibrary(bool passSelfContained, bool passRun bool buildingSelfContained = passSelfContained || passRuntimeIdentifier; - ValidateProperties(testAsset, _testProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: passRuntimeIdentifier); + ValidateProperties(testAsset, _testProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: buildingSelfContained); ValidateProperties(testAsset, _referencedProject, expectSelfContained: false, expectRuntimeIdentifier: false); } @@ -103,8 +103,8 @@ public void TestGlobalPropertyFlowToExe(bool passSelfContained, bool passRuntime bool buildingSelfContained = passSelfContained || passRuntimeIdentifier; - ValidateProperties(testAsset, _testProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: passRuntimeIdentifier); - ValidateProperties(testAsset, _referencedProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: passRuntimeIdentifier); + ValidateProperties(testAsset, _testProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: buildingSelfContained); + ValidateProperties(testAsset, _referencedProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: buildingSelfContained); } @@ -139,9 +139,9 @@ public void TestGlobalPropertyFlowToExeWithSelfContainedFalse(bool passSelfConta bool buildingSelfContained = passSelfContained || passRuntimeIdentifier; - ValidateProperties(testAsset, _testProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: passRuntimeIdentifier); + ValidateProperties(testAsset, _testProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: buildingSelfContained); // SelfContained will only flow to referenced project if it's explicitly passed in this case - ValidateProperties(testAsset, _referencedProject, expectSelfContained: passSelfContained, expectRuntimeIdentifier: passRuntimeIdentifier); + ValidateProperties(testAsset, _referencedProject, expectSelfContained: passSelfContained, expectRuntimeIdentifier: buildingSelfContained); } } @@ -159,9 +159,11 @@ public void TestGlobalPropertyFlowToLibraryWithRuntimeIdentifier(bool passSelfCo bool buildingSelfContained = passSelfContained || passRuntimeIdentifier; - ValidateProperties(testAsset, _testProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: passRuntimeIdentifier); - ValidateProperties(testAsset, _referencedProject, expectSelfContained: passSelfContained, expectRuntimeIdentifier: passRuntimeIdentifier, - expectedRuntimeIdentifier: passRuntimeIdentifier ? "" : _referencedProject.RuntimeIdentifier); + ValidateProperties(testAsset, _testProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: buildingSelfContained); + ValidateProperties(testAsset, _referencedProject, expectSelfContained: passSelfContained, expectRuntimeIdentifier: buildingSelfContained, + // Right now passing "--self-contained" also causes the RuntimeIdentifier to be passed as a global property. + // That should change with https://github.com/dotnet/sdk/pull/26143, which will likely require updating this and other tests in this class + expectedRuntimeIdentifier: buildingSelfContained ? "" : _referencedProject.RuntimeIdentifier); } [RequiresMSBuildVersionTheory("17.4.0.41702")] @@ -184,13 +186,13 @@ public void TestGlobalPropertyFlowToMultitargetedProject(bool passSelfContained, bool buildingSelfContained = passSelfContained || passRuntimeIdentifier; - ValidateProperties(testAsset, _testProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: passRuntimeIdentifier, + ValidateProperties(testAsset, _testProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: buildingSelfContained, targetFramework: "net6.0"); - ValidateProperties(testAsset, _testProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: passRuntimeIdentifier, + ValidateProperties(testAsset, _testProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: buildingSelfContained, targetFramework: "net7.0"); ValidateProperties(testAsset, _referencedProject, expectSelfContained: false, expectRuntimeIdentifier: false, targetFramework: "net6.0"); - ValidateProperties(testAsset, _referencedProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: passRuntimeIdentifier, + ValidateProperties(testAsset, _referencedProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: buildingSelfContained, targetFramework: "net7.0"); } @@ -251,7 +253,7 @@ private static void ValidateProperties(TestAsset testAsset, TestProject testProj targetFramework = targetFramework ?? testProject.TargetFrameworks; - if (string.IsNullOrEmpty(expectedRuntimeIdentifier) && expectRuntimeIdentifier) + if (string.IsNullOrEmpty(expectedRuntimeIdentifier) && (expectSelfContained || expectRuntimeIdentifier)) { // RuntimeIdentifier might be inferred, so look at the output path to figure out what the actual value used was string dir = (Path.Combine(testAsset.TestRoot, testProject.Name, "bin", "Debug", targetFramework)); diff --git a/src/Tests/Microsoft.NET.TestFramework/TestAssetsManager.cs b/src/Tests/Microsoft.NET.TestFramework/TestAssetsManager.cs index 81abb499c35f..e3f2de3311e2 100644 --- a/src/Tests/Microsoft.NET.TestFramework/TestAssetsManager.cs +++ b/src/Tests/Microsoft.NET.TestFramework/TestAssetsManager.cs @@ -59,6 +59,7 @@ public TestAsset CopyTestAsset( /// Defaults to the name of the caller function (presumably the test). /// Used to prevent file collisions on tests which share the same test project. /// Use this for theories. + /// Pass in the unique theory parameters that can indentify that theory from others. /// The Identifier is used to distinguish between theory child tests. Generally it should be created using a combination of all of the theory parameter values. /// This is distinct from the test project name and is used to prevent file collisions between theory tests that use the same test project. /// The extension type of the desired test project, e.g. .csproj, or .fsproj.