Skip to content

Commit

Permalink
Ensure testing elements are properly reset/cleared.
Browse files Browse the repository at this point in the history
Prior to this change, the `innerHTML` of the `#ember-testing` element was reset
after each test (much in the same way as QUnit itself resets the `qunit-fixture`
between tests). There were two specific issues with the approach:

1. The `teardownRenderingContext` was **required** to run after the
  `teardownContext`. If the order was set correctly (`teardownRenderContext(this)`
  _then_ `teardownContext(this)`) errors would be thrown because Ember is
  attempting to clean up the rendering engine and its expected DOM elements to
  remove are no longer present. This meant that we could not use the normal rule
  of thumb for these things: that teardown is done in the reverse order of setup.
2. Any attributes or properties added to the `#ember-testing` element were
  **not** being cleaned up, and caused a large number of cascading test
  failures. Ember's acceptance tests set an `ember-application` class on what it
  thinks is the `rootElement`, if it sees that class is already present it
  throws an error. Due to the way tests were being improperly cleaned up, one
  failed acceptance test (which failed in such a way as proper cleanup could not
  happen) would essentially cause **all** of the remaining acceptance tests to
  fail.

The fix here was to swap from capturing and resetting the `#ember-testing`
elements `innerHTML` to capturing and resetting the `#ember-testing-container`'s `innerHTML`.
This has the effect of ensuring that the `#ember-testing` element (which is
contained _within_ the `#ember-testing-container` element) is reset completely,
and allows Ember's rendering system to continue to do its normal cleanup without
error (because the elements it expected to exist are still present).
  • Loading branch information
rwjblue committed Oct 17, 2017
1 parent 7d9faa3 commit 7ad93d2
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 6 deletions.
7 changes: 3 additions & 4 deletions addon-test-support/setup-rendering-context.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,12 @@ export function clearRender() {
export default function(context) {
let guid = guidFor(context);

let rootTestElement = document.getElementById('ember-testing');
let fixtureResetValue = rootTestElement.innerHTML;
let testElementContainer = document.getElementById('ember-testing-container');
let fixtureResetValue = testElementContainer.outerHTML;

RENDERING_CLEANUP[guid] = [
() => {
rootTestElement.innerHTML = fixtureResetValue;
element = undefined;
testElementContainer.innerHTML = fixtureResetValue;
},
];

Expand Down
2 changes: 1 addition & 1 deletion tests/unit/settled-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,8 @@ module('settle', function(hooks) {
this.server.shutdown();
await settled();

teardownContext(this);
teardownRenderingContext(this);
teardownContext(this);
});

test('it works when async exists in `init`', async function(assert) {
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/setup-rendering-context-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ module('setupRenderingContext', function(hooks) {
});

hooks.afterEach(function() {
teardownContext(this);
teardownRenderingContext(this);
teardownContext(this);
});

test('render exposes an `.element` property', async function(assert) {
Expand Down
53 changes: 53 additions & 0 deletions tests/unit/teardown-rendering-context-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import { module, test } from 'qunit';
import {
setupContext,
setupRenderingContext,
teardownContext,
teardownRenderingContext,
} from 'ember-test-helpers';
import hasEmberVersion from 'ember-test-helpers/has-ember-version';

module('setupRenderingContext', function(hooks) {
if (!hasEmberVersion(2, 4)) {
return;
}

hooks.beforeEach(function() {
setupContext(this);
setupRenderingContext(this);
});

hooks.afterEach(function() {
teardownContext(this);
});

test('clears any attributes added to the ember-testing div', function(assert) {
let beforeTeardownEl = document.getElementById('ember-testing');
beforeTeardownEl.setAttribute('data-was-set', '');

assert.ok(
beforeTeardownEl.hasAttribute('data-was-set'),
'precond - attribute is present before teardown'
);
assert.ok(document.contains(beforeTeardownEl), 'precond - ember-testing element is in DOM');

teardownRenderingContext(this);

let afterTeardownEl = document.getElementById('ember-testing');

assert.notOk(
afterTeardownEl.hasAttribute('data-was-set'),
'attribute is not present on ember-testing that is in DOM'
);
assert.ok(document.contains(afterTeardownEl), 'ember-testing element is still in DOM');

assert.ok(
beforeTeardownEl.hasAttribute('data-was-set'),
'attribute is still present on prior ember-testing element after teardown'
);
assert.notOk(
document.contains(beforeTeardownEl),
'previous ember-testing element is no longer in DOM'
);
});
});

0 comments on commit 7ad93d2

Please sign in to comment.