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.

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

Date object in stub.withArgs is not working as expected #1865

Closed
mrkesh opened this issue Jul 23, 2018 · 7 comments
Closed

Date object in stub.withArgs is not working as expected #1865

mrkesh opened this issue Jul 23, 2018 · 7 comments

Comments

@mrkesh
Copy link

mrkesh commented Jul 23, 2018

Describe the bug
Setting up multiple return values for a stub method with Date objects as argument always returns the last value.

Sample code

it('withArgs with dates should be fine but it is not', function() {

   var d1 = new Date('Jan 01 2012 12:00:00 GMT'),
       d2 = new Date('Jan 01 2016 12:00:00 GMT'),
       foo = {
           bar: sinon.stub()
        };

   //foo.bar.withArgs(sinon.match.same(d1)).returns(100);
   //foo.bar.withArgs(sinon.match.same(d2)).returns(101);
   foo.bar.withArgs(d1).returns(100);
   foo.bar.withArgs(d2).returns(101);

   chai.assert.equal(foo.bar(d1), 100);    // fails! Expects 101
});

Expected behavior
Calling foo.bar(d1) should yield 100. Regardless of the setup, the stub will always return 101, the last return value set for the bar stub method. Using sinon.match.same works though.

Library version
Started failing from 6.1.3. Worked previously.

@mroderick
Copy link
Member

@mrkesh "worked previously" is a bit vague. Could you use git bisect to figure out when this was broken?

@mrkesh
Copy link
Author

mrkesh commented Jul 24, 2018

@mroderick , do you mean the actual commit that broke the it? The test created above passed with 6.1.2.

@fatso83
Copy link
Contributor

fatso83 commented Jul 31, 2018

He means the actual commit, yes.
Try filling out this template: https://github.com/fatso83/git-bisect-scripts?files=1

@mrkesh
Copy link
Author

mrkesh commented Aug 1, 2018

Ok, used git bisect and it turns out that the commit which introduced the bug was:

commit 79aaf8dce88f6a1ddc86c3b561dd16c20149397c
Author: Nathan Mahdavi <mail@nathanmahdavi.com>
Date:   Fri Jul 6 23:22:36 2018 +0200

    add samsam.deepEqual to check for object

I used the test above to for verification.

@43081j
Copy link
Contributor

43081j commented Aug 1, 2018

👋

FYI here's why this happens:

  • Two dates are compared here
  • The two dates are not deeply equal, so execution continues
  • Object.keys(new Date) is an empty array, so we return true in this block

Thus the two dates are returned as equal and we retrieve the first fake (the first withArgs call) instead of the second.

I would do a PR but i'm not too sure what you'd like to do to fix this.

@miryn
Copy link

miryn commented Aug 20, 2018

Hello everyone!

I run into the similar issue recently:

const sinon = require('sinon')
const assert = require('assert')

const stub = sinon.stub()

const d1 = new Date('2000-01-01')
const d2 = new Date('2001-01-01')

stub(d1)

assert(stub.calledWith(d1) === true)
assert(stub.calledWith(d2) === false)

When I explored the code I ended up with the same result as @43081j did. 014aecf might be a potential source of the issue.

@mrwillihog
Copy link
Contributor

mrwillihog commented Sep 14, 2018

I can confirm this issue was fixed by #1901. Running the tests from the original issue and the one mentioned by @miryn and both now pass.

@fatso83 fatso83 closed this as completed Sep 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants