search

atom / settings-view

🔧 Edit Atom settings
MIT License
273 stars 275 forks source link

Use a less timing-sensitive condition to hopefully fix flaky test #1125

Closed nathansobo closed 5 years ago

nathansobo commented 5 years ago

Jasmine 1.3 queueing makes async testing weird. In this case, we resolve a promise with the expectation that it asynchronously displays a notification. Then, in a waitsFor block on the next line, we subscribe to onDidAddNotification. The problem is that this waitsFor block might not even be run before the notification gets displayed. So the notification is displayed before we even subscribe to onDidAddNotification and the waitsForBlock gets stuck. This kind of thing seems especially frequent on Windows/AppVeyor.

The ideal fix would be to convert all these tests to JS and use async/await. Then we could synchronously subscribe to onDidAddNotification after resolving the promise and avoid this race. But in this case, I think using a looser condition should work as well.

nathansobo commented 5 years ago

/cc @atom/core @50Wliu because I think it's important that everyone understands this source of indeterminacy when combining Jasmine 1.3 waitsFor blocks with promises in async tests.