times() shouldn't be default #9

Open
soggie opened this Issue Jul 11, 2011 · 9 comments

Comments

Projects
None yet
2 participants

soggie commented Jul 11, 2011

I'm a little confused about why times() is defaulted to 1 for all mock functions. I mean, in unit tests I generally don't need times() at all. The only time I use it is when I am testing collection objects, but in most of my unit test cases I do call the same function with the same parameters multiple times to check for static property contamination, but there's no upper limit to how many times I call the same code.

Therefore I think a function shouldn't have "One time use only" by default.

arunoda closed this Jul 11, 2011

arunoda reopened this Jul 11, 2011

Owner

arunoda commented Jul 11, 2011

Yes. You've a valid point :)
I've to work on that.
But .times(n) has some use case too.

I would suggest this could be a possible feature for the next release.
it would be.
nodemock.mock('foo').takes(NUMBER, STRING, FUNCTION)
Do you have any say on this.

soggie commented Jul 11, 2011

What does the other two args do?

arunoda reply@reply.github.com wrote:

Yes. You've a valid point :)
I've to work on that.
But .times(n) has some use case too.

I would suggest this could be a possible feature for the next release.
it would be.
nodemock.mock('foo').takes(NUMBER, STRING, FUNCTION)
Do you have any say on this.

Reply to this email directly or view it on GitHub:
#9 (comment)

Owner

arunoda commented Jul 11, 2011

What you meant by other two args?

soggie commented Jul 11, 2011

You said times(number, string, function). Or did you meant either one of the three data types? :p

arunoda reply@reply.github.com wrote:

What you meant by other two args?

Reply to this email directly or view it on GitHub:
#9 (comment)

Owner

arunoda commented Jul 11, 2011

yes. NUMBER is an constant saying allow all numbers in that mock.
And it follows to others as well. :)

On Mon, Jul 11, 2011 at 11:57 PM, soggie <
reply@reply.github.com>wrote:

You said times(number, string, function). Or did you meant either one of
the three data types? :p

arunoda reply@reply.github.com wrote:

What you meant by other two args?

Reply to this email directly or view it on GitHub:
#9 (comment)

Reply to this email directly or view it on GitHub:
#9 (comment)

Arunoda Susiripala

http://gplus.to/arunoda
@arunoda http://twitter.com/arunoda

soggie commented Jul 13, 2011

How about this:

var mocker = nodemock.mock('foo').takes('INSERT INTO users ...').times(3)
                            .when(1).returns({ id : 1})
                            .when(2).returns({ id : 2 })
                            .when(3).returns({ id : 3 });

mocker.foo('INSERT INTO users ...');   // returns id = 1
mocker.foo('INSERT INTO users ...');   // returns id = 2
mocker.foo('INSERT INTO users ...');   // returns id = 3
mocker.foo('INSERT INTO users ...');   // throws exception

That would allow me to simulate a LOT of my test cases, as I tend to use cases like these to test for static contamination like:

var a = require('a');
console.log(a.foo.subvalue);   // should return 'foo'
var b = Object.create(a);
b.foo.subvalue = 'trololo';
console.log(a.foo.subvalue);   // should return 'trololo' now
var c = JSON.parse(JSON.stringify(a));
c.foo.subvalue = 'nyancat';
console.log(a.foo.subvalue);   // should still be 'trololo'

As you can imagine, this would be extremely useful when testing out these cases because now I can control what I return on which sequence, and I can even do boundary tests to check for out of bounds exceptions in my containers.

Finally, going back to the times() issue, I think a better solution would be, if times() is not specified, every call to the mock function would return the same result (which should be the default case) while times() is only used if one needs to control the repetitions.

I know this might sound like a huge refactoring process (which I would be all too gladly to help out), and might cause regression problems with current nodemock users, but I think in the long run it would be far more useful when covering a wider range of test cases.

Great job as always!

Owner

arunoda commented Jul 13, 2011

On Wed, Jul 13, 2011 at 8:10 AM, soggie <
reply@reply.github.com>wrote:

How about this:

var mocker = nodemock.mock('foo').takes('INSERT INTO users
...').times(3).returns({ id : 1}).onTimes(1).returns({ id : 2
}).onTimes(2).returns({ id : 3 }).onTimes(3);

mocker.foo('INSERT INTO users ...'); // returns id = 1
mocker.foo('INSERT INTO users ...'); // returns id = 2
mocker.foo('INSERT INTO users ...'); // returns id = 3
mocker.foo('INSERT INTO users ...'); // throws exception

That would allow me to simulate a LOT of my test cases, as I tend to use
cases like these to test for static contamination like:

var a = require('a');
console.log(a.foo.subvalue); // should return 'foo'
var b = Object.create(a);
b.foo.subvalue = 'trololo';
console.log(a.foo.subvalue); // should return 'trololo' now
var c = JSON.parse(JSON.stringify(a));
c.foo.subvalue = 'nyancat';
console.log(a.foo.subvalue); // should still be 'trololo'

As you can imagine, this would be extremely useful when testing out these
cases because now I can control what I return on which sequence, and I can
even do boundary tests to check for out of bounds exceptions in my
containers.

Finally, going back to the times() issue, I think a better solution would
be, if times() is not specified, every call to the mock function would
return the same result (which should be the default case) while times() is
only used if one needs to control the repetitions.

I know this might sound like a huge refactoring process (which I would be
all too gladly to help out), and might cause regression problems with
current nodemock users, but I think in the long run it would be far more
useful when covering a wider range of test cases.

Great job as always!

Reply to this email directly or view it on GitHub:
#9 (comment)

And We've this kind of feature and you can have a look at it.
We support this.
var mocked = nodemock.mock('foo').takes('INSERT INTO users
...').returns({id:1})
mocked.mock('foo').takes('INSERT INTO users ...').returns({id:2})
mocked.mock('foo').takes('INSERT INTO users ...').returns({id:3})
mocked.mock('foo').takes('INSERT INTO users ...').returns({id:4})

This works great.

Arunoda Susiripala

http://gplus.to/arunoda
@arunoda http://twitter.com/arunoda

soggie commented Jul 13, 2011

I understand, my problem with this method however, is that it assumes that I'll be calling them in sequence. Since most of our code is asynchronous, without the ability to assign a value to specific sequence numbers (they might not even be numerics, since we use hashes for our DB upgrade scripts), it would be hard to create a test environment for this.

Owner

arunoda commented Jul 13, 2011

Yes. It's assumed that It'll be called under sequentially.
We have a special trick to control the mock (the callback) under async env.
It allows you to call the callback at anywhere you want.

It's not documented yet. (as it's not used so much and I cannot assure it's
stability)
Here's a the usage have a look at it.

http://pastebin.com/iz6TGH0d

On Wed, Jul 13, 2011 at 8:39 AM, soggie <
reply@reply.github.com>wrote:

I understand, my problem with this method however, is that it assumes that
I'll be calling them in sequence. However, since most of our code is
asynchronous, without the ability to assign a value to specific sequence
numbers (they might not even be numerics, since we use hashes for our DB
upgrade scripts), it would be hard to create a test environment for this.

Reply to this email directly or view it on GitHub:
#9 (comment)

Arunoda Susiripala

http://gplus.to/arunoda
@arunoda http://twitter.com/arunoda

realistschuckle referenced this issue in realistschuckle/nodemock Jan 29, 2014

Open

times() shouldn't be default #1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment