From 9111e360397815e624ebb814a8b5777a180fed74 Mon Sep 17 00:00:00 2001 From: Mate Pek Date: Thu, 6 Dec 2018 13:29:43 +0100 Subject: [PATCH 1/6] runningtimeout begins --- CHANGELOG.md | 6 +++- README.md | 29 +++++++++--------- package.json | 9 ++++++ src/C2AllTestSuiteInfo.ts | 1 + src/C2TestAdapter.ts | 13 ++++++++ src/C2TestSuiteInfo.ts | 43 ++++++++++++++++++-------- src/test/C2TestAdapter.test.ts | 55 ++++++++++++++++++++++++++++++++-- src/test/Helpers.ts | 4 +-- 8 files changed, 127 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f6b1c39d..43f44a52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,10 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -## [2.2.6] +## [2.3.0] Stability improvements. +### Added + +- `catch2TestExplorer.defaultRunningTimeoutSec` to prevent infinite loops + ## [2.2.5] - 2018-12-04 ### Fixed diff --git a/README.md b/README.md index cddde498..ed69ae45 100644 --- a/README.md +++ b/README.md @@ -12,20 +12,21 @@ This extension allows you to run your [Catch2 tests](https://github.com/catchorg ## Configuration -| Property | Description | -| ------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `catch2TestExplorer.executables` | The location of your test executables (relative to the workspace folder or absolute path) and with a lot of other setting. Details: [below](#catch2TestExplorer.executables) | -| `catch2TestExplorer.defaultCwd` | The working directory where the test is run (relative to the workspace folder or absolue path), if it isn't provided in "executables". (Resolves: \${workspaceFolder}) | -| `catch2TestExplorer.defaultEnv` | Default environment variables to be set when running the tests, if it isn't provided in 'executables'. (Resolves: \${workspaceFolder}) | -| `catch2TestExplorer.debugConfigTemplate` | Set the necessary debug configuraitons and the debug button will work. Details: [below](#catch2TestExplorer.debugConfigTemplate) | -| `catch2TestExplorer.debugBreakOnFailure` | Debugger breaks on failure while debugging the test. This is a Catch2 parameter: --break | -| `catch2TestExplorer.defaultNoThrow` | Skips all assertions that test that an exception is thrown, e.g. REQUIRE_THROWS. This is a Catch2 parameter: --nothrow | -| `catch2TestExplorer.defaultRngSeed` | Specify a seed for the Random Number Generator. For details see [Catch2 documentation](https://github.com/catchorg/Catch2/blob/master/docs/command-line.md#rng-seed) | -| `catch2TestExplorer.defaultWatchTimeoutSec` | Test executables are being watched. In case of one compiles too much this variable can help with it. Unit: second. | -| `catch2TestExplorer.enableSourceDecoration` | Sets the source code decorations: Errored lines will be highlited. | -| `catch2TestExplorer.workerMaxNumber` | The variable maximize the number of the parallel test execution. | -| `testExplorer.onStart` | (This is part of the [dependency extension](https://github.com/hbenl/vscode-test-explorer#configuration)'s settings.) | -| `testExplorer.onReload` | (This is part of the [dependency extension](https://github.com/hbenl/vscode-test-explorer#configuration)'s settings.) | +| Property | Description | +| --------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `catch2TestExplorer.executables` | The location of your test executables (relative to the workspace folder or absolute path) and with a lot of other setting. Details: [below](#catch2TestExplorer.executables) | +| `catch2TestExplorer.defaultCwd` | The working directory where the test is run (relative to the workspace folder or absolue path), if it isn't provided in "executables". (Resolves: \${workspaceFolder}) | +| `catch2TestExplorer.defaultEnv` | Default environment variables to be set when running the tests, if it isn't provided in 'executables'. (Resolves: \${workspaceFolder}) | +| `catch2TestExplorer.debugConfigTemplate` | Set the necessary debug configuraitons and the debug button will work. Details: [below](#catch2TestExplorer.debugConfigTemplate) | +| `catch2TestExplorer.debugBreakOnFailure` | Debugger breaks on failure while debugging the test. This is a Catch2 parameter: --break | +| `catch2TestExplorer.defaultNoThrow` | Skips all assertions that test that an exception is thrown, e.g. REQUIRE_THROWS. This is a Catch2 parameter: --nothrow | +| `catch2TestExplorer.defaultRngSeed` | Specify a seed for the Random Number Generator. For details see [Catch2 documentation](https://github.com/catchorg/Catch2/blob/master/docs/command-line.md#rng-seed) | +| `catch2TestExplorer.defaultWatchTimeoutSec` | Test executables are being watched. In case of one compiles too much this variable can help with it. Unit: second. | +| `catch2TestExplorer.defaultRunningTimeoutSec` | Test executable is running in a process. In case of an inifinite loop, it will run forever, unless this parameter is set. | +| `catch2TestExplorer.enableSourceDecoration` | Sets the source code decorations: Errored lines will be highlited. | +| `catch2TestExplorer.workerMaxNumber` | The variable maximize the number of the parallel test execution. | +| `testExplorer.onStart` | (This is part of the [dependency extension](https://github.com/hbenl/vscode-test-explorer#configuration)'s settings.) | +| `testExplorer.onReload` | (This is part of the [dependency extension](https://github.com/hbenl/vscode-test-explorer#configuration)'s settings.) | ### catch2TestExplorer.executables diff --git a/package.json b/package.json index 09e623fc..01afa187 100644 --- a/package.json +++ b/package.json @@ -129,6 +129,15 @@ "default": 10, "scope": "resource" }, + "catch2TestExplorer.defaultRunningTimeoutSec": { + "description": "Test executable is running in a process. In case of an inifinite loop, it will run forever, unless this parameter is set.", + "type": [ + "null", + "number" + ], + "default": null, + "scope": "resource" + }, "catch2TestExplorer.workerMaxNumber": { "description": "The variable maximize the number of the parallel test execution.", "type": "number", diff --git a/src/C2AllTestSuiteInfo.ts b/src/C2AllTestSuiteInfo.ts index 7cbd595f..00a64f83 100644 --- a/src/C2AllTestSuiteInfo.ts +++ b/src/C2AllTestSuiteInfo.ts @@ -37,6 +37,7 @@ export class C2AllTestSuiteInfo implements TestSuiteInfo, vscode.Disposable { public isEnabledSourceDecoration: boolean, public rngSeed: string | number | null, public execWatchTimeout: number, + public execRunningTimeout: number, public isNoThrow: boolean, ) { this.label = workspaceFolder.name + ' - Catch2'; diff --git a/src/C2TestAdapter.ts b/src/C2TestAdapter.ts index 54af6d0b..d80ef3c4 100644 --- a/src/C2TestAdapter.ts +++ b/src/C2TestAdapter.ts @@ -88,6 +88,12 @@ export class C2TestAdapter implements TestAdapter, vscode.Disposable { this._allTests.execWatchTimeout = this._getDefaultExecWatchTimeout(this._getConfiguration()); } + if (configChange.affectsConfiguration( + 'catch2TestExplorer.defaultRunningTimeoutSec', + this._workspaceFolder.uri)) { + this._allTests.execRunningTimeout = + this._getDefaultExecRunningTimeout(this._getConfiguration()); + } if (configChange.affectsConfiguration( 'catch2TestExplorer.defaultNoThrow', this._workspaceFolder.uri)) { @@ -104,6 +110,7 @@ export class C2TestAdapter implements TestAdapter, vscode.Disposable { this._getEnableSourceDecoration(config), this._getDefaultRngSeed(config), this._getDefaultExecWatchTimeout(config), + this._getDefaultExecRunningTimeout(config), this._getDefaultNoThrow(config)); } @@ -142,6 +149,7 @@ export class C2TestAdapter implements TestAdapter, vscode.Disposable { this._getEnableSourceDecoration(config), this._getDefaultRngSeed(config), this._getDefaultExecWatchTimeout(config), + this._getDefaultExecRunningTimeout(config), this._getDefaultNoThrow(config)); this._testsEmitter.fire({ type: 'started' }); @@ -358,6 +366,11 @@ export class C2TestAdapter implements TestAdapter, vscode.Disposable { return config.get('defaultWatchTimeoutSec', 10) * 1000; } + private _getDefaultExecRunningTimeout(config: vscode.WorkspaceConfiguration): + number { + return config.get('defaultRunningTimeoutSec', 0) * 1000; + } + private _getGlobalAndDefaultEnvironmentVariables( config: vscode.WorkspaceConfiguration): { [prop: string]: string | undefined } { const processEnv = process.env; diff --git a/src/C2TestSuiteInfo.ts b/src/C2TestSuiteInfo.ts index f3fd3f19..9f5ec33d 100644 --- a/src/C2TestSuiteInfo.ts +++ b/src/C2TestSuiteInfo.ts @@ -4,7 +4,7 @@ import { ChildProcess, spawn, SpawnOptions } from 'child_process'; import * as path from 'path'; -import { inspect } from 'util'; +import { inspect, promisify } from 'util'; import { TestEvent, TestSuiteInfo } from 'vscode-test-adapter-api'; import * as xml2js from 'xml2js'; @@ -78,15 +78,15 @@ export class C2TestSuiteInfo implements TestSuiteInfo { this._isKill = false; this._proc = undefined; + let childrenToRun: 'all' | C2TestInfo[] = 'all'; + if (tests.delete(this.id)) { for (let i = 0; i < this.children.length; i++) { const c = this.children[i]; tests.delete(c.id); } - - return this._runInner('all', taskPool); } else { - let childrenToRun: C2TestInfo[] = []; + childrenToRun = []; for (let i = 0; i < this.children.length; i++) { const c = this.children[i]; @@ -94,9 +94,9 @@ export class C2TestSuiteInfo implements TestSuiteInfo { } if (childrenToRun.length == 0) return Promise.resolve(); - - return this._runInner(childrenToRun, taskPool); } + + return this._runInner(childrenToRun, taskPool); } private _runInner(childrenToRun: C2TestInfo[] | 'all', taskPool: TaskPool): @@ -153,6 +153,7 @@ export class C2TestSuiteInfo implements TestSuiteInfo { }); const data = new class { + process: ChildProcess | undefined = undefined; buffer: string = ''; inTestCase: boolean = false; currentChild: C2TestInfo | undefined = undefined; @@ -160,8 +161,8 @@ export class C2TestSuiteInfo implements TestSuiteInfo { rngSeed: number | undefined = undefined; unporessedXmlTestCases: string[] = []; processedTestCases: C2TestInfo[] = []; - } - (); + }(); + data.process = this._proc; const testCaseTagRe = //; @@ -253,6 +254,8 @@ export class C2TestSuiteInfo implements TestSuiteInfo { }); this._proc.on('close', (code: number | null, signal: string | null) => { + data.process = undefined; + if (code !== null && code !== undefined) pResolver && pResolver(code); if (signal !== null && signal !== undefined) @@ -261,9 +264,23 @@ export class C2TestSuiteInfo implements TestSuiteInfo { pRejecter && pRejecter('unknown'); }); + const startTime = Date.now(); + const killIfTimout = (): Promise => { + if (data.process === undefined) { return Promise.resolve(); } + else if ((Date.now() - startTime) > this.allTests.execRunningTimeout) { + data.process.kill(); + data.process = undefined; + //TODO reject + return Promise.resolve(); + } else { + return promisify(setTimeout)(1000).then(killIfTimout); + } + }; + promisify(setTimeout)(1500).then(killIfTimout); + return p.catch( (reason: any) => { - this.allTests.log.error(inspect(reason)); + this.allTests.log.error(inspect([reason, this, data], true, 2)); }).then(() => { if (data.inTestCase) { if (data.currentChild !== undefined) { @@ -393,18 +410,18 @@ export class C2TestSuiteInfo implements TestSuiteInfo { if (match && match.length == 5) { const matchedPath = match[1] ? match[1] : match[3]; - filePath = path.resolve(this.allTests.workspaceFolder.uri.fsPath, matchedPath); + filePath = path.join(this.allTests.workspaceFolder.uri.fsPath, matchedPath); try { if (!c2fs.existsSync(filePath) && this.execOptions.cwd) { - filePath = path.resolve(this.execOptions.cwd, matchedPath); + filePath = path.join(this.execOptions.cwd, matchedPath); } if (!c2fs.existsSync(filePath)) { let parent = path.dirname(this.execPath); - filePath = path.resolve(parent, matchedPath); + filePath = path.join(parent, matchedPath); let parentParent = path.dirname(parent); while (!c2fs.existsSync(filePath) && parent != parentParent) { parent = parentParent; - filePath = path.resolve(parent, matchedPath); + filePath = path.join(parent, matchedPath); parentParent = path.dirname(parent); } } diff --git a/src/test/C2TestAdapter.test.ts b/src/test/C2TestAdapter.test.ts index 4182a0b7..17b4d04e 100644 --- a/src/test/C2TestAdapter.test.ts +++ b/src/test/C2TestAdapter.test.ts @@ -254,6 +254,12 @@ describe('C2TestAdapter', function () { }); }) + it('defaultRunningTimeoutSec', function () { + return updateConfig('defaultRunningTimeoutSec', 8765).then(function () { + assert.equal((adapter)._allTests.execRunningTimeout, 8765000); + }); + }) + it('defaultNoThrow', function () { return updateConfig('defaultNoThrow', true).then(function () { assert.equal((adapter)._allTests.isNoThrow, true); @@ -367,7 +373,7 @@ describe('C2TestAdapter', function () { }) describe('load', function () { - this.slow(500); + this.slow(1000); const uniqueIdC = new Set(); let adapter: TestAdapter; @@ -404,9 +410,9 @@ describe('C2TestAdapter', function () { assert.deepStrictEqual(testStatesEvents, []); } - beforeEach(function () { + beforeEach(async function () { this.timeout(8000); - return updateConfig('workerMaxNumber', 3); + await updateConfig('workerMaxNumber', 3); }) afterEach(function () { @@ -950,6 +956,49 @@ describe('C2TestAdapter', function () { // this tests the sinon stubs too await adapter.run([s1t1.id]); + + assert.deepStrictEqual(testStatesEvents, [ + ...expected, { 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' } + ]); + }) + + it.only('should timeout', async function () { + await updateConfig('defaultRunningTimeoutSec', 2); + await loadAdapterAndAssert(); + const withArgs = spawnStub.withArgs( + example1.suite1.execPath, example1.suite1.t1.outputs[0][0]); + const cp = new ChildProcessStub(undefined, 'SIGTERM'); + cp.write(''); // no close + withArgs.onCall(withArgs.callCount) + .returns(cp); + + await adapter.run([s1t1.id]); + 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: 'suite', state: 'running', suite: suite1 }, diff --git a/src/test/Helpers.ts b/src/test/Helpers.ts index 3bbe2680..fda5cfc5 100644 --- a/src/test/Helpers.ts +++ b/src/test/Helpers.ts @@ -21,7 +21,7 @@ export class ChildProcessStub extends EventEmitter { else this.emit('close', close, null); }); - if (data != undefined) { + if (data !== undefined) { if (typeof data !== 'string') { for (let line of data) { this.write(line); @@ -34,7 +34,7 @@ export class ChildProcessStub extends EventEmitter { } kill() { - this.stdout.emit('end'); + this.stdout.push(null); } write(data: string): void { From 4ffb6ccd026ef766bb45c05500042467a0fef17a Mon Sep 17 00:00:00 2001 From: Mate Pek Date: Sat, 15 Dec 2018 13:13:09 +0100 Subject: [PATCH 2/6] readme fixes --- .github/ISSUE_TEMPLATE/bug_report.md | 21 ++++++++------------- README.md | 2 +- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index f2ddf0ee..c92c64ea 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -3,22 +3,20 @@ name: Bug report about: Create a report to help us improve --- -**Describe the bug** -A clear and concise description of what the bug is. +**Before you file an issue, please check the [README.md](https://github.com/matepek/vscode-catch2-test-adapter#known-issues)'s known issues section.** -**To Reproduce** -Steps to reproduce the behavior: +**Describe the bug** (A clear and concise description of what the bug is.) + +**To Reproduce** (Steps to reproduce the behavior:) 1. Go to '...' 2. Click on '....' 3. Scroll down to '....' 4. See error -**Expected behavior** -A clear and concise description of what you expected to happen. +**Expected behavior** (A clear and concise description of what you expected to happen.) -**Screenshots** -If applicable, add screenshots to help explain your problem. +**Screenshots** (If applicable, add screenshots to help explain your problem.) **Desktop (please complete the following information):** @@ -27,9 +25,6 @@ If applicable, add screenshots to help explain your problem. - Catch2 Version - OS Type and Version -**Log** -Attach log. (optional, Set: `catch2TestExplorer.logfile` and reproduce the bug.) -_Warning_: Log probably contains file and test names too. Consider security concerns before you attach it! +**Log** (Attach log. (optional, Set: `catch2TestExplorer.logfile` and reproduce the bug.) _Warning_: Log probably contains file and test names too.) -**Additional context** -Add any other context about the problem here. +**Additional context** Add any other context about the problem here. diff --git a/README.md b/README.md index f768c2c9..b4544c22 100644 --- a/README.md +++ b/README.md @@ -141,7 +141,7 @@ Example: ## Known issues - (2018-09-03) On windows the navigate to source button isn't working. It is a framework bug. -- (2018-11-17) Long (>80 character) filename, test-name or description can cause test-list parsing failures. Workaround: `#define CATCH_CONFIG_CONSOLE_WIDTH 9999` +- (2018-11-17) Long (>80 character) filename, test-name or description can cause test-list parsing failures. Workaround: `#define CATCH_CONFIG_CONSOLE_WIDTH 300` ## TODOs From 9503a49676d2883b688de3c5306e25d17a0f2214 Mon Sep 17 00:00:00 2001 From: Mate Pek Date: Sat, 15 Dec 2018 13:14:54 +0100 Subject: [PATCH 3/6] npm update --- package-lock.json | 72 +++++++++++++++++++++-------------------------- package.json | 4 +-- 2 files changed, 34 insertions(+), 42 deletions(-) diff --git a/package-lock.json b/package-lock.json index a2b4664a..4afe4c7b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -14,30 +14,24 @@ } }, "@sinonjs/formatio": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/@sinonjs/formatio/-/formatio-3.0.0.tgz", - "integrity": "sha512-vdjoYLDptCgvtJs57ULshak3iJe4NW3sJ3g36xVDGff5AE8P30S6A093EIEPjdi2noGhfuNOEkbxt3J3awFW1w==", + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/@sinonjs/formatio/-/formatio-3.1.0.tgz", + "integrity": "sha512-ZAR2bPHOl4Xg6eklUGpsdiIJ4+J1SNag1DHHrG/73Uz/nVwXqjgUtRPLoS+aVyieN9cSbc0E4LsU984tWcDyNg==", "dev": true, "requires": { - "@sinonjs/samsam": "2.1.0" - }, - "dependencies": { - "@sinonjs/samsam": { - "version": "2.1.0", - "resolved": "https://registry.npmjs.org/@sinonjs/samsam/-/samsam-2.1.0.tgz", - "integrity": "sha512-5x2kFgJYupaF1ns/RmharQ90lQkd2ELS8A9X0ymkAAdemYHGtI2KiUHG8nX2WU0T1qgnOU5YMqnBM2V7NUanNw==", - "dev": true, - "requires": { - "array-from": "^2.1.1" - } - } + "@sinonjs/samsam": "^2 || ^3" } }, "@sinonjs/samsam": { - "version": "2.1.2", - "resolved": "https://registry.npmjs.org/@sinonjs/samsam/-/samsam-2.1.2.tgz", - "integrity": "sha512-ZwTHAlC9akprWDinwEPD4kOuwaYZlyMwVJIANsKNC3QVp0AHB04m7RnB4eqeWfgmxw8MGTzS9uMaw93Z3QcZbw==", - "dev": true + "version": "3.0.2", + "resolved": "https://registry.npmjs.org/@sinonjs/samsam/-/samsam-3.0.2.tgz", + "integrity": "sha512-m08g4CS3J6lwRQk1pj1EO+KEVWbrbXsmi9Pw0ySmrIbcVxVaedoFgLvFsV8wHLwh01EpROVz3KvVcD1Jmks9FQ==", + "dev": true, + "requires": { + "@sinonjs/commons": "^1.0.2", + "array-from": "^2.1.1", + "lodash.get": "^4.4.2" + } }, "@types/bluebird": { "version": "3.5.24", @@ -100,9 +94,9 @@ "dev": true }, "@types/node": { - "version": "10.12.12", - "resolved": "https://registry.npmjs.org/@types/node/-/node-10.12.12.tgz", - "integrity": "sha512-Pr+6JRiKkfsFvmU/LK68oBRCQeEg36TyAbPhc2xpez24OOZZCuoIhWGTd39VZy6nGafSbxzGouFPTFD/rR1A0A==", + "version": "10.12.15", + "resolved": "https://registry.npmjs.org/@types/node/-/node-10.12.15.tgz", + "integrity": "sha512-9kROxduaN98QghwwHmxXO2Xz3MaWf+I1sLVAA6KJDF5xix+IyXVhds0MAfdNwtcpSrzhaTsNB0/jnL86fgUhqA==", "dev": true }, "@types/request": { @@ -939,7 +933,7 @@ }, "string_decoder": { "version": "0.10.31", - "resolved": "http://registry.npmjs.org/string_decoder/-/string_decoder-0.10.31.tgz", + "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-0.10.31.tgz", "integrity": "sha1-YuIDvEF2bGwoyfyEMB2rHFMQ+pQ=", "dev": true }, @@ -1333,9 +1327,9 @@ } }, "just-extend": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/just-extend/-/just-extend-3.0.0.tgz", - "integrity": "sha512-Fu3T6pKBuxjWT/p4DkqGHFRsysc8OauWr4ZRTY9dIx07Y9O0RkoR5jcv28aeD1vuAwhm3nLkDurwLXoALp4DpQ==", + "version": "4.0.2", + "resolved": "https://registry.npmjs.org/just-extend/-/just-extend-4.0.2.tgz", + "integrity": "sha512-FrLwOgm+iXrPV+5zDU6Jqu4gCRXbWEQg2O3SKONsWE4w7AXFRkryS53bpWdaL9cNol+AmR3AEYz6kn+o0fCPnw==", "dev": true }, "kind-of": { @@ -1541,13 +1535,13 @@ "dev": true }, "nise": { - "version": "1.4.6", - "resolved": "https://registry.npmjs.org/nise/-/nise-1.4.6.tgz", - "integrity": "sha512-1GedetLKzmqmgwabuMSqPsT7oumdR77SBpDfNNJhADRIeA3LN/2RVqR4fFqwvzhAqcTef6PPCzQwITE/YQ8S8A==", + "version": "1.4.8", + "resolved": "https://registry.npmjs.org/nise/-/nise-1.4.8.tgz", + "integrity": "sha512-kGASVhuL4tlAV0tvA34yJYZIVihrUt/5bDwpp4tTluigxUr2bBlJeDXmivb6NuEdFkqvdv/Ybb9dm16PSKUhtw==", "dev": true, "requires": { - "@sinonjs/formatio": "3.0.0", - "just-extend": "^3.0.0", + "@sinonjs/formatio": "^3.1.0", + "just-extend": "^4.0.2", "lolex": "^2.3.2", "path-to-regexp": "^1.7.0", "text-encoding": "^0.6.4" @@ -1976,20 +1970,18 @@ "dev": true }, "sinon": { - "version": "7.1.1", - "resolved": "https://registry.npmjs.org/sinon/-/sinon-7.1.1.tgz", - "integrity": "sha512-iYagtjLVt1vN3zZY7D8oH7dkjNJEjLjyuzy8daX5+3bbQl8gaohrheB9VfH1O3L6LKuue5WTJvFluHiuZ9y3nQ==", + "version": "7.2.2", + "resolved": "https://registry.npmjs.org/sinon/-/sinon-7.2.2.tgz", + "integrity": "sha512-WLagdMHiEsrRmee3jr6IIDntOF4kbI6N2pfbi8wkv50qaUQcBglkzkjtoOEbeJ2vf1EsrHhLI+5Ny8//WHdMoA==", "dev": true, "requires": { "@sinonjs/commons": "^1.2.0", - "@sinonjs/formatio": "^3.0.0", - "@sinonjs/samsam": "^2.1.2", + "@sinonjs/formatio": "^3.1.0", + "@sinonjs/samsam": "^3.0.2", "diff": "^3.5.0", - "lodash.get": "^4.4.2", "lolex": "^3.0.0", - "nise": "^1.4.6", - "supports-color": "^5.5.0", - "type-detect": "^4.0.8" + "nise": "^1.4.7", + "supports-color": "^5.5.0" }, "dependencies": { "diff": { diff --git a/package.json b/package.json index 18d2b19c..109c54ea 100644 --- a/package.json +++ b/package.json @@ -55,14 +55,14 @@ "@types/entities": "^1.1.0", "@types/fs-extra": "^5.0.4", "@types/mocha": "^5.2.5", - "@types/node": "^10.12.12", + "@types/node": "^10.12.15", "@types/request-promise": "4.1.42", "@types/sinon": "^5.0.7", "@types/xml2js": "^0.4.3", "deep-equal": "^1.0.1", "fs-extra": "^7.0.1", "request-promise": "4.2.2", - "sinon": "^7.1.1", + "sinon": "^7.2.2", "typescript": "^3.2.2", "vsce": "^1.53.2", "vscode": "^1.1.26" From 9731dc3c2ff93dfdb6ef446995ba779c9f4f8c52 Mon Sep 17 00:00:00 2001 From: Mate Pek Date: Sat, 15 Dec 2018 13:33:34 +0100 Subject: [PATCH 4/6] better error handling --- CHANGELOG.md | 4 + package.json | 2 +- src/C2TestInfo.ts | 163 ++++++++++++++------------------- src/C2TestSuiteInfo.ts | 2 +- src/test/C2TestAdapter.test.ts | 20 ++-- 5 files changed, 86 insertions(+), 105 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d87c0a6c..c9c1b0ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [2.2.7] + +Better error handling and logging. + ## [2.2.6] - 2018-12-10 Stability improvements. diff --git a/package.json b/package.json index 109c54ea..9bc1192a 100644 --- a/package.json +++ b/package.json @@ -5,7 +5,7 @@ "icon": "resources/icon.png", "author": "Mate Pek", "publisher": "matepek", - "version": "2.2.6", + "version": "2.2.7", "license": "Unlicense", "homepage": "https://github.com/matepek/vscode-catch2-test-adapter", "repository": { diff --git a/src/C2TestInfo.ts b/src/C2TestInfo.ts index 6c81bc8d..21798bae 100644 --- a/src/C2TestInfo.ts +++ b/src/C2TestInfo.ts @@ -2,11 +2,12 @@ // vscode-catch2-test-adapter was written by Mate Pek, and is placed in the // public domain. The author hereby disclaims copyright to this source code. -import { TestDecoration, TestEvent, TestInfo } from 'vscode-test-adapter-api'; +import { TestEvent, TestInfo } from 'vscode-test-adapter-api'; import * as xml2js from 'xml2js'; import { C2TestSuiteInfo } from './C2TestSuiteInfo'; import { generateUniqueId } from './IdGenerator'; +import { inspect } from 'util'; export class C2TestInfo implements TestInfo { readonly type: 'test' = 'test'; @@ -74,134 +75,110 @@ export class C2TestInfo implements TestInfo { private _processXmlTagTestCase(testCase: any, rngSeed: number | undefined): TestEvent { try { - let message = undefined; - let decorations = undefined; - let success = false; - [message, decorations, success] = - this._processXmlTagTestCaseInner(testCase, ''); - - if (rngSeed) { - message = - 'Randomness seeded to: ' + rngSeed.toString() + '\n' + message; - } - const testEvent: TestEvent = { type: 'test', test: this, - state: success ? 'passed' : 'failed', - message: message.length ? message : undefined, - decorations: decorations.length ? decorations : undefined + state: 'failed', + message: '', + decorations: [] }; + if (rngSeed) { + testEvent.message += 'Randomness seeded to: ' + rngSeed.toString() + '\n'; + } + + this._processXmlTagTestCaseInner(testCase, '', testEvent); + + if (testEvent.message === '') testEvent.message = ''; + if (testEvent.decorations.length == 0) testEvent.decorations = undefined; + return testEvent; } catch (e) { throw e; } } - private _processXmlTagTestCaseInner(testCase: any, title: string): - [string, TestDecoration[], boolean] { + private _processXmlTagTestCaseInner(testCase: any, title: string, testEvent: TestEvent): + void { title = testCase.$.name + '(line: ' + testCase.$.line + ')'; - let message = ''; - let decorations: TestDecoration[] = []; - let success = false; if (testCase.OverallResult[0].$.success === 'true') { - success = true; + testEvent.state = 'passed'; } if (testCase.OverallResult[0].$.hasOwnProperty('durationInSeconds')) { - message += 'Duration: ' + testCase.OverallResult[0].$.durationInSeconds + - ' second(s)\n'; + testEvent.message += 'Duration: ' + testCase.OverallResult[0].$.durationInSeconds + ' second(s)\n'; } - if (testCase.hasOwnProperty('Expression')) { - for (let j = 0; j < testCase.Expression.length; ++j) { - try { - let messageL = undefined; - let decorationsL = undefined; - [messageL, decorationsL] = - this._processXmlTagExpressionInner(testCase.Expression[j], title); - message += messageL; - decorations = decorations.concat(decorationsL); - } catch (error) { - } - } - } + this._processXmlTagExpressions(testCase, title, testEvent); + + this._processXmlTagSections(testCase, title, testEvent); + + this._processXmlTagFatalErrorConditions(testCase, title, testEvent); + } - if (testCase.hasOwnProperty('Section')) { - for (let j = 0; j < testCase.Section.length; ++j) { + private _processXmlTagExpressions(xml: any, title: string, testEvent: TestEvent): + void { + if (xml.hasOwnProperty('Expression')) { + for (let j = 0; j < xml.Expression.length; ++j) { + const expr = xml.Expression[j]; try { - let messageL = undefined; - let decorationsL = undefined; - [messageL, decorationsL] = - this._processXmlTagSectionInner(testCase.Section[j], title); - message += messageL; - decorations = decorations.concat(decorationsL); + testEvent.message += '>>> ' + title + ' ' + expr.$.type + ' (line: ' + expr.$.line + ')\n' + + ' Original:\n ' + + expr.Original.map((x: string) => x.trim()).join(' | ') + '\n' + + ' Expanded:\n ' + + expr.Expanded.map((x: string) => x.trim()).join(' | ') + '\n' + + '<<<\n'; + testEvent.decorations!.push({ + line: Number(expr.$.line) - 1 /*It looks vscode works like this.*/, + message: + 'Expanded: ' + expr.Expanded.map((x: string) => x.trim()).join(' | ') + }); } catch (error) { + this.parent.allTests.log.error(inspect(error)); } + this._processXmlTagFatalErrorConditions(expr, title, testEvent); } } - - return [message, decorations, success]; } - private _processXmlTagExpressionInner(expr: any, title: string): - [string, TestDecoration[]] { - let message = ''; - let decorations: TestDecoration[] = []; - - message += '>>> ' + title + ' ' + expr.$.type + ' (line: ' + expr.$.line + - ')' + - ' \n'; - message += ' Original:\n '; - message += expr.Original.map((x: string) => x.trim()).join(' | '); - message += '\n Expanded:\n '; - message += expr.Expanded.map((x: string) => x.trim()).join(' | ') + '\n'; - message += '<<<\n'; - decorations.push({ - line: Number(expr.$.line) - 1 /*It looks vscode works like this.*/, - message: - 'Expanded: ' + expr.Expanded.map((x: string) => x.trim()).join(' | ') - }); - - return [message, decorations]; - } + private _processXmlTagSections(xml: any, title: string, testEvent: TestEvent): + void { + if (xml.hasOwnProperty('Section')) { + for (let j = 0; j < xml.Section.length; ++j) { + const section = xml.Section[j]; + try { + title += ' | ' + section.$.name + '(line: ' + section.$.line + ')'; - private _processXmlTagSectionInner(section: any, title: string): - [string, TestDecoration[]] { - title += ' | ' + section.$.name + '(line: ' + section.$.line + ')'; - let message = ''; - let decorations: TestDecoration[] = []; + this._processXmlTagExpressions(section, title, testEvent); - if (section.hasOwnProperty('Expression')) { - for (let j = 0; j < section.Expression.length; ++j) { - try { - let messageL = undefined; - let decorationsL = undefined; - [messageL, decorationsL] = - this._processXmlTagExpressionInner(section.Expression[j], title); - message += messageL; - decorations = decorations.concat(decorationsL); + this._processXmlTagSections(section, title, testEvent); } catch (error) { + this.parent.allTests.log.error(inspect(error)); } } } + } - if (section.hasOwnProperty('Section')) { - for (let j = 0; j < section.Section.length; ++j) { - try { - let messageL = undefined; - let decorationsL = undefined; - [messageL, decorationsL] = - this._processXmlTagSectionInner(section.Section[j], title); - message += messageL; - decorations = decorations.concat(decorationsL); - } catch (error) { + private _processXmlTagFatalErrorConditions(expr: any, title: string, testEvent: TestEvent): + void { + if (expr.hasOwnProperty('FatalErrorCondition')) { + try { + for (let j = 0; j < expr.FatalErrorCondition.length; ++j) { + const fatal = expr.FatalErrorCondition[j]; + + testEvent.message += '>>> ' + title + ' ' + expr.$.type + ' (line: ' + expr.$.line + ')\n'; + if (fatal.hasOwnProperty('_')) { + testEvent.message += 'Fatal error: ' + fatal._.trim() + '\n'; + } else { + testEvent.message += 'Unknown fatal error: ' + inspect(fatal) + '\n'; + } } } + catch (error) { + testEvent.message += 'Unknown fatal error: ' + inspect(error); + this.parent.allTests.log.error(inspect(error)); + } } - - return [message, decorations]; } } diff --git a/src/C2TestSuiteInfo.ts b/src/C2TestSuiteInfo.ts index cd1d5f84..4d8a1d30 100644 --- a/src/C2TestSuiteInfo.ts +++ b/src/C2TestSuiteInfo.ts @@ -28,7 +28,7 @@ export class C2TestSuiteInfo implements TestSuiteInfo { constructor( public readonly origLabel: string, - private readonly allTests: C2AllTestSuiteInfo, + public readonly allTests: C2AllTestSuiteInfo, public readonly execPath: string, public readonly execOptions: SpawnOptions) { this.label = origLabel; diff --git a/src/test/C2TestAdapter.test.ts b/src/test/C2TestAdapter.test.ts index 5e934272..4486e4d6 100644 --- a/src/test/C2TestAdapter.test.ts +++ b/src/test/C2TestAdapter.test.ts @@ -490,7 +490,7 @@ describe('C2TestAdapter', function () { test: s1t2, decorations: [{ line: 14, message: 'Expanded: false' }], message: - 'Duration: 0.000204 second(s)\n>>> s1t2(line: 13) REQUIRE (line: 15) \n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n' + 'Duration: 0.000204 second(s)\n>>> s1t2(line: 13) REQUIRE (line: 15)\n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n' }, { type: 'suite', state: 'completed', suite: suite1 }, { type: 'finished' }, @@ -522,7 +522,7 @@ describe('C2TestAdapter', function () { test: s1t2, decorations: [{ line: 14, message: 'Expanded: false' }], message: - 'Duration: 0.000204 second(s)\n>>> s1t2(line: 13) REQUIRE (line: 15) \n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n' + 'Duration: 0.000204 second(s)\n>>> s1t2(line: 13) REQUIRE (line: 15)\n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n' }, { type: 'suite', state: 'completed', suite: suite1 }, { type: 'finished' }, @@ -696,7 +696,7 @@ describe('C2TestAdapter', function () { test: s1t2, decorations: [{ line: 14, message: 'Expanded: false' }], message: - 'Duration: 0.000204 second(s)\n>>> s1t2(line: 13) REQUIRE (line: 15) \n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n' + 'Duration: 0.000204 second(s)\n>>> s1t2(line: 13) REQUIRE (line: 15)\n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n' }; assert.ok(testStatesEvI(s1t2running) < testStatesEvI(s1t2finished)); assert.ok(testStatesEvI(s1t2finished) < testStatesEvI(s1finished)); @@ -730,7 +730,7 @@ describe('C2TestAdapter', function () { test: s2t3, decorations: [{ line: 20, message: 'Expanded: false' }], message: - 'Duration: 0.000178 second(s)\n>>> s2t3(line: 19) REQUIRE (line: 21) \n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n' + 'Duration: 0.000178 second(s)\n>>> s2t3(line: 19) REQUIRE (line: 21)\n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n' }; assert.ok(testStatesEvI(s2t3running) < testStatesEvI(s2t3finished)); assert.ok(testStatesEvI(s2t3finished) < testStatesEvI(s2finished)); @@ -807,7 +807,7 @@ describe('C2TestAdapter', function () { test: s2t3, decorations: [{ line: 20, message: 'Expanded: false' }], message: - 'Duration: 0.000596 second(s)\n>>> s2t3(line: 19) REQUIRE (line: 21) \n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n' + 'Duration: 0.000596 second(s)\n>>> s2t3(line: 19) REQUIRE (line: 21)\n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n' }, { type: 'suite', state: 'completed', suite: suite2 }, { type: 'finished' } @@ -835,7 +835,7 @@ describe('C2TestAdapter', function () { test: s2t3, decorations: [{ line: 20, message: 'Expanded: false' }], message: - 'Duration: 0.000596 second(s)\n>>> s2t3(line: 19) REQUIRE (line: 21) \n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n' + 'Duration: 0.000596 second(s)\n>>> s2t3(line: 19) REQUIRE (line: 21)\n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n' }, { type: 'suite', state: 'completed', suite: suite2 }, { type: 'finished' } @@ -865,7 +865,7 @@ describe('C2TestAdapter', function () { test: s1t2, decorations: [{ line: 14, message: 'Expanded: false' }], message: - 'Duration: 0.000204 second(s)\n>>> s1t2(line: 13) REQUIRE (line: 15) \n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n' + 'Duration: 0.000204 second(s)\n>>> s1t2(line: 13) REQUIRE (line: 15)\n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n' }, { type: 'suite', state: 'completed', suite: suite1 }, { type: 'finished' } @@ -1266,7 +1266,7 @@ describe('C2TestAdapter', function () { test: s1t2, decorations: [{ line: 14, message: 'Expanded: false' }], message: - 'Duration: 0.000204 second(s)\n>>> s1t2(line: 13) REQUIRE (line: 15) \n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n' + 'Duration: 0.000204 second(s)\n>>> s1t2(line: 13) REQUIRE (line: 15)\n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n' }, { type: 'suite', state: 'completed', suite: suite1 }, { type: 'finished' }, @@ -1307,7 +1307,7 @@ describe('C2TestAdapter', function () { test: s1t2, decorations: [{ line: 14, message: 'Expanded: false' }], message: - 'Duration: 0.000204 second(s)\n>>> s1t2(line: 13) REQUIRE (line: 15) \n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n' + 'Duration: 0.000204 second(s)\n>>> s1t2(line: 13) REQUIRE (line: 15)\n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n' }, { type: 'suite', state: 'completed', suite: suite1 }, { type: 'finished' }, @@ -1442,7 +1442,7 @@ describe('C2TestAdapter', function () { test: s1t2, decorations: [{ line: 14, message: 'Expanded: false' }], message: - 'Duration: 0.000204 second(s)\n>>> s1t2(line: 13) REQUIRE (line: 15) \n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n' + 'Duration: 0.000204 second(s)\n>>> s1t2(line: 13) REQUIRE (line: 15)\n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n' }, { type: 'suite', state: 'completed', suite: suite1 }, { type: 'finished' }, From 9770e33a0ec0ce9451be741385676f022f7a5d5e Mon Sep 17 00:00:00 2001 From: Mate Pek Date: Sat, 15 Dec 2018 20:49:23 +0100 Subject: [PATCH 5/6] refactor test output --- src/C2TestInfo.ts | 27 +++++++------ src/test/C2TestAdapter.test.ts | 70 +++++++++++++++++----------------- src/test/Helpers.ts | 1 - 3 files changed, 50 insertions(+), 48 deletions(-) diff --git a/src/C2TestInfo.ts b/src/C2TestInfo.ts index 882534b7..336d221c 100644 --- a/src/C2TestInfo.ts +++ b/src/C2TestInfo.ts @@ -84,10 +84,10 @@ export class C2TestInfo implements TestInfo { }; if (rngSeed) { - testEvent.message += 'Randomness seeded to: ' + rngSeed.toString() + '\n'; + testEvent.message += 'Randomness seeded to: ' + rngSeed.toString() + '.\n'; } - this._processXmlTagTestCaseInner(testCase, '', testEvent); + this._processXmlTagTestCaseInner(testCase, testEvent); if (testEvent.message === '') testEvent.message = ''; if (testEvent.decorations!.length == 0) testEvent.decorations = undefined; @@ -98,16 +98,16 @@ export class C2TestInfo implements TestInfo { } } - private _processXmlTagTestCaseInner(testCase: any, title: string, testEvent: TestEvent): + private _processXmlTagTestCaseInner(testCase: any, testEvent: TestEvent): void { - title = testCase.$.name + '(line: ' + testCase.$.line + ')'; + const title = '>>> "' + testCase.$.name + '" at line ' + testCase.$.line; if (testCase.OverallResult[0].$.success === 'true') { testEvent.state = 'passed'; } if (testCase.OverallResult[0].$.hasOwnProperty('durationInSeconds')) { - testEvent.message += 'Duration: ' + testCase.OverallResult[0].$.durationInSeconds + ' second(s)\n'; + testEvent.message += 'Duration: ' + testCase.OverallResult[0].$.durationInSeconds + ' second(s).\n'; } this._processXmlTagExpressions(testCase, title, testEvent); @@ -123,16 +123,18 @@ export class C2TestInfo implements TestInfo { for (let j = 0; j < xml.Expression.length; ++j) { const expr = xml.Expression[j]; try { - testEvent.message += '>>> ' + title + ' ' + expr.$.type + ' (line: ' + expr.$.line + ')\n' + testEvent.message += title + ' -> ' + + (expr.$.type ? expr.$.type : '') + + ' at line ' + expr.$.line + ':\n' + ' Original:\n ' + expr.Original.map((x: string) => x.trim()).join(' | ') + '\n' + ' Expanded:\n ' + expr.Expanded.map((x: string) => x.trim()).join(' | ') + '\n' - + '<<<\n'; + + '<<<\n\n'; testEvent.decorations!.push({ line: Number(expr.$.line) - 1 /*It looks vscode works like this.*/, message: - 'Expanded: ' + expr.Expanded.map((x: string) => x.trim()).join(' | ') + '-> ' + expr.Expanded.map((x: string) => x.trim()).join(' | ') }); } catch (error) { this.parent.allTests.log.error(inspect(error)); @@ -148,7 +150,7 @@ export class C2TestInfo implements TestInfo { for (let j = 0; j < xml.Section.length; ++j) { const section = xml.Section[j]; try { - title += ' | ' + section.$.name + '(line: ' + section.$.line + ')'; + title += ' -> "' + section.$.name + '" at line ' + section.$.line; this._processXmlTagExpressions(section, title, testEvent); @@ -167,12 +169,13 @@ export class C2TestInfo implements TestInfo { for (let j = 0; j < expr.FatalErrorCondition.length; ++j) { const fatal = expr.FatalErrorCondition[j]; - testEvent.message += '>>> ' + title + ' ' + expr.$.type + ' (line: ' + expr.$.line + ')\n'; + testEvent.message += title + ' -> at line ' + expr.$.line + ':\n'; if (fatal.hasOwnProperty('_')) { - testEvent.message += 'Fatal error: ' + fatal._.trim() + '\n'; + testEvent.message += ' Fatal error: ' + fatal._.trim() + '\n'; } else { - testEvent.message += 'Unknown fatal error: ' + inspect(fatal) + '\n'; + testEvent.message += ' Unknown fatal error: ' + inspect(fatal) + '\n'; } + testEvent.message += '<<<\n\n'; } } catch (error) { diff --git a/src/test/C2TestAdapter.test.ts b/src/test/C2TestAdapter.test.ts index 6d97ddd9..a8f581ee 100644 --- a/src/test/C2TestAdapter.test.ts +++ b/src/test/C2TestAdapter.test.ts @@ -464,7 +464,7 @@ describe('C2TestAdapter', function () { state: 'passed', test: s1t1, decorations: undefined, - message: 'Duration: 0.000112 second(s)\n' + message: 'Duration: 0.000112 second(s).\n' }, { type: 'suite', state: 'completed', suite: suite1 }, { type: 'finished' }, @@ -487,16 +487,16 @@ describe('C2TestAdapter', function () { state: 'passed', test: s1t1, decorations: undefined, - message: 'Duration: 0.000132 second(s)\n' + message: 'Duration: 0.000132 second(s).\n' }, { type: 'test', state: 'running', test: s1t2 }, { type: 'test', state: 'failed', test: s1t2, - decorations: [{ line: 14, message: 'Expanded: false' }], + decorations: [{ line: 14, message: '-> false' }], message: - 'Duration: 0.000204 second(s)\n>>> s1t2(line: 13) REQUIRE (line: 15)\n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n' + 'Duration: 0.000204 second(s).\n>>> "s1t2" at line 13 -> REQUIRE at line 15:\n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n\n' }, { type: 'suite', state: 'completed', suite: suite1 }, { type: 'finished' }, @@ -519,16 +519,16 @@ describe('C2TestAdapter', function () { state: 'passed', test: s1t1, decorations: undefined, - message: 'Duration: 0.000132 second(s)\n' + message: 'Duration: 0.000132 second(s).\n' }, { type: 'test', state: 'running', test: s1t2 }, { type: 'test', state: 'failed', test: s1t2, - decorations: [{ line: 14, message: 'Expanded: false' }], + decorations: [{ line: 14, message: '-> false' }], message: - 'Duration: 0.000204 second(s)\n>>> s1t2(line: 13) REQUIRE (line: 15)\n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n' + 'Duration: 0.000204 second(s).\n>>> "s1t2" at line 13 -> REQUIRE at line 15:\n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n\n' }, { type: 'suite', state: 'completed', suite: suite1 }, { type: 'finished' }, @@ -558,7 +558,7 @@ describe('C2TestAdapter', function () { state: 'passed', test: s1t1, decorations: undefined, - message: 'Duration: 0.000112 second(s)\n' + message: 'Duration: 0.000112 second(s).\n' }, { type: 'suite', state: 'completed', suite: suite1 }, { type: 'finished' } @@ -588,7 +588,7 @@ describe('C2TestAdapter', function () { test: s1t1, decorations: undefined, message: - 'Randomness seeded to: 2\nDuration: 0.000327 second(s)\n' + 'Randomness seeded to: 2.\nDuration: 0.000327 second(s).\n' }, { type: 'suite', state: 'completed', suite: suite1 }, { type: 'finished' } @@ -688,7 +688,7 @@ describe('C2TestAdapter', function () { state: 'passed', test: s1t1, decorations: undefined, - message: 'Duration: 0.000132 second(s)\n' + message: 'Duration: 0.000132 second(s).\n' }; assert.ok(testStatesEvI(s1t1running) < testStatesEvI(s1t1finished)); assert.ok(testStatesEvI(s1t1finished) < testStatesEvI(s1finished)); @@ -700,9 +700,9 @@ describe('C2TestAdapter', function () { type: 'test', state: 'failed', test: s1t2, - decorations: [{ line: 14, message: 'Expanded: false' }], + decorations: [{ line: 14, message: '-> false' }], message: - 'Duration: 0.000204 second(s)\n>>> s1t2(line: 13) REQUIRE (line: 15)\n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n' + 'Duration: 0.000204 second(s).\n>>> "s1t2" at line 13 -> REQUIRE at line 15:\n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n\n' }; assert.ok(testStatesEvI(s1t2running) < testStatesEvI(s1t2finished)); assert.ok(testStatesEvI(s1t2finished) < testStatesEvI(s1finished)); @@ -715,7 +715,7 @@ describe('C2TestAdapter', function () { state: 'passed', test: s2t1, decorations: undefined, - message: 'Duration: 0.00037 second(s)\n' + message: 'Duration: 0.00037 second(s).\n' }; assert.ok(testStatesEvI(s2t1running) < testStatesEvI(s2t1finished)); assert.ok(testStatesEvI(s2t1finished) < testStatesEvI(s2finished)); @@ -734,9 +734,9 @@ describe('C2TestAdapter', function () { type: 'test', state: 'failed', test: s2t3, - decorations: [{ line: 20, message: 'Expanded: false' }], + decorations: [{ line: 20, message: '-> false' }], message: - 'Duration: 0.000178 second(s)\n>>> s2t3(line: 19) REQUIRE (line: 21)\n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n' + 'Duration: 0.000178 second(s).\n>>> "s2t3" at line 19 -> REQUIRE at line 21:\n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n\n' }; assert.ok(testStatesEvI(s2t3running) < testStatesEvI(s2t3finished)); assert.ok(testStatesEvI(s2t3finished) < testStatesEvI(s2finished)); @@ -768,7 +768,7 @@ describe('C2TestAdapter', function () { state: 'passed', test: s1t1, decorations: undefined, - message: 'Duration: 0.000112 second(s)\n' + message: 'Duration: 0.000112 second(s).\n' }, { type: 'suite', state: 'completed', suite: suite1 }, { type: 'finished' } @@ -790,7 +790,7 @@ describe('C2TestAdapter', function () { state: 'passed', test: s2t2, decorations: undefined, - message: 'Duration: 0.001294 second(s)\n' + message: 'Duration: 0.001294 second(s).\n' }, { type: 'suite', state: 'completed', suite: suite2 }, { type: 'finished' } @@ -811,9 +811,9 @@ describe('C2TestAdapter', function () { type: 'test', state: 'failed', test: s2t3, - decorations: [{ line: 20, message: 'Expanded: false' }], + decorations: [{ line: 20, message: '-> false' }], message: - 'Duration: 0.000596 second(s)\n>>> s2t3(line: 19) REQUIRE (line: 21)\n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n' + 'Duration: 0.000596 second(s).\n>>> "s2t3" at line 19 -> REQUIRE at line 21:\n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n\n' }, { type: 'suite', state: 'completed', suite: suite2 }, { type: 'finished' } @@ -839,9 +839,9 @@ describe('C2TestAdapter', function () { type: 'test', state: 'failed', test: s2t3, - decorations: [{ line: 20, message: 'Expanded: false' }], + decorations: [{ line: 20, message: '-> false' }], message: - 'Duration: 0.000596 second(s)\n>>> s2t3(line: 19) REQUIRE (line: 21)\n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n' + 'Duration: 0.000596 second(s).\n>>> "s2t3" at line 19 -> REQUIRE at line 21:\n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n\n' }, { type: 'suite', state: 'completed', suite: suite2 }, { type: 'finished' } @@ -863,15 +863,15 @@ describe('C2TestAdapter', function () { state: 'passed', test: s1t1, decorations: undefined, - message: 'Duration: 0.000132 second(s)\n' + message: 'Duration: 0.000132 second(s).\n' }, { type: 'test', state: 'running', test: s1t2 }, { type: 'test', state: 'failed', test: s1t2, - decorations: [{ line: 14, message: 'Expanded: false' }], + decorations: [{ line: 14, message: '-> false' }], message: - 'Duration: 0.000204 second(s)\n>>> s1t2(line: 13) REQUIRE (line: 15)\n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n' + 'Duration: 0.000204 second(s).\n>>> "s1t2" at line 13 -> REQUIRE at line 15:\n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n\n' }, { type: 'suite', state: 'completed', suite: suite1 }, { type: 'finished' } @@ -920,7 +920,7 @@ describe('C2TestAdapter', function () { state: 'passed', test: s1t1, decorations: undefined, - message: 'Duration: 0.000112 second(s)\n' + message: 'Duration: 0.000112 second(s).\n' }, { type: 'suite', state: 'completed', suite: suite1 }, { type: 'finished' } @@ -966,7 +966,7 @@ describe('C2TestAdapter', function () { state: 'passed', test: s1t1, decorations: undefined, - message: 'Duration: 0.000112 second(s)\n' + message: 'Duration: 0.000112 second(s).\n' }, { type: 'suite', state: 'completed', suite: suite1 }, { type: 'finished' } @@ -1007,7 +1007,7 @@ describe('C2TestAdapter', function () { state: 'passed', test: s1t1, decorations: undefined, - message: 'Duration: 0.000112 second(s)\n' + message: 'Duration: 0.000112 second(s).\n' }, { type: 'suite', state: 'completed', suite: suite1 }, { type: 'finished' } @@ -1305,16 +1305,16 @@ describe('C2TestAdapter', function () { state: 'passed', test: s1t1, decorations: undefined, - message: 'Duration: 0.000132 second(s)\n' + message: 'Duration: 0.000132 second(s).\n' }, { type: 'test', state: 'running', test: s1t2 }, { type: 'test', state: 'failed', test: s1t2, - decorations: [{ line: 14, message: 'Expanded: false' }], + decorations: [{ line: 14, message: '-> false' }], message: - 'Duration: 0.000204 second(s)\n>>> s1t2(line: 13) REQUIRE (line: 15)\n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n' + 'Duration: 0.000204 second(s).\n>>> "s1t2" at line 13 -> REQUIRE at line 15:\n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n\n' }, { type: 'suite', state: 'completed', suite: suite1 }, { type: 'finished' }, @@ -1353,9 +1353,9 @@ describe('C2TestAdapter', function () { type: 'test', state: 'failed', test: s1t2, - decorations: [{ line: 14, message: 'Expanded: false' }], + decorations: [{ line: 14, message: '-> false' }], message: - 'Duration: 0.000204 second(s)\n>>> s1t2(line: 13) REQUIRE (line: 15)\n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n' + 'Duration: 0.000204 second(s).\n>>> "s1t2" at line 13 -> REQUIRE at line 15:\n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n\n' }, { type: 'suite', state: 'completed', suite: suite1 }, { type: 'finished' }, @@ -1488,9 +1488,9 @@ describe('C2TestAdapter', function () { type: 'test', state: 'failed', test: s1t2, - decorations: [{ line: 14, message: 'Expanded: false' }], + decorations: [{ line: 14, message: '-> false' }], message: - 'Duration: 0.000204 second(s)\n>>> s1t2(line: 13) REQUIRE (line: 15)\n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n' + 'Duration: 0.000204 second(s).\n>>> "s1t2" at line 13 -> REQUIRE at line 15:\n Original:\n std::false_type::value\n Expanded:\n false\n<<<\n\n' }, { type: 'suite', state: 'completed', suite: suite1 }, { type: 'finished' }, @@ -1502,7 +1502,7 @@ describe('C2TestAdapter', function () { state: 'passed', test: s1t1, decorations: undefined, - message: 'Duration: 0.000132 second(s)\n' + message: 'Duration: 0.000132 second(s).\n' }, { type: 'suite', state: 'completed', suite: suite1 }, { type: 'finished' }, diff --git a/src/test/Helpers.ts b/src/test/Helpers.ts index 08e41eae..4303b380 100644 --- a/src/test/Helpers.ts +++ b/src/test/Helpers.ts @@ -7,7 +7,6 @@ import { Readable } from 'stream'; import * as vscode from 'vscode'; export class ChildProcessStub extends EventEmitter { - private _buffer: string = ''; readonly stdout: Readable; public closed: boolean = false; From 8ded3da3b8be8dd648eb2f7b025b18237b1f6563 Mon Sep 17 00:00:00 2001 From: Mate Pek Date: Mon, 17 Dec 2018 10:13:06 +0100 Subject: [PATCH 6/6] better error handling --- src/C2AllTestSuiteInfo.ts | 4 +-- src/C2TestAdapter.ts | 5 ++-- src/C2TestSuiteInfo.ts | 23 +++++++++-------- src/test/C2TestAdapter.test.ts | 47 +++++++++++++--------------------- src/test/Helpers.ts | 2 +- 5 files changed, 37 insertions(+), 44 deletions(-) diff --git a/src/C2AllTestSuiteInfo.ts b/src/C2AllTestSuiteInfo.ts index 00a64f83..cee996ae 100644 --- a/src/C2AllTestSuiteInfo.ts +++ b/src/C2AllTestSuiteInfo.ts @@ -31,13 +31,13 @@ export class C2AllTestSuiteInfo implements TestSuiteInfo, vscode.Disposable { vscode.EventEmitter, public readonly testStatesEmitter: vscode.EventEmitter, + TestSuiteEvent | TestEvent>, public readonly autorunEmitter: vscode.EventEmitter, 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 index f9502a3d..e6f49bfa 100644 --- 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('defaultRunningTimeoutSec', 0) * 1000; + null | number { + const r = config.get('defaultRunningTimeoutSec', null); + return r !== null ? r * 1000 : r; } private _getGlobalAndDefaultEnvironmentVariables( diff --git a/src/C2TestSuiteInfo.ts b/src/C2TestSuiteInfo.ts index 70f34682..5d523547 100644 --- a/src/C2TestSuiteInfo.ts +++ b/src/C2TestSuiteInfo.ts @@ -267,10 +267,11 @@ export class C2TestSuiteInfo implements TestSuiteInfo { const startTime = Date.now(); const killIfTimout = (): Promise => { 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,8 +281,12 @@ 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)); @@ -289,20 +294,19 @@ export class C2TestSuiteInfo implements TestSuiteInfo { 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 index a8f581ee..26fe29f7 100644 --- 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.spy(cp, 'kill'); cp.write(''); // 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 index 4303b380..52cb8988 100644 --- 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, close?: number | string) {