Merge pull request #661 from Turbo87/stacks

Fix stacktraces for overwritten properties and methods
chaijs · Apr 4, 2016 · 4dfefb0 · 4dfefb0
2 parents 5608a0b + 58532d9
commit 4dfefb0
Show file tree

Hide file tree

Showing 7 changed files with 192 additions and 63 deletions.
diff --git 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
@@ -5,7 +5,6 @@
  */
 
 var chai = require('../../chai');
-var config = require('../config');
 var flag = require('./flag');
 var transferFlags = require('./transferFlags');
 
@@ -37,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 && config.includeStack === false)
+    if (!keep_ssfi && 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
@@ -5,7 +5,6 @@
  */
 
 var chai = require('../../chai');
-var config = require('../config');
 var flag = require('./flag');
 var transferFlags = require('./transferFlags');
 
@@ -40,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 && config.includeStack === false)
+        if (!keep_ssfi && old_ssfi)
           flag(this, 'ssfi', addProperty);
 
         var result = getter.call(this);

diff --git 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;
   }
 };
diff --git 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

diff --git 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() {

diff --git 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 () {
@@ -417,6 +471,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('');