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

Add ember-raf-scheduler registerWaiter using setupForTest or document it? #956

Closed
acorncom opened this issue Jul 11, 2022 · 3 comments
Closed

Comments

@acorncom
Copy link

We've recently begun using ember-table in several client apps (and quite liking it, thanks for all the hard work!). One thing we've started noticing after we introduced it was that we're seeing flakey tests. It's been hard to get to the bottom of what was going on, but when seeing this across three different apps of varying ages and on various versions of newer Ember, it didn't seem like it was app-specific.

I dug in further today and it looks like there have been intermittent discussions of the same issue by a number of different folks (see #810, #828, #667). Suggestions were to pass showAll=true to vertical-collection to disable occlusion (which was where we were seeing snags) but that seemed a bit like an ugly hack. Finally found html-next/vertical-collection#231 which referenced registering a test waiter for ember-raf-scheduler and when that was added in in our app all "flakiness" went away.

Should we consider adding the ember-raf-scheduler#registerWaiter call as part of the setupForTest that ember-table publishes or note the potential need for it for testing purposes in the test section of the readme? I'm happy to do the work, but thought I'd ask what your preferences would be for the library

@kpfefferle
Copy link
Member

Soooo... this just changed my life. An app that I work on has been in the awful habit of pausing our tests for a split second to let this vertical-collection rendering finish before making assertions or interacting with the table. Thanks to this tip and the ember-raf-scheduler test waiter, we don't need to do this any more!

Given that even we didn't know that this would fix our issue, I definitely think we should at least add this to the documentation around testing. Anywhere else in the test helpers that we think it would be useful as an out-of-the-box solution would be great too IMO.

Thanks for bringing this to my attention, @acorncom!

@acorncom
Copy link
Author

@kpfefferle yeah, I saw your notes about your snags, which was part of the reason I wrote up the issue when I finally figured things out: "If Kevin is hitting this and he works at Addepar, how many other folks are also? ... 🤔" I'm delighted it helped you so quickly as well.

@mixonic any thoughts on preferences on how we tackle this?

@acorncom
Copy link
Author

Fixed as part of #963 thanks folks!

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

No branches or pull requests

2 participants