Skip to content

Commit

Permalink
[BUGFIX beta] revert jquery-less pendingRequest event handling
Browse files Browse the repository at this point in the history
In general,Ember's testing system leverages `ajaxSend` and
`ajaxComplete` events to know when `jQuery.ajax` requests are pending
(and it should therefore "pause settledness" of the testing helpers like
`click` / `andThen` / etc).

In the Ember 3.1 cycle Ember changed from using `jQuery.on` to
`document.addEventListener` to subscribe to these events. Unfortunately,
`jQuery.ajax` **does not** trigger listeners added in this way (we would
have had to do `document.onajaxSend = function() { /* stuff here */ }`).
The result of this change is that "normal" acceptance testing helpers
will _no longer_ be able to detect and wait for pending `jQuery.ajax`.

So, for example, you might expect that given the following

```js
await click('#some-selector');

assert.equal(
   $('#something-that-should-exist-after-an-ajax-call').text(),
   'hi'
);
```

the click helper would wait until `jQuery.ajax` calls were settled
before the assertion, but prior to the changes in this commit
the promise returned by `click` would **not** wait until the
`jQuery.ajax` request was completed and therefore the assertion would
fail.

---

This commit reverts the changes to use `document.addEventListener` back
to using `jQuery.on` directly when `jQuery` is available.

Many bothans died to bring us this information. 😭 😢 :crying_cat:

Paired with @rwjblue to resolve this issue.
  • Loading branch information
rondale-sc committed Apr 4, 2018
1 parent f99538d commit 8609228
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 24 deletions.
17 changes: 9 additions & 8 deletions packages/ember-testing/lib/setup_for_testing.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* global self */

import { setTesting } from 'ember-debug';
import { jQuery } from 'ember-views';
import { getAdapter, setAdapter } from './test/adapter';
import {
incrementPendingRequests,
Expand All @@ -9,9 +10,7 @@ import {
} from './test/pending_requests';
import Adapter from './adapters/adapter';
import QUnitAdapter from './adapters/qunit';
/**
@module ember
*/

/**
Sets Ember up for testing. This is useful to perform
basic setup steps in order to unit test.
Expand All @@ -33,11 +32,13 @@ export default function setupForTesting() {
setAdapter(typeof self.QUnit === 'undefined' ? new Adapter() : new QUnitAdapter());
}

document.removeEventListener('ajaxSend', incrementPendingRequests);
document.removeEventListener('ajaxComplete', decrementPendingRequests);
if (jQuery) {
jQuery(document).off('ajaxSend', incrementPendingRequests);
jQuery(document).off('ajaxComplete', decrementPendingRequests);

clearPendingRequests();
clearPendingRequests();

document.addEventListener('ajaxSend', incrementPendingRequests);
document.addEventListener('ajaxComplete', decrementPendingRequests);
jQuery(document).on('ajaxSend', incrementPendingRequests);
jQuery(document).on('ajaxComplete', decrementPendingRequests);
}
}
6 changes: 2 additions & 4 deletions packages/ember-testing/lib/test/pending_requests.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,11 @@ export function clearPendingRequests() {
requests.length = 0;
}

export function incrementPendingRequests({ detail } = { detail: { xhr: null } }) {
let xhr = detail.xhr;
export function incrementPendingRequests(_, xhr) {
requests.push(xhr);
}

export function decrementPendingRequests({ detail } = { detail: { xhr: null } }) {
let xhr = detail.xhr;
export function decrementPendingRequests(_, xhr) {
for (let i = 0; i < requests.length; i++) {
if (xhr === requests[i]) {
requests.splice(i, 1);
Expand Down
22 changes: 10 additions & 12 deletions packages/ember-testing/tests/helpers_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { Route } from 'ember-routing';
import { Controller, RSVP } from 'ember-runtime';
import { later } from 'ember-metal';
import { Component } from 'ember-glimmer';
import { jQueryDisabled } from 'ember-views';
import { jQueryDisabled, jQuery } from 'ember-views';

import Test from '../test';
import setupForTesting from '../setup_for_testing';
Expand All @@ -26,12 +26,6 @@ function registerHelper() {
Test.registerHelper('LeakyMcLeakLeak', () => {});
}

function customEvent(name, xhr) {
let event = document.createEvent('CustomEvent');
event.initCustomEvent(name, true, true, { xhr });
document.dispatchEvent(event);
}

function assertHelpers(assert, application, helperContainer, expected) {
if (!helperContainer) {
helperContainer = window;
Expand Down Expand Up @@ -1040,15 +1034,19 @@ if (!jQueryDisabled) {
moduleFor(
'ember-testing: pendingRequests',
class extends HelpersApplicationTestCase {
trigger(type, xhr) {
jQuery(document).trigger(type, xhr);
}

[`@test pendingRequests is maintained for ajaxSend and ajaxComplete events`](assert) {
assert.equal(pendingRequests(), 0);

let xhr = { some: 'xhr' };

customEvent('ajaxSend', xhr);
this.trigger('ajaxSend', xhr);
assert.equal(pendingRequests(), 1, 'Ember.Test.pendingRequests was incremented');

customEvent('ajaxComplete', xhr);
this.trigger('ajaxComplete', xhr);
assert.equal(pendingRequests(), 0, 'Ember.Test.pendingRequests was decremented');
}

Expand All @@ -1059,7 +1057,7 @@ if (!jQueryDisabled) {

let xhr = { some: 'xhr' };

customEvent('ajaxSend', xhr);
this.trigger('ajaxSend', xhr);
assert.equal(pendingRequests(), 1, 'Ember.Test.pendingRequests was incremented');

setupForTesting();
Expand All @@ -1068,10 +1066,10 @@ if (!jQueryDisabled) {

let altXhr = { some: 'more xhr' };

customEvent('ajaxSend', altXhr);
this.trigger('ajaxSend', altXhr);
assert.equal(pendingRequests(), 1, 'Ember.Test.pendingRequests was incremented');

customEvent('ajaxComplete', xhr);
this.trigger('ajaxComplete', xhr);
assert.equal(
pendingRequests(),
1,
Expand Down

0 comments on commit 8609228

Please sign in to comment.