forEach() requires 'this' argument

[hrimhari]

Otherwise, this is evaluated to undefined, which causes cryptic errors like:

Cannot read property 'method' of undefined

instead of:

writeFileSync received wrong arguments(...)

Purpose (TL;DR) - mandatory

give a concise (one or two short sentences) description of what what problem is being solved by this PR

Example: Fix issue #123456 by re-structuring the colour selection conditional in method paintBlue

Background (Problem in detail) - optional

When relevant, give a more thorough description of what the problem the PR is trying to solve. Examples of good topics for this section are:

• Link to an existing GitHub issue describing the problem
• Describing the problem in greater detail than the TL;DR section above
• How you discovered the issue, if it's not already described as an issue on GitHub
• Discussion of different approaches to solving this problem and why you chose your proposed solution

Solution - optional

When contributing code (and not just fixing typos, documentation and configuration), please describe why/how your solution works. This helps reviewers spot any mistakes in the implementation.

Example:
"This solution works by adding a paintBlue() method"
Then your reviewer might spot a mistake in the implementation, if paintBlue() uses the colour red.

How to verify - mandatory

1. Check out this branch (see github instructions below)
2. npm install

[coveralls]

Coverage remained the same at 95.372% when pulling 917ac62 on hrimhari:patch-1 into f701abe on sinonjs:master.

[ktsn]

I'm facing this problem when using withArgs too, hope this get merged.

[mantoni]

Please add a test to verify.

[ktsn]

I'm not sure about the assertion library (referee) so I may be wrong, but the assert.exception seems ignores any exceptions.

I confirmed that in the test case sinonMock .expectation .withArgs throws when called with wrong args in mock-test.js by removing assert.exception that wraps expectation(2, 2, 3);
It throws Cannot read property 'method' of undefined that is not seems like the expected exception.

[mantoni]

@ktsn assert.exceptions does not re-throw the exception, but verifies the exception type. In this case, the name of the exception is expected to be "ExpectationError". The message you see is the message of the exception, which is not asserted by this test case.

[ktsn]

@mantoni Yes, the second argument specifies the name of exception however the actual exception name is "TypeError" but there is no assertion error... Does that mean the existing test case is something wrong or assert.exception has any bug, doesn't it?

[mantoni]

@cjohansen You know about the details in referee better than me. Can you have a look?

[hrimhari]

You already have tests, but they all seem to misuse referee.assert.exception() by passing a string as matcher. My understanding of how referee.assert.exception() works is that if you pass a string as second parameter it will take it as a message, not a matcher. See https://github.com/busterjs/referee/blob/master/lib/referee.js#L551
You'd have to review all your test cases regarding exceptions.

[hrimhari]

One of the tests that should be failing: https://github.com/hrimhari/sinon/blob/master/test/mock-test.js#L513

[coveralls]

Coverage decreased (-0.007%) to 95.365% when pulling 217a032 on hrimhari:patch-1 into f701abe on sinonjs:master.

[hrimhari]

Any news?

[hrimhari]

@mantoni bump

[mantoni]

Seems fine. Sorry to keep you waiting @hrimhari.