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

Guard against last minute removal of toBeActive spy #181

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

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