Adding constraints around naming test waiters

README.md

@@ -25,6 +25,7 @@ This addon implements the design specified in [RFC 581](https://github.com/ember
 - [Installation](#installation)
 - [Quickstart](#quickstart)
   - [buildWaiter function](#buildwaiter-function)
+    - [Waiter naming conventions](#waiter-naming-conventions)
   - [waitForPromise function](#waitforpromise-function)
   - [Waiting on all waiters](#waiting-on-all-waiters)
 - [General Design](#general-design)
@@ -59,17 +60,31 @@ that provides a number of methods. The key methods that allow you to control asy
 a pair to _begin_ waiting and _end_ waiting respectively. The `beginAsync` method returns a `token`, which uniquely identifies that async operation. To mark the
 async operation as complete, call `endAsync`, passing in the `token` that was returned from the prior `beginAsync` call.
 
+#### Waiter naming conventions
+
+When building your waiter, you should ensure you use a meaningful name. At a minimum, your name needs to be constructed of a `namespace`, and optionally a `descriptor` in the format `namespace[:descriptor]`. Suggestions for naming conventions are as follows:
+
+For apps:
+
+1. `file-name` - if your file has only one waiter, the file name becomes the namespace
+1. `file-name:waiter-1`, `file-name:waiter-2`, ... - if your file has more than one waiter, the file name becomes the namespace and we add descriptors
+
+For addons:
+
+1. `addon-name` - if your addon has only one waiter, the addon name becomes the namespace
+1. `addon-name:waiter-1`, `addon-name:waiter-2`, ... - if your addon has more than one waiter, the addon name becomes the namespace and we add descriptors
+
 ```js
 import Component from '@ember/component';
 import { buildWaiter } from 'ember-test-waiters';
 
-let waiter = buildWaiter('friend-waiter');
+let waiter = buildWaiter('ember-friendz:friend-waiter');
 
 export default class Friendz extends Component {
   didInsertElement() {
     let token = waiter.beginAsync();
 
-    someAsyncWork()
+    makeFriendz()
       .then(() => {
         //... some work
       })
@@ -91,8 +106,8 @@ import { waitForPromise } from 'ember-test-waiters';
 
 export default class MoreFriendz extends Component {
   didInsertElement() {
-    waitForPromise(someAsyncWork).then(() => {
-      doOtherThings();
+    waitForPromise(makeFriendz).then(() => {
+      return goForDrinks();
     });
   }
 }

addon/build-waiter.ts

@@ -3,6 +3,14 @@ import { Primitive, TestWaiter, TestWaiterDebugInfo, WaiterName } from 'ember-te
 import { DEBUG } from '@glimmer/env';
 import Token from './token';
 import { register } from './waiter-manager';
+import { warn } from '@ember/debug';
+
+const WAITER_NAME_PATTERN = /^[^:]*:?.*/;
+let WAITER_NAMES = DEBUG ? new Set() : undefined;
+
+export function _resetWaiterNames() {
+  WAITER_NAMES = new Set();
+}
 
 function getNextToken(): Token {
   return new Token();
@@ -135,7 +143,25 @@ class NoopTestWaiter implements TestWaiter {
  */
 export default function buildWaiter(name: string): TestWaiter {
   if (DEBUG) {
-    return new TestWaiterImpl(name);
+    warn(`The waiter name '${name}' is already in use`, !WAITER_NAMES!.has(name), {
+      id: 'ember-test-waiters.duplicate-waiter-name',
+    });
+    WAITER_NAMES!.add(name);
+  }
+
+  if (!DEBUG) {
+    return new NoopTestWaiter(name);
   }
-  return new NoopTestWaiter(name);
+
+  warn(
+    `You must provide a name that contains a descriptive prefix separated by a colon.
+
+      Example: ember-fictitious-addon:some-file
+
+      You passed: ${name}`,
+    WAITER_NAME_PATTERN.test(name),
+    { id: 'ember-test-waiters.invalid-waiter-name' }
+  );
+
+  return new TestWaiterImpl(name);
 }

addon/index.ts

@@ -17,5 +17,5 @@ export {
   hasPendingWaiters,
 } from './waiter-manager';
 
-export { default as buildWaiter } from './build-waiter';
+export { default as buildWaiter, _resetWaiterNames } from './build-waiter';
 export { default as waitForPromise } from './wait-for-promise';

addon/wait-for-promise.ts

@@ -2,7 +2,7 @@ import { DEBUG } from '@glimmer/env';
 import RSVP from 'rsvp';
 import buildWaiter from './build-waiter';
 
-const PROMISE_WAITER = buildWaiter('promise-waiter');
+const PROMISE_WAITER = buildWaiter('ember-test-waiters:promise-waiter');
 
 type PromiseType<T> = Promise<T> | RSVP.Promise<T>;
 

tests/unit/build-waiter-test.ts

@@ -1,41 +1,68 @@
 import MockStableError, { overrideError, resetError } from './utils/mock-stable-error';
-import { _reset, buildWaiter, getPendingWaiterState, getWaiters } from 'ember-test-waiters';
+import {
+  _reset,
+  _resetWaiterNames,
+  buildWaiter,
+  getPendingWaiterState,
+  getWaiters,
+} from 'ember-test-waiters';
 import { module, test } from 'qunit';
 
 import { Promise } from 'rsvp';
 import Token from 'ember-test-waiters/token';
+import { registerWarnHandler } from '@ember/debug';
 
-module('test-waiter', function(hooks) {
+module('build-waiter', function(hooks) {
   hooks.afterEach(function() {
     _reset();
+    _resetWaiterNames();
     resetError();
+    registerWarnHandler(() => {});
   });
 
-  test('test waiter can be instantiated with a name', function(assert) {
-    let name = 'my-waiter';
+  test('test waiter can be instantiated with a namespace only', function(assert) {
+    let name = 'my-addon';
     let waiter = buildWaiter(name);
 
     assert.equal(waiter.name, name);
   });
 
+  test('test waiter can be instantiated with a namespace and descriptor', function(assert) {
+    let name = 'my-addon:my-waiter';
+    let waiter = buildWaiter(name);
+
+    assert.equal(waiter.name, name);
+  });
+
+  test('test waiters will warn when waiter name is used more than once', function(assert) {
+    registerWarnHandler((message, options) => {
+      console.log('message', message);
+      assert.equal(message, "The waiter name 'ember-test-waiters:first' is already in use");
+      assert.equal(options.id, 'ember-test-waiters.duplicate-waiter-name');
+    });
+
+    buildWaiter('ember-test-waiters:first');
+    buildWaiter('ember-test-waiters:first');
+  });
+
   test('test waiters return a token from beginAsync when no token provided', function(assert) {
-    let waiter = buildWaiter('my-waiter');
+    let waiter = buildWaiter('my-addon:my-waiter');
 
     let token = waiter.beginAsync();
 
     assert.ok(token instanceof Token, 'A token was returned from beginAsync');
   });
 
   test('test waiters return a truthy token from beginAsync when no token provided', function(assert) {
-    let waiter = buildWaiter('my-waiter');
+    let waiter = buildWaiter('my-addon:my-waiter');
 
     let token = waiter.beginAsync();
 
     assert.ok(token, 'A token was returned from beginAsync and is truthy');
   });
 
   test('test waiters automatically register when beginAsync is invoked when no token provided', function(assert) {
-    let waiter = buildWaiter('my-waiter');
+    let waiter = buildWaiter('my-addon:my-waiter');
 
     let token = waiter.beginAsync();
 
@@ -50,7 +77,7 @@ module('test-waiter', function(hooks) {
   });
 
   test('test waiters automatically register when beginAsync is invoked using a custom token', function(assert) {
-    let waiter = buildWaiter('my-waiter');
+    let waiter = buildWaiter('ember-test-waiters:my-waiter');
     let waiterItem = {};
 
     waiter.beginAsync(waiterItem);
@@ -66,7 +93,7 @@ module('test-waiter', function(hooks) {
   });
 
   test('test waiters removes item from items map when endAsync is invoked', function(assert) {
-    let waiter = buildWaiter('my-waiter');
+    let waiter = buildWaiter('ember-test-waiters:my-waiter');
 
     let token = waiter.beginAsync();
     waiter.endAsync(token);
@@ -76,7 +103,7 @@ module('test-waiter', function(hooks) {
   });
 
   test('test waiters removes item from items map when endAsync is invoked using a custom token', function(assert) {
-    let waiter = buildWaiter('my-waiter');
+    let waiter = buildWaiter('ember-test-waiters:my-waiter');
     let waiterItem = {};
 
     waiter.beginAsync(waiterItem);
@@ -87,7 +114,7 @@ module('test-waiter', function(hooks) {
   });
 
   test('beginAsync will throw if a prior call to beginAsync with the same token occurred', function(assert) {
-    let waiter = buildWaiter('my-waiter');
+    let waiter = buildWaiter('ember-test-waiters:my-waiter');
 
     assert.throws(
       () => {
@@ -100,7 +127,7 @@ module('test-waiter', function(hooks) {
   });
 
   test('beginAsync will throw if a prior call to beginAsync with the same token occurred', function(assert) {
-    let waiter = buildWaiter('my-waiter');
+    let waiter = buildWaiter('ember-test-waiters:my-waiter');
     let token = {};
 
     assert.throws(
@@ -114,7 +141,7 @@ module('test-waiter', function(hooks) {
   });
 
   test('endAsync will throw if a prior call to beginAsync with the same token did not occur', function(assert) {
-    let waiter = buildWaiter('my-waiter');
+    let waiter = buildWaiter('ember-test-waiters:my-waiter');
     let token = 0;
 
     assert.throws(
@@ -127,7 +154,7 @@ module('test-waiter', function(hooks) {
   });
 
   test('endAsync will throw if a prior call to beginAsync with the same token did not occur using custom token', function(assert) {
-    let waiter = buildWaiter('my-waiter');
+    let waiter = buildWaiter('ember-test-waiters:my-waiter');
     let waiterItem = {};
 
     assert.throws(
@@ -142,7 +169,7 @@ module('test-waiter', function(hooks) {
   test('endAsync will not throw if endAsync called twice in a row with the same token', function(assert) {
     assert.expect(0);
 
-    let waiter = buildWaiter('my-waiter');
+    let waiter = buildWaiter('ember-test-waiters:my-waiter');
     let token = waiter.beginAsync();
 
     waiter.endAsync(token);
@@ -152,7 +179,7 @@ module('test-waiter', function(hooks) {
   test('endAsync will not throw if endAsync called twice in a row with the same token using custom token', function(assert) {
     assert.expect(0);
 
-    let waiter = buildWaiter('my-waiter');
+    let waiter = buildWaiter('ember-test-waiters:my-waiter');
     let waiterItem = {};
 
     waiter.beginAsync(waiterItem);
@@ -161,7 +188,7 @@ module('test-waiter', function(hooks) {
   });
 
   test('waitUntil returns the correct value if the waiter should wait', function(assert) {
-    let waiter = buildWaiter('my-waiter');
+    let waiter = buildWaiter('ember-test-waiters:my-waiter');
     let waiterItem = {};
 
     assert.ok(waiter.waitUntil(), 'waitUntil returns true');
@@ -176,7 +203,7 @@ module('test-waiter', function(hooks) {
   });
 
   test('waiter contains debug info for a waiter item', function(assert) {
-    let waiter = buildWaiter('my-waiter');
+    let waiter = buildWaiter('ember-test-waiters:my-waiter');
     let waiterItem = {};
 
     overrideError(MockStableError);
@@ -187,7 +214,7 @@ module('test-waiter', function(hooks) {
   });
 
   test('waiter executes beginAsync and endAsync at the correct times in relation to thenables', async function(assert) {
-    const promiseWaiter = buildWaiter('promise-waiter');
+    const promiseWaiter = buildWaiter('ember-test-waiters:promise-waiter');
     function waitForPromise<T>(promise: Promise<T>, label?: string) {
       let result = promise;
 
@@ -221,7 +248,7 @@ module('test-waiter', function(hooks) {
     assert.deepEqual(getPendingWaiterState(), {
       pending: 1,
       waiters: {
-        'promise-waiter': [
+        'ember-test-waiters:promise-waiter': [
           {
             label: undefined,
             stack: 'STACK',
@@ -249,7 +276,7 @@ module('test-waiter', function(hooks) {
   });
 
   test('waiter can clear items', function(assert) {
-    let waiter = buildWaiter('my-waiter');
+    let waiter = buildWaiter('ember-test-waiters:my-waiter');
 
     waiter.beginAsync();
 

tests/unit/wait-for-promise-test.ts

@@ -30,7 +30,7 @@ if (DEBUG) {
         assert.deepEqual(getPendingWaiterState(), {
           pending: 1,
           waiters: {
-            'promise-waiter': [
+            'ember-test-waiters:promise-waiter': [
               {
                 label: undefined,
                 stack: 'STACK',
@@ -87,7 +87,7 @@ if (DEBUG) {
         assert.deepEqual(getPendingWaiterState(), {
           pending: 1,
           waiters: {
-            'promise-waiter': [
+            'ember-test-waiters:promise-waiter': [
               {
                 label: undefined,
                 stack: 'STACK',

tests/unit/waiter-manager-noop-test.ts

@@ -4,7 +4,7 @@ import { module, test } from 'qunit';
 import { DEBUG } from '@glimmer/env';
 
 if (!DEBUG) {
-  module('test-waiter | DEBUG: false', function(hooks) {
+  module('waiter-manager-noop | DEBUG: false', function(hooks) {
     hooks.afterEach(function() {
       _reset();
     });
@@ -20,16 +20,16 @@ if (!DEBUG) {
     });
 
     test('register will correctly add a waiter', function(assert) {
-      let waiter = buildWaiter('first');
+      let waiter = buildWaiter('ember-test-waiters:first');
 
       register(waiter);
 
       let waiters = getWaiters().map(w => w.name);
-      assert.deepEqual(waiters, ['first']);
+      assert.deepEqual(waiters, ['ember-test-waiters:first']);
     });
 
     test('a NoopTestWaiter always returns true from waitUntil', function(assert) {
-      let waiter = buildWaiter('first');
+      let waiter = buildWaiter('ember-test-waiters:first');
 
       assert.ok(waiter.waitUntil(), 'waitUntil returns true');
       let token = waiter.beginAsync();
@@ -39,7 +39,7 @@ if (!DEBUG) {
     });
 
     test('a NoopTestWaiter always returns true from waitUntil', function(assert) {
-      let waiter = buildWaiter('first');
+      let waiter = buildWaiter('ember-test-waiters:first');
       let waiterItem = {};
 
       assert.ok(waiter.waitUntil(), 'waitUntil returns true');