Skip to content

Commit

Permalink
The onActionDone method should take TestRunErrorFormattableAdapter …
Browse files Browse the repository at this point in the history 
…instance instead of Error (closes #4867) (#4895)

* one more case of adapter on action done

* refactoring

* reset cached screenshotPath

* action arguments: errors[] => err
  • Loading branch information
@AlexKamaev
AlexKamaev committed Mar 24, 2020
1 parent e567911 commit fa84ced
Show file tree
Hide file tree
Showing 6 changed files with 99 additions and 42 deletions.
14 changes: 5 additions & 9 deletions src/reporter/index.js
View file
Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}
Expand Down
47 changes: 32 additions & 15 deletions src/test-run/index.js
View file
Expand Up @@ -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');
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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);
});
Expand Down Expand Up @@ -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);

Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 });
}
}

Expand Down
6 changes: 3 additions & 3 deletions test/functional/fixtures/reporter/reporter.js
View file
Expand Up @@ -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;

Expand All @@ -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) {
Expand Down
42 changes: 38 additions & 4 deletions test/functional/fixtures/reporter/test.js
View file
Expand Up @@ -308,7 +308,7 @@ describe('Reporter', () => {
options: new ClickOptions(),
selector: 'Selector(\'#non-existing-target\')'
},
errors: ['E24']
err: 'E24'
}
]);
});
Expand Down Expand Up @@ -541,7 +541,7 @@ describe('Reporter', () => {
selector: 'Selector(\'#non-existing-element\')',
type: 'click'
},
errors: ['E24']
err: 'E24'
},
{
name: 'useRole',
Expand All @@ -553,8 +553,7 @@ describe('Reporter', () => {
phase: 'initialized'
},
type: 'useRole'
},
errors: ['E24']
}
}
]);
});
Expand Down Expand Up @@ -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);
});
});
});
});
6 changes: 6 additions & 0 deletions test/functional/fixtures/reporter/testcafe-fixtures/index-test.js
View file
Expand Up @@ -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');
});
26 changes: 15 additions & 11 deletions test/server/test-controller-events-test.js
View file
@@ -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 () {
Expand Down Expand Up @@ -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 };
}
});

Expand All @@ -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' });
});
});
});

0 comments on commit fa84ced

Please sign in to comment.