-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Improve error message when installing tools targeting higher .NET versions #51665
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.
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
Changes from 1 commit
21d375c
8787231
a612275
1618a37
008fccc
0c2384b
b9f9b35
fa4e0f5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
Added three end-to-end tests that verify the improved error message when installing/running a tool targeting net99.0: - InstallToolWithHigherFrameworkAsGlobalToolShowsAppropriateError: Tests global tool installation - InstallToolWithHigherFrameworkAsLocalToolShowsAppropriateError: Tests local tool installation - RunToolWithHigherFrameworkUsingDnxShowsAppropriateError: Tests running tool with dnx The tests create a minimal .nupkg with net99.0 framework and verify that the error message includes helpful information about the version mismatch. Co-authored-by: dsplaisted <[email protected]>
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -500,6 +500,141 @@ static XElement GetToolSettingsFile(string packagePath) | |
|
|
||
| } | ||
|
|
||
| [Fact] | ||
| public void InstallToolWithHigherFrameworkAsGlobalToolShowsAppropriateError() | ||
| { | ||
| var toolPackagesPath = CreateNet99ToolPackage(); | ||
| var testDirectory = _testAssetsManager.CreateTestDirectory(); | ||
| var homeFolder = Path.Combine(testDirectory.Path, "home"); | ||
|
|
||
| var result = new DotnetToolCommand(Log, "install", "-g", "Net99Tool", "--add-source", toolPackagesPath) | ||
| .WithEnvironmentVariables(homeFolder) | ||
| .WithWorkingDirectory(testDirectory.Path) | ||
| .Execute(); | ||
|
|
||
| result.Should().Fail() | ||
| .And.HaveStdErrContaining("requires a higher version of .NET") | ||
| .And.HaveStdErrContaining(".NET 99"); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void InstallToolWithHigherFrameworkAsLocalToolShowsAppropriateError() | ||
| { | ||
| var toolPackagesPath = CreateNet99ToolPackage(); | ||
| var testDirectory = _testAssetsManager.CreateTestDirectory(); | ||
| var homeFolder = Path.Combine(testDirectory.Path, "home"); | ||
|
|
||
| new DotnetCommand(Log, "new", "tool-manifest") | ||
| .WithEnvironmentVariables(homeFolder) | ||
| .WithWorkingDirectory(testDirectory.Path) | ||
| .Execute() | ||
| .Should().Pass(); | ||
|
|
||
| var result = new DotnetToolCommand(Log, "install", "Net99Tool", "--add-source", toolPackagesPath) | ||
| .WithEnvironmentVariables(homeFolder) | ||
| .WithWorkingDirectory(testDirectory.Path) | ||
| .Execute(); | ||
|
|
||
| result.Should().Fail() | ||
| .And.HaveStdErrContaining("requires a higher version of .NET") | ||
| .And.HaveStdErrContaining(".NET 99"); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void RunToolWithHigherFrameworkUsingDnxShowsAppropriateError() | ||
| { | ||
| var toolPackagesPath = CreateNet99ToolPackage(); | ||
| var testDirectory = _testAssetsManager.CreateTestDirectory(); | ||
| var homeFolder = Path.Combine(testDirectory.Path, "home"); | ||
|
|
||
| var result = new DotnetToolCommand(Log, "exec", "Net99Tool", "--yes", "--source", toolPackagesPath) | ||
| .WithEnvironmentVariables(homeFolder) | ||
| .WithWorkingDirectory(testDirectory.Path) | ||
| .Execute(); | ||
|
|
||
| result.Should().Fail() | ||
| .And.HaveStdErrContaining("requires a higher version of .NET") | ||
| .And.HaveStdErrContaining(".NET 99"); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Creates a tool package that targets net99.0 to simulate a tool requiring a higher .NET version | ||
| /// </summary> | ||
| private string CreateNet99ToolPackage() | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you update this method to use the NuGet pack command to create the package rather than working with a raw zip file? This will mean you won't have to create the .rels and content types files and should be more resilient to changes in how NuGet creates packages in the future. Probably the best way to do this is to lay the files out on disk and create a .nuspec file, then pack a project that specifies that nuspec file. You might also be able to add each file that goes in the package as items in the project with appropriate metadata tu put them in the right path in the package.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated in commit b9f9b35. The method now uses |
||
| { | ||
| var testDirectory = _testAssetsManager.CreateTestDirectory(identifier: "net99tool"); | ||
| var packageOutputPath = Path.Combine(testDirectory.Path, "packages"); | ||
| Directory.CreateDirectory(packageOutputPath); | ||
|
|
||
| // Create a minimal .nupkg with net99.0 framework | ||
| var nupkgPath = Path.Combine(packageOutputPath, "Net99Tool.1.0.0.nupkg"); | ||
|
|
||
| using (var archive = ZipFile.Open(nupkgPath, ZipArchiveMode.Create)) | ||
| { | ||
| // Add nuspec file | ||
| var nuspecEntry = archive.CreateEntry("Net99Tool.nuspec"); | ||
| using (var writer = new StreamWriter(nuspecEntry.Open())) | ||
| { | ||
| writer.Write(@"<?xml version=""1.0"" encoding=""utf-8""?> | ||
| <package xmlns=""http://schemas.microsoft.com/packaging/2012/06/nuspec.xsd""> | ||
| <metadata> | ||
| <id>Net99Tool</id> | ||
| <version>1.0.0</version> | ||
| <authors>Test</authors> | ||
| <description>Test tool targeting net99.0</description> | ||
| <packageTypes> | ||
| <packageType name=""DotnetTool"" /> | ||
| </packageTypes> | ||
| </metadata> | ||
| </package>"); | ||
| } | ||
|
|
||
| // Add DotnetToolSettings.xml | ||
| var settingsEntry = archive.CreateEntry("tools/net99.0/any/DotnetToolSettings.xml"); | ||
| using (var writer = new StreamWriter(settingsEntry.Open())) | ||
| { | ||
| writer.Write(@"<?xml version=""1.0"" encoding=""utf-8""?> | ||
| <DotNetCliTool Version=""1""> | ||
| <Commands> | ||
| <Command Name=""net99tool"" EntryPoint=""Net99Tool.dll"" Runner=""dotnet"" /> | ||
| </Commands> | ||
| </DotNetCliTool>"); | ||
| } | ||
|
|
||
| // Add a minimal DLL (just empty file for this test) | ||
| var dllEntry = archive.CreateEntry("tools/net99.0/any/Net99Tool.dll"); | ||
| using (var writer = new StreamWriter(dllEntry.Open())) | ||
| { | ||
| writer.Write(""); | ||
| } | ||
|
|
||
| // Add .rels file | ||
| var relsEntry = archive.CreateEntry("_rels/.rels"); | ||
| using (var writer = new StreamWriter(relsEntry.Open())) | ||
| { | ||
| writer.Write(@"<?xml version=""1.0"" encoding=""utf-8""?> | ||
| <Relationships xmlns=""http://schemas.openxmlformats.org/package/2006/relationships""> | ||
| <Relationship Type=""http://schemas.microsoft.com/packaging/2010/07/manifest"" Target=""/Net99Tool.nuspec"" Id=""Re0"" /> | ||
| </Relationships>"); | ||
| } | ||
|
|
||
| // Add [Content_Types].xml | ||
| var contentTypesEntry = archive.CreateEntry("[Content_Types].xml"); | ||
| using (var writer = new StreamWriter(contentTypesEntry.Open())) | ||
| { | ||
| writer.Write(@"<?xml version=""1.0"" encoding=""utf-8""?> | ||
| <Types xmlns=""http://schemas.openxmlformats.org/package/2006/content-types""> | ||
| <Default Extension=""rels"" ContentType=""application/vnd.openxmlformats-package.relationships+xml"" /> | ||
| <Default Extension=""nuspec"" ContentType=""application/octet-stream"" /> | ||
| <Default Extension=""xml"" ContentType=""application/xml"" /> | ||
| <Default Extension=""dll"" ContentType=""application/octet-stream"" /> | ||
| </Types>"); | ||
| } | ||
| } | ||
|
|
||
| return packageOutputPath; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Opens the nupkg and verifies that it does not contain a dependency on the given dll. | ||
| /// </summary> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you update the WithEnvironmentVariable method so that it also sets NUGET_PACKAGES to a
.nuget\packagesfolder under the home folder? This will make it so that the test package doesn't leak out into a user-wide or repo packages folder (if that were to happen then the tests could end up consuming older versions of the mock package since the version doesn't change).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in commit b9f9b35. The
WithEnvironmentVariablesmethod now setsNUGET_PACKAGESto{homeFolder}/.nuget/packagesto prevent test package leakage.