From 6ad7f6c42d769dd00ecef616959084292cadf39d Mon Sep 17 00:00:00 2001 From: Chandler Anderson Date: Thu, 27 Mar 2025 07:13:36 -0400 Subject: [PATCH 1/6] fix: handle error for sync test runs & check for valid error before replacing --- src/commands/apex/run/test.ts | 38 ++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/src/commands/apex/run/test.ts b/src/commands/apex/run/test.ts index 0687ce59..d34c4de7 100644 --- a/src/commands/apex/run/test.ts +++ b/src/commands/apex/run/test.ts @@ -170,11 +170,16 @@ export default class Test extends SfCommand { ...(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; + + try { + return (await testService.runTestSynchronous( + payload, + flags['code-coverage'], + this.cancellationTokenSource.token + )) as TestResult; + } catch (e) { + throw handleTestError(SfError.wrap(e), flags); + } } private async runTestAsynchronous( @@ -212,17 +217,26 @@ export default class Test extends SfCommand { 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 handleTestError(SfError.wrap(e), flags); } } } +function handleTestError( + error: SfError, + flags: { + tests?: string[]; + 'class-names'?: string[]; + 'suite-names'?: string[]; + } +): SfError { + const hasTests = flags['class-names']?.length ?? flags['suite-names']?.length ?? flags.tests?.length; + if (hasTests && error.message.includes('Always provide a classes, suites, tests, or testLevel property')) { + error.message = 'There are no Apex tests to run in this org.'; + error.actions = ['Ensure Apex Tests exist in the org, and try again.']; + } + return error; +} const validateFlags = async ( classNames?: string[], suiteNames?: string[], From 16021a3d5b4b2b0c7fb61b504657a020e98f5957 Mon Sep 17 00:00:00 2001 From: Chandler Anderson Date: Thu, 27 Mar 2025 07:39:07 -0400 Subject: [PATCH 2/6] style: clarify flag and add line break after new function --- src/commands/apex/run/test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/commands/apex/run/test.ts b/src/commands/apex/run/test.ts index d34c4de7..b897ffe8 100644 --- a/src/commands/apex/run/test.ts +++ b/src/commands/apex/run/test.ts @@ -230,13 +230,17 @@ function handleTestError( 'suite-names'?: string[]; } ): SfError { - const hasTests = flags['class-names']?.length ?? flags['suite-names']?.length ?? flags.tests?.length; - if (hasTests && error.message.includes('Always provide a classes, suites, tests, or testLevel property')) { + const hasRequiredTestFlags = flags['class-names']?.length ?? flags['suite-names']?.length ?? flags.tests?.length; + if ( + hasRequiredTestFlags && + error.message.includes('Always provide a classes, suites, tests, or testLevel property') + ) { error.message = 'There are no Apex tests to run in this org.'; error.actions = ['Ensure Apex Tests exist in the org, and try again.']; } return error; } + const validateFlags = async ( classNames?: string[], suiteNames?: string[], From 6a745032362aca779180cf3ee5d8e7fea76fe2fa Mon Sep 17 00:00:00 2001 From: Chandler Anderson Date: Fri, 28 Mar 2025 16:53:13 -0400 Subject: [PATCH 3/6] fix: repair logic and add tests --- src/commands/apex/run/test.ts | 13 ++++++++----- test/commands/apex/run/test.test.ts | 26 +++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/src/commands/apex/run/test.ts b/src/commands/apex/run/test.ts index b897ffe8..af4e8823 100644 --- a/src/commands/apex/run/test.ts +++ b/src/commands/apex/run/test.ts @@ -230,11 +230,14 @@ function handleTestError( 'suite-names'?: string[]; } ): SfError { - const hasRequiredTestFlags = flags['class-names']?.length ?? flags['suite-names']?.length ?? flags.tests?.length; - if ( - hasRequiredTestFlags && - error.message.includes('Always provide a classes, suites, tests, or testLevel property') - ) { + const hasTestNames = !!flags.tests?.length; + const hasClassNames = !!flags['class-names']?.length; + const hasSuiteNames = !!flags['suite-names']?.length; + if (hasTestNames || hasClassNames || hasSuiteNames) { + return error; + } + + if (error.message.includes('Always provide a classes, suites, tests, or testLevel property')) { error.message = 'There are no Apex tests to run in this org.'; error.actions = ['Ensure Apex Tests exist in the org, and try again.']; } diff --git a/test/commands/apex/run/test.test.ts b/test/commands/apex/run/test.test.ts index 7d0a9702..e01027b5 100644 --- a/test/commands/apex/run/test.test.ts +++ b/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,29 @@ describe('apex:test:run', () => { expect(e.message).to.include('cannot also be provided when using'); } }); + + it('rejects when no Apex tests are in the org', async () => { + const testLevel = TestLevel.RunLocalTests.toString(); + const serverMessage = 'Always provide a classes, suites, tests, or testLevel property'; + const expectedMessage = 'There are no Apex tests to run in this org.'; + + sandbox.stub(TestService.prototype, 'runTestAsynchronous').throws({ message: serverMessage }); + try { + await Test.run(['--test-level', testLevel, '--concise']); + assert.fail('Unexpected successful outcome for async run.'); + } catch (e) { + assert(e instanceof Error); + expect(e.message).to.include(expectedMessage); + } + + sandbox.stub(TestService.prototype, 'runTestSynchronous').throws({ message: serverMessage }); + try { + await Test.run(['--test-level', testLevel, '--synchronous', '--concise']); + assert.fail('Unexpected successful outcome for sync run.'); + } catch (e) { + assert(e instanceof Error); + expect(e.message).to.include(expectedMessage); + } + }); }); }); From dfac9be7ac028e15ec33f0424655cf09ee9aed64 Mon Sep 17 00:00:00 2001 From: Chandler Anderson Date: Fri, 28 Mar 2025 17:52:24 -0400 Subject: [PATCH 4/6] fix: improve condition handling and tests, add comments --- src/commands/apex/run/test.ts | 27 ++++++++++++++++----------- test/commands/apex/run/test.test.ts | 23 ++++++++++++++++------- 2 files changed, 32 insertions(+), 18 deletions(-) diff --git a/src/commands/apex/run/test.ts b/src/commands/apex/run/test.ts index af4e8823..1d74c696 100644 --- a/src/commands/apex/run/test.ts +++ b/src/commands/apex/run/test.ts @@ -178,7 +178,7 @@ export default class Test extends SfCommand { this.cancellationTokenSource.token )) as TestResult; } catch (e) { - throw handleTestError(SfError.wrap(e), flags); + throw handleTestingServerError(SfError.wrap(e), flags); } } @@ -217,12 +217,12 @@ export default class Test extends SfCommand { flags.wait )) as TestRunIdResult; } catch (e) { - throw handleTestError(SfError.wrap(e), flags); + throw handleTestingServerError(SfError.wrap(e), flags); } } } -function handleTestError( +function handleTestingServerError( error: SfError, flags: { tests?: string[]; @@ -230,18 +230,23 @@ function handleTestError( 'suite-names'?: string[]; } ): SfError { - const hasTestNames = !!flags.tests?.length; - const hasClassNames = !!flags['class-names']?.length; - const hasSuiteNames = !!flags['suite-names']?.length; - if (hasTestNames || hasClassNames || hasSuiteNames) { + if (!error.message.includes('Always provide a classes, suites, tests, or testLevel property')) { return error; } - if (error.message.includes('Always provide a classes, suites, tests, or testLevel property')) { - error.message = 'There are no Apex tests to run in this org.'; - error.actions = ['Ensure Apex Tests exist in the org, and try again.']; + // If error message condition is valid, return the original error. + const hasNoTestNames = !!flags.tests?.length; + const hasNoClassNames = !!flags['class-names']?.length; + const hasNoSuiteNames = !!flags['suite-names']?.length; + if (hasNoTestNames && hasNoClassNames && hasNoSuiteNames) { + return error; } - 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 ( diff --git a/test/commands/apex/run/test.test.ts b/test/commands/apex/run/test.test.ts index e01027b5..e04b30f5 100644 --- a/test/commands/apex/run/test.test.ts +++ b/test/commands/apex/run/test.test.ts @@ -545,28 +545,37 @@ describe('apex:test:run', () => { } }); - it('rejects when no Apex tests are in the org', async () => { - const testLevel = TestLevel.RunLocalTests.toString(); - const serverMessage = 'Always provide a classes, suites, tests, or testLevel property'; + 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.'; - sandbox.stub(TestService.prototype, 'runTestAsynchronous').throws({ message: serverMessage }); + const runTestAsynchronousSpy = sandbox.stub(TestService.prototype, 'runTestAsynchronous').throws(serverError); try { - await Test.run(['--test-level', testLevel, '--concise']); + await Test.run(['--test-level', TestLevel.RunLocalTests.toString(), '--concise']); assert.fail('Unexpected successful outcome for async run.'); } catch (e) { assert(e instanceof Error); expect(e.message).to.include(expectedMessage); } + expect(runTestAsynchronousSpy.calledOnce).to.be.true; - sandbox.stub(TestService.prototype, 'runTestSynchronous').throws({ message: serverMessage }); + const runTestSynchronousSpy = sandbox.stub(TestService.prototype, 'runTestSynchronous').throws(serverError); try { - await Test.run(['--test-level', testLevel, '--synchronous', '--concise']); + await Test.run([ + '--test-level', + TestLevel.RunSpecifiedTests.toString(), + '--class-names', + 'myApex', + '--synchronous', + ]); assert.fail('Unexpected successful outcome for sync 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; }); }); }); From 0f5e800d6dc002871194e010c536cf2a6dca6377 Mon Sep 17 00:00:00 2001 From: Chandler Anderson Date: Fri, 28 Mar 2025 18:08:50 -0400 Subject: [PATCH 5/6] fix: include test level and fix flagging logic --- src/commands/apex/run/test.ts | 16 +++++++++------- test/commands/apex/run/test.test.ts | 20 ++++++++++++++++---- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/src/commands/apex/run/test.ts b/src/commands/apex/run/test.ts index 1d74c696..c54a73bc 100644 --- a/src/commands/apex/run/test.ts +++ b/src/commands/apex/run/test.ts @@ -178,7 +178,7 @@ export default class Test extends SfCommand { this.cancellationTokenSource.token )) as TestResult; } catch (e) { - throw handleTestingServerError(SfError.wrap(e), flags); + throw handleTestingServerError(SfError.wrap(e), flags, testLevel); } } @@ -217,7 +217,7 @@ export default class Test extends SfCommand { flags.wait )) as TestRunIdResult; } catch (e) { - throw handleTestingServerError(SfError.wrap(e), flags); + throw handleTestingServerError(SfError.wrap(e), flags, testLevel); } } } @@ -228,17 +228,19 @@ function handleTestingServerError( 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 hasNoTestNames = !!flags.tests?.length; - const hasNoClassNames = !!flags['class-names']?.length; - const hasNoSuiteNames = !!flags['suite-names']?.length; - if (hasNoTestNames && hasNoClassNames && hasNoSuiteNames) { + 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; } diff --git a/test/commands/apex/run/test.test.ts b/test/commands/apex/run/test.test.ts index e04b30f5..e41b0a8b 100644 --- a/test/commands/apex/run/test.test.ts +++ b/test/commands/apex/run/test.test.ts @@ -551,12 +551,13 @@ describe('apex:test:run', () => { const runTestAsynchronousSpy = sandbox.stub(TestService.prototype, 'runTestAsynchronous').throws(serverError); try { - await Test.run(['--test-level', TestLevel.RunLocalTests.toString(), '--concise']); - assert.fail('Unexpected successful outcome for async run.'); + await Test.run(['--test-level', TestLevel.RunSpecifiedTests.toString()]); + assert.fail('Unexpected successful outcome for async local test run.'); } catch (e) { assert(e instanceof Error); - expect(e.message).to.include(expectedMessage); + expect(e.message).to.include(serverError.message); } + expect(runTestAsynchronousSpy.calledOnce).to.be.true; const runTestSynchronousSpy = sandbox.stub(TestService.prototype, 'runTestSynchronous').throws(serverError); @@ -568,7 +569,7 @@ describe('apex:test:run', () => { 'myApex', '--synchronous', ]); - assert.fail('Unexpected successful outcome for sync run.'); + assert.fail('Unexpected successful outcome for sync specified tests run.'); } catch (e) { assert(e instanceof Error); expect(e.message).to.include(expectedMessage); @@ -576,6 +577,17 @@ describe('apex:test:run', () => { 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); }); }); }); From 7e9b2accd841bbecee6e99697cdf849ac78c0dc4 Mon Sep 17 00:00:00 2001 From: Chandler Anderson <18584424+zenibako@users.noreply.github.com> Date: Fri, 28 Mar 2025 18:24:56 -0400 Subject: [PATCH 6/6] chore: update assert.fail message --- test/commands/apex/run/test.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/commands/apex/run/test.test.ts b/test/commands/apex/run/test.test.ts index e41b0a8b..5a09a42c 100644 --- a/test/commands/apex/run/test.test.ts +++ b/test/commands/apex/run/test.test.ts @@ -552,7 +552,7 @@ describe('apex:test:run', () => { 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 local test run.'); + 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);