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

Potential issue with stepTimer in UtteranceQueueTests #51

Closed
jessegreenberg opened this issue Jan 21, 2022 · 3 comments
Closed

Potential issue with stepTimer in UtteranceQueueTests #51

jessegreenberg opened this issue Jan 21, 2022 · 3 comments

Comments

@jessegreenberg
Copy link
Contributor

The stepTimer emits in before() like

    // step the timer, because utteranceQueue runs on timer
    intervalID = setInterval( () => { // eslint-disable-line bad-sim-text
      stepTimer.emit( timerInterval ); // step timer in seconds
    }, timerInterval * 1000 );

No matter how much time actually passes in the interval, we emit that timeInterval time as elapsed. I think this may be causing inconsistent test results. Usually when I run the tests as they are on master they pass, but every once in a while some fail.

@jessegreenberg jessegreenberg self-assigned this Jan 21, 2022
@jessegreenberg
Copy link
Contributor Author

From #50

@jessegreenberg
Copy link
Contributor Author

jessegreenberg commented Jan 21, 2022

I changed these to emit the elapsed time since the list time stepTimer has emitted and so far tests have not failed for me like they did before. This is noticeably more consistent for me on Firefox where I saw elapsed time swing from 16-48 ms in between each call of the window.setInterval callback.

@zepumph just want to make sure you are OK with this change?

@zepumph
Copy link
Member

zepumph commented Jan 24, 2022

Looks really, really great. Good bug fix. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants