From be8189c1ad933eae9e123bb183d97b143275ff22 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Thu, 31 Mar 2016 20:28:10 +0200 Subject: [PATCH 1/6] Remove unnecessary config checks config.includeStack is already checked by the assert() function --- lib/chai/utils/addChainableMethod.js | 3 +-- lib/chai/utils/addMethod.js | 3 +-- lib/chai/utils/addProperty.js | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/chai/utils/addChainableMethod.js b/lib/chai/utils/addChainableMethod.js index ebc106773..b3b2e70ed 100644 --- a/lib/chai/utils/addChainableMethod.js +++ b/lib/chai/utils/addChainableMethod.js @@ -10,7 +10,6 @@ var transferFlags = require('./transferFlags'); var flag = require('./flag'); -var config = require('../config'); /*! * Module variables @@ -79,7 +78,7 @@ module.exports = function (ctx, name, method, chainingBehavior) { var assert = function assert() { var old_ssfi = flag(this, 'ssfi'); - if (old_ssfi && config.includeStack === false) + if (old_ssfi) flag(this, 'ssfi', assert); var result = chainableBehavior.method.apply(this, arguments); return result === undefined ? this : result; diff --git a/lib/chai/utils/addMethod.js b/lib/chai/utils/addMethod.js index 439201a5c..4a3b74eb8 100644 --- a/lib/chai/utils/addMethod.js +++ b/lib/chai/utils/addMethod.js @@ -5,7 +5,6 @@ */ var chai = require('../../chai'); -var config = require('../config'); var flag = require('./flag'); var transferFlags = require('./transferFlags'); @@ -38,7 +37,7 @@ var transferFlags = require('./transferFlags'); module.exports = function (ctx, name, method) { ctx[name] = function () { var old_ssfi = flag(this, 'ssfi'); - if (old_ssfi && config.includeStack === false) + if (old_ssfi) flag(this, 'ssfi', ctx[name]); var result = method.apply(this, arguments); diff --git a/lib/chai/utils/addProperty.js b/lib/chai/utils/addProperty.js index cccb05996..c61d902e9 100644 --- a/lib/chai/utils/addProperty.js +++ b/lib/chai/utils/addProperty.js @@ -5,7 +5,6 @@ */ var chai = require('../../chai'); -var config = require('../config'); var flag = require('./flag'); var transferFlags = require('./transferFlags'); @@ -41,7 +40,7 @@ module.exports = function (ctx, name, getter) { Object.defineProperty(ctx, name, { get: function addProperty() { var old_ssfi = flag(this, 'ssfi'); - if (old_ssfi && config.includeStack === false) + if (old_ssfi) flag(this, 'ssfi', addProperty); var result = getter.call(this); From cf378e4782c8e61dda8403b3bbb5a03e7bc2f07a Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Thu, 31 Mar 2016 21:08:46 +0200 Subject: [PATCH 2/6] test/configuration: Use describe('includeStack') block --- test/configuration.js | 114 +++++++++++++++++++++--------------------- 1 file changed, 58 insertions(+), 56 deletions(-) diff --git a/test/configuration.js b/test/configuration.js index 18f61fb5a..3ce43254c 100644 --- a/test/configuration.js +++ b/test/configuration.js @@ -25,71 +25,73 @@ describe('configuration', function () { chai.expect('foo').to.not.exist; } - it('includeStack 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'); + 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'); + } } - } - }); + }); - it('includeStack is false for method assertions', function () { - chai.config.includeStack = 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'); + it('is false for method assertions', function () { + chai.config.includeStack = 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'); + } } - } - }); + }); - it('includeStack is true for property assertions', function () { - chai.config.includeStack = true; - - 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'); + it('is true for property assertions', function () { + chai.config.includeStack = true; + + 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'); + } } - } - }); + }); - it('includeStack is false for property assertions', function () { - chai.config.includeStack = false; - - 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'); + it('is false for property assertions', function () { + chai.config.includeStack = false; + + 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'); + } } - } + }); }); describe('truncateThreshold', function() { From cceadf6d82399151205bcd72659a0d35efe93fc9 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Thu, 31 Mar 2016 21:53:43 +0200 Subject: [PATCH 3/6] overwriteProperty: Fix "ssfi" flag overwriting --- lib/chai/utils/addProperty.js | 3 ++- lib/chai/utils/overwriteProperty.js | 12 +++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/chai/utils/addProperty.js b/lib/chai/utils/addProperty.js index c61d902e9..48e1b2dad 100644 --- a/lib/chai/utils/addProperty.js +++ b/lib/chai/utils/addProperty.js @@ -39,8 +39,9 @@ module.exports = function (ctx, name, getter) { Object.defineProperty(ctx, name, { get: function addProperty() { + var keep_ssfi = flag(this, 'keep_ssfi'); var old_ssfi = flag(this, 'ssfi'); - if (old_ssfi) + if (!keep_ssfi && old_ssfi) flag(this, 'ssfi', addProperty); var result = getter.call(this); diff --git a/lib/chai/utils/overwriteProperty.js b/lib/chai/utils/overwriteProperty.js index a13e7b2ae..fa5452f0f 100644 --- a/lib/chai/utils/overwriteProperty.js +++ b/lib/chai/utils/overwriteProperty.js @@ -4,6 +4,8 @@ * MIT Licensed */ +var flag = require('./flag'); + /** * ### overwriteProperty (ctx, name, fn) * @@ -46,8 +48,16 @@ module.exports = function (ctx, name, getter) { _super = _get.get Object.defineProperty(ctx, name, - { get: function () { + { get: function overwriteProperty() { + var keep_ssfi = flag(this, 'keep_ssfi'); + var old_ssfi = flag(this, 'ssfi'); + if (!keep_ssfi && old_ssfi) + flag(this, 'ssfi', overwriteProperty); + + flag(this, 'keep_ssfi', true); var result = getter(_super).call(this); + flag(this, 'keep_ssfi', false); + return result === undefined ? this : result; } , configurable: true From 1d8c3d9856c273e84b2bae41005595ee2ba875c8 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Thu, 31 Mar 2016 22:56:17 +0200 Subject: [PATCH 4/6] overwriteMethod: Fix "ssfi" flag overwriting --- lib/chai/utils/addMethod.js | 3 ++- lib/chai/utils/overwriteMethod.js | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/chai/utils/addMethod.js b/lib/chai/utils/addMethod.js index 4a3b74eb8..91c048d80 100644 --- a/lib/chai/utils/addMethod.js +++ b/lib/chai/utils/addMethod.js @@ -36,8 +36,9 @@ var transferFlags = require('./transferFlags'); module.exports = function (ctx, name, method) { ctx[name] = function () { + var keep_ssfi = flag(this, 'keep_ssfi'); var old_ssfi = flag(this, 'ssfi'); - if (old_ssfi) + if (!keep_ssfi && old_ssfi) flag(this, 'ssfi', ctx[name]); var result = method.apply(this, arguments); diff --git a/lib/chai/utils/overwriteMethod.js b/lib/chai/utils/overwriteMethod.js index 1d5f6bd53..20657a57c 100644 --- a/lib/chai/utils/overwriteMethod.js +++ b/lib/chai/utils/overwriteMethod.js @@ -4,6 +4,8 @@ * MIT Licensed */ +var flag = require('./flag'); + /** * ### overwriteMethod (ctx, name, fn) * @@ -46,7 +48,15 @@ module.exports = function (ctx, name, method) { _super = _method; ctx[name] = function () { + var keep_ssfi = flag(this, 'keep_ssfi'); + var old_ssfi = flag(this, 'ssfi'); + if (!keep_ssfi && old_ssfi) + flag(this, 'ssfi', ctx[name]); + + flag(this, 'keep_ssfi', true); var result = method(_super).apply(this, arguments); + flag(this, 'keep_ssfi', false); + return result === undefined ? this : result; } }; From c4f522f6fbf0cb06c5be2a65665a6ef7781bd077 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Sun, 3 Apr 2016 19:47:58 +0200 Subject: [PATCH 5/6] test: Add "overwriteProperty" stack trace tests --- test/utilities.js | 54 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/test/utilities.js b/test/utilities.js index ecea26d36..4882feb51 100644 --- a/test/utilities.js +++ b/test/utilities.js @@ -417,6 +417,60 @@ describe('utilities', function () { expect(expect('foo').result).to.equal('result'); }); + describe('overwriteProperty', function () { + before(function() { + chai.config.includeStack = false; + + chai.use(function(_chai, _) { + _chai.Assertion.addProperty('four', function() { + this.assert(this._obj === 4, 'expected #{this} to be 4', 'expected #{this} to not be 4', 4); + }); + + _chai.Assertion.overwriteProperty('four', function(_super) { + return function() { + if (typeof this._obj === 'string') { + this.assert(this._obj === 'four', 'expected #{this} to be \'four\'', 'expected #{this} to not be \'four\'', 'four'); + } else { + _super.call(this); + } + } + }); + }); + }); + + after(function() { + delete chai.Assertion.prototype.four; + }); + + it('calling _super has correct stack trace', function() { + try { + expect(5).to.be.four; + expect(false, 'should not get here because error thrown').to.be.ok; + } 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) { + expect(err.stack).to.include('utilities.js'); + expect(err.stack).to.not.include('overwriteProperty'); + } + } + }); + + it('overwritten behavior has correct stack trace', function() { + try { + expect('five').to.be.four; + expect(false, 'should not get here because error thrown').to.be.ok; + } 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) { + expect(err.stack).to.include('utilities.js'); + expect(err.stack).to.not.include('overwriteProperty'); + } + } + }); + }); + it('getMessage', function () { chai.use(function (_chai, _) { expect(_.getMessage({}, [])).to.equal(''); From 58532d9b06f86d69f5dac5133e6b95f1e841cd13 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Sun, 3 Apr 2016 22:59:34 +0200 Subject: [PATCH 6/6] test: Add "overwriteMethod" stack trace tests --- test/utilities.js | 54 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/test/utilities.js b/test/utilities.js index 4882feb51..5b62f9c82 100644 --- a/test/utilities.js +++ b/test/utilities.js @@ -320,6 +320,60 @@ describe('utilities', function () { expect(expect('foo').result()).to.equal('result'); }); + describe('overwriteMethod', function () { + before(function() { + chai.config.includeStack = false; + + chai.use(function(_chai, _) { + _chai.Assertion.addMethod('four', function() { + this.assert(this._obj === 4, 'expected #{this} to be 4', 'expected #{this} to not be 4', 4); + }); + + _chai.Assertion.overwriteMethod('four', function(_super) { + return function() { + if (typeof this._obj === 'string') { + this.assert(this._obj === 'four', 'expected #{this} to be \'four\'', 'expected #{this} to not be \'four\'', 'four'); + } else { + _super.call(this); + } + } + }); + }); + }); + + after(function() { + delete chai.Assertion.prototype.four; + }); + + it('calling _super has correct stack trace', function() { + try { + expect(5).to.be.four(); + expect(false, 'should not get here because error thrown').to.be.ok; + } 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) { + expect(err.stack).to.include('utilities.js'); + expect(err.stack).to.not.include('overwriteMethod'); + } + } + }); + + it('overwritten behavior has correct stack trace', function() { + try { + expect('five').to.be.four(); + expect(false, 'should not get here because error thrown').to.be.ok; + } 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) { + expect(err.stack).to.include('utilities.js'); + expect(err.stack).to.not.include('overwriteMethod'); + } + } + }); + }); + it('addProperty', function () { chai.use(function (_chai, _) { _chai.Assertion.addProperty('tea', function () {