From 8bb08ccbb0d9f3e8b020960b509d62833554e080 Mon Sep 17 00:00:00 2001
From: Robert Jackson <rjackson@linkedin.com>
Date: Sat, 10 Feb 2018 12:13:11 -0500
Subject: [PATCH] Remove simple alias for `wait` -> `settled`.

The original goal of `settled` was to "rebrand" `wait` as `settled` without the
'baggage" (since `wait` had some legacy APIs that should not exist in
`settled`). Unfortunately, the code refactoring that I had originally done that
allowed them to be shared resulted in introducing bugs like those
reported.

This commit completely severs the implementation of `wait` from that of
`settled`, by leveraging the (IMHO very nice) layers of abstraction that we
have created since the original extraction.  Implementing `wait` with
`waitUntil` and `getSettledState` makes this quite trivial now.

tldr of changes:

* `isSettled` no longer has to support the legacy `wait` contract (this was
  undocumented private behavior _only_ kept around to support the old semantics
  of `wait`)
* `wait` is implemented as its own `waitUntil` with the same
  semantics it has had since the 0.6.x series
---
 .../@ember/test-helpers/settled.js            | 25 +----------
 addon-test-support/ember-test-helpers/wait.js | 43 ++++++++++++++++++-
 tests/integration/settled-test.js             |  9 ++++
 tests/unit/wait-test.js                       | 12 ++++++
 4 files changed, 65 insertions(+), 24 deletions(-)
 create mode 100644 tests/unit/wait-test.js

diff --git a/addon-test-support/@ember/test-helpers/settled.js b/addon-test-support/@ember/test-helpers/settled.js
index 520b5dd2b..907a11c45 100644
--- a/addon-test-support/@ember/test-helpers/settled.js
+++ b/addon-test-support/@ember/test-helpers/settled.js
@@ -162,28 +162,9 @@ export function getSettledState() {
   @returns {boolean} `true` if settled, `false` otherwise
 */
 export function isSettled() {
-  let waitForTimers = true;
-  let waitForAJAX = true;
-  let waitForWaiters = true;
-
-  if (arguments[0] !== undefined) {
-    let options = arguments[0];
-    waitForTimers = 'waitForTimers' in options ? options.waitForTimers : true;
-    waitForAJAX = 'waitForAJAX' in options ? options.waitForAJAX : true;
-    waitForWaiters = 'waitForWaiters' in options ? options.waitForWaiters : true;
-  }
-
   let { hasPendingTimers, hasRunLoop, hasPendingRequests, hasPendingWaiters } = getSettledState();
 
-  if (waitForTimers && (hasPendingTimers || hasRunLoop)) {
-    return false;
-  }
-
-  if (waitForAJAX && hasPendingRequests) {
-    return false;
-  }
-
-  if (waitForWaiters && hasPendingWaiters) {
+  if (hasPendingTimers || hasRunLoop || hasPendingRequests || hasPendingWaiters) {
     return false;
   }
 
@@ -198,7 +179,5 @@ export function isSettled() {
   @returns {Promise<void>} resolves when settled
 */
 export default function settled() {
-  let options = arguments[0];
-
-  return waitUntil(() => isSettled(options), { timeout: Infinity });
+  return waitUntil(isSettled, { timeout: Infinity });
 }
diff --git a/addon-test-support/ember-test-helpers/wait.js b/addon-test-support/ember-test-helpers/wait.js
index 132e3d7ae..17b7f6c0c 100644
--- a/addon-test-support/ember-test-helpers/wait.js
+++ b/addon-test-support/ember-test-helpers/wait.js
@@ -1,7 +1,48 @@
 export {
-  default,
   _setupAJAXHooks,
   _setupPromiseListeners,
   _teardownAJAXHooks,
   _teardownPromiseListeners,
 } from '@ember/test-helpers/settled';
+
+import { waitUntil, getSettledState } from '@ember/test-helpers';
+
+/**
+  Returns a promise that resolves when in a settled state (see `isSettled` for
+  a definition of "settled state").
+
+  @private
+  @deprecated
+  @param {Object} [options={}] the options to be used for waiting
+  @param {boolean} [options.waitForTimers=true] should timers be waited upon
+  @param {boolean} [options.waitForAjax=true] should $.ajax requests be waited upon
+  @param {boolean} [options.waitForWaiters=true] should test waiters be waited upon
+  @returns {Promise<void>} resolves when settled
+*/
+export default function wait(options = {}) {
+  if (typeof options !== 'object' || options === null) {
+    options = {};
+  }
+
+  return waitUntil(() => {
+    let waitForTimers = 'waitForTimers' in options ? options.waitForTimers : true;
+    let waitForAJAX = 'waitForAJAX' in options ? options.waitForAJAX : true;
+    let waitForWaiters = 'waitForWaiters' in options ? options.waitForWaiters : true;
+
+    let { hasPendingTimers, hasRunLoop, hasPendingRequests, hasPendingWaiters } = getSettledState();
+
+    if (waitForTimers && (hasPendingTimers || hasRunLoop)) {
+      return false;
+    }
+
+    if (waitForAJAX && hasPendingRequests) {
+      return false;
+    }
+
+    if (waitForWaiters && hasPendingWaiters) {
+      return false;
+    }
+
+    return true;
+  });
+}
diff --git a/tests/integration/settled-test.js b/tests/integration/settled-test.js
index 2d15e9b2a..479615177 100644
--- a/tests/integration/settled-test.js
+++ b/tests/integration/settled-test.js
@@ -154,6 +154,15 @@ module('settled real-world scenarios', function(hooks) {
     assert.equal(this.element.textContent, 'async value');
   });
 
+  test('does not error for various argument types', async function(assert) {
+    assert.expect(0); // no assertions, just shouldn't error
+
+    await settled(3000);
+    await settled(null);
+    await settled(undefined);
+    await settled();
+  });
+
   test('it works when async exists in an event/action', async function(assert) {
     this.owner.register('component:x-test-2', TestComponent2);
 
diff --git a/tests/unit/wait-test.js b/tests/unit/wait-test.js
new file mode 100644
index 000000000..40e955057
--- /dev/null
+++ b/tests/unit/wait-test.js
@@ -0,0 +1,12 @@
+import { module, test } from 'qunit';
+import wait from 'ember-test-helpers/wait';
+
+module('wait: unit tests', function() {
+  test('issues a helpful assertion for invalid arguments', async function(assert) {
+    assert.expect(0); // no assertions, just shouldn't error
+
+    await wait(3000);
+    await wait(null);
+    await wait();
+  });
+});