Merge pull request #72 from apex-dev-tools/feedback-fixes

Update report formatting and fix minor issues

Author: pwrightcertinia

CHANGELOG.md

@@ -1,5 +1,17 @@
 # test-runner - Changelog
 
+## 3.1.0 - 2023-11-24
+
+* Format millisecond/second time values in reports as numbers.
+  * For JSON: `"time": "12030 ms"` is now `"time": 12030`.
+  * For JUnit XML: `value="12.03 s"` is now `value="12.03"`.
+* Add elapsed time to test run status logging.
+* Add `rerunExecutionTime` value to main test reports.
+  * This is sum of all the re-run `RunTime`s. Similar to `testExecutionTime`.
+* Remove spaces in class time CSV report.
+* Add output of class time report to JSON.
+* Optimise missing test re-run request payload when requesting all tests in a class.
+
 ## 3.0.0 - 2023-10-23
 
 * **BREAKING**: `AsyncTestRunner` now does not re-throw errors. Instead it returns a `TestRunnerResult` type which includes all test results retrieved and optional error.

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@apexdevtools/test-runner",
-  "version": "3.0.0",
+  "version": "3.1.0",
   "description": "Apex parallel test runner with reliability goodness",
   "author": {
     "name": "Apex Dev Tools Team",
@@ -37,7 +37,7 @@
     "url": "https://github.com/apex-dev-tools/test-runner/issues"
   },
   "homepage": "https://github.com/apex-dev-tools/test-runner#readme",
-  "packageManager": "pnpm@8.2.0",
+  "packageManager": "pnpm@8.9.2",
   "dependencies": {
     "@apexdevtools/sfdx-auth-helper": "^2.1.0",
     "@salesforce/apex-node": "^1.6.2",

src/command/Testall.ts

@@ -194,20 +194,18 @@ export class Testall {
 
     // Filter expected by actual results to find residual
     // Try again if something was missed
-    const missingTests = await this.resolveMissingTests(
-      expectedTestsPromise,
-      store.tests
-    );
+    const expectedTests = await expectedTestsPromise;
+    const missingTests = this.resolveMissingTests(expectedTests, store.tests);
 
     if (missingTests.size > 0) {
       this._logger.logTestallRerun(missingTests);
 
       const testItems: TestItem[] = Array.from(
         missingTests,
-        ([className, methods]) => ({
-          className,
-          testMethods: Array.from(methods),
-        })
+        ([className, methods]) =>
+          methods.size === expectedTests.get(className)?.size
+            ? { className } // run all methods
+            : { className, testMethods: Array.from(methods) }
       );
 
       await this.asyncRun(
@@ -219,11 +217,10 @@ export class Testall {
     }
   }
 
-  private async resolveMissingTests(
-    expectedTestsPromise: Promise<Map<string, Set<string>>>,
+  private resolveMissingTests(
+    expectedTests: Map<string, Set<string>>,
     results: Map<string, ApexTestResult>
-  ): Promise<Map<string, Set<string>>> {
-    const expectedTests = await expectedTestsPromise;
+  ): Map<string, Set<string>> {
     const missingTests = new Map<string, Set<string>>();
 
     expectedTests.forEach((methods, className) => {
@@ -314,7 +311,7 @@ export class Testall {
 
       return this.convertToSyncResult(result, timestamp);
     } catch (err) {
-      this._logger.logMessage(
+      this._logger.logErrorMessage(
         `${getTestName(currentResult)} re-run failed. ${this.getErrorMsg(err)}`
       );
     }
@@ -383,7 +380,7 @@ export class Testall {
 
       store.saveCoverage(coverage);
     } catch (err) {
-      this._logger.logMessage(
+      this._logger.logErrorMessage(
         `Failed to get coverage: ${this.getErrorMsg(err)}`
       );
     }

src/log/BaseLogger.ts

@@ -3,6 +3,7 @@
  */
 
 import path from 'path';
+import os from 'os';
 import { TestallOptions, getMaxErrorsForReRun } from '../command/Testall';
 import { ApexTestResult, BaseTestResult } from '../model/ApexTestResult';
 import { ApexTestRunResult } from '../model/ApexTestRunResult';
@@ -26,27 +27,31 @@ export abstract class BaseLogger implements Logger {
   logError(error: MaybeError): void {
     if (error instanceof Error) {
       if (error.name == 'ALREADY_IN_PROCESS') {
-        this.logMessage(
+        this.logErrorMessage(
           "One or more of the tests is already queued to run, they can't be requeued"
         );
       } else {
-        this.logMessage(error.message);
+        this.logErrorMessage(error.message);
         if (error.stack !== undefined)
-          this.logMessage('Error stack: ' + error.stack);
+          this.logErrorMessage('Error stack: ' + error.stack);
       }
     } else {
-      this.logMessage('Error: ' + JSON.stringify(error));
+      this.logErrorMessage('Error: ' + JSON.stringify(error));
     }
 
     if (error.data !== undefined) {
-      this.logMessage('Additional data: ' + JSON.stringify(error.data));
+      this.logErrorMessage('Additional data: ' + JSON.stringify(error.data));
     }
   }
 
   logWarning(message: string): void {
     this.logMessage('Warning: ' + message);
   }
 
+  logErrorMessage(message: string): void {
+    this.logMessage(message);
+  }
+
   logOutputFile(filepath: string, contents: string): void {
     // if filepath is absolute it will be used instead
     // given resolve() right to left logic
@@ -106,10 +111,12 @@ export abstract class BaseLogger implements Logger {
     // i.e its failed with a different message, show what happened
     if (rerunMsg && firstMsg) {
       if (rerunMsg !== firstMsg) {
-        this.logMessage(` [Before] ${firstMsg}`);
-        this.logMessage(` [After] ${rerunMsg}`);
+        this.logErrorMessage(`${os.EOL} [Before] ${firstMsg}`);
+        this.logErrorMessage(` [After] ${rerunMsg}${os.EOL}`);
       } else {
-        this.logMessage(` [Before and After] ${rerunMsg}`);
+        this.logErrorMessage(
+          `${os.EOL} [Before and After] ${rerunMsg}${os.EOL}`
+        );
       }
     }
   }
@@ -134,7 +141,11 @@ export abstract class BaseLogger implements Logger {
     );
   }
 
-  logStatus(testRunResult: ApexTestRunResult, tests: ApexTestResult[]): void {
+  logStatus(
+    testRunResult: ApexTestRunResult,
+    tests: ApexTestResult[],
+    elapsedTime: string
+  ): void {
     const status = testRunResult.Status;
     const outcomes = groupByOutcome(tests);
     const completed = tests.length;
@@ -144,7 +155,7 @@ export abstract class BaseLogger implements Logger {
     const complete = total > 0 ? Math.floor((completed * 100) / total) : 0;
 
     this.logMessage(
-      `[${status}] Passed: ${passed} | Failed: ${failed} | ${completed}/${total} Complete (${complete}%)`
+      `${elapsedTime} [${status}] Passed: ${passed} | Failed: ${failed} | ${completed}/${total} Complete (${complete}%)`
     );
   }
 
@@ -159,16 +170,22 @@ export abstract class BaseLogger implements Logger {
 
     Object.entries(failedResultsByClassId).forEach(([, results]) => {
       const tests = results.slice(0, 2);
+      const hasMore = results.length > 2;
 
-      this.logMessage(`  Failing Tests: ${getClassName(tests[0])}`);
+      this.logErrorMessage(
+        `${os.EOL}  Failing tests in '${getClassName(tests[0])}':`
+      );
 
-      tests.forEach(t => {
+      tests.forEach((t, i) => {
         const msg = t.Message ? ` - ${t.Message}` : '';
-        this.logMessage(`    * ${t.MethodName}${msg}`);
+        const suffix = !hasMore && i == tests.length - 1 ? os.EOL : '';
+        this.logErrorMessage(`    * ${t.MethodName}${msg}${suffix}`);
       });
 
-      results.length > 2 &&
-        this.logMessage(`    (and ${results.length - 2} more...)`);
+      hasMore &&
+        this.logErrorMessage(
+          `    (and ${results.length - 2} more...)${os.EOL}`
+        );
     });
   }
 

src/log/Logger.ts

@@ -13,6 +13,7 @@ export interface Logger {
 
   // For general use
   logError(error: any): void;
+  logErrorMessage(message: any): void;
   logWarning(message: any): void;
   logMessage(message: any): void;
 
@@ -34,7 +35,11 @@ export interface Logger {
   // Test runner
   logRunStarted(testRunId: string): void;
   logNoProgress(testRunId: string): void;
-  logStatus(status: ApexTestRunResult, tests: ApexTestResult[]): void;
+  logStatus(
+    status: ApexTestRunResult,
+    tests: ApexTestResult[],
+    elapsedTime: string
+  ): void;
   logTestFailures(newResults: ApexTestResult[]): void;
 
   // Test job cancelling

src/results/ClassTimeGenerator.ts

@@ -8,7 +8,7 @@ import { SfDate } from 'jsforce';
 import path from 'path';
 
 /*
- * Create a report (CSV) of summary stats for each test class. The report can be useful in finding long running
+ * Create a report (CSV/JSON) of summary stats for each test class. The report can be useful in finding long running
  * test which are delaying the completion of a test run.
  */
 export class ClassTimeGenerator implements OutputGenerator {
@@ -52,13 +52,33 @@ export class ClassTimeGenerator implements OutputGenerator {
     // Report results as CSV
     const lines: string[] = [];
     classRanges.forEach((v, k) => {
-      lines.push(`${k}, ${v[0]}, ${v[1]}, ${v[2]}`);
+      lines.push(`${k},${v[0]},${v[1]},${v[2]}`);
     });
     logger.logOutputFile(
       path.join(outputDirBase, fileName + '-time.csv'),
-      'ClassName, StartTime, EndTime, TotalTime\n' +
+      'ClassName,StartTime,EndTime,TotalTime\n' +
         `# ${this.instanceUrl} ${this.orgId} ${this.username}\n` +
         lines.join('\n')
     );
+
+    // Report results as json
+    const json: {
+      className: string;
+      startTime: number;
+      endTime: number;
+      totalTime: number;
+    }[] = [];
+    classRanges.forEach((v, k) => {
+      json.push({
+        className: k,
+        startTime: v[0],
+        endTime: v[1],
+        totalTime: v[2],
+      });
+    });
+    logger.logOutputFile(
+      path.join(outputDirBase, fileName + '-time.json'),
+      JSON.stringify(json, undefined, 2)
+    );
   }
 }