From 5a1e6764d6980f499aaa91e3396b47f38b084aa5 Mon Sep 17 00:00:00 2001 From: Grant Snodgrass Date: Sat, 10 Dec 2016 12:42:12 -0500 Subject: [PATCH 1/8] refactor(proxify): move feature detection to util Currently, only one module needs to detect if Chai's proxy protection is enabled. However, upcoming changes will involve performing this detection in other modules as well. This commit moves the detection logic to its own utility module for easy reuse. --- lib/chai/utils/index.js | 8 +++- lib/chai/utils/isProxyEnabled.js | 24 ++++++++++ lib/chai/utils/proxify.js | 4 +- test/utilities.js | 79 ++++++++++++++++++++++++++++++++ 4 files changed, 112 insertions(+), 3 deletions(-) create mode 100644 lib/chai/utils/isProxyEnabled.js diff --git a/lib/chai/utils/index.js b/lib/chai/utils/index.js index b17f085e4..d4f329c75 100644 --- a/lib/chai/utils/index.js +++ b/lib/chai/utils/index.js @@ -154,11 +154,17 @@ exports.checkError = require('check-error'); exports.proxify = require('./proxify'); /*! - * Proxify util + * addLengthGuard util */ exports.addLengthGuard = require('./addLengthGuard'); +/*! + * isProxyEnabled helper + */ + +exports.isProxyEnabled = require('./isProxyEnabled'); + /*! * isNaN method */ diff --git a/lib/chai/utils/isProxyEnabled.js b/lib/chai/utils/isProxyEnabled.js new file mode 100644 index 000000000..5bea49f84 --- /dev/null +++ b/lib/chai/utils/isProxyEnabled.js @@ -0,0 +1,24 @@ +var config = require('../config'); + +/*! + * Chai - isProxyEnabled helper + * Copyright(c) 2012-2014 Jake Luer + * MIT Licensed + */ + +/** + * # isProxyEnabled() + * + * Helper function to check if Chai's proxy protection feature is enabled. If + * proxies are unsupported or disabled via the user's Chai config, then return + * false. Otherwise, return true. + * + * @namespace Utils + * @name isProxyEnabled + */ + +module.exports = function isProxyEnabled () { + return config.useProxy && + typeof Proxy !== 'undefined' && + typeof Reflect !== 'undefined'; +}; diff --git a/lib/chai/utils/proxify.js b/lib/chai/utils/proxify.js index a1e41c127..f01bf622c 100644 --- a/lib/chai/utils/proxify.js +++ b/lib/chai/utils/proxify.js @@ -1,5 +1,6 @@ var config = require('../config'); var getProperties = require('./getProperties'); +var isProxyEnabled = require('./isProxyEnabled'); /*! * Chai - proxify utility @@ -27,8 +28,7 @@ var getProperties = require('./getProperties'); */ module.exports = function proxify (obj, nonChainableMethodName) { - if (!config.useProxy || typeof Proxy === 'undefined' || typeof Reflect === 'undefined') - return obj; + if (!isProxyEnabled()) return obj; return new Proxy(obj, { get: function getProperty (target, property) { diff --git a/test/utilities.js b/test/utilities.js index a1098f3d9..98a30d3ed 100644 --- a/test/utilities.js +++ b/test/utilities.js @@ -1118,4 +1118,83 @@ describe('utilities', function () { }).to.throw('Invalid Chai property: hoagie.length. Due to a compatibility issue, "length" cannot directly follow "hoagie". Use "hoagie.lengthOf" instead.'); }); }); + + describe("isProxyEnabled", function () { + if (typeof Proxy === 'undefined' || typeof Reflect === 'undefined') return; + + var origProxy, origReflect, origUseProxy, isProxyEnabled; + + before(function () { + chai.use(function (_chai, _) { + isProxyEnabled = _.isProxyEnabled; + }); + + origProxy = Proxy; + origReflect = Reflect; + origUseProxy = chai.config.useProxy; + }); + + beforeEach(function () { + Proxy = origProxy; + Reflect = origReflect; + chai.config.useProxy = true; + }); + + after(function () { + Proxy = origProxy; + Reflect = origReflect; + chai.config.useProxy = origUseProxy; + }); + + it("returns true if Proxy is defined, Reflect is defined, and useProxy is true", function () { + expect(isProxyEnabled()).to.be.true; + }); + + it("returns false if Proxy is defined, Reflect is defined, and useProxy is false", function () { + chai.config.useProxy = false; + + expect(isProxyEnabled()).to.be.false; + }); + + it("returns false if Proxy is defined, Reflect is undefined, and useProxy is true", function () { + Reflect = undefined; + + expect(isProxyEnabled()).to.be.false; + }); + + it("returns false if Proxy is defined, Reflect is undefined, and useProxy is false", function () { + Reflect = undefined; + chai.config.useProxy = false; + + expect(isProxyEnabled()).to.be.false; + }); + + it("returns false if Proxy is undefined, Reflect is defined, and useProxy is true", function () { + Proxy = undefined; + + expect(isProxyEnabled()).to.be.false; + }); + + it("returns false if Proxy is undefined, Reflect is defined, and useProxy is false", function () { + Proxy = undefined; + chai.config.useProxy = false; + + expect(isProxyEnabled()).to.be.false; + }); + + it("returns false if Proxy is undefined, Reflect is undefined, and useProxy is true", function () { + Proxy = undefined; + Reflect = undefined; + + expect(isProxyEnabled()).to.be.false; + }); + + it("returns false if Proxy is undefined, Reflect is undefined, and useProxy is false", function () { + Proxy = undefined; + Reflect = undefined; + chai.config.useProxy = false; + + expect(isProxyEnabled()).to.be.false; + }); + }); }); From 78f1808735606068ba68aad9370bb97009d281a9 Mon Sep 17 00:00:00 2001 From: Grant Snodgrass Date: Sat, 10 Dec 2016 15:48:31 -0500 Subject: [PATCH 2/8] refactor(proxify): use Reflect for property access The proper way to perform an operation's original behavior from within a proxy trap is by using `Reflect`. --- lib/chai/utils/proxify.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/chai/utils/proxify.js b/lib/chai/utils/proxify.js index f01bf622c..f32974e2f 100644 --- a/lib/chai/utils/proxify.js +++ b/lib/chai/utils/proxify.js @@ -64,7 +64,7 @@ module.exports = function proxify (obj, nonChainableMethodName) { } } - return target[property]; + return Reflect.get(target, property); } }); }; From 96f958c8144e7742e758ca2820e531b784eaa6a2 Mon Sep 17 00:00:00 2001 From: Grant Snodgrass Date: Sat, 10 Dec 2016 16:14:38 -0500 Subject: [PATCH 3/8] refactor(utils): improve function names Many of the utility functions had slightly misleading names or no names at all. This commit renames the functions with misleading names and adds names to functions that were missing one. --- lib/chai/utils/addChainableMethod.js | 18 +++++++++--------- lib/chai/utils/addMethod.js | 10 +++++----- lib/chai/utils/addProperty.js | 6 +++--- lib/chai/utils/compareByInspect.js | 2 +- lib/chai/utils/expectTypes.js | 2 +- lib/chai/utils/flag.js | 2 +- lib/chai/utils/getActual.js | 2 +- lib/chai/utils/getMessage.js | 2 +- lib/chai/utils/isProxyEnabled.js | 2 +- lib/chai/utils/objDisplay.js | 2 +- lib/chai/utils/overwriteChainableMethod.js | 6 +++--- lib/chai/utils/overwriteMethod.js | 10 +++++----- lib/chai/utils/overwriteProperty.js | 6 +++--- lib/chai/utils/proxify.js | 4 ++-- lib/chai/utils/test.js | 2 +- lib/chai/utils/transferFlags.js | 2 +- 16 files changed, 39 insertions(+), 39 deletions(-) diff --git a/lib/chai/utils/addChainableMethod.js b/lib/chai/utils/addChainableMethod.js index 1b20333dd..46fa374e8 100644 --- a/lib/chai/utils/addChainableMethod.js +++ b/lib/chai/utils/addChainableMethod.js @@ -59,7 +59,7 @@ var call = Function.prototype.call, * @api public */ -module.exports = function (ctx, name, method, chainingBehavior) { +module.exports = function addChainableMethod(ctx, name, method, chainingBehavior) { if (typeof chainingBehavior !== 'function') { chainingBehavior = function () { }; } @@ -76,13 +76,13 @@ module.exports = function (ctx, name, method, chainingBehavior) { ctx.__methods[name] = chainableBehavior; Object.defineProperty(ctx, name, - { get: function () { + { get: function chainableMethodGetter() { chainableBehavior.chainingBehavior.call(this); - var assert = function assert() { + var chainableMethodWrapper = function () { var old_ssfi = flag(this, 'ssfi'); if (old_ssfi) - flag(this, 'ssfi', assert); + flag(this, 'ssfi', chainableMethodWrapper); var result = chainableBehavior.method.apply(this, arguments); if (result !== undefined) { @@ -94,12 +94,12 @@ module.exports = function (ctx, name, method, chainingBehavior) { return newAssertion; }; - addLengthGuard(assert, name, true); + addLengthGuard(chainableMethodWrapper, name, true); // Use `__proto__` if available if (hasProtoSupport) { // Inherit all properties from the object by replacing the `Function` prototype - var prototype = assert.__proto__ = Object.create(this); + var prototype = chainableMethodWrapper.__proto__ = Object.create(this); // Restore the `call` and `apply` methods from `Function` prototype.call = call; prototype.apply = apply; @@ -110,13 +110,13 @@ module.exports = function (ctx, name, method, chainingBehavior) { asserterNames.forEach(function (asserterName) { if (!excludeNames.test(asserterName)) { var pd = Object.getOwnPropertyDescriptor(ctx, asserterName); - Object.defineProperty(assert, asserterName, pd); + Object.defineProperty(chainableMethodWrapper, asserterName, pd); } }); } - transferFlags(this, assert); - return proxify(assert); + transferFlags(this, chainableMethodWrapper); + return proxify(chainableMethodWrapper); } , configurable: true }); diff --git a/lib/chai/utils/addMethod.js b/lib/chai/utils/addMethod.js index 6f6bf75fd..bc0d8d9f2 100644 --- a/lib/chai/utils/addMethod.js +++ b/lib/chai/utils/addMethod.js @@ -36,12 +36,12 @@ var transferFlags = require('./transferFlags'); * @api public */ -module.exports = function (ctx, name, method) { - var fn = function () { +module.exports = function addMethod(ctx, name, method) { + var methodWrapper = function () { var keep_ssfi = flag(this, 'keep_ssfi'); var old_ssfi = flag(this, 'ssfi'); if (!keep_ssfi && old_ssfi) - flag(this, 'ssfi', fn); + flag(this, 'ssfi', methodWrapper); var result = method.apply(this, arguments); if (result !== undefined) @@ -52,6 +52,6 @@ module.exports = function (ctx, name, method) { return newAssertion; }; - addLengthGuard(fn, name, false); - ctx[name] = proxify(fn, name); + addLengthGuard(methodWrapper, name, false); + ctx[name] = proxify(methodWrapper, name); }; diff --git a/lib/chai/utils/addProperty.js b/lib/chai/utils/addProperty.js index 48e1b2dad..1358ad93c 100644 --- a/lib/chai/utils/addProperty.js +++ b/lib/chai/utils/addProperty.js @@ -34,15 +34,15 @@ var transferFlags = require('./transferFlags'); * @api public */ -module.exports = function (ctx, name, getter) { +module.exports = function addProperty(ctx, name, getter) { getter = getter === undefined ? new Function() : getter; Object.defineProperty(ctx, name, - { get: function addProperty() { + { get: function propertyGetter() { var keep_ssfi = flag(this, 'keep_ssfi'); var old_ssfi = flag(this, 'ssfi'); if (!keep_ssfi && old_ssfi) - flag(this, 'ssfi', addProperty); + flag(this, 'ssfi', propertyGetter); var result = getter.call(this); if (result !== undefined) diff --git a/lib/chai/utils/compareByInspect.js b/lib/chai/utils/compareByInspect.js index 071de9ca8..708ff28c0 100644 --- a/lib/chai/utils/compareByInspect.js +++ b/lib/chai/utils/compareByInspect.js @@ -26,6 +26,6 @@ var inspect = require('./inspect'); * @api public */ -module.exports = function (a, b) { +module.exports = function compareByInspect(a, b) { return inspect(a) < inspect(b) ? -1 : 1; }; diff --git a/lib/chai/utils/expectTypes.js b/lib/chai/utils/expectTypes.js index 8fa7f1fd0..4b0bd4564 100644 --- a/lib/chai/utils/expectTypes.js +++ b/lib/chai/utils/expectTypes.js @@ -22,7 +22,7 @@ var AssertionError = require('assertion-error'); var flag = require('./flag'); var type = require('type-detect'); -module.exports = function (obj, types) { +module.exports = function expectTypes(obj, types) { obj = flag(obj, 'object'); types = types.map(function (t) { return t.toLowerCase(); }); types.sort(); diff --git a/lib/chai/utils/flag.js b/lib/chai/utils/flag.js index ef13236e0..ffb48f068 100644 --- a/lib/chai/utils/flag.js +++ b/lib/chai/utils/flag.js @@ -23,7 +23,7 @@ * @api private */ -module.exports = function (obj, key, value) { +module.exports = function flag(obj, key, value) { var flags = obj.__flags || (obj.__flags = Object.create(null)); if (arguments.length === 3) { flags[key] = value; diff --git a/lib/chai/utils/getActual.js b/lib/chai/utils/getActual.js index e2f28faa8..2b5b3fa5f 100644 --- a/lib/chai/utils/getActual.js +++ b/lib/chai/utils/getActual.js @@ -15,6 +15,6 @@ * @name getActual */ -module.exports = function (obj, args) { +module.exports = function getActual(obj, args) { return args.length > 4 ? args[4] : obj._obj; }; diff --git a/lib/chai/utils/getMessage.js b/lib/chai/utils/getMessage.js index dedd8f9d8..1c9744feb 100644 --- a/lib/chai/utils/getMessage.js +++ b/lib/chai/utils/getMessage.js @@ -32,7 +32,7 @@ var flag = require('./flag') * @api public */ -module.exports = function (obj, args) { +module.exports = function getMessage(obj, args) { var negate = flag(obj, 'negate') , val = flag(obj, 'object') , expected = args[3] diff --git a/lib/chai/utils/isProxyEnabled.js b/lib/chai/utils/isProxyEnabled.js index 5bea49f84..f0ef041db 100644 --- a/lib/chai/utils/isProxyEnabled.js +++ b/lib/chai/utils/isProxyEnabled.js @@ -17,7 +17,7 @@ var config = require('../config'); * @name isProxyEnabled */ -module.exports = function isProxyEnabled () { +module.exports = function isProxyEnabled() { return config.useProxy && typeof Proxy !== 'undefined' && typeof Reflect !== 'undefined'; diff --git a/lib/chai/utils/objDisplay.js b/lib/chai/utils/objDisplay.js index 68142581a..d87d938e1 100644 --- a/lib/chai/utils/objDisplay.js +++ b/lib/chai/utils/objDisplay.js @@ -24,7 +24,7 @@ var config = require('../config'); * @api public */ -module.exports = function (obj) { +module.exports = function objDisplay(obj) { var str = inspect(obj) , type = Object.prototype.toString.call(obj); diff --git a/lib/chai/utils/overwriteChainableMethod.js b/lib/chai/utils/overwriteChainableMethod.js index 007ed125a..4d28f5aa7 100644 --- a/lib/chai/utils/overwriteChainableMethod.js +++ b/lib/chai/utils/overwriteChainableMethod.js @@ -40,11 +40,11 @@ var transferFlags = require('./transferFlags'); * @api public */ -module.exports = function (ctx, name, method, chainingBehavior) { +module.exports = function overwriteChainableMethod(ctx, name, method, chainingBehavior) { var chainableBehavior = ctx.__methods[name]; var _chainingBehavior = chainableBehavior.chainingBehavior; - chainableBehavior.chainingBehavior = function () { + chainableBehavior.chainingBehavior = function overwritingChainableMethodGetter() { var result = chainingBehavior(_chainingBehavior).call(this); if (result !== undefined) { return result; @@ -56,7 +56,7 @@ module.exports = function (ctx, name, method, chainingBehavior) { }; var _method = chainableBehavior.method; - chainableBehavior.method = function () { + chainableBehavior.method = function overwritingChainableMethodWrapper() { var result = method(_method).apply(this, arguments); if (result !== undefined) { return result; diff --git a/lib/chai/utils/overwriteMethod.js b/lib/chai/utils/overwriteMethod.js index 66f55a70f..3342c4299 100644 --- a/lib/chai/utils/overwriteMethod.js +++ b/lib/chai/utils/overwriteMethod.js @@ -44,7 +44,7 @@ var transferFlags = require('./transferFlags'); * @api public */ -module.exports = function (ctx, name, method) { +module.exports = function overwriteMethod(ctx, name, method) { var _method = ctx[name] , _super = function () { throw new Error(name + ' is not a function'); @@ -53,11 +53,11 @@ module.exports = function (ctx, name, method) { if (_method && 'function' === typeof _method) _super = _method; - var fn = function () { + var overwritingMethodWrapper = function () { var keep_ssfi = flag(this, 'keep_ssfi'); var old_ssfi = flag(this, 'ssfi'); if (!keep_ssfi && old_ssfi) - flag(this, 'ssfi', fn); + flag(this, 'ssfi', overwritingMethodWrapper); flag(this, 'keep_ssfi', true); var result = method(_super).apply(this, arguments); @@ -72,6 +72,6 @@ module.exports = function (ctx, name, method) { return newAssertion; } - addLengthGuard(fn, name, false); - ctx[name] = proxify(fn, name); + addLengthGuard(overwritingMethodWrapper, name, false); + ctx[name] = proxify(overwritingMethodWrapper, name); }; diff --git a/lib/chai/utils/overwriteProperty.js b/lib/chai/utils/overwriteProperty.js index 155ccbdab..1f8a28b30 100644 --- a/lib/chai/utils/overwriteProperty.js +++ b/lib/chai/utils/overwriteProperty.js @@ -42,7 +42,7 @@ var transferFlags = require('./transferFlags'); * @api public */ -module.exports = function (ctx, name, getter) { +module.exports = function overwriteProperty(ctx, name, getter) { var _get = Object.getOwnPropertyDescriptor(ctx, name) , _super = function () {}; @@ -50,11 +50,11 @@ module.exports = function (ctx, name, getter) { _super = _get.get Object.defineProperty(ctx, name, - { get: function overwriteProperty() { + { get: function overwritingPropertyGetter() { var keep_ssfi = flag(this, 'keep_ssfi'); var old_ssfi = flag(this, 'ssfi'); if (!keep_ssfi && old_ssfi) - flag(this, 'ssfi', overwriteProperty); + flag(this, 'ssfi', overwritingPropertyGetter); flag(this, 'keep_ssfi', true); var result = getter(_super).call(this); diff --git a/lib/chai/utils/proxify.js b/lib/chai/utils/proxify.js index f32974e2f..ed73e6694 100644 --- a/lib/chai/utils/proxify.js +++ b/lib/chai/utils/proxify.js @@ -27,11 +27,11 @@ var isProxyEnabled = require('./isProxyEnabled'); * @name proxify */ -module.exports = function proxify (obj, nonChainableMethodName) { +module.exports = function proxify(obj, nonChainableMethodName) { if (!isProxyEnabled()) return obj; return new Proxy(obj, { - get: function getProperty (target, property) { + get: function proxyGetter(target, property) { // This check is here because we should not throw errors on Symbol properties // such as `Symbol.toStringTag`. // The values for which an error should be thrown can be configured using diff --git a/lib/chai/utils/test.js b/lib/chai/utils/test.js index 453d4c72f..4afc5fc5c 100644 --- a/lib/chai/utils/test.js +++ b/lib/chai/utils/test.js @@ -21,7 +21,7 @@ var flag = require('./flag'); * @name test */ -module.exports = function (obj, args) { +module.exports = function test(obj, args) { var negate = flag(obj, 'negate') , expr = args[0]; return negate ? !expr : expr; diff --git a/lib/chai/utils/transferFlags.js b/lib/chai/utils/transferFlags.js index fa2c379d7..ea1316b54 100644 --- a/lib/chai/utils/transferFlags.js +++ b/lib/chai/utils/transferFlags.js @@ -27,7 +27,7 @@ * @api private */ -module.exports = function (assertion, object, includeAll) { +module.exports = function transferFlags(assertion, object, includeAll) { var flags = assertion.__flags || (assertion.__flags = Object.create(null)); if (!object.__flags) { From 8fa4f7857456053e6cbaae511947699257161162 Mon Sep 17 00:00:00 2001 From: Grant Snodgrass Date: Sat, 10 Dec 2016 23:22:13 -0500 Subject: [PATCH 4/8] test(includeStack): improve tests Only a couple of types of assertions were being tested for correct stack traces. This commit cleans up the existing tests and adds tests for the missing assertion types. --- test/configuration.js | 382 +++++++++++++++++++++++++++++++++++------- 1 file changed, 320 insertions(+), 62 deletions(-) diff --git a/test/configuration.js b/test/configuration.js index d0399c4a4..f83d36608 100644 --- a/test/configuration.js +++ b/test/configuration.js @@ -19,78 +19,336 @@ describe('configuration', function () { }); }); - function fooThrows () { - chai.expect('foo').to.be.equal('bar'); - } - function fooPropThrows () { - chai.expect('foo').to.not.exist; - } - describe('includeStack', function() { - it('is true for method assertions', function () { - chai.config.includeStack = true; - - try { - fooThrows(); - assert.ok(false, 'should not get here because error thrown'); - } catch (err) { - // not all browsers support err.stack - if ('undefined' !== typeof err.stack) { - assert.include(err.stack, 'assertEqual', 'should have internal stack trace in error message'); - assert.include(err.stack, 'fooThrows', 'should have user stack trace in error message'); - } - } + // Skip tests if `Error.captureStackTrace` is unsupported + if (typeof Error.captureStackTrace === 'undefined') return; + + try { + throw Error(); + } catch (err) { + // Skip tests if `err.stack` is unsupported + if (typeof err.stack === 'undefined') return; + } - }); + // Create overwritten assertions that always fail + before(function () { + chai.util.addProperty(chai.Assertion.prototype, 'tmpProperty', function () {}); + chai.util.overwriteProperty(chai.Assertion.prototype, 'tmpProperty', function () { + return function () { + this.assert(false); + }; + }); - it('is false for method assertions', function () { - chai.config.includeStack = false; + chai.util.addMethod(chai.Assertion.prototype, 'tmpMethod', function () {}); + chai.util.overwriteMethod(chai.Assertion.prototype, 'tmpMethod', function () { + return function () { + this.assert(false); + }; + }); - try { - fooThrows(); - assert.ok(false, 'should not get here because error thrown'); - } catch (err) { - // IE 10 supports err.stack in Chrome format, but without - // `Error.captureStackTrace` support that allows tuning of the error - // message. - if ('undefined' !== typeof err.stack && 'undefined' !== typeof Error.captureStackTrace) { - assert.notInclude(err.stack, 'assertEqual', 'should not have internal stack trace in error message'); - assert.include(err.stack, 'fooThrows', 'should have user stack trace in error message'); - } - } + chai.util.addChainableMethod(chai.Assertion.prototype, 'tmpChainableMethod', function () {}, function () {}); + chai.util.overwriteChainableMethod(chai.Assertion.prototype, 'tmpChainableMethod', function (_super) { + return function () { + this.assert(false); + }; + }, function () { + return function () {}; + }); }); - it('is true for property assertions', function () { - chai.config.includeStack = true; + // Delete overwritten assertions + after(function () { + delete chai.Assertion.prototype.tmpProperty; + delete chai.Assertion.prototype.tmpMethod; + delete chai.Assertion.prototype.tmpChainableMethod; + }); - try { - fooPropThrows(); - assert.ok(false, 'should not get here because error thrown'); - } catch (err) { - // not all browsers support err.stack - // Phantom does not include function names for getter exec - if ('undefined' !== typeof err.stack && 'undefined' !== typeof Error.captureStackTrace) { - assert.include(err.stack, 'addProperty', 'should have internal stack trace in error message'); - assert.include(err.stack, 'fooPropThrows', 'should have user stack trace in error message'); - } - } + // Functions that always throw an error + function badPropertyAssertion() { + expect(42).to.be.false; + } + function badOverwrittenPropertyAssertion() { + expect(42).tmpProperty; + } + function badMethodAssertion() { + expect(42).to.equal(false); + } + function badOverwrittenMethodAssertion() { + expect(42).tmpMethod(); + } + function badChainableMethodAssertion() { + expect(42).to.be.a('string'); + } + function badOverwrittenChainableMethodAssertion() { + expect(42).tmpChainableMethod(); + } + + describe('when true', function () { + describe('failed property assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = true; + + try { + badPropertyAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.contain('propertyGetter'); + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badPropertyAssertion'); + }); + }); + + describe('failed overwritten property assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = true; + + try { + badOverwrittenPropertyAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.contain('overwritingPropertyGetter'); + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badOverwrittenPropertyAssertion'); + }); + }); + + describe('failed method assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = true; + + try { + badMethodAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.contain('methodWrapper'); + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badMethodAssertion'); + }); + }); + + describe('failed overwritten method assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = true; + + try { + badOverwrittenMethodAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.contain('overwritingMethodWrapper'); + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badOverwrittenMethodAssertion'); + }); + }); + + describe('failed chainable method assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = true; + + try { + badChainableMethodAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.contain('chainableMethodWrapper'); + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badChainableMethodAssertion'); + }); + }); + + describe('failed overwritten chainable method assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = true; + + try { + badOverwrittenChainableMethodAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.contain('overwritingChainableMethodWrapper'); + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badOverwrittenChainableMethodAssertion'); + }); + }); }); - it('is false for property assertions', function () { - chai.config.includeStack = false; + describe('when false', function () { + describe('failed property assertions', function () { + var caughtErr = '__PRETEST__'; - try { - fooPropThrows(); - assert.ok(false, 'should not get here because error thrown'); - } catch (err) { - // IE 10 supports err.stack in Chrome format, but without - // `Error.captureStackTrace` support that allows tuning of the error - // message. - if ('undefined' !== typeof err.stack && 'undefined' !== typeof Error.captureStackTrace) { - assert.notInclude(err.stack, 'addProperty', 'should not have internal stack trace in error message'); - assert.include(err.stack, 'fooPropThrows', 'should have user stack trace in error message'); - } - } + before(function () { + chai.config.includeStack = false; + + try { + badPropertyAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should not include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.not.contain('propertyGetter'); + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badPropertyAssertion'); + }); + }); + + describe('failed overwritten property assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = false; + + try { + badOverwrittenPropertyAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should not include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.not.contain('overwritingPropertyGetter'); + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badOverwrittenPropertyAssertion'); + }); + }); + + describe('failed method assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = false; + + try { + badMethodAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should not include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.not.contain('methodWrapper'); + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badMethodAssertion'); + }); + }); + + describe('failed overwritten method assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = false; + + try { + badOverwrittenMethodAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should not include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.not.contain('overwritingMethodWrapper'); + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badOverwrittenMethodAssertion'); + }); + }); + + describe('failed chainable method assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = false; + + try { + badChainableMethodAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should not include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.not.contain('chainableMethodWrapper'); + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badChainableMethodAssertion'); + }); + }); + + describe('failed overwritten chainable method assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = false; + + try { + badOverwrittenChainableMethodAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should not include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.not.contain('overwritingChainableMethodWrapper'); + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badOverwrittenChainableMethodAssertion'); + }); + }); }); }); From b88e53639ec75867e85144b7eef6df5e7be677bc Mon Sep 17 00:00:00 2001 From: Grant Snodgrass Date: Sun, 11 Dec 2016 11:17:15 -0500 Subject: [PATCH 5/8] fix: remove proxy frames from stack traces Proxy-related implementation frames were showing up in the stack traces for failed property assertions. This commit removes them by setting the proxy getter (instead of the property getter) as the starting point to remove all implementation frames. --- lib/chai/utils/addProperty.js | 11 +++++++---- lib/chai/utils/overwriteProperty.js | 11 +++++++---- lib/chai/utils/proxify.js | 9 ++++++++- test/configuration.js | 16 ++++++++++++++++ 4 files changed, 38 insertions(+), 9 deletions(-) diff --git a/lib/chai/utils/addProperty.js b/lib/chai/utils/addProperty.js index 1358ad93c..6e535db5d 100644 --- a/lib/chai/utils/addProperty.js +++ b/lib/chai/utils/addProperty.js @@ -6,6 +6,7 @@ var chai = require('../../chai'); var flag = require('./flag'); +var isProxyEnabled = require('./isProxyEnabled'); var transferFlags = require('./transferFlags'); /** @@ -39,10 +40,12 @@ module.exports = function addProperty(ctx, name, getter) { Object.defineProperty(ctx, name, { get: function propertyGetter() { - var keep_ssfi = flag(this, 'keep_ssfi'); - var old_ssfi = flag(this, 'ssfi'); - if (!keep_ssfi && old_ssfi) - flag(this, 'ssfi', propertyGetter); + if (!isProxyEnabled()) { + var keep_ssfi = flag(this, 'keep_ssfi'); + var old_ssfi = flag(this, 'ssfi'); + if (!keep_ssfi && old_ssfi) + flag(this, 'ssfi', propertyGetter); + } var result = getter.call(this); if (result !== undefined) diff --git a/lib/chai/utils/overwriteProperty.js b/lib/chai/utils/overwriteProperty.js index 1f8a28b30..0ffb26834 100644 --- a/lib/chai/utils/overwriteProperty.js +++ b/lib/chai/utils/overwriteProperty.js @@ -6,6 +6,7 @@ var chai = require('../../chai'); var flag = require('./flag'); +var isProxyEnabled = require('./isProxyEnabled'); var transferFlags = require('./transferFlags'); /** @@ -51,10 +52,12 @@ module.exports = function overwriteProperty(ctx, name, getter) { Object.defineProperty(ctx, name, { get: function overwritingPropertyGetter() { - var keep_ssfi = flag(this, 'keep_ssfi'); - var old_ssfi = flag(this, 'ssfi'); - if (!keep_ssfi && old_ssfi) - flag(this, 'ssfi', overwritingPropertyGetter); + if (!isProxyEnabled()) { + var keep_ssfi = flag(this, 'keep_ssfi'); + var old_ssfi = flag(this, 'ssfi'); + if (!keep_ssfi && old_ssfi) + flag(this, 'ssfi', overwritingPropertyGetter); + } flag(this, 'keep_ssfi', true); var result = getter(_super).call(this); diff --git a/lib/chai/utils/proxify.js b/lib/chai/utils/proxify.js index ed73e6694..a9ba811d2 100644 --- a/lib/chai/utils/proxify.js +++ b/lib/chai/utils/proxify.js @@ -1,4 +1,5 @@ var config = require('../config'); +var flag = require('./flag'); var getProperties = require('./getProperties'); var isProxyEnabled = require('./isProxyEnabled'); @@ -27,6 +28,8 @@ var isProxyEnabled = require('./isProxyEnabled'); * @name proxify */ +var builtins = ['__flags', '__methods', '_obj', 'assert']; + module.exports = function proxify(obj, nonChainableMethodName) { if (!isProxyEnabled()) return obj; @@ -48,7 +51,7 @@ module.exports = function proxify(obj, nonChainableMethodName) { var orderedProperties = getProperties(target).filter(function(property) { return !Object.prototype.hasOwnProperty(property) && - ['__flags', '__methods', '_obj', 'assert'].indexOf(property) === -1; + builtins.indexOf(property) === -1; }).sort(function(a, b) { return stringDistance(property, a) - stringDistance(property, b); }); @@ -64,6 +67,10 @@ module.exports = function proxify(obj, nonChainableMethodName) { } } + if (builtins.indexOf(property) === -1) { + flag(target, 'ssfi', proxyGetter); + } + return Reflect.get(target, property); } }); diff --git a/test/configuration.js b/test/configuration.js index f83d36608..77d2053ab 100644 --- a/test/configuration.js +++ b/test/configuration.js @@ -99,6 +99,10 @@ describe('configuration', function () { it('should include Chai frames in stack trace', function () { expect(caughtErr.stack).to.contain('propertyGetter'); + + if (typeof Proxy !== 'undefined' && typeof Reflect !== 'undefined') { + expect(caughtErr.stack).to.contain('proxyGetter'); + } }); it('should include user frames in stack trace', function () { @@ -121,6 +125,10 @@ describe('configuration', function () { it('should include Chai frames in stack trace', function () { expect(caughtErr.stack).to.contain('overwritingPropertyGetter'); + + if (typeof Proxy !== 'undefined' && typeof Reflect !== 'undefined') { + expect(caughtErr.stack).to.contain('proxyGetter'); + } }); it('should include user frames in stack trace', function () { @@ -233,6 +241,10 @@ describe('configuration', function () { it('should not include Chai frames in stack trace', function () { expect(caughtErr.stack).to.not.contain('propertyGetter'); + + if (typeof Proxy !== 'undefined' && typeof Reflect !== 'undefined') { + expect(caughtErr.stack).to.not.contain('proxyGetter'); + } }); it('should include user frames in stack trace', function () { @@ -255,6 +267,10 @@ describe('configuration', function () { it('should not include Chai frames in stack trace', function () { expect(caughtErr.stack).to.not.contain('overwritingPropertyGetter'); + + if (typeof Proxy !== 'undefined' && typeof Reflect !== 'undefined') { + expect(caughtErr.stack).to.not.contain('proxyGetter'); + } }); it('should include user frames in stack trace', function () { From bf71196c7fc6a6613a98d754b6f756c1a0740a7a Mon Sep 17 00:00:00 2001 From: Grant Snodgrass Date: Sun, 11 Dec 2016 12:15:13 -0500 Subject: [PATCH 6/8] test(includeStack): add `should` interface Only the `expect` interface was being tested for correct stack traces. This commit adds identical tests for the `should` interface. --- test/configuration.js | 888 ++++++++++++++++++++++++++++-------------- 1 file changed, 598 insertions(+), 290 deletions(-) diff --git a/test/configuration.js b/test/configuration.js index 77d2053ab..d53f164fc 100644 --- a/test/configuration.js +++ b/test/configuration.js @@ -63,306 +63,614 @@ describe('configuration', function () { delete chai.Assertion.prototype.tmpChainableMethod; }); - // Functions that always throw an error - function badPropertyAssertion() { - expect(42).to.be.false; - } - function badOverwrittenPropertyAssertion() { - expect(42).tmpProperty; - } - function badMethodAssertion() { - expect(42).to.equal(false); - } - function badOverwrittenMethodAssertion() { - expect(42).tmpMethod(); - } - function badChainableMethodAssertion() { - expect(42).to.be.a('string'); - } - function badOverwrittenChainableMethodAssertion() { - expect(42).tmpChainableMethod(); - } - - describe('when true', function () { - describe('failed property assertions', function () { - var caughtErr = '__PRETEST__'; - - before(function () { - chai.config.includeStack = true; - - try { - badPropertyAssertion(); - } catch (err) { - caughtErr = err; - } - }); - - it('should include Chai frames in stack trace', function () { - expect(caughtErr.stack).to.contain('propertyGetter'); - - if (typeof Proxy !== 'undefined' && typeof Reflect !== 'undefined') { - expect(caughtErr.stack).to.contain('proxyGetter'); - } - }); - - it('should include user frames in stack trace', function () { - expect(caughtErr.stack).to.contain('badPropertyAssertion'); - }); - }); - - describe('failed overwritten property assertions', function () { - var caughtErr = '__PRETEST__'; - - before(function () { - chai.config.includeStack = true; - - try { - badOverwrittenPropertyAssertion(); - } catch (err) { - caughtErr = err; - } - }); - - it('should include Chai frames in stack trace', function () { - expect(caughtErr.stack).to.contain('overwritingPropertyGetter'); - - if (typeof Proxy !== 'undefined' && typeof Reflect !== 'undefined') { - expect(caughtErr.stack).to.contain('proxyGetter'); - } - }); - - it('should include user frames in stack trace', function () { - expect(caughtErr.stack).to.contain('badOverwrittenPropertyAssertion'); - }); - }); - - describe('failed method assertions', function () { - var caughtErr = '__PRETEST__'; - - before(function () { - chai.config.includeStack = true; - - try { - badMethodAssertion(); - } catch (err) { - caughtErr = err; - } - }); - - it('should include Chai frames in stack trace', function () { - expect(caughtErr.stack).to.contain('methodWrapper'); - }); - - it('should include user frames in stack trace', function () { - expect(caughtErr.stack).to.contain('badMethodAssertion'); - }); - }); - - describe('failed overwritten method assertions', function () { - var caughtErr = '__PRETEST__'; - - before(function () { - chai.config.includeStack = true; - - try { - badOverwrittenMethodAssertion(); - } catch (err) { - caughtErr = err; - } - }); - - it('should include Chai frames in stack trace', function () { - expect(caughtErr.stack).to.contain('overwritingMethodWrapper'); - }); + describe('expect interface', function () { + // Functions that always throw an error + function badPropertyAssertion() { + expect(42).to.be.false; + } + function badOverwrittenPropertyAssertion() { + expect(42).tmpProperty; + } + function badMethodAssertion() { + expect(42).to.equal(false); + } + function badOverwrittenMethodAssertion() { + expect(42).tmpMethod(); + } + function badChainableMethodAssertion() { + expect(42).to.be.a('string'); + } + function badOverwrittenChainableMethodAssertion() { + expect(42).tmpChainableMethod(); + } - it('should include user frames in stack trace', function () { - expect(caughtErr.stack).to.contain('badOverwrittenMethodAssertion'); + describe('when true', function () { + describe('failed property assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = true; + + try { + badPropertyAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.contain('propertyGetter'); + + if (typeof Proxy !== 'undefined' && typeof Reflect !== 'undefined') { + expect(caughtErr.stack).to.contain('proxyGetter'); + } + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badPropertyAssertion'); + }); + }); + + describe('failed overwritten property assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = true; + + try { + badOverwrittenPropertyAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.contain('overwritingPropertyGetter'); + + if (typeof Proxy !== 'undefined' && typeof Reflect !== 'undefined') { + expect(caughtErr.stack).to.contain('proxyGetter'); + } + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badOverwrittenPropertyAssertion'); + }); + }); + + describe('failed method assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = true; + + try { + badMethodAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.contain('methodWrapper'); + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badMethodAssertion'); + }); + }); + + describe('failed overwritten method assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = true; + + try { + badOverwrittenMethodAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.contain('overwritingMethodWrapper'); + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badOverwrittenMethodAssertion'); + }); + }); + + describe('failed chainable method assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = true; + + try { + badChainableMethodAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.contain('chainableMethodWrapper'); + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badChainableMethodAssertion'); + }); + }); + + describe('failed overwritten chainable method assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = true; + + try { + badOverwrittenChainableMethodAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.contain('overwritingChainableMethodWrapper'); + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badOverwrittenChainableMethodAssertion'); + }); }); }); - - describe('failed chainable method assertions', function () { - var caughtErr = '__PRETEST__'; - - before(function () { - chai.config.includeStack = true; - - try { - badChainableMethodAssertion(); - } catch (err) { - caughtErr = err; - } - }); - - it('should include Chai frames in stack trace', function () { - expect(caughtErr.stack).to.contain('chainableMethodWrapper'); - }); - - it('should include user frames in stack trace', function () { - expect(caughtErr.stack).to.contain('badChainableMethodAssertion'); - }); - }); - - describe('failed overwritten chainable method assertions', function () { - var caughtErr = '__PRETEST__'; - - before(function () { - chai.config.includeStack = true; - - try { - badOverwrittenChainableMethodAssertion(); - } catch (err) { - caughtErr = err; - } - }); - - it('should include Chai frames in stack trace', function () { - expect(caughtErr.stack).to.contain('overwritingChainableMethodWrapper'); - }); - - it('should include user frames in stack trace', function () { - expect(caughtErr.stack).to.contain('badOverwrittenChainableMethodAssertion'); + + describe('when false', function () { + describe('failed property assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = false; + + try { + badPropertyAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should not include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.not.contain('propertyGetter'); + + if (typeof Proxy !== 'undefined' && typeof Reflect !== 'undefined') { + expect(caughtErr.stack).to.not.contain('proxyGetter'); + } + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badPropertyAssertion'); + }); + }); + + describe('failed overwritten property assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = false; + + try { + badOverwrittenPropertyAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should not include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.not.contain('overwritingPropertyGetter'); + + if (typeof Proxy !== 'undefined' && typeof Reflect !== 'undefined') { + expect(caughtErr.stack).to.not.contain('proxyGetter'); + } + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badOverwrittenPropertyAssertion'); + }); + }); + + describe('failed method assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = false; + + try { + badMethodAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should not include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.not.contain('methodWrapper'); + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badMethodAssertion'); + }); + }); + + describe('failed overwritten method assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = false; + + try { + badOverwrittenMethodAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should not include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.not.contain('overwritingMethodWrapper'); + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badOverwrittenMethodAssertion'); + }); + }); + + describe('failed chainable method assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = false; + + try { + badChainableMethodAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should not include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.not.contain('chainableMethodWrapper'); + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badChainableMethodAssertion'); + }); + }); + + describe('failed overwritten chainable method assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = false; + + try { + badOverwrittenChainableMethodAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should not include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.not.contain('overwritingChainableMethodWrapper'); + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badOverwrittenChainableMethodAssertion'); + }); }); }); }); - describe('when false', function () { - describe('failed property assertions', function () { - var caughtErr = '__PRETEST__'; - - before(function () { - chai.config.includeStack = false; - - try { - badPropertyAssertion(); - } catch (err) { - caughtErr = err; - } - }); - - it('should not include Chai frames in stack trace', function () { - expect(caughtErr.stack).to.not.contain('propertyGetter'); - - if (typeof Proxy !== 'undefined' && typeof Reflect !== 'undefined') { - expect(caughtErr.stack).to.not.contain('proxyGetter'); - } - }); - - it('should include user frames in stack trace', function () { - expect(caughtErr.stack).to.contain('badPropertyAssertion'); - }); - }); - - describe('failed overwritten property assertions', function () { - var caughtErr = '__PRETEST__'; - - before(function () { - chai.config.includeStack = false; - - try { - badOverwrittenPropertyAssertion(); - } catch (err) { - caughtErr = err; - } - }); - - it('should not include Chai frames in stack trace', function () { - expect(caughtErr.stack).to.not.contain('overwritingPropertyGetter'); - - if (typeof Proxy !== 'undefined' && typeof Reflect !== 'undefined') { - expect(caughtErr.stack).to.not.contain('proxyGetter'); - } - }); - - it('should include user frames in stack trace', function () { - expect(caughtErr.stack).to.contain('badOverwrittenPropertyAssertion'); - }); - }); - - describe('failed method assertions', function () { - var caughtErr = '__PRETEST__'; - - before(function () { - chai.config.includeStack = false; - - try { - badMethodAssertion(); - } catch (err) { - caughtErr = err; - } - }); - - it('should not include Chai frames in stack trace', function () { - expect(caughtErr.stack).to.not.contain('methodWrapper'); - }); - - it('should include user frames in stack trace', function () { - expect(caughtErr.stack).to.contain('badMethodAssertion'); - }); - }); - - describe('failed overwritten method assertions', function () { - var caughtErr = '__PRETEST__'; - - before(function () { - chai.config.includeStack = false; - - try { - badOverwrittenMethodAssertion(); - } catch (err) { - caughtErr = err; - } - }); - - it('should not include Chai frames in stack trace', function () { - expect(caughtErr.stack).to.not.contain('overwritingMethodWrapper'); - }); - - it('should include user frames in stack trace', function () { - expect(caughtErr.stack).to.contain('badOverwrittenMethodAssertion'); - }); - }); - - describe('failed chainable method assertions', function () { - var caughtErr = '__PRETEST__'; - - before(function () { - chai.config.includeStack = false; - - try { - badChainableMethodAssertion(); - } catch (err) { - caughtErr = err; - } - }); - - it('should not include Chai frames in stack trace', function () { - expect(caughtErr.stack).to.not.contain('chainableMethodWrapper'); - }); + describe('should interface', function () { + // Functions that always throw an error + function badPropertyAssertion() { + (42).should.be.false; + } + function badOverwrittenPropertyAssertion() { + (42).should.tmpProperty; + } + function badMethodAssertion() { + (42).should.equal(false); + } + function badOverwrittenMethodAssertion() { + (42).should.tmpMethod(); + } + function badChainableMethodAssertion() { + (42).should.be.a('string'); + } + function badOverwrittenChainableMethodAssertion() { + (42).should.tmpChainableMethod(); + } - it('should include user frames in stack trace', function () { - expect(caughtErr.stack).to.contain('badChainableMethodAssertion'); + describe('when true', function () { + describe('failed property assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = true; + + try { + badPropertyAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.contain('propertyGetter'); + + if (typeof Proxy !== 'undefined' && typeof Reflect !== 'undefined') { + expect(caughtErr.stack).to.contain('proxyGetter'); + } + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badPropertyAssertion'); + }); + }); + + describe('failed overwritten property assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = true; + + try { + badOverwrittenPropertyAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.contain('overwritingPropertyGetter'); + + if (typeof Proxy !== 'undefined' && typeof Reflect !== 'undefined') { + expect(caughtErr.stack).to.contain('proxyGetter'); + } + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badOverwrittenPropertyAssertion'); + }); + }); + + describe('failed method assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = true; + + try { + badMethodAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.contain('methodWrapper'); + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badMethodAssertion'); + }); + }); + + describe('failed overwritten method assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = true; + + try { + badOverwrittenMethodAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.contain('overwritingMethodWrapper'); + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badOverwrittenMethodAssertion'); + }); + }); + + describe('failed chainable method assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = true; + + try { + badChainableMethodAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.contain('chainableMethodWrapper'); + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badChainableMethodAssertion'); + }); + }); + + describe('failed overwritten chainable method assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = true; + + try { + badOverwrittenChainableMethodAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.contain('overwritingChainableMethodWrapper'); + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badOverwrittenChainableMethodAssertion'); + }); }); }); - - describe('failed overwritten chainable method assertions', function () { - var caughtErr = '__PRETEST__'; - - before(function () { - chai.config.includeStack = false; - - try { - badOverwrittenChainableMethodAssertion(); - } catch (err) { - caughtErr = err; - } - }); - - it('should not include Chai frames in stack trace', function () { - expect(caughtErr.stack).to.not.contain('overwritingChainableMethodWrapper'); - }); - - it('should include user frames in stack trace', function () { - expect(caughtErr.stack).to.contain('badOverwrittenChainableMethodAssertion'); + + describe('when false', function () { + describe('failed property assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = false; + + try { + badPropertyAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should not include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.not.contain('propertyGetter'); + + if (typeof Proxy !== 'undefined' && typeof Reflect !== 'undefined') { + expect(caughtErr.stack).to.not.contain('proxyGetter'); + } + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badPropertyAssertion'); + }); + }); + + describe('failed overwritten property assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = false; + + try { + badOverwrittenPropertyAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should not include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.not.contain('overwritingPropertyGetter'); + + if (typeof Proxy !== 'undefined' && typeof Reflect !== 'undefined') { + expect(caughtErr.stack).to.not.contain('proxyGetter'); + } + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badOverwrittenPropertyAssertion'); + }); + }); + + describe('failed method assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = false; + + try { + badMethodAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should not include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.not.contain('methodWrapper'); + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badMethodAssertion'); + }); + }); + + describe('failed overwritten method assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = false; + + try { + badOverwrittenMethodAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should not include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.not.contain('overwritingMethodWrapper'); + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badOverwrittenMethodAssertion'); + }); + }); + + describe('failed chainable method assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = false; + + try { + badChainableMethodAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should not include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.not.contain('chainableMethodWrapper'); + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badChainableMethodAssertion'); + }); + }); + + describe('failed overwritten chainable method assertions', function () { + var caughtErr = '__PRETEST__'; + + before(function () { + chai.config.includeStack = false; + + try { + badOverwrittenChainableMethodAssertion(); + } catch (err) { + caughtErr = err; + } + }); + + it('should not include Chai frames in stack trace', function () { + expect(caughtErr.stack).to.not.contain('overwritingChainableMethodWrapper'); + }); + + it('should include user frames in stack trace', function () { + expect(caughtErr.stack).to.contain('badOverwrittenChainableMethodAssertion'); + }); }); }); }); From 82ca613d0ffe8a3d078a54635371346102d78fb5 Mon Sep 17 00:00:00 2001 From: Grant Snodgrass Date: Sun, 11 Dec 2016 15:39:23 -0500 Subject: [PATCH 7/8] refactor(ssfi): remove dead code and update docs There was some dead code leftover from before `includeStack` was made into a config value (as opposed to existing as a property on the Assertion object). This commit removes that dead code, and adds inline documentation for the remaining stack-related code. --- lib/chai/utils/addChainableMethod.js | 11 ++++++++--- lib/chai/utils/addMethod.js | 13 ++++++++++--- lib/chai/utils/addProperty.js | 24 +++++++++++++++++++----- lib/chai/utils/overwriteMethod.js | 18 +++++++++++++++--- lib/chai/utils/overwriteProperty.js | 28 +++++++++++++++++++++++----- lib/chai/utils/proxify.js | 9 +++++++++ 6 files changed, 84 insertions(+), 19 deletions(-) diff --git a/lib/chai/utils/addChainableMethod.js b/lib/chai/utils/addChainableMethod.js index 46fa374e8..a79a707ab 100644 --- a/lib/chai/utils/addChainableMethod.js +++ b/lib/chai/utils/addChainableMethod.js @@ -80,9 +80,14 @@ module.exports = function addChainableMethod(ctx, name, method, chainingBehavior chainableBehavior.chainingBehavior.call(this); var chainableMethodWrapper = function () { - var old_ssfi = flag(this, 'ssfi'); - if (old_ssfi) - flag(this, 'ssfi', chainableMethodWrapper); + // Use this chainable method wrapper as the starting point for + // removing implementation frames from the stack trace of a failed + // assertion. Note that this is the correct starting point even if + // this assertion has been overwritten since overwriting a chainable + // method merely replaces the saved methods in `ctx.__methods` instead + // of completely replacing the overwritten assertion. + flag(this, 'ssfi', chainableMethodWrapper); + var result = chainableBehavior.method.apply(this, arguments); if (result !== undefined) { diff --git a/lib/chai/utils/addMethod.js b/lib/chai/utils/addMethod.js index bc0d8d9f2..a5d32060a 100644 --- a/lib/chai/utils/addMethod.js +++ b/lib/chai/utils/addMethod.js @@ -38,10 +38,17 @@ var transferFlags = require('./transferFlags'); module.exports = function addMethod(ctx, name, method) { var methodWrapper = function () { - var keep_ssfi = flag(this, 'keep_ssfi'); - var old_ssfi = flag(this, 'ssfi'); - if (!keep_ssfi && old_ssfi) + // If this assertion hasn't been overwritten, then use this method wrapper + // as the starting point for removing implementation frames from the stack + // trace of a failed assertion. + // + // Note: If this assertion has been overwritten, and thus the `keep_ssfi` + // flag is set, then the overwriting method wrapper is used as the starting + // point instead. This prevents the overwriting method wrapper from showing + // up in the stack trace since it's invoked before this method wrapper. + if (!flag(this, 'keep_ssfi')) { flag(this, 'ssfi', methodWrapper); + } var result = method.apply(this, arguments); if (result !== undefined) diff --git a/lib/chai/utils/addProperty.js b/lib/chai/utils/addProperty.js index 6e535db5d..f58400e5d 100644 --- a/lib/chai/utils/addProperty.js +++ b/lib/chai/utils/addProperty.js @@ -40,11 +40,25 @@ module.exports = function addProperty(ctx, name, getter) { Object.defineProperty(ctx, name, { get: function propertyGetter() { - if (!isProxyEnabled()) { - var keep_ssfi = flag(this, 'keep_ssfi'); - var old_ssfi = flag(this, 'ssfi'); - if (!keep_ssfi && old_ssfi) - flag(this, 'ssfi', propertyGetter); + // If proxy protection is disabled and this assertion hasn't been + // overwritten, then use this property getter as the starting point for + // removing implementation frames from the stack trace of a failed + // assertion. + // + // Notes: + // + // - If proxy protection is enabled, then the proxy getter is used as + // the starting point instead. This prevents the proxy getter from + // showing up in the stack trace since it's invoked before this + // property getter. + // + // - If proxy protection is disabled but this assertion has been + // overwritten, and thus the `keep_ssfi` flag is set, then the + // overwriting property getter is used as the starting point instead. + // This prevents the overwriting property getter from showing up in + // the stack trace since it's invoked before this property getter. + if (!isProxyEnabled() && !flag(this, 'keep_ssfi')) { + flag(this, 'ssfi', propertyGetter); } var result = getter.call(this); diff --git a/lib/chai/utils/overwriteMethod.js b/lib/chai/utils/overwriteMethod.js index 3342c4299..5d2306e8e 100644 --- a/lib/chai/utils/overwriteMethod.js +++ b/lib/chai/utils/overwriteMethod.js @@ -54,11 +54,23 @@ module.exports = function overwriteMethod(ctx, name, method) { _super = _method; var overwritingMethodWrapper = function () { - var keep_ssfi = flag(this, 'keep_ssfi'); - var old_ssfi = flag(this, 'ssfi'); - if (!keep_ssfi && old_ssfi) + // If proxy protection is disabled and this overwriting assertion hasn't + // been overwritten again by yet another assertion, then use this method + // wrapper as the starting point for removing implementation frames from the + // stack trace of a failed assertion. + // + // Note: If this assertion has been overwritten, and thus the `keep_ssfi` + // flag is set, then the overwriting method wrapper is used as the starting + // point instead. This prevents the overwriting method wrapper from showing + // up in the stack trace since it's invoked before this method wrapper. + if (!flag(this, 'keep_ssfi')) { flag(this, 'ssfi', overwritingMethodWrapper); + } + // The `keep_ssfi` flag is set so that if this assertion ends up calling + // the overwritten assertion, then the overwritten assertion doesn't attempt + // to use itself as the starting point for removing implementation frames + // from the stack trace of a failed assertion. flag(this, 'keep_ssfi', true); var result = method(_super).apply(this, arguments); flag(this, 'keep_ssfi', false); diff --git a/lib/chai/utils/overwriteProperty.js b/lib/chai/utils/overwriteProperty.js index 0ffb26834..f988ced32 100644 --- a/lib/chai/utils/overwriteProperty.js +++ b/lib/chai/utils/overwriteProperty.js @@ -52,13 +52,31 @@ module.exports = function overwriteProperty(ctx, name, getter) { Object.defineProperty(ctx, name, { get: function overwritingPropertyGetter() { - if (!isProxyEnabled()) { - var keep_ssfi = flag(this, 'keep_ssfi'); - var old_ssfi = flag(this, 'ssfi'); - if (!keep_ssfi && old_ssfi) - flag(this, 'ssfi', overwritingPropertyGetter); + // If proxy protection is disabled and this overwriting assertion hasn't + // been overwritten again by yet another assertion, then use this + // property getter as the starting point for removing implementation + // frames from the stack trace of a failed assertion. + // + // Notes: + // + // - If proxy protection is enabled, then the proxy getter is used as + // the starting point instead. This prevents the proxy getter from + // showing up in the stack trace since it's invoked before this + // property getter. + // + // - If proxy protection is disabled but this assertion has been + // overwritten, and thus the `keep_ssfi` flag is set, then the + // overwriting property getter is used as the starting point instead. + // This prevents the overwriting property getter from showing up in + // the stack trace since it's invoked before this property getter. + if (!isProxyEnabled() && !flag(this, 'keep_ssfi')) { + flag(this, 'ssfi', overwritingPropertyGetter); } + // The `keep_ssfi` flag is set so that if this assertion ends up calling + // the overwritten assertion, then the overwritten assertion doesn't + // attempt to use itself as the starting point for removing + // implementation frames from the stack trace of a failed assertion. flag(this, 'keep_ssfi', true); var result = getter(_super).call(this); flag(this, 'keep_ssfi', false); diff --git a/lib/chai/utils/proxify.js b/lib/chai/utils/proxify.js index a9ba811d2..85c6cbf08 100644 --- a/lib/chai/utils/proxify.js +++ b/lib/chai/utils/proxify.js @@ -67,6 +67,15 @@ module.exports = function proxify(obj, nonChainableMethodName) { } } + // Use this proxy getter as the starting point for removing implementation + // frames from the stack trace of a failed assertion. For property + // assertions, this prevents the proxy getter from showing up in the stack + // trace since it's invoked before the property getter. For method and + // chainable method assertions, this flag will end up getting changed to + // the method wrapper, which is good since this frame will no longer be in + // the stack once the method is invoked. Note that Chai builtin assertion + // properties such as `__flags` are skipped since this is only meant to + // capture the starting point of an assertion. if (builtins.indexOf(property) === -1) { flag(target, 'ssfi', proxyGetter); } From 0c0983618e025576b7c86e607cd104ebba3bc882 Mon Sep 17 00:00:00 2001 From: Grant Snodgrass Date: Sun, 18 Dec 2016 18:47:40 -0500 Subject: [PATCH 8/8] refactor(Assertion): small edit and improve docs - Rename third parameter of Assertion constructor from `stack` to `ssfi` for consistency's sake. - Add documentation to Assertion constructor explaining what the `object`, `message`, and `ssfi` flags are for. --- lib/chai/assertion.js | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/lib/chai/assertion.js b/lib/chai/assertion.js index 107eb083c..491add77f 100644 --- a/lib/chai/assertion.js +++ b/lib/chai/assertion.js @@ -26,11 +26,34 @@ module.exports = function (_chai, util) { * * Creates object for chaining. * + * `Assertion` objects contain metadata in the form of flags. Three flags can + * be assigned during instantiation by passing arguments to this constructor: + * + * - `object`: This flag contains the target of the assertion. For example, in + * the assertion `expect(numKittens).to.equal(7);`, the `object` flag will + * contain `numKittens` so that the `equal` assertion can reference it when + * needed. + * + * - `message`: This flag contains an optional custom error message to be + * prepended to the error message that's generated by the assertion when it + * fails. + * + * - `ssfi`: This flag stands for "start stack function indicator". It + * contains a function reference that serves as the starting point for + * removing frames from the stack trace of the error that's created by the + * assertion when it fails. The goal is to provide a cleaner stack trace to + * end users by removing Chai's internal functions. Note that it only works + * in environments that support `Error.captureStackTrace`, and only when + * `Chai.config.includeStack` hasn't been set to `false`. + * + * @param {Mixed} obj target of the assertion + * @param {String} msg (optional) custom error message + * @param {Function} stack (optional) starting point for removing stack frames * @api private */ - function Assertion (obj, msg, stack) { - flag(this, 'ssfi', stack || Assertion); + function Assertion (obj, msg, ssfi) { + flag(this, 'ssfi', ssfi || Assertion); flag(this, 'object', obj); flag(this, 'message', msg);