Make calledAfter symetric with calledBefore

[lucasfcosta]

Purpose (TL;DR)

This makes calledAfter symetric with calledBefore, as I explained in #1398.

Background (Problem in detail)

Semantically speaking, calledAfter does not do what it says it does.

When you say you expect something to have been calledAfter anotherThing, you don't care if something has been called last or not, all you care about is if something has been called any times after anotherThing.

This is the way calledBefore works. It is not concerned about whether or not the current spy was the first to be called, it just checks if the current spy has been called any times before the passed spy.

Solution

I just check if the last call to the current spy (this) has happened after the first call to the passed spy (spyFn) by comparing their callIds

I also fixed a test which had the incorrect spec for this.

How to verify

1. Check out this branch (see github instructions below)
2. npm install
3. npm test -> This will run all tests including the modified one with the correct spec

[coveralls]

Coverage remained the same at 95.02% when pulling c94d00a on lucasfcosta:simmetric-callAfter into ad78132 on sinonjs:master.

[mroderick]

This looks good! I've applied the PATCH label, as I feel this is fixing something that is broken, not changing a design decision. Does anyone feel differently about this?

[mantoni]

Patch should be fine. I think this is not going to make any passing tests fail.

[fatso83]

I think I put MAJOR on the accompanying issue as this could break users tests, which AFAIK is the definition of a breaking change (?). But this is almost a semantic discussion. If you are fine with patch, then I'm fine with patch. This is a very small part of Sinon anyhow, and not one that is used that frequently :-)

[lucasfcosta]

I'd use PATCH too.

Even though this might break user's tests, it will break them because they were relying on incorrect behavior and since we're just correcting an error it's a fix.

Imagine if this was a more explicit error, such as:

sum(2 + 2) // returns 5

// Test:
it('should sum two and two', () => {
  assert.equals(sum(2, 2), 5)
})

In that case, fixing sum would break user's test, but that's because they were relying on incorrect behavior.

[mroderick]

This has become [email protected] on npm