Skip to content

Commit

Permalink
Get rid of "retainLines" compiler option (closes #1267) (#1369)
Browse files Browse the repository at this point in the history
* Get rid of "retainLines" compiler option (closes #1267)

* Update custom "callsite-record" module

* update callsite-record to 4.0.0

* comment added
  • Loading branch information
georgiy-abbasov authored and inikulin committed Apr 3, 2017
1 parent 459c517 commit 8e449c8
Show file tree
Hide file tree
Showing 61 changed files with 128 additions and 75 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
"babel-runtime": "^6.22.0",
"bin-v8-flags-filter": "^1.0.0",
"callsite": "^1.0.0",
"callsite-record": "^3.2.1",
"callsite-record": "^4.0.0",
"chai": "^3.0.0",
"chalk": "^1.1.0",
"commander": "^2.8.1",
Expand Down
6 changes: 3 additions & 3 deletions src/api/test-controller/index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import Promise from 'pinkie';
import { identity, assign, isNil as isNullOrUndefined } from 'lodash';
import { MissingAwaitError } from '../../errors/test-run';
import getCallsite from '../../errors/get-callsite';
import { getCallsiteForMethod } from '../../errors/get-callsite';
import ClientFunctionBuilder from '../../client-functions/client-function-builder';
import Assertion from './assertion';
import { getDelegatedAPIList, delegateAPI } from '../../utils/delegated-api';
Expand Down Expand Up @@ -82,7 +82,7 @@ export default class TestController {
_enqueueTask (apiMethodName, createTaskExecutor) {
this._checkForMissingAwait();

var callsite = getCallsite(apiMethodName);
var callsite = getCallsiteForMethod(apiMethodName);
var executor = createTaskExecutor(callsite);

this.executionChain = this.executionChain.then(executor);
Expand Down Expand Up @@ -242,7 +242,7 @@ export default class TestController {
}

_getNativeDialogHistory$ () {
var callsite = getCallsite('getNativeDialogHistory');
var callsite = getCallsiteForMethod('getNativeDialogHistory');

return this.testRun.executeCommand(new GetNativeDialogHistoryCommand(), callsite);
}
Expand Down
4 changes: 2 additions & 2 deletions src/client-functions/client-function-builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import compileClientFunction from '../compiler/es-next/compile-client-function';
import { APIError, ClientFunctionAPIError } from '../errors/runtime';
import { assertType, is } from '../errors/runtime/type-assertions';
import MESSAGE from '../errors/runtime/message';
import getCallsite from '../errors/get-callsite';
import { getCallsiteForMethod } from '../errors/get-callsite';
import ClientFunctionResultPromise from './result-promise';
import testRunMarker from '../test-run/marker-symbol';

Expand Down Expand Up @@ -64,7 +64,7 @@ export default class ClientFunctionBuilder {

var clientFn = function __$$clientFunction$$ () {
var testRun = builder.getBoundTestRun() || testRunTracker.resolveContextTestRun();
var callsite = getCallsite(builder.callsiteNames.execution);
var callsite = getCallsiteForMethod(builder.callsiteNames.execution);
var args = [];

// OPTIMIZATION: don't leak `arguments` object.
Expand Down
14 changes: 7 additions & 7 deletions src/client-functions/selectors/add-api.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { assign } from 'lodash';
import clientFunctionBuilderSymbol from '../builder-symbol';
import { ELEMENT_SNAPSHOT_PROPERTIES, NODE_SNAPSHOT_PROPERTIES } from './snapshot-properties';
import { CantObtainInfoForElementSpecifiedBySelectorError } from '../../errors/test-run';
import getCallsite from '../../errors/get-callsite';
import { getCallsiteForMethod } from '../../errors/get-callsite';
import ClientFunctionBuilder from '../client-function-builder';
import ClientFunctionResultPromise from '../result-promise';
import { assertType, is } from '../../errors/runtime/type-assertions';
Expand Down Expand Up @@ -106,7 +106,7 @@ function addSnapshotProperties (obj, getSelector, properties) {
properties.forEach(prop => {
Object.defineProperty(obj, prop, {
get: () => {
var callsite = getCallsite('get');
var callsite = getCallsiteForMethod('get');

return ClientFunctionResultPromise.fromFn(async () => {
var snapshot = await getSnapshot(getSelector, callsite);
Expand Down Expand Up @@ -149,7 +149,7 @@ function addSnapshotPropertyShorthands (obj, getSelector, customDOMProperties, c
addCustomMethods(obj, getSelector, customMethods);

obj.getStyleProperty = prop => {
var callsite = getCallsite('getStyleProperty');
var callsite = getCallsiteForMethod('getStyleProperty');

return ClientFunctionResultPromise.fromFn(async () => {
var snapshot = await getSnapshot(getSelector, callsite);
Expand All @@ -159,7 +159,7 @@ function addSnapshotPropertyShorthands (obj, getSelector, customDOMProperties, c
};

obj.getAttribute = attrName => {
var callsite = getCallsite('getAttribute');
var callsite = getCallsiteForMethod('getAttribute');

return ClientFunctionResultPromise.fromFn(async () => {
var snapshot = await getSnapshot(getSelector, callsite);
Expand All @@ -169,7 +169,7 @@ function addSnapshotPropertyShorthands (obj, getSelector, customDOMProperties, c
};

obj.getBoundingClientRectProperty = prop => {
var callsite = getCallsite('getBoundingClientRectProperty');
var callsite = getCallsiteForMethod('getBoundingClientRectProperty');

return ClientFunctionResultPromise.fromFn(async () => {
var snapshot = await getSnapshot(getSelector, callsite);
Expand All @@ -179,7 +179,7 @@ function addSnapshotPropertyShorthands (obj, getSelector, customDOMProperties, c
};

obj.hasClass = name => {
var callsite = getCallsite('hasClass');
var callsite = getCallsiteForMethod('hasClass');

return ClientFunctionResultPromise.fromFn(async () => {
var snapshot = await getSnapshot(getSelector, callsite);
Expand All @@ -192,7 +192,7 @@ function addSnapshotPropertyShorthands (obj, getSelector, customDOMProperties, c
function createCounter (getSelector, SelectorBuilder) {
var builder = new SelectorBuilder(getSelector(), { counterMode: true }, { instantiation: 'Selector' });
var counter = builder.getFunction();
var callsite = getCallsite('get');
var callsite = getCallsiteForMethod('get');

return async () => {
try {
Expand Down
9 changes: 4 additions & 5 deletions src/compiler/es-next/compile-client-function.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ var babelArtifactPolyfills = {
'typeof': {
re: new RegExp(escapeRe(
'var _typeof = typeof Symbol === "function" && typeof Symbol.iterator === "symbol" ? ' +
'function (obj) {return typeof obj;} : ' +
'function (obj) {return obj && typeof Symbol === "function" && obj.constructor === Symbol ' +
'&& obj !== Symbol.prototype ? "symbol" : typeof obj;};'
'function (obj) { return typeof obj; } : ' +
'function (obj) { return obj && typeof Symbol === "function" && obj.constructor === Symbol ' +
'&& obj !== Symbol.prototype ? "symbol" : typeof obj; };'
), 'g'),

getCode: () => 'var _typeof = function(obj) { return typeof obj; };',
Expand All @@ -58,7 +58,6 @@ function getBabelOptions () {
return {
presets: [presetFallback],
sourceMaps: false,
retainLines: true,
ast: false,
babelrc: false,
highlightCode: false
Expand Down Expand Up @@ -94,7 +93,7 @@ function addBabelArtifactsPolyfills (fnCode, dependenciesDefinition) {
return polyfillsCode;
}, '');

return `(function(){${dependenciesDefinition}${polyfills} return ${modifiedFnCode}})();`;
return `(function(){${dependenciesDefinition}${polyfills} return ${modifiedFnCode.trim()}})();`;
}

function getDependenciesDefinition (dependencies) {
Expand Down
1 change: 0 additions & 1 deletion src/compiler/es-next/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ export default class ESNextCompiler {
],
filename: filename,
sourceMaps: true,
retainLines: true,
ast: false,
babelrc: false,
highlightCode: false,
Expand Down
16 changes: 14 additions & 2 deletions src/errors/get-callsite.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,31 @@
import createCallsiteRecord from 'callsite-record';
import stackCleaningHook from './stack-cleaning-hook';
import { wrapCallSite } from 'source-map-support';

const STACK_TRACE_LIMIT = 2000;

export default function getCallsite (methodName, typeName) {
function getCallsite (options) {
var originalStackCleaningEnabled = stackCleaningHook.enabled;
var originalStackTraceLimit = Error.stackTraceLimit;

stackCleaningHook.enabled = false;
Error.stackTraceLimit = STACK_TRACE_LIMIT;

var callsiteRecord = createCallsiteRecord(methodName, typeName);
var callsiteRecord = createCallsiteRecord(options);

Error.stackTraceLimit = originalStackTraceLimit;
stackCleaningHook.enabled = originalStackCleaningEnabled;

return callsiteRecord;
}

export function getCallsiteForMethod (methodName, typeName) {
return getCallsite({ byFunctionName: methodName, typeName, processFrameFn: wrapCallSite });
}

export function getCallsiteForError (error, isCallsiteFrame) {
// NOTE: "source-map-support" process this kind of error automatically, cause
// in this case there is an appeal to "err.stack" inside "callsite-record" which
// provokes wrapping of frames, so there is no need to specify "processFrameFn".
return getCallsite({ forError: error, isCallsiteFrame });
}
4 changes: 2 additions & 2 deletions src/errors/process-test-fn-error.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { sep } from 'path';
import getCallsite from './get-callsite';
import { getCallsiteForError } from './get-callsite';
import { APIError } from './runtime';

import {
Expand Down Expand Up @@ -34,7 +34,7 @@ export default function processTestFnError (err) {

// NOTE: assertion libraries can add their source files to the error stack frames.
// We should skip them to create a correct callsite for the assertion error.
var callsite = isAssertionError ? getCallsite(err, isAssertionErrorCallsiteFrame) : getCallsite(err);
var callsite = isAssertionError ? getCallsiteForError(err, isAssertionErrorCallsiteFrame) : getCallsiteForError(err);

return isAssertionError ?
new ExternalAssertionLibraryError(err, callsite) :
Expand Down
4 changes: 2 additions & 2 deletions src/errors/runtime/index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { renderers } from 'callsite-record';
import MESSAGE from './message';
import createStackFilter from '../create-stack-filter';
import getCallsite from '../get-callsite';
import { getCallsiteForMethod } from '../get-callsite';
import renderTemplate from '../../utils/render-template';

// Errors
Expand Down Expand Up @@ -34,7 +34,7 @@ export class APIError extends Error {

// NOTE: `rawMessage` is used in error substitution if it occurs in test run.
this.rawMessage = rawMessage;
this.callsite = getCallsite(methodName);
this.callsite = getCallsiteForMethod(methodName);
this.constructor = APIError;

// HACK: prototype properties don't work with built-in subclasses
Expand Down
24 changes: 24 additions & 0 deletions test/server/compiler-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -452,5 +452,29 @@ describe('Compiler', function () {
expect(stack[2].source).to.have.string('testfile.js');
});
});

it('Incorrect callsite stack for failed assertion in a method of some class (GH-1267)', function () {
return compile('test/server/data/test-suites/regression-gh-1267/testfile.js')
.then(function (compiled) {
return compiled.tests[0].fn(testRunMock);
})
.then(function () {
throw 'Promise rejection expected';
})
.catch(function (err) {
var callsite = err.callsite.renderSync({ renderer: renderers.noColor });

expect(callsite).to.contains(
' 13 |}\n' +
' 14 |\n' +
" 15 |test('test', async t => {\n" +
' 16 | const page = new Page();\n' +
' 17 |\n' +
' > 18 | await page.expect(t);\n' +
' 19 |});\n' +
' 20 |\n'
);
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Browser: Chrome 15.0.874 / Mac OS X 10.8.1
Screenshot: /unix/path/with/<tag>

18 |function func1 () {
19 | record = createCallsiteRecord('func1');
19 | record = createCallsiteRecord({ byFunctionName: 'func1' });
20 |}
21 |
22 |(function func2 () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Browser: Chrome 15.0.874 / Mac OS X 10.8.1
Screenshot: /unix/path/with/<tag>

18 |function func1 () {
19 | record = createCallsiteRecord('func1');
19 | record = createCallsiteRecord({ byFunctionName: 'func1' });
20 |}
21 |
22 |(function func2 () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Browser: Chrome 15.0.874 / Mac OS X 10.8.1
Screenshot: /unix/path/with/<tag>

18 |function func1 () {
19 | record = createCallsiteRecord('func1');
19 | record = createCallsiteRecord({ byFunctionName: 'func1' });
20 |}
21 |
22 |(function func2 () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Browser: Chrome 15.0.874 / Mac OS X 10.8.1
Screenshot: /unix/path/with/<tag>

18 |function func1 () {
19 | record = createCallsiteRecord('func1');
19 | record = createCallsiteRecord({ byFunctionName: 'func1' });
20 |}
21 |
22 |(function func2 () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Browser: Chrome 15.0.874 / Mac OS X 10.8.1
Screenshot: /unix/path/with/<tag>

18 |function func1 () {
19 | record = createCallsiteRecord('func1');
19 | record = createCallsiteRecord({ byFunctionName: 'func1' });
20 |}
21 |
22 |(function func2 () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Browser: Chrome 15.0.874 / Mac OS X 10.8.1
Screenshot: /unix/path/with/<tag>

18 |function func1 () {
19 | record = createCallsiteRecord('func1');
19 | record = createCallsiteRecord({ byFunctionName: 'func1' });
20 |}
21 |
22 |(function func2 () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Browser: Chrome 15.0.874 / Mac OS X 10.8.1
Screenshot: /unix/path/with/<tag>

18 |function func1 () {
19 | record = createCallsiteRecord('func1');
19 | record = createCallsiteRecord({ byFunctionName: 'func1' });
20 |}
21 |
22 |(function func2 () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Browser: Chrome 15.0.874 / Mac OS X 10.8.1
Screenshot: /unix/path/with/<tag>

18 |function func1 () {
19 | record = createCallsiteRecord('func1');
19 | record = createCallsiteRecord({ byFunctionName: 'func1' });
20 |}
21 |
22 |(function func2 () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Browser: Chrome 15.0.874 / Mac OS X 10.8.1
Screenshot: /unix/path/with/<tag>

18 |function func1 () {
19 | record = createCallsiteRecord('func1');
19 | record = createCallsiteRecord({ byFunctionName: 'func1' });
20 |}
21 |
22 |(function func2 () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Browser: Chrome 15.0.874 / Mac OS X 10.8.1
Screenshot: /unix/path/with/<tag>

18 |function func1 () {
19 | record = createCallsiteRecord('func1');
19 | record = createCallsiteRecord({ byFunctionName: 'func1' });
20 |}
21 |
22 |(function func2 () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Browser: Chrome 15.0.874 / Mac OS X 10.8.1
Screenshot: /unix/path/with/<tag>

18 |function func1 () {
19 | record = createCallsiteRecord('func1');
19 | record = createCallsiteRecord({ byFunctionName: 'func1' });
20 |}
21 |
22 |(function func2 () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Browser: Chrome 15.0.874 / Mac OS X 10.8.1
Screenshot: /unix/path/with/<tag>

18 |function func1 () {
19 | record = createCallsiteRecord('func1');
19 | record = createCallsiteRecord({ byFunctionName: 'func1' });
20 |}
21 |
22 |(function func2 () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Browser: Chrome 15.0.874 / Mac OS X 10.8.1
Screenshot: /unix/path/with/<tag>

18 |function func1 () {
19 | record = createCallsiteRecord('func1');
19 | record = createCallsiteRecord({ byFunctionName: 'func1' });
20 |}
21 |
22 |(function func2 () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Browser: Chrome 15.0.874 / Mac OS X 10.8.1
Screenshot: /unix/path/with/<tag>

18 |function func1 () {
19 | record = createCallsiteRecord('func1');
19 | record = createCallsiteRecord({ byFunctionName: 'func1' });
20 |}
21 |
22 |(function func2 () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Browser: Chrome 15.0.874 / Mac OS X 10.8.1
Screenshot: /unix/path/with/<tag>

18 |function func1 () {
19 | record = createCallsiteRecord('func1');
19 | record = createCallsiteRecord({ byFunctionName: 'func1' });
20 |}
21 |
22 |(function func2 () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Browser: Chrome 15.0.874 / Mac OS X 10.8.1
Screenshot: /unix/path/with/<tag>

18 |function func1 () {
19 | record = createCallsiteRecord('func1');
19 | record = createCallsiteRecord({ byFunctionName: 'func1' });
20 |}
21 |
22 |(function func2 () {
Expand Down
Loading

0 comments on commit 8e449c8

Please sign in to comment.