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

Dummy connection changes #47

Closed
wants to merge 1 commit into from
Closed

Conversation

jkeen
Copy link
Contributor

@jkeen jkeen commented Feb 7, 2018

When writing an acceptance test, the audio wasn't being incremented because of the Ember.checkTestWaiters thing. I think we can skip this (???) if we handle the tick better. Hifi's test pass with this change, plus wnyc-web-client plus new-sounds-web-client.

@noslouch
Copy link
Contributor

noslouch commented Feb 8, 2018

But now tests are failing with this change. What if startTicking used Ember.run.later to do the interval? I think the original problem was that tests would fail on and off because fake audio files didn't stop ticking after a test ended. I was trying to force them to stop in test mode, but I think checkWaiters is not reliable in this case.

@jkeen
Copy link
Contributor Author

jkeen commented Feb 8, 2018

The good news is you're right about the tests failing, the bad news is that I don't think they're failing specifically due to these changes. I just triggered a rebuild on master and they're failing on there too. 💣

Something changed. Something is up.

@jkeen
Copy link
Contributor Author

jkeen commented Feb 8, 2018

Also, we tried the Ember.run.later initially, but I think the ember crowd recommends not to do that in tests because tests wouldn't complete, so I switched to timeout

emberjs/ember.js#3008

@jkeen
Copy link
Contributor Author

jkeen commented Jun 16, 2019

Closing this since the landscape has changed, and i think the testing issues are bigger than this little change.

@jkeen jkeen closed this Jun 16, 2019
@jkeen jkeen deleted the bug/dummy-connection-ticking branch April 16, 2020 22:14
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