From 3e8fd62306edf3017680388c2142ef92e796f0f5 Mon Sep 17 00:00:00 2001 From: AlexKamaev Date: Sun, 22 Mar 2020 12:13:46 +0300 Subject: [PATCH] refactoring --- src/test-run/index.js | 43 ++++++++++++---------- test/functional/fixtures/reporter/test.js | 3 +- test/server/test-controller-events-test.js | 24 +++++++----- 3 files changed, 39 insertions(+), 31 deletions(-) diff --git a/src/test-run/index.js b/src/test-run/index.js index 48c64d10cd..e87fbc6976 100644 --- a/src/test-run/index.js +++ b/src/test-run/index.js @@ -295,14 +295,10 @@ export default class TestRun extends AsyncEventEmitter { await fn(this); } catch (err) { - let screenshotPath = this.errScreenshotPath; + await this._makeScreenshotOnFail(); - const { screenshots } = this.opts; + this.addError(err); - if (!screenshotPath && screenshots && screenshots.takeOnFails) - screenshotPath = await this.executeCommand(new browserManipulationCommands.TakeScreenshotOnFailCommand()); - - this.addError(err, screenshotPath); return false; } @@ -379,19 +375,19 @@ export default class TestRun extends AsyncEventEmitter { return false; } - _createErrorAdapter (err, screenshotPath) { + _createErrorAdapter (err) { return new TestRunErrorFormattableAdapter(err, { userAgent: this.browserConnection.userAgent, - screenshotPath: screenshotPath || '', + screenshotPath: this.errScreenshotPath || '', testRunPhase: this.phase }); } - addError (err, screenshotPath) { + addError (err) { const errList = err instanceof TestCafeErrorList ? err.items : [err]; errList.forEach(item => { - const adapter = this._createErrorAdapter(item, screenshotPath); + const adapter = this._createErrorAdapter(item); this.errs.push(adapter); }); @@ -618,9 +614,9 @@ export default class TestRun extends AsyncEventEmitter { } async executeAction (actionName, command, callsite) { - let adapter = null; - let error = null; - let result = null; + let errorAdapter = null; + let error = null; + let result = null; await this.emitActionStart(actionName, command); @@ -630,15 +626,17 @@ export default class TestRun extends AsyncEventEmitter { catch (err) { error = err; - const { screenshots } = this.opts; - - if (!this.errScreenshotPath && screenshots && screenshots.takeOnFails) - this.errScreenshotPath = await this.executeCommand(new browserManipulationCommands.TakeScreenshotOnFailCommand()); + // NOTE: check if error is TestCafeErrorList is specific for the `useRole` action + // if error is TestCafeErrorList we do not need to create an adapter, + // since error is already was processed in role initializer + if (!(err instanceof TestCafeErrorList)) { + await this._makeScreenshotOnFail(); - adapter = this._createErrorAdapter(processTestFnError(error), this.errScreenshotPath); + errorAdapter = this._createErrorAdapter(processTestFnError(err)); + } } - await this.emitActionDone(actionName, command, result, adapter); + await this.emitActionDone(actionName, command, result, errorAdapter); if (error) throw error; @@ -718,6 +716,13 @@ export default class TestRun extends AsyncEventEmitter { return Promise.reject(err); } + async _makeScreenshotOnFail () { + const { screenshots } = this.opts; + + if (!this.errScreenshotPath && screenshots && screenshots.takeOnFails) + this.errScreenshotPath = await this.executeCommand(new browserManipulationCommands.TakeScreenshotOnFailCommand()); + } + _decorateWithFlag (fn, flagName, value) { return async () => { this[flagName] = value; diff --git a/test/functional/fixtures/reporter/test.js b/test/functional/fixtures/reporter/test.js index e9b1dd83fb..1f15482dd4 100644 --- a/test/functional/fixtures/reporter/test.js +++ b/test/functional/fixtures/reporter/test.js @@ -553,8 +553,7 @@ describe('Reporter', () => { phase: 'initialized' }, type: 'useRole' - }, - errors: [ void 0 ] // TODO: investigate + } } ]); }); diff --git a/test/server/test-controller-events-test.js b/test/server/test-controller-events-test.js index 34babb3525..5dcf8dd495 100644 --- a/test/server/test-controller-events-test.js +++ b/test/server/test-controller-events-test.js @@ -1,11 +1,12 @@ -const expect = require('chai').expect; -const AsyncEventEmitter = require('../../lib/utils/async-event-emitter'); -const TestRun = require('../../lib/test-run'); -const TestController = require('../../lib/api/test-controller'); -const Task = require('../../lib/runner/task'); -const BrowserJob = require('../../lib/runner/browser-job'); -const Reporter = require('../../lib/reporter'); -const { Role } = require('../../lib/api/exportable-lib'); +const expect = require('chai').expect; +const AsyncEventEmitter = require('../../lib/utils/async-event-emitter'); +const TestRun = require('../../lib/test-run'); +const TestController = require('../../lib/api/test-controller'); +const Task = require('../../lib/runner/task'); +const BrowserJob = require('../../lib/runner/browser-job'); +const Reporter = require('../../lib/reporter'); +const { Role } = require('../../lib/api/exportable-lib'); +const TestRunErrorFormattableAdapter = require('../../lib/errors/test-run/formattable-adapter'); class TestRunMock extends TestRun { constructor () { @@ -175,10 +176,12 @@ describe('TestController action events', () => { it('Error action', () => { let actionResult = null; + let errorAdapter = null; initializeReporter({ async reportTestActionDone (name, { command, errors }) { - actionResult = { name, command: command.type, err: errors[0].message }; + errorAdapter = errors[0]; + actionResult = { name, command: command.type, err: errors[0].errMsg }; } }); @@ -191,8 +194,9 @@ describe('TestController action events', () => { throw new Error(); }) .catch(err => { + expect(errorAdapter).instanceOf(TestRunErrorFormattableAdapter); expect(err.message).eql('test error'); - expect(actionResult).eql({ name: 'click', command: 'click', err: 'test error' }); + expect(actionResult).eql({ name: 'click', command: 'click', err: 'Error: test error' }); }); }); });