Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(sandbox): adds support for chai.spy.sandbox #61

Merged
merged 1 commit into from Jan 16, 2017
Merged

feat(sandbox): adds support for chai.spy.sandbox #61

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
237 changes: 209 additions & 28 deletions lib/spy.js
View file
Open in desktop
Expand Up @@ -13,6 +13,151 @@ module.exports = function (chai, _) {
var Assertion = chai.Assertion
, flag = _.flag
, i = _.inspect
, STATE_KEY = typeof Symbol === 'undefined' ? '__state' : Symbol('state')
, spyAmount = 0
, DEFAULT_SANDBOX = new Sandbox()

/**
* # Sandbox constructor (function)
*
* Initialize new Sandbox instance
*
* @returns new sandbox
* @api private
*/

function Sandbox() {
this[STATE_KEY] = {};
}

/**
* # Sandbox.on (function)
*
* Wraps an object method into spy assigned to sandbox. All calls will
* pass through to the original function.
*
* var spy = chai.spy.sandbox();
* var isArray = spy.on(Array, 'isArray');
*
* const array = []
* const spy = chai.spy.sandbox();
* const [push, pop] = spy.on(array, ['push', 'pop']);
*
* @param {Object} object
* @param {String|String[]} method name or methods names to spy on
* @returns created spy or created spies
* @api public
*/

Sandbox.prototype.on = function (object, methodName) {
if (Array.isArray(methodName)) {
return methodName.map(function (name) {
return this.on(object, name);
}, this);
}

var isMethod = typeof object[methodName] === 'function'

if (methodName in object && !isMethod) {
throw new Error([
'Unable to spy property "', methodName,
'". Only methods and non-existing properties can be spied.'
].join(''))
}

if (isMethod && object[methodName].__spy) {
throw new Error('"' + methodName + '" is already a spy')
}

var method = chai.spy('object.' + methodName, object[methodName]);
var trackingId = ++spyAmount

this[STATE_KEY][trackingId] = method;
method.__spy.tracked = {
object: object
, methodName: methodName
, originalMethod: object[methodName]
, isOwnMethod: object.hasOwnProperty(methodName)
};
object[methodName] = method;

return method;
};

/**
* # Sandbox.restore (function)
*
* Restores previously wrapped object's method.
* Restores all spied objects of a sandbox if called without parameters.
*
* var spy = chai.spy.sandbox();
* var object = spy.on(Array, 'isArray');
* spy.restore(Array, 'isArray'); // or spy.restore();
*
* @param {Object} [object]
* @param {String|String[]} [methods] method name or method names
* @return {Sandbox} Sandbox instance
* @api public
*/

Sandbox.prototype.restore = function (object, methods) {
var hasFilter = Boolean(object && methods);
var sandbox = this;

if (methods && !Array.isArray(methods)) {
methods = [methods]
}

Object.keys(this[STATE_KEY]).some(function (spyId) {
var spy = sandbox[STATE_KEY][spyId];
var tracked = spy.__spy.tracked;
var isObjectSpied = !object || object === tracked.object;
var isMethodSpied = !methods || methods.indexOf(tracked.methodName) !== -1;

delete sandbox[STATE_KEY][spyId];

if (!isObjectSpied && !isMethodSpied) {
return false;
}

sandbox.restoreTrackedObject(spy);

if (hasFilter) {
return true;
}
});

return this;
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be nice to either return this - allowing for chaining (spy.restore(Array.prototype, 'push').restore(Array.prototype, 'pop')) or alternatively allow passing an Array to restore's second arg (spy.restore(Array, ['push', 'pop'])).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, make sense. Will implement both suggestions


/**
* # Sandbox.restoreTrackedObject (function)
*
* Restores tracked object's method
*
* var spy = chai.spy.sandbox();
* var isArray = spy.on(Array, 'isArray');
* spy.restoreTrackedObject(isArray);
*
* @param {Spy} spy
* @api private
*/

Sandbox.prototype.restoreTrackedObject = function (spy) {
var tracked = spy.__spy.tracked;

if (!tracked) {
throw new Error('It is not possible to restore a non-tracked spy.')
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this silent failure path. We should throw an error here, telling the user they cannot restore a non-tracked spy.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure


if (tracked.isOwnMethod) {
tracked.object[tracked.methodName] = tracked.originalMethod;
} else {
delete tracked.object[tracked.methodName];
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work here! This conditional will resolve lots of subtle errors.


spy.__spy.tracked = null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very interesting - having the spy clear its tracking, but still be a spy. A developer could restore a spy method after using it (maybe even inside the spy itself) but still assert on the spy after its call. Very cool. Opens us up for later cool things like:

var myArray = [];
pushSpy = chai.spy.on.once(myArray, 'push')
expect(myArray.push).to.be.a.spy();
myArray.push('foo');
expect(myArray.push).to.not.be.a.spy.and.equal(Array.prototype.push);
expect(pushSpy).to.have.been.called(1).with.exactly('foo');

};

/**
* # chai.spy (function)
Expand Down Expand Up @@ -62,10 +207,11 @@ module.exports = function (chai, _) {

proxy.prototype = fn.prototype;
proxy.toString = function toString() {
var l = this.__spy.calls.length;
var state = this.__spy;
var l = state.calls.length;
var s = "{ Spy";
if (this.__spy.name)
s += " '" + this.__spy.name + "'";
if (state.name)
s += " '" + state.name + "'";
if (l > 0)
s += ", " + l + " call" + (l > 1 ? 's' : '');
s += " }";
Expand All @@ -75,7 +221,7 @@ module.exports = function (chai, _) {
/**
* # proxy.reset (function)
*
* Resets __spy object parameters for instantiation and reuse
* Resets spy's state object parameters for instantiation and reuse
* @returns proxy spy object
*/
proxy.reset = function() {
Expand All @@ -91,53 +237,67 @@ module.exports = function (chai, _) {
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe not as part of this PR, but I'd like to remove the reset method from the spy and push it up into our API - or possibly even get rid of reset entirely now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I also think that method should be removed and this should be a part of a separate PR


/**
* # chai.spy.on (function)
* # chai.spy.sandbox (function)
*
* Wraps an object method into spy. All calls will
* pass through to the original function.
* Creates sandbox which allow to restore spied objects with spy.on.
* All calls will pass through to the original function.
*
* var spy = chai.spy.on(Array, 'isArray');
* var spy = chai.spy.sandbox();
* var isArray = spy.on(Array, 'isArray');
*
* @param {Object} object
* @param {...String} method names to spy on
* @param {String} method name to spy on
* @returns passed object
* @api public
*/

chai.spy.on = function (object) {
var methodNames = Array.prototype.slice.call(arguments, 1);
chai.spy.sandbox = function () {
return new Sandbox()
};

methodNames.forEach(function(methodName) {
object[methodName] = chai.spy(object[methodName]);
});
/**
* # chai.spy.on (function)
*
* The same as Sandbox.on.
* Assignes newly created spy to DEFAULT sandbox
*
* var isArray = chai.spy.on(Array, 'isArray');
*
* @see Sandbox.on
* @api public
*/

return object;
chai.spy.on = function () {
return DEFAULT_SANDBOX.on.apply(DEFAULT_SANDBOX, arguments)
};

/**
* # chai.spy.object (function)
* # chai.spy.interface (function)
*
* Creates an object interface with spied methods.
*
* Creates an object with spied methods.
* var events = chai.spy.interface('Events', ['trigger', 'on']);
*
* var object = chai.spy.object('Array', [ 'push', 'pop' ]);
* var array = chai.spy.interface({
* push(item) {
* this.items = this.items || [];
* return this.items.push(item);
* }
* });
*
* @param {String} [name] object name
* @param {String[]|Object} method names or method definitions
* @param {String|Object} name object or object name
* @param {String[]} [methods] method names
* @returns object with spied methods
* @api public
*/

chai.spy.object = function (name, methods) {
chai.spy.interface = function (name, methods) {
var defs = {};

if (name && typeof name === 'object') {
methods = name;
name = 'object';
}

if (methods && !Array.isArray(methods)) {
defs = methods;
methods = Object.keys(methods);
methods = Object.keys(name);
defs = name;
name = 'mock';
}

return methods.reduce(function (object, methodName) {
Expand All @@ -146,6 +306,27 @@ module.exports = function (chai, _) {
}, {});
};

/**
* # chai.spy.restore (function)
*
* The same as Sandbox.restore.
* Restores spy assigned to DEFAULT sandbox
*
* var array = []
* chai.spy.on(array, 'push');
* expect(array.push).to.be.spy // true
*
* chai.spy.restore()
* expect(array.push).to.be.spy // false
*
* @see Sandbox.restore
* @api public
*/

chai.spy.restore = function () {
return DEFAULT_SANDBOX.restore.apply(DEFAULT_SANDBOX, arguments)
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels like we could shorten a lot of this to just;

chai.spy = new Sandbox();
chai.spy.sandbox = () => new Sandbox();

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But then it won't be possible to create regular spies, like:

var success = chai.spy()

asyncOperation().then(success)

expect(success).to.have.been.called()

Copy link
Contributor Author

@stalniy stalniy Dec 11, 2016 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to make things shorter I can do this:

['on', 'restore', 'object'].forEach(function (methodName) {
  chai.spy[methodName] = function() {
    return DEFAULT_SANDBOX[methodName].apply(DEFAULT_SANDBOX, arguments)
  };
})

Update: but then it will be harder to write down JSDocs

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah good point about regular spies. Let's keep the code as is then 👍


/**
* # chai.spy.returns (function)
*
Expand Down