From 4b0b35e5eee774b8946fbfdf9f902b3bb9ab25a5 Mon Sep 17 00:00:00 2001 From: "Brett V. Forsgren" Date: Wed, 30 Oct 2024 17:30:20 -0600 Subject: [PATCH] manage C#-only experiments with `ExperimentsManager` --- .../EntryPointTests.Update.cs | 11 + .../Commands/UpdateCommand.cs | 10 +- .../DependencySolverEnvironment.cs | 12 - .../Run/SerializationTests.cs | 121 ++++++++++ .../Update/UpdateWorkerTestBase.cs | 64 ++++-- .../Update/UpdateWorkerTests.DirsProj.cs | 6 +- .../UpdateWorkerTests.PackageReference.cs | 39 ++-- .../UpdateWorkerTests.PackagesConfig.cs | 2 +- .../NuGetUpdater.Core/ExperimentsManager.cs | 52 +++++ .../NuGetUpdater.Core/Run/RunWorker.cs | 7 +- .../Updater/PackageReferenceUpdater.cs | 11 +- .../Updater/UpdaterWorker.cs | 6 +- .../Utilities/MSBuildHelper.cs | 5 - nuget/lib/dependabot/nuget/file_updater.rb | 11 +- nuget/lib/dependabot/nuget/native_helpers.rb | 23 +- .../dependabot/nuget/file_updater_spec.rb | 54 +++++ .../dependabot/nuget/native_helpers_spec.rb | 214 ++++++++++-------- 17 files changed, 478 insertions(+), 170 deletions(-) delete mode 100644 nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/DependencySolverEnvironment.cs create mode 100644 nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/ExperimentsManager.cs diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Update.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Update.cs index e27dd2335ee..b11a681b87f 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Update.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Update.cs @@ -1,3 +1,4 @@ +using System.IO; using System.Text; using NuGetUpdater.Core; @@ -18,6 +19,8 @@ public async Task WithSolution() await Run(path => [ "update", + "--job-path", + Path.Combine(path, "job.json"), "--repo-root", path, "--solution-or-project", @@ -119,6 +122,8 @@ public async Task WithProject() await Run(path => [ "update", + "--job-path", + Path.Combine(path, "job.json"), "--repo-root", path, "--solution-or-project", @@ -197,6 +202,8 @@ public async Task WithDirsProjAndDirectoryBuildPropsThatIsOutOfDirectoryButStill await Run(path => [ "update", + "--job-path", + Path.Combine(path, "job.json"), "--repo-root", path, "--solution-or-project", @@ -325,6 +332,7 @@ public async Task UpdaterDoesNotUseRepoGlobalJsonForMSBuildTasks(string? working MockNuGetPackage.CreateSimplePackage("Some.Package", "13.0.1", "net8.0"), ]; await MockNuGetPackagesInDirectory(testPackages, tempDir.DirectoryPath); + await MockJobFileInDirectory(tempDir.DirectoryPath); var globalJsonPath = Path.Join(tempDir.DirectoryPath, "global.json"); var srcGlobalJsonPath = Path.Join(tempDir.DirectoryPath, "src", "global.json"); @@ -353,6 +361,8 @@ await File.WriteAllTextAsync(projectPath, """ IEnumerable executableArgs = [ executableName, "update", + "--job-path", + Path.Combine(tempDir.DirectoryPath, "job.json"), "--repo-root", tempDir.DirectoryPath, "--solution-or-project", @@ -402,6 +412,7 @@ private static async Task Run(Func getArgs, (string Path, stri try { + await MockJobFileInDirectory(path); await MockNuGetPackagesInDirectory(packages, path); var args = getArgs(path); diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/UpdateCommand.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/UpdateCommand.cs index cb70e01eb5f..0b79b6da45e 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/UpdateCommand.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/UpdateCommand.cs @@ -6,6 +6,7 @@ namespace NuGetUpdater.Cli.Commands; internal static class UpdateCommand { + internal static readonly Option JobPathOption = new("--job-path") { IsRequired = true }; internal static readonly Option RepoRootOption = new("--repo-root", () => new DirectoryInfo(Environment.CurrentDirectory)) { IsRequired = false }; internal static readonly Option SolutionOrProjectFileOption = new("--solution-or-project") { IsRequired = true }; internal static readonly Option DependencyNameOption = new("--dependency") { IsRequired = true }; @@ -18,6 +19,7 @@ internal static Command GetCommand(Action setExitCode) { Command command = new("update", "Applies the changes from an analysis report to update a dependency.") { + JobPathOption, RepoRootOption, SolutionOrProjectFileOption, DependencyNameOption, @@ -29,12 +31,14 @@ internal static Command GetCommand(Action setExitCode) command.TreatUnmatchedTokensAsErrors = true; - command.SetHandler(async (repoRoot, solutionOrProjectFile, dependencyName, newVersion, previousVersion, isTransitive, resultOutputPath) => + command.SetHandler(async (jobPath, repoRoot, solutionOrProjectFile, dependencyName, newVersion, previousVersion, isTransitive, resultOutputPath) => { - var worker = new UpdaterWorker(new ConsoleLogger()); + var logger = new ConsoleLogger(); + var experimentsManager = await ExperimentsManager.FromJobFileAsync(jobPath.FullName, logger); + var worker = new UpdaterWorker(experimentsManager, logger); await worker.RunAsync(repoRoot.FullName, solutionOrProjectFile.FullName, dependencyName, previousVersion, newVersion, isTransitive, resultOutputPath); setExitCode(0); - }, RepoRootOption, SolutionOrProjectFileOption, DependencyNameOption, NewVersionOption, PreviousVersionOption, IsTransitiveOption, ResultOutputPathOption); + }, JobPathOption, RepoRootOption, SolutionOrProjectFileOption, DependencyNameOption, NewVersionOption, PreviousVersionOption, IsTransitiveOption, ResultOutputPathOption); return command; } diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/DependencySolverEnvironment.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/DependencySolverEnvironment.cs deleted file mode 100644 index 7d47ff271ff..00000000000 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/DependencySolverEnvironment.cs +++ /dev/null @@ -1,12 +0,0 @@ -namespace NuGetUpdater.Core.Test; - -/// -/// Prepares the environment to use the new dependency solver. -/// -public class DependencySolverEnvironment : TemporaryEnvironment -{ - public DependencySolverEnvironment(bool useDependencySolver = true) - : base([("UseNewNugetPackageResolver", useDependencySolver ? "true" : "false")]) - { - } -} diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Run/SerializationTests.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Run/SerializationTests.cs index ff7f7dd88d5..063ff5460dd 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Run/SerializationTests.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Run/SerializationTests.cs @@ -59,6 +59,115 @@ public void DeserializeJob() Assert.Equal("specific-sdk", jobWrapper.Job.Source.Directory); } + [Fact] + public void DeserializeExperimentsManager() + { + var jobWrapper = RunWorker.Deserialize(""" + { + "job": { + "package-manager": "nuget", + "allowed-updates": [ + { + "update-type": "all" + } + ], + "source": { + "provider": "github", + "repo": "some-org/some-repo", + "directory": "some-dir" + }, + "experiments": { + "nuget_legacy_dependency_solver": true, + "unexpected_bool": true, + "unexpected_number": 42, + "unexpected_null": null, + "unexpected_string": "abc", + "unexpected_array": [1, "two", 3.0], + "unexpected_object": { + "a": 1, + "b": "two" + } + } + } + } + """); + var experimentsManager = ExperimentsManager.GetExperimentsManager(jobWrapper.Job.Experiments); + Assert.True(experimentsManager.UseLegacyDependencySolver); + } + + [Fact] + public void DeserializeExperimentsManager_EmptyExperiments() + { + var jobWrapper = RunWorker.Deserialize(""" + { + "job": { + "package-manager": "nuget", + "allowed-updates": [ + { + "update-type": "all" + } + ], + "source": { + "provider": "github", + "repo": "some-org/some-repo", + "directory": "some-dir" + }, + "experiments": { + } + } + } + """); + var experimentsManager = ExperimentsManager.GetExperimentsManager(jobWrapper.Job.Experiments); + Assert.False(experimentsManager.UseLegacyDependencySolver); + } + + [Fact] + public void DeserializeExperimentsManager_NoExperiments() + { + var jobWrapper = RunWorker.Deserialize(""" + { + "job": { + "package-manager": "nuget", + "allowed-updates": [ + { + "update-type": "all" + } + ], + "source": { + "provider": "github", + "repo": "some-org/some-repo", + "directory": "some-dir" + } + } + } + """); + var experimentsManager = ExperimentsManager.GetExperimentsManager(jobWrapper.Job.Experiments); + Assert.False(experimentsManager.UseLegacyDependencySolver); + } + + [Fact] + public async Task DeserializeExperimentsManager_UnsupportedJobFileShape_InfoIsReportedAndEmptyExperimentSetIsReturned() + { + // arrange + using var tempDir = new TemporaryDirectory(); + var jobFilePath = Path.Combine(tempDir.DirectoryPath, "job.json"); + var jobContent = """ + { + "this-is-not-a-job-and-parsing-will-fail-but-an-empty-experiment-set-should-sill-be-returned": { + } + } + """; + await File.WriteAllTextAsync(jobFilePath, jobContent); + var capturingTestLogger = new CapturingTestLogger(); + + // act - this is the entrypoint the update command uses to parse the job file + var experimentsManager = await ExperimentsManager.FromJobFileAsync(jobFilePath, capturingTestLogger); + + // assert + Assert.False(experimentsManager.UseLegacyDependencySolver); + Assert.Single(capturingTestLogger.Messages.Where(m => m.Contains("Error deserializing job file"))); + } + [Fact] public void SerializeError() { @@ -67,4 +176,16 @@ public void SerializeError() var expected = """{"data":{"error-type":"job_repo_not_found","error-details":{"message":"some message"}}}"""; Assert.Equal(expected, actual); } + + private class CapturingTestLogger : ILogger + { + private readonly List _messages = new(); + + public IReadOnlyList Messages => _messages; + + public void Log(string message) + { + _messages.Add(message); + } + } } 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 4a2e129e323..0f244fa0086 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTestBase.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTestBase.cs @@ -1,3 +1,7 @@ +using System.Text.Json; + +using NuGetUpdater.Core.Run; +using NuGetUpdater.Core.Run.ApiModel; using NuGetUpdater.Core.Test.Updater; using NuGetUpdater.Core.Updater; @@ -19,11 +23,12 @@ protected static Task TestNoChange( bool isTransitive = false, TestFile[]? additionalFiles = null, MockNuGetPackage[]? packages = null, + ExperimentsManager? experimentsManager = null, string projectFilePath = "test-project.csproj") { return useSolution - ? TestNoChangeforSolution(dependencyName, oldVersion, newVersion, projectFiles: [(projectFilePath, projectContents)], isTransitive, additionalFiles, packages) - : TestNoChangeforProject(dependencyName, oldVersion, newVersion, projectContents, isTransitive, additionalFiles, packages, projectFilePath); + ? TestNoChangeforSolution(dependencyName, oldVersion, newVersion, projectFiles: [(projectFilePath, projectContents)], isTransitive, additionalFiles, packages, experimentsManager) + : TestNoChangeforProject(dependencyName, oldVersion, newVersion, projectContents, isTransitive, additionalFiles, packages, experimentsManager, projectFilePath); } protected static Task TestUpdate( @@ -37,11 +42,12 @@ protected static Task TestUpdate( TestFile[]? additionalFiles = null, TestFile[]? additionalFilesExpected = null, MockNuGetPackage[]? packages = null, + ExperimentsManager? experimentsManager = null, string projectFilePath = "test-project.csproj") { return useSolution - ? TestUpdateForSolution(dependencyName, oldVersion, newVersion, projectFiles: [(projectFilePath, projectContents)], projectFilesExpected: [(projectFilePath, expectedProjectContents)], isTransitive, additionalFiles, additionalFilesExpected, packages) - : TestUpdateForProject(dependencyName, oldVersion, newVersion, projectFile: (projectFilePath, projectContents), expectedProjectContents, isTransitive, additionalFiles, additionalFilesExpected, packages); + ? 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); } protected static Task TestUpdate( @@ -54,11 +60,12 @@ protected static Task TestUpdate( bool isTransitive = false, TestFile[]? additionalFiles = null, TestFile[]? additionalFilesExpected = null, - MockNuGetPackage[]? packages = null) + MockNuGetPackage[]? packages = null, + ExperimentsManager? experimentsManager = null) { return useSolution - ? TestUpdateForSolution(dependencyName, oldVersion, newVersion, projectFiles: [projectFile], projectFilesExpected: [(projectFile.Path, expectedProjectContents)], isTransitive, additionalFiles, additionalFilesExpected, packages) - : TestUpdateForProject(dependencyName, oldVersion, newVersion, projectFile, expectedProjectContents, isTransitive, additionalFiles, additionalFilesExpected, packages); + ? 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); } protected static Task TestNoChangeforProject( @@ -69,6 +76,7 @@ protected static Task TestNoChangeforProject( bool isTransitive = false, TestFile[]? additionalFiles = null, MockNuGetPackage[]? packages = null, + ExperimentsManager? experimentsManager = null, string projectFilePath = "test-project.csproj") => TestUpdateForProject( dependencyName, @@ -79,7 +87,8 @@ protected static Task TestNoChangeforProject( isTransitive, additionalFiles, additionalFilesExpected: additionalFiles, - packages: packages); + packages: packages, + experimentsManager: experimentsManager); protected static Task TestUpdateForProject( string dependencyName, @@ -91,6 +100,7 @@ protected static Task TestUpdateForProject( TestFile[]? additionalFiles = null, TestFile[]? additionalFilesExpected = null, MockNuGetPackage[]? packages = null, + ExperimentsManager? experimentsManager = null, string projectFilePath = "test-project.csproj", ExpectedUpdateOperationResult? expectedResult = null) => TestUpdateForProject( @@ -103,6 +113,7 @@ protected static Task TestUpdateForProject( additionalFiles, additionalFilesExpected, packages, + experimentsManager, expectedResult); protected static async Task TestUpdateForProject( @@ -115,6 +126,7 @@ protected static async Task TestUpdateForProject( TestFile[]? additionalFiles = null, TestFile[]? additionalFilesExpected = null, MockNuGetPackage[]? packages = null, + ExperimentsManager? experimentsManager = null, ExpectedUpdateOperationResult? expectedResult = null) { additionalFiles ??= []; @@ -134,7 +146,8 @@ protected static async Task TestUpdateForProject( await MockNuGetPackagesInDirectory(packages, temporaryDirectory); // run update - var worker = new UpdaterWorker(new TestLogger()); + experimentsManager ??= new ExperimentsManager(); + var worker = new UpdaterWorker(experimentsManager, new TestLogger()); var projectPath = placeFilesInSrc ? $"src/{projectFilePath}" : projectFilePath; var actualResult = await worker.RunWithErrorHandlingAsync(temporaryDirectory, projectPath, dependencyName, oldVersion, newVersion, isTransitive); if (expectedResult is { }) @@ -172,7 +185,8 @@ protected static Task TestNoChangeforSolution( TestFile[] projectFiles, bool isTransitive = false, TestFile[]? additionalFiles = null, - MockNuGetPackage[]? packages = null) + MockNuGetPackage[]? packages = null, + ExperimentsManager? experimentsManager = null) => TestUpdateForSolution( dependencyName, oldVersion, @@ -182,7 +196,8 @@ protected static Task TestNoChangeforSolution( isTransitive, additionalFiles, additionalFilesExpected: additionalFiles, - packages: packages); + packages: packages, + experimentsManager: experimentsManager); protected static async Task TestUpdateForSolution( string dependencyName, @@ -193,7 +208,8 @@ protected static async Task TestUpdateForSolution( bool isTransitive = false, TestFile[]? additionalFiles = null, TestFile[]? additionalFilesExpected = null, - MockNuGetPackage[]? packages = null) + MockNuGetPackage[]? packages = null, + ExperimentsManager? experimentsManager = null) { additionalFiles ??= []; additionalFilesExpected ??= []; @@ -236,8 +252,9 @@ protected static async Task TestUpdateForSolution( { await MockNuGetPackagesInDirectory(packages, temporaryDirectory); + experimentsManager ??= new ExperimentsManager(); var slnPath = Path.Combine(temporaryDirectory, slnName); - var worker = new UpdaterWorker(new TestLogger()); + var worker = new UpdaterWorker(experimentsManager, new TestLogger()); await worker.RunAsync(temporaryDirectory, slnPath, dependencyName, oldVersion, newVersion, isTransitive); }); @@ -246,6 +263,27 @@ protected static async Task TestUpdateForSolution( AssertContainsFiles(expectedResult, actualResult); } + public static async Task MockJobFileInDirectory(string temporaryDirectory) + { + var jobFile = new JobFile() + { + Job = new() + { + AllowedUpdates = + [ + new() { UpdateType = "all" } + ], + Source = new() + { + Provider = "github", + Repo = "test/repo", + Directory = "/", + } + } + }; + await File.WriteAllTextAsync(Path.Join(temporaryDirectory, "job.json"), JsonSerializer.Serialize(jobFile, RunWorker.SerializerOptions)); + } + public static async Task MockNuGetPackagesInDirectory(MockNuGetPackage[]? packages, string temporaryDirectory) { if (packages is not null) diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.DirsProj.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.DirsProj.cs index 1a7909b82d8..82cd79478c7 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.DirsProj.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.DirsProj.cs @@ -350,7 +350,8 @@ static async Task TestUpdateForDirsProj( bool isTransitive = false, (string Path, string Content)[]? additionalFiles = null, (string Path, string Content)[]? additionalFilesExpected = null, - MockNuGetPackage[]? packages = null) + MockNuGetPackage[]? packages = null, + ExperimentsManager? experimentsManager = null) { additionalFiles ??= []; additionalFilesExpected ??= []; @@ -363,8 +364,9 @@ static async Task TestUpdateForDirsProj( { await MockNuGetPackagesInDirectory(packages, temporaryDirectory); + experimentsManager ??= new ExperimentsManager(); var projectPath = Path.Combine(temporaryDirectory, projectFileName); - var worker = new UpdaterWorker(new TestLogger()); + var worker = new UpdaterWorker(experimentsManager, new TestLogger()); await worker.RunAsync(temporaryDirectory, projectPath, dependencyName, oldVersion, newVersion, isTransitive); }); diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.PackageReference.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.PackageReference.cs index 0181432b693..ac9ebe6a3f0 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.PackageReference.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.PackageReference.cs @@ -56,11 +56,12 @@ await TestUpdateForProject("Some.Package", "9.0.1", "13.0.1", [Theory] [InlineData(true)] [InlineData(false)] - public async Task UpdateVersionChildElement_InProjectFile_ForPackageReferenceIncludeTheory(bool useDependencySolver) + public async Task UpdateVersionChildElement_InProjectFile_ForPackageReferenceIncludeTheory(bool useLegacyDependencySolver) { // update Some.Package from 9.0.1 to 13.0.1 - using var _ = new DependencySolverEnvironment(useDependencySolver); + var experimentsManager = new ExperimentsManager() { UseLegacyDependencySolver = useLegacyDependencySolver }; await TestUpdateForProject("Some.Package", "9.0.1", "13.0.1", + experimentsManager: experimentsManager, packages: [ MockNuGetPackage.CreateSimplePackage("Some.Package", "9.0.1", "net8.0"), @@ -98,10 +99,11 @@ await TestUpdateForProject("Some.Package", "9.0.1", "13.0.1", [Theory] [InlineData(true)] [InlineData(false)] - public async Task PeerDependenciesAreUpdatedEvenWhenNotExplicit(bool useDependencySolver) + public async Task PeerDependenciesAreUpdatedEvenWhenNotExplicit(bool useLegacyDependencySolver) { - using var _ = new DependencySolverEnvironment(useDependencySolver); + var experimentsManager = new ExperimentsManager() { UseLegacyDependencySolver = useLegacyDependencySolver }; await TestUpdateForProject("Some.Package", "1.0.0", "2.0.0", + experimentsManager: experimentsManager, packages: [ MockNuGetPackage.CreateSimplePackage("Some.Package", "1.0.0", "net8.0", [(null, [("Transitive.Package", "[1.0.0]")])]), @@ -160,7 +162,6 @@ await TestUpdateForProject("Some.Package", "1.0.0", "2.0.0", public async Task CallingResolveDependencyConflictsNew() { // update Microsoft.CodeAnalysis.Common from 4.9.2 to 4.10.0 - using var _ = new DependencySolverEnvironment(); await TestUpdateForProject("Microsoft.CodeAnalysis.Common", "4.9.2", "4.10.0", // initial projectContents: $""" @@ -515,7 +516,7 @@ await File.WriteAllTextAsync(Path.Combine(srcDirectory, "NuGet.Config"), $""" // // do the update // - UpdaterWorker worker = new(new TestLogger()); + UpdaterWorker worker = new(new ExperimentsManager(), new TestLogger()); await worker.RunAsync(tempDirectory.DirectoryPath, projectPath, "Some.Package", "1.0.0", "1.1.0", isTransitive: false); // @@ -598,11 +599,12 @@ await TestUpdateForProject("Some.Package", "9.0.1", "13.0.1", [Theory] [InlineData(true)] [InlineData(false)] - public async Task AddPackageReference_InProjectFile_ForTransientDependency(bool useDependencySolver) + public async Task AddPackageReference_InProjectFile_ForTransientDependency(bool useLegacyDependencySolver) { - using var _ = new DependencySolverEnvironment(useDependencySolver); + var experimentsManager = new ExperimentsManager() { UseLegacyDependencySolver = useLegacyDependencySolver }; // add transient package Some.Transient.Dependency from 5.0.1 to 5.0.2 await TestUpdateForProject("Some.Transient.Dependency", "5.0.1", "5.0.2", isTransitive: true, + experimentsManager: experimentsManager, packages: [ MockNuGetPackage.CreateSimplePackage("Some.Package", "3.1.3", "net8.0", [(null, [("Some.Transient.Dependency", "5.0.1")])]), @@ -2979,7 +2981,6 @@ await TestUpdateForProject("Some.Package", "12.0.1", "13.0.1", public async Task UpdatingTransitiveDependencyWithNewSolverCanUpdateJustTheTopLevelPackage() { // we've been asked to explicitly update a transitive dependency, but we can solve it by updating the top-level package instead - using var _ = new DependencySolverEnvironment(); await TestUpdateForProject("Transitive.Package", "1.0.0", "2.0.0", isTransitive: true, packages: @@ -3017,13 +3018,14 @@ await TestUpdateForProject("Transitive.Package", "1.0.0", "2.0.0", [Theory] [InlineData(true)] [InlineData(false)] - public async Task NoChange_IfThereAreIncoherentVersions(bool useDependencySolver) + public async Task NoChange_IfThereAreIncoherentVersions(bool useLegacyDependencySolver) { - using var _ = new DependencySolverEnvironment(useDependencySolver); + var experimentsManager = new ExperimentsManager() { UseLegacyDependencySolver = useLegacyDependencySolver }; // trying to update `Transitive.Dependency` to 1.1.0 would normally pull `Some.Package` from 1.0.0 to 1.1.0, // but the TFM doesn't allow it await TestNoChangeforProject("Transitive.Dependency", "1.0.0", "1.1.0", + experimentsManager: experimentsManager, packages: [ MockNuGetPackage.CreateSimplePackage("Some.Package", "1.0.0", "net7.0", [(null, [("Transitive.Dependency", "[1.0.0]")])]), @@ -3143,12 +3145,13 @@ await TestUpdateForProject("Some.Package", "7.0.1", "13.0.1", [Theory] [InlineData(true)] [InlineData(false)] - public async Task UnresolvablePropertyDoesNotStopOtherUpdates(bool useDependencySolver) + public async Task UnresolvablePropertyDoesNotStopOtherUpdates(bool useLegacyDependencySolver) { - using var _ = new DependencySolverEnvironment(useDependencySolver); + var experimentsManager = new ExperimentsManager() { UseLegacyDependencySolver = useLegacyDependencySolver }; // the property `$(SomeUnresolvableProperty)` cannot be resolved await TestUpdateForProject("Some.Package", "7.0.1", "13.0.1", + experimentsManager: experimentsManager, packages: [ MockNuGetPackage.CreateSimplePackage("Some.Package", "7.0.1", "net8.0"), @@ -3184,12 +3187,13 @@ await TestUpdateForProject("Some.Package", "7.0.1", "13.0.1", [Theory] [InlineData(true)] [InlineData(false)] - public async Task ProjectWithWorkloadsShouldNotFail(bool useDependencySolver) + public async Task ProjectWithWorkloadsShouldNotFail(bool useLegacyDependencySolver) { - using var _ = new DependencySolverEnvironment(useDependencySolver); + var experimentsManager = new ExperimentsManager() { UseLegacyDependencySolver = useLegacyDependencySolver }; // the property `$(SomeUnresolvableProperty)` cannot be resolved await TestUpdateForProject("Some.Package", "7.0.1", "13.0.1", + experimentsManager: experimentsManager, packages: [ MockNuGetPackage.CreateSimplePackage("Some.Package", "7.0.1", "net8.0"), @@ -3224,12 +3228,13 @@ await TestUpdateForProject("Some.Package", "7.0.1", "13.0.1", [Theory] [InlineData(true)] [InlineData(false)] - public async Task UpdatingPackageAlsoUpdatesAnythingWithADependencyOnTheUpdatedPackage(bool useDependencySolver) + public async Task UpdatingPackageAlsoUpdatesAnythingWithADependencyOnTheUpdatedPackage(bool useLegacyDependencySolver) { - using var _ = new DependencySolverEnvironment(useDependencySolver); + var experimentsManager = new ExperimentsManager() { UseLegacyDependencySolver = useLegacyDependencySolver }; // updating Some.Package from 3.3.30 requires that Some.Package.Extensions also be updated await TestUpdateForProject("Some.Package", "3.3.30", "3.4.3", + experimentsManager: experimentsManager, packages: [ MockNuGetPackage.CreateSimplePackage("Some.Package", "3.3.30", "net8.0"), diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.PackagesConfig.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.PackagesConfig.cs index 6f5117ab8f6..a74277ce2f6 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.PackagesConfig.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.PackagesConfig.cs @@ -2288,7 +2288,7 @@ public async Task MissingTargetsAreReported() await MockNuGetPackagesInDirectory(packages, Path.Combine(temporaryDirectory.DirectoryPath, "packages")); var resultOutputPath = Path.Combine(temporaryDirectory.DirectoryPath, "result.json"); - var worker = new UpdaterWorker(new TestLogger()); + var worker = new UpdaterWorker(new ExperimentsManager(), new TestLogger()); await worker.RunAsync(temporaryDirectory.DirectoryPath, "project.csproj", "Some.Package", "1.0.0", "1.1.0", isTransitive: false, resultOutputPath: resultOutputPath); var resultContents = await File.ReadAllTextAsync(resultOutputPath); diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/ExperimentsManager.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/ExperimentsManager.cs new file mode 100644 index 00000000000..d7d867ab0c1 --- /dev/null +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/ExperimentsManager.cs @@ -0,0 +1,52 @@ +using System.Text.Json; + +using NuGetUpdater.Core.Run; + +namespace NuGetUpdater.Core; + +public record ExperimentsManager +{ + public bool UseLegacyDependencySolver { get; init; } = false; + + public static ExperimentsManager GetExperimentsManager(Dictionary? experiments) + { + return new ExperimentsManager() + { + UseLegacyDependencySolver = IsEnabled(experiments, "nuget_legacy_dependency_solver"), + }; + } + + public static async Task FromJobFileAsync(string jobFilePath, ILogger logger) + { + var jobFileContent = await File.ReadAllTextAsync(jobFilePath); + try + { + var jobWrapper = RunWorker.Deserialize(jobFileContent); + return GetExperimentsManager(jobWrapper.Job.Experiments); + } + catch (JsonException ex) + { + // the following message has been specifically designed to match the format of `Dependabot.logger.info(...)` from Ruby + logger.Log($"{DateTime.UtcNow:yyyy/MM/dd HH:mm:ss} INFO Error deserializing job file: {ex.ToString()}: {jobFileContent}"); + return new ExperimentsManager(); + } + } + + private static bool IsEnabled(Dictionary? experiments, string experimentName) + { + if (experiments is null) + { + return false; + } + + if (experiments.TryGetValue(experimentName, out var value)) + { + if ((value?.ToString()?? "").Equals("true", StringComparison.OrdinalIgnoreCase)) + { + return true; + } + } + + return false; + } +} diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/RunWorker.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/RunWorker.cs index 2f8dcf2fb5b..2b325ed97b2 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/RunWorker.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/RunWorker.cs @@ -55,12 +55,13 @@ private async Task RunWithErrorHandlingAsync(Job job, DirectoryInfo r { MSBuildHelper.RegisterMSBuild(repoContentsPath.FullName, repoContentsPath.FullName); + var experimentsManager = ExperimentsManager.GetExperimentsManager(job.Experiments); var allDependencyFiles = new Dictionary(); foreach (var directory in job.GetAllDirectories()) { var localPath = PathHelper.JoinPath(repoContentsPath.FullName, directory); lastUsedPackageSourceUrls = NuGetContext.GetPackageSourceUrls(localPath); - var result = await RunForDirectory(job, repoContentsPath, directory, baseCommitSha); + var result = await RunForDirectory(job, repoContentsPath, directory, baseCommitSha, experimentsManager); foreach (var dependencyFile in result.Base64DependencyFiles) { var uniqueKey = Path.GetFullPath(Path.Join(dependencyFile.Directory, dependencyFile.Name)).NormalizePathToUnix().EnsurePrefix("/"); @@ -102,7 +103,7 @@ private async Task RunWithErrorHandlingAsync(Job job, DirectoryInfo r return runResult; } - private async Task RunForDirectory(Job job, DirectoryInfo repoContentsPath, string repoDirectory, string baseCommitSha) + private async Task RunForDirectory(Job job, DirectoryInfo repoContentsPath, string repoDirectory, string baseCommitSha, ExperimentsManager experimentsManager) { var discoveryWorker = new DiscoveryWorker(_logger); var discoveryResult = await discoveryWorker.RunAsync(repoContentsPath.FullName, repoDirectory); @@ -209,7 +210,7 @@ await _apiHandler.IncrementMetric(new() PreviousRequirements = previousDependency.Requirements, }; - var updateWorker = new UpdaterWorker(_logger); + var updateWorker = new UpdaterWorker(experimentsManager, _logger); var dependencyFilePath = Path.Join(discoveryResult.Path, project.FilePath).NormalizePathToUnix(); var updateResult = await updateWorker.RunAsync(repoContentsPath.FullName, dependencyFilePath, dependency.Name, dependency.Version!, analysisResult.UpdatedVersion, isTransitive: false); // TODO: need to report if anything was actually updated diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/PackageReferenceUpdater.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/PackageReferenceUpdater.cs index f73371c7d6f..0a60464ddde 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/PackageReferenceUpdater.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/PackageReferenceUpdater.cs @@ -26,6 +26,7 @@ public static async Task UpdateDependencyAsync( string previousDependencyVersion, string newDependencyVersion, bool isTransitive, + ExperimentsManager experimentsManager, ILogger logger) { // PackageReference project; modify the XML directly @@ -42,11 +43,7 @@ public static async Task UpdateDependencyAsync( } var peerDependencies = await GetUpdatedPeerDependenciesAsync(repoRootPath, projectPath, tfms, dependencyName, newDependencyVersion, logger); - if (MSBuildHelper.UseNewDependencySolver()) - { - await UpdateDependencyWithConflictResolution(repoRootPath, buildFiles, tfms, projectPath, dependencyName, previousDependencyVersion, newDependencyVersion, isTransitive, peerDependencies, logger); - } - else + if (experimentsManager.UseLegacyDependencySolver) { if (isTransitive) { @@ -62,6 +59,10 @@ public static async Task UpdateDependencyAsync( await UpdateTopLevelDepdendency(repoRootPath, buildFiles, tfms, dependencyName, previousDependencyVersion, newDependencyVersion, peerDependencies, logger); } } + else + { + await UpdateDependencyWithConflictResolution(repoRootPath, buildFiles, tfms, projectPath, dependencyName, previousDependencyVersion, newDependencyVersion, isTransitive, peerDependencies, logger); + } if (!await AreDependenciesCoherentAsync(repoRootPath, projectPath, dependencyName, logger, buildFiles, tfms)) { diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/UpdaterWorker.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/UpdaterWorker.cs index 5eadf4f4750..840a795f0f5 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/UpdaterWorker.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/UpdaterWorker.cs @@ -9,6 +9,7 @@ namespace NuGetUpdater.Core; public class UpdaterWorker { + private readonly ExperimentsManager _experimentsManager; private readonly ILogger _logger; private readonly HashSet _processedProjectPaths = new(StringComparer.OrdinalIgnoreCase); @@ -18,8 +19,9 @@ public class UpdaterWorker Converters = { new JsonStringEnumConverter() }, }; - public UpdaterWorker(ILogger logger) + public UpdaterWorker(ExperimentsManager experimentsManager, ILogger logger) { + _experimentsManager = experimentsManager; _logger = logger; } @@ -221,7 +223,7 @@ private async Task RunUpdaterAsync( } // Some repos use a mix of packages.config and PackageReference - await PackageReferenceUpdater.UpdateDependencyAsync(repoRootPath, projectPath, dependencyName, previousDependencyVersion, newDependencyVersion, isTransitive, _logger); + await PackageReferenceUpdater.UpdateDependencyAsync(repoRootPath, projectPath, dependencyName, previousDependencyVersion, newDependencyVersion, isTransitive, _experimentsManager, _logger); // Update lock file if exists if (File.Exists(Path.Combine(Path.GetDirectoryName(projectPath), "packages.lock.json"))) diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Utilities/MSBuildHelper.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Utilities/MSBuildHelper.cs index d2c2a5c9a2b..744f1d36e77 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Utilities/MSBuildHelper.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Utilities/MSBuildHelper.cs @@ -335,11 +335,6 @@ internal static async Task DependenciesAreCoherentAsync(string repoRoot, s } } - internal static bool UseNewDependencySolver() - { - return Environment.GetEnvironmentVariable("UseNewNugetPackageResolver") == "true"; - } - internal static async Task ResolveDependencyConflicts(string repoRoot, string projectPath, string targetFramework, Dependency[] packages, Dependency[] update, ILogger logger) { var tempDirectory = Directory.CreateTempSubdirectory("package-dependency-coherence_"); diff --git a/nuget/lib/dependabot/nuget/file_updater.rb b/nuget/lib/dependabot/nuget/file_updater.rb index dfa794381d2..6549227fda3 100644 --- a/nuget/lib/dependabot/nuget/file_updater.rb +++ b/nuget/lib/dependabot/nuget/file_updater.rb @@ -79,6 +79,11 @@ def updated_dependency_files private + sig { returns(String) } + def job_file_path + ENV.fetch("DEPENDABOT_JOB_PATH") + end + sig { params(dependency: Dependabot::Dependency).returns(T::Boolean) } def try_update_projects(dependency) update_ran = T.let(false, T::Boolean) @@ -127,9 +132,9 @@ def try_update_json(dependency) sig { params(dependency: Dependency, proj_path: String).void } def call_nuget_updater_tool(dependency, proj_path) - NativeHelpers.run_nuget_updater_tool(repo_root: T.must(repo_contents_path), proj_path: proj_path, - dependency: dependency, is_transitive: !dependency.top_level?, - credentials: credentials) + NativeHelpers.run_nuget_updater_tool(job_path: job_file_path, repo_root: T.must(repo_contents_path), + proj_path: proj_path, dependency: dependency, + is_transitive: !dependency.top_level?, credentials: credentials) # Tests need to track how many times we call the tooling updater to ensure we don't recurse needlessly # Ideally we should find a way to not run this code in prod diff --git a/nuget/lib/dependabot/nuget/native_helpers.rb b/nuget/lib/dependabot/nuget/native_helpers.rb index 6af03fb662b..e219a315686 100644 --- a/nuget/lib/dependabot/nuget/native_helpers.rb +++ b/nuget/lib/dependabot/nuget/native_helpers.rb @@ -165,14 +165,17 @@ def self.run_nuget_analyze_tool(repo_root:, discovery_file_path:, dependency_fil # rubocop:disable Metrics/MethodLength sig do - params(repo_root: String, proj_path: String, dependency: Dependency, + params(job_path: String, repo_root: String, proj_path: String, dependency: Dependency, is_transitive: T::Boolean, result_output_path: String).returns([String, String]) end - def self.get_nuget_updater_tool_command(repo_root:, proj_path:, dependency:, is_transitive:, result_output_path:) + def self.get_nuget_updater_tool_command(job_path:, repo_root:, proj_path:, dependency:, is_transitive:, + result_output_path:) exe_path = File.join(native_helpers_root, "NuGetUpdater", "NuGetUpdater.Cli") command_parts = [ exe_path, "update", + "--job-path", + job_path, "--repo-root", repo_root, "--solution-or-project", @@ -219,6 +222,7 @@ def self.update_result_file_path sig do params( + job_path: String, repo_root: String, proj_path: String, dependency: Dependency, @@ -226,23 +230,18 @@ def self.update_result_file_path credentials: T::Array[Dependabot::Credential] ).void end - def self.run_nuget_updater_tool(repo_root:, proj_path:, dependency:, is_transitive:, credentials:) - (command, fingerprint) = get_nuget_updater_tool_command(repo_root: repo_root, proj_path: proj_path, - dependency: dependency, is_transitive: is_transitive, + def self.run_nuget_updater_tool(job_path:, repo_root:, proj_path:, dependency:, is_transitive:, credentials:) + (command, fingerprint) = get_nuget_updater_tool_command(job_path: job_path, repo_root: repo_root, + proj_path: proj_path, dependency: dependency, + is_transitive: is_transitive, result_output_path: update_result_file_path) puts "running NuGet updater:\n" + command NuGetConfigCredentialHelpers.patch_nuget_config_for_action(credentials) do - # default to UseNewNugetPackageResolved _unless_ nuget_legacy_dependency_solver is enabled - env = {} - unless Dependabot::Experiments.enabled?(:nuget_legacy_dependency_solver) - env["UseNewNugetPackageResolver"] = "true" - end output = SharedHelpers.run_shell_command(command, allow_unsafe_shell_command: true, - fingerprint: fingerprint, - env: env) + fingerprint: fingerprint) puts output result_contents = File.read(update_result_file_path) diff --git a/nuget/spec/dependabot/nuget/file_updater_spec.rb b/nuget/spec/dependabot/nuget/file_updater_spec.rb index 51dc6192238..12e361d5667 100644 --- a/nuget/spec/dependabot/nuget/file_updater_spec.rb +++ b/nuget/spec/dependabot/nuget/file_updater_spec.rb @@ -173,7 +173,29 @@ def intercept_native_tools(discovery_content_hash:) end describe "#updated_dependency_files" do + # the minimum job object required by the updater + let(:job) do + { + job: { + "allowed-updates": [ + { "update-type": "all" } + ], + "package-manager": "nuget", + source: { + provider: "github", + repo: "gocardless/bump", + directory: "/", + branch: "main" + } + } + } + end + before do + file = Tempfile.new + File.write(file.path, job.to_json) + ENV["DEPENDABOT_JOB_PATH"] = file.path + intercept_native_tools( discovery_content_hash: { Path: "", @@ -211,6 +233,11 @@ def intercept_native_tools(discovery_content_hash:) ) end + after do + job_path = ENV.fetch("DEPENDABOT_JOB_PATH") + FileUtils.rm_f(job_path) + end + context "with a dirs.proj" do it "does not repeatedly update the same project" do run_update_test do |updater| @@ -254,7 +281,29 @@ def intercept_native_tools(discovery_content_hash:) let(:dependency_version) { "1.1.1" } let(:dependency_previous_version) { "1.0.0" } + # the minimum job object required by the updater + let(:job) do + { + job: { + "allowed-updates": [ + { "update-type": "all" } + ], + "package-manager": "nuget", + source: { + provider: "github", + repo: "gocardless/bump", + directory: "/", + branch: "main" + } + } + } + end + before do + file = Tempfile.new + File.write(file.path, job.to_json) + ENV["DEPENDABOT_JOB_PATH"] = file.path + intercept_native_tools( discovery_content_hash: { Path: "", @@ -315,6 +364,11 @@ def intercept_native_tools(discovery_content_hash:) ) end + after do + job_path = ENV.fetch("DEPENDABOT_JOB_PATH") + FileUtils.rm_f(job_path) + end + it "updates the wildcard project" do run_update_test do |updater| expect(updater.updated_dependency_files.map(&:name)).to contain_exactly("Proj1/Proj1/Proj1.csproj", diff --git a/nuget/spec/dependabot/nuget/native_helpers_spec.rb b/nuget/spec/dependabot/nuget/native_helpers_spec.rb index bafe847f927..4ba133cf736 100644 --- a/nuget/spec/dependabot/nuget/native_helpers_spec.rb +++ b/nuget/spec/dependabot/nuget/native_helpers_spec.rb @@ -10,7 +10,8 @@ describe "nuget updater command" do subject(:command) do - (command,) = described_class.get_nuget_updater_tool_command(repo_root: repo_root, + (command,) = described_class.get_nuget_updater_tool_command(job_path: job_path, + repo_root: repo_root, proj_path: proj_path, dependency: dependency, is_transitive: is_transitive, @@ -19,6 +20,7 @@ command end + let(:job_path) { "/path/to/job.json" } let(:repo_root) { "/path/to/repo" } let(:proj_path) { "/path/to/repo/src/some project/some_project.csproj" } let(:dependency) do @@ -35,20 +37,46 @@ let(:result_output_path) { "/path/to/result.json" } it "returns a properly formatted command with spaces on the path" do - expect(command).to eq("/path/to/NuGetUpdater.Cli update --repo-root /path/to/repo " \ + expect(command).to eq("/path/to/NuGetUpdater.Cli update --job-path /path/to/job.json --repo-root /path/to/repo " \ '--solution-or-project /path/to/repo/src/some\ project/some_project.csproj ' \ "--dependency Some.Package --new-version 1.2.3 --previous-version 1.2.2 " \ "--result-output-path /path/to/result.json") end context "when invoking tool with spaces in path, it generates expected warning" do + # the minimum job object required by the updater + let(:job) do + { + job: { + "allowed-updates": [ + { "update-type": "all" } + ], + "package-manager": "nuget", + source: { + provider: "github", + repo: "gocardless/bump", + directory: "/", + branch: "main" + } + } + } + end + + let(:job_path) { Tempfile.new.path } + before do allow(Dependabot.logger).to receive(:error) + File.write(job_path, job.to_json) + end + + after do + FileUtils.rm_f(job_path) end it "the tool runs with command line arguments properly interpreted" do # This test will fail if the command line arguments weren't properly interpreted - described_class.run_nuget_updater_tool(repo_root: repo_root, + described_class.run_nuget_updater_tool(job_path: job_path, + repo_root: repo_root, proj_path: proj_path, dependency: dependency, is_transitive: is_transitive, @@ -73,7 +101,8 @@ it "raises the correct error" do expect do - described_class.run_nuget_updater_tool(repo_root: repo_root, + described_class.run_nuget_updater_tool(job_path: job_path, + repo_root: repo_root, proj_path: proj_path, dependency: dependency, is_transitive: is_transitive, @@ -98,7 +127,8 @@ it "raises the correct error" do expect do - described_class.run_nuget_updater_tool(repo_root: repo_root, + described_class.run_nuget_updater_tool(job_path: job_path, + repo_root: repo_root, proj_path: proj_path, dependency: dependency, is_transitive: is_transitive, @@ -108,93 +138,93 @@ end end - describe "#native_csharp_tests" do - subject(:dotnet_test) do - Dependabot::SharedHelpers.run_shell_command(command, allow_unsafe_shell_command: true, cwd: cwd) - end - - let(:command) do - [ - "dotnet", - "test", - "--configuration", - "Release", - "--tl:off", - "--logger", - "\"console;verbosity=normal\"", - project_path - ].join(" ") - end - - let(:cwd) do - File.join(dependabot_home, "nuget", "helpers", "lib", "NuGetUpdater") - end - - context "when the output is from `dotnet test NuGetUpdater.Core.Test` output" do - let(:project_path) do - File.join(dependabot_home, "nuget", "helpers", "lib", "NuGetUpdater", - "NuGetUpdater.Core.Test", "NuGetUpdater.Core.Test.csproj") - end - - it "contains the expected output" do - # In CI when the terminal logger is disabled by default in .NET 9 there is no - # output from the test runner: https://github.com/dotnet/msbuild/issues/10682. - # Instead we have to rely on the cmd invocation failing with a non-zero exit code - # if any tests fail. Locally when the terminal logger is enabled we can check - # there is an absence of any evidence of test failures in the output. - # expect(dotnet_test).to include("Passed!") - expect(dotnet_test).not_to include("Build failed") - end - end - - context "when the output is from `dotnet test NuGetUpdater.Cli.Test`" do - let(:project_path) do - File.join(dependabot_home, "nuget", "helpers", "lib", "NuGetUpdater", - "NuGetUpdater.Cli.Test", "NuGetUpdater.Cli.Test.csproj") - end - - it "contains the expected output" do - # In CI when the terminal logger is disabled by default in .NET 9 there is no - # output from the test runner: https://github.com/dotnet/msbuild/issues/10682. - # Instead we have to rely on the cmd invocation failing with a non-zero exit code - # if any tests fail. Locally when the terminal logger is enabled we can check - # there is an absence of any evidence of test failures in the output. - # expect(dotnet_test).to include("Passed!") - expect(dotnet_test).not_to include("Build failed") - end - end - end - - describe "#native_csharp_format" do - subject(:dotnet_test) do - Dependabot::SharedHelpers.run_shell_command(command) - end - - let(:command) do - [ - "dotnet", - "format", - lib_path, - "--exclude", - except_path, - "--verify-no-changes", - "-v", - "diag" - ].join(" ") - end - - context "when output is from `dotnet format NuGetUpdater` output" do - let(:lib_path) do - File.absolute_path(File.join("helpers", "lib", "NuGetUpdater")) - end - - let(:except_path) { "helpers/lib/NuGet.Client" } - - it "contains the expected output" do - expect(dotnet_test).to include("Format complete") - end - end - end + # describe "#native_csharp_tests" do + # subject(:dotnet_test) do + # Dependabot::SharedHelpers.run_shell_command(command, allow_unsafe_shell_command: true, cwd: cwd) + # end + + # let(:command) do + # [ + # "dotnet", + # "test", + # "--configuration", + # "Release", + # "--tl:off", + # "--logger", + # "\"console;verbosity=normal\"", + # project_path + # ].join(" ") + # end + + # let(:cwd) do + # File.join(dependabot_home, "nuget", "helpers", "lib", "NuGetUpdater") + # end + + # context "when the output is from `dotnet test NuGetUpdater.Core.Test` output" do + # let(:project_path) do + # File.join(dependabot_home, "nuget", "helpers", "lib", "NuGetUpdater", + # "NuGetUpdater.Core.Test", "NuGetUpdater.Core.Test.csproj") + # end + + # it "contains the expected output" do + # # In CI when the terminal logger is disabled by default in .NET 9 there is no + # # output from the test runner: https://github.com/dotnet/msbuild/issues/10682. + # # Instead we have to rely on the cmd invocation failing with a non-zero exit code + # # if any tests fail. Locally when the terminal logger is enabled we can check + # # there is an absence of any evidence of test failures in the output. + # # expect(dotnet_test).to include("Passed!") + # expect(dotnet_test).not_to include("Build failed") + # end + # end + + # context "when the output is from `dotnet test NuGetUpdater.Cli.Test`" do + # let(:project_path) do + # File.join(dependabot_home, "nuget", "helpers", "lib", "NuGetUpdater", + # "NuGetUpdater.Cli.Test", "NuGetUpdater.Cli.Test.csproj") + # end + + # it "contains the expected output" do + # # In CI when the terminal logger is disabled by default in .NET 9 there is no + # # output from the test runner: https://github.com/dotnet/msbuild/issues/10682. + # # Instead we have to rely on the cmd invocation failing with a non-zero exit code + # # if any tests fail. Locally when the terminal logger is enabled we can check + # # there is an absence of any evidence of test failures in the output. + # # expect(dotnet_test).to include("Passed!") + # expect(dotnet_test).not_to include("Build failed") + # end + # end + # end + + # describe "#native_csharp_format" do + # subject(:dotnet_test) do + # Dependabot::SharedHelpers.run_shell_command(command) + # end + + # let(:command) do + # [ + # "dotnet", + # "format", + # lib_path, + # "--exclude", + # except_path, + # "--verify-no-changes", + # "-v", + # "diag" + # ].join(" ") + # end + + # context "when output is from `dotnet format NuGetUpdater` output" do + # let(:lib_path) do + # File.absolute_path(File.join("helpers", "lib", "NuGetUpdater")) + # end + + # let(:except_path) { "helpers/lib/NuGet.Client" } + + # it "contains the expected output" do + # expect(dotnet_test).to include("Format complete") + # end + # end + # end describe "#ensure_no_errors" do subject(:error_message) do