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

Fix failing tests (#56) #57

Merged
merged 1 commit into from
Feb 18, 2019
Merged

Fix failing tests (#56) #57

merged 1 commit into from
Feb 18, 2019

Conversation

seanpoulter
Copy link
Contributor

@seanpoulter seanpoulter commented Apr 16, 2018

As reported in #55, two tests have been failing when running locally. Investigation has shown that the change event handler in bindInputPanel is not called. Despite the tests waiting twice the debouncing delay, the debounced function is not called.

The root cause of the test failures is the fake timers are not applying to the now() call in the debounce library. Based on the docs for fake timers, calling sinon.useFakeTimers() should:

Starts the clock at the UNIX epoch (timestamp of 0).

After calling clock.tick(ms) we should see ms, but instead we get the current timestamp (below highlighted variable on mid left):

screen shot 2018-04-15 at 23 43 08

Changing the order of the imports ensures that debounce and date-now use the fake timers, and the tests pass. With the changes in this PR, notice how the timestamp is our value for clock.tick(ms):

screen shot 2018-04-15 at 23 57 12

Those failing tests pass:
screen shot 2018-04-15 at 23 59 03

@seanpoulter
Copy link
Contributor Author

This should fix those tests @buzzdecafe and close #56.

Heads up, I accidentally created the PR before finishing my notes. Take a look on GitHub if your notification email didn't make sense. 😉

@buzzdecafe
Copy link
Member

wow that's all there was too it? Very nice catch, @seanpoulter . I wanna verify on my local tonight, then will merge. Thanks. (sinon often feels a bit too magical for me.)

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.

3 participants