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.

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

Get rid of "retainLines" compiler option (closes #1267) #1369

Merged
merged 4 commits into from
Apr 3, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 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