Skip to content

Commit 7079298

Browse files
AlexKamaevAlexKamaev
committed
throw error in proxy-bypass is not string and refactor runner validation
1 parent b7736b5 commit 7079298

File tree

5 files changed

+72
-64
lines changed

5 files changed

+72
-64
lines changed

‎src/runner/index.js‎

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import Reporter from '../reporter';
99
import Task from './task';
1010
import { GeneralError } from '../errors/runtime';
1111
import MESSAGE from '../errors/runtime/message';
12+
import { assertType, is } from '../errors/runtime/type-assertions';
1213

1314

1415
const DEFAULT_SELECTOR_TIMEOUT = 10000;
@@ -26,7 +27,7 @@ export default class Runner extends EventEmitter {
2627

2728
this.opts = {
2829
externalProxyHost: null,
29-
proxyBypass: [],
30+
proxyBypass: null,
3031
screenshotPath: null,
3132
takeScreenshotsOnFails: false,
3233
skipJsErrors: false,
@@ -119,6 +120,21 @@ export default class Runner extends EventEmitter {
119120
assets.forEach(asset => this.proxy.GET(asset.path, asset.info));
120121
}
121122

123+
_validateRunOptions () {
124+
const concurrency = this.bootstrapper.concurrency;
125+
const speed = this.opts.speed;
126+
const proxyBypass = this.opts.proxyBypass;
127+
128+
if (typeof speed !== 'number' || isNaN(speed) || speed < 0.01 || speed > 1)
129+
throw new GeneralError(MESSAGE.invalidSpeedValue);
130+
131+
if (typeof concurrency !== 'number' || isNaN(concurrency) || concurrency < 1)
132+
throw new GeneralError(MESSAGE.invalidConcurrencyFactor);
133+
134+
if (proxyBypass)
135+
assertType(is.string, null, '"proxyBypass" argument', proxyBypass);
136+
}
137+
122138

123139
// API
124140
embeddingOptions (opts) {
@@ -143,9 +159,6 @@ export default class Runner extends EventEmitter {
143159
}
144160

145161
concurrency (concurrency) {
146-
if (typeof concurrency !== 'number' || isNaN(concurrency) || concurrency < 1)
147-
throw new GeneralError(MESSAGE.invalidConcurrencyFactor);
148-
149162
this.bootstrapper.concurrency = concurrency;
150163

151164
return this;
@@ -168,9 +181,7 @@ export default class Runner extends EventEmitter {
168181

169182
useProxy (externalProxyHost, proxyBypass) {
170183
this.opts.externalProxyHost = externalProxyHost;
171-
172-
if (proxyBypass && typeof proxyBypass === 'string')
173-
this.opts.proxyBypass = proxyBypass.split(',');
184+
this.opts.proxyBypass = proxyBypass;
174185

175186
return this;
176187
}
@@ -198,12 +209,13 @@ export default class Runner extends EventEmitter {
198209
this.opts.assertionTimeout = assertionTimeout === void 0 ? DEFAULT_ASSERTION_TIMEOUT : assertionTimeout;
199210
this.opts.pageLoadTimeout = pageLoadTimeout === void 0 ? DEFAULT_PAGE_LOAD_TIMEOUT : pageLoadTimeout;
200211

201-
if (typeof speed !== 'number' || isNaN(speed) || speed < 0.01 || speed > 1)
202-
throw new GeneralError(MESSAGE.invalidSpeedValue);
203-
204212
this.opts.speed = speed;
205213

206-
var runTaskPromise = this.bootstrapper.createRunnableConfiguration()
214+
var runTaskPromise = Promise.resolve()
215+
.then(() => {
216+
this._validateRunOptions();
217+
return this.bootstrapper.createRunnableConfiguration();
218+
})
207219
.then(({ reporterPlugins, browserSet, tests, testedApp }) => {
208220
this.emit('done-bootstrapping');
209221

‎src/runner/test-run-controller.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ export default class TestRunController extends EventEmitter {
128128
testRun.start();
129129

130130
const pageUrl = testRun.test.pageUrl;
131-
const needBypassHost = checkUrl(pageUrl, this.opts.proxyBypass);
131+
const needBypassHost = this.opts.proxyBypass && checkUrl(pageUrl, this.opts.proxyBypass.split(','));
132132
const externalProxyHost = needBypassHost ? null : this.opts.externalProxyHost;
133133

134134
return this.proxy.openSession(pageUrl, testRun, externalProxyHost);

‎test/functional/fixtures/proxy/test.js‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,5 @@ describe('Using proxy-bypass', function () {
2222
it('Should bypass using proxy by set of rules', function () {
2323
return runTests('testcafe-fixtures/index.test.js', null, { useProxy: ERROR_PROXY_URL, proxyBypass: 'dummy,localhost:3000' });
2424
});
25-
26-
it('Should use proxy and not fail on wrong proxy bypass type', function () {
27-
return runTests('testcafe-fixtures/index.test.js', null, { useProxy: TRANSPARENT_PROXY_URL, proxyBypass: 1 });
28-
});
2925
});
3026

‎test/server/match-url-test.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,6 @@ it('Should check does url match rule', function () {
128128
expect(matchUrl('127.0.0.1', rule)).to.be.true;
129129
expect(matchUrl('http://127.0.0.1', rule)).to.be.true;
130130
expect(matchUrl('https://127.0.0.1', rule)).to.be.true;
131-
expect(matchUrl('https://127.0.0.1', rule)).to.be.true;
132131

133132
rule = '127.0.0.';
134133

‎test/server/runner-test.js‎

Lines changed: 48 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -497,58 +497,59 @@ describe('Runner', function () {
497497
});
498498

499499
it('Should raise an error if speed option has wrong value', function () {
500-
var incorrectSpeedErrorMessage = 'Speed should be a number between 0.01 and 1.';
500+
var exceptionCount = 0;
501+
var incorrectSpeedError = function (speed) {
502+
return runner
503+
.run({ speed })
504+
.catch(function (err) {
505+
exceptionCount++;
506+
expect(err.message).eql('Speed should be a number between 0.01 and 1.');
507+
});
508+
};
501509

502-
return testCafe
503-
.createBrowserConnection()
504-
.then(function (browserConnection) {
505-
return runner
506-
.browsers(browserConnection)
507-
.run({ speed: 'yo' });
508-
})
509-
.catch(function (err) {
510-
expect(err.message).eql(incorrectSpeedErrorMessage);
511-
})
512-
.then(function () {
513-
return runner.run({ speed: -0.01 });
514-
}).catch(function (err) {
515-
expect(err.message).eql(incorrectSpeedErrorMessage);
516-
})
517-
.then(function () {
518-
return runner.run({ speed: 1.01 });
519-
}).catch(function (err) {
520-
expect(err.message).eql(incorrectSpeedErrorMessage);
521-
});
510+
return Promise.resolve()
511+
.then(() => incorrectSpeedError('yo'))
512+
.then(() => incorrectSpeedError(-0.01))
513+
.then(() => incorrectSpeedError(0))
514+
.then(() => incorrectSpeedError(1.01))
515+
.then(() => expect(exceptionCount).to.be.eql(4));
522516
});
523517

524518
it('Should raise an error if concurrency option has wrong value', function () {
525-
var incorrectConcurrencyFactorErrorMessage = 'The concurrency factor should be an integer greater or equal to 1.';
519+
var exceptionCount = 0;
520+
var incorrectConcurrencyFactorError = function (concurrency) {
521+
return runner
522+
.concurrency(concurrency)
523+
.run()
524+
.catch(function (err) {
525+
exceptionCount++;
526+
expect(err.message).eql('The concurrency factor should be an integer greater or equal to 1.');
527+
});
528+
};
526529

527-
return testCafe
528-
.createBrowserConnection()
529-
.then(function (browserConnection) {
530-
return runner
531-
.browsers(browserConnection)
532-
.concurrency('yo');
533-
})
534-
.catch(function (err) {
535-
expect(err.message).eql(incorrectConcurrencyFactorErrorMessage);
536-
})
537-
.then(function () {
538-
return runner.concurrency(-1);
539-
}).catch(function (err) {
540-
expect(err.message).eql(incorrectConcurrencyFactorErrorMessage);
541-
})
542-
.then(function () {
543-
return runner.concurrency(0.1);
544-
}).catch(function (err) {
545-
expect(err.message).eql(incorrectConcurrencyFactorErrorMessage);
546-
})
547-
.then(function () {
548-
return runner.concurrency(0);
549-
}).catch(function (err) {
550-
expect(err.message).eql(incorrectConcurrencyFactorErrorMessage);
551-
});
530+
return Promise.resolve()
531+
.then(() => incorrectConcurrencyFactorError('yo'))
532+
.then(() => incorrectConcurrencyFactorError(-1))
533+
.then(() => incorrectConcurrencyFactorError(0.1))
534+
.then(() => incorrectConcurrencyFactorError(0))
535+
.then(() => expect(exceptionCount).to.be.eql(4));
536+
});
537+
538+
it('Should raise an error if proxyBypass option has wrong type', function () {
539+
var exceptionCount = 0;
540+
var expectProxyBypassError = function (proxyBypass, type) {
541+
runner.opts.proxyBypass = proxyBypass;
542+
return runner.run()
543+
.catch(function (err) {
544+
exceptionCount++;
545+
expect(err.message).contains('"proxyBypass" argument is expected to be a string, but it was ' + type);
546+
});
547+
};
548+
549+
return expectProxyBypassError(1, 'number')
550+
.then(() => expectProxyBypassError({}, 'object'))
551+
.then(() => expectProxyBypassError(true, 'bool'))
552+
.then(() => expect(exceptionCount).to.be.eql(3));
552553
});
553554
});
554555

0 commit comments

Comments
 (0)