All-in-one NuGet task PR feedback

Author: emanuelquintero

Tasks/NuGetCommand/Common/INuGetCommandOptions.ts

@@ -1,4 +1,4 @@
-import {NuGetEnvironmentSettings} from "./NuGetToolRunner";
+import { NuGetEnvironmentSettings } from "./NuGetToolRunner";
 
 export interface INuGetCommandOptions {
     /** settings used to initialize the environment NuGet.exe is invoked in */

Tasks/NuGetCommand/Common/NuGetConfigHelper.ts

@@ -30,23 +30,24 @@ export class NuGetConfigHelper {
 
     public ensureTempConfigCreated() {
         // save nuget config file to agent build directory
-        console.log("save nuget.config to temp config file");
-        if (!(fs.existsSync(this.tempNugetConfigDir))) {
-            fs.mkdirSync(this.tempNugetConfigDir);
+        console.log(tl.loc("Info_SavingTempConfig"));
+        
+        if (!tl.exist(this.tempNugetConfigDir)) {
+            tl.mkdirP(this.tempNugetConfigDir);
         }
 
         this.tempNugetConfigPath = path.join(this.tempNugetConfigDir, this.tempNugetConfigFileName);
 
-        if (!fs.existsSync(this.tempNugetConfigPath))
+        if (!tl.exist(this.tempNugetConfigPath))
         {
             if (this.nugetConfigPath) {
                 // don't use cp as that copies the read-only flag, and tfvc sets that on files
                 let content = fs.readFileSync(this.nugetConfigPath);
-                fs.writeFileSync(this.tempNugetConfigPath, content);
+                tl.writeFile(this.tempNugetConfigPath, content);
             }
             else {
                 // small file, use writeFileSync
-                fs.writeFileSync(this.tempNugetConfigPath, "<configuration/>");
+                tl.writeFile(this.tempNugetConfigPath, "<configuration/>");
             }
         }
     }

Tasks/NuGetCommand/Common/NuGetToolRunner.ts

@@ -143,7 +143,7 @@ export async function getNuGetQuirksAsync(nuGetExePath: string): Promise<NuGetQu
 // when using the credential provider.
 // Unfortunately, on on-premises TFS, we must use credential provider to override NTLM auth with the build
 // identity's token.
-// Therefore, we are enabling credential provider on on-premises and disabling it on hosted. We allow for test
+// Therefore, we are enabling credential provider on on-premises and disabling it on hosted (only when the version of NuGet does not support it). We allow for test
 // instances by an override variable.
 
 export function isCredentialProviderEnabled(quirks: NuGetQuirks): boolean {

Tasks/NuGetCommand/Strings/resources.resjson/en-US/resources.resjson

@@ -5,7 +5,6 @@
   "loc.instanceNameFormat": "NuGet $(command)",
   "loc.group.displayName.restoreAuth": "Feeds and authentication",
   "loc.group.displayName.restoreAdvanced": "Advanced",
-  "loc.group.displayName.pushAuth": "Destination feed and authentication",
   "loc.group.displayName.pushAdvanced": "Advanced",
   "loc.group.displayName.packOptions": "Pack options",
   "loc.group.displayName.packAdvanced": "Advanced",
@@ -14,9 +13,9 @@
   "loc.input.label.solution": "Path to solution, packages.config, or project.json",
   "loc.input.help.solution": "The path to the solution, packages.config, or project.json file that references the packages to be restored.",
   "loc.input.label.selectOrConfig": "Feeds to use",
-  "loc.input.help.selectOrConfig": "You can either select feeds from VSTS and/or NuGet.org here, or commit a nuget.config file to your source code repository and set its path here.",
+  "loc.input.help.selectOrConfig": "You can either select a feed from VSTS and/or NuGet.org here, or commit a nuget.config file to your source code repository and set its path here.",
   "loc.input.label.feedRestore": "Use packages from this VSTS/TFS feed",
-  "loc.input.help.feedRestore": "Include the selected feeds in the generated NuGet.config. You must have Package Management installed and licensed to select a feed here.",
+  "loc.input.help.feedRestore": "Include the selected feed in the generated NuGet.config. You must have Package Management installed and licensed to select a feed here.",
   "loc.input.label.includeNuGetOrg": "Use packages from NuGet.org",
   "loc.input.help.includeNuGetOrg": "Include NuGet.org in the generated NuGet.config.",
   "loc.input.label.nugetConfigPath": "Path to NuGet.config",
@@ -47,7 +46,7 @@
   "loc.input.label.outputDir": "Package Folder",
   "loc.input.help.outputDir": "Folder where packages will be created. If empty, packages will be created alongside the csproj or nuspec file.",
   "loc.input.label.versioningScheme": "Automatic package versioning",
-  "loc.input.help.versioningScheme": "Cannot be used with include referenced projects. If you choose 'Use the date and time', this will generate a [SemVer](http://semver.org/spec/v1.0.0.html)-compliant version formatted as `X.Y.Z-ci-datetime` where you choose X, Y, and Z.\n\nIf you choose 'Use an environment variable', you must select an environment variable and ensure it contains the version number you want to use.\n\nIf you choose 'Use the build number', this will use the build number to version you package. **Note:** Under General set the build format to be '[$(BuildDefinitionName)_$(Year:yyyy).$(Month).$(DayOfMonth)$(Rev:.r)](https://go.microsoft.com/fwlink/?LinkID=627416)'.",
+  "loc.input.help.versioningScheme": "Cannot be used with include referenced projects. If you choose 'Use the date and time', this will generate a [SemVer](http://semver.org/spec/v1.0.0.html)-compliant version formatted as `X.Y.Z-ci-datetime` where you choose X, Y, and Z.\n\nIf you choose 'Use an environment variable', you must select an environment variable and ensure it contains the version number you want to use.\n\nIf you choose 'Use the build number', this will use the build number to version your package. **Note:** Under Options set the build number format to be '[$(BuildDefinitionName)_$(Year:yyyy).$(Month).$(DayOfMonth)$(Rev:.r)](https://go.microsoft.com/fwlink/?LinkID=627416)'.",
   "loc.input.label.includeReferencedProjects": "Include referenced projects",
   "loc.input.help.includeReferencedProjects": "Include referenced projects either as dependencies or as part of the package. Cannot be used with automatic package versioning. If a referenced project has a corresponding nuspec file that has the same name as the project, then that referenced project is added as a dependency. Otherwise, the referenced project is added as part of the package. [Learn more](https://docs.nuget.org/consume/command-line-reference#pack-command) at NuGet.org.",
   "loc.input.label.versionEnvVar": "Environment variable",
@@ -59,7 +58,7 @@
   "loc.input.label.requestedPatchVersion": "Patch",
   "loc.input.help.requestedPatchVersion": "The 'Z' in version [X.Y.Z](http://semver.org/spec/v1.0.0.html)",
   "loc.input.label.buildProperties": "Additional build properties",
-  "loc.input.help.buildProperties": "Semicolon delimited list of properties used to build the package.",
+  "loc.input.help.buildProperties": "Specifies a list of token=value pairs, separated by semicolons, where each occurrence of $token$ in the .nuspec file will be replaced with the given value. Values can be strings in quotation marks.",
   "loc.input.label.verbosityPack": "Verbosity",
   "loc.input.help.verbosityPack": "Specifies the amount of detail displayed in the output.",
   "loc.input.label.arguments": "Command and arguments",
@@ -114,5 +113,7 @@
   "loc.messages.Warning_ForceNuGetCannotSkipConflicts": "NuGet.exe was forced to be used for publish, if push conflicts occur it will result in a failure.",
   "loc.messages.Info_MatchingUrlWasFoundSettingAuth": "Using authentication information for the following URI: ",
   "loc.messages.Error_PushNotARegularFile": "%s is not a file. Check the 'Path/Pattern to nupkg' property of the task.",
-  "loc.messages.Warning_AllowDuplicatesOnlyAvailableHosted": "The 'Allow duplicates to be skipped' option is currently only available on Visual Studio Team Services. If NuGet.exe encounters a conflict, the task will fail."
+  "loc.messages.Warning_AllowDuplicatesOnlyAvailableHosted": "The 'Allow duplicates to be skipped' option is currently only available on Visual Studio Team Services. If NuGet.exe encounters a conflict, the task will fail.",
+  "loc.messages.Info_SavingTempConfig": "Saving NuGet.config to a temporary config file.",
+  "loc.messages.Error_CommandNotRecognized": "The command %s was not recognized."
 }

Tasks/NuGetCommand/Tests/L0.ts

@@ -2,7 +2,7 @@ import * as path from 'path';
 import * as assert from 'assert';
 import * as ttm from 'vsts-task-lib/mock-test';
 
-describe('NuGetRestore Suite', function () {
+describe('NuGetCommand Suite', function () {
     before(() => {
     });
 
@@ -240,7 +240,7 @@ describe('NuGetRestore Suite', function () {
 
         tr.run()
         assert(tr.invokedToolCount == 1, 'should have run NuGet once');
-        assert(tr.ran('c:\\from\\tool\\installer\\nuget.exe pack c:\\agent\\home\\directory\\foo.nuspec -NonInteractive -OutputDirectory C:\\out\\dir -version x.y.z-CI-YYYYMMDD-HHMMSS'), 'it should have run NuGet');
+        assert(tr.ran('c:\\from\\tool\\installer\\nuget.exe pack c:\\agent\\home\\directory\\foo.nuspec -NonInteractive -OutputDirectory c:\\agent\\home\\directory -Properties -version x.y.z-CI-YYYYMMDD-HHMMSS'), 'it should have run NuGet');
         assert(tr.stdOutContained('setting console code page'), 'it should have run chcp');
         assert(tr.stdOutContained('NuGet output here'), "should have nuget output");
         assert(tr.succeeded, 'should have succeeded');
@@ -256,7 +256,7 @@ describe('NuGetRestore Suite', function () {
 
         tr.run()
         assert(tr.invokedToolCount == 1, 'should have run NuGet once');
-        assert(tr.ran('c:\\from\\tool\\installer\\nuget.exe pack c:\\agent\\home\\directory\\foo.nuspec -NonInteractive -OutputDirectory C:\\out\\dir -version XX.YY.ZZ'), 'it should have run NuGet');
+        assert(tr.ran('c:\\from\\tool\\installer\\nuget.exe pack c:\\agent\\home\\directory\\foo.nuspec -NonInteractive -OutputDirectory c:\\agent\\home\\directory -Properties -version XX.YY.ZZ'), 'it should have run NuGet');
         assert(tr.stdOutContained('setting console code page'), 'it should have run chcp');
         assert(tr.stdOutContained('NuGet output here'), "should have nuget output");
         assert(tr.succeeded, 'should have succeeded');
@@ -272,7 +272,7 @@ describe('NuGetRestore Suite', function () {
 
         tr.run()
         assert(tr.invokedToolCount == 1, 'should have run NuGet once');
-        assert(tr.ran('c:\\from\\tool\\installer\\nuget.exe pack c:\\agent\\home\\directory\\foo.nuspec -NonInteractive -OutputDirectory C:\\out\\dir -version 1.2.3'), 'it should have run NuGet');
+        assert(tr.ran('c:\\from\\tool\\installer\\nuget.exe pack c:\\agent\\home\\directory\\foo.nuspec -NonInteractive -OutputDirectory c:\\agent\\home\\directory -Properties -version 1.2.3'), 'it should have run NuGet');
         assert(tr.stdOutContained('setting console code page'), 'it should have run chcp');
         assert(tr.stdOutContained('NuGet output here'), "should have nuget output");
         assert(tr.succeeded, 'should have succeeded');

Tasks/NuGetCommand/Tests/PackTests/packBuildNumber.ts

@@ -21,7 +21,7 @@ let a: ma.TaskLibAnswers = <ma.TaskLibAnswers>{
     },
     "which": {},
     "exec": {
-        "c:\\from\\tool\\installer\\nuget.exe pack c:\\agent\\home\\directory\\foo.nuspec -NonInteractive -OutputDirectory C:\\out\\dir -version 1.2.3": {
+        "c:\\from\\tool\\installer\\nuget.exe pack c:\\agent\\home\\directory\\foo.nuspec -NonInteractive -OutputDirectory c:\\agent\\home\\directory -Properties -version 1.2.3": {
             "code": 0,
             "stdout": "NuGet output here",
             "stderr": ""
@@ -34,6 +34,9 @@ let a: ma.TaskLibAnswers = <ma.TaskLibAnswers>{
         "c:\\agent\\home\\directory\\foo.nuspec": {
             "isFile": true
         }
+    }, 
+    "findMatch": {
+        "foo.nuspec" : ["c:\\agent\\home\\directory\\foo.nuspec"]
     }
 };
 nmh.setAnswers(a);

Tasks/NuGetCommand/Tests/PackTests/packEnvVar.ts

@@ -22,7 +22,7 @@ let a: ma.TaskLibAnswers = <ma.TaskLibAnswers>{
     },
     "which": {},
     "exec": {
-        "c:\\from\\tool\\installer\\nuget.exe pack c:\\agent\\home\\directory\\foo.nuspec -NonInteractive -OutputDirectory C:\\out\\dir -version XX.YY.ZZ": {
+        "c:\\from\\tool\\installer\\nuget.exe pack c:\\agent\\home\\directory\\foo.nuspec -NonInteractive -OutputDirectory c:\\agent\\home\\directory -Properties -version XX.YY.ZZ": {
             "code": 0,
             "stdout": "NuGet output here",
             "stderr": ""
@@ -35,6 +35,9 @@ let a: ma.TaskLibAnswers = <ma.TaskLibAnswers>{
         "c:\\agent\\home\\directory\\foo.nuspec": {
             "isFile": true
         }
+    }, 
+    "findMatch": {
+        "foo.nuspec" : ["c:\\agent\\home\\directory\\foo.nuspec"]
     }
 };
 nmh.setAnswers(a);

Tasks/NuGetCommand/Tests/PackTests/packPrerelease.ts

@@ -24,7 +24,7 @@ let a: ma.TaskLibAnswers = <ma.TaskLibAnswers>{
     },
     "which": {},
     "exec": {
-        "c:\\from\\tool\\installer\\nuget.exe pack c:\\agent\\home\\directory\\foo.nuspec -NonInteractive -OutputDirectory C:\\out\\dir -version x.y.z-CI-YYYYMMDD-HHMMSS": {
+        "c:\\from\\tool\\installer\\nuget.exe pack c:\\agent\\home\\directory\\foo.nuspec -NonInteractive -OutputDirectory c:\\agent\\home\\directory -Properties -version x.y.z-CI-YYYYMMDD-HHMMSS": {
             "code": 0,
             "stdout": "NuGet output here",
             "stderr": ""
@@ -37,6 +37,9 @@ let a: ma.TaskLibAnswers = <ma.TaskLibAnswers>{
         "c:\\agent\\home\\directory\\foo.nuspec": {
             "isFile": true
         }
+    }, 
+    "findMatch": {
+        "foo.nuspec" : ["c:\\agent\\home\\directory\\foo.nuspec"]
     }
 };
 nmh.setAnswers(a);

Tasks/NuGetCommand/Tests/PublishTests/externalFeedNuGet.ts

@@ -9,7 +9,7 @@ let nmh: util.NugetMockHelper = new util.NugetMockHelper(tmr);
 
 nmh.setNugetVersionInputDefault();
 tmr.setInput('command', 'push');
-tmr.setInput('searchPattern', 'foo.nupkg');
+tmr.setInput('searchPatternPush', 'foo.nupkg');
 tmr.setInput('nuGetFeedType', 'external');
 tmr.setInput('connectedServiceName', 'externalNuGetFeed');
 
@@ -31,6 +31,9 @@ let a: ma.TaskLibAnswers = <ma.TaskLibAnswers>{
         "c:\\agent\\home\\directory\\foo.nupkg": {
             "isFile": true
         }
+    }, 
+    "findMatch": {
+        "foo.nupkg" : ["c:\\agent\\home\\directory\\foo.nupkg"]
     }
 };
 nmh.setAnswers(a);
@@ -41,6 +44,5 @@ nmh.registerDefaultNugetVersionMock();
 nmh.registerToolRunnerMock();
 nmh.registerNugetConfigMock();
 nmh.RegisterLocationServiceMocks();
-//nmh.registerCommonUtilities();
 
 tmr.run();

Tasks/NuGetCommand/Tests/PublishTests/internalFeedNuGet.ts

@@ -9,7 +9,7 @@ let nmh: util.NugetMockHelper = new util.NugetMockHelper(tmr);
 
 nmh.setNugetVersionInputDefault();
 tmr.setInput('command', 'push');
-tmr.setInput('searchPattern', 'foo.nupkg');
+tmr.setInput('searchPatternPush', 'foo.nupkg');
 tmr.setInput('nuGetFeedType', 'internal');
 tmr.setInput('feedPublish', 'FeedFooId');
 tmr.setInput('allowPackageConflicts', 'false');
@@ -32,6 +32,9 @@ let a: ma.TaskLibAnswers = <ma.TaskLibAnswers>{
         "c:\\agent\\home\\directory\\foo.nupkg": {
             "isFile": true
         }
+    }, 
+    "findMatch": {
+        "foo.nupkg" : ["c:\\agent\\home\\directory\\foo.nupkg"]
     }
 };
 nmh.setAnswers(a);