Skip to content

Commit

Permalink
fix: guard against undefined test results
Browse files Browse the repository at this point in the history
@W-15298950@
Account for a undefined ApexTestRunResult when formatting results
  • Loading branch information
peternhale committed Mar 21, 2024
1 parent a0258ce commit f78cfc7
Show file tree
Hide file tree
Showing 9 changed files with 217 additions and 266 deletions.
1 change: 1 addition & 0 deletions src/execute/executeService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export class ExecuteService {
}
}
}
throw new Error(nls.localize('authForAnonymousApexFailed'));
}

@elapsedTime()
Expand Down
3 changes: 2 additions & 1 deletion src/i18n/i18n.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,5 +91,6 @@ export const messages = {
covIdFormatErr: 'Cannot specify code coverage with a TestRunId result',
startHandshake: 'Attempting StreamingClient handshake',
finishHandshake: 'Finished StreamingClient handshake',
subscribeStarted: 'Subscribing to ApexLog events'
subscribeStarted: 'Subscribing to ApexLog events',
authForAnonymousApexFailed: 'The authentication for execute anonymous failed'
};
4 changes: 2 additions & 2 deletions src/i18n/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ import { Localization, Message } from './localization';

function loadMessageBundle(): Message {
try {
const layer = new Message(messages);
return layer;
return new Message(messages);
} catch (e) {
console.error('Cannot find messages in i18n module');
}
return undefined;
}

export const nls = new Localization(loadMessageBundle());
Expand Down
20 changes: 10 additions & 10 deletions src/streaming/streamingClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -296,16 +296,16 @@ export class StreamingClient {
value: result
});

for (let i = 0; i < result.records.length; i++) {
const item = result.records[i];
if (
item.Status === ApexTestQueueItemStatus.Queued ||
item.Status === ApexTestQueueItemStatus.Holding ||
item.Status === ApexTestQueueItemStatus.Preparing ||
item.Status === ApexTestQueueItemStatus.Processing
) {
return null;
}
if (
result.records.some(
(item) =>
item.Status === ApexTestQueueItemStatus.Queued ||
item.Status === ApexTestQueueItemStatus.Holding ||
item.Status === ApexTestQueueItemStatus.Preparing ||
item.Status === ApexTestQueueItemStatus.Processing
)
) {
return null;
}
return result;
}
Expand Down
77 changes: 36 additions & 41 deletions src/tests/asyncTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/

'use strict';

