fix: guard against undefined test results (#354)

* fix: guard against undefined test results

@W-15298950@
Account for a undefined ApexTestRunResult when formatting results

* chore: apply review suggestions and write test

* chore: remove possibility of returning undefined

* chore: address review issues

* chore: fix typo

* chore: merge main

src/execute/executeService.ts

@@ -56,6 +56,7 @@ export class ExecuteService {
         }
       }
     }
+    throw new Error(nls.localize('authForAnonymousApexFailed'));
   }
 
   @elapsedTime()

src/i18n/i18n.ts

@@ -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'
 };

src/i18n/index.ts

@@ -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());

src/streaming/streamingClient.ts

@@ -9,8 +9,8 @@ import { Client } from 'faye';
 import { Connection, LoggerLevel } from '@salesforce/core';
 import {
   RetrieveResultsInterval,
-  StreamingErrors,
   StreamMessage,
+  StreamingErrors,
   TestResultMessage
 } from './types';
 import { Progress } from '../common';
@@ -19,7 +19,8 @@ import { elapsedTime, refreshAuth } from '../utils';
 import {
   ApexTestProgressValue,
   ApexTestQueueItem,
-  ApexTestQueueItemRecord
+  ApexTestQueueItemRecord,
+  ApexTestQueueItemStatus
 } from '../tests/types';
 
 const TEST_RESULT_CHANNEL = '/systemTopic/TestResult';
