Sm/multiple-flag-bug

messages/commonFlags.md

@@ -0,0 +1,32 @@
+# commaWarningForTests
+
+The previous version of this command used a comma-separated list for tests. We've changed how you specify multiple tests, so if you continue using your current syntax, your tests will probably not run as you expect. 
+
+%s
+
+# flags.tests.summary
+
+Apex tests to run when --test-level is RunSpecifiedTests.
+
+# flags.tests.description
+
+If a test name contains a space, enclose it in double quotes.
+For multiple test names, use one of the following formats:
+
+- Repeat the flag for multiple test names. --tests Test1 --tests Test2 --tests "Test With Space"
+- Separate the test names with spaces --tests Test1 Test2 "Test With Space"
+
+# flags.coverage-formatters.summary
+
+Format of the code coverage results.
+
+# flags.coverage-formatters.description
+
+For multiple formatters, repeat the flag for each formatter.
+--coverage-formatters lcov --coverage-formatters clover
+
+# commaWarningForCoverageFormatters
+
+The previous version of this command used a comma-separated list for coverage formatters. We've changed how you specify multiple coverage formatters, so if you continue using your current syntax, your coverage results will probably not look as you expect. 
+
+%s

messages/deploy.metadata.md

@@ -148,14 +148,6 @@ Ignore warnings and allow a deployment to complete successfully.
 
 If a warning occurs and this flag is set to true, the success status of the deployment is set to true. When this flag is set to false, success is set to false, and the warning is treated like an error.
 
-# flags.tests.summary
-
-Apex tests to run when --test-level is RunSpecifiedTests.
-
-# flags.tests.description
-
-Separate multiple test names with commas, and enclose the entire flag value in double quotes if a test contains a space.
-
 # flags.verbose.summary
 
 Show verbose output of the deploy result.
@@ -227,10 +219,6 @@ No local changes to deploy.
 
 Output JUnit test results.
 
-# flags.coverage-formatters.summary
-
-Format of the code coverage results.
-
 # flags.results-dir.summary
 
 Output directory for code coverage and JUnit results; defaults to the deploy ID.

messages/deploy.metadata.report.md

@@ -45,10 +45,6 @@ For performance reasons, this flag uses job IDs for deploy operations that start
 
 Output JUnit test results.
 
-# flags.coverage-formatters.summary
-
-Format of the code coverage results
-
 # flags.results-dir.summary
 
 Output directory for code coverage and JUnit results; defaults to the deploy ID.

messages/deploy.metadata.resume.md

@@ -65,10 +65,6 @@ Job ID %s is not resumable with status %s.
 
 Output JUnit test results.
 
-# flags.coverage-formatters.summary
-
-Format of the code coverage results.
-
 # flags.results-dir.summary
 
 Output directory for code coverage and JUnit results; defaults to the deploy ID.

messages/deploy.metadata.validate.md

@@ -86,10 +86,6 @@ All child components are included. If you specify this flag, don’t specify --m
 
 The API to use for validating the deployment.
 
-# flags.tests.summary
-
-Apex tests to run when --test-level is RunSpecifiedTests.
-
 # flags.verbose.summary
 
 Show verbose output of the validation result.

package.json

@@ -7,7 +7,7 @@
   "dependencies": {
     "@oclif/core": "^2.8.2",
     "@salesforce/apex-node": "^1.6.0",
-    "@salesforce/core": "^3.34.6",
+    "@salesforce/core": "^3.36.0",
     "@salesforce/kit": "^1.9.2",
     "@salesforce/sf-plugins-core": "^2.4.2",
     "@salesforce/source-deploy-retrieve": "^8.0.2",
@@ -269,4 +269,4 @@
       "output": []
     }
   }
-}
+}

src/commands/project/deploy/report.ts

@@ -12,7 +12,8 @@ import { DeployResult, MetadataApiDeployStatus } from '@salesforce/source-deploy
 import { buildComponentSet } from '../../../utils/deploy';
 import { DeployCache } from '../../../utils/deployCache';
 import { DeployReportResultFormatter } from '../../../formatters/deployReportResultFormatter';
-import { DeployResultJson, reportsFormatters } from '../../../utils/types';
+import { DeployResultJson } from '../../../utils/types';
+import { coverageFormattersFlag } from '../../../utils/flags';
 
 Messages.importMessagesDirectory(__dirname);
 const messages = Messages.loadMessages('@salesforce/plugin-deploy-retrieve', 'deploy.metadata.report');
