jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/344885 )
Change subject: tests: Don't assume 1 wait call per test
......................................................................
tests: Don't assume 1 wait call per test
The delay/timeout logic in actions#fetch will require more than one wait
call, for example.
Changes:
* Update the stub created in setupWait to store all deferred-promise
pairs and update the integration tests so that they don't use the stub
accidentally.
* Remove all references to the waitDeferred/waitPromise properties.
* Fix tests that relied on the waitDeferred/waitPromise properties being
available regardless of whether wait had been called.
* Update outdated or brittle - in the sense that it didn't reference
constants but their values - inline documentation.
Change-Id: I94345cdf4126b6c540d4fb8135a7a7e4d0507bed
---
M tests/node-qunit/actions.test.js
M tests/node-qunit/integration.test.js
2 files changed, 78 insertions(+), 52 deletions(-)
Approvals:
Jhernandez: Looks good to me, approved
jenkins-bot: Verified
diff --git a/tests/node-qunit/actions.test.js b/tests/node-qunit/actions.test.js
index 6a42800..5798979 100644
--- a/tests/node-qunit/actions.test.js
+++ b/tests/node-qunit/actions.test.js
@@ -66,9 +66,18 @@
* @param {Object} module
*/
function setupWait( module ) {
- module.waitDeferred = $.Deferred();
- module.waitPromise = module.waitDeferred.promise();
- module.wait = module.sandbox.stub().returns( module.waitPromise );
+ module.waitDeferreds = [];
+ module.waitPromises = [];
+
+ module.wait = module.sandbox.spy( function () {
+ var deferred = $.Deferred(),
+ promise = deferred.promise();
+
+ module.waitDeferreds.push( deferred );
+ module.waitPromises.push( promise );
+
+ return promise;
+ } );
mock( '../../src/wait', module.wait );
// Re-require actions so that it uses the mocked wait module
@@ -101,7 +110,7 @@
return that.state;
};
- // Fake implementation of mw.now
+ // The worst-case implementation of mw.now.
mw.now = function () { return Date.now(); };
setupWait( this );
@@ -120,7 +129,7 @@
this.sandbox.stub( mw, 'now' ).returns( new Date() );
this.sandbox.stub( actions, 'fetch' );
- // Set the state for when dispatch is called. New token is accepted
+ // Stub the state tree being updated by the LINK_DWELL action.
this.state.preview = {
activeToken: generateToken()
};
@@ -147,18 +156,18 @@
// ---
- this.waitPromise.then( function () {
+ this.waitPromises[ 0 ].then( function () {
assert.strictEqual(
dispatch.callCount,
2,
- 'The fetch action is dispatched after 50 ms'
+ 'The fetch action is dispatched after FETCH_COMPLETE
milliseconds.'
);
done();
} );
- // After 50 ms...
- this.waitDeferred.resolve();
+ // After FETCH_START_DELAY milliseconds...
+ this.waitDeferreds[ 0 ].resolve();
} );
QUnit.test( '#linkDwell doesn\'t continue when previews are disabled',
function ( assert ) {
@@ -166,23 +175,28 @@
event = {},
dispatch = this.sandbox.spy();
+ // Stub the state tree being updated by the LINK_DWELL action.
+ this.state.preview = {
+ enabled: false,
+ activeLink: this.el,
+ activeToken: generateToken()
+ };
+
actions.linkDwell( this.el, event, /* gateway = */ null, generateToken
)(
dispatch,
this.getState
);
- this.state.preview = {
- enabled: false
- };
+ assert.strictEqual( this.wait.callCount, 1 );
- this.waitPromise.then( function () {
+ this.waitPromises[ 0 ].then( function () {
assert.strictEqual( dispatch.callCount, 1 );
done();
} );
- // After 500 ms...
- this.waitDeferred.resolve();
+ // After FETCH_START_DELAY milliseconds...
+ this.waitDeferreds[ 0 ].resolve();
} );
QUnit.test( '#linkDwell doesn\'t continue if the token has changed', function
( assert ) {
@@ -190,11 +204,19 @@
event = {},
dispatch = this.sandbox.spy();
+ // Stub the state tree being updated by a LINK_DWELL action.
+ this.state.preview = {
+ enabled: true,
+ activeLink: this.el,
+ activeToken: generateToken()
+ };
+
actions.linkDwell( this.el, event, /* gateway = */ null, generateToken
)(
dispatch,
this.getState
);
+ // Stub the state tree being updated by another LINK_DWELL action.
this.state.preview = {
enabled: true,
@@ -203,26 +225,27 @@
// changed, but the active token has.
activeLink: this.el,
- activeToken: '0123456789'
+ activeToken: 'banana'
};
- this.waitPromise.then( function () {
+ this.waitPromises[ 0 ].then( function () {
assert.strictEqual( dispatch.callCount, 1 );
done();
} );
- // After 500 ms...
- this.waitDeferred.resolve();
+ // After FETCH_START_DELAY milliseconds...
+ this.waitDeferreds[ 0 ].resolve();
} );
-QUnit.test( '#linkDwell doesn\'t continue if the interaction is the same one',
function ( assert ) {
+QUnit.test( '#linkDwell dispatches the fetch action', function ( assert ) {
var done = assert.async(),
event = {},
dispatch = this.sandbox.spy();
this.state.preview = {
- activeToken: '0123456789'
+ enabled: true,
+ activeToken: generateToken()
};
actions.linkDwell( this.el, event, /* gateway = */ null, generateToken
)(
@@ -230,14 +253,14 @@
this.getState
);
- this.waitPromise.then( function () {
- assert.strictEqual( dispatch.callCount, 1 );
+ this.waitPromises[ 0 ].then( function () {
+ assert.strictEqual( dispatch.callCount, 2 );
done();
} );
- // After 500 ms...
- this.waitDeferred.resolve();
+ // After FETCH_START_DELAY milliseconds...
+ this.waitDeferreds[ 0 ].resolve();
} );
QUnit.module( 'ext.popups/actions#fetch', {
@@ -293,54 +316,52 @@
result = {},
done = assert.async( 2 );
- assert.expect( 4 );
+ assert.expect( 3 );
this.fetch();
this.gatewayPromise.then( function () {
+ assert.deepEqual(
+ that.dispatch.getCall( 1 ).args[ 0 ],
+ {
+ type: 'FETCH_END',
+ el: that.el,
+ timestamp: 250
+ },
+ 'It should dispatch the FETCH_END action when the
gateway request ends.'
+ );
+
assert.ok(
that.wait.calledWith( 250 ),
'FETCH_COMPLETE is delayed by 250 (500 - 250) ms. ' +
'If you\'ve changed FETCH_COMPLETE_TARGET_DELAY, then
have you spoken with #Design about changing this value?'
);
- that.waitDeferred.resolve();
-
done();
} );
- this.waitPromise.then( function () {
- // Let the wait.then execute to run the dispatch before
asserting
- setTimeout( function () {
- assert.equal( that.dispatch.callCount, 3, 'dispatch
called for start and end' );
- assert.deepEqual(
- that.dispatch.getCall( 1 ).args[ 0 ],
- {
- type: 'FETCH_END',
- el: that.el,
- timestamp: 250
- },
- 'It should dispatch the FETCH_END action when
the gateway request ends.'
- );
+ this.now += 250;
+ this.gatewayDeferred.resolve( result );
+ this.waitPromises[ 0 ].then( function () {
+ setTimeout( function () {
assert.deepEqual(
that.dispatch.getCall( 2 ).args[ 0 ],
{
type: 'FETCH_COMPLETE',
el: that.el,
- result: result,
- timestamp: 250
+ timestamp: 500,
+ result: {}
},
- 'It should dispatch the FETCH_COMPLETE action
when the preview model is resolved.'
+ 'It should dispatch the FETCH_END action when
the gateway request ends.'
);
done();
} );
} );
- // The API request took 250 ms.
this.now += 250;
- this.gatewayDeferred.resolve( result );
+ this.waitDeferreds[ 0 ].resolve();
} );
QUnit.test(
@@ -403,7 +424,7 @@
'Have you spoken with #Design about changing this value?'
);
- this.waitPromise.then( function () {
+ this.waitPromises[ 0 ].then( function () {
assert.ok(
dispatch.calledWith( {
type: 'ABANDON_END',
@@ -415,8 +436,8 @@
done();
} );
- // After 300 ms...
- this.waitDeferred.resolve();
+ // After ABANDON_END_DELAY milliseconds...
+ this.waitDeferreds[ 0 ].resolve();
} );
QUnit.module( 'ext.popups/actions#saveSettings' );
diff --git a/tests/node-qunit/integration.test.js
b/tests/node-qunit/integration.test.js
index 1bbc84c..3584895 100644
--- a/tests/node-qunit/integration.test.js
+++ b/tests/node-qunit/integration.test.js
@@ -1,7 +1,8 @@
var mock = require( 'mock-require' ),
Redux = require( 'redux' ),
ReduxThunk = require( 'redux-thunk' ),
- wait = require( '../../src/wait' );
+ wait = require( '../../src/wait' ),
+ mw = mediaWiki;
function identity( x ) { return x; }
function constant( x ) { return function () { return x; }; }
@@ -45,6 +46,9 @@
var that = this,
reducers, actions, registerChangeListener;
+ // The worst-case implementation of mw.now.
+ mw.now = function () { return Date.now(); };
+
this.resetWait = function () {
that.waitDeferred = $.Deferred();
that.waitPromise = that.waitDeferred.promise();
@@ -56,9 +60,10 @@
mock( '../../src/wait', this.wait );
- // Require modules after the setting require mocks
+ // Require modules after the setting require mocks,
invalidating the
+ // require cache for modules that depend on the wait function.
+ actions = mock.reRequire( '../../src/actions' );
reducers = require( '../../src/reducers' );
- actions = require( '../../src/actions' );
registerChangeListener = require( '../../src/changeListener' );
this.store = Redux.createStore(
--
To view, visit https://gerrit.wikimedia.org/r/344885
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I94345cdf4126b6c540d4fb8135a7a7e4d0507bed
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/extensions/Popups
Gerrit-Branch: master
Gerrit-Owner: Phuedx <[email protected]>
Gerrit-Reviewer: Jhernandez <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