@@ -298,10 +299,10 @@ export class StreamingClient {
     if (
       result.records.some(
         (item) =>
-          item.Status === 'Queued' /* ApexTestQueueItemStatus.Queued */ ||
-          item.Status === 'Holding' /* ApexTestQueueItemStatus.Holding */ ||
-          item.Status === 'Preparing' /* ApexTestQueueItemStatus.Preparing */ ||
-          item.Status === 'Processing' /* ApexTestQueueItemStatus.Processing */
+          item.Status === ApexTestQueueItemStatus.Queued ||
+          item.Status === ApexTestQueueItemStatus.Holding ||
+          item.Status === ApexTestQueueItemStatus.Preparing ||
+          item.Status === ApexTestQueueItemStatus.Processing
       )
     ) {
       return null;

src/tests/asyncTests.ts

@@ -9,7 +9,7 @@ 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,
@@ -20,7 +20,6 @@ import {
   ApexTestResultData,
   ApexTestResultOutcome,
   ApexTestRunResult,
-  ApexTestRunResultRecord,
   ApexTestRunResultStatus,
   AsyncTestArrayConfiguration,
   AsyncTestConfiguration,
@@ -32,9 +31,16 @@ import * as util from 'util';
 import { QUERY_RECORD_LIMIT } from './constants';
 import { CodeCoverage } from './codeCoverage';
 import { HttpRequest } from 'jsforce';
-import { elapsedTime } from '../utils/elapsedTime';
 import { isValidTestRunID } from '../narrowing';
 
+const finishedStatuses = [
+  ApexTestRunResultStatus.Aborted,
+  ApexTestRunResultStatus.Failed,
+  ApexTestRunResultStatus.Completed,
+  ApexTestRunResultStatus.Passed,
+  ApexTestRunResultStatus.Skipped
+];
+
 export class AsyncTests {
   public readonly connection: Connection;
   private readonly codecoverage: CodeCoverage;
@@ -83,12 +89,12 @@ export class AsyncTests {
       }
 
       const asyncRunResult = await sClient.subscribe(undefined, testRunId);
-      const testRunSummary = await this.checkRunStatus(asyncRunResult.runId);
+      const runResult = await this.checkRunStatus(asyncRunResult.runId);
       return await this.formatAsyncResults(
         asyncRunResult,
         getCurrentTime(),
         codeCoverage,
-        testRunSummary,
+        runResult.testRunSummary,
         progress
       );
     } catch (e) {
@@ -113,13 +119,13 @@ export class AsyncTests {
       await sClient.init();
       await sClient.handshake();
       let queueItem: ApexTestQueueItem;
-      let testRunSummary = await this.checkRunStatus(testRunId);
+      let runResult = await this.checkRunStatus(testRunId);
 
-      if (testRunSummary !== undefined) {
+      if (runResult.testsComplete) {
         queueItem = await sClient.handler(undefined, testRunId);
       } else {
         queueItem = (await sClient.subscribe(undefined, testRunId)).queueItem;
-        testRunSummary = await this.checkRunStatus(testRunId);
+        runResult = await this.checkRunStatus(testRunId);
       }
 
       token &&
@@ -135,7 +141,7 @@ export class AsyncTests {
         { queueItem, runId: testRunId },
         getCurrentTime(),
         codeCoverage,
-        testRunSummary
+        runResult.testRunSummary
       );
     } catch (e) {
       throw formatTestErrors(e);
@@ -146,50 +152,37 @@ export class AsyncTests {
   public async checkRunStatus(
     testRunId: string,
     progress?: Progress<ApexTestProgressValue>
-  ): Promise<ApexTestRunResultRecord | undefined> {
+  ): Promise<{
+    testsComplete: boolean;
+    testRunSummary: 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;
-
-    if (testRunSummaryResults.records.length === 0) {
+    try {
+      const testRunSummaryResults =
+        await this.connection.singleRecordQuery<ApexTestRunResult>(
+          testRunSummaryQuery,
+          {
+            tooling: true
+          }
+        );
+      return {
+        testsComplete: finishedStatuses.includes(testRunSummaryResults.Status),
+        testRunSummary: 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;
   }
 
   /**
@@ -206,7 +199,7 @@ export class AsyncTests {
     asyncRunResult: AsyncTestRun,
     commandStartTime: number,
     codeCoverage = false,
-    testRunSummary: ApexTestRunResultRecord,
+    testRunSummary: ApexTestRunResult,
     progress?: Progress<ApexTestProgressValue>
   ): Promise<TestResult> {
     const coveredApexClassIdSet = new Set<string>();

src/tests/testService.ts

@@ -378,11 +378,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'));
         }

src/tests/types.ts

@@ -255,7 +255,7 @@ export const enum ApexTestRunResultStatus {
   Skipped = 'Skipped'
 }
 
-export type ApexTestRunResultRecord = {
+export type ApexTestRunResult = {
   /**
    * The parent Apex job ID for the result
    */
@@ -267,23 +267,17 @@ export type ApexTestRunResultRecord = {
   /**
    * The time at which the test run started.
    */
-  StartTime: string;
+  StartTime: string | undefined;
   /**
    * The time it took the test to run, in seconds.
    */
-  TestTime: number;
+  TestTime: number | undefined;
   /**
    * The user who ran the test run
    */
   UserId: string;
 };
 
-export type ApexTestRunResult = {
-  done: boolean;
-  totalSize: number;
-  records: ApexTestRunResultRecord[];
-};
-
 export const enum ApexTestQueueItemStatus {
   Holding = 'Holding',
   Queued = 'Queued',

src/utils/dateUtil.ts

@@ -18,9 +18,13 @@ export function getCurrentTime(): number {
  * @returns formatted date and time
  */
 export function formatStartTime(
-  startTime: string | number,
+  startTime: string | number | undefined,
   format: 'ISO' | 'locale' = 'locale'
 ): string {
+  if (!startTime) {
+    return '';
+  }
+
   const date = new Date(startTime);
   if (format === 'ISO') {
     return date.toISOString();

src/utils/elapsedTime.ts

@@ -7,8 +7,7 @@
 
 /* eslint-disable @typescript-eslint/no-explicit-any,@typescript-eslint/no-unsafe-assignment,@typescript-eslint/no-unsafe-return */
 
-import { Logger, LoggerLevel } from '@salesforce/core';
-import { LoggerLevelValue } from '@salesforce/core/lib/logger/logger';
+import { Logger, LoggerLevel, LoggerLevelValue } from '@salesforce/core';
 
 const log = (
   level: LoggerLevelValue,