Set EnableWindowsTargeting to true when updating the lock file #11037
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Are there any downsides to optimistically setting this property? This is the only documentation I can find on it: |
As far as I know, there isn't any. |
|
@na1307 The change looks good and the documentation makes sense. Could you add a unit test verifying the behavior? All CI (and production) runs in a Linux container, so you should get the appropriate behavior. |
I know roughly how the tests are written, but I don't know what to do because of the contentHash in the lock file. Should I just use Some.Package like other tests, or should I use an actual existing package? |
|
Why is there still no answer... @brettfo |
|
You can write a unit test that uses real NuGet packages and I'll fix it up to use fake local packages later. As for verification, you might need to update the test methods to allow for a custom verification action or regular expression match. If my description wasn't helpful (they often aren't :) ) and if you can point me to some example projects/files/packages I can help with writing the test. |
|
@brettfo I've added tests. I don't know if this will be enough. |
|
@na1307 Thank you for the tests. I don't have access to push the test changes, but you can save the following file as `tests.patch` contents:From 87cf71c28643d4980ca89bdce3b2825ded56d320 Mon Sep 17 00:00:00 2001
From: "Brett V. Forsgren" <[email protected]>
Date: Thu, 19 Dec 2024 11:25:23 -0700
Subject: [PATCH] use local packages for tests
---
.../Update/UpdateWorkerTestBase.cs | 18 +-
.../Update/UpdateWorkerTests.LockFile.cs | 280 ++++++------------
2 files changed, 113 insertions(+), 185 deletions(-)
diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTestBase.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTestBase.cs
index 3372eceb7..9785bfcc1 100644
--- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTestBase.cs
+++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTestBase.cs
@@ -47,7 +47,7 @@ public abstract class UpdateWorkerTestBase : TestBase
{
return useSolution
? TestUpdateForSolution(dependencyName, oldVersion, newVersion, projectFiles: [(projectFilePath, projectContents)], projectFilesExpected: [(projectFilePath, expectedProjectContents)], isTransitive, additionalFiles, additionalFilesExpected, packages, experimentsManager)
- : TestUpdateForProject(dependencyName, oldVersion, newVersion, projectFile: (projectFilePath, projectContents), expectedProjectContents, isTransitive, additionalFiles, additionalFilesExpected, packages, experimentsManager);
+ : TestUpdateForProject(dependencyName, oldVersion, newVersion, projectFile: (projectFilePath, projectContents), expectedProjectContents, isTransitive, additionalFiles, additionalFilesExpected, additionalChecks: null, packages: packages, experimentsManager: experimentsManager);
}
protected static Task TestUpdate(
@@ -65,7 +65,7 @@ public abstract class UpdateWorkerTestBase : TestBase
{
return useSolution
? TestUpdateForSolution(dependencyName, oldVersion, newVersion, projectFiles: [projectFile], projectFilesExpected: [(projectFile.Path, expectedProjectContents)], isTransitive, additionalFiles, additionalFilesExpected, packages, experimentsManager)
- : TestUpdateForProject(dependencyName, oldVersion, newVersion, projectFile, expectedProjectContents, isTransitive, additionalFiles, additionalFilesExpected, packages, experimentsManager);
+ : TestUpdateForProject(dependencyName, oldVersion, newVersion, projectFile, expectedProjectContents, isTransitive, additionalFiles, additionalFilesExpected, additionalChecks: null, packages: packages, experimentsManager: experimentsManager);
}
protected static Task TestNoChangeforProject(
@@ -101,6 +101,7 @@ public abstract class UpdateWorkerTestBase : TestBase
bool isTransitive = false,
TestFile[]? additionalFiles = null,
TestFile[]? additionalFilesExpected = null,
+ Action<string>? additionalChecks = null,
MockNuGetPackage[]? packages = null,
ExperimentsManager? experimentsManager = null,
string projectFilePath = "test-project.csproj",
@@ -114,6 +115,7 @@ public abstract class UpdateWorkerTestBase : TestBase
isTransitive,
additionalFiles,
additionalFilesExpected,
+ additionalChecks,
packages,
experimentsManager,
expectedResult);
@@ -127,6 +129,7 @@ public abstract class UpdateWorkerTestBase : TestBase
bool isTransitive = false,
TestFile[]? additionalFiles = null,
TestFile[]? additionalFilesExpected = null,
+ Action<string>? additionalChecks = null,
MockNuGetPackage[]? packages = null,
ExperimentsManager? experimentsManager = null,
ExpectedUpdateOperationResult? expectedResult = null)
@@ -156,6 +159,17 @@ public abstract class UpdateWorkerTestBase : TestBase
{
ValidateUpdateOperationResult(expectedResult, actualResult!);
}
+
+ if (additionalChecks is not null)
+ {
+ var sourcesDirectory = temporaryDirectory;
+ if (placeFilesInSrc)
+ {
+ sourcesDirectory = Path.Combine(temporaryDirectory, "src");
+ }
+
+ additionalChecks(sourcesDirectory);
+ }
});
var expectedResultFiles = additionalFilesExpected.Prepend((projectFilePath, expectedProjectContents)).ToArray();
diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.LockFile.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.LockFile.cs
index ca4f13a24..efa02e929 100644
--- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.LockFile.cs
+++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.LockFile.cs
@@ -9,8 +9,12 @@ public partial class UpdateWorkerTests
[Fact]
public async Task UpdateSingleDependency()
{
- // update Newtonsoft.Json from 13.0.1 to 13.0.3
- await TestUpdateForProject("Newtonsoft.Json", "13.0.1", "13.0.3",
+ await TestUpdateForProject("Some.Package", "1.0.0", "2.0.0",
+ packages:
+ [
+ MockNuGetPackage.CreateSimplePackage("Some.Package", "1.0.0", "net8.0"),
+ MockNuGetPackage.CreateSimplePackage("Some.Package", "2.0.0", "net8.0"),
+ ],
// initial
projectContents: $"""
<Project Sdk="Microsoft.NET.Sdk">
@@ -20,10 +24,14 @@ public partial class UpdateWorkerTests
</PropertyGroup>
<ItemGroup>
- <PackageReference Include="Newtonsoft.Json" Version="13.0.1" />
+ <PackageReference Include="Some.Package" Version="1.0.0" />
</ItemGroup>
</Project>
""",
+ additionalFiles:
+ [
+ ("packages.lock.json", "{}")
+ ],
// expected
expectedProjectContents: $"""
<Project Sdk="Microsoft.NET.Sdk">
@@ -33,54 +41,27 @@ public partial class UpdateWorkerTests
</PropertyGroup>
<ItemGroup>
- <PackageReference Include="Newtonsoft.Json" Version="13.0.3" />
+ <PackageReference Include="Some.Package" Version="2.0.0" />
</ItemGroup>
</Project>
""",
- additionalFiles:
- [
- ("packages.lock.json", """
- {
- "version": 1,
- "dependencies": {
- "net8.0": {
- "Newtonsoft.Json": {
- "type": "Direct",
- "requested": "[13.0.1, )",
- "resolved": "13.0.1",
- "contentHash": "ppPFpBcvxdsfUonNcvITKqLl3bqxWbDCZIzDWHzjpdAHRFfZe0Dw9HmA0+za13IdyrgJwpkDTDA9fHaxOrt20A=="
- }
- }
- }
- }
- """)
- ],
- additionalFilesExpected:
- [
- ("packages.lock.json", """
- {
- "version": 1,
- "dependencies": {
- "net8.0": {
- "Newtonsoft.Json": {
- "type": "Direct",
- "requested": "[13.0.3, )",
- "resolved": "13.0.3",
- "contentHash": "HrC5BXdl00IP9zeV+0Z848QWPAoCr9P3bDEZguI+gkLcBKAOxix/tLEAAHC+UvDNPv4a2d18lOReHMOagPa+zQ=="
- }
- }
- }
- }
- """)
- ]
+ additionalChecks: path =>
+ {
+ var lockContents = File.ReadAllText(Path.Combine(path, "packages.lock.json"));
+ Assert.Contains("\"resolved\": \"2.0.0\"", lockContents);
+ }
);
}
-
+
[Fact]
public async Task UpdateSingleDependency_CentralPackageManagement()
{
- // update Newtonsoft.Json from 13.0.1 to 13.0.3
- await TestUpdateForProject("Newtonsoft.Json", "13.0.1", "13.0.3",
+ await TestUpdateForProject("Some.Package", "1.0.0", "2.0.0",
+ packages:
+ [
+ MockNuGetPackage.CreateSimplePackage("Some.Package", "1.0.0", "net8.0"),
+ MockNuGetPackage.CreateSimplePackage("Some.Package", "2.0.0", "net8.0"),
+ ],
// initial
projectContents: $"""
<Project Sdk="Microsoft.NET.Sdk">
@@ -90,10 +71,25 @@ public partial class UpdateWorkerTests
</PropertyGroup>
<ItemGroup>
- <PackageReference Include="Newtonsoft.Json" />
+ <PackageReference Include="Some.Package" />
</ItemGroup>
</Project>
""",
+ additionalFiles:
+ [
+ ("packages.lock.json", "{}"),
+ ("Directory.Packages.props", """
+ <Project>
+ <PropertyGroup>
+ <ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
+ </PropertyGroup>
+
+ <ItemGroup>
+ <PackageVersion Include="Some.Package" Version="1.0.0" />
+ </ItemGroup>
+ </Project>
+ """)
+ ],
// expected
expectedProjectContents: $"""
<Project Sdk="Microsoft.NET.Sdk">
@@ -103,56 +99,12 @@ public partial class UpdateWorkerTests
</PropertyGroup>
<ItemGroup>
- <PackageReference Include="Newtonsoft.Json" />
+ <PackageReference Include="Some.Package" />
</ItemGroup>
</Project>
""",
- additionalFiles:
- [
- ("packages.lock.json", """
- {
- "version": 2,
- "dependencies": {
- "net8.0": {
- "Newtonsoft.Json": {
- "type": "Direct",
- "requested": "[13.0.1, )",
- "resolved": "13.0.1",
- "contentHash": "ppPFpBcvxdsfUonNcvITKqLl3bqxWbDCZIzDWHzjpdAHRFfZe0Dw9HmA0+za13IdyrgJwpkDTDA9fHaxOrt20A=="
- }
- }
- }
- }
- """),
- ("Directory.Packages.props", """
- <Project>
- <PropertyGroup>
- <ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
- </PropertyGroup>
-
- <ItemGroup>
- <PackageVersion Include="Newtonsoft.Json" Version="13.0.1" />
- </ItemGroup>
- </Project>
- """)
- ],
additionalFilesExpected:
[
- ("packages.lock.json", """
- {
- "version": 2,
- "dependencies": {
- "net8.0": {
- "Newtonsoft.Json": {
- "type": "Direct",
- "requested": "[13.0.3, )",
- "resolved": "13.0.3",
- "contentHash": "HrC5BXdl00IP9zeV+0Z848QWPAoCr9P3bDEZguI+gkLcBKAOxix/tLEAAHC+UvDNPv4a2d18lOReHMOagPa+zQ=="
- }
- }
- }
- }
- """),
("Directory.Packages.props", """
<Project>
<PropertyGroup>
@@ -160,19 +112,28 @@ public partial class UpdateWorkerTests
</PropertyGroup>
<ItemGroup>
- <PackageVersion Include="Newtonsoft.Json" Version="13.0.3" />
+ <PackageVersion Include="Some.Package" Version="2.0.0" />
</ItemGroup>
</Project>
""")
- ]
+ ],
+ additionalChecks: path =>
+ {
+ var lockContents = File.ReadAllText(Path.Combine(path, "packages.lock.json"));
+ Assert.Contains("\"resolved\": \"2.0.0\"", lockContents);
+ }
);
}
-
+
[Fact]
public async Task UpdateSingleDependency_WindowsSpecific()
{
- // update Newtonsoft.Json from 13.0.1 to 13.0.3
- await TestUpdateForProject("Newtonsoft.Json", "13.0.1", "13.0.3",
+ await TestUpdateForProject("Some.Package", "1.0.0", "2.0.0",
+ packages:
+ [
+ MockNuGetPackage.CreateSimplePackage("Some.Package", "1.0.0", "net8.0"),
+ MockNuGetPackage.CreateSimplePackage("Some.Package", "2.0.0", "net8.0"),
+ ],
// initial
projectContents: $"""
<Project Sdk="Microsoft.NET.Sdk">
@@ -183,10 +144,14 @@ public partial class UpdateWorkerTests
</PropertyGroup>
<ItemGroup>
- <PackageReference Include="Newtonsoft.Json" Version="13.0.1" />
+ <PackageReference Include="Some.Package" Version="1.0.0" />
</ItemGroup>
</Project>
""",
+ additionalFiles:
+ [
+ ("packages.lock.json", "{}")
+ ],
// expected
expectedProjectContents: $"""
<Project Sdk="Microsoft.NET.Sdk">
@@ -197,54 +162,27 @@ public partial class UpdateWorkerTests
</PropertyGroup>
<ItemGroup>
- <PackageReference Include="Newtonsoft.Json" Version="13.0.3" />
+ <PackageReference Include="Some.Package" Version="2.0.0" />
</ItemGroup>
</Project>
""",
- additionalFiles:
- [
- ("packages.lock.json", """
- {
- "version": 1,
- "dependencies": {
- "net8.0-windows7.0": {
- "Newtonsoft.Json": {
- "type": "Direct",
- "requested": "[13.0.1, )",
- "resolved": "13.0.1",
- "contentHash": "ppPFpBcvxdsfUonNcvITKqLl3bqxWbDCZIzDWHzjpdAHRFfZe0Dw9HmA0+za13IdyrgJwpkDTDA9fHaxOrt20A=="
- }
- }
- }
- }
- """)
- ],
- additionalFilesExpected:
- [
- ("packages.lock.json", """
- {
- "version": 1,
- "dependencies": {
- "net8.0-windows7.0": {
- "Newtonsoft.Json": {
- "type": "Direct",
- "requested": "[13.0.3, )",
- "resolved": "13.0.3",
- "contentHash": "HrC5BXdl00IP9zeV+0Z848QWPAoCr9P3bDEZguI+gkLcBKAOxix/tLEAAHC+UvDNPv4a2d18lOReHMOagPa+zQ=="
- }
- }
- }
- }
- """)
- ]
+ additionalChecks: path =>
+ {
+ var lockContents = File.ReadAllText(Path.Combine(path, "packages.lock.json"));
+ Assert.Contains("\"resolved\": \"2.0.0\"", lockContents);
+ }
);
}
-
+
[Fact]
public async Task UpdateSingleDependency_CentralPackageManagement_WindowsSpecific()
{
- // update Newtonsoft.Json from 13.0.1 to 13.0.3
- await TestUpdateForProject("Newtonsoft.Json", "13.0.1", "13.0.3",
+ await TestUpdateForProject("Some.Package", "1.0.0", "2.0.0",
+ packages:
+ [
+ MockNuGetPackage.CreateSimplePackage("Some.Package", "1.0.0", "net8.0"),
+ MockNuGetPackage.CreateSimplePackage("Some.Package", "2.0.0", "net8.0"),
+ ],
// initial
projectContents: $"""
<Project Sdk="Microsoft.NET.Sdk">
@@ -255,10 +193,25 @@ public partial class UpdateWorkerTests
</PropertyGroup>
<ItemGroup>
- <PackageReference Include="Newtonsoft.Json" />
+ <PackageReference Include="Some.Package" />
</ItemGroup>
</Project>
""",
+ additionalFiles:
+ [
+ ("packages.lock.json", "{}"),
+ ("Directory.Packages.props", """
+ <Project>
+ <PropertyGroup>
+ <ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
+ </PropertyGroup>
+
+ <ItemGroup>
+ <PackageVersion Include="Some.Package" Version="1.0.0" />
+ </ItemGroup>
+ </Project>
+ """)
+ ],
// expected
expectedProjectContents: $"""
<Project Sdk="Microsoft.NET.Sdk">
@@ -269,56 +222,12 @@ public partial class UpdateWorkerTests
</PropertyGroup>
<ItemGroup>
- <PackageReference Include="Newtonsoft.Json" />
+ <PackageReference Include="Some.Package" />
</ItemGroup>
</Project>
""",
- additionalFiles:
- [
- ("packages.lock.json", """
- {
- "version": 2,
- "dependencies": {
- "net8.0-windows7.0": {
- "Newtonsoft.Json": {
- "type": "Direct",
- "requested": "[13.0.1, )",
- "resolved": "13.0.1",
- "contentHash": "ppPFpBcvxdsfUonNcvITKqLl3bqxWbDCZIzDWHzjpdAHRFfZe0Dw9HmA0+za13IdyrgJwpkDTDA9fHaxOrt20A=="
- }
- }
- }
- }
- """),
- ("Directory.Packages.props", """
- <Project>
- <PropertyGroup>
- <ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
- </PropertyGroup>
-
- <ItemGroup>
- <PackageVersion Include="Newtonsoft.Json" Version="13.0.1" />
- </ItemGroup>
- </Project>
- """)
- ],
additionalFilesExpected:
[
- ("packages.lock.json", """
- {
- "version": 2,
- "dependencies": {
- "net8.0-windows7.0": {
- "Newtonsoft.Json": {
- "type": "Direct",
- "requested": "[13.0.3, )",
- "resolved": "13.0.3",
- "contentHash": "HrC5BXdl00IP9zeV+0Z848QWPAoCr9P3bDEZguI+gkLcBKAOxix/tLEAAHC+UvDNPv4a2d18lOReHMOagPa+zQ=="
- }
- }
- }
- }
- """),
("Directory.Packages.props", """
<Project>
<PropertyGroup>
@@ -326,11 +235,16 @@ public partial class UpdateWorkerTests
</PropertyGroup>
<ItemGroup>
- <PackageVersion Include="Newtonsoft.Json" Version="13.0.3" />
+ <PackageVersion Include="Some.Package" Version="2.0.0" />
</ItemGroup>
</Project>
""")
- ]
+ ],
+ additionalChecks: path =>
+ {
+ var lockContents = File.ReadAllText(Path.Combine(path, "packages.lock.json"));
+ Assert.Contains("\"resolved\": \"2.0.0\"", lockContents);
+ }
);
}
}
--
2.47.1.windows.1
|
Co-authored-by: Brett V. Forsgren <[email protected]>
|
@brettfo I applied the patch. Thank you very much for your help. |
brettfo
approved these changes
Dec 20, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What are you trying to accomplish?
I'm trying to prevent the lock file update from failing when updating NuGet packages for Windows specific projects.
Fix #11036
Anything you want to highlight for special attention from reviewers?
No
How will you know you've accomplished your goal?
Added
-p:EnableWindowsTargeting=trueto dotnet restore command. This prevents dotnet cli from failing when restoring Windows specific projects.Checklist