Skip to content
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

validate shape of packages.config and consolidate error handling #11303

Merged
merged 1 commit into from
Jan 15, 2025
Merged

validate shape of packages.config and consolidate error handling #11303

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 7 additions & 1 deletion nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Analyze.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ public async Task FindsUpdatedPackageAndReturnsTheCorrectData()
await RunAsync(path =>
[
"analyze",
"--job-id",
"TEST-JOB-ID",
"--job-path",
Path.Combine(path, "job.json"),
"--repo-root",
Expand Down Expand Up @@ -146,6 +148,8 @@ public async Task DotNetToolsJsonCanBeAnalyzed()
await RunAsync(path =>
[
"analyze",
"--job-id",
"TEST-JOB-ID",
"--job-path",
Path.Combine(path, "job.json"),
"--repo-root",
Expand Down Expand Up @@ -235,6 +239,8 @@ public async Task GlobalJsonCanBeAnalyzed()
await RunAsync(path =>
[
"analyze",
"--job-id",
"TEST-JOB-ID",
"--job-path",
Path.Combine(path, "job.json"),
"--repo-root",
Expand Down Expand Up @@ -345,7 +351,7 @@ private static async Task RunAsync(
{
if (args[i] == "--job-path")
{
var experimentsResult = await ExperimentsManager.FromJobFileAsync(args[i + 1]);
var experimentsResult = await ExperimentsManager.FromJobFileAsync("TEST-JOB-ID", args[i + 1]);
experimentsManager = experimentsResult.ExperimentsManager;
}
}
Expand Down
52 changes: 46 additions & 6 deletions nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Discover.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System.Text;
using System.Text.Json;
using System.Text.Json.Serialization;

using NuGetUpdater.Core;
using NuGetUpdater.Core.Discover;
Expand All @@ -25,6 +26,8 @@ public async Task PathWithSpaces(bool useDirectDiscovery)
await RunAsync(path =>
[
"discover",
"--job-id",
"TEST-JOB-ID",
"--job-path",
Path.Combine(path, "job.json"),
"--repo-root",
Expand Down Expand Up @@ -83,6 +86,8 @@ public async Task WithSolution(bool useDirectDiscovery)
await RunAsync(path =>
[
"discover",
"--job-id",
"TEST-JOB-ID",
"--job-path",
Path.Combine(path, "job.json"),
"--repo-root",
Expand Down Expand Up @@ -178,6 +183,8 @@ public async Task WithProject(bool useDirectDiscovery)
await RunAsync(path =>
[
"discover",
"--job-id",
"TEST-JOB-ID",
"--job-path",
Path.Combine(path, "job.json"),
"--repo-root",
Expand Down Expand Up @@ -251,6 +258,8 @@ public async Task WithDirectory(bool useDirectDiscovery)
await RunAsync(path =>
[
"discover",
"--job-id",
"TEST-JOB-ID",
"--job-path",
Path.Combine(path, "job.json"),
"--repo-root",
Expand Down Expand Up @@ -321,6 +330,8 @@ public async Task WithDuplicateDependenciesOfDifferentTypes()
await RunAsync(path =>
[
"discover",
"--job-id",
"TEST-JOB-ID",
"--job-path",
Path.Combine(path, "job.json"),
"--repo-root",
Expand Down Expand Up @@ -398,6 +409,8 @@ public async Task JobFileParseErrorIsReported_InvalidJson()
await RunAsync(path =>
[
"discover",
"--job-id",
"TEST-JOB-ID",
"--job-path",
jobFilePath,
"--repo-root",
Expand All @@ -412,8 +425,7 @@ await RunAsync(path =>
{
Path = "/",
Projects = [],
ErrorType = ErrorType.Unknown,
ErrorDetailsPattern = "JsonException",
ErrorRegex = "Error deserializing job file contents",
}
);
}
Expand Down Expand Up @@ -445,6 +457,8 @@ await File.WriteAllTextAsync(jobFilePath, """
await RunAsync(path =>
[
"discover",
"--job-id",
"TEST-JOB-ID",
"--job-path",
jobFilePath,
"--repo-root",
Expand All @@ -459,8 +473,7 @@ await RunAsync(path =>
{
Path = "/",
Projects = [],
ErrorType = ErrorType.BadRequirement,
ErrorDetailsPattern = "not a valid requirement",
Error = new Core.Run.ApiModel.BadRequirement("not a valid requirement"),
}
);
}
Expand Down Expand Up @@ -497,7 +510,7 @@ private static async Task RunAsync(
switch (args[i])
{
case "--job-path":
var experimentsResult = await ExperimentsManager.FromJobFileAsync(args[i + 1]);
var experimentsResult = await ExperimentsManager.FromJobFileAsync("TEST-JOB-ID", args[i + 1]);
experimentsManager = experimentsResult.ExperimentsManager;
break;
case "--output":
Expand All @@ -520,11 +533,38 @@ private static async Task RunAsync(

resultPath ??= Path.Join(path, DiscoveryWorker.DiscoveryResultFileName);
var resultJson = await File.ReadAllTextAsync(resultPath);
var resultObject = JsonSerializer.Deserialize<WorkspaceDiscoveryResult>(resultJson, DiscoveryWorker.SerializerOptions);
var serializerOptions = new JsonSerializerOptions()
{
Converters = { new TestJobErrorBaseConverter() }
};
foreach (var converter in DiscoveryWorker.SerializerOptions.Converters)
{
serializerOptions.Converters.Add(converter);
}
var resultObject = JsonSerializer.Deserialize<WorkspaceDiscoveryResult>(resultJson, serializerOptions);
return resultObject!;
});

ValidateWorkspaceResult(expectedResult, actualResult, experimentsManager);
}

private class TestJobErrorBaseConverter : JsonConverter<Core.Run.ApiModel.JobErrorBase>
{
public override Core.Run.ApiModel.JobErrorBase? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
var dict = JsonSerializer.Deserialize<Dictionary<string, JsonElement>>(ref reader, options)!;
return dict["error-type"].GetString() switch
{
"illformed_requirement" => new Core.Run.ApiModel.BadRequirement(dict["error-details"].GetProperty("message").GetString()!),
"unknown_error" => new Core.Run.ApiModel.UnknownError(new Exception("Error deserializing job file contents"), "TEST-JOB-ID"),
_ => throw new NotImplementedException($"Unknown error type: {dict["error-type"]}"),
};
}

public override void Write(Utf8JsonWriter writer, Core.Run.ApiModel.JobErrorBase value, JsonSerializerOptions options)
{
throw new NotImplementedException();
}
}
}
}
8 changes: 8 additions & 0 deletions nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Update.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ public async Task WithSolution()
await Run(path =>
[
"update",
"--job-id",
"TEST-JOB-ID",
"--job-path",
Path.Combine(path, "job.json"),
"--repo-root",
Expand Down Expand Up @@ -122,6 +124,8 @@ public async Task WithProject()
await Run(path =>
[
"update",
"--job-id",
"TEST-JOB-ID",
"--job-path",
Path.Combine(path, "job.json"),
"--repo-root",
Expand Down Expand Up @@ -202,6 +206,8 @@ public async Task WithDirsProjAndDirectoryBuildPropsThatIsOutOfDirectoryButStill
await Run(path =>
[
"update",
"--job-id",
"TEST-JOB-ID",
"--job-path",
Path.Combine(path, "job.json"),
"--repo-root",
Expand Down Expand Up @@ -361,6 +367,8 @@ await File.WriteAllTextAsync(projectPath, """
IEnumerable<string> executableArgs = [
executableName,
"update",
"--job-id",
"TEST-JOB-ID",
"--job-path",
Path.Combine(tempDir.DirectoryPath, "job.json"),
"--repo-root",
Expand Down
10 changes: 6 additions & 4 deletions nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/AnalyzeCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ namespace NuGetUpdater.Cli.Commands;

internal static class AnalyzeCommand
{
internal static readonly Option<string> JobIdOption = new("--job-id") { IsRequired = true };
internal static readonly Option<FileInfo> JobPathOption = new("--job-path") { IsRequired = true };
internal static readonly Option<DirectoryInfo> RepoRootOption = new("--repo-root") { IsRequired = true };
internal static readonly Option<FileInfo> DependencyFilePathOption = new("--dependency-file-path") { IsRequired = true };
Expand All @@ -17,6 +18,7 @@ internal static Command GetCommand(Action<int> setExitCode)
{
Command command = new("analyze", "Determines how to update a dependency based on the workspace discovery information.")
{
JobIdOption,
JobPathOption,
RepoRootOption,
DependencyFilePathOption,
Expand All @@ -26,13 +28,13 @@ internal static Command GetCommand(Action<int> setExitCode)

command.TreatUnmatchedTokensAsErrors = true;

command.SetHandler(async (jobPath, repoRoot, discoveryPath, dependencyPath, analysisDirectory) =>
command.SetHandler(async (jobId, jobPath, repoRoot, discoveryPath, dependencyPath, analysisDirectory) =>
{
var logger = new ConsoleLogger();
var (experimentsManager, _errorResult) = await ExperimentsManager.FromJobFileAsync(jobPath.FullName);
var worker = new AnalyzeWorker(experimentsManager, logger);
var (experimentsManager, _errorResult) = await ExperimentsManager.FromJobFileAsync(jobId, jobPath.FullName);
var worker = new AnalyzeWorker(jobId, experimentsManager, logger);
await worker.RunAsync(repoRoot.FullName, discoveryPath.FullName, dependencyPath.FullName, analysisDirectory.FullName);
}, JobPathOption, RepoRootOption, DiscoveryFilePathOption, DependencyFilePathOption, AnalysisFolderOption);
}, JobIdOption, JobPathOption, RepoRootOption, DiscoveryFilePathOption, DependencyFilePathOption, AnalysisFolderOption);

return command;
}
Expand Down
15 changes: 8 additions & 7 deletions nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/DiscoverCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ namespace NuGetUpdater.Cli.Commands;

internal static class DiscoverCommand
{
internal static readonly Option<string> JobIdOption = new("--job-id") { IsRequired = true };
internal static readonly Option<FileInfo> JobPathOption = new("--job-path") { IsRequired = true };
internal static readonly Option<DirectoryInfo> RepoRootOption = new("--repo-root") { IsRequired = true };
internal static readonly Option<string> WorkspaceOption = new("--workspace") { IsRequired = true };
Expand All @@ -16,6 +17,7 @@ internal static Command GetCommand(Action<int> setExitCode)
{
Command command = new("discover", "Generates a report of the workspace dependencies and where they are located.")
{
JobIdOption,
JobPathOption,
RepoRootOption,
WorkspaceOption,
Expand All @@ -24,27 +26,26 @@ internal static Command GetCommand(Action<int> setExitCode)

command.TreatUnmatchedTokensAsErrors = true;

command.SetHandler(async (jobPath, repoRoot, workspace, outputPath) =>
command.SetHandler(async (jobId, jobPath, repoRoot, workspace, outputPath) =>
{
var (experimentsManager, errorResult) = await ExperimentsManager.FromJobFileAsync(jobPath.FullName);
if (errorResult is not null)
var (experimentsManager, error) = await ExperimentsManager.FromJobFileAsync(jobId, jobPath.FullName);
if (error is not null)
{
// to make testing easier, this should be a `WorkspaceDiscoveryResult` object
var discoveryErrorResult = new WorkspaceDiscoveryResult
{
Error = error,
Path = workspace,
Projects = [],
ErrorType = errorResult.ErrorType,
ErrorDetails = errorResult.ErrorDetails,
};
await DiscoveryWorker.WriteResultsAsync(repoRoot.FullName, outputPath.FullName, discoveryErrorResult);
return;
}

var logger = new ConsoleLogger();
var worker = new DiscoveryWorker(experimentsManager, logger);
var worker = new DiscoveryWorker(jobId, experimentsManager, logger);
await worker.RunAsync(repoRoot.FullName, workspace, outputPath.FullName);
}, JobPathOption, RepoRootOption, WorkspaceOption, OutputOption);
}, JobIdOption, JobPathOption, RepoRootOption, WorkspaceOption, OutputOption);

return command;
}
Expand Down
8 changes: 4 additions & 4 deletions nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/RunCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ internal static Command GetCommand(Action<int> setExitCode)
command.SetHandler(async (jobPath, repoContentsPath, apiUrl, jobId, outputPath, baseCommitSha) =>
{
var apiHandler = new HttpApiHandler(apiUrl.ToString(), jobId);
var (experimentsManager, _errorResult) = await ExperimentsManager.FromJobFileAsync(jobPath.FullName);
var (experimentsManager, _errorResult) = await ExperimentsManager.FromJobFileAsync(jobId, jobPath.FullName);
var logger = new ConsoleLogger();
var discoverWorker = new DiscoveryWorker(experimentsManager, logger);
var analyzeWorker = new AnalyzeWorker(experimentsManager, logger);
var updateWorker = new UpdaterWorker(experimentsManager, logger);
var discoverWorker = new DiscoveryWorker(jobId, experimentsManager, logger);
var analyzeWorker = new AnalyzeWorker(jobId, experimentsManager, logger);
var updateWorker = new UpdaterWorker(jobId, experimentsManager, logger);
var worker = new RunWorker(jobId, apiHandler, discoverWorker, analyzeWorker, updateWorker, logger);
await worker.RunAsync(jobPath, repoContentsPath, baseCommitSha, outputPath);
}, JobPathOption, RepoContentsPathOption, ApiUrlOption, JobIdOption, OutputPathOption, BaseCommitShaOption);
Expand Down
22 changes: 17 additions & 5 deletions nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/UpdateCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ namespace NuGetUpdater.Cli.Commands;

internal static class UpdateCommand
{
internal static readonly Option<string> JobIdOption = new("--job-id") { IsRequired = true };
internal static readonly Option<FileInfo> JobPathOption = new("--job-path") { IsRequired = true };
internal static readonly Option<DirectoryInfo> RepoRootOption = new("--repo-root", () => new DirectoryInfo(Environment.CurrentDirectory)) { IsRequired = false };
internal static readonly Option<FileInfo> SolutionOrProjectFileOption = new("--solution-or-project") { IsRequired = true };
Expand All @@ -19,6 +20,7 @@ internal static Command GetCommand(Action<int> setExitCode)
{
Command command = new("update", "Applies the changes from an analysis report to update a dependency.")
{
JobIdOption,
JobPathOption,
RepoRootOption,
SolutionOrProjectFileOption,
Expand All @@ -30,15 +32,25 @@ internal static Command GetCommand(Action<int> setExitCode)
};

command.TreatUnmatchedTokensAsErrors = true;

command.SetHandler(async (jobPath, repoRoot, solutionOrProjectFile, dependencyName, newVersion, previousVersion, isTransitive, resultOutputPath) =>
command.SetHandler(async (context) =>
{
var (experimentsManager, _errorResult) = await ExperimentsManager.FromJobFileAsync(jobPath.FullName);
// since we have more than 8 arguments, we have to pull them out manually
Copy link
Contributor

Choose a reason for hiding this comment

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

That's annoying.

var jobId = context.ParseResult.GetValueForOption(JobIdOption)!;
var jobPath = context.ParseResult.GetValueForOption(JobPathOption)!;
var repoRoot = context.ParseResult.GetValueForOption(RepoRootOption)!;
var solutionOrProjectFile = context.ParseResult.GetValueForOption(SolutionOrProjectFileOption)!;
var dependencyName = context.ParseResult.GetValueForOption(DependencyNameOption)!;
var newVersion = context.ParseResult.GetValueForOption(NewVersionOption)!;
var previousVersion = context.ParseResult.GetValueForOption(PreviousVersionOption)!;
var isTransitive = context.ParseResult.GetValueForOption(IsTransitiveOption);
var resultOutputPath = context.ParseResult.GetValueForOption(ResultOutputPathOption);

var (experimentsManager, _error) = await ExperimentsManager.FromJobFileAsync(jobId, jobPath.FullName);
var logger = new ConsoleLogger();
var worker = new UpdaterWorker(experimentsManager, logger);
var worker = new UpdaterWorker(jobId, experimentsManager, logger);
await worker.RunAsync(repoRoot.FullName, solutionOrProjectFile.FullName, dependencyName, previousVersion, newVersion, isTransitive, resultOutputPath);
setExitCode(0);
}, JobPathOption, RepoRootOption, SolutionOrProjectFileOption, DependencyNameOption, NewVersionOption, PreviousVersionOption, IsTransitiveOption, ResultOutputPathOption);
});

return command;
}
Expand Down
Loading
Loading