Add dotnet package update command

[zivkan]

NuGet.Client added a (VERY!!!) MVP version of dotnet package update.

Rather than adding a new API to NuGet.Client every time we add a new command (and therefore are unable to add integration tests until after the dotnet/sdk repo is updated), I added a generic NuGetCommands.Add(RootCommand) API, so in the future NuGet can add its own integration tests in the same PR that adds new commands.

Added a single, fairly simple test here, to make sure that the command works, but the NuGet.Client repo has all the exhaustive tests for the command.

[Copilot]

Pull Request Overview

This PR introduces a new MVP version of the "dotnet package update" command and integrates it into the CLI toolchain via a generic API for adding NuGet commands. Key changes include:

• Adding a new test to verify the package update flow.
• Integrating the new command with the root command in the parser.
• Creating an API that allows NuGet to add its own integration tests in the future.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
test/dotnet.Tests/CommandTests/NuGet/GivenANuGetCommand.cs	Adds tests for the new package update command ensuring dependency versions are updated.
src/Cli/dotnet/Parser.cs	Modifies the command configuration to accept a RootCommand and integrates NuGet's generic command registration.

Comments suppressed due to low confidence (1)

test/dotnet.Tests/CommandTests/NuGet/GivenANuGetCommand.cs:175

• Consider verifying that SelectToken returns a single value. If there is a possibility of receiving an array, explicitly extract the appropriate element before converting to string.

var updatedPackageVersionString = JObject.Parse(listPackageCommandResult.StdOut).SelectToken("$.projects[0].frameworks[0].topLevelPackages[?(@.id == 'dotnet-hello')].requestedVersion").ToString();

[marcpopMSFT]

VerifyCompletions baselines will need to be updated. @baronfel any suggestions on how to modify these tests so they don't need to be updated every time the CLI changes slightly?

[baronfel]

Daniel asked similar - I'll send a PR with more docs soon.

The point of the tests is to keep an eye on the CLI, since it is our user interface. When the CLI changes in a way that impacts completions (new commands, options, defaults) we need to inspect and reconcile the baselines. Most of this happens in the dotnet.Tests project, in the Microsoft.DotNet.Cli.Completions.Tests.DotnetCliSnapshotTests tests.

These tests use Verify to perform snapshot testing - storing the results of a known good 'baseline' as a snapshot and comparing that the the output of the same action performed with changes in your PR. Verify calls these baselines 'verified' files. When the test computes a new snapshot for comparison, this is called a 'received' file. Verify compares the 'verified' file to the 'received' file for each test, and if they are not the same provides a git-diff in the console output for the test.

To fix these tests, you need to diff the two files and visually inspect the changes. If the changes to the 'received' file are what you want to see (new commands, new options, renames, etc) then you rename the 'received' file to 'verified' and commit that change to the 'verified' file. There are two MSBuild Targets on the dotnet.Tests project that you can use to help you do this, both of which are intended to be run after you run the snapshot tests locally:

• CompareCliSnapshots - this Target copies the .received. files from the artifacts directory, where they are created due to the way we run tests, to the ./CompletionTests/snapshots directory in the dotnet.Tests project. This makes it much easier to diff the two.
• UpdateCliSnapshots - this Target renames the .received. files to .verified., and so acts as a giant 'I accept these changes' button. Only use this if you've diffed the snapshots and are sure they match you expectations.

[marcpopMSFT]

Are we truly getting value in flagging every single CLI change in these tests though? I would assume that most cases of this would just end up taking the new output. We had tests like this from templating and they were constantly breaking from external causes so we had to greatly scope down what the test was checking for to ensure they stay passing. It's a tradeoff of what you check for and the smoothness of the PR process.

I'd also point out that updating these requires building and running the tests locally which can be an annoying requirement if the person already moved on. I did have some success getting copilot to update a different PR for these tests so maybe we just rely on that.

[baronfel]

building and running the tests locally which can be an annoying requirement if the person already moved on. I did have some success getting copilot to update a different PR for these tests so maybe we just rely on that.

