From 3980c65785097a80bc84411162a3aadfb1388827 Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Mon, 3 Jun 2024 16:14:38 -0600 Subject: [PATCH 1/3] fix: remove uncaughtException listner / refactor messages file to match command name --- messages/{report.md => gettest.md} | 0 src/commands/apex/get/test.ts | 33 ++++++++------- src/commands/apex/run/test.ts | 66 +++++++++++++++--------------- 3 files changed, 48 insertions(+), 51 deletions(-) rename messages/{report.md => gettest.md} (100%) diff --git a/messages/report.md b/messages/gettest.md similarity index 100% rename from messages/report.md rename to messages/gettest.md diff --git a/src/commands/apex/get/test.ts b/src/commands/apex/get/test.ts index f3f53509..a8069ce9 100644 --- a/src/commands/apex/get/test.ts +++ b/src/commands/apex/get/test.ts @@ -19,7 +19,7 @@ import { RunResult, TestReporter } from '../../../reporters/index.js'; import { resultFormat } from '../../../utils.js'; Messages.importMessagesDirectoryFromMetaUrl(import.meta.url); -const messages = Messages.loadMessages('@salesforce/plugin-apex', 'report'); +const messages = Messages.loadMessages('@salesforce/plugin-apex', 'gettest'); export default class Test extends SfCommand { public static readonly summary = messages.getMessage('summary'); public static readonly description = messages.getMessage('description'); @@ -63,25 +63,24 @@ export default class Test extends SfCommand { }; public async run(): Promise { - const { flags } = await this.parse(Test); + try { + const { flags } = await this.parse(Test); - // add listener for errors - process.on('uncaughtException', (err) => { - throw messages.createError('apexLibErr', [err.message]); - }); + const conn = flags['target-org'].getConnection(flags['api-version']); - const conn = flags['target-org'].getConnection(flags['api-version']); + const testService = new TestService(conn); + const result = await testService.reportAsyncResults(flags['test-run-id'], flags['code-coverage']); - const testService = new TestService(conn); - const result = await testService.reportAsyncResults(flags['test-run-id'], flags['code-coverage']); + const testReporter = new TestReporter(new Ux({ jsonEnabled: this.jsonEnabled() }), conn, this.config.bin); - const testReporter = new TestReporter(new Ux({ jsonEnabled: this.jsonEnabled() }), conn, this.config.bin); - - return testReporter.report(result, { - 'output-dir': flags['output-dir'], - 'result-format': flags['result-format'], - json: flags.json, - 'code-coverage': flags['code-coverage'], - }); + return await testReporter.report(result, { + 'output-dir': flags['output-dir'], + 'result-format': flags['result-format'], + json: flags.json, + 'code-coverage': flags['code-coverage'], + }); + } catch (e) { + throw messages.createError('apexLibErr', [(e as Error).message]); + } } } diff --git a/src/commands/apex/run/test.ts b/src/commands/apex/run/test.ts index 8a0eda78..eccc0e95 100644 --- a/src/commands/apex/run/test.ts +++ b/src/commands/apex/run/test.ts @@ -17,7 +17,6 @@ import { } from '@salesforce/sf-plugins-core'; import { Messages, SfError } from '@salesforce/core'; import { Duration } from '@salesforce/kit'; - import { RunResult, TestReporter } from '../../../reporters/index.js'; import { resultFormat } from '../../../utils.js'; @@ -122,11 +121,6 @@ export default class Test extends SfCommand { flags['test-level'] as TestLevel ); - // add listener for errors - process.on('uncaughtException', (err) => { - throw messages.createError('apexLibErr', [err.message]); - }); - // graceful shutdown const exitHandler = async (): Promise => { await this.cancellationTokenSource.asyncCancel(); @@ -139,36 +133,40 @@ export default class Test extends SfCommand { process.on('SIGTERM', exitHandler); const conn = flags['target-org'].getConnection(flags['api-version']); - const testService = new TestService(conn); - - // NOTE: This is a *bug*. Synchronous test runs should throw an error when multiple test classes are specified - // This was re-introduced due to https://github.com/forcedotcom/salesforcedx-vscode/issues/3154 - // Address with W-9163533 - const result = - flags.synchronous && testLevel === TestLevel.RunSpecifiedTests - ? await this.runTest(testService, flags, testLevel) - : await this.runTestAsynchronous(testService, flags, testLevel); - - if (this.cancellationTokenSource.token.isCancellationRequested) { - throw new SfError('Cancelled'); - } - - if ('summary' in result) { - const testReporter = new TestReporter(new Ux({ jsonEnabled: this.jsonEnabled() }), conn, this.config.bin); - return testReporter.report(result, flags); - } else { - // Tests were ran asynchronously or the --wait timed out. - // Log the proper 'apex get test' command for the user to run later - this.log(messages.getMessage('runTestReportCommand', [this.config.bin, result.testRunId, conn.getUsername()])); - this.info(messages.getMessage('runTestSyncInstructions')); - - if (flags['output-dir']) { - // testService writes a file with just the test run id in it to test-run-id.txt - // github.com/forcedotcom/salesforcedx-apex/blob/c986abfabee3edf12f396f1d2e43720988fa3911/src/tests/testService.ts#L245-L246 - await testService.writeResultFiles(result, { dirPath: flags['output-dir'] }, flags['code-coverage']); + try { + const testService = new TestService(conn); + + // NOTE: This is a *bug*. Synchronous test runs should throw an error when multiple test classes are specified + // This was re-introduced due to https://github.com/forcedotcom/salesforcedx-vscode/issues/3154 + // Address with W-9163533 + const result = + flags.synchronous && testLevel === TestLevel.RunSpecifiedTests + ? await this.runTest(testService, flags, testLevel) + : await this.runTestAsynchronous(testService, flags, testLevel); + + if (this.cancellationTokenSource.token.isCancellationRequested) { + throw new SfError('Cancelled'); } - return result; + if ('summary' in result) { + const testReporter = new TestReporter(new Ux({ jsonEnabled: this.jsonEnabled() }), conn, this.config.bin); + return await testReporter.report(result, flags); + } else { + // Tests were ran asynchronously or the --wait timed out. + // Log the proper 'apex get test' command for the user to run later + this.log(messages.getMessage('runTestReportCommand', [this.config.bin, result.testRunId, conn.getUsername()])); + this.info(messages.getMessage('runTestSyncInstructions')); + + if (flags['output-dir']) { + // testService writes a file with just the test run id in it to test-run-id.txt + // github.com/forcedotcom/salesforcedx-apex/blob/c986abfabee3edf12f396f1d2e43720988fa3911/src/tests/testService.ts#L245-L246 + await testService.writeResultFiles(result, { dirPath: flags['output-dir'] }, flags['code-coverage']); + } + + return result; + } + } catch (e) { + throw messages.createError('apexLibErr', [(e as Error).message]); } } From b7796f34db1d78215eefd91825214b9131b63887 Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Tue, 4 Jun 2024 09:07:11 -0600 Subject: [PATCH 2/3] chore: update error handling to not overwrite our errors --- src/commands/apex/get/test.ts | 11 +++++++++-- src/commands/apex/run/test.ts | 9 ++++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/commands/apex/get/test.ts b/src/commands/apex/get/test.ts index a8069ce9..aea4a9e9 100644 --- a/src/commands/apex/get/test.ts +++ b/src/commands/apex/get/test.ts @@ -14,7 +14,7 @@ import { SfCommand, Ux, } from '@salesforce/sf-plugins-core'; -import { Messages } from '@salesforce/core'; +import { Messages, SfError } from '@salesforce/core'; import { RunResult, TestReporter } from '../../../reporters/index.js'; import { resultFormat } from '../../../utils.js'; @@ -80,7 +80,14 @@ export default class Test extends SfCommand { 'code-coverage': flags['code-coverage'], }); } catch (e) { - throw messages.createError('apexLibErr', [(e as Error).message]); + if (e instanceof SfError) { + // something we threw + throw e; + } else { + // what used to be caught by the + // process.on('uncaughtException', err => { + throw SfError.wrap(messages.createError('apexLibErr', [(e as Error).message])); + } } } } diff --git a/src/commands/apex/run/test.ts b/src/commands/apex/run/test.ts index eccc0e95..b091cfe7 100644 --- a/src/commands/apex/run/test.ts +++ b/src/commands/apex/run/test.ts @@ -166,7 +166,14 @@ export default class Test extends SfCommand { return result; } } catch (e) { - throw messages.createError('apexLibErr', [(e as Error).message]); + if (e instanceof SfError) { + // something we threw + throw e; + } else { + // what used to be caught by the + // process.on('uncaughtException', err => { + throw SfError.wrap(messages.createError('apexLibErr', [(e as Error).message])); + } } } From 8b002b7e9a4861ebc20efebccbfce178953d9289 Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Fri, 7 Jun 2024 10:21:31 -0600 Subject: [PATCH 3/3] chore: remove try/catch --- src/commands/apex/get/test.ts | 35 +++++++------------ src/commands/apex/run/test.ts | 65 +++++++++++++++-------------------- 2 files changed, 39 insertions(+), 61 deletions(-) diff --git a/src/commands/apex/get/test.ts b/src/commands/apex/get/test.ts index aea4a9e9..aef55c7d 100644 --- a/src/commands/apex/get/test.ts +++ b/src/commands/apex/get/test.ts @@ -14,7 +14,7 @@ import { SfCommand, Ux, } from '@salesforce/sf-plugins-core'; -import { Messages, SfError } from '@salesforce/core'; +import { Messages } from '@salesforce/core'; import { RunResult, TestReporter } from '../../../reporters/index.js'; import { resultFormat } from '../../../utils.js'; @@ -63,31 +63,20 @@ export default class Test extends SfCommand { }; public async run(): Promise { - try { - const { flags } = await this.parse(Test); + const { flags } = await this.parse(Test); - const conn = flags['target-org'].getConnection(flags['api-version']); + const conn = flags['target-org'].getConnection(flags['api-version']); - const testService = new TestService(conn); - const result = await testService.reportAsyncResults(flags['test-run-id'], flags['code-coverage']); + const testService = new TestService(conn); + const result = await testService.reportAsyncResults(flags['test-run-id'], flags['code-coverage']); - const testReporter = new TestReporter(new Ux({ jsonEnabled: this.jsonEnabled() }), conn, this.config.bin); + const testReporter = new TestReporter(new Ux({ jsonEnabled: this.jsonEnabled() }), conn, this.config.bin); - return await testReporter.report(result, { - 'output-dir': flags['output-dir'], - 'result-format': flags['result-format'], - json: flags.json, - 'code-coverage': flags['code-coverage'], - }); - } catch (e) { - if (e instanceof SfError) { - // something we threw - throw e; - } else { - // what used to be caught by the - // process.on('uncaughtException', err => { - throw SfError.wrap(messages.createError('apexLibErr', [(e as Error).message])); - } - } + return testReporter.report(result, { + 'output-dir': flags['output-dir'], + 'result-format': flags['result-format'], + json: flags.json, + 'code-coverage': flags['code-coverage'], + }); } } diff --git a/src/commands/apex/run/test.ts b/src/commands/apex/run/test.ts index b091cfe7..f707b48a 100644 --- a/src/commands/apex/run/test.ts +++ b/src/commands/apex/run/test.ts @@ -133,47 +133,36 @@ export default class Test extends SfCommand { process.on('SIGTERM', exitHandler); const conn = flags['target-org'].getConnection(flags['api-version']); - try { - const testService = new TestService(conn); - - // NOTE: This is a *bug*. Synchronous test runs should throw an error when multiple test classes are specified - // This was re-introduced due to https://github.com/forcedotcom/salesforcedx-vscode/issues/3154 - // Address with W-9163533 - const result = - flags.synchronous && testLevel === TestLevel.RunSpecifiedTests - ? await this.runTest(testService, flags, testLevel) - : await this.runTestAsynchronous(testService, flags, testLevel); + const testService = new TestService(conn); + + // NOTE: This is a *bug*. Synchronous test runs should throw an error when multiple test classes are specified + // This was re-introduced due to https://github.com/forcedotcom/salesforcedx-vscode/issues/3154 + // Address with W-9163533 + const result = + flags.synchronous && testLevel === TestLevel.RunSpecifiedTests + ? await this.runTest(testService, flags, testLevel) + : await this.runTestAsynchronous(testService, flags, testLevel); + + if (this.cancellationTokenSource.token.isCancellationRequested) { + throw new SfError('Cancelled'); + } - if (this.cancellationTokenSource.token.isCancellationRequested) { - throw new SfError('Cancelled'); + if ('summary' in result) { + const testReporter = new TestReporter(new Ux({ jsonEnabled: this.jsonEnabled() }), conn, this.config.bin); + return testReporter.report(result, flags); + } else { + // Tests were ran asynchronously or the --wait timed out. + // Log the proper 'apex get test' command for the user to run later + this.log(messages.getMessage('runTestReportCommand', [this.config.bin, result.testRunId, conn.getUsername()])); + this.info(messages.getMessage('runTestSyncInstructions')); + + if (flags['output-dir']) { + // testService writes a file with just the test run id in it to test-run-id.txt + // github.com/forcedotcom/salesforcedx-apex/blob/c986abfabee3edf12f396f1d2e43720988fa3911/src/tests/testService.ts#L245-L246 + await testService.writeResultFiles(result, { dirPath: flags['output-dir'] }, flags['code-coverage']); } - if ('summary' in result) { - const testReporter = new TestReporter(new Ux({ jsonEnabled: this.jsonEnabled() }), conn, this.config.bin); - return await testReporter.report(result, flags); - } else { - // Tests were ran asynchronously or the --wait timed out. - // Log the proper 'apex get test' command for the user to run later - this.log(messages.getMessage('runTestReportCommand', [this.config.bin, result.testRunId, conn.getUsername()])); - this.info(messages.getMessage('runTestSyncInstructions')); - - if (flags['output-dir']) { - // testService writes a file with just the test run id in it to test-run-id.txt - // github.com/forcedotcom/salesforcedx-apex/blob/c986abfabee3edf12f396f1d2e43720988fa3911/src/tests/testService.ts#L245-L246 - await testService.writeResultFiles(result, { dirPath: flags['output-dir'] }, flags['code-coverage']); - } - - return result; - } - } catch (e) { - if (e instanceof SfError) { - // something we threw - throw e; - } else { - // what used to be caught by the - // process.on('uncaughtException', err => { - throw SfError.wrap(messages.createError('apexLibErr', [(e as Error).message])); - } + return result; } }