diff --git a/src/reporter/index.js b/src/reporter/index.js index ceb2f498ad..ee43c10684 100644 --- a/src/reporter/index.js +++ b/src/reporter/index.js @@ -2,7 +2,6 @@ import { find, sortBy, union } from 'lodash'; import { writable as isWritableStream } from 'is-stream'; import ReporterPluginHost from './plugin-host'; import formatCommand from './command/format-command'; -import TestCafeErrorList from '../errors/error-list'; export default class Reporter { constructor (plugin, task, outStream, name) { @@ -135,14 +134,11 @@ export default class Reporter { reportItem.pendingTestRunDonePromise.resolve(); } - _prepareReportTestActionEventArgs ({ command, result, testRun, errors }) { + _prepareReportTestActionEventArgs ({ command, result, testRun, err }) { const args = {}; - if (errors) { - errors = errors instanceof TestCafeErrorList ? errors.items : [errors]; - - args.errors = errors; - } + if (err) + args.err = err; return Object.assign(args, { testRunId: testRun.id, @@ -218,9 +214,9 @@ export default class Reporter { } }); - task.on('test-action-done', async ({ apiActionName, command, result, testRun, errors }) => { + task.on('test-action-done', async ({ apiActionName, command, result, testRun, err }) => { if (this.plugin.reportTestActionDone) { - const args = this._prepareReportTestActionEventArgs({ command, result, testRun, errors }); + const args = this._prepareReportTestActionEventArgs({ command, result, testRun, err }); await this.plugin.reportTestActionDone(apiActionName, args); } diff --git a/src/test-run/index.js b/src/test-run/index.js index 1f63169bcf..b544c810e3 100644 --- a/src/test-run/index.js +++ b/src/test-run/index.js @@ -42,6 +42,7 @@ import { } from './commands/utils'; import { TEST_RUN_ERRORS } from '../errors/types'; +import processTestFnError from '../errors/process-test-fn-error'; const lazyRequire = require('import-lazy')(require); const SessionController = lazyRequire('./session-controller'); @@ -294,16 +295,15 @@ export default class TestRun extends AsyncEventEmitter { await fn(this); } catch (err) { - let screenshotPath = null; + await this._makeScreenshotOnFail(); - const { screenshots } = this.opts; + this.addError(err); - if (screenshots && screenshots.takeOnFails) - screenshotPath = await this.executeCommand(new browserManipulationCommands.TakeScreenshotOnFailCommand()); - - this.addError(err, screenshotPath); return false; } + finally { + this.errScreenshotPath = null; + } return !this._addPendingPageErrorIfAny(); } @@ -378,19 +378,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); }); @@ -617,8 +617,9 @@ export default class TestRun extends AsyncEventEmitter { } async executeAction (actionName, command, callsite) { - let error = null; - let result = null; + let errorAdapter = null; + let error = null; + let result = null; await this.emitActionStart(actionName, command); @@ -627,9 +628,18 @@ export default class TestRun extends AsyncEventEmitter { } catch (err) { error = err; + + // 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(); + + errorAdapter = this._createErrorAdapter(processTestFnError(err)); + } } - await this.emitActionDone(actionName, command, result, error); + await this.emitActionDone(actionName, command, result, errorAdapter); if (error) throw error; @@ -709,6 +719,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; @@ -844,9 +861,9 @@ export default class TestRun extends AsyncEventEmitter { await this.emit('action-start', { command, apiActionName }); } - async emitActionDone (apiActionName, command, result, errors) { + async emitActionDone (apiActionName, command, result, err) { if (!this.preventEmitActionEvents) - await this.emit('action-done', { command, apiActionName, result, errors }); + await this.emit('action-done', { command, apiActionName, result, err }); } } diff --git a/test/functional/fixtures/reporter/reporter.js b/test/functional/fixtures/reporter/reporter.js index 4bc0b50824..58e20f3461 100644 --- a/test/functional/fixtures/reporter/reporter.js +++ b/test/functional/fixtures/reporter/reporter.js @@ -48,7 +48,7 @@ function generateReporter (log, options = {}) { log.push(item); }, - async reportTestActionDone (name, { command, test, fixture, errors }) { + async reportTestActionDone (name, { command, test, fixture, err }) { if (!emitOnDone) return; @@ -57,8 +57,8 @@ function generateReporter (log, options = {}) { const item = { name, action: 'done', command }; - if (errors && errors.length) - item.errors = errors.map(err => err.code); + if (err) + item.err = err.code; if (includeTestInfo) { if (test.id) { diff --git a/test/functional/fixtures/reporter/test.js b/test/functional/fixtures/reporter/test.js index c62f677bee..0a967e1a15 100644 --- a/test/functional/fixtures/reporter/test.js +++ b/test/functional/fixtures/reporter/test.js @@ -308,7 +308,7 @@ describe('Reporter', () => { options: new ClickOptions(), selector: 'Selector(\'#non-existing-target\')' }, - errors: ['E24'] + err: 'E24' } ]); }); @@ -541,7 +541,7 @@ describe('Reporter', () => { selector: 'Selector(\'#non-existing-element\')', type: 'click' }, - errors: ['E24'] + err: 'E24' }, { name: 'useRole', @@ -553,8 +553,7 @@ describe('Reporter', () => { phase: 'initialized' }, type: 'useRole' - }, - errors: ['E24'] + } } ]); }); @@ -675,5 +674,40 @@ describe('Reporter', () => { }); }); + describe('Screenshot errors', () => { + it('Screenshot on action error', () => { + let testDoneErrors = null; + const actionDoneErrors = []; + + function screenshotReporter () { + return { + async reportTestActionDone (name, { err }) { + actionDoneErrors.push(err); + }, + async reportTaskStart () { + }, + async reportFixtureStart () { + }, + async reportTestDone (name, testRunInfo) { + testDoneErrors = testRunInfo.errs; + }, + async reportTaskDone () { + } + }; + } + return runTests('testcafe-fixtures/index-test.js', 'Screenshot on action error', { + only: 'chrome', + reporter: screenshotReporter, + screenshotsOnFails: true + }) + .then(() => { + expect(actionDoneErrors[0]).is.undefined; + expect(actionDoneErrors[1].code).eql('E24'); + expect(testDoneErrors.map(err => err.code)).eql(['E24', 'E8']); + expect(testDoneErrors[0].screenshotPath).is.not.empty; + expect(testDoneErrors[0].screenshotPath).eql(testDoneErrors[1].screenshotPath); + }); + }); + }); }); diff --git a/test/functional/fixtures/reporter/testcafe-fixtures/index-test.js b/test/functional/fixtures/reporter/testcafe-fixtures/index-test.js index 9d85f46cd6..e63ab00018 100644 --- a/test/functional/fixtures/reporter/testcafe-fixtures/index-test.js +++ b/test/functional/fixtures/reporter/testcafe-fixtures/index-test.js @@ -63,3 +63,9 @@ test('Client Function', async () => { test('Eval', async t => { await t.eval(() => document.getElementById('#target')); }); + +test('Screenshot on action error', async t => { + t.hover('body'); + + await t.click('#unexisting-element'); +}); diff --git a/test/server/test-controller-events-test.js b/test/server/test-controller-events-test.js index 34babb3525..fed613cb86 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 }; + async reportTestActionDone (name, { command, err }) { + errorAdapter = err; + actionResult = { name, command: command.type, err: err.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' }); }); }); });