import { Connection } from '@salesforce/core';
import { CancellationToken, Progress } from '../common';
import { nls } from '../i18n';
import { AsyncTestRun, StreamingClient } from '../streaming';
import { formatStartTime, getCurrentTime } from '../utils';
import { elapsedTime, formatStartTime, getCurrentTime } from '../utils';
import { formatTestErrors, getAsyncDiagnostic } from './diagnosticUtil';
import {
ApexTestProgressValue,
Expand All @@ -20,7 +22,6 @@ import {
ApexTestResultData,
ApexTestResultOutcome,
ApexTestRunResult,
ApexTestRunResultRecord,
ApexTestRunResultStatus,
AsyncTestArrayConfiguration,
AsyncTestConfiguration,
Expand All @@ -32,7 +33,14 @@ import * as util from 'util';
import { QUERY_RECORD_LIMIT } from './constants';
import { CodeCoverage } from './codeCoverage';
import { HttpRequest } from 'jsforce';
import { elapsedTime } from '../utils/elapsedTime';

const finishedStatuses = [
ApexTestRunResultStatus.Aborted,
ApexTestRunResultStatus.Failed,
ApexTestRunResultStatus.Completed,
ApexTestRunResultStatus.Passed,
ApexTestRunResultStatus.Skipped
];

export class AsyncTests {
public readonly connection: Connection;
Expand Down Expand Up @@ -145,49 +153,34 @@ export class AsyncTests {
public async checkRunStatus(
testRunId: string,
progress?: Progress<ApexTestProgressValue>
): Promise<ApexTestRunResultRecord | undefined> {
): Promise<ApexTestRunResult> {
if (!isValidTestRunID(testRunId)) {
throw new Error(nls.localize('invalidTestRunIdErr', testRunId));
}

let testRunSummaryQuery =
'SELECT AsyncApexJobId, Status, ClassesCompleted, ClassesEnqueued, ';
testRunSummaryQuery +=
'MethodsEnqueued, StartTime, EndTime, TestTime, UserId ';
testRunSummaryQuery += `FROM ApexTestRunResult WHERE AsyncApexJobId = '${testRunId}'`;
const testRunSummaryQuery = `SELECT AsyncApexJobId, Status, ClassesCompleted, ClassesEnqueued, MethodsEnqueued, StartTime, EndTime, TestTime, UserId FROM ApexTestRunResult WHERE AsyncApexJobId = '${testRunId}'`;

progress?.report({
type: 'FormatTestResultProgress',
value: 'retrievingTestRunSummary',
message: nls.localize('retrievingTestRunSummary')
});

const testRunSummaryResults = (await this.connection.tooling.query(
testRunSummaryQuery,
{
autoFetch: true
}
)) as ApexTestRunResult;
try {
const testRunSummaryResults = (await this.connection.singleRecordQuery(
testRunSummaryQuery,
{
tooling: true
}
)) as ApexTestRunResult;

if (testRunSummaryResults.records.length === 0) {
if (finishedStatuses.includes(testRunSummaryResults.Status)) {
return testRunSummaryResults;
}
} catch (e) {
throw new Error(nls.localize('noTestResultSummary', testRunId));
}

if (
testRunSummaryResults.records[0].Status ===
ApexTestRunResultStatus.Aborted ||
testRunSummaryResults.records[0].Status ===
ApexTestRunResultStatus.Failed ||
testRunSummaryResults.records[0].Status ===
ApexTestRunResultStatus.Completed ||
testRunSummaryResults.records[0].Status ===
ApexTestRunResultStatus.Passed ||
testRunSummaryResults.records[0].Status ===
ApexTestRunResultStatus.Skipped
) {
return testRunSummaryResults.records[0];
}

return undefined;
}

Expand All @@ -196,7 +189,7 @@ export class AsyncTests {
* @param asyncRunResult TestQueueItem and RunId for an async run
* @param commandStartTime start time for the async test run
* @param codeCoverage should report code coverages
* @param testRunSummary test run summary
* @param testRunSummary test run summary | undefined
* @param progress progress reporter
* @returns
*/
Expand All @@ -205,7 +198,7 @@ export class AsyncTests {
asyncRunResult: AsyncTestRun,
commandStartTime: number,
codeCoverage = false,
testRunSummary: ApexTestRunResultRecord,
testRunSummary: ApexTestRunResult | undefined,
progress?: Progress<ApexTestProgressValue>
): Promise<TestResult> {
const coveredApexClassIdSet = new Set<string>();
Expand All @@ -215,12 +208,12 @@ export class AsyncTests {
const { apexTestClassIdSet, testResults, globalTests } =
await this.buildAsyncTestResults(apexTestResults);

let outcome = testRunSummary.Status;
let outcome = testRunSummary?.Status ?? 'Unknown';
if (globalTests.failed > 0) {
outcome = ApexTestRunResultStatus.Failed;
} else if (globalTests.passed === 0) {
outcome = ApexTestRunResultStatus.Skipped;
} else if (testRunSummary.Status === ApexTestRunResultStatus.Completed) {
} else if (testRunSummary?.Status === ApexTestRunResultStatus.Completed) {
outcome = ApexTestRunResultStatus.Passed;
}

Expand All @@ -235,15 +228,17 @@ export class AsyncTests {
passRate: calculatePercentage(globalTests.passed, testResults.length),
failRate: calculatePercentage(globalTests.failed, testResults.length),
skipRate: calculatePercentage(globalTests.skipped, testResults.length),
testStartTime: formatStartTime(testRunSummary.StartTime, 'ISO'),
testExecutionTimeInMs: testRunSummary.TestTime ?? 0,
testTotalTimeInMs: testRunSummary.TestTime ?? 0,
testStartTime: testRunSummary?.StartTime
? formatStartTime(testRunSummary.StartTime, 'ISO')
: '',
testExecutionTimeInMs: testRunSummary?.TestTime ?? 0,
testTotalTimeInMs: testRunSummary?.TestTime ?? 0,
commandTimeInMs: getCurrentTime() - commandStartTime,
hostname: this.connection.instanceUrl,
orgId: this.connection.getAuthInfoFields().orgId,
username: this.connection.getUsername(),
testRunId: asyncRunResult.runId,
userId: testRunSummary.UserId
userId: testRunSummary?.UserId ?? 'Unknown'
},
tests: testResults
};
Expand Down Expand Up @@ -394,6 +389,7 @@ export class AsyncTests {
/**
* Abort test run with test run id
* @param testRunId
* @param progress
*/
public async abortTestRun(
testRunId: string,
Expand Down Expand Up @@ -430,7 +426,7 @@ export class AsyncTests {
private getTestRunRequestAction(
options: AsyncTestConfiguration | AsyncTestArrayConfiguration
): () => Promise<string> {
const requestTestRun = async (): Promise<string> => {
return async (): Promise<string> => {
const url = `${this.connection.tooling._baseUrl()}/runTestsAsynchronous`;
const request: HttpRequest = {
method: 'POST',
Expand All @@ -448,6 +444,5 @@ export class AsyncTests {
return Promise.reject(e);
}
};
return requestTestRun;
}
}
10 changes: 4 additions & 6 deletions src/tests/testService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { AsyncTests } from './asyncTests';
import { SyncTests } from './syncTests';
import { formatTestErrors } from './diagnosticUtil';
import { QueryResult } from '../utils/types';
import { elapsedTime } from '../utils/elapsedTime';
import { elapsedTime } from '../utils';

export class TestService {
private readonly connection: Connection;
Expand Down Expand Up @@ -341,11 +341,9 @@ export class TestService {
try {
if (tests) {
const payload = await this.buildTestPayload(tests);
const classes = payload.tests?.map((testItem) => {
if (testItem.className) {
return testItem.className;
}
});
const classes = payload.tests
?.filter((testItem) => testItem.className)
.map((testItem) => testItem.className);
if (new Set(classes).size !== 1) {
throw new Error(nls.localize('syncClassErr'));
}
Expand Down
8 changes: 1 addition & 7 deletions src/tests/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ export const enum ApexTestRunResultStatus {
Skipped = 'Skipped'
}

export type ApexTestRunResultRecord = {
export type ApexTestRunResult = {
/**
* The parent Apex job ID for the result
*/
Expand All @@ -278,12 +278,6 @@ export type ApexTestRunResultRecord = {
UserId: string;
};

export type ApexTestRunResult = {
done: boolean;
totalSize: number;
records: ApexTestRunResultRecord[];
};

export const enum ApexTestQueueItemStatus {
Holding = 'Holding',
Queued = 'Queued',
Expand Down
Loading

0 comments on commit f78cfc7

Please sign in to comment.