fix: handle error for sync test runs & check for valid error first (#710)

* fix: handle error for sync test runs & check for valid error before replacing

* style: clarify flag and add line break after new function

* fix: repair logic and add tests

* fix: improve condition handling and tests, add comments

* fix: include test level and fix flagging logic

* chore: update assert.fail message

Author: zenibako

src/commands/apex/run/test.ts

@@ -170,11 +170,16 @@ export default class Test extends SfCommand<RunCommandResult> {
       ...(await testService.buildSyncPayload(testLevel, flags.tests?.join(','), flags['class-names']?.join(','))),
       skipCodeCoverage: !flags['code-coverage'],
     };
-    return testService.runTestSynchronous(
-      payload,
-      flags['code-coverage'],
-      this.cancellationTokenSource.token
-    ) as Promise<TestResult>;
+
+    try {
+      return (await testService.runTestSynchronous(
+        payload,
+        flags['code-coverage'],
+        this.cancellationTokenSource.token
+      )) as TestResult;
+    } catch (e) {
+      throw handleTestingServerError(SfError.wrap(e), flags, testLevel);
+    }
   }
 
   private async runTestAsynchronous(
@@ -212,17 +217,40 @@ export default class Test extends SfCommand<RunCommandResult> {
         flags.wait
       )) as TestRunIdResult;
     } catch (e) {
-      const error = SfError.wrap(e);
-      if (error.message.includes('Always provide a classes, suites, tests, or testLevel property')) {
-        error.message = 'There are no apex tests to run in the org';
-        error.actions = ['Ensure Apex Tests exist in the org'];
-      }
-
-      throw error;
+      throw handleTestingServerError(SfError.wrap(e), flags, testLevel);
     }
   }
 }
 
+function handleTestingServerError(
+  error: SfError,
+  flags: {
+    tests?: string[];
+    'class-names'?: string[];
+    'suite-names'?: string[];
+  },
+  testLevel: TestLevel
+): SfError {
+  if (!error.message.includes('Always provide a classes, suites, tests, or testLevel property')) {
+    return error;
+  }
+
+  // If error message condition is valid, return the original error.
+  const hasSpecifiedTestLevel = testLevel === TestLevel.RunSpecifiedTests;
+  const hasNoTestNames = !flags.tests?.length;
+  const hasNoClassNames = !flags['class-names']?.length;
+  const hasNoSuiteNames = !flags['suite-names']?.length;
+  if (hasSpecifiedTestLevel && hasNoTestNames && hasNoClassNames && hasNoSuiteNames) {
+    return error;
+  }
+
+  // Otherwise, assume there are no Apex tests in the org and return clearer message.
+  return Object.assign(error, {
+    message: 'There are no Apex tests to run in this org.',
+    actions: ['Ensure Apex Tests exist in the org, and try again.'],
+  });
+}
+
 const validateFlags = async (
   classNames?: string[],
   suiteNames?: string[],

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

@@ -9,7 +9,7 @@ import { Messages, Org } from '@salesforce/core';
 import sinon from 'sinon';
 import { Ux, stubSfCommandUx } from '@salesforce/sf-plugins-core';
 import { assert, expect } from 'chai';
-import { TestService } from '@salesforce/apex-node';
+import { TestLevel, TestService } from '@salesforce/apex-node';
 import Test from '../../../../src/commands/apex/run/test.js';
 import {
   runWithCoverage,
@@ -544,5 +544,50 @@ describe('apex:test:run', () => {
         expect(e.message).to.include('cannot also be provided when using');
       }
     });
+
+    it('rejects no Apex tests in the org', async () => {
+      const serverError = { message: 'Always provide a classes, suites, tests, or testLevel property' };
+      const expectedMessage = 'There are no Apex tests to run in this org.';
+
+      const runTestAsynchronousSpy = sandbox.stub(TestService.prototype, 'runTestAsynchronous').throws(serverError);
+      try {
+        await Test.run(['--test-level', TestLevel.RunSpecifiedTests.toString()]);
+        assert.fail('Unexpected successful outcome for async specified test run without tests.');
+      } catch (e) {
+        assert(e instanceof Error);
+        expect(e.message).to.include(serverError.message);
+      }
+
+      expect(runTestAsynchronousSpy.calledOnce).to.be.true;
+
+      const runTestSynchronousSpy = sandbox.stub(TestService.prototype, 'runTestSynchronous').throws(serverError);
+      try {
+        await Test.run([
+          '--test-level',
+          TestLevel.RunSpecifiedTests.toString(),
+          '--class-names',
+          'myApex',
+          '--synchronous',
+        ]);
+        assert.fail('Unexpected successful outcome for sync specified tests run.');
+      } catch (e) {
+        assert(e instanceof Error);
+        expect(e.message).to.include(expectedMessage);
+      }
+
+      expect(runTestSynchronousSpy.calledOnce).to.be.true;
+      expect(runTestAsynchronousSpy.calledOnce).to.be.true;
+
+      try {
+        await Test.run(['--test-level', TestLevel.RunLocalTests.toString(), '--synchronous']);
+        assert.fail('Unexpected successful outcome for sync local test run.');
+      } catch (e) {
+        assert(e instanceof Error);
+        expect(e.message).to.include(expectedMessage);
+      }
+
+      expect(runTestSynchronousSpy.calledOnce).to.be.true;
+      expect(runTestAsynchronousSpy.callCount).to.equal(2);
+    });
   });
 });