feat: use deprecatedArray for consistency in sf commands and compatibility

Author: mshanemc

messages/runtest.md

@@ -20,15 +20,15 @@ NOTE: The testRunCoverage value (JSON and JUnit result formats) is a percentage
 
 - Run the specified Apex test classes in your default org and display results in human-readable form:
 
-  <%= config.bin %> <%= command.id %> --class-names "MyClassTest,MyOtherClassTest" --result-format human
+  <%= config.bin %> <%= command.id %> --class-names MyClassTest --class-names MyOtherClassTest --result-format human
 
 - Run the specified Apex test suites in your default org and include code coverage results and additional details:
 
-  <%= config.bin %> <%= command.id %> --suite-names "MySuite,MyOtherSuite" --code-coverage --detailed-coverage
+  <%= config.bin %> <%= command.id %> --suite-names MySuite --suite-names MyOtherSuite --code-coverage --detailed-coverage
 
 - Run the specified Apex tests in your default org and display results in human-readable output:
 
-  <%= config.bin %> <%= command.id %> --tests "MyClassTest.testCoolFeature,MyClassTest.testAwesomeFeature,AnotherClassTest,namespace.TheirClassTest.testThis" --result-format human
+  <%= config.bin %> <%= command.id %> --tests MyClassTest.testCoolFeature --tests MyClassTest.testAwesomeFeature --tests AnotherClassTest --tests namespace.TheirClassTest.testThis --result-format human
 
 - Run all tests in the org with the specified username with the specified test level; save the output to the specified directory:
 
@@ -40,27 +40,33 @@ Format of the test results.
 
 # flags.class-names.summary
 
-Comma-separated list of Apex test class names to run; default is all classes.
+Apex test class names to run; default is all classes.
 
 # flags.class-names.description
 
 If you select --class-names, you can't specify --suite-names or --tests.
+For multiple classes, repeat the flag for each.
+--class-names Class1 --class-names Class2
 
 # flags.suite-names.summary
 
-Comma-separated list of Apex test suite names to run; default is all suites.
+Apex test suite names to run; default is all suites.
 
 # flags.suite-names.description
 
 If you select --suite-names, you can't specify --class-names or --tests.
+For multiple suites, repeat the flag for each.
+--suite-names Suite1 --suite-names Suite2
 
 # flags.tests.summary
 
-Comma-separated list of Apex test class names or IDs and, if applicable, test methods to run; default is all tests.
+Apex test class names or IDs and, if applicable, test methods to run; default is all tests.
 
 # flags.tests.description
 
 If you specify --tests, you can't specify --class-names or --suite-names
+For multiple tests, repeat the flag for each.
+--tests Test1 --tests Test2
 
 # flags.code-coverage.summary
 
@@ -98,10 +104,6 @@ Display detailed code coverage per test.
 
 Run "%s apex get test -i %s -o %s" to retrieve test results
 
-# classSuiteTestErr
-
-Specify either classnames, suitenames, or tests
-
 # syncClassErr
 
 Synchronous test runs can include test methods from only one Apex class. Omit the --synchronous flag or include tests from only one class

src/commands/apex/run.ts

@@ -45,6 +45,7 @@ export default class Run extends SfCommand<ExecuteResult> {
       aliases: ['apexcodefile'],
       char: 'f',
       summary: messages.getMessage('flags.file'),
+      exists: true,
     }),
   };
 

src/commands/apex/run/test.ts

