Skip to content

Fix cancelLoadIfUnobserved(afterDelay), taking the delay into account#279

Merged
pcantrell merged 3 commits intobustoutsolutions:masterfrom
acecilia:cancelLoadIfUnobservedFix
Sep 30, 2018
Merged

Fix cancelLoadIfUnobserved(afterDelay), taking the delay into account#279
pcantrell merged 3 commits intobustoutsolutions:masterfrom
acecilia:cancelLoadIfUnobservedFix

Conversation

@acecilia
Copy link
Copy Markdown
Contributor

@acecilia acecilia commented Sep 27, 2018

Before this fix, the delay passed to cancelLoadIfUnobserved(afterDelay) was not used (a value of 0.05 seconds was fixed).
Added new tests.

@pcantrell
Copy link
Copy Markdown
Member

Ha, I’ll be darned! Thanks for catching this.

I’d be tempted to say that this is too small a thing to test, except … I screwed it up, so obviously it isn’t! I worry a little about writing timing-dependent tests like this: Travis is so slow and timings are so inconsistent, timing-dependent things like this can make CI sporadically fail. (Siesta’s already in that boat with waiting for Nocilla cleanup, and it's not great.) But I think your testing approach is good, so I’ll merge it and if it breaks CI we can add slop to the tests later.

Thanks!

@pcantrell pcantrell merged commit b13dad7 into bustoutsolutions:master Sep 30, 2018
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