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

do not check regex optionally (closes #2074) #2980

Merged
merged 7 commits into from
Oct 30, 2018
Merged

do not check regex optionally (closes #2074) #2980

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4340df5
do not check regex optionally
AlexKamaev Oct 22, 2018
db3b5b8
add test
AlexKamaev Oct 24, 2018
2d6811b
fix test
AlexKamaev Oct 24, 2018
3f6f6d2
dirk text
AlexKamaev Oct 25, 2018
89ba68e
improve
AlexKamaev Oct 25, 2018
6414a8a
boring fix
AlexKamaev Oct 25, 2018
fdb6804
typo
AlexKamaev Oct 29, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/cli/argument-parser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ export default class CLIArgumentParser {
.option('--dev', 'enables mechanisms to log and diagnose errors')
.option('--qr-code', 'outputs QR-code that repeats URLs used to connect the remote browsers')
.option('--sf, --stop-on-first-fail', 'stop an entire test run if any test fails')
.option('--disable-test-syntax-validation', 'disables checks for \'test\' and \'fixture\' directives to run dynamically loaded tests')


// NOTE: these options will be handled by chalk internally
.option('--color', 'force colors in command line')
Expand Down
6 changes: 4 additions & 2 deletions src/compiler/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@ const testFileCompilers = [
];

export default class Compiler {
constructor (sources) {
constructor (sources, disableTestSyntaxValidation) {
this.sources = sources;

this.disableTestSyntaxValidation = disableTestSyntaxValidation;
}

static getSupportedTestFileExtensions () {
Expand All @@ -43,7 +45,7 @@ export default class Compiler {

code = stripBom(code).toString();

const compiler = find(testFileCompilers, c => c.canCompile(code, filename));
const compiler = find(testFileCompilers, c => c.canCompile(code, filename, this.disableTestSyntaxValidation));

return compiler ? await compiler.compile(code, filename) : null;
}
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/test-file/base.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ export default class TestFileCompilerBase {
throw new Error('Not implemented');
}

canCompile (code, filename) {
return this.supportedExtensionRe.test(filename) && this._hasTests(code);
canCompile (code, filename, disableTestSyntaxValidation) {
return this.supportedExtensionRe.test(filename) && (disableTestSyntaxValidation || this._hasTests(code));
AndreyBelym marked this conversation as resolved.
Show resolved Hide resolved
}

cleanUp () {
Expand Down
17 changes: 9 additions & 8 deletions src/runner/bootstrapper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,14 @@ export default class Bootstrapper {
constructor (browserConnectionGateway) {
this.browserConnectionGateway = browserConnectionGateway;

this.concurrency = 1;
this.sources = [];
this.browsers = [];
this.reporters = [];
this.filter = null;
this.appCommand = null;
this.appInitDelay = DEFAULT_APP_INIT_DELAY;
this.concurrency = 1;
this.sources = [];
this.browsers = [];
this.reporters = [];
this.filter = null;
this.appCommand = null;
this.appInitDelay = DEFAULT_APP_INIT_DELAY;
this.disableTestSyntaxValidation = false;
}

static _splitBrowserInfo (browserInfo) {
Expand Down Expand Up @@ -73,7 +74,7 @@ export default class Bootstrapper {
throw new GeneralError(MESSAGE.testSourcesNotSet);

const parsedFileList = await parseFileList(this.sources, process.cwd());
const compiler = new Compiler(parsedFileList);
const compiler = new Compiler(parsedFileList, this.disableTestSyntaxValidation);
let tests = await compiler.getTests();

const testsWithOnlyFlag = tests.filter(test => test.only);
Expand Down
4 changes: 3 additions & 1 deletion src/runner/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ export default class Runner extends EventEmitter {
return this;
}

run ({ skipJsErrors, disablePageReloads, quarantineMode, debugMode, selectorTimeout, assertionTimeout, pageLoadTimeout, speed = 1, debugOnFail, skipUncaughtErrors, stopOnFirstFail } = {}) {
run ({ skipJsErrors, disablePageReloads, quarantineMode, debugMode, selectorTimeout, assertionTimeout, pageLoadTimeout, speed = 1, debugOnFail, skipUncaughtErrors, stopOnFirstFail, disableTestSyntaxValidation } = {}) {
this.opts.skipJsErrors = !!skipJsErrors;
this.opts.disablePageReloads = !!disablePageReloads;
this.opts.quarantineMode = !!quarantineMode;
Expand All @@ -262,6 +262,8 @@ export default class Runner extends EventEmitter {
this.opts.skipUncaughtErrors = !!skipUncaughtErrors;
this.opts.stopOnFirstFail = !!stopOnFirstFail;

this.bootstrapper.disableTestSyntaxValidation = disableTestSyntaxValidation;

const runTaskPromise = Promise.resolve()
.then(() => {
this._validateRunOptions();
Expand Down
9 changes: 9 additions & 0 deletions test/functional/fixtures/regression/gh-2074/pages/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<!DOCTYPE html>
<html>
<head>
<meta charset="UTF-8">
<title>gh-2074</title>
</head>
<body>
</body>
</html>
17 changes: 17 additions & 0 deletions test/functional/fixtures/regression/gh-2074/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
const expect = require('chai').expect;

describe('[Regression](GH-2074)', function () {
it('Should execute test located in external module', function () {
return runTests('testcafe-fixtures/index.js', null, { shouldFail: true, disableTestSyntaxValidation: true })
.catch(errors => {
if (Array.isArray(errors))
expect(errors[0]).contains('test is executed');
else {
Object.values(errors).forEach(err => {
expect(err[0]).contains('test is executed');
});
}
});
});
});

3 changes: 3 additions & 0 deletions test/functional/fixtures/regression/gh-2074/testcafe-fixtures/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import libraryTest from './lib';

libraryTest();
9 changes: 9 additions & 0 deletions test/functional/fixtures/regression/gh-2074/testcafe-fixtures/lib.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export default function libraryTests () {
fixture('gh-2074').page('../pages/index.html');

test('Do nothing', async () => {
throw new Error('test is executed');
});
}


50 changes: 27 additions & 23 deletions test/functional/setup.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,28 +157,30 @@ before(function () {
global.testCafe = testCafe;

global.runTests = (fixture, testName, opts) => {
const stream = createTestStream();
const runner = testCafe.createRunner();
const fixturePath = typeof fixture !== 'string' || path.isAbsolute(fixture) ? fixture : path.join(path.dirname(caller()), fixture);
const skipJsErrors = opts && opts.skipJsErrors;
const disablePageReloads = opts && opts.disablePageReloads;
const quarantineMode = opts && opts.quarantineMode;
const selectorTimeout = opts && opts.selectorTimeout || FUNCTIONAL_TESTS_SELECTOR_TIMEOUT;
const assertionTimeout = opts && opts.assertionTimeout || FUNCTIONAL_TESTS_ASSERTION_TIMEOUT;
const pageLoadTimeout = opts && opts.pageLoadTimeout || FUNCTIONAL_TESTS_PAGE_LOAD_TIMEOUT;
const onlyOption = opts && opts.only;
const skipOption = opts && opts.skip;
const screenshotPath = opts && opts.setScreenshotPath ? '___test-screenshots___' : '';
const screenshotPathPattern = opts && opts.screenshotPathPattern;
const screenshotsOnFails = opts && opts.screenshotsOnFails;
const speed = opts && opts.speed;
const appCommand = opts && opts.appCommand;
const appInitDelay = opts && opts.appInitDelay;
const externalProxyHost = opts && opts.useProxy;
const proxyBypass = opts && opts.proxyBypass;
const customReporters = opts && opts.reporters;
const skipUncaughtErrors = opts && opts.skipUncaughtErrors;
const stopOnFirstFail = opts && opts.stopOnFirstFail;
const stream = createTestStream();
const runner = testCafe.createRunner();
const fixturePath = typeof fixture !== 'string' ||
path.isAbsolute(fixture) ? fixture : path.join(path.dirname(caller()), fixture);
const skipJsErrors = opts && opts.skipJsErrors;
const disablePageReloads = opts && opts.disablePageReloads;
const quarantineMode = opts && opts.quarantineMode;
const selectorTimeout = opts && opts.selectorTimeout || FUNCTIONAL_TESTS_SELECTOR_TIMEOUT;
const assertionTimeout = opts && opts.assertionTimeout || FUNCTIONAL_TESTS_ASSERTION_TIMEOUT;
const pageLoadTimeout = opts && opts.pageLoadTimeout || FUNCTIONAL_TESTS_PAGE_LOAD_TIMEOUT;
const onlyOption = opts && opts.only;
const skipOption = opts && opts.skip;
const screenshotPath = opts && opts.setScreenshotPath ? '___test-screenshots___' : '';
const screenshotPathPattern = opts && opts.screenshotPathPattern;
const screenshotsOnFails = opts && opts.screenshotsOnFails;
const speed = opts && opts.speed;
const appCommand = opts && opts.appCommand;
const appInitDelay = opts && opts.appInitDelay;
const externalProxyHost = opts && opts.useProxy;
const proxyBypass = opts && opts.proxyBypass;
const customReporters = opts && opts.reporters;
const skipUncaughtErrors = opts && opts.skipUncaughtErrors;
const stopOnFirstFail = opts && opts.stopOnFirstFail;
const disableTestSyntaxValidation = opts && opts.disableTestSyntaxValidation;

const actualBrowsers = browsersInfo.filter(browserInfo => {
const { alias, userAgent } = browserInfo.settings;
Expand Down Expand Up @@ -216,6 +218,7 @@ before(function () {
return runner
.useProxy(externalProxyHost, proxyBypass)
.browsers(connections)

.filter(test => {
return testName ? test === testName : true;
})
Expand All @@ -231,7 +234,8 @@ before(function () {
pageLoadTimeout,
speed,
stopOnFirstFail,
skipUncaughtErrors
skipUncaughtErrors,
disableTestSyntaxValidation
})
.then(failedCount => {
if (customReporters)
Expand Down
3 changes: 2 additions & 1 deletion test/server/cli-argument-parser-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,8 @@ describe('CLI argument parser', function () {
{ long: '--skip-uncaught-errors' },
{ long: '--color' },
{ long: '--no-color' },
{ long: '--stop-on-first-fail', short: '--sf' }
{ long: '--stop-on-first-fail', short: '--sf' },
{ long: '--disable-test-syntax-validation' }
];

const parser = new CliArgumentParser('');
Expand Down
100 changes: 100 additions & 0 deletions test/server/compiler-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,39 @@ describe('Compiler', function () {
});

describe('ES-next', function () {
it('Should compile test defined in separate module if option is enabled', function () {
const sources = [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also add tests for negative cases:
If there are no test and/or fixture definitions into imported files, then TestCafe should report clear error messages.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

'test/server/data/test-suites/test-as-module/with-tests/testfile.js'
];

return compile(sources, true)
.then(function (compiled) {
const tests = compiled.tests;
const fixtures = compiled.fixtures;

expect(tests.length).eql(1);
expect(fixtures.length).eql(1);

expect(tests[0].name).eql('test');
expect(fixtures[0].name).eql('Library tests');
});
});

it('Should not compile test defined in separate module if option is disabled', function () {
const sources = [
'test/server/data/test-suites/test-as-module/with-tests/testfile.js'
];

return compile(sources)
.then(function (compiled) {
const tests = compiled.tests;
const fixtures = compiled.fixtures;

expect(tests.length).eql(0);
expect(fixtures.length).eql(0);
});
});

it('Should compile test files and their dependencies', function () {
const sources = [
'test/server/data/test-suites/basic/testfile1.js',
Expand Down Expand Up @@ -140,6 +173,39 @@ describe('Compiler', function () {


describe('TypeScript', function () {
it('Should compile test defined in separate module if option is enabled', function () {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does we need addtional tests written with TypeScript and CoffeeScript?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have several compilers and every on them has it's own set of similar tests. It's true that I modified base behavior which affects all of the compilers at the same time, but I think it'd better to have full set of tests

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok

const sources = [
'test/server/data/test-suites/test-as-module/with-tests/testfile.ts'
];

return compile(sources, true)
.then(function (compiled) {
const tests = compiled.tests;
const fixtures = compiled.fixtures;

expect(tests.length).eql(1);
expect(fixtures.length).eql(1);

expect(tests[0].name).eql('test');
expect(fixtures[0].name).eql('Library tests');
});
});

it('Should not compile test defined in separate module if option is disabled', function () {
const sources = [
'test/server/data/test-suites/test-as-module/with-tests/testfile.ts'
];

return compile(sources)
.then(function (compiled) {
const tests = compiled.tests;
const fixtures = compiled.fixtures;

expect(tests.length).eql(0);
expect(fixtures.length).eql(0);
});
});

it('Should compile test files and their dependencies', function () {
const sources = [
'test/server/data/test-suites/typescript-basic/testfile1.ts',
Expand Down Expand Up @@ -248,6 +314,40 @@ describe('Compiler', function () {


describe('CoffeeScript', function () {
it('Should compile test defined in separate module if option is enabled', function () {
const sources = [
'test/server/data/test-suites/test-as-module/with-tests/testfile.coffee'
];

return compile(sources, true)
.then(function (compiled) {
const tests = compiled.tests;
const fixtures = compiled.fixtures;

expect(tests.length).eql(1);
expect(fixtures.length).eql(1);

expect(tests[0].name).eql('test');
expect(fixtures[0].name).eql('Library tests');
});
});

it('Should not compile test defined in separate module if option is disabled', function () {
const sources = [
'test/server/data/test-suites/test-as-module/with-tests/testfile.coffee'
];

return compile(sources)
.then(function (compiled) {
const tests = compiled.tests;
const fixtures = compiled.fixtures;

expect(tests.length).eql(0);
expect(fixtures.length).eql(0);
});
});


it('Should compile test files and their dependencies', function () {
const sources = [
'test/server/data/test-suites/coffeescript-basic/testfile1.coffee',
Expand Down
7 changes: 7 additions & 0 deletions test/server/data/test-suites/test-as-module/with-tests/lib.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default function libraryTests () {
fixture('Library tests').page('http://example.com');

test('test', async t => {
await t.click('h1');
});
}
3 changes: 3 additions & 0 deletions test/server/data/test-suites/test-as-module/with-tests/testfile.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import libraryTest from './lib';

libraryTest();
3 changes: 3 additions & 0 deletions test/server/data/test-suites/test-as-module/with-tests/testfile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import libraryTest from './lib';

libraryTest();
3 changes: 3 additions & 0 deletions test/server/data/test-suites/test-as-module/with-tests/testfile.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import libraryTest from './lib';

libraryTest();
2 changes: 2 additions & 0 deletions test/server/data/test-suites/test-as-module/without-tests/lib.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export default function libraryTests () {
}
3 changes: 3 additions & 0 deletions test/server/data/test-suites/test-as-module/without-tests/testfile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import libraryTest from './lib';

libraryTest();
4 changes: 2 additions & 2 deletions test/server/helpers/compile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@ const sortBy = require('lodash').sortBy;
const resolve = require('path').resolve;
const Compiler = require('../../../lib/compiler');

module.exports = function compile (sources) {
module.exports = function compile (sources, disableTestSyntaxValidation = false) {
sources = Array.isArray(sources) ? sources : [sources];

sources = sources.map(function (filename) {
return resolve(filename);
});

const compiler = new Compiler(sources);
const compiler = new Compiler(sources, disableTestSyntaxValidation);

return compiler.getTests()
.then(function (tests) {
Expand Down
10 changes: 10 additions & 0 deletions test/server/runner-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,16 @@ describe('Runner', () => {
expect(err.message).eql('No test file specified.');
});
});

it('Should raise an error if the source and imported module have no tests', () => {
return runner
.browsers(connection)
.src(['test/server/data/test-suites/test-as-module/without-tests/testfile.js'])
.run()
.catch(err => {
expect(err.message).eql('No tests to run. Either the test files contain no tests or the filter function is too restrictive.');
});
});
});

describe('.filter()', () => {
Expand Down