From 760debe6d43a6e55fc92c2ecd0b96eb573603e8c Mon Sep 17 00:00:00 2001
From: Robert Jackson <rjackson@linkedin.com>
Date: Tue, 13 Jun 2017 22:51:09 -0400
Subject: [PATCH 01/12] Add new QUnit testing API RFC.

---
 text/0230-simplify-qunit-testing-api.md | 383 ++++++++++++++++++++++++
 1 file changed, 383 insertions(+)
 create mode 100644 text/0230-simplify-qunit-testing-api.md

diff --git a/text/0230-simplify-qunit-testing-api.md b/text/0230-simplify-qunit-testing-api.md
new file mode 100644
index 0000000000..f3408309df
--- /dev/null
+++ b/text/0230-simplify-qunit-testing-api.md
@@ -0,0 +1,383 @@
+- Start Date: 2017-06-13
+- RFC PR: [emberjs/rfcs#232](https://github.com/emberjs/rfcs/pull/232)
+- Ember Issue: (leave this empty)
+
+# Summary
+
+In order to embrace newer features being added by QUnit (our chosen default
+testing framework), we need to reduce the brittle coupling between `ember-qunit`
+and QUnit itself. 
+
+This RFC proposes a new testing syntax, that will expose QUnit API directly while
+also making tests much easier to understand. 
+
+# Motivation
+
+QUnit feature development has been accelerating since the ramp up to QUnit 2.0.
+A number of new features have been added that make testing our applications
+much easier, but the current structure of `ember-qunit` impedes our ability 
+to take advantage of some of these features.
+
+Developers are often confused by our `moduleFor*` APIs, questions like these
+are very common: 
+
+* What "magic" is `ember-qunit` doing? 
+* Where are the lines between QUnit and ember-qunit? 
+* How can I use QUnit for plain JS objects? 
+
+The way that `ember-qunit` uses to wrap QUnit functionality makes the division
+of responsiblity much harder to understand, and leads folks to believe that there
+is much more going on in `ember-qunit` than there is. It should be much clearer
+what `ember-qunit` is responsible for and what we rely on QUnit for.
+
+When this RFC has been implemented and rolled out, these questions should all be
+addressed and our testing system will both: embrace QUnit much more **and** 
+be much more framework agnostic. Sounds like a neat trick, huh?
+
+# Detailed design
+
+The primary change being proposed in this RFC is to migrate to using the QUnit
+nested module syntax, and update our custom setup/teardown into a more functional
+API.
+
+Lets look at a basic example:
+
+```js
+// **** before ****
+
+import { moduleForComponent, test } from 'ember-qunit';
+import hbs from 'htmlbars-inline-precompile';
+
+moduleForComponent('x-foo', {
+  integration: true
+});
+
+test('renders', function(assert) {
+  assert.expect(1);
+
+  this.render(hbs`{{pretty-color name="red"}}`);
+
+  assert.equal(this.$('.color-name').text(), 'red');
+});
+
+// **** after ****
+
+import { module, test } from 'qunit';
+import { setupRenderingTest } from 'ember-qunit';
+import hbs from 'htmlbars-inline-precompile';
+
+module('x-foo', function(hooks) {
+  setupRenderingTest(hooks);
+
+  test('renders', async function(assert) {
+    assert.expect(1);
+
+    await this.render(hbs`{{pretty-color name="red"}}`);
+
+    assert.equal(this.$('.color-name').text(), 'red');
+  });
+});
+```
+
+As you can see, this proposal leverages QUnit's nested module API in a way that
+makes it much clearer what is going on. It is quite obvious what QUnit is doing
+(acting like a general testing framework) and what `ember-qunit` is doing 
+(setting up rendering functionality).
+
+This API was heavily influenced by the work that 
+[Tobias Bieniek](https://github.com/Turbo87) did in 
+[emberjs/ember-mocha#84](https://github.com/emberjs/ember-mocha/pull/84).
+
+## QUnit Nested Modules API
+
+Even though it is not a proposal of this RFC, the QUnit nested module
+syntax may seem foreign to some folks so lets briefly review.
+
+With nested modules, a normal 1.x QUnit module setup changes from:
+
+```js
+QUnit.module('some description', {
+  before() {},
+  beforeEach() {},
+  afterEach() {},
+  after() {}
+});
+
+QUnit.test('it blends', function(assert) {
+  assert.ok(true, 'of course!');
+});
+```
+
+Into:
+
+```js
+QUnit.module('some description', function(hooks) {
+
+  hooks.before(() => {});
+  hooks.beforeEach(() => {});
+  hooks.afterEach(() => {});
+  hooks.after(() => {});
+
+  QUnit.test('it blends', function(assert) {
+    assert.ok(true, 'of course!');
+  });
+});
+```
+
+This makes it much simpler to support multiple `before`, `beforeEach`, `afterEach`,
+and `after` callbacks, and it also allows for arbitrary nesting of modules (though
+this RFC is not advocating for arbitrary levels of nesting).
+
+You can read more about QUnit nested modules
+[here](http://api.qunitjs.com/QUnit/module#nested-module-nested-hooks-). The new APIs
+proposed in this RFC expect to be leveraging nested modules. 
+
+## New APIs
+
+The following new methods will be exposed from `ember-qunit`:
+
+```ts
+interface QUnitModuleHooks {
+  before(callback: Function): void;
+  beforeEach(callback: Function): void;
+  afterEach(callback: Function): void;
+  after(callback: Function): void;
+}
+
+declare module 'ember-qunit' {
+  // ...snip... 
+  export function setupTest(hooks: QUnitModuleHooks): void;
+  export function setupRenderingTest(hooks: QUnitModuleHooks): void;
+}
+```
+
+### `setupTest`
+
+This function will:
+
+* invoking `ember-test-helper`s `setContext` with the tests context
+* create an owner object and set it on the test context (e.g. `this.owner`)
+* setup `this.set`, `this.setProperties`, `this.get`, and `this.getProperties` to
+  the test context
+* setup `this.pauseTest` and `this.resumeTest` methods to allow easy pausing/resuming
+  of tests
+
+### `setupRenderingTest`
+
+This function will:
+
+* run the `setupTest` implementation
+* setup `this.$` method to run jQuery selectors rooted to the testing container 
+* setup a getter for `this.element` which returns the DOM element representing
+  the element that was rendered via `this.render`
+* setup Ember's renderer and create a `this.render` method which accepts a 
+  compiled template to render and returns a promise which resolves once rendering
+  is completed
+* setup `this.clearRender` method which clears any previously rendered DOM (
+  also used during cleanup)
+
+When invoked, `this.render` will render the provided template and return a
+promise that resolves when rendering is completed.
+
+
+## Migration Examples
+
+The migration can likely be largely automated (following the 
+[excellent codemod](https://github.com/Turbo87/ember-mocha-codemods) that 
+[Tobias Bieniek](https://github.com/turbo87) wrote for a similar `ember-mocha`
+the transition), for folks but it is still useful to review concrete scenarios
+of tests before and after this RFC is implemented.
+
+### Component / Helper Integration Test
+
+```js
+// **** before ****
+
+import { moduleForComponent, test } from 'ember-qunit';
+import hbs from 'htmlbars-inline-precompile';
+
+moduleForComponent('x-foo', {
+  integration: true
+});
+
+test('renders', function(assert) {
+  assert.expect(1);
+
+  this.render(hbs`{{pretty-color name="red"}}`);
+
+  assert.equal(this.$('.color-name').text(), 'red');
+});
+
+// **** after ****
+
+import { module, test } from 'qunit';
+import { setupRenderingTest } from 'ember-qunit';
+import hbs from 'htmlbars-inline-precompile';
+
+module('x-foo', function(hooks) {
+  setupRenderingTest(hooks);
+
+  test('renders', async function(assert) {
+    assert.expect(1);
+
+    await this.render(hbs`{{pretty-color name="red"}}`);
+
+    assert.equal(this.$('.color-name').text(), 'red');
+  });
+});
+```
+
+### Component Unit Test
+
+```js
+// **** before ****
+
+import { moduleForComponent, test } from 'ember-qunit';
+
+moduleForComponent('x-foo', {
+  unit: true
+});
+
+test('computes properly', function(assert) {
+  assert.expect(1);
+
+  let subject = this.subject({
+    name: 'something'
+  });
+
+  let result = subject.get('someCP');
+  assert.equal(result, 'expected value');
+});
+
+// **** after ****
+
+import { module, test } from 'qunit';
+import { setupTest } from 'ember-qunit';
+
+module('x-foo', function(hooks) {
+  setupTest(hooks);
+
+  test('computed properly', function(assert) {
+    assert.expect(1);
+
+    let Factory = this.owner.factoryFor('component:x-foo');
+    let subject = Factory.create({
+      name: 'something'
+    });
+
+    let result = subject.get('someCP');
+    assert.equal(result, 'expected value');
+  });
+});
+```
+
+### Service/Route/Controller Test
+
+```js
+// **** before ****
+
+import { moduleFor, test } from 'ember-qunit';
+
+moduleFor('service:flash', 'Unit | Service | Flash', {
+  unit: true
+});
+
+test('should allow messages to be queued', function (assert) {
+  assert.expect(4);
+  
+  let subject = this.subject();
+  
+  subject.show('some message here');
+  
+  let messages = subject.messages;
+  
+  assert.deepEqual(messages, [
+    'some message here'
+  ]);
+});
+
+// **** after ****
+
+import { module, test } from 'qunit';
+import { setupTest } from 'ember-qunit';
+
+module('Unit | Service | Flash', function(hooks) {
+  setupTest(hooks);
+  
+  test('should allow messages to be queued', function (assert) {
+    assert.expect(4);
+  
+    let subject = this.owner.lookup('service:flash');
+  
+    subject.show('some message here');
+  
+    let messages = subject.messages;
+  
+    assert.deepEqual(messages, [
+      'some message here'
+    ]);
+  });
+});
+
+```
+
+## Ecosystem Updates
+
+The blueprints in all official projects (and any provided by popular addons) 
+will need to be updated to detect `ember-qunit` version and emit the correct
+output.
+
+This includes:
+
+* ember-source
+* ember-data
+* ember-cli-legacy-blueprints
+* others?
+
+## Update Guides
+
+The guides includes a section for testing, this section needs to be reviewed
+and revamped to match the proposal here.
+
+## Deprecate older APIs
+
+Once this RFC is implemented, the older APIs will be deprecated and retained 
+for a full LTS cycle (assuming speedy landing, this would mean the older APIs 
+would be deprecated around Ember 2.20). After that timeframe, the older APIs
+will be removed from `ember-qunit` and `ember-test-helpers` and they will
+release with SemVer major version bumps.
+
+Note that while the older `moduleFor` and `moduleForComponent` APIs will be
+deprecated, they will still be possible to use since the host application can
+pin to a version of `ember-qunit` / `ember-test-helpers` that support its own
+usage. This is a large benefit of migrating these testing features away from 
+`Ember`'s internals, and into the addon space.
+
+## Relationship to "Grant Testing Unification"
+
+This RFC is a small stepping stone towards the future where all types of tests
+share a similar API. The API proposed here is much easier to extend to provide
+the functionality that is required for [emberjs/rfcs#119](https://github.com/emberjs/rfcs/pull/119).
+
+# How We Teach This
+
+This change requires updates to the API documentation of `ember-qunit` and the
+main Ember guides' testing section. The changes are largely intended to reduce
+confusion, making it easier to teach and understand testing in Ember.
+
+# Drawbacks
+
+## Churn
+
+As mentioned in [emberjs/rfcs#229](https://github.com/emberjs/rfcs/pull/229), test
+related churn is quite painful and annoying. In order to maintain the general
+goodwill of folks, we must ensure that we avoid needless churn. 
+
+This RFC should be implemented in conjunction with 
+[emberjs/rfcs#229](https://github.com/emberjs/rfcs/pull/229) so that we can avoid
+multiple back to back changes in the blueprints.
+
+# Alternatives
+
+The simplest alternative is to do nothing. This would loose all of the positive
+benefits mentioned in this RFC, but should still be considered a possibility...

From 98c3e7bf5e1732cb5faf7289a7cb41002f2c7be8 Mon Sep 17 00:00:00 2001
From: Robert Jackson <rjackson@linkedin.com>
Date: Wed, 14 Jun 2017 00:00:14 -0400
Subject: [PATCH 02/12] Fix typo in section title.

---
 text/0230-simplify-qunit-testing-api.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/text/0230-simplify-qunit-testing-api.md b/text/0230-simplify-qunit-testing-api.md
index f3408309df..e1baa7b517 100644
--- a/text/0230-simplify-qunit-testing-api.md
+++ b/text/0230-simplify-qunit-testing-api.md
@@ -353,7 +353,7 @@ pin to a version of `ember-qunit` / `ember-test-helpers` that support its own
 usage. This is a large benefit of migrating these testing features away from 
 `Ember`'s internals, and into the addon space.
 
-## Relationship to "Grant Testing Unification"
+## Relationship to "Grand Testing Unification"
 
 This RFC is a small stepping stone towards the future where all types of tests
 share a similar API. The API proposed here is much easier to extend to provide

From 9fbe9ac129750e7b22d12ebff255edb6fb5b6321 Mon Sep 17 00:00:00 2001
From: Robert Jackson <rjackson@linkedin.com>
Date: Wed, 14 Jun 2017 00:09:03 -0400
Subject: [PATCH 03/12] Add drawback for pending QUnit release.

---
 text/0230-simplify-qunit-testing-api.md | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/text/0230-simplify-qunit-testing-api.md b/text/0230-simplify-qunit-testing-api.md
index e1baa7b517..d8e8e4aee8 100644
--- a/text/0230-simplify-qunit-testing-api.md
+++ b/text/0230-simplify-qunit-testing-api.md
@@ -377,6 +377,18 @@ This RFC should be implemented in conjunction with
 [emberjs/rfcs#229](https://github.com/emberjs/rfcs/pull/229) so that we can avoid
 multiple back to back changes in the blueprints.
 
+## [qunitjs/qunit#977](https://github.com/qunitjs/qunit/issues/977)
+
+Until very recently, the QUnit nested module API was only able to allow a single
+callback for each of the hooks per-nesting level. This means that the proposal in
+this RFC (which requires the hooks to be setup by `ember-qunit`) would disallow
+user-land `beforeEach`/`afterEach` hooks to be setup.
+
+The work around is "simple" (if somewhat annoying), which is to "just nest another
+level". The good news is that [Trent Willis](https://github.com/trentwillis) fixed
+the underlying problem in [qunitjs/qunit#1188](https://github.com/qunitjs/qunit/pull/1188),
+which should be released as 2.3.4 well before this RFC is merged.
+
 # Alternatives
 
 The simplest alternative is to do nothing. This would loose all of the positive

From 822566b4f94977652b85054ab85fd82fc0e591af Mon Sep 17 00:00:00 2001
From: Robert Jackson <rjackson@linkedin.com>
Date: Wed, 14 Jun 2017 09:39:35 -0400
Subject: [PATCH 04/12] Fixup some bad prose.

---
 text/0230-simplify-qunit-testing-api.md | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/text/0230-simplify-qunit-testing-api.md b/text/0230-simplify-qunit-testing-api.md
index d8e8e4aee8..42006cfa65 100644
--- a/text/0230-simplify-qunit-testing-api.md
+++ b/text/0230-simplify-qunit-testing-api.md
@@ -25,7 +25,7 @@ are very common:
 * Where are the lines between QUnit and ember-qunit? 
 * How can I use QUnit for plain JS objects? 
 
-The way that `ember-qunit` uses to wrap QUnit functionality makes the division
+The way that `ember-qunit` wraps QUnit functionality makes the division
 of responsiblity much harder to understand, and leads folks to believe that there
 is much more going on in `ember-qunit` than there is. It should be much clearer
 what `ember-qunit` is responsible for and what we rely on QUnit for.
@@ -155,7 +155,7 @@ declare module 'ember-qunit' {
 
 This function will:
 
-* invoking `ember-test-helper`s `setContext` with the tests context
+* invoke `ember-test-helper`s `setContext` with the tests context
 * create an owner object and set it on the test context (e.g. `this.owner`)
 * setup `this.set`, `this.setProperties`, `this.get`, and `this.getProperties` to
   the test context
@@ -185,7 +185,7 @@ promise that resolves when rendering is completed.
 The migration can likely be largely automated (following the 
 [excellent codemod](https://github.com/Turbo87/ember-mocha-codemods) that 
 [Tobias Bieniek](https://github.com/turbo87) wrote for a similar `ember-mocha`
-the transition), for folks but it is still useful to review concrete scenarios
+the transition), but it is still useful to review concrete scenarios
 of tests before and after this RFC is implemented.
 
 ### Component / Helper Integration Test

From 46fd4a791e282c929f35daeac8baa14323848750 Mon Sep 17 00:00:00 2001
From: Robert Jackson <rjackson@linkedin.com>
Date: Wed, 14 Jun 2017 09:50:35 -0400
Subject: [PATCH 05/12] Mention that ember-mocha detects version and uses
 correct blueprints.

---
 text/0230-simplify-qunit-testing-api.md | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/text/0230-simplify-qunit-testing-api.md b/text/0230-simplify-qunit-testing-api.md
index 42006cfa65..6a8eb763fd 100644
--- a/text/0230-simplify-qunit-testing-api.md
+++ b/text/0230-simplify-qunit-testing-api.md
@@ -334,6 +334,9 @@ This includes:
 * ember-cli-legacy-blueprints
 * others?
 
+This exact process was done for `ember-mocha`'s migration, making this a well
+trodden path.
+
 ## Update Guides
 
 The guides includes a section for testing, this section needs to be reviewed

From 9f79f620e56b734a96b1e4c220a588bcc89c1013 Mon Sep 17 00:00:00 2001
From: Robert Jackson <rjackson@linkedin.com>
Date: Wed, 14 Jun 2017 10:07:31 -0400
Subject: [PATCH 06/12] Add section specifically calling out significant
 divergences.

---
 text/0230-simplify-qunit-testing-api.md | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/text/0230-simplify-qunit-testing-api.md b/text/0230-simplify-qunit-testing-api.md
index 6a8eb763fd..49909ee7f5 100644
--- a/text/0230-simplify-qunit-testing-api.md
+++ b/text/0230-simplify-qunit-testing-api.md
@@ -179,6 +179,22 @@ This function will:
 When invoked, `this.render` will render the provided template and return a
 promise that resolves when rendering is completed.
 
+## Changes from Current System
+
+Here is a brief list of the more important but possibly understated changes
+being proposed here:
+
+* `this.subject` is removed in favor of using the standard public API for looking up
+  and creating instances (`this.owner.lookup` and `this.owner.factoryFor`)
+* `this.render` will begin being asynchronous to allow for further iteration in the
+  underlying rendering engines ability to speed up render times (by yielding back
+  to the browser and not blocking the main thread)
+* `this.pauseTest` and `this.resumeTest` are being added
+* `this.element` is being introduced as a public API for DOM assertions in a jQuery-less
+  environment
+* QUnit nested modules are required
+
+These changes generally do not affect our ability to write a codemod to aide in the migration.
 
 ## Migration Examples
 

From bbd5fc624a373b487ed81d53852268ffe7287550 Mon Sep 17 00:00:00 2001
From: Robert Jackson <rjackson@linkedin.com>
Date: Wed, 14 Jun 2017 10:16:01 -0400
Subject: [PATCH 07/12] Remove wishy washy language around nested modules.

---
 text/0230-simplify-qunit-testing-api.md | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/text/0230-simplify-qunit-testing-api.md b/text/0230-simplify-qunit-testing-api.md
index 49909ee7f5..1d7e871a7e 100644
--- a/text/0230-simplify-qunit-testing-api.md
+++ b/text/0230-simplify-qunit-testing-api.md
@@ -125,8 +125,7 @@ QUnit.module('some description', function(hooks) {
 ```
 
 This makes it much simpler to support multiple `before`, `beforeEach`, `afterEach`,
-and `after` callbacks, and it also allows for arbitrary nesting of modules (though
-this RFC is not advocating for arbitrary levels of nesting).
+and `after` callbacks, and it also allows for arbitrary nesting of modules.
 
 You can read more about QUnit nested modules
 [here](http://api.qunitjs.com/QUnit/module#nested-module-nested-hooks-). The new APIs

From 8acc518428301bfa7bc1af4116c983ffdb654f4e Mon Sep 17 00:00:00 2001
From: Robert Jackson <rjackson@linkedin.com>
Date: Wed, 14 Jun 2017 10:16:19 -0400
Subject: [PATCH 08/12] Add mention that setup methods do not take object name.

---
 text/0230-simplify-qunit-testing-api.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/text/0230-simplify-qunit-testing-api.md b/text/0230-simplify-qunit-testing-api.md
index 1d7e871a7e..ab02ceedb0 100644
--- a/text/0230-simplify-qunit-testing-api.md
+++ b/text/0230-simplify-qunit-testing-api.md
@@ -183,6 +183,7 @@ promise that resolves when rendering is completed.
 Here is a brief list of the more important but possibly understated changes
 being proposed here:
 
+* the various setup methods no longer need to know the name of the object under test
 * `this.subject` is removed in favor of using the standard public API for looking up
   and creating instances (`this.owner.lookup` and `this.owner.factoryFor`)
 * `this.render` will begin being asynchronous to allow for further iteration in the

From 5ecf17018ea98d78cd1f10d21b953cd01a775688 Mon Sep 17 00:00:00 2001
From: Robert Jackson <rjackson@linkedin.com>
Date: Wed, 14 Jun 2017 10:22:19 -0400
Subject: [PATCH 09/12] Update motivation to mention embracing owner API.

---
 text/0230-simplify-qunit-testing-api.md | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/text/0230-simplify-qunit-testing-api.md b/text/0230-simplify-qunit-testing-api.md
index ab02ceedb0..3b941c5b6e 100644
--- a/text/0230-simplify-qunit-testing-api.md
+++ b/text/0230-simplify-qunit-testing-api.md
@@ -30,9 +30,18 @@ of responsiblity much harder to understand, and leads folks to believe that ther
 is much more going on in `ember-qunit` than there is. It should be much clearer
 what `ember-qunit` is responsible for and what we rely on QUnit for.
 
+This RFC also aims to remove a number of custom testing only APIs that exist today
+(largely because the container/registry system was completely private when the
+current tools were authored). Instead of things like `this.subject`, `this.register`,
+`this.inject`, or `this.lookup` we can rely on the standard way of performing these
+functions in Ember via the owner API.
+
 When this RFC has been implemented and rolled out, these questions should all be
 addressed and our testing system will both: embrace QUnit much more **and** 
-be much more framework agnostic. Sounds like a neat trick, huh?
+be much more framework agnostic, all the while dropping custom testing only APIs
+in favor of public APIs that work across tests and app code.
+
+Sounds like a neat trick, huh?
 
 # Detailed design
 

From b6d26cb1894cfa824601e091655c621faa1618f2 Mon Sep 17 00:00:00 2001
From: Robert Jackson <rjackson@linkedin.com>
Date: Wed, 14 Jun 2017 10:23:44 -0400
Subject: [PATCH 10/12] Add `this.inject` and `this.register` to list of
 removed APIs.

---
 text/0230-simplify-qunit-testing-api.md | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/text/0230-simplify-qunit-testing-api.md b/text/0230-simplify-qunit-testing-api.md
index 3b941c5b6e..6ed8fef3e7 100644
--- a/text/0230-simplify-qunit-testing-api.md
+++ b/text/0230-simplify-qunit-testing-api.md
@@ -195,6 +195,8 @@ being proposed here:
 * the various setup methods no longer need to know the name of the object under test
 * `this.subject` is removed in favor of using the standard public API for looking up
   and creating instances (`this.owner.lookup` and `this.owner.factoryFor`)
+* `this.inject` is removed in favor of using `this.owner.lookup` directly
+* `this.register` is removed in favor of using `this.owner.register` directly
 * `this.render` will begin being asynchronous to allow for further iteration in the
   underlying rendering engines ability to speed up render times (by yielding back
   to the browser and not blocking the main thread)

From c416cc0e376b62de450e8c7e8d7c687026d1bdd7 Mon Sep 17 00:00:00 2001
From: Robert Jackson <rjackson@linkedin.com>
Date: Fri, 16 Jun 2017 13:01:02 -0400
Subject: [PATCH 11/12] Add concern RE: `hooks` naming.

---
 text/0230-simplify-qunit-testing-api.md | 37 +++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/text/0230-simplify-qunit-testing-api.md b/text/0230-simplify-qunit-testing-api.md
index 6ed8fef3e7..dfe9672b24 100644
--- a/text/0230-simplify-qunit-testing-api.md
+++ b/text/0230-simplify-qunit-testing-api.md
@@ -423,3 +423,40 @@ which should be released as 2.3.4 well before this RFC is merged.
 
 The simplest alternative is to do nothing. This would loose all of the positive
 benefits mentioned in this RFC, but should still be considered a possibility...
+
+# Unanswered Questions
+
+## `hooks` argument
+
+A few folks (e.g. [@ebryn](https://github.com/ebryn) and [@stefanpenner](https://github.com/stefanpenner))
+have approached me with concerns around the `hooks` argument I have mentioned/used here. The concerns
+are generally an initial reaction to the QUnit nested modules API in general and not directly related
+to this RFC (other than it highlighting a new feature that they haven't used before).
+
+The main concerns are:
+
+* Teaching folks what `hooks` means is a bit more difficult because it does not represent the "test
+  environment", but rather just a way to invoke the callbacks for `before` / `beforeEach` / `after` /
+  `afterEach`.
+* Passing only `hooks` to the helper functions proposed in the RFC means that if we ever need to thread
+  more information through, we either have to use `hooks` as a transport or change our API to add more
+  arguments.
+* It seems somewhat impossible to communicate across multiple helpers (again without using `hooks`
+  as a state/transport mechanism).
+
+I've kicked off a conversation over with the QUnit folks in https://github.com/qunitjs/qunit/issues/1200.
+If that PR were merged this proposal would be modified to the following syntax:
+
+```js
+// current proposal
+module('x-foo', function(hooks) {
+  setupRenderingTest(hooks);
+  // ....snip....
+});
+
+// after qunitjs/qunit#1200
+module('x-foo', function(hooks) {
+  setupRenderingTest(this);
+  // ....snip....
+});
+```

From 58c0564fe4eaaa638032b53ee95ba5465e03a7a5 Mon Sep 17 00:00:00 2001
From: Robert Jackson <rjackson@linkedin.com>
Date: Fri, 16 Jun 2017 13:03:18 -0400
Subject: [PATCH 12/12] Add possible mitigation for `hooks` argument.

---
 text/0230-simplify-qunit-testing-api.md | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/text/0230-simplify-qunit-testing-api.md b/text/0230-simplify-qunit-testing-api.md
index dfe9672b24..d88dcd25ef 100644
--- a/text/0230-simplify-qunit-testing-api.md
+++ b/text/0230-simplify-qunit-testing-api.md
@@ -460,3 +460,7 @@ module('x-foo', function(hooks) {
   // ....snip....
 });
 ```
+
+Another possible solution is to rename the argument (here and in the blueprints) to `module`.
+This is more in line with what the QUnit folks view it as: the "module context" that
+is being created for that specific `QUnit.module` invocation.