Skip to content

Conversation

@pkaminski
Copy link
Contributor

@pkaminski pkaminski commented May 23, 2016

Since the broadcast of becameInactive is synchronous, it's possible for the toBeActive spy to be removed by client code just before it gets activated. This change guards against this (admittedly extremely rare) scenario by treating the spy as if though it didn't exist in the first place. Note that it's possible that another spy should've been selected instead, then, but since the condition is so rare it's probably OK to wait until the next interval fires to recompute the actual active spy.


This change is Reviewable

Since the broadcast of `becameInactive` is synchronous, it's possible for the `toBeActive` spy to be removed by client code just before it gets activated.  This change guards against this (admittedly extremely rare) scenario by treating the spy as if though it didn't exist in the first place.  Note that it's possible that another spy should've been selected instead, then, but since the condition is so rare it's probably OK to wait until the next interval fires to recompute the actual active spy.
@oblador
Copy link
Owner

oblador commented May 23, 2016

Thanks for your PR! Can you look into why the test is failing?

@pkaminski
Copy link
Contributor Author

I'm mystified at the failing tests. They all pass on my machine (against Chrome 50), and the failure appears to be in an area completely unrelated to the change I made (a context can't be found). Is it possible to force the tests to rerun, in case this is some kind of transient?

@oblador
Copy link
Owner

oblador commented May 23, 2016

Reran with same results, I'll have to look into the errors which might be unrelated. Can I ask you to follow the style and put the statement in { }?

@pkaminski
Copy link
Contributor Author

Oops, fixed. The joys of working on multiple projects with different styles. 😝

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

Successfully merging this pull request may close these issues.

2 participants