one thing we could/should do here is have the .received files auto-upload like binlogs do. I can try setting that up.

[marcpopMSFT]

fwiw, I was able to get copilot to fix these tests just with the output from the test on a different PR. So maybe the binlog isn't necessary.

[zivkan]

one thing we could/should do here is have the .received files auto-upload like binlogs do. I can try setting that up

This still means that as NuGet adds new options to existing commands, where the command definition is in the NuGet.Client repo, that NuGet's insertions into the SDK will fail, right?

Regarding dotnet package update, I'm going with a "start with MVP, then incrementally add features as I have capacity to implement them". This means NuGet's insertions will be blocked "regularly" until the command is close to feature complete.

[zivkan]

@marcpopMSFT @baronfel I updated the completion snapshot tests, and they seem to be passing, but I see a whole bunch of failures in dotnet watch tests. I'm trying hard to get this in for preview 6. Can you please help me, given the short deadline? thanks.

[baronfel]

The watch tests have some known issues - I think we can skip them for purposes of this PR.

[zivkan]

Azure DevOps' test view on the CI build isn't showing the failed test, but looking at the Helix console log, I see this, but only on Windows, so I think the test is passing on Linux and Mac:

Received: DotnetCliSnapshotTests.VerifyCompletions.received.zsh
Verified: DotnetCliSnapshotTests.VerifyCompletions.verified.zsh
Compare Result:
0846                         '-r=[The target runtime to run for.]:RUNTIME_IDENTIFIER:->dotnet_dynamic_complete' \
[xUnit.net 00:02:55.86]          -                         '--project=[The path to the project file to run (defaults to the current directory if there is only one project).]: : ' \
[xUnit.net 00:02:55.86]          -                         '--launch-profile=[The name of the launch profile (if any) to use when launching the application.]: : ' \
[xUnit.net 00:02:55.86]          -                         '-lp=[The name of the launch profile (if any) to use when launching the application.]: : ' \
[xUnit.net 00:02:55.86]          +                         '--project=[The path to the project file to run (defaults to the current directory if there is only one project).]:project: ' \
[xUnit.net 00:02:55.86]          +                         '--launch-profile=[The name of the launch profile (if any) to use when launching the application.]:launch-profile: ' \
[xUnit.net 00:02:55.86]          +                         '-lp=[The name of the launch profile (if any) to use when launching the application.]:launch-profile: ' \
[xUnit.net 00:02:55.86]       0850                         '--no-launch-profile[Do not attempt to use launchSettings.json to configure the application.]' \

Reforatting the diff:

'-r=[The target runtime to run for.]:RUNTIME_IDENTIFIER:->dotnet_dynamic_complete' \
-                         '--project=[The path to the project file to run (defaults to the current directory if there is only one project).]: : ' \
-                         '--launch-profile=[The name of the launch profile (if any) to use when launching the application.]: : ' \
-                         '-lp=[The name of the launch profile (if any) to use when launching the application.]: : ' \
+                         '--project=[The path to the project file to run (defaults to the current directory if there is only one project).]:project: ' \
+                         '--launch-profile=[The name of the launch profile (if any) to use when launching the application.]:launch-profile: ' \
+                         '-lp=[The name of the launch profile (if any) to use when launching the application.]:launch-profile: ' \
'--no-launch-profile[Do not attempt to use launchSettings.json to configure the application.]' \

However, we can see that this pull request is not modifying lines 846 to 850.

Additionally, when I run the test locally, I can't reproduce the failure, but this test keeps failing on CI, even after multiple retries.

Any ideas, @baronfel ?

[baronfel]

we've got a known issue for this specifically - several of us have hit this same issue. I have no rationale for why this happens.

[zivkan]

Sorry, I didn't think to search for a known issue first:

• DotnetCliSnapshotTests.VerifyCompletions failed in CI #48817

[marcpopMSFT]

failures are known issues in watch, container, and completion tests. merging per request