better error handling

matepek · Dec 17, 2018 · 8ded3da · 8ded3da
1 parent 9770e33
commit 8ded3da
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 44 deletions.
diff --git a/src/C2AllTestSuiteInfo.ts b/src/C2AllTestSuiteInfo.ts
@@ -31,13 +31,13 @@ export class C2AllTestSuiteInfo implements TestSuiteInfo, vscode.Disposable {
       vscode.EventEmitter<TestLoadStartedEvent | TestLoadFinishedEvent>,
     public readonly testStatesEmitter:
       vscode.EventEmitter<TestRunStartedEvent | TestRunFinishedEvent |
-      TestSuiteEvent | TestEvent>,
+        TestSuiteEvent | TestEvent>,
     public readonly autorunEmitter: vscode.EventEmitter<void>,
     public readonly variableToValue: [string, string][],
     public isEnabledSourceDecoration: boolean,
     public rngSeed: string | number | null,
     public execWatchTimeout: number,
-    public execRunningTimeout: number,
+    public execRunningTimeout: null | number,
     public isNoThrow: boolean,
   ) {
     this.label = workspaceFolder.name + ' - Catch2';

diff --git a/src/C2TestAdapter.ts b/src/C2TestAdapter.ts
@@ -354,8 +354,9 @@ export class C2TestAdapter implements TestAdapter, vscode.Disposable {
   }
 
   private _getDefaultExecRunningTimeout(config: vscode.WorkspaceConfiguration):
-    number {
-    return config.get<number>('defaultRunningTimeoutSec', 0) * 1000;
+    null | number {
+    const r = config.get<null | number>('defaultRunningTimeoutSec', null);
+    return r !== null ? r * 1000 : r;
   }
 
   private _getGlobalAndDefaultEnvironmentVariables(

diff --git a/src/C2TestSuiteInfo.ts b/src/C2TestSuiteInfo.ts
@@ -267,10 +267,11 @@ export class C2TestSuiteInfo implements TestSuiteInfo {
     const startTime = Date.now();
     const killIfTimout = (): Promise<void> => {
       if (data.process === undefined) { return Promise.resolve(); }
-      else if ((Date.now() - startTime) > this.allTests.execRunningTimeout) {
+      else if (this.allTests.execRunningTimeout !== null
+        && Date.now() - startTime > this.allTests.execRunningTimeout) {
+        pResolver && pResolver('catch2TestExplorer.defaultRunningTimeoutSec: ' + this.allTests.execRunningTimeout / 1000);
         data.process.kill();
         data.process = undefined;
-        //TODO reject
         return Promise.resolve();
       } else {
         return promisify(setTimeout)(1000).then(killIfTimout);
@@ -280,29 +281,32 @@ export class C2TestSuiteInfo implements TestSuiteInfo {
 
     return p.catch(
       (reason: any) => {
+        this._proc && this._proc.kill();
+        this._proc = undefined;
+
         this.allTests.log.error(inspect([reason, this, data], true, 2));
-      }).then(() => {
+        return reason;
+      }).then((codeOrReason: number | string) => {
         if (data.inTestCase) {
           if (data.currentChild !== undefined) {
             this.allTests.log.warn('data.currentChild !== undefined: ' + inspect(data));
             this.allTests.testStatesEmitter.fire({
               type: 'test',
               test: data.currentChild!,
               state: 'failed',
-              message: 'Unexpected test error. (Is Catch2 crashed?)\n'
+              message: 'Fatal error: Wrong Catch2 xml output. Error: ' + inspect(codeOrReason) + '\n',
             });
           } else {
             this.allTests.log.warn('data.inTestCase: ' + inspect(data));
           }
         }
 
-        this.allTests.log.info(
-          'proc finished: ' + inspect([this.execPath, execParams]));
+        this.allTests.log.info('proc finished: ' + inspect([this.execPath, execParams]));
 
         this._proc = undefined;
         taskPool.release();
-        this.allTests.testStatesEmitter.fire(
-          { type: 'suite', suite: this, state: 'completed' });
+
+        this.allTests.testStatesEmitter.fire({ type: 'suite', suite: this, state: 'completed' });
 
         const isTestRemoved = (childrenToRun === 'all' &&
           this.children.filter(c => !c.skipped).length >
@@ -313,8 +317,7 @@ export class C2TestSuiteInfo implements TestSuiteInfo {
           this.allTests
             .sendLoadEvents(() => {
               return this.reloadChildren().catch(e => {
-                this.allTests.log.error(
-                  'reloading-error: ' + inspect(e));
+                this.allTests.log.error('reloading-error: ' + inspect(e));
                 // Suite possibly deleted: It is a dead suite.
               });
             })

diff --git a/src/test/C2TestAdapter.test.ts b/src/test/C2TestAdapter.test.ts
@@ -903,7 +903,7 @@ describe('C2TestAdapter', function () {
               type: 'test',
               state: 'failed',
               test: s1t1,
-              message: 'Unexpected test error. (Is Catch2 crashed?)\n'
+              message: 'Fatal error: Wrong Catch2 xml output. Error: 1\n'
             },
             { type: 'suite', state: 'completed', suite: suite1 },
             { type: 'finished' }
@@ -948,7 +948,7 @@ describe('C2TestAdapter', function () {
               type: 'test',
               state: 'failed',
               test: s1t1,
-              message: 'Unexpected test error. (Is Catch2 crashed?)\n'
+              message: 'Fatal error: Wrong Catch2 xml output. Error: \'SIGTERM\'\n'
             },
             { type: 'suite', state: 'completed', suite: suite1 },
             { type: 'finished' }
@@ -973,42 +973,33 @@ describe('C2TestAdapter', function () {
           ]);
         })
 
-        it.skip('should timeout', async function () {
-          await updateConfig('defaultRunningTimeoutSec', 2);
+        it('should timeout', async function () {
+          this.timeout(8000);
+          this.slow(4000);
+          await updateConfig('defaultRunningTimeoutSec', 3);
           await loadAdapterAndAssert();
           const withArgs = spawnStub.withArgs(
             example1.suite1.execPath, example1.suite1.t1.outputs[0][0]);
           const cp = new ChildProcessStub(undefined, 'SIGTERM');
+          const spyKill = <sinon.SinonSpy<never, void>>sinon.spy(cp, 'kill');
           cp.write('<?xml version="1.0" encoding="UTF-8"?><Catch name="suite1">'); // no close
           withArgs.onCall(withArgs.callCount).returns(cp);
 
+          const start = Date.now();
           await adapter.run([s1t1.id]);
+          const elapsed = Date.now() - start;
+          assert.ok(3000 <= elapsed && elapsed <= 5000, elapsed.toString());
+          assert.strictEqual(spyKill.callCount, 1);
+
           cp.close();
 
-          const expected = [
-            { type: 'started', tests: [s1t1.id] },
-            { type: 'suite', state: 'running', suite: suite1 },
-            { type: 'suite', state: 'completed', suite: suite1 },
-            { type: 'finished' }
-          ];
           await waitFor(this, () => {
             return testStatesEvents.length >= 4;
-          })
-          assert.deepStrictEqual(testStatesEvents, expected);
-
-          // this tests the sinon stubs too
-          await adapter.run([s1t1.id]);
+          });
 
           assert.deepStrictEqual(testStatesEvents, [
-            ...expected, { type: 'started', tests: [s1t1.id] },
+            { type: 'started', tests: [s1t1.id] },
             { type: 'suite', state: 'running', suite: suite1 },
-            { type: 'test', state: 'running', test: s1t1 }, {
-              type: 'test',
-              state: 'passed',
-              test: s1t1,
-              decorations: undefined,
-              message: 'Duration: 0.000112 second(s).\n'
-            },
             { type: 'suite', state: 'completed', suite: suite1 },
             { type: 'finished' }
           ]);
@@ -1098,12 +1089,10 @@ describe('C2TestAdapter', function () {
               example1.suite2.execPath, example1.suite2.outputs[2][0]);
             withArgs.onCall(withArgs.callCount).returns(spawnEvent);
           }
-          const run = adapter.run([root.id]);
-          return await run.then(function () {
-            adapter.cancel();
-            assert.equal(spyKill1.callCount, 0);
-            assert.equal(spyKill2.callCount, 0);
-          });
+          await adapter.run([root.id]);
+          adapter.cancel();
+          assert.equal(spyKill1.callCount, 0);
+          assert.equal(spyKill2.callCount, 0);
         })
 
         it('reloads because of fswatcher event: touch(changed)',

diff --git a/src/test/Helpers.ts b/src/test/Helpers.ts
@@ -11,7 +11,7 @@ export class ChildProcessStub extends EventEmitter {
   public closed: boolean = false;
 
   private _read() {
-    this.stdout.push(null);
+    //this.stdout.push(null);
   }
 
   constructor(data?: string | Iterable<string>, close?: number | string) {