@@ -39,11 +40,7 @@ export default class DeployMetadataReport extends SfCommand<DeployResultJson> {
       summary: messages.getMessage('flags.use-most-recent.summary'),
       exactlyOne: ['use-most-recent', 'job-id'],
     }),
-    'coverage-formatters': Flags.string({
-      multiple: true,
-      summary: messages.getMessage('flags.coverage-formatters.summary'),
-      options: reportsFormatters,
-    }),
+    'coverage-formatters': coverageFormattersFlag,
     junit: Flags.boolean({ summary: messages.getMessage('flags.junit.summary') }),
     'results-dir': Flags.directory({
       dependsOn: ['junit', 'coverage-formatters'],

src/commands/project/deploy/resume.ts

@@ -12,10 +12,11 @@ import { Duration } from '@salesforce/kit';
 import { getVersionMessage } from '../../../utils/output';
 import { DeployResultFormatter } from '../../../formatters/deployResultFormatter';
 import { DeployProgress } from '../../../utils/progressBar';
-import { DeployResultJson, reportsFormatters } from '../../../utils/types';
+import { DeployResultJson } from '../../../utils/types';
 import { determineExitCode, executeDeploy, isNotResumable } from '../../../utils/deploy';
 import { DeployCache } from '../../../utils/deployCache';
 import { DEPLOY_STATUS_CODES_DESCRIPTIONS } from '../../../utils/errorCodes';
+import { coverageFormattersFlag } from '../../../utils/flags';
 
 Messages.importMessagesDirectory(__dirname);
 const messages = Messages.loadMessages('@salesforce/plugin-deploy-retrieve', 'deploy.metadata.resume');
@@ -60,11 +61,7 @@ export default class DeployMetadataResume extends SfCommand<DeployResultJson> {
       helpValue: '<minutes>',
       min: 1,
     }),
-    'coverage-formatters': Flags.string({
-      multiple: true,
-      summary: messages.getMessage('flags.coverage-formatters.summary'),
-      options: reportsFormatters,
-    }),
+    'coverage-formatters': coverageFormattersFlag,
     junit: Flags.boolean({ summary: messages.getMessage('flags.junit.summary') }),
     'results-dir': Flags.directory({
       dependsOn: ['junit', 'coverage-formatters'],

src/commands/project/deploy/start.ts

@@ -12,12 +12,12 @@ import { getVersionMessage } from '../../../utils/output';
 import { AsyncDeployResultFormatter } from '../../../formatters/asyncDeployResultFormatter';
 import { DeployResultFormatter } from '../../../formatters/deployResultFormatter';
 import { DeployProgress } from '../../../utils/progressBar';
-import { DeployResultJson, TestLevel, reportsFormatters } from '../../../utils/types';
+import { DeployResultJson, TestLevel } from '../../../utils/types';
 import { executeDeploy, resolveApi, validateTests, determineExitCode } from '../../../utils/deploy';
 import { DeployCache } from '../../../utils/deployCache';
 import { DEPLOY_STATUS_CODES_DESCRIPTIONS } from '../../../utils/errorCodes';
 import { ConfigVars } from '../../../configMeta';
-import { fileOrDirFlag, testLevelFlag } from '../../../utils/flags';
+import { coverageFormattersFlag, fileOrDirFlag, testLevelFlag, testsFlag } from '../../../utils/flags';
 import { writeConflictTable } from '../../../utils/conflicts';
 
 Messages.importMessagesDirectory(__dirname);
@@ -105,12 +105,7 @@ export default class DeployMetadata extends SfCommand<DeployResultJson> {
       summary: messages.getMessage('flags.target-org.summary'),
       required: true,
     }),
-    tests: Flags.string({
-      char: 't',
-      multiple: true,
-      summary: messages.getMessage('flags.tests.summary'),
-      description: messages.getMessage('flags.tests.description'),
-    }),
+    tests: testsFlag,
     'test-level': testLevelFlag({
       default: TestLevel.NoTestRun,
       description: messages.getMessage('flags.test-level.description'),
@@ -144,11 +139,7 @@ export default class DeployMetadata extends SfCommand<DeployResultJson> {
       summary: messages.getMessage('flags.post-destructive-changes.summary'),
       dependsOn: ['manifest'],
     }),
-    'coverage-formatters': Flags.string({
-      multiple: true,
-      summary: messages.getMessage('flags.coverage-formatters.summary'),
-      options: reportsFormatters,
-    }),
+    'coverage-formatters': coverageFormattersFlag,
     junit: Flags.boolean({ summary: messages.getMessage('flags.junit.summary'), dependsOn: ['coverage-formatters'] }),
     'results-dir': Flags.directory({
       dependsOn: ['coverage-formatters'],

src/commands/project/deploy/validate.ts

@@ -16,7 +16,7 @@ import { DeployResultJson, TestLevel } from '../../../utils/types';
 import { executeDeploy, resolveApi, determineExitCode } from '../../../utils/deploy';
 import { DEPLOY_STATUS_CODES_DESCRIPTIONS } from '../../../utils/errorCodes';
 import { ConfigVars } from '../../../configMeta';
-import { fileOrDirFlag, testLevelFlag } from '../../../utils/flags';
+import { fileOrDirFlag, testLevelFlag, testsFlag } from '../../../utils/flags';
 
 Messages.importMessagesDirectory(__dirname);
 const messages = Messages.loadMessages('@salesforce/plugin-deploy-retrieve', 'deploy.metadata.validate');
@@ -79,11 +79,7 @@ export default class DeployMetadataValidate extends SfCommand<DeployResultJson>
       summary: messages.getMessage('flags.target-org.summary'),
       required: true,
     }),
-    tests: Flags.string({
-      char: 't',
-      multiple: true,
-      summary: messages.getMessage('flags.tests.summary'),
-    }),
+    tests: testsFlag,
     'test-level': testLevelFlag({
       options: [TestLevel.RunAllTestsInOrg, TestLevel.RunLocalTests, TestLevel.RunSpecifiedTests],
       default: TestLevel.RunLocalTests,

src/utils/flags.ts

@@ -7,11 +7,12 @@
 import * as fs from 'fs';
 import { resolve, extname } from 'path';
 import { Flags } from '@oclif/core';
-import { Messages } from '@salesforce/core';
-import { PathInfo, TestLevel } from './types';
+import { Messages, Lifecycle } from '@salesforce/core';
+import { PathInfo, TestLevel, reportsFormatters } from './types';
 
 Messages.importMessagesDirectory(__dirname);
 const messages = Messages.loadMessages('@salesforce/plugin-deploy-retrieve', 'validation');
+const commonFlagMessages = Messages.loadMessages('@salesforce/plugin-deploy-retrieve', 'commonFlags');
 
 const parsePathInfo = async (input: string, opts: { exists?: boolean }): Promise<PathInfo> => {
   if (opts.exists && !fs.existsSync(input)) {
@@ -86,3 +87,41 @@ export const fileOrDirFlag = Flags.custom<PathInfo, { exists?: boolean }>({
 export const zipFileFlag = Flags.custom<string>({
   parse: async (input) => Promise.resolve(resolveZipFileName(input)),
 });
+
+export const testsFlag = Flags.string({
+  char: 't',
+  multiple: true,
+  summary: commonFlagMessages.getMessage('flags.tests.summary'),
+  description: commonFlagMessages.getMessage('flags.tests.description'),
+  // the old version allowed comma separated values, and the change is confusing enough to deserve a warning
+  parse: async (input: string): Promise<string> =>
+    commaWarningForMultipleFlags(
+      input,
+      commonFlagMessages.getMessage('commaWarningForTests', [commonFlagMessages.getMessage('flags.tests.description')])
+    ),
+});
+
+export const coverageFormattersFlag = Flags.string({
+  multiple: true,
+  summary: commonFlagMessages.getMessage('flags.coverage-formatters.summary'),
+  description: commonFlagMessages.getMessage('flags.coverage-formatters.description'),
+  parse: async (input: string): Promise<string> =>
+    commaWarningForMultipleFlags(
+      input,
+      commonFlagMessages.getMessage('commaWarningForCoverageFormatters', [
+        commonFlagMessages.getMessage('flags.coverage-formatters.description'),
+      ])
+    ),
+  options: reportsFormatters,
+});
+/**
+ * use when the old version allowed comma separated values, and the change is confusing enough to deserve a warning
+ * Put this as the parse function, like the testsFlag above
+ *
+ */
+export const commaWarningForMultipleFlags = async (input: string, warningText: string): Promise<string> => {
+  if (input.includes(',')) {
+    await Lifecycle.getInstance().emitWarning(warningText);
+  }
+  return input;
+};