From 46c51d8bdb82647c213daead0ddd7adc8b8ea5b3 Mon Sep 17 00:00:00 2001
From: Robert Jackson <rjackson@linkedin.com>
Date: Fri, 15 Dec 2017 12:54:19 -0500
Subject: [PATCH] Split cleanup into two buckets.

The following are the two cleanup buckets:

* `RENDERING_CLEANUP` is done when `teardownRenderingContext` is done
(which should **always** be before `teardownContext` is ran).
* `CLEANUP` is done _after_ everything else is destroyed and settled.

---

This refactor fixes the vast majority of issues on IE11. Prior to these
changes we were resetting the DOM test fixtures to their original values
_before_ we had properly cleaned up the DOM. This meant that when the
DOM cleanup actually does run, the various nodes being removed by
glimmer's internal cleanup system are not actually present in DOM.
Apparently, Chrome / FireFox / Safari / Edge are all fine with this
situation (and silently allow `node.removeChild(someThingNotInNode)`
without an error), but IE11 did properly (IMHO) error that
`someThingNotInNode` was not found.
---
 .../@ember/test-helpers/setup-context.js      |  6 +++
 .../test-helpers/setup-rendering-context.js   | 20 ++++++----
 .../@ember/test-helpers/teardown-context.js   | 38 ++++++++++++++-----
 3 files changed, 46 insertions(+), 18 deletions(-)

diff --git a/addon-test-support/@ember/test-helpers/setup-context.js b/addon-test-support/@ember/test-helpers/setup-context.js
index 523643aa4..4eaab3368 100644
--- a/addon-test-support/@ember/test-helpers/setup-context.js
+++ b/addon-test-support/@ember/test-helpers/setup-context.js
@@ -1,5 +1,6 @@
 import { run } from '@ember/runloop';
 import { set, setProperties, get, getProperties } from '@ember/object';
+import { guidFor } from '@ember/object/internals';
 import buildOwner from './build-owner';
 import { _setupPromiseListeners } from './ext/rsvp';
 import { _setupAJAXHooks } from './settled';
@@ -49,6 +50,8 @@ export function resumeTest() {
   return context.resumeTest();
 }
 
+export const CLEANUP = Object.create(null);
+
 /*
  * Responsible for:
  *
@@ -62,6 +65,9 @@ export default function(context, options = {}) {
   Ember.testing = true;
   setContext(context);
 
+  let contextGuid = guidFor(context);
+  CLEANUP[contextGuid] = [];
+
   return nextTickPromise()
     .then(() => {
       let { resolver } = options;
diff --git a/addon-test-support/@ember/test-helpers/setup-rendering-context.js b/addon-test-support/@ember/test-helpers/setup-rendering-context.js
index e86f73523..4426e4528 100644
--- a/addon-test-support/@ember/test-helpers/setup-rendering-context.js
+++ b/addon-test-support/@ember/test-helpers/setup-rendering-context.js
@@ -2,7 +2,7 @@ import { guidFor } from '@ember/object/internals';
 import { run } from '@ember/runloop';
 import Ember from 'ember';
 import global from './global';
-import { getContext } from './setup-context';
+import { getContext, CLEANUP } from './setup-context';
 import { nextTickPromise } from './-utils';
 import settled from './settled';
 
@@ -38,16 +38,17 @@ export function clearRender() {
  * - Adding `this.clearRender` to the provided context
  */
 export default function(context) {
-  let guid = guidFor(context);
+  let contextGuid = guidFor(context);
+  RENDERING_CLEANUP[contextGuid] = [];
 
   let testElementContainer = document.getElementById('ember-testing-container');
   let fixtureResetValue = testElementContainer.innerHTML;
 
-  RENDERING_CLEANUP[guid] = [
-    () => {
-      testElementContainer.innerHTML = fixtureResetValue;
-    },
-  ];
+  // push this into the final cleanup bucket, to be ran _after_ the owner
+  // is destroyed and settled (e.g. flushed run loops, etc)
+  CLEANUP[contextGuid].push(() => {
+    testElementContainer.innerHTML = fixtureResetValue;
+  });
 
   return nextTickPromise().then(() => {
     let { owner } = context;
@@ -67,7 +68,10 @@ export default function(context) {
       : owner._lookupFactory('view:-outlet');
     let OutletTemplate = owner.lookup('template:-outlet');
     let toplevelView = OutletView.create();
-    RENDERING_CLEANUP[guid].push(() => toplevelView.destroy());
+
+    // push this into the rendering specific cleanup bucket, to be ran during
+    // `teardownRenderingContext` but before the owner itself is destroyed
+    RENDERING_CLEANUP[contextGuid].push(() => toplevelView.destroy());
 
     let hasOutletTemplate = Boolean(OutletTemplate);
     let outletState = {
diff --git a/addon-test-support/@ember/test-helpers/teardown-context.js b/addon-test-support/@ember/test-helpers/teardown-context.js
index 0524460a9..e8b2331d2 100644
--- a/addon-test-support/@ember/test-helpers/teardown-context.js
+++ b/addon-test-support/@ember/test-helpers/teardown-context.js
@@ -1,23 +1,41 @@
+import { guidFor } from '@ember/object/internals';
 import { run } from '@ember/runloop';
 import { _teardownPromiseListeners } from './ext/rsvp';
 import { _teardownAJAXHooks } from './settled';
-import { unsetContext } from './setup-context';
+import { unsetContext, CLEANUP } from './setup-context';
 import { nextTickPromise } from './-utils';
 import settled from './settled';
 import Ember from 'ember';
 
 export default function(context) {
-  return nextTickPromise().then(() => {
-    let { owner } = context;
+  return nextTickPromise()
+    .then(() => {
+      let { owner } = context;
 
-    _teardownPromiseListeners();
-    _teardownAJAXHooks();
+      _teardownPromiseListeners();
+      _teardownAJAXHooks();
 
-    run(owner, 'destroy');
-    Ember.testing = false;
+      run(owner, 'destroy');
+      Ember.testing = false;
 
-    unsetContext();
+      unsetContext();
 
-    return settled();
-  });
+      return settled();
+    })
+    .finally(() => {
+      let guid = guidFor(context);
+      let destroyables = CLEANUP[guid];
+
+      if (!destroyables) {
+        return;
+      }
+
+      for (let i = 0; i < destroyables.length; i++) {
+        destroyables[i];
+      }
+
+      delete CLEANUP[guid];
+
+      return settled();
+    });
 }