@@ -6,6 +6,7 @@
  */
 import { CancellationTokenSource, TestLevel, TestResult, TestRunIdResult, TestService } from '@salesforce/apex-node';
 import {
+  arrayWithDeprecation,
   Flags,
   loglevel,
   orgApiVersionFlagWithDeprecations,
@@ -24,6 +25,7 @@ const messages = Messages.loadMessages('@salesforce/plugin-apex', 'runtest');
 
 export const TestLevelValues = ['RunLocalTests', 'RunAllTestsInOrg', 'RunSpecifiedTests'];
 export type RunCommandResult = RunResult | TestRunIdResult;
+const exclusiveTestSpecifiers = ['class-names', 'suite-names', 'tests'];
 export default class Test extends SfCommand<RunCommandResult> {
   public static readonly summary = messages.getMessage('summary');
   public static readonly description = messages.getMessage('description');
@@ -55,12 +57,13 @@ export default class Test extends SfCommand<RunCommandResult> {
       description: messages.getMessage('flags.test-level.description'),
       options: TestLevelValues,
     }),
-    'class-names': Flags.string({
+    'class-names': arrayWithDeprecation({
       deprecateAliases: true,
       aliases: ['classnames'],
       char: 'n',
       summary: messages.getMessage('flags.class-names.summary'),
       description: messages.getMessage('flags.class-names.description'),
+      exclusive: exclusiveTestSpecifiers.filter((specifier) => specifier !== 'class-names'),
     }),
     'result-format': Flags.string({
       deprecateAliases: true,
@@ -70,17 +73,19 @@ export default class Test extends SfCommand<RunCommandResult> {
       options: resultFormat,
       default: 'human',
     }),
-    'suite-names': Flags.string({
+    'suite-names': arrayWithDeprecation({
       deprecateAliases: true,
       aliases: ['suitenames'],
       char: 's',
       summary: messages.getMessage('flags.suite-names.summary'),
       description: messages.getMessage('flags.suite-names.description'),
+      exclusive: exclusiveTestSpecifiers.filter((specifier) => specifier !== 'suite-names'),
     }),
-    tests: Flags.string({
+    tests: arrayWithDeprecation({
       char: 't',
       summary: messages.getMessage('flags.tests.summary'),
       description: messages.getMessage('flags.tests.description'),
+      exclusive: exclusiveTestSpecifiers.filter((specifier) => specifier !== 'tests'),
     }),
     // we want to pass `undefined` to the API
     // eslint-disable-next-line sf-plugin/flag-min-max-default
@@ -108,9 +113,7 @@ export default class Test extends SfCommand<RunCommandResult> {
   public async run(): Promise<RunCommandResult> {
     const { flags } = await this.parse(Test);
 
-    const testLevel = await this.validateFlags(
-      flags['code-coverage'],
-      flags['result-format'],
+    const testLevel = await validateFlags(
       flags['class-names'],
       flags['suite-names'],
       flags.tests,
@@ -159,48 +162,17 @@ export default class Test extends SfCommand<RunCommandResult> {
     }
   }
 
-  // eslint-disable-next-line class-methods-use-this
-  public async validateFlags(
-    codeCoverage?: boolean,
-    resultFormatFlag?: string,
-    classNames?: string,
-    suiteNames?: string,
-    tests?: string,
-    synchronous?: boolean,
-    testLevel?: TestLevel
-  ): Promise<TestLevel> {
-    if ((classNames && (suiteNames || tests)) || (suiteNames && tests)) {
-      return Promise.reject(new Error(messages.getMessage('classSuiteTestErr')));
-    }
-
-    if (synchronous && (suiteNames || (classNames && classNames.split(',').length > 1))) {
-      return Promise.reject(new Error(messages.getMessage('syncClassErr')));
-    }
-
-    if ((tests || classNames || suiteNames) && testLevel && testLevel !== 'RunSpecifiedTests') {
-      return Promise.reject(new Error(messages.getMessage('testLevelErr')));
-    }
-
-    if (testLevel) {
-      return testLevel;
-    }
-    if (classNames || suiteNames || tests) {
-      return TestLevel.RunSpecifiedTests;
-    }
-    return TestLevel.RunLocalTests;
-  }
-
   private async runTest(
     testService: TestService,
     flags: {
-      tests?: string;
-      'class-names'?: string;
+      tests?: string[];
+      'class-names'?: string[];
       'code-coverage'?: boolean;
     },
     testLevel: TestLevel
   ): Promise<TestResult> {
     const payload = {
-      ...(await testService.buildSyncPayload(testLevel, flags.tests, flags['class-names'])),
+      ...(await testService.buildSyncPayload(testLevel, flags.tests?.join(','), flags['class-names']?.join(','))),
       skipCodeCoverage: !flags['code-coverage'],
     };
     return testService.runTestSynchronous(
@@ -213,9 +185,9 @@ export default class Test extends SfCommand<RunCommandResult> {
   private async runTestAsynchronous(
     testService: TestService,
     flags: {
-      tests?: string;
-      'class-names'?: string;
-      'suite-names'?: string;
+      tests?: string[];
+      'class-names'?: string[];
+      'suite-names'?: string[];
       'code-coverage'?: boolean;
       synchronous?: boolean;
       'result-format'?: string;
@@ -225,7 +197,12 @@ export default class Test extends SfCommand<RunCommandResult> {
     testLevel: TestLevel
   ): Promise<TestRunIdResult> {
     const payload = {
-      ...(await testService.buildAsyncPayload(testLevel, flags.tests, flags['class-names'], flags['suite-names'])),
+      ...(await testService.buildAsyncPayload(
+        testLevel,
+        flags.tests?.join(','),
+        flags['class-names']?.join(','),
+        flags['suite-names']?.join(',')
+      )),
       skipCodeCoverage: !flags['code-coverage'],
     };
 
@@ -239,3 +216,28 @@ export default class Test extends SfCommand<RunCommandResult> {
     ) as Promise<TestRunIdResult>;
   }
 }
+
+// eslint-disable-next-line class-methods-use-this
+const validateFlags = async (
+  classNames?: string[],
+  suiteNames?: string[],
+  tests?: string[],
+  synchronous?: boolean,
+  testLevel?: TestLevel
+): Promise<TestLevel> => {
+  if (synchronous && (suiteNames || (classNames?.length && classNames.length > 1))) {
+    return Promise.reject(new Error(messages.getMessage('syncClassErr')));
+  }
+
+  if ((tests || classNames || suiteNames) && testLevel && testLevel !== 'RunSpecifiedTests') {
+    return Promise.reject(new Error(messages.getMessage('testLevelErr')));
+  }
+
+  if (testLevel) {
+    return testLevel;
+  }
+  if (classNames || suiteNames || tests) {
+    return TestLevel.RunSpecifiedTests;
+  }
+  return TestLevel.RunLocalTests;
+};

test/commands/apex/run/test.test.ts

@@ -10,7 +10,7 @@ import { Messages, Org } from '@salesforce/core';
 import { createSandbox, SinonSandbox } from 'sinon';
 import { Ux } from '@salesforce/sf-plugins-core';
 import { Config } from '@oclif/core';
-import { expect } from 'chai';
+import { assert, expect } from 'chai';
 import { TestService } from '@salesforce/apex-node';
 import Test from '../../../../src/commands/apex/run/test';
 import {
@@ -401,22 +401,26 @@ describe('apex:test:run', () => {
     });
 
     it('rejects classname/suitnames/test variations', async () => {
+      // uses oclif exclusive now
       try {
         await new Test(['--class-names', 'myApex', '--suite-names', 'testsuite'], config).run();
       } catch (e) {
-        expect((e as Error).message).to.equal(messages.getMessage('classSuiteTestErr'));
+        assert(e instanceof Error);
+        expect(e.message).to.include('cannot also be provided when using');
       }
 
       try {
         await new Test(['--class-names', 'myApex', '--tests', 'testsuite'], config).run();
       } catch (e) {
-        expect((e as Error).message).to.equal(messages.getMessage('classSuiteTestErr'));
+        assert(e instanceof Error);
+        expect(e.message).to.include('cannot also be provided when using');
       }
 
       try {
         await new Test(['--suite-names', 'myApex', '--tests', 'testsuite'], config).run();
       } catch (e) {
-        expect((e as Error).message).to.equal(messages.getMessage('classSuiteTestErr'));
+        assert(e instanceof Error);
+        expect(e.message).to.include('cannot also be provided when using');
       }
     });
   });