Make etch perform updates synchronously in specs

[maxbrunsfeld]

Whitespace-insensitive diff

We're seeing this package's tests fail intermittently in Atom's main Circle CI build.

https://circleci.com/gh/atom/atom/2664

KeyBindingResolverView
  when a keydown event occurs
    it displays all commands for the keydown event but does not clear for the keyup when there is no keyup binding
      timeout: timed out after 60000 msec waiting for promise to be resolved or rejected
    it displays all commands for the keydown event but does not clear for the keyup when there is no keyup binding
      timeout: timed out after 60000 msec waiting for promise to be resolved or rejected

I think it's because of a race condition between etch's DOM updates and jasmine's asynchronous queue execution.

Because there's no complex logic around the timing of DOM updates in this package, I think it's ok to simply use synchronous DOM updates in the tests and 🔥 all of the waitsFor and runs.

/cc @as-cii @nathansobo

This does seem like a usability issue with etch. Why is waiting for etch.getScheduler().getNextUpdatePromise() not sufficient to guarantee that the DOM has updated in this case?

[maxbrunsfeld]

I also removed the conditionals guarding keyup-related tests, because atom/atom-keymap#156 is on both beta and stable.

[nathansobo]

Great idea.

[as-cii]

Because there's no complex logic around the timing of DOM updates in this package, I think it's ok to simply use synchronous DOM updates in the tests and 🔥 all of the waitsFor and runs.

👍

This does seem like a usability issue with etch. Why is waiting for etch.getScheduler().getNextUpdatePromise() not sufficient to guarantee that the DOM has updated in this case?

Yeah, it's unclear to me why that wouldn't work. Maybe it's worth taking a closer look if we can reproduce the timeouts consistently?

[maxbrunsfeld]

Maybe it's worth taking a closer look if we can reproduce the timeouts consistently?

One thing I experimented with was making requestAnimationFrame fire more slowly in the tests, e.g.

originalRAF = window.requestAnimationFrame
window.requestAnimationFrame = (callback) ->
  setTimeout((-> originalRAF(callback)), 100)

If etch was working like I'd expect, I'd think that this wouldn't cause failures, because getNextUpdatePromise would be delayed until after the delayed animation frame. But in fact, I saw that this made the failures happen more consistently.