Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

The onActionDone method should take TestRunErrorFormattableAdapter instance instead of Error (closes #4867) #4847

Closed
wants to merge 2 commits into from
Closed

The onActionDone method should take TestRunErrorFormattableAdapter instance instead of Error (closes #4867) #4847

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3a23d22
onActionDone event should take errorsAdapters as parameter
AlexKamaev Mar 16, 2020
f6e49d1
refactoring
AlexKamaev Mar 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 19 additions & 6 deletions src/api/wrap-test-function.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,16 @@ import TestController from './test-controller';
import testRunTracker from './test-run-tracker';
import TestCafeErrorList from '../errors/error-list';
import { MissingAwaitError } from '../errors/test-run';
import TestRunErrorFormattableAdapter from '../errors/test-run/formattable-adapter';


export default function wrapTestFunction (fn) {
return async testRun => {
let result = null;
const errList = new TestCafeErrorList();
const markeredfn = testRunTracker.addTrackingMarkerToFunction(testRun.id, fn);
let result = null;
let screenshotPath = null;
let executionError = null;
const errList = new TestCafeErrorList();
const markeredfn = testRunTracker.addTrackingMarkerToFunction(testRun.id, fn);

testRun.controller = new TestController(testRun);

Expand All @@ -17,7 +21,10 @@ export default function wrapTestFunction (fn) {
result = await markeredfn(testRun.controller);
}
catch (err) {
errList.addError(err);
if (err instanceof TestRunErrorFormattableAdapter)
executionError = err;
else
errList.addError(err);
}

if (!errList.hasUncaughtErrorsInTestCode) {
Expand All @@ -26,8 +33,14 @@ export default function wrapTestFunction (fn) {
});
}

if (errList.hasErrors)
throw errList;
if (errList.hasErrors) {
screenshotPath = executionError && executionError.screenshotPath;

await testRun.addErrorWithScreenshot(errList, screenshotPath);
}

if (testRun.errs.length)
throw testRun.errs;

return result;
};
Expand Down
14 changes: 5 additions & 9 deletions src/reporter/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
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
44 changes: 26 additions & 18 deletions src/test-run/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import * as INJECTABLES from '../assets/injectables';
import { findProblematicScripts } from '../custom-client-scripts/utils';
import getCustomClientScriptUrl from '../custom-client-scripts/get-url';
import { getPluralSuffix, getConcatenatedValuesString } from '../utils/string';
import processTestFnError from '../errors/process-test-fn-error';

import {
isCommandRejectableByPageError,
Expand Down Expand Up @@ -294,18 +295,21 @@ export default class TestRun extends AsyncEventEmitter {
await fn(this);
}
catch (err) {
let screenshotPath = null;
return false;
}

return !this._addPendingPageErrorIfAny();
}

async addErrorWithScreenshot (error, screenshotPath) {
if (!screenshotPath) {
const { screenshots } = this.opts;

if (screenshots && screenshots.takeOnFails)
screenshotPath = await this.executeCommand(new browserManipulationCommands.TakeScreenshotOnFailCommand());

this.addError(err, screenshotPath);
return false;
}

return !this._addPendingPageErrorIfAny();
this.addError(error, screenshotPath);
}

async _runBeforeHook () {
Expand Down Expand Up @@ -386,14 +390,16 @@ export default class TestRun extends AsyncEventEmitter {
});
}

addError (err, screenshotPath) {
_createErrorAdapters (err, screenshotPath) {
const errList = err instanceof TestCafeErrorList ? err.items : [err];

errList.forEach(item => {
const adapter = this._createErrorAdapter(item, screenshotPath);
return errList.map(item => this._createErrorAdapter(item, screenshotPath));
}

this.errs.push(adapter);
});
addError (err, screenshotPath) {
const errList = err instanceof TestCafeErrorList ? err.items : [err];

errList.map(item => this.errs.push(this._createErrorAdapter(item, screenshotPath)));
}

normalizeRequestHookErrors () {
Expand Down Expand Up @@ -617,7 +623,6 @@ export default class TestRun extends AsyncEventEmitter {
}

async executeAction (actionName, command, callsite) {
let error = null;
let result = null;

await this.emitActionStart(actionName, command);
Expand All @@ -626,13 +631,16 @@ export default class TestRun extends AsyncEventEmitter {
result = await this.executeCommand(command, callsite);
}
catch (err) {
error = err;
}
const testFnError = processTestFnError(err);

await this.addErrorWithScreenshot(testFnError);

await this.emitActionDone(actionName, command, result, error);
await this.emitActionDone(actionName, command, void 0, this.errs[0]);

throw this.errs[0];
}

if (error)
throw error;
await this.emitActionDone(actionName, command, result);

return result;
}
Expand Down Expand Up @@ -844,9 +852,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
82 changes: 67 additions & 15 deletions test/functional/fixtures/reporter/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,11 +528,35 @@ describe('Reporter', () => {
it('Complex nested command error', function () {
const log = [];

return runTests('testcafe-fixtures/index-test.js', 'Complex nested command error', generateRunOptions(log))
return runTests('testcafe-fixtures/index-test.js', 'Complex nested command error', generateRunOptions(log, { includeTestInfo: true }))
.then(() => {
expect(log).eql([
{ name: 'useRole', action: 'start' },
{ name: 'click', action: 'start' },
{
name: 'useRole',
action: 'start',
fixture: {
id: 'fixture-id',
name: 'Reporter'
},
test: {
id: 'test-id',
name: 'Complex nested command error',
phase: 'inTest'
}
},
{
name: 'click',
action: 'start',
fixture: {
id: 'fixture-id',
name: 'Reporter'
},
test: {
id: 'test-id',
name: 'Complex nested command error',
phase: 'inRoleInitializer'
}
},
{
name: 'click',
action: 'done',
Expand All @@ -541,18 +565,14 @@ describe('Reporter', () => {
selector: 'Selector(\'#non-existing-element\')',
type: 'click'
},
errors: ['E24']
},
{
name: 'useRole',
action: 'done',
command: {
role: {
loginPage: 'http://localhost:3000/fixtures/reporter/pages/index.html',
options: {},
phase: 'initialized'
},
type: 'useRole'
fixture: {
id: 'fixture-id',
name: 'Reporter'
},
test: {
id: 'test-id',
name: 'Complex nested command error',
phase: 'inRoleInitializer'
},
errors: ['E24']
}
Expand Down Expand Up @@ -675,5 +695,37 @@ describe('Reporter', () => {
});
});

it('Screenshot on action error', () => {
let testDoneErrors = null;
let actionDoneError = null;

function screenshotReporter () {
return {
async reportTestActionDone (name, { err }) {
actionDoneError = 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(actionDoneError.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
Open in desktop
Original file line number Diff line number Diff line change
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.click('body');

await t.click('#unexisting-element');
});
26 changes: 16 additions & 10 deletions test/server/compiler-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const exportableLib = require('../../lib/api/exportable-lib');
const createStackFilter = require('../../lib/errors/create-stack-filter.js');
const assertError = require('./helpers/assert-runtime-error').assertError;
const compile = require('./helpers/compile');
const TestRunMockCtor = require('./helpers/test-run-mock');


const copy = promisify(fs.copyFile);
Expand All @@ -22,7 +23,7 @@ const remove = promisify(fs.unlink);
require('source-map-support').install();

describe('Compiler', function () {
const testRunMock = { id: 'yo' };
const testRunMock = { id: 'yo', errs: [] };

const tsCompilerPath = path.resolve('src/compiler/test-file/formats/typescript/compiler.ts');
const apiBasedPath = path.resolve('src/compiler/test-file/api-based.js');
Expand Down Expand Up @@ -541,6 +542,11 @@ describe('Compiler', function () {
this.id = 'PPBqWA9';
this.commands = [];
this.expectedError = expectedError;
this.errs = null;
};

TestRunMock.prototype.addErrorWithScreenshot = function (errList) {
this.errs = errList;
};

TestRunMock.prototype.executeCommand = function (command) {
Expand All @@ -552,7 +558,7 @@ describe('Compiler', function () {
it('Should be resolved if the test passed', function () {
const sources = ['test/server/data/test-suites/raw/test.testcafe'];
let test = null;
const testRun = new TestRunMock();
const testRun = new TestRunMockCtor();

return compile(sources)
.then(function (compiled) {
Expand All @@ -568,7 +574,7 @@ describe('Compiler', function () {
it('Should be rejected if the test failed', function () {
const sources = ['test/server/data/test-suites/raw/test.testcafe'];
const expectedError = 'test-error';
const testRun = new TestRunMock(expectedError);
const testRun = new TestRunMockCtor(expectedError);

return compile(sources)
.then(function (compiled) {
Expand All @@ -577,9 +583,9 @@ describe('Compiler', function () {
.then(function () {
throw new Error('Promise rejection is expected');
})
.catch(function (errList) {
expect(errList.items[0].code).eql(TEST_RUN_ERRORS.uncaughtErrorInTestCode);
expect(errList.items[0].errMsg).contains('test-error');
.catch(function (errs) {
expect(errs[0].code).eql(TEST_RUN_ERRORS.uncaughtErrorInTestCode);
expect(errs[0].errMsg).contains('test-error');
expect(testRun.commands.length).eql(1);
});
});
Expand All @@ -606,7 +612,7 @@ describe('Compiler', function () {

return compile(src)
.then(function (compiled) {
return compiled.tests[0].fn({ id: 'test' });
return compiled.tests[0].fn({ id: 'test', errs: [] });
})
.then(function (compiledClientFn) {
expect(normalizeCode(compiledClientFn)).eql(normalizeCode(expected));
Expand Down Expand Up @@ -851,14 +857,14 @@ describe('Compiler', function () {
it('Incorrect callsite stack in error report if "import" is used (GH-1226)', function () {
return compile('test/server/data/test-suites/regression-gh-1226/testfile.js')
.then(function (compiled) {
return compiled.tests[0].fn(testRunMock);
return compiled.tests[0].fn(new TestRunMockCtor('err'));
})
.then(function () {
throw 'Promise rejection expected';
})
.catch(function (errList) {
.catch(function (errs) {
const stackTraceLimit = 200;
const err = errList.items[0];
const err = errs[0];
const stack = err.callsite.stackFrames.filter(createStackFilter(stackTraceLimit));

expect(stack.length).eql(3);
Expand Down