master: output formatting and colorization

matepek · Nov 22, 2021 · 03e2ce7 · 03e2ce7
1 parent 56b2e52
commit 03e2ce7
Show file tree

Hide file tree

Showing 9 changed files with 76 additions and 60 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## [Unreleased]
 
+## [4.0.1]
+
+### Changed
+
+- Improved Output formatting and colorization
+
 ## [4.0.0] - 2021-11-22
 
 Replaced [Test Explorer](https://marketplace.visualstudio.com/items?itemName=hbenl.vscode-test-explorer) with native API.

diff --git a/src/AbstractExecutable.ts b/src/AbstractExecutable.ts
@@ -625,6 +625,8 @@ export abstract class AbstractExecutable implements Disposable {
       cancellationToken,
     );
 
+    testRun.appendOutput(runInfo.getProcStartLine());
+
     this.shared.log.info('proc started', runInfo.process.pid, this.properties.path, this.properties, execParams);
 
     runInfo.setPriorityAsync(this.shared.log);
@@ -679,6 +681,8 @@ export abstract class AbstractExecutable implements Disposable {
       );
       const result = await runInfo.result;
 
+      testRun.appendOutput(runInfo.getProcStopLine(result));
+
       if (result.value === ExecutableRunResultValue.Errored) {
         this.shared.log.warn(result.toString(), result, runInfo, this);
         testRun.appendOutput('❌ Executable run is finished with error.');

diff --git a/src/RunningExecutable.ts b/src/RunningExecutable.ts
@@ -1,13 +1,13 @@
 import * as os from 'os';
+import * as ansi from 'ansi-colors';
 
 import { ChildProcessWithoutNullStreams } from './util/FSWrapper';
 import { AbstractTest } from './AbstractTest';
 import { LoggerWrapper } from './LoggerWrapper';
 import { promisify } from 'util';
-import { CancellationToken } from './Util';
-import { debugAssert } from './util/DevelopmentHelper';
+import { CancellationToken, generateId } from './Util';
 import { SpawnBuilder } from './Spawner';
-
+import { assert } from './util/DevelopmentHelper';
 ///
 
 export enum ExecutableRunResultValue {
@@ -20,12 +20,7 @@ export enum ExecutableRunResultValue {
 ///
 
 export class ExecutableRunResult {
-  private constructor(public readonly value: ExecutableRunResultValue, private readonly _error: Error | undefined) {}
-
-  public get error(): Error {
-    debugAssert(this._error);
-    return this._error!;
-  }
+  private constructor(public readonly value: ExecutableRunResultValue, private readonly _error: string | undefined) {}
 
   public get Ok(): boolean {
     return this.value === ExecutableRunResultValue.Ok;
@@ -40,7 +35,8 @@ export class ExecutableRunResult {
       case ExecutableRunResultValue.TimeoutByUser:
         return 'TimeoutByUser';
       case ExecutableRunResultValue.Errored:
-        return this.error.toString();
+        assert(this._error);
+        return this._error!;
     }
   }
 
@@ -49,11 +45,11 @@ export class ExecutableRunResult {
   }
 
   public static Error(message: string): ExecutableRunResult {
-    return new ExecutableRunResult(ExecutableRunResultValue.Errored, Error(message));
+    return new ExecutableRunResult(ExecutableRunResultValue.Errored, message);
   }
 
   public static createFromSignal(signal: string): ExecutableRunResult {
-    return new ExecutableRunResult(ExecutableRunResultValue.Errored, Error('Signal received: ' + signal));
+    return new ExecutableRunResult(ExecutableRunResultValue.Errored, 'Signal received: ' + signal);
   }
 
   public static createFromErrorCode(code: number): ExecutableRunResult {
@@ -107,6 +103,8 @@ export class RunningExecutable {
     });
   }
 
+  public readonly runPrefix = ansi.gray(`$${generateId()}| `);
+
   public killProcess(timeout: number | null = null): void {
     try {
       if (!this._closed && !this._killed) {
@@ -174,6 +172,21 @@ export class RunningExecutable {
   private _killed = false;
 
   public readonly result: Promise<ExecutableRunResult>;
+
+  public getProcStartLine(): string {
+    return (
+      this.runPrefix +
+      ansi.gray(`Started PID#${this.pid} - \`${this.process.spawnfile}\`\r\n`) +
+      this.runPrefix +
+      '\r\n'
+    );
+  }
+
+  public getProcStopLine(result: ExecutableRunResult): string {
+    return (
+      this.runPrefix + ansi.gray(`Stopped PID#${this.pid} - ${result.toString()} - \`${this.process.spawnfile}\`\r\n`)
+    );
+  }
 }
 
 ///

diff --git a/src/TestResultBuilder.ts b/src/TestResultBuilder.ts
@@ -1,11 +1,10 @@
 import * as vscode from 'vscode';
 import * as ansi from 'ansi-colors';
 
-import { generateId, parseLine } from './Util';
+import { parseLine } from './Util';
 import { LoggerWrapper } from './LoggerWrapper';
 import { AbstractTest, SubTest } from './AbstractTest';
 import { debugBreak } from './util/DevelopmentHelper';
-import { RunningExecutable } from './RunningExecutable';
 
 type TestResult = 'skipped' | 'failed' | 'errored' | 'passed';
 
@@ -23,11 +22,6 @@ export class TestResultBuilder<T extends AbstractTest = AbstractTest> {
     this._log = test.shared.log;
   }
 
-  public static calcRunPrefix(_runInfo: RunningExecutable): string {
-    //return `[#${runInfo.pid}] `;
-    return `[#${generateId()}] `;
-  }
-
   private readonly _log: LoggerWrapper;
   private readonly _message: vscode.TestMessage[] = [];
   private _result: TestResult | undefined = undefined;
@@ -39,7 +33,7 @@ export class TestResultBuilder<T extends AbstractTest = AbstractTest> {
     if (this.addBeginEndMsg) {
       const locStr = TestResultBuilder.getLocationAtStr(this.test.file, this.test.line);
       if (this.level === 0) {
-        this.addOutputLine(ansi.bold(`# \`${this.test.label}\` started`) + `${locStr}`);
+        this.addOutputLine(ansi.bold(`# \`${ansi.italic(this.test.label)}\` started`) + `${locStr}`);
       } else {
         this.addOutputLine(-1, prefixForNewSubCase(this.level) + '`' + ansi.italic(this.test.label) + '`' + locStr);
       }
@@ -105,9 +99,9 @@ export class TestResultBuilder<T extends AbstractTest = AbstractTest> {
     if (file) {
       const lineP = parseLine(line);
       if (typeof lineP == 'number') {
-        return ansi.grey(` (at ${file}:${lineP})`);
+        return ansi.grey(` @ ${file}:${lineP}`);
       } else {
-        return ansi.grey(` (at ${file})`);
+        return ansi.grey(` @ ${file}`);
       }
     }
     return '';
@@ -135,7 +129,7 @@ export class TestResultBuilder<T extends AbstractTest = AbstractTest> {
     this.addMessage(file, line, 'Expanded: `' + expanded + '`');
 
     const loc = TestResultBuilder.getLocationAtStr(file, line);
-    this.addOutputLine(1, `Expression failed${loc}:`);
+    this.addOutputLine(1, 'Expression ' + ansi.red('failed') + loc + ':');
     this.addOutputLine(2, '❕Original:  ' + original);
     this.addOutputLine(2, '❗️Expanded:  ' + expanded);
   }
@@ -173,7 +167,6 @@ export class TestResultBuilder<T extends AbstractTest = AbstractTest> {
     const loc = TestResultBuilder.getLocationAtStr(file, line);
     this.addOutputLine(1, `${title}${loc}${message.length ? ':' : ''}`);
     this.addOutputLine(2, ...message);
-    //if (message.length) this.addOutputLine(1, `⬆ ${title}`);
   }
 
   ///
@@ -183,11 +176,11 @@ export class TestResultBuilder<T extends AbstractTest = AbstractTest> {
       case 'passed':
         return ansi.green(this._result);
       case 'failed':
-        return ansi.red(this._result);
+        return ansi.bold.red(this._result);
       case 'skipped':
         return this._result;
       case 'errored':
-        return ansi.bgRedBright(this._result);
+        return ansi.bold.bgRed(this._result);
       case undefined:
         return '';
     }
@@ -198,10 +191,11 @@ export class TestResultBuilder<T extends AbstractTest = AbstractTest> {
       const d = this._duration ? ansi.grey(` in ${Math.round(this._duration * 1000) / 1000000} second(s)`) : '';
 
       if (this.level === 0) {
-        this.addOutputLine(ansi.bold(`# \`${this.test.label}\` ${this.coloredResult()}`) + `${d}`, '');
-      } else if (this._result !== 'passed') {
-        this.addOutputLine(ansi.bold(`# ${this.coloredResult()}`) + `${d}`);
+        this.addOutputLine(`# \`${ansi.italic(this.test.label)}\` ${this.coloredResult()}` + `${d}`, '');
       }
+      // else if (this._result !== 'passed') {
+      //   this.addOutputLine(`# ${this.coloredResult()}${d}`);
+      // }
     }
   }
 
@@ -263,11 +257,11 @@ export class TestResultBuilder<T extends AbstractTest = AbstractTest> {
 
 ///
 
-const indentPrefix = '| ';
+const indentPrefix = ansi.grey('| ');
 
 const prefixForNewSubCase = (indentLevel: number): string => {
   if (indentLevel === 0) return '';
-  else return indentPrefix.repeat(indentLevel - 1) + '| ';
+  else return indentPrefix.repeat(indentLevel - 1) + ansi.grey('| ');
 };
 
 const reindentLines = (indentLevel: number, lines: string[]): string[] => {

diff --git a/src/framework/Catch2Executable.ts b/src/framework/Catch2Executable.ts
@@ -330,7 +330,6 @@ export class Catch2Executable extends AbstractExecutable {
     const executable = this; //eslint-disable-line
     const log = this.shared.log;
     let rngSeed: number | undefined = undefined;
-    const runPrefix = TestResultBuilder.calcRunPrefix(runInfo);
 
     const parser = new XmlParser(
       this.shared.log,
@@ -356,7 +355,7 @@ export class Catch2Executable extends AbstractExecutable {
               } else {
                 expectedToRunAndFoundTests.push(test);
               }
-              const builder = new TestResultBuilder(test, testRun, runPrefix, true);
+              const builder = new TestResultBuilder(test, testRun, runInfo.runPrefix, true);
               return new TestCaseTagProcessor(executable.shared, builder, test, tag.attribs);
             }
           }
@@ -374,7 +373,7 @@ export class Catch2Executable extends AbstractExecutable {
 
       parser.writeStdErr(c).then(hasHandled => {
         if (!hasHandled) {
-          executable.processStdErr(testRun, runPrefix, c);
+          executable.processStdErr(testRun, runInfo.runPrefix, c);
         }
       });
     });

diff --git a/src/framework/DOCExecutable.ts b/src/framework/DOCExecutable.ts
@@ -185,7 +185,6 @@ export class DOCExecutable extends AbstractExecutable {
     const expectedToRunAndFoundTests: DOCTest[] = [];
     const executable = this; //eslint-disable-line
     let options: Option = {};
-    const runPrefix = TestResultBuilder.calcRunPrefix(runInfo);
 
     const parser = new XmlParser(
       this.shared.log,
@@ -200,7 +199,7 @@ export class DOCExecutable extends AbstractExecutable {
               return new TestSuiteTagProcessor(
                 executable.shared,
                 testRun,
-                runPrefix,
+                runInfo.runPrefix,
                 (testNameAsId: string) => executable._getTest<DOCTest>(testNameAsId),
                 executable._createAndAddTest,
                 unexpectedTests,

diff --git a/src/framework/GoogleBenchmarkExecutable.ts b/src/framework/GoogleBenchmarkExecutable.ts
@@ -128,7 +128,6 @@ export class GoogleBenchmarkExecutable extends AbstractExecutable {
   protected async _handleProcess(testRun: vscode.TestRun, runInfo: RunningExecutable): Promise<HandleProcessResult> {
     // at first it's good enough
     runInfo.childrenToRun.forEach(test => testRun.started(test.item));
-    const runPrefix = TestResultBuilder.calcRunPrefix(runInfo);
 
     const unexpectedTests: GoogleBenchmarkTest[] = [];
     const expectedToRunAndFoundTests: GoogleBenchmarkTest[] = [];
@@ -185,7 +184,7 @@ export class GoogleBenchmarkExecutable extends AbstractExecutable {
             expectedToRunAndFoundTests.push(test);
           }
 
-          const builder = new TestResultBuilder(test, testRun, runPrefix, true);
+          const builder = new TestResultBuilder(test, testRun, runInfo.runPrefix, true);
           parseAndProcessTestCase(this.shared.log, builder, benchmark);
 
           data.lastProcessedBenchmarkIndex = i;
@@ -199,7 +198,7 @@ export class GoogleBenchmarkExecutable extends AbstractExecutable {
       data.sequentialProcessP = data.sequentialProcessP.then(() => processChunk(chunk.toLocaleString()));
     });
     runInfo.process.stderr.on('data', (chunk: Uint8Array) =>
-      this.processStdErr(testRun, runPrefix, chunk.toLocaleString()),
+      this.processStdErr(testRun, runInfo.runPrefix, chunk.toLocaleString()),
     );
 
     await runInfo.result;