Skip to content

Commit

Permalink
Merge pull request #661 from Turbo87/stacks
Browse files Browse the repository at this point in the history 
Fix stacktraces for overwritten properties and methods
  • Loading branch information
@keithamus
keithamus committed Apr 4, 2016
2 parents 5608a0b + 58532d9 commit 4dfefb0
Show file tree
Hide file tree
Showing 7 changed files with 192 additions and 63 deletions.
3 changes: 1 addition & 2 deletions lib/chai/utils/addChainableMethod.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

var transferFlags = require('./transferFlags');
var flag = require('./flag');
var config = require('../config');

/*!
* Module variables
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions lib/chai/utils/addMethod.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
*/

var chai = require('../../chai');
var config = require('../config');
var flag = require('./flag');
var transferFlags = require('./transferFlags');

Expand Down Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions lib/chai/utils/addProperty.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
*/

var chai = require('../../chai');
var config = require('../config');
var flag = require('./flag');
var transferFlags = require('./transferFlags');

Expand Down Expand Up @@ -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);
Expand Down
10 changes: 10 additions & 0 deletions lib/chai/utils/overwriteMethod.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
* MIT Licensed
*/

var flag = require('./flag');

/**
* ### overwriteMethod (ctx, name, fn)
*
Expand Down Expand Up @@ -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;
}
};
12 changes: 11 additions & 1 deletion lib/chai/utils/overwriteProperty.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
* MIT Licensed
*/

var flag = require('./flag');

/**
* ### overwriteProperty (ctx, name, fn)
*
Expand Down Expand Up @@ -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
Expand Down
114 changes: 58 additions & 56 deletions test/configuration.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
108 changes: 108 additions & 0 deletions test/utilities.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down Expand Up @@ -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('');
Expand Down

0 comments on commit 4dfefb0

Please sign in to comment.