Connections property deprecated error.

[wbyoung]

I'm getting the following error: connections property is deprecated. Use getConnections() method when using calledOnce, calledTwice, etc.

It doesn't happen when using expect(spy.callCount).to.eql(1).

[domenic]

Can you give a small reproducible code snippet, with no external libraries except Chai, Sinon, and Sinon-Chai, that I can run on my own computer?

[wbyoung]

It appears that I can't give you that without externals. I just checked and it seems to be something to do with running requests through express, but I'll give you something as small as I can.

[domenic]

Yes, it sounds very likely you are triggering some issue with a third-party library, and not a bug in Sinon-Chai.

[wbyoung]

Here's an example.

Any ideas on which module could be triggering the issue would be great!

[domenic]

It looks like an issue with Express's close method: http://stackoverflow.com/questions/18381899/express-js-app-not-shutting-down-cleanly-with-server-close

[wbyoung]

I might be misunderstanding that Stack Overflow page, but commenting out the call to server.close doesn't resolve the issue. And commenting out the calledOnce assertion does resolve the issue.

What am I missing?

[wbyoung]

And why would changing the assertion to expect(spy.callCount).to.eql(1) change the result as well?

[domenic]

I don't really know the answers to these questions. I'd suggest getting a debugger out and placing a breakpoint where that message is logged, and you can then trace the call stack to find who is responsible for it.

[wbyoung]

The %C in the formatting that sinon-chai defines is causing the spy to print arguments that were passed to each invocation of the spy.

In this specific case, that's causing a deep enumeration of the request and response arguments. A response argument will eventually deep enumerate to a Sever, and when continuing to deep enumerate that object, it hits the connections property.

So this would be a problem with any Server instance that sinon formats. sinon doesn't really convert objects to strings, though. It doesn't generate error messages, so there's no need. It leaves that as an option for the user.

sinon-chai requires providing success/failure messages to chai, though. Because of this, it's creating strings from the argument objects via spy.printf.

I'm a bit confused as to why it's using the %C option for the callCount, calledOnce, calledTwice, and calledThrice assertions. This was introduced in c6ed04f. There aren't test cases covering it, and it seems weird that the error message would include the arguments to the spy when we're only concerned with the number of times it was called.

I'm going to send a pull request to have the %C removed which addresses my issue, but I completely understand if it's not something you want to change.

[wbyoung]

Closing in favor of the pull request.

[domenic]

So, the reasoning behind those error strings is that we are copying Sinon:

https://github.com/cjohansen/Sinon.JS/blob/master/lib/sinon/assert.js#L171-L190

I think the issue is that we are calculating the messages ahead of time, even if the assertion fails. I am not sure whether we can fix this, given how Chai works. But I am pretty sure that if your vanilla-Sinon assertion fails, it will calculate the same error message, and thus cause the same deprecation notice to be printed.

That said, it would be nice to fix this to only calculate the error messages on failure. I wonder if we can think of a way to do that...

[wbyoung]

I tried down that route a little, but didn't get very far before deciding it would require some pretty big changes (potentially to Sinon as well).

The reason I was hoping to get it fixed was because if all tests pass, I don't really want to see the warning. It's a little concerning. It's not the end of the world, though.

[domenic]

I think there might be a way. For example, we could check to see if the test will pass, and if so, don't pass any message to Chai. This gets a bit tricky with the "not" cases, which is what Chai's assert was supposed to take care of for us, but could still work.

Alternately, we could pull-request Chai to allow functions for the messages, so that if the message is a function instead of a string, it evaluates it when it's time to print. That would also allow lazy message determination.

[wbyoung]

Sounds good. I'll take a look if I find some time, but it's not a huge priority (and I wouldn't imagine it is for you either).

[wbyoung]

@domenic awesome, thanks!