From ff776dca15b1ac96e44d25c964e973a792944a0d Mon Sep 17 00:00:00 2001 From: mingxuanzhang Date: Thu, 14 Mar 2024 11:00:53 -0700 Subject: [PATCH 1/7] chore: add error handling in the bottlenecks --- src/i18n/i18n.ts | 2 + src/tests/testService.ts | 134 +++++++++++++++++++++++++-------------- 2 files changed, 88 insertions(+), 48 deletions(-) diff --git a/src/i18n/i18n.ts b/src/i18n/i18n.ts index 0905d220..a2c3a529 100644 --- a/src/i18n/i18n.ts +++ b/src/i18n/i18n.ts @@ -80,6 +80,8 @@ export const messages = { 'Must provide a suite name or suite id to retrieve test classes in suite', missingSuiteErr: 'Suite does not exist', missingTestClassErr: 'Apex class %s does not exist in the org', + jsonStringifyErr: + 'The test result in the format of %s is too large to be stringified. Please try to run fewer tests at a time.', testSuiteMsg: 'Apex test class %s already exists in Apex test suite %s', classSuiteMsg: `Added Apex class %s to your Apex test suite %s`, error_no_default_username: diff --git a/src/tests/testService.ts b/src/tests/testService.ts index 2f6e5d63..61d2b438 100644 --- a/src/tests/testService.ts +++ b/src/tests/testService.ts @@ -186,7 +186,11 @@ export class TestService { codeCoverage = false, token?: CancellationToken ): Promise { - return await this.syncService.runTests(options, codeCoverage, token); + try { + return await this.syncService.runTests(options, codeCoverage, token); + } catch (error) { + throw error; + } } /** @@ -205,13 +209,17 @@ export class TestService { progress?: Progress, token?: CancellationToken ): Promise { - return await this.asyncService.runTests( - options, - codeCoverage, - immediatelyReturn, - progress, - token - ); + try { + return await this.asyncService.runTests( + options, + codeCoverage, + immediatelyReturn, + progress, + token + ); + } catch (error) { + throw error; + } } /** @@ -226,11 +234,15 @@ export class TestService { codeCoverage = false, token?: CancellationToken ): Promise { - return await this.asyncService.reportAsyncResults( - testRunId, - codeCoverage, - token - ); + try { + return await this.asyncService.reportAsyncResults( + testRunId, + codeCoverage, + token + ); + } catch (error) { + throw error; + } } /** @@ -270,32 +282,46 @@ export class TestService { switch (format) { case ResultFormat.json: - fileMap.push({ - path: join( - dirPath, - testRunId ? `test-result-${testRunId}.json` : `test-result.json` - ), - content: stringify(result) - }); - break; + try { + fileMap.push({ + path: join( + dirPath, + testRunId + ? `test-result-${testRunId}.json` + : `test-result.json` + ), + content: stringify(result) + }); + break; + } catch (error) { + throw new Error(nls.localize('jsonStringifyErr', format)); + } case ResultFormat.tap: - const tapResult = new TapReporter().format(result); - fileMap.push({ - path: join(dirPath, `test-result-${testRunId}-tap.txt`), - content: tapResult - }); - break; + try { + const tapResult = new TapReporter().format(result); + fileMap.push({ + path: join(dirPath, `test-result-${testRunId}-tap.txt`), + content: tapResult + }); + break; + } catch (error) { + throw new Error(nls.localize('jsonStringifyErr', format)); + } case ResultFormat.junit: - const junitResult = new JUnitReporter().format(result); - fileMap.push({ - path: join( - dirPath, - testRunId - ? `test-result-${testRunId}-junit.xml` - : `test-result-junit.xml` - ), - content: junitResult - }); + try { + const junitResult = new JUnitReporter().format(result); + fileMap.push({ + path: join( + dirPath, + testRunId + ? `test-result-${testRunId}-junit.xml` + : `test-result-junit.xml` + ), + content: junitResult + }); + } catch (error) { + throw new Error(nls.localize('jsonStringifyErr', format)); + } break; } } @@ -309,20 +335,32 @@ export class TestService { const coverageRecords = result.tests.map((record) => { return record.perClassCoverage; }); - fileMap.push({ - path: join(dirPath, `test-result-${testRunId}-codecoverage.json`), - content: stringify(coverageRecords) - }); + + try { + const content = stringify(coverageRecords); + fileMap.push({ + path: join(dirPath, `test-result-${testRunId}-codecoverage.json`), + content + }); + } catch (error) { + throw new error( + nls.localize('jsonStringifyErr', 'code coverage records') + ); + } } fileInfos?.forEach((fileInfo) => { - fileMap.push({ - path: join(dirPath, fileInfo.filename), - content: - typeof fileInfo.content !== 'string' - ? stringify(fileInfo.content) - : fileInfo.content - }); + try { + fileMap.push({ + path: join(dirPath, fileInfo.filename), + content: + typeof fileInfo.content !== 'string' + ? stringify(fileInfo.content) + : fileInfo.content + }); + } catch (error) { + throw new error(nls.localize('jsonStringifyErr', 'fileInfos')); + } }); createFiles(fileMap); From 6dd4ce5d2532530d91246cf578d74880af40de4f Mon Sep 17 00:00:00 2001 From: mingxuanzhang Date: Thu, 21 Mar 2024 11:19:28 -0700 Subject: [PATCH 2/7] chore: catch exception in runAsyncTest workflow --- src/i18n/i18n.ts | 2 + src/streaming/streamingClient.ts | 17 +++-- src/tests/asyncTests.ts | 116 +++++++++++++++++-------------- src/tests/codeCoverage.ts | 85 ++++++++++++---------- 4 files changed, 126 insertions(+), 94 deletions(-) diff --git a/src/i18n/i18n.ts b/src/i18n/i18n.ts index a2c3a529..05384005 100644 --- a/src/i18n/i18n.ts +++ b/src/i18n/i18n.ts @@ -82,6 +82,8 @@ export const messages = { missingTestClassErr: 'Apex class %s does not exist in the org', jsonStringifyErr: 'The test result in the format of %s is too large to be stringified. Please try to run fewer tests at a time.', + largeTestResultErr: + 'The test result in the format of %s is too large to be stored in the heap. Please try to run fewer tests at a time.', testSuiteMsg: 'Apex test class %s already exists in Apex test suite %s', classSuiteMsg: `Added Apex class %s to your Apex test suite %s`, error_no_default_username: diff --git a/src/streaming/streamingClient.ts b/src/streaming/streamingClient.ts index bf6ebf98..0575b658 100644 --- a/src/streaming/streamingClient.ts +++ b/src/streaming/streamingClient.ts @@ -280,12 +280,17 @@ export class StreamingClient { testRunId: string ): Promise { const queryApexTestQueueItem = `SELECT Id, Status, ApexClassId, TestRunResultId FROM ApexTestQueueItem WHERE ParentJobId = '${testRunId}'`; - const result = await this.conn.tooling.query( - queryApexTestQueueItem, - { - autoFetch: true - } - ); + let result; + try { + result = await this.conn.tooling.query( + queryApexTestQueueItem, + { + autoFetch: true + } + ); + } catch (error) { + throw new Error(nls.localize('largeTestResultErr', 'ApexTestQueueItem')); + } if (result.records.length === 0) { throw new Error(nls.localize('noTestQueueResults', testRunId)); diff --git a/src/tests/asyncTests.ts b/src/tests/asyncTests.ts index 59aaacf0..e66c346a 100644 --- a/src/tests/asyncTests.ts +++ b/src/tests/asyncTests.ts @@ -314,8 +314,12 @@ export class AsyncTests { const queryPromises = queries.map((query) => { return queryAll(this.connection, query, true); }); - const apexTestResults = await Promise.all(queryPromises); - return apexTestResults as ApexTestResult[]; + try { + const apexTestResults = await Promise.all(queryPromises); + return apexTestResults as ApexTestResult[]; + } catch (error) { + throw new Error(nls.localize('largeTestResultErr', 'ApexTestResult[]')); + } } @elapsedTime() @@ -336,59 +340,65 @@ export class AsyncTests { let skipped = 0; // Iterate over test results, format and add them as results.tests - const testResults: ApexTestResultData[] = []; - for (const result of apexTestResults) { - result.records.forEach((item) => { - switch (item.Outcome) { - case ApexTestResultOutcome.Pass: - passed++; - break; - case ApexTestResultOutcome.Fail: - case ApexTestResultOutcome.CompileFail: - failed++; - break; - case ApexTestResultOutcome.Skip: - skipped++; - break; - } - - apexTestClassIdSet.add(item.ApexClass.Id); - // Can only query the FullName field if a single record is returned, so manually build the field - item.ApexClass.FullName = item.ApexClass.NamespacePrefix - ? `${item.ApexClass.NamespacePrefix}.${item.ApexClass.Name}` - : item.ApexClass.Name; - - const diagnostic = - item.Message || item.StackTrace ? getAsyncDiagnostic(item) : null; - - testResults.push({ - id: item.Id, - queueItemId: item.QueueItemId, - stackTrace: item.StackTrace, - message: item.Message, - asyncApexJobId: item.AsyncApexJobId, - methodName: item.MethodName, - outcome: item.Outcome, - apexLogId: item.ApexLogId, - apexClass: { - id: item.ApexClass.Id, - name: item.ApexClass.Name, - namespacePrefix: item.ApexClass.NamespacePrefix, - fullName: item.ApexClass.FullName - }, - runTime: item.RunTime ?? 0, - testTimestamp: item.TestTimestamp, // TODO: convert timestamp - fullName: `${item.ApexClass.FullName}.${item.MethodName}`, - ...(diagnostic ? { diagnostic } : {}) + try { + const testResults: ApexTestResultData[] = []; + for (const result of apexTestResults) { + result.records.forEach((item) => { + switch (item.Outcome) { + case ApexTestResultOutcome.Pass: + passed++; + break; + case ApexTestResultOutcome.Fail: + case ApexTestResultOutcome.CompileFail: + failed++; + break; + case ApexTestResultOutcome.Skip: + skipped++; + break; + } + + apexTestClassIdSet.add(item.ApexClass.Id); + // Can only query the FullName field if a single record is returned, so manually build the field + item.ApexClass.FullName = item.ApexClass.NamespacePrefix + ? `${item.ApexClass.NamespacePrefix}.${item.ApexClass.Name}` + : item.ApexClass.Name; + + const diagnostic = + item.Message || item.StackTrace ? getAsyncDiagnostic(item) : null; + + testResults.push({ + id: item.Id, + queueItemId: item.QueueItemId, + stackTrace: item.StackTrace, + message: item.Message, + asyncApexJobId: item.AsyncApexJobId, + methodName: item.MethodName, + outcome: item.Outcome, + apexLogId: item.ApexLogId, + apexClass: { + id: item.ApexClass.Id, + name: item.ApexClass.Name, + namespacePrefix: item.ApexClass.NamespacePrefix, + fullName: item.ApexClass.FullName + }, + runTime: item.RunTime ?? 0, + testTimestamp: item.TestTimestamp, // TODO: convert timestamp + fullName: `${item.ApexClass.FullName}.${item.MethodName}`, + ...(diagnostic ? { diagnostic } : {}) + }); }); - }); - } + } - return { - apexTestClassIdSet, - testResults, - globalTests: { passed, failed, skipped } - }; + return { + apexTestClassIdSet, + testResults, + globalTests: { passed, failed, skipped } + }; + } catch (error) { + throw new Error( + nls.localize('largeTestResultErr', 'ApexTestResultData[]') + ); + } } /** diff --git a/src/tests/codeCoverage.ts b/src/tests/codeCoverage.ts index 2411bb47..242f31a2 100644 --- a/src/tests/codeCoverage.ts +++ b/src/tests/codeCoverage.ts @@ -17,6 +17,7 @@ import * as util from 'util'; import { calculatePercentage, queryAll } from './utils'; import { QUERY_RECORD_LIMIT } from './constants'; import { elapsedTime } from '../utils/elapsedTime'; +import { nls } from '../i18n'; export class CodeCoverage { public readonly connection: Connection; @@ -54,43 +55,52 @@ export class CodeCoverage { if (apexTestClassSet.size === 0) { return new Map(); } - - const perClassCodeCovResults = - await this.queryPerClassCodeCov(apexTestClassSet); + let perClassCodeCovResults; + try { + perClassCodeCovResults = + await this.queryPerClassCodeCov(apexTestClassSet); + } catch (error) { + throw new Error(nls.localize('largeTestResultErr', 'ApexCodeCoverage[]')); + } const perClassCoverageMap = new Map(); - perClassCodeCovResults.forEach((chunk) => { - chunk.records.forEach((item) => { - const totalLines = item.NumLinesCovered + item.NumLinesUncovered; - const percentage = calculatePercentage( - item.NumLinesCovered, - totalLines - ); - - const value = { - apexClassOrTriggerName: item.ApexClassOrTrigger.Name, - apexClassOrTriggerId: item.ApexClassOrTrigger.Id, - apexTestClassId: item.ApexTestClassId, - apexTestMethodName: item.TestMethodName, - numLinesCovered: item.NumLinesCovered, - numLinesUncovered: item.NumLinesUncovered, - percentage, - ...(item.Coverage ? { coverage: item.Coverage } : {}) - }; - const key = `${item.ApexTestClassId}-${item.TestMethodName}`; - if (perClassCoverageMap.get(key)) { - perClassCoverageMap.get(key).push(value); - } else { - perClassCoverageMap.set( - `${item.ApexTestClassId}-${item.TestMethodName}`, - [value] + try { + perClassCodeCovResults.forEach((chunk) => { + chunk.records.forEach((item) => { + const totalLines = item.NumLinesCovered + item.NumLinesUncovered; + const percentage = calculatePercentage( + item.NumLinesCovered, + totalLines ); - } - }); - }); - return perClassCoverageMap; + const value = { + apexClassOrTriggerName: item.ApexClassOrTrigger.Name, + apexClassOrTriggerId: item.ApexClassOrTrigger.Id, + apexTestClassId: item.ApexTestClassId, + apexTestMethodName: item.TestMethodName, + numLinesCovered: item.NumLinesCovered, + numLinesUncovered: item.NumLinesUncovered, + percentage, + ...(item.Coverage ? { coverage: item.Coverage } : {}) + }; + const key = `${item.ApexTestClassId}-${item.TestMethodName}`; + if (perClassCoverageMap.get(key)) { + perClassCoverageMap.get(key).push(value); + } else { + perClassCoverageMap.set( + `${item.ApexTestClassId}-${item.TestMethodName}`, + [value] + ); + } + }); + }); + return perClassCoverageMap; + } catch (error) { + throw new Error( + nls.localize('largeTestResultErr', 'Map') + ); + } } /** @@ -107,9 +117,14 @@ export class CodeCoverage { if (apexClassIdSet.size === 0) { return { codeCoverageResults: [], totalLines: 0, coveredLines: 0 }; } - - const codeCoverageAggregates = - await this.queryAggregateCodeCov(apexClassIdSet); + let codeCoverageAggregates; + try { + codeCoverageAggregates = await this.queryAggregateCodeCov(apexClassIdSet); + } catch (error) { + throw new Error( + nls.localize('largeTestResultErr', 'ApexCodeCoverageAggregate[]') + ); + } let totalLinesCovered = 0; let totalLinesUncovered = 0; From 6f60eb95a6e07fdafb40271bc5143ade1ae26c18 Mon Sep 17 00:00:00 2001 From: mingxuanzhang Date: Thu, 21 Mar 2024 14:15:55 -0700 Subject: [PATCH 3/7] chore: polish error handling --- src/i18n/i18n.ts | 4 +- src/streaming/streamingClient.ts | 7 +- src/tests/asyncTests.ts | 31 ++++++--- src/tests/codeCoverage.ts | 17 ++++- src/tests/syncTests.ts | 110 +++++++++++++++++-------------- src/tests/testService.ts | 23 +++++-- 6 files changed, 122 insertions(+), 70 deletions(-) diff --git a/src/i18n/i18n.ts b/src/i18n/i18n.ts index 05384005..78fe0208 100644 --- a/src/i18n/i18n.ts +++ b/src/i18n/i18n.ts @@ -81,9 +81,9 @@ export const messages = { missingSuiteErr: 'Suite does not exist', missingTestClassErr: 'Apex class %s does not exist in the org', jsonStringifyErr: - 'The test result in the format of %s is too large to be stringified. Please try to run fewer tests at a time.', + 'The test result in the format of %s is too large to be stringified. Please try to run fewer tests at a time. \nError: %s', largeTestResultErr: - 'The test result in the format of %s is too large to be stored in the heap. Please try to run fewer tests at a time.', + 'The test result in the format of %s is too large to be stored in the heap. Please try to run fewer tests at a time. \nError: %s', testSuiteMsg: 'Apex test class %s already exists in Apex test suite %s', classSuiteMsg: `Added Apex class %s to your Apex test suite %s`, error_no_default_username: diff --git a/src/streaming/streamingClient.ts b/src/streaming/streamingClient.ts index 0575b658..702ca779 100644 --- a/src/streaming/streamingClient.ts +++ b/src/streaming/streamingClient.ts @@ -289,7 +289,12 @@ export class StreamingClient { } ); } catch (error) { - throw new Error(nls.localize('largeTestResultErr', 'ApexTestQueueItem')); + throw new Error( + nls.localize('largeTestResultErr', [ + 'ApexTestQueueItem', + error?.message + ]) + ); } if (result.records.length === 0) { diff --git a/src/tests/asyncTests.ts b/src/tests/asyncTests.ts index e66c346a..883a747c 100644 --- a/src/tests/asyncTests.ts +++ b/src/tests/asyncTests.ts @@ -162,12 +162,22 @@ export class AsyncTests { message: nls.localize('retrievingTestRunSummary') }); - const testRunSummaryResults = (await this.connection.tooling.query( - testRunSummaryQuery, - { - autoFetch: true - } - )) as ApexTestRunResult; + let testRunSummaryResults; + try { + testRunSummaryResults = (await this.connection.tooling.query( + testRunSummaryQuery, + { + autoFetch: true + } + )) as ApexTestRunResult; + } catch (error) { + throw new Error( + nls.localize('largeTestResultErr', [ + 'ApexTestRunResult', + error?.message + ]) + ); + } if (testRunSummaryResults.records.length === 0) { throw new Error(nls.localize('noTestResultSummary', testRunId)); @@ -318,7 +328,9 @@ export class AsyncTests { const apexTestResults = await Promise.all(queryPromises); return apexTestResults as ApexTestResult[]; } catch (error) { - throw new Error(nls.localize('largeTestResultErr', 'ApexTestResult[]')); + throw new Error( + nls.localize('largeTestResultErr', ['ApexTestResult[]', error?.message]) + ); } } @@ -396,7 +408,10 @@ export class AsyncTests { }; } catch (error) { throw new Error( - nls.localize('largeTestResultErr', 'ApexTestResultData[]') + nls.localize('largeTestResultErr', [ + 'ApexTestResultData[]', + error?.message + ]) ); } } diff --git a/src/tests/codeCoverage.ts b/src/tests/codeCoverage.ts index 242f31a2..738b090c 100644 --- a/src/tests/codeCoverage.ts +++ b/src/tests/codeCoverage.ts @@ -60,7 +60,12 @@ export class CodeCoverage { perClassCodeCovResults = await this.queryPerClassCodeCov(apexTestClassSet); } catch (error) { - throw new Error(nls.localize('largeTestResultErr', 'ApexCodeCoverage[]')); + throw new Error( + nls.localize('largeTestResultErr', [ + 'ApexCodeCoverage[]', + error?.message + ]) + ); } const perClassCoverageMap = new Map(); @@ -98,7 +103,10 @@ export class CodeCoverage { return perClassCoverageMap; } catch (error) { throw new Error( - nls.localize('largeTestResultErr', 'Map') + nls.localize('largeTestResultErr', [ + 'Map', + error?.message + ]) ); } } @@ -122,7 +130,10 @@ export class CodeCoverage { codeCoverageAggregates = await this.queryAggregateCodeCov(apexClassIdSet); } catch (error) { throw new Error( - nls.localize('largeTestResultErr', 'ApexCodeCoverageAggregate[]') + nls.localize('largeTestResultErr', [ + 'ApexCodeCoverageAggregate[]', + error?.message + ]) ); } diff --git a/src/tests/syncTests.ts b/src/tests/syncTests.ts index b33bf952..c51a918e 100644 --- a/src/tests/syncTests.ts +++ b/src/tests/syncTests.ts @@ -10,6 +10,7 @@ import { CancellationToken } from '../common'; import { formatStartTime, getCurrentTime } from '../utils'; import { CodeCoverage } from './codeCoverage'; import { formatTestErrors, getSyncDiagnostic } from './diagnosticUtil'; +import { nls } from '../i18n'; import { ApexTestResultData, ApexTestResultOutcome, @@ -154,58 +155,67 @@ export class SyncTests { const testResults: ApexTestResultData[] = []; const apexTestClassIdSet = new Set(); - apiTestResult.successes.forEach((item) => { - const nms = item.namespace ? `${item.namespace}.` : ''; - apexTestClassIdSet.add(item.id); - testResults.push({ - id: '', - queueItemId: '', - stackTrace: '', - message: '', - asyncApexJobId: '', - methodName: item.methodName, - outcome: ApexTestResultOutcome.Pass, - apexLogId: apiTestResult.apexLogId, - apexClass: { - id: item.id, - name: item.name, - namespacePrefix: item.namespace, - fullName: `${nms}${item.name}` - }, - runTime: item.time ?? 0, - testTimestamp: '', - fullName: `${nms}${item.name}.${item.methodName}` + try { + apiTestResult.successes.forEach((item) => { + const nms = item.namespace ? `${item.namespace}.` : ''; + apexTestClassIdSet.add(item.id); + testResults.push({ + id: '', + queueItemId: '', + stackTrace: '', + message: '', + asyncApexJobId: '', + methodName: item.methodName, + outcome: ApexTestResultOutcome.Pass, + apexLogId: apiTestResult.apexLogId, + apexClass: { + id: item.id, + name: item.name, + namespacePrefix: item.namespace, + fullName: `${nms}${item.name}` + }, + runTime: item.time ?? 0, + testTimestamp: '', + fullName: `${nms}${item.name}.${item.methodName}` + }); }); - }); - - apiTestResult.failures.forEach((item) => { - const nms = item.namespace ? `${item.namespace}__` : ''; - apexTestClassIdSet.add(item.id); - const diagnostic = - item.message || item.stackTrace ? getSyncDiagnostic(item) : null; - - testResults.push({ - id: '', - queueItemId: '', - stackTrace: item.stackTrace, - message: item.message, - asyncApexJobId: '', - methodName: item.methodName, - outcome: ApexTestResultOutcome.Fail, - apexLogId: apiTestResult.apexLogId, - apexClass: { - id: item.id, - name: item.name, - namespacePrefix: item.namespace, - fullName: `${nms}${item.name}` - }, - runTime: item.time ?? 0, - testTimestamp: '', - fullName: `${nms}${item.name}.${item.methodName}`, - ...(diagnostic ? { diagnostic } : {}) + + apiTestResult.failures.forEach((item) => { + const nms = item.namespace ? `${item.namespace}__` : ''; + apexTestClassIdSet.add(item.id); + const diagnostic = + item.message || item.stackTrace ? getSyncDiagnostic(item) : null; + + testResults.push({ + id: '', + queueItemId: '', + stackTrace: item.stackTrace, + message: item.message, + asyncApexJobId: '', + methodName: item.methodName, + outcome: ApexTestResultOutcome.Fail, + apexLogId: apiTestResult.apexLogId, + apexClass: { + id: item.id, + name: item.name, + namespacePrefix: item.namespace, + fullName: `${nms}${item.name}` + }, + runTime: item.time ?? 0, + testTimestamp: '', + fullName: `${nms}${item.name}.${item.methodName}`, + ...(diagnostic ? { diagnostic } : {}) + }); }); - }); - return { apexTestClassIdSet, testResults }; + return { apexTestClassIdSet, testResults }; + } catch (error) { + throw new Error( + nls.localize('largeTestResultErr', [ + 'ApexTestResultData[]', + error?.message + ]) + ); + } } } diff --git a/src/tests/testService.ts b/src/tests/testService.ts index 61d2b438..8edae9f8 100644 --- a/src/tests/testService.ts +++ b/src/tests/testService.ts @@ -294,7 +294,9 @@ export class TestService { }); break; } catch (error) { - throw new Error(nls.localize('jsonStringifyErr', format)); + throw new Error( + nls.localize('jsonStringifyErr', [format, error?.message]) + ); } case ResultFormat.tap: try { @@ -305,7 +307,9 @@ export class TestService { }); break; } catch (error) { - throw new Error(nls.localize('jsonStringifyErr', format)); + throw new Error( + nls.localize('jsonStringifyErr', [format, error?.message]) + ); } case ResultFormat.junit: try { @@ -320,7 +324,9 @@ export class TestService { content: junitResult }); } catch (error) { - throw new Error(nls.localize('jsonStringifyErr', format)); + throw new Error( + nls.localize('jsonStringifyErr', [format, error?.message]) + ); } break; } @@ -343,8 +349,11 @@ export class TestService { content }); } catch (error) { - throw new error( - nls.localize('jsonStringifyErr', 'code coverage records') + throw new Error( + nls.localize('jsonStringifyErr', [ + 'code coverage records', + error?.message + ]) ); } } @@ -359,7 +368,9 @@ export class TestService { : fileInfo.content }); } catch (error) { - throw new error(nls.localize('jsonStringifyErr', 'fileInfos')); + throw new Error( + nls.localize('jsonStringifyErr', ['fileInfos', error?.message]) + ); } }); From f3c10e6dbc49928a3638752043edb4c130bc566c Mon Sep 17 00:00:00 2001 From: mingxuanzhang Date: Fri, 22 Mar 2024 15:38:02 -0700 Subject: [PATCH 4/7] chore: remove unnecessary catches --- src/i18n/i18n.ts | 6 ++---- src/tests/testService.ts | 38 +++++++++++++------------------------- 2 files changed, 15 insertions(+), 29 deletions(-) diff --git a/src/i18n/i18n.ts b/src/i18n/i18n.ts index 78fe0208..ddd59284 100644 --- a/src/i18n/i18n.ts +++ b/src/i18n/i18n.ts @@ -80,10 +80,8 @@ export const messages = { 'Must provide a suite name or suite id to retrieve test classes in suite', missingSuiteErr: 'Suite does not exist', missingTestClassErr: 'Apex class %s does not exist in the org', - jsonStringifyErr: - 'The test result in the format of %s is too large to be stringified. Please try to run fewer tests at a time. \nError: %s', - largeTestResultErr: - 'The test result in the format of %s is too large to be stored in the heap. Please try to run fewer tests at a time. \nError: %s', + jsonStringifyErr: `The test result in the format of %s is too large to be stringified. Please try to run fewer tests at a time. Error: %s`, + largeTestResultErr: `The test result in the format of %s is too large to be stored in the heap. Please try to run fewer tests at a time. Error: %s`, testSuiteMsg: 'Apex test class %s already exists in Apex test suite %s', classSuiteMsg: `Added Apex class %s to your Apex test suite %s`, error_no_default_username: diff --git a/src/tests/testService.ts b/src/tests/testService.ts index 8edae9f8..b469472f 100644 --- a/src/tests/testService.ts +++ b/src/tests/testService.ts @@ -186,11 +186,7 @@ export class TestService { codeCoverage = false, token?: CancellationToken ): Promise { - try { - return await this.syncService.runTests(options, codeCoverage, token); - } catch (error) { - throw error; - } + return await this.syncService.runTests(options, codeCoverage, token); } /** @@ -209,17 +205,13 @@ export class TestService { progress?: Progress, token?: CancellationToken ): Promise { - try { - return await this.asyncService.runTests( - options, - codeCoverage, - immediatelyReturn, - progress, - token - ); - } catch (error) { - throw error; - } + return await this.asyncService.runTests( + options, + codeCoverage, + immediatelyReturn, + progress, + token + ); } /** @@ -234,15 +226,11 @@ export class TestService { codeCoverage = false, token?: CancellationToken ): Promise { - try { - return await this.asyncService.reportAsyncResults( - testRunId, - codeCoverage, - token - ); - } catch (error) { - throw error; - } + return await this.asyncService.reportAsyncResults( + testRunId, + codeCoverage, + token + ); } /** From 4b35861d0199e9e65891886dd1774a8d9952f803 Mon Sep 17 00:00:00 2001 From: mingxuanzhang Date: Sun, 24 Mar 2024 18:36:39 -0700 Subject: [PATCH 5/7] chore: add unit tests and change scope for tested methods --- hello | 1 + src/streaming/streamingClient.ts | 2 +- src/tests/codeCoverage.ts | 4 +- test-result-123-junit.xml | 26 +++++++ test-run-id.txt | 1 + test/streaming/streamingClient.test.ts | 31 +++++++++ test/tests/asyncTests.test.ts | 32 +++++++++ test/tests/codeCoverage.test.ts | 67 ++++++++++++++++++ test/tests/testService.test.ts | 96 +++++++++++++++++++++++++- 9 files changed, 256 insertions(+), 4 deletions(-) create mode 100644 hello create mode 100644 test-result-123-junit.xml create mode 100644 test-run-id.txt diff --git a/hello b/hello new file mode 100644 index 00000000..04fea064 --- /dev/null +++ b/hello @@ -0,0 +1 @@ +world \ No newline at end of file diff --git a/src/streaming/streamingClient.ts b/src/streaming/streamingClient.ts index 702ca779..704619b4 100644 --- a/src/streaming/streamingClient.ts +++ b/src/streaming/streamingClient.ts @@ -276,7 +276,7 @@ export class StreamingClient { return null; } - private async getCompletedTestRun( + public async getCompletedTestRun( testRunId: string ): Promise { const queryApexTestQueueItem = `SELECT Id, Status, ApexClassId, TestRunResultId FROM ApexTestQueueItem WHERE ParentJobId = '${testRunId}'`; diff --git a/src/tests/codeCoverage.ts b/src/tests/codeCoverage.ts index 738b090c..4a2b4933 100644 --- a/src/tests/codeCoverage.ts +++ b/src/tests/codeCoverage.ts @@ -179,7 +179,7 @@ export class CodeCoverage { } @elapsedTime() - private async queryPerClassCodeCov( + public async queryPerClassCodeCov( apexTestClassSet: Set ): Promise { const perClassCodeCovQuery = @@ -188,7 +188,7 @@ export class CodeCoverage { } @elapsedTime() - private async queryAggregateCodeCov( + public async queryAggregateCodeCov( apexClassIdSet: Set ): Promise { const codeCoverageQuery = diff --git a/test-result-123-junit.xml b/test-result-123-junit.xml new file mode 100644 index 00000000..6dae50e1 --- /dev/null +++ b/test-result-123-junit.xml @@ -0,0 +1,26 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test-run-id.txt b/test-run-id.txt new file mode 100644 index 00000000..d800886d --- /dev/null +++ b/test-run-id.txt @@ -0,0 +1 @@ +123 \ No newline at end of file diff --git a/test/streaming/streamingClient.test.ts b/test/streaming/streamingClient.test.ts index b504a56f..345cf7e1 100644 --- a/test/streaming/streamingClient.test.ts +++ b/test/streaming/streamingClient.test.ts @@ -548,3 +548,34 @@ describe('Streaming API Client', () => { assert.calledOnce(clearIntervalStub); }); }); + +describe('getCompletedTestRun', () => { + const $$ = new TestContext(); + + beforeEach(async () => { + sandboxStub = createSandbox(); + // Stub retrieveMaxApiVersion to get over "Domain Not Found: The org cannot be found" error + sandboxStub + .stub(Connection.prototype, 'retrieveMaxApiVersion') + .resolves('50.0'); + await $$.stubAuths(testData); + mockConnection = await testData.getConnection(); + }); + + afterEach(() => { + sandboxStub.restore(); + }); + + it('should return largeTestResultErr if query fails', () => { + const mockToolingQuery = sandboxStub.stub(mockConnection.tooling, 'query'); + const errorMessage = '123'; + mockToolingQuery.rejects(new Error(errorMessage)); + const streamClient = new StreamingClient(mockConnection); + try { + streamClient.getCompletedTestRun('testRunId'); + } catch (error) { + expect(error.message).to.include('ApexTestQueueItem'); + expect(error.message).to.include(errorMessage); + } + }); +}); diff --git a/test/tests/asyncTests.test.ts b/test/tests/asyncTests.test.ts index d20415b9..9c2d9cee 100644 --- a/test/tests/asyncTests.test.ts +++ b/test/tests/asyncTests.test.ts @@ -735,6 +735,23 @@ describe('Run Apex tests asynchronously', () => { expect(result.length).to.eql(1); }); + it('should throw error when queryPromises fail in getAsyncTestResults', async () => { + sandboxStub + .stub(mockConnection.tooling, 'query') + .resolves({ done: true, totalSize: 1, records: [] }); + const errorMessage = '123'; + sandboxStub.stub(utils, 'queryAll').rejects(new Error(errorMessage)); + const asyncTestSrv = new AsyncTests(mockConnection); + try { + await asyncTestSrv.getAsyncTestResults(pollResponse); + fail('should throw an error'); + } catch (e) { + expect(e.message).to.include( + nls.localize('largeTestResultErr', ['ApexTestResult[]', errorMessage]) + ); + } + }); + it('should format multiple queries correctly', async () => { const queryOneIds = queryIds.slice(0, QUERY_RECORD_LIMIT).join("','"); const queryOne = `${queryStart}('${queryOneIds}')`; @@ -1405,6 +1422,21 @@ describe('Run Apex tests asynchronously', () => { expect(handlerStub.calledOnce).to.be.true; }); }); + + it('should throw error when query fails in checkRunStatus', async () => { + const asyncTestSrv = new AsyncTests(mockConnection); + const mockToolingQuery = sandboxStub.stub(mockConnection.tooling, 'query'); + const errorMessage = '123'; + mockToolingQuery.onFirstCall().rejects(new Error(errorMessage)); + try { + await asyncTestSrv.checkRunStatus(testRunId); + fail('should throw an error'); + } catch (e) { + expect(e.message).to.include( + nls.localize('largeTestResultErr', ['ApexTestRunResult', errorMessage]) + ); + } + }); }); describe('elapsedTime', () => { diff --git a/test/tests/codeCoverage.test.ts b/test/tests/codeCoverage.test.ts index dd575aa5..234caf8d 100644 --- a/test/tests/codeCoverage.test.ts +++ b/test/tests/codeCoverage.test.ts @@ -18,6 +18,7 @@ import { ApexCodeCoverageAggregateRecord } from '../../src/tests/types'; import { QUERY_RECORD_LIMIT } from '../../src/tests/constants'; +import { nls } from '../../src/i18n'; let mockConnection: Connection; let sandboxStub: SinonSandbox; @@ -158,6 +159,72 @@ describe('Get code coverage results', () => { expect(coveredLines).to.equal(0); }); + it('should throw error when queryPerClassCodeCov fail', async () => { + const errorMessage = '123'; + const codeCov = new CodeCoverage(mockConnection); + sandboxStub + .stub(codeCov, 'queryPerClassCodeCov') + .rejects(new Error(errorMessage)); + try { + await codeCov.getPerClassCodeCoverage( + new Set(['0001x05958', '0001x05959', '0001x05951']) + ); + } catch (e) { + expect(e.message).to.be.include( + nls.localize('largeTestResultErr', ['ApexCodeCoverage[]', errorMessage]) + ); + } + }); + + it('should throw error when queryAggregateCodeCov fail', async () => { + const codeCoverageQueryResult = [ + { + ApexClassOrTrigger: { Id: '0001x05958', Name: 'ApexTrigger1' }, + NumLinesCovered: 5, + NumLinesUncovered: 1, + Coverage: { coveredLines: [1, 2, 3, 4, 5], uncoveredLines: [6] } + }, + { + ApexClassOrTrigger: { Id: '0001x05959', Name: 'ApexTrigger2' }, + NumLinesCovered: 6, + NumLinesUncovered: 2, + Coverage: { coveredLines: [1, 2, 3, 4, 5, 6], uncoveredLines: [7, 8] } + }, + { + ApexClassOrTrigger: { Id: '0001x05951', Name: 'ApexTrigger3' }, + NumLinesCovered: 7, + NumLinesUncovered: 3, + Coverage: { + coveredLines: [1, 2, 3, 4, 5, 6, 7], + uncoveredLines: [8, 9, 10] + } + } + ]; + + toolingQueryStub.resolves({ + done: true, + totalSize: 3, + records: codeCoverageQueryResult + } as ApexCodeCoverageAggregate); + const errorMessage = '123'; + const codeCov = new CodeCoverage(mockConnection); + sandboxStub + .stub(codeCov, 'queryAggregateCodeCov') + .rejects(new Error(errorMessage)); + try { + await codeCov.getPerClassCodeCoverage( + new Set(['0001x05958', '0001x05959', '0001x05951']) + ); + } catch (e) { + expect(e.message).to.be.include( + nls.localize('largeTestResultErr', [ + 'ApexCodeCoverageAggregate[]', + errorMessage + ]) + ); + } + }); + it('should return per class code coverage for multiple test classes', async () => { const perClassCodeCovResult = [ { diff --git a/test/tests/testService.test.ts b/test/tests/testService.test.ts index 4244848c..16e02ba7 100644 --- a/test/tests/testService.test.ts +++ b/test/tests/testService.test.ts @@ -8,8 +8,10 @@ import { AuthInfo, Connection } from '@salesforce/core'; import { MockTestOrgData, TestContext } from '@salesforce/core/lib/testSetup'; import { fail } from 'assert'; import { expect } from 'chai'; +import * as u from '../../src/tests/utils'; import { createSandbox, SinonSandbox, SinonStub, spy } from 'sinon'; -import { TestService } from '../../src'; +import { ApexTestResultData, ResultFormat, TestService } from '../../src'; +import { nls } from '../../src/i18n'; let mockConnection: Connection; let sandboxStub: SinonSandbox; @@ -288,3 +290,95 @@ describe('Apex Test Suites', async () => { }); }); }); + +describe('WriteResultFiles', async () => { + const $$ = new TestContext(); + beforeEach(async () => { + sandboxStub = createSandbox(); + await $$.stubAuths(testData); + // Stub retrieveMaxApiVersion to get over "Domain Not Found: The org cannot be found" error + sandboxStub + .stub(Connection.prototype, 'retrieveMaxApiVersion') + .resolves('50.0'); + mockConnection = await Connection.create({ + authInfo: await AuthInfo.create({ + username: testData.username + }) + }); + + toolingQueryStub = sandboxStub.stub(mockConnection.tooling, 'query'); + toolingCreateStub = sandboxStub.stub(mockConnection.tooling, 'create'); + }); + + afterEach(async () => { + sandboxStub.restore(); + }); + + it('should throw error when stringify fail', async () => { + toolingQueryStub.resolves({ records: [{ Id: 'xxxxxxx243' }] }); + const errorMessage = '123'; + sandboxStub.stub(u, 'stringify').throws(new Error(errorMessage)); + const mockResult = { + summary: { + failRate: '123', + testsRan: 1, + orgId: '123', + outcome: '123', + passing: 1, + failing: 0, + skipped: 0, + passRate: '123', + skipRate: '123', + testStartTime: '123', + testExecutionTimeInMs: 1, + testTotalTimeInMs: 1, + commandTimeInMs: 1, + hostname: '123', + username: '123', + testRunId: '123', + userId: '123' + }, + tests: [ + { + id: '123', + queueItemId: '123', + stackTrace: '123', + message: '123', + asyncApexJobId: '123', + methodName: 'method', + outcome: 'CompileFail', + apexLogId: '123', + apexClass: { + id: '123', + name: 'apex', + namespacePrefix: 'a', + fullName: 'apexClass' + }, + runTime: 1, + testTimestamp: '123', + fullName: 'mockApexTestResultData' + } as ApexTestResultData + ] + }; + + const outputDirConfig = { + dirPath: './', + resultFormats: [ResultFormat.json], + fileInfos: [ + { + filename: 'hello', + content: 'world' + } + ] + }; + const testService = new TestService(mockConnection); + try { + await testService.writeResultFiles(mockResult, outputDirConfig, false); + fail('Should throw an error'); + } catch (e) { + expect(e.message).to.include( + nls.localize('jsonStringifyErr', [ResultFormat.json, errorMessage]) + ); + } + }); +}); From f069842d92aad0589694d7ed7a0d87741968cc1a Mon Sep 17 00:00:00 2001 From: mingxuanzhang Date: Mon, 25 Mar 2024 14:59:55 -0700 Subject: [PATCH 6/7] chore: stub fs.createWriteStream --- test-result-123-junit.xml | 26 -------------------------- test-run-id.txt | 1 - test/tests/testService.test.ts | 5 +++++ 3 files changed, 5 insertions(+), 27 deletions(-) delete mode 100644 test-result-123-junit.xml delete mode 100644 test-run-id.txt diff --git a/test-result-123-junit.xml b/test-result-123-junit.xml deleted file mode 100644 index 6dae50e1..00000000 --- a/test-result-123-junit.xml +++ /dev/null @@ -1,26 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/test-run-id.txt b/test-run-id.txt deleted file mode 100644 index d800886d..00000000 --- a/test-run-id.txt +++ /dev/null @@ -1 +0,0 @@ -123 \ No newline at end of file diff --git a/test/tests/testService.test.ts b/test/tests/testService.test.ts index 16e02ba7..fa213cff 100644 --- a/test/tests/testService.test.ts +++ b/test/tests/testService.test.ts @@ -9,6 +9,8 @@ import { MockTestOrgData, TestContext } from '@salesforce/core/lib/testSetup'; import { fail } from 'assert'; import { expect } from 'chai'; import * as u from '../../src/tests/utils'; +import * as stream from 'stream'; +import * as fs from 'fs'; import { createSandbox, SinonSandbox, SinonStub, spy } from 'sinon'; import { ApexTestResultData, ResultFormat, TestService } from '../../src'; import { nls } from '../../src/i18n'; @@ -318,6 +320,9 @@ describe('WriteResultFiles', async () => { toolingQueryStub.resolves({ records: [{ Id: 'xxxxxxx243' }] }); const errorMessage = '123'; sandboxStub.stub(u, 'stringify').throws(new Error(errorMessage)); + sandboxStub + .stub(fs, 'createWriteStream') + .returns(new stream.PassThrough() as never); const mockResult = { summary: { failRate: '123', From b0b55ffa6801db7064d1bb5ac1864ad5e666f600 Mon Sep 17 00:00:00 2001 From: mingxuanzhang Date: Mon, 25 Mar 2024 15:04:51 -0700 Subject: [PATCH 7/7] chore: delete hello --- hello | 1 - 1 file changed, 1 deletion(-) delete mode 100644 hello diff --git a/hello b/hello deleted file mode 100644 index 04fea064..00000000 --- a/hello +++ /dev/null @@ -1 +0,0 @@ -world \ No newline at end of file