Skip to content

Commit

Permalink
fix 'wrong callsite for RequestHooks errors' (close #2555) (#2603)
Browse files Browse the repository at this point in the history
* tmp

* fix 'wrong callsite for RequestHooks errors' (close #2555)
  • Loading branch information
miherlosev authored and helen-dikareva committed Jul 10, 2018
1 parent 8cde8f7 commit 27fb1e7
Show file tree
Hide file tree
Showing 13 changed files with 174 additions and 103 deletions.
13 changes: 7 additions & 6 deletions src/api/request-hooks/request-logger.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import RequestHook from './hook';
import { parse as parseUserAgent } from 'useragent';
import testRunTracker from '../test-run-tracker';
import ReExecutablePromise from '../../utils/re-executable-promise';
import { RequestHookConfigureAPIError } from '../../errors/test-run/index';
import { APIError } from '../../errors/runtime';
import MESSAGE from '../../errors/runtime/message';

const DEFAULT_OPTIONS = {
logRequestHeaders: false,
Expand All @@ -14,10 +15,10 @@ const DEFAULT_OPTIONS = {
stringifyResponseBody: false
};

class RequestLogger extends RequestHook {
class RequestLoggerImplementation extends RequestHook {
constructor (requestFilterRuleInit, options) {
options = Object.assign({}, DEFAULT_OPTIONS, options);
RequestLogger._assertLogOptions(options);
RequestLoggerImplementation._assertLogOptions(options);

const configureResponseEventOptions = new ConfigureResponseEventOptions(options.logResponseHeaders, options.logResponseBody);

Expand All @@ -30,10 +31,10 @@ class RequestLogger extends RequestHook {

static _assertLogOptions (logOptions) {
if (!logOptions.logRequestBody && logOptions.stringifyRequestBody)
throw new RequestHookConfigureAPIError(RequestLogger.name, 'Cannot stringify the request body because it is not logged. Specify { logRequestBody: true } in log options.');
throw new APIError('RequestLogger', MESSAGE.requestHookConfigureAPIError, 'RequestLogger', 'Cannot stringify the request body because it is not logged. Specify { logRequestBody: true } in log options.');

if (!logOptions.logResponseBody && logOptions.stringifyResponseBody)
throw new RequestHookConfigureAPIError(RequestLogger.name, 'Cannot stringify the response body because it is not logged. Specify { logResponseBody: true } in log options.');
throw new APIError('RequestLogger', MESSAGE.requestHookConfigureAPIError, 'RequestLogger', 'Cannot stringify the response body because it is not logged. Specify { logResponseBody: true } in log options.');
}

onRequest (event) {
Expand Down Expand Up @@ -120,6 +121,6 @@ class RequestLogger extends RequestHook {
}

export default function createRequestLogger (requestFilterRuleInit, logOptions) {
return new RequestLogger(requestFilterRuleInit, logOptions);
return new RequestLoggerImplementation(requestFilterRuleInit, logOptions);
}

7 changes: 4 additions & 3 deletions src/api/request-hooks/request-mock.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import RequestHook from './hook';
import { ResponseMock, RequestFilterRule } from 'testcafe-hammerhead';
import { RequestHookConfigureAPIError } from '../../errors/test-run/index';
import { APIError } from '../../errors/runtime';
import MESSAGE from '../../errors/runtime/message';

class RequestMock extends RequestHook {
constructor () {
Expand All @@ -21,7 +22,7 @@ class RequestMock extends RequestHook {
// API
onRequestTo (requestFilterRuleInit) {
if (this.pendingRequestFilterRuleInit)
throw new RequestHookConfigureAPIError(RequestMock.name, "The 'respond' method was not called after 'onRequestTo'. You must call the 'respond' method to provide the mocked response.");
throw new APIError('onRequestTo', MESSAGE.requestHookConfigureAPIError, RequestMock.name, "The 'respond' method was not called after 'onRequestTo'. You must call the 'respond' method to provide the mocked response.");

this.pendingRequestFilterRuleInit = requestFilterRuleInit;

Expand All @@ -30,7 +31,7 @@ class RequestMock extends RequestHook {

respond (body, statusCode, headers) {
if (!this.pendingRequestFilterRuleInit)
throw new RequestHookConfigureAPIError(RequestMock.name, "The 'onRequestTo' method was not called before 'respond'. You must call the 'onRequestTo' method to provide the URL requests to which are mocked.");
throw new APIError('respond', MESSAGE.requestHookConfigureAPIError, RequestMock.name, "The 'onRequestTo' method was not called before 'respond'. You must call the 'onRequestTo' method to provide the URL requests to which are mocked.");

const mock = new ResponseMock(body, statusCode, headers);
const rule = new RequestFilterRule(this.pendingRequestFilterRuleInit);
Expand Down
3 changes: 2 additions & 1 deletion src/errors/runtime/message.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 0 additions & 11 deletions src/errors/test-run/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -477,14 +477,3 @@ export class SetNativeDialogHandlerCodeWrongTypeError extends TestRunErrorBase {
this.actualType = actualType;
}
}

// Request Hooks
export class RequestHookConfigureAPIError extends TestRunErrorBase {
constructor (requestHookName, errMsg) {
super(TYPE.requestHookConfigureAPIError);

this.requestHookName = requestHookName;
this.errMsg = errMsg;
}
}

6 changes: 0 additions & 6 deletions src/errors/test-run/templates.js
Original file line number Diff line number Diff line change
Expand Up @@ -262,12 +262,6 @@ export default {
${err.errMsg}
`),

[TYPE.requestHookConfigureAPIError]: err => markup(err, `
There was an error while configuring the request hook:
${err.requestHookName}: ${err.errMsg}
`),

[TYPE.assertionUnawaitedPromiseError]: err => markup(err, `
Attempted to run assertions on a Promise object. Did you forget to await it? If not, pass "{ allowUnawaitedPromise: true }" to the assertion options.
`)
Expand Down
3 changes: 1 addition & 2 deletions src/errors/test-run/type.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,5 @@ export default {
invalidElementScreenshotDimensionsError: 'invalidElementScreenshotDimensionsError',
roleSwitchInRoleInitializerError: 'roleSwitchInRoleInitializerError',
assertionExecutableArgumentError: 'assertionExecutableArgumentError',
assertionUnawaitedPromiseError: 'assertionUnawaitedPromiseError',
requestHookConfigureAPIError: 'requestHookConfigureAPIError'
assertionUnawaitedPromiseError: 'assertionUnawaitedPromiseError'
};
114 changes: 114 additions & 0 deletions test/server/api-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1336,4 +1336,118 @@ describe('API', function () {
});
});
});

describe('Request Hooks', () => {
describe('Should raise errors for wrong RequestLogger construction', () => {
it('Cannot stringify the request body', () => {
const testFile = resolve('test/server/data/test-suites/request-hooks/request-logger/cannot-stringify-request-body.js');

return compile(testFile)
.then(() => {
throw new Error('Promise rejection expected');
})
.catch(err => {
assertAPIError(err, {
stackTop: testFile,

message: 'Cannot prepare tests due to an error.\n\n' +
'There was an error while configuring the request hook:\n\n' +
'RequestLogger: Cannot stringify the request body because it is not logged. Specify { logRequestBody: true } in log options.',

callsite: ' 1 |import { RequestLogger } from \'testcafe\';\n' +
' 2 |\n' +
' 3 |fixture `Fixture`;\n' +
' 4 |\n' +
" > 5 |const logger = new RequestLogger('', {\n" +
' 6 | logRequestBody: false,\n' +
' 7 | stringifyRequestBody: true\n' +
' 8 |});'
});
});
});

it('Cannot stringify the response body', () => {
const testFile = resolve('test/server/data/test-suites/request-hooks/request-logger/cannot-stringify-response-body.js');

return compile(testFile)
.then(() => {
throw new Error('Promise rejection expected');
})
.catch(err => {
assertAPIError(err, {
stackTop: testFile,

message: 'Cannot prepare tests due to an error.\n\n' +
'There was an error while configuring the request hook:\n\n' +
'RequestLogger: Cannot stringify the response body because it is not logged. Specify { logResponseBody: true } in log options.',

callsite: ' 1 |import { RequestLogger } from \'testcafe\';\n' +
' 2 |\n' +
' 3 |fixture `Fixture`;\n' +
' 4 |\n' +
" > 5 |const logger = new RequestLogger('', {\n" +
' 6 | logResponseBody: false,\n' +
' 7 | stringifyResponseBody: true\n' +
' 8 |});'
});
});
});
});

describe('Should raise errors for wrong RequestMock api order call', () => {
it("The 'respond' method was not called after 'onRequestTo'", () => {
const testFile = resolve('test/server/data/test-suites/request-hooks/request-mock/respond-was-not-called-after-on-request-to.js');

return compile(testFile)
.then(() => {
throw new Error('Promise rejection expected');
})
.catch(err => {
assertAPIError(err, {
stackTop: testFile,

message: 'Cannot prepare tests due to an error.\n\n' +
'There was an error while configuring the request hook:\n\n' +
"RequestMock: The 'respond' method was not called after 'onRequestTo'. You must call the 'respond' method to provide the mocked response.",

callsite: ' 1 |import { RequestMock } from \'testcafe\';\n' +
' 2 |\n' +
' 3 |fixture `Fixture`;\n' +
' 4 |\n' +
' > 5 |const mock = RequestMock().onRequestTo({}).onRequestTo({});\n' +
' 6 |\n' +
' 7 |test(\'test\', async t => {});\n' +
' 8 |\n'
});
});
});

it("The 'onRequestTo' method was not called before 'respond'", () => {
const testFile = resolve('test/server/data/test-suites/request-hooks/request-mock/on-request-to-was-not-called-before-respond.js');

return compile(testFile)
.then(() => {
throw new Error('Promise rejection expected');
})
.catch(err => {
assertAPIError(err, {
stackTop: testFile,

message: 'Cannot prepare tests due to an error.\n\n' +
'There was an error while configuring the request hook:\n\n' +
"RequestMock: The 'onRequestTo' method was not called before 'respond'. You must call the 'onRequestTo' method to provide the URL requests to which are mocked.",

callsite: ' 1 |import { RequestMock } from \'testcafe\';\n' +
' 2 |\n' +
' 3 |fixture `Fixture`;\n' +
' 4 |\n' +
' > 5 |const mock = RequestMock().respond(() => {}).onRequestTo({});\n' +
' 6 |\n' +
' 7 |test(\'test\', async t => {});\n' +
' 8 |'
});
});
});
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { RequestLogger } from 'testcafe';

fixture `Fixture`;

const logger = new RequestLogger('', {
logRequestBody: false,
stringifyRequestBody: true
});

test('test', async () => {});
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { RequestLogger } from 'testcafe';

fixture `Fixture`;

const logger = new RequestLogger('', {
logResponseBody: false,
stringifyResponseBody: true
});

test('test', async () => {});
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { RequestMock } from 'testcafe';

fixture `Fixture`;

const mock = RequestMock().respond(() => {}).onRequestTo({});

test('test', async t => {});
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { RequestMock } from 'testcafe';

fixture `Fixture`;

const mock = RequestMock().onRequestTo({}).onRequestTo({});

test('test', async t => {});
81 changes: 12 additions & 69 deletions test/server/request-hooks-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@ const RequestLogger = exportableLib.RequestLogger;
const RequestHook = exportableLib.RequestHook;
const Promise = require('pinkie');
const nanoid = require('nanoid');

const assertThrow = require('./helpers/assert-error').assertThrow;
const expect = require('chai').expect;
const noop = require('lodash').noop;
const expect = require('chai').expect;

describe('RequestLogger', () => {
const createProxyRequestEventMock = (testRunId, requestId) => {
Expand Down Expand Up @@ -102,32 +99,6 @@ describe('RequestLogger', () => {
});
});

describe('Assert log options', () => {
it('Related to request body', () => {
assertThrow(() => {
RequestLogger('http://example.com', { stringifyRequestBody: true });
}, {
isTestCafeError: true,
requestHookName: 'RequestLogger',
errMsg: 'Cannot stringify the request body because it is not logged. Specify { logRequestBody: true } in log options.',
type: 'requestHookConfigureAPIError',
callsite: null
});
});

it('Related to response body', () => {
assertThrow(() => {
RequestLogger('http://example.com', { stringifyResponseBody: true });
}, {
isTestCafeError: true,
requestHookName: 'RequestLogger',
errMsg: 'Cannot stringify the response body because it is not logged. Specify { logResponseBody: true } in log options.',
type: 'requestHookConfigureAPIError',
callsite: null
});
});
});

it('.clear method', () => {
const logger = new RequestLogger('http://example.com');

Expand Down Expand Up @@ -206,46 +177,18 @@ describe('RequestLogger', () => {
});

describe('RequestMock', () => {
describe('Throwing errors', () => {
describe('Chaining', () => {
it('.respond().onRequestTo()', () => {
assertThrow(() => {
RequestMock().respond(noop).onRequestTo({});
}, {
isTestCafeError: true,
requestHookName: 'RequestMock',
errMsg: "The 'onRequestTo' method was not called before 'respond'. You must call the 'onRequestTo' method to provide the URL requests to which are mocked.",
type: 'requestHookConfigureAPIError',
callsite: null
});
});

it('onRequestTo().onRequestTo()', () => {
assertThrow(() => {
RequestMock().onRequestTo({}).onRequestTo({});
}, {
isTestCafeError: true,
requestHookName: 'RequestMock',
errMsg: "The 'respond' method was not called after 'onRequestTo'. You must call the 'respond' method to provide the mocked response.",
type: 'requestHookConfigureAPIError',
callsite: null
});
});
describe('Throwing errors on construction', () => {
it('Without configure', () => {
expect(() => {
RequestMock();
}).to.not.throw;
});

describe('Construction', () => {
it('Without configure', () => {
expect(() => {
RequestMock();
}).to.not.throw;
});
it('With configure', () => {
expect(() => {
RequestMock()
.onRequestTo('http://example.com')
.respond('<html></html>');
}).to.not.throw;
});
it('With configure', () => {
expect(() => {
RequestMock()
.onRequestTo('http://example.com')
.respond('<html></html>');
}).to.not.throw;
});
});

Expand Down
5 changes: 0 additions & 5 deletions test/server/test-run-error-formatting-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ var InvalidElementScreenshotDimensionsError = require('../../lib/error
var SetTestSpeedArgumentError = require('../../lib/errors/test-run').SetTestSpeedArgumentError;
var RoleSwitchInRoleInitializerError = require('../../lib/errors/test-run').RoleSwitchInRoleInitializerError;
var ActionRoleArgumentError = require('../../lib/errors/test-run').ActionRoleArgumentError;
var RequestHookConfigureAPIError = require('../../lib/errors/test-run').RequestHookConfigureAPIError;

var TEST_FILE_STACK_ENTRY_RE = new RegExp('\\s*\\n?\\(' + escapeRe(require.resolve('./data/test-callsite')), 'g');

Expand Down Expand Up @@ -337,10 +336,6 @@ describe('Error formatting', function () {
assertErrorMessage('assertion-executable-argument-error', new AssertionExecutableArgumentError('actual', '1 + temp', 'Unexpected identifier'));
});

it('Should format "requestHookConfigureAPIError"', () => {
assertErrorMessage('request-hook-configure-api-error', new RequestHookConfigureAPIError('RequestMock', 'test error message'));
});

it('Should format "assertionUnawaitedPromiseError"', function () {
assertErrorMessage('assertion-unawaited-promise-error', new AssertionUnawaitedPromiseError(testCallsite));
});
Expand Down

0 comments on commit 27fb1e7

Please sign in to comment.