mediawiki-extensions-Popups/tests/node-qunit/actions.test.js

632 lines
14 KiB
JavaScript
Raw Normal View History

import { createStubUser, createStubTitle } from './stubs';
import * as actions from '../../src/actions';
import * as WaitModule from '../../src/wait';
import actionTypes from '../../src/actionTypes';
import { setDwellTime, previewTypes } from '../../src/preview/model';
const REFERRER = 'https://en.wikipedia.org/wiki/Kitten',
TEST_TITLE = createStubTitle( 0, 'Foo' );
function generateToken() {
return 'ABC';
}
QUnit.module( 'ext.popups/actions' );
QUnit.test( '#boot', ( assert ) => {
const config = new Map(),
stubUser = createStubUser( /* isAnon = */ true );
config.set( 'wgTitle', 'Foo' );
config.set( 'wgNamespaceNumber', 0 );
config.set( 'wgArticleId', 2 );
config.set( 'wgUserEditCount', 3 );
config.set( 'wgPopupsConflictsWithNavPopupGadget', true );
const stubUserSettings = {
getPreviewCount() {
return 22;
}
};
const action = actions.boot(
{ page: false },
stubUser,
stubUserSettings,
config,
REFERRER
);
assert.deepEqual(
action,
{
type: actionTypes.BOOT,
initiallyEnabled: { page: false },
isNavPopupsEnabled: true,
pageToken: '9876543210',
page: {
url: REFERRER,
title: 'Foo',
namespaceId: 0,
id: 2
},
user: {
isAnon: true,
editCount: 3
}
},
'boots with the initial state'
);
} );
Generalize settings code (attempt 2) This reverts commit a6a65204c69399b8332c1894ee7ad7cece0fbeb5. to restore custom preview types. -- Changes since revert The previous patch accidentally removed the syncUserSettings changeListener. This has now been restored with several modifications: * We have a migrate script which rewrites existing localStorage settings to the new system * The existing save functions are generalized. The changes since this patch are captured in Ia73467799a9b535f7a3cf7268727c9fab7af0d7e -- More information A new REGISTER_SETTING action replaces the BOOT action for registering settings. This allows custom preview types to be associated with a setting. They do this by adding the enabled property to the module they provide to mw.popups.register Every time the new action is called, we refresh the settings dialog UI with the new settings. Previously the settings dialog was hardcoded, but now it is generated from the registered preview types by deriving associated messages and checking they exist, so by default custom types will not show up in the settings. Benefits: * This change empowers us to add a setting for Math previews to allow them to be enabled or disabled. * Allows us to separate references as its own module Additional notes: * The syncUserSettings.js changeListener is no longer needed as the logic for this is handled inside the "userSettings" change listener in response to the "settings" reducer which is responding to SETTINGS_CHANGE and REGISTER_SETTING actions. Upon merging: * https://www.mediawiki.org/wiki/Extension:Popups#Extensibility will be updated to detail how a setting can be registered. Bug: T334261 Bug: T326692 Change-Id: Ie17d622870511ac9730fc9fa525698fc3aa0d5b6
2023-10-19 21:12:09 +00:00
QUnit.test( '#registerSetting', ( assert ) => {
const action = actions.registerSetting(
'foo',
false
);
assert.deepEqual(
action,
{
type: actionTypes.REGISTER_SETTING,
name: 'foo',
enabled: false
},
'Setting action'
);
} );
/**
* Stubs `wait.js` and adds the deferred and its promise as properties
* of the module.
*
* @param {Object} module
*/
function setupWait( module ) {
Update: cancel unused HTTP requests in flight Whenever an HTTP request sequence is started, i.e. wait for the fetch start time, issue a network request, and return the result, abort the process if the results are known to no longer be needed. This occurs when a user has dwelt upon one link and then abandoned it either during the fetch start wait time or during the fetch network request itself. This change is accomplished by preserving the pending promises in two actions, LINK_DWELL and FETCH_START, and whenever the ABANDON_START action is issued, it now aborts any previously pending XHR-like promise, called a "AbortPromise" which is just a thenable with an abort() method. There is a similar concept in Core: https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/core/+/ecc812f06e7dff587b3f31dc18189adbf4616351/resources/src/mediawiki.api/index.js. Aborting pending requests has big implications for client and server logging as requests are quickly canceled, especially on slower connections. These differences can be observed on the network tab of DevTools and the log in Redux DevTools. Consider, for instance, the scenario of dwelling upon and quickly abandoning a single link prior to this patch: BOOT EVENT_LOGGED LINK_DWELL FETCH_START ABANDON_START FETCH_END STATSV_LOGGED ABANDON_END EVENT_LOGGED FETCH_COMPLETE And after this patch when the fetch timer is canceled (prior to an actual network request): BOOT EVENT_LOGGED LINK_DWELL ABANDON_START ABANDON_END EVENT_LOGGED In the above sequence, FETCH_* and STATSV_LOGGED actions never occur. And after this patch when the network request itself is canceled: BOOT EVENT_LOGGED LINK_DWELL FETCH_START ABANDON_START FETCH_FAILED STATSV_LOGGED FETCH_COMPLETE ABANDON_END EVENT_LOGGED FETCH_FAILED occurs intentionally, STATSV_LOGGED and FETCH_COMPLETE still happen even though the fetch didn't complete successfully, and FETCH_END doesn't. Additionally, since less data is transmitted, it's possible that the timing and success rate of logging will improve on low bandwidth connections. Also, this patch tries to revise the JSDocs where possible to support type checking and fix a call to the missing assert.fail() function in changeListener.test.js. Bug: T197700 Change-Id: I9a73b3086fc8fb0edd897a347b5497d5362e20ef
2018-06-25 13:26:11 +00:00
module.waitPromise = $.Deferred().resolve().promise( { abort() {} } );
module.wait = module.sandbox.stub( WaitModule, 'default' ).callsFake(
() => module.waitPromise
);
}
/**
* Sets up a link/mw.Title stub pair that can be passed to the linkDwell action
* creator.
*
* @param {Object} module
*/
function setupEl( module ) {
module.title = TEST_TITLE;
module.el = $( '<a>' ).get( 0 );
}
QUnit.module( 'ext.popups/actions#linkDwell @integration', {
beforeEach() {
this.state = {
preview: {}
};
Improve & fix action and integration tests This change fixes some issues with assertions not running, removes unnecessary promise dances, and improves legibility and some code patterns in the action and integration node tests mainly. Detailed changes: * actions.js * Fully migrate out of jQuery 1 promises (no done/fail) * Fix linkDwell action not returning the fetch action promise * actions.test.js * Simplify setupWait for the tests * It always autoresolves immediately the wait call to ensure speedy tests * No waitDeferreds or waitPromises array coordination, rely on action returned promises instead * Get rid of that = this in favor of arrow functions * Rename generic "p" promises to meaningful names * Add assert.expect for more solid tests (so that we don't skip assertions in the future if we change them) * Fix some assertions that weren't being run because of the incorrect promise being returned (p.then, and then just returning p) * Get rid of $.when stubbing in favor of waiting for the promise returned from the action * Get rid of hacky setTimeout(..., 0) to run assertions after the promises * integration.test.js * Get rid of wait(0) calls to hack around asynchronous actions * Use the action returned promises instead of the waitPromises/Deferreds * Remove unused "el" parameter being passed to this.abandon in several tests * Remove unnecessary test helper this.abandonPreview (it was the same as this.abandon) * Clarify a bit the last and more complex test with some comments and variable name changes * Get rid of that=this in favor of arrow functions * container.test.js * Get rid of that=this in favor of arrow functions * previewBehavior.test.js * Get rid of that=this in favor of arrow functions * Get rid of $.each in favor of .forEach Bug: T170807 Change-Id: I06fafd92a1712f143018e2ddff24fadf1a6882b3
2018-02-16 12:52:56 +00:00
this.getState = () => this.state;
// The worst-case implementation of mw.now.
mw.now = () => Date.now();
setupEl( this );
}
} );
QUnit.test( '#linkDwell', function ( assert ) {
const measures = {},
dispatch = this.sandbox.spy();
this.sandbox.stub( mw, 'now' ).returns( new Date() );
this.sandbox.stub( actions, 'fetch' );
// Stub the state tree being updated by the LINK_DWELL action.
this.state.preview = {
activeToken: generateToken()
};
const linkDwelled = actions.linkDwell(
this.title, this.el, measures, null, generateToken, previewTypes.TYPE_PAGE
)(
dispatch,
this.getState
);
Update: cancel unused HTTP requests in flight Whenever an HTTP request sequence is started, i.e. wait for the fetch start time, issue a network request, and return the result, abort the process if the results are known to no longer be needed. This occurs when a user has dwelt upon one link and then abandoned it either during the fetch start wait time or during the fetch network request itself. This change is accomplished by preserving the pending promises in two actions, LINK_DWELL and FETCH_START, and whenever the ABANDON_START action is issued, it now aborts any previously pending XHR-like promise, called a "AbortPromise" which is just a thenable with an abort() method. There is a similar concept in Core: https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/core/+/ecc812f06e7dff587b3f31dc18189adbf4616351/resources/src/mediawiki.api/index.js. Aborting pending requests has big implications for client and server logging as requests are quickly canceled, especially on slower connections. These differences can be observed on the network tab of DevTools and the log in Redux DevTools. Consider, for instance, the scenario of dwelling upon and quickly abandoning a single link prior to this patch: BOOT EVENT_LOGGED LINK_DWELL FETCH_START ABANDON_START FETCH_END STATSV_LOGGED ABANDON_END EVENT_LOGGED FETCH_COMPLETE And after this patch when the fetch timer is canceled (prior to an actual network request): BOOT EVENT_LOGGED LINK_DWELL ABANDON_START ABANDON_END EVENT_LOGGED In the above sequence, FETCH_* and STATSV_LOGGED actions never occur. And after this patch when the network request itself is canceled: BOOT EVENT_LOGGED LINK_DWELL FETCH_START ABANDON_START FETCH_FAILED STATSV_LOGGED FETCH_COMPLETE ABANDON_END EVENT_LOGGED FETCH_FAILED occurs intentionally, STATSV_LOGGED and FETCH_COMPLETE still happen even though the fetch didn't complete successfully, and FETCH_END doesn't. Additionally, since less data is transmitted, it's possible that the timing and success rate of logging will improve on low bandwidth connections. Also, this patch tries to revise the JSDocs where possible to support type checking and fix a call to the missing assert.fail() function in changeListener.test.js. Bug: T197700 Change-Id: I9a73b3086fc8fb0edd897a347b5497d5362e20ef
2018-06-25 13:26:11 +00:00
assert.propEqual(
dispatch.getCall( 0 ).args[ 0 ], {
type: actionTypes.LINK_DWELL,
el: this.el,
previewType: 'page',
measures,
token: 'ABC',
timestamp: mw.now(),
title: 'Foo',
namespaceId: 0,
promise: Promise.resolve()
},
'The dispatcher was called with the correct arguments.'
);
// Stub the state tree being updated.
this.state.preview = {
enabled: { page: true },
activeLink: this.el,
activeToken: generateToken()
};
// ---
return linkDwelled.then( () => {
assert.strictEqual(
dispatch.callCount,
2,
'The fetch action is dispatched after FETCH_COMPLETE milliseconds.'
);
} );
} );
QUnit.test( '#linkDwell doesn\'t continue when previews are disabled', function ( assert ) {
const event = {},
dispatch = this.sandbox.spy();
// Stub the state tree being updated by the LINK_DWELL action.
this.state.preview = {
enabled: { page: false },
activeLink: this.el,
activeToken: generateToken()
};
const linkDwelled = actions.linkDwell(
this.title, this.el, event, /* gateway = */ null, generateToken, previewTypes.TYPE_PAGE
)(
dispatch,
this.getState
);
assert.strictEqual(
dispatch.callCount,
1,
'The dispatcher was called once.'
);
return linkDwelled.then( () => {
assert.strictEqual(
dispatch.callCount,
1,
'The dispatcher was not called again.'
);
} );
} );
QUnit.test( '#linkDwell doesn\'t continue if the token has changed', function ( assert ) {
const event = {},
dispatch = this.sandbox.spy();
// Stub the state tree being updated by a LINK_DWELL action.
this.state.preview = {
enabled: { page: true },
activeLink: this.el,
activeToken: generateToken()
};
const linkDwelled = actions.linkDwell(
this.title, this.el, event, /* gateway = */ null, generateToken, previewTypes.TYPE_PAGE
)(
dispatch,
this.getState
);
// Stub the state tree being updated by another LINK_DWELL action.
this.state.preview = {
enabled: { page: true },
// Consider the user tabbing back and forth between two links in the time
// it takes to start fetching data via the gateway: the active link hasn't
// changed, but the active token has.
activeLink: this.el,
activeToken: 'banana'
};
return linkDwelled.then( () => {
assert.strictEqual(
dispatch.callCount,
1,
'The dispatcher was called once.'
);
} );
} );
QUnit.test( '#linkDwell dispatches the fetch action', function ( assert ) {
const event = {},
dispatch = this.sandbox.spy();
this.state.preview = {
enabled: { page: true },
activeToken: generateToken()
};
Improve & fix action and integration tests This change fixes some issues with assertions not running, removes unnecessary promise dances, and improves legibility and some code patterns in the action and integration node tests mainly. Detailed changes: * actions.js * Fully migrate out of jQuery 1 promises (no done/fail) * Fix linkDwell action not returning the fetch action promise * actions.test.js * Simplify setupWait for the tests * It always autoresolves immediately the wait call to ensure speedy tests * No waitDeferreds or waitPromises array coordination, rely on action returned promises instead * Get rid of that = this in favor of arrow functions * Rename generic "p" promises to meaningful names * Add assert.expect for more solid tests (so that we don't skip assertions in the future if we change them) * Fix some assertions that weren't being run because of the incorrect promise being returned (p.then, and then just returning p) * Get rid of $.when stubbing in favor of waiting for the promise returned from the action * Get rid of hacky setTimeout(..., 0) to run assertions after the promises * integration.test.js * Get rid of wait(0) calls to hack around asynchronous actions * Use the action returned promises instead of the waitPromises/Deferreds * Remove unused "el" parameter being passed to this.abandon in several tests * Remove unnecessary test helper this.abandonPreview (it was the same as this.abandon) * Clarify a bit the last and more complex test with some comments and variable name changes * Get rid of that=this in favor of arrow functions * container.test.js * Get rid of that=this in favor of arrow functions * previewBehavior.test.js * Get rid of that=this in favor of arrow functions * Get rid of $.each in favor of .forEach Bug: T170807 Change-Id: I06fafd92a1712f143018e2ddff24fadf1a6882b3
2018-02-16 12:52:56 +00:00
return actions.linkDwell(
this.title, this.el, event, /* gateway = */ null, generateToken, previewTypes.TYPE_PAGE
)(
dispatch,
this.getState
).then( () => {
assert.strictEqual(
dispatch.callCount,
2,
'The dispatcher was called twice.'
);
} );
} );
QUnit.module( 'ext.popups/actions#fetch', {
beforeEach() {
this.now = 0;
Improve & fix action and integration tests This change fixes some issues with assertions not running, removes unnecessary promise dances, and improves legibility and some code patterns in the action and integration node tests mainly. Detailed changes: * actions.js * Fully migrate out of jQuery 1 promises (no done/fail) * Fix linkDwell action not returning the fetch action promise * actions.test.js * Simplify setupWait for the tests * It always autoresolves immediately the wait call to ensure speedy tests * No waitDeferreds or waitPromises array coordination, rely on action returned promises instead * Get rid of that = this in favor of arrow functions * Rename generic "p" promises to meaningful names * Add assert.expect for more solid tests (so that we don't skip assertions in the future if we change them) * Fix some assertions that weren't being run because of the incorrect promise being returned (p.then, and then just returning p) * Get rid of $.when stubbing in favor of waiting for the promise returned from the action * Get rid of hacky setTimeout(..., 0) to run assertions after the promises * integration.test.js * Get rid of wait(0) calls to hack around asynchronous actions * Use the action returned promises instead of the waitPromises/Deferreds * Remove unused "el" parameter being passed to this.abandon in several tests * Remove unnecessary test helper this.abandonPreview (it was the same as this.abandon) * Clarify a bit the last and more complex test with some comments and variable name changes * Get rid of that=this in favor of arrow functions * container.test.js * Get rid of that=this in favor of arrow functions * previewBehavior.test.js * Get rid of that=this in favor of arrow functions * Get rid of $.each in favor of .forEach Bug: T170807 Change-Id: I06fafd92a1712f143018e2ddff24fadf1a6882b3
2018-02-16 12:52:56 +00:00
this.sandbox.stub( mw, 'now' ).callsFake( () => this.now );
setupWait( this );
setupEl( this );
this.gatewayDeferred = $.Deferred();
this.gateway = {
fetchPreviewForTitle: this.sandbox.stub().returns(
Update: cancel unused HTTP requests in flight Whenever an HTTP request sequence is started, i.e. wait for the fetch start time, issue a network request, and return the result, abort the process if the results are known to no longer be needed. This occurs when a user has dwelt upon one link and then abandoned it either during the fetch start wait time or during the fetch network request itself. This change is accomplished by preserving the pending promises in two actions, LINK_DWELL and FETCH_START, and whenever the ABANDON_START action is issued, it now aborts any previously pending XHR-like promise, called a "AbortPromise" which is just a thenable with an abort() method. There is a similar concept in Core: https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/core/+/ecc812f06e7dff587b3f31dc18189adbf4616351/resources/src/mediawiki.api/index.js. Aborting pending requests has big implications for client and server logging as requests are quickly canceled, especially on slower connections. These differences can be observed on the network tab of DevTools and the log in Redux DevTools. Consider, for instance, the scenario of dwelling upon and quickly abandoning a single link prior to this patch: BOOT EVENT_LOGGED LINK_DWELL FETCH_START ABANDON_START FETCH_END STATSV_LOGGED ABANDON_END EVENT_LOGGED FETCH_COMPLETE And after this patch when the fetch timer is canceled (prior to an actual network request): BOOT EVENT_LOGGED LINK_DWELL ABANDON_START ABANDON_END EVENT_LOGGED In the above sequence, FETCH_* and STATSV_LOGGED actions never occur. And after this patch when the network request itself is canceled: BOOT EVENT_LOGGED LINK_DWELL FETCH_START ABANDON_START FETCH_FAILED STATSV_LOGGED FETCH_COMPLETE ABANDON_END EVENT_LOGGED FETCH_FAILED occurs intentionally, STATSV_LOGGED and FETCH_COMPLETE still happen even though the fetch didn't complete successfully, and FETCH_END doesn't. Additionally, since less data is transmitted, it's possible that the timing and success rate of logging will improve on low bandwidth connections. Also, this patch tries to revise the JSDocs where possible to support type checking and fix a call to the missing assert.fail() function in changeListener.test.js. Bug: T197700 Change-Id: I9a73b3086fc8fb0edd897a347b5497d5362e20ef
2018-06-25 13:26:11 +00:00
this.gatewayDeferred.promise( { abort() {} } )
Improve & fix action and integration tests This change fixes some issues with assertions not running, removes unnecessary promise dances, and improves legibility and some code patterns in the action and integration node tests mainly. Detailed changes: * actions.js * Fully migrate out of jQuery 1 promises (no done/fail) * Fix linkDwell action not returning the fetch action promise * actions.test.js * Simplify setupWait for the tests * It always autoresolves immediately the wait call to ensure speedy tests * No waitDeferreds or waitPromises array coordination, rely on action returned promises instead * Get rid of that = this in favor of arrow functions * Rename generic "p" promises to meaningful names * Add assert.expect for more solid tests (so that we don't skip assertions in the future if we change them) * Fix some assertions that weren't being run because of the incorrect promise being returned (p.then, and then just returning p) * Get rid of $.when stubbing in favor of waiting for the promise returned from the action * Get rid of hacky setTimeout(..., 0) to run assertions after the promises * integration.test.js * Get rid of wait(0) calls to hack around asynchronous actions * Use the action returned promises instead of the waitPromises/Deferreds * Remove unused "el" parameter being passed to this.abandon in several tests * Remove unnecessary test helper this.abandonPreview (it was the same as this.abandon) * Clarify a bit the last and more complex test with some comments and variable name changes * Get rid of that=this in favor of arrow functions * container.test.js * Get rid of that=this in favor of arrow functions * previewBehavior.test.js * Get rid of that=this in favor of arrow functions * Get rid of $.each in favor of .forEach Bug: T170807 Change-Id: I06fafd92a1712f143018e2ddff24fadf1a6882b3
2018-02-16 12:52:56 +00:00
)
};
this.dispatch = this.sandbox.spy();
this.token = '1234567890';
// Sugar.
setDwellTime( previewTypes.TYPE_PAGE, 350 );
Improve & fix action and integration tests This change fixes some issues with assertions not running, removes unnecessary promise dances, and improves legibility and some code patterns in the action and integration node tests mainly. Detailed changes: * actions.js * Fully migrate out of jQuery 1 promises (no done/fail) * Fix linkDwell action not returning the fetch action promise * actions.test.js * Simplify setupWait for the tests * It always autoresolves immediately the wait call to ensure speedy tests * No waitDeferreds or waitPromises array coordination, rely on action returned promises instead * Get rid of that = this in favor of arrow functions * Rename generic "p" promises to meaningful names * Add assert.expect for more solid tests (so that we don't skip assertions in the future if we change them) * Fix some assertions that weren't being run because of the incorrect promise being returned (p.then, and then just returning p) * Get rid of $.when stubbing in favor of waiting for the promise returned from the action * Get rid of hacky setTimeout(..., 0) to run assertions after the promises * integration.test.js * Get rid of wait(0) calls to hack around asynchronous actions * Use the action returned promises instead of the waitPromises/Deferreds * Remove unused "el" parameter being passed to this.abandon in several tests * Remove unnecessary test helper this.abandonPreview (it was the same as this.abandon) * Clarify a bit the last and more complex test with some comments and variable name changes * Get rid of that=this in favor of arrow functions * container.test.js * Get rid of that=this in favor of arrow functions * previewBehavior.test.js * Get rid of that=this in favor of arrow functions * Get rid of $.each in favor of .forEach Bug: T170807 Change-Id: I06fafd92a1712f143018e2ddff24fadf1a6882b3
2018-02-16 12:52:56 +00:00
this.fetch = () => {
return actions.fetch(
this.gateway, this.title, this.el, this.token, previewTypes.TYPE_PAGE
Improve & fix action and integration tests This change fixes some issues with assertions not running, removes unnecessary promise dances, and improves legibility and some code patterns in the action and integration node tests mainly. Detailed changes: * actions.js * Fully migrate out of jQuery 1 promises (no done/fail) * Fix linkDwell action not returning the fetch action promise * actions.test.js * Simplify setupWait for the tests * It always autoresolves immediately the wait call to ensure speedy tests * No waitDeferreds or waitPromises array coordination, rely on action returned promises instead * Get rid of that = this in favor of arrow functions * Rename generic "p" promises to meaningful names * Add assert.expect for more solid tests (so that we don't skip assertions in the future if we change them) * Fix some assertions that weren't being run because of the incorrect promise being returned (p.then, and then just returning p) * Get rid of $.when stubbing in favor of waiting for the promise returned from the action * Get rid of hacky setTimeout(..., 0) to run assertions after the promises * integration.test.js * Get rid of wait(0) calls to hack around asynchronous actions * Use the action returned promises instead of the waitPromises/Deferreds * Remove unused "el" parameter being passed to this.abandon in several tests * Remove unnecessary test helper this.abandonPreview (it was the same as this.abandon) * Clarify a bit the last and more complex test with some comments and variable name changes * Get rid of that=this in favor of arrow functions * container.test.js * Get rid of that=this in favor of arrow functions * previewBehavior.test.js * Get rid of that=this in favor of arrow functions * Get rid of $.each in favor of .forEach Bug: T170807 Change-Id: I06fafd92a1712f143018e2ddff24fadf1a6882b3
2018-02-16 12:52:56 +00:00
)( this.dispatch );
};
}
} );
QUnit.test( 'it should fetch data from the gateway immediately', function ( assert ) {
this.fetch();
assert.true(
this.gateway.fetchPreviewForTitle.calledWith( TEST_TITLE ),
'The gateway was called with the correct arguments.'
);
assert.strictEqual( this.dispatch.callCount, 1 );
Update: cancel unused HTTP requests in flight Whenever an HTTP request sequence is started, i.e. wait for the fetch start time, issue a network request, and return the result, abort the process if the results are known to no longer be needed. This occurs when a user has dwelt upon one link and then abandoned it either during the fetch start wait time or during the fetch network request itself. This change is accomplished by preserving the pending promises in two actions, LINK_DWELL and FETCH_START, and whenever the ABANDON_START action is issued, it now aborts any previously pending XHR-like promise, called a "AbortPromise" which is just a thenable with an abort() method. There is a similar concept in Core: https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/core/+/ecc812f06e7dff587b3f31dc18189adbf4616351/resources/src/mediawiki.api/index.js. Aborting pending requests has big implications for client and server logging as requests are quickly canceled, especially on slower connections. These differences can be observed on the network tab of DevTools and the log in Redux DevTools. Consider, for instance, the scenario of dwelling upon and quickly abandoning a single link prior to this patch: BOOT EVENT_LOGGED LINK_DWELL FETCH_START ABANDON_START FETCH_END STATSV_LOGGED ABANDON_END EVENT_LOGGED FETCH_COMPLETE And after this patch when the fetch timer is canceled (prior to an actual network request): BOOT EVENT_LOGGED LINK_DWELL ABANDON_START ABANDON_END EVENT_LOGGED In the above sequence, FETCH_* and STATSV_LOGGED actions never occur. And after this patch when the network request itself is canceled: BOOT EVENT_LOGGED LINK_DWELL FETCH_START ABANDON_START FETCH_FAILED STATSV_LOGGED FETCH_COMPLETE ABANDON_END EVENT_LOGGED FETCH_FAILED occurs intentionally, STATSV_LOGGED and FETCH_COMPLETE still happen even though the fetch didn't complete successfully, and FETCH_END doesn't. Additionally, since less data is transmitted, it's possible that the timing and success rate of logging will improve on low bandwidth connections. Also, this patch tries to revise the JSDocs where possible to support type checking and fix a call to the missing assert.fail() function in changeListener.test.js. Bug: T197700 Change-Id: I9a73b3086fc8fb0edd897a347b5497d5362e20ef
2018-06-25 13:26:11 +00:00
assert.propEqual(
this.dispatch.getCall( 0 ).args[ 0 ],
{
type: actionTypes.FETCH_START,
el: this.el,
title: 'Foo',
namespaceId: 0,
Update: cancel unused HTTP requests in flight Whenever an HTTP request sequence is started, i.e. wait for the fetch start time, issue a network request, and return the result, abort the process if the results are known to no longer be needed. This occurs when a user has dwelt upon one link and then abandoned it either during the fetch start wait time or during the fetch network request itself. This change is accomplished by preserving the pending promises in two actions, LINK_DWELL and FETCH_START, and whenever the ABANDON_START action is issued, it now aborts any previously pending XHR-like promise, called a "AbortPromise" which is just a thenable with an abort() method. There is a similar concept in Core: https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/core/+/ecc812f06e7dff587b3f31dc18189adbf4616351/resources/src/mediawiki.api/index.js. Aborting pending requests has big implications for client and server logging as requests are quickly canceled, especially on slower connections. These differences can be observed on the network tab of DevTools and the log in Redux DevTools. Consider, for instance, the scenario of dwelling upon and quickly abandoning a single link prior to this patch: BOOT EVENT_LOGGED LINK_DWELL FETCH_START ABANDON_START FETCH_END STATSV_LOGGED ABANDON_END EVENT_LOGGED FETCH_COMPLETE And after this patch when the fetch timer is canceled (prior to an actual network request): BOOT EVENT_LOGGED LINK_DWELL ABANDON_START ABANDON_END EVENT_LOGGED In the above sequence, FETCH_* and STATSV_LOGGED actions never occur. And after this patch when the network request itself is canceled: BOOT EVENT_LOGGED LINK_DWELL FETCH_START ABANDON_START FETCH_FAILED STATSV_LOGGED FETCH_COMPLETE ABANDON_END EVENT_LOGGED FETCH_FAILED occurs intentionally, STATSV_LOGGED and FETCH_COMPLETE still happen even though the fetch didn't complete successfully, and FETCH_END doesn't. Additionally, since less data is transmitted, it's possible that the timing and success rate of logging will improve on low bandwidth connections. Also, this patch tries to revise the JSDocs where possible to support type checking and fix a call to the missing assert.fail() function in changeListener.test.js. Bug: T197700 Change-Id: I9a73b3086fc8fb0edd897a347b5497d5362e20ef
2018-06-25 13:26:11 +00:00
timestamp: this.now,
promise: $.Deferred().promise( { abort() {} } )
},
'It dispatches the FETCH_START action immediately.'
);
} );
QUnit.test( 'it should dispatch the FETCH_END action when the API request ends', function ( assert ) {
const fetched = this.fetch();
this.now += 115;
this.gatewayDeferred.resolve( {} );
Improve & fix action and integration tests This change fixes some issues with assertions not running, removes unnecessary promise dances, and improves legibility and some code patterns in the action and integration node tests mainly. Detailed changes: * actions.js * Fully migrate out of jQuery 1 promises (no done/fail) * Fix linkDwell action not returning the fetch action promise * actions.test.js * Simplify setupWait for the tests * It always autoresolves immediately the wait call to ensure speedy tests * No waitDeferreds or waitPromises array coordination, rely on action returned promises instead * Get rid of that = this in favor of arrow functions * Rename generic "p" promises to meaningful names * Add assert.expect for more solid tests (so that we don't skip assertions in the future if we change them) * Fix some assertions that weren't being run because of the incorrect promise being returned (p.then, and then just returning p) * Get rid of $.when stubbing in favor of waiting for the promise returned from the action * Get rid of hacky setTimeout(..., 0) to run assertions after the promises * integration.test.js * Get rid of wait(0) calls to hack around asynchronous actions * Use the action returned promises instead of the waitPromises/Deferreds * Remove unused "el" parameter being passed to this.abandon in several tests * Remove unnecessary test helper this.abandonPreview (it was the same as this.abandon) * Clarify a bit the last and more complex test with some comments and variable name changes * Get rid of that=this in favor of arrow functions * container.test.js * Get rid of that=this in favor of arrow functions * previewBehavior.test.js * Get rid of that=this in favor of arrow functions * Get rid of $.each in favor of .forEach Bug: T170807 Change-Id: I06fafd92a1712f143018e2ddff24fadf1a6882b3
2018-02-16 12:52:56 +00:00
return fetched.then( () => {
assert.deepEqual(
Improve & fix action and integration tests This change fixes some issues with assertions not running, removes unnecessary promise dances, and improves legibility and some code patterns in the action and integration node tests mainly. Detailed changes: * actions.js * Fully migrate out of jQuery 1 promises (no done/fail) * Fix linkDwell action not returning the fetch action promise * actions.test.js * Simplify setupWait for the tests * It always autoresolves immediately the wait call to ensure speedy tests * No waitDeferreds or waitPromises array coordination, rely on action returned promises instead * Get rid of that = this in favor of arrow functions * Rename generic "p" promises to meaningful names * Add assert.expect for more solid tests (so that we don't skip assertions in the future if we change them) * Fix some assertions that weren't being run because of the incorrect promise being returned (p.then, and then just returning p) * Get rid of $.when stubbing in favor of waiting for the promise returned from the action * Get rid of hacky setTimeout(..., 0) to run assertions after the promises * integration.test.js * Get rid of wait(0) calls to hack around asynchronous actions * Use the action returned promises instead of the waitPromises/Deferreds * Remove unused "el" parameter being passed to this.abandon in several tests * Remove unnecessary test helper this.abandonPreview (it was the same as this.abandon) * Clarify a bit the last and more complex test with some comments and variable name changes * Get rid of that=this in favor of arrow functions * container.test.js * Get rid of that=this in favor of arrow functions * previewBehavior.test.js * Get rid of that=this in favor of arrow functions * Get rid of $.each in favor of .forEach Bug: T170807 Change-Id: I06fafd92a1712f143018e2ddff24fadf1a6882b3
2018-02-16 12:52:56 +00:00
this.dispatch.getCall( 1 ).args[ 0 ],
{
type: actionTypes.FETCH_END,
Improve & fix action and integration tests This change fixes some issues with assertions not running, removes unnecessary promise dances, and improves legibility and some code patterns in the action and integration node tests mainly. Detailed changes: * actions.js * Fully migrate out of jQuery 1 promises (no done/fail) * Fix linkDwell action not returning the fetch action promise * actions.test.js * Simplify setupWait for the tests * It always autoresolves immediately the wait call to ensure speedy tests * No waitDeferreds or waitPromises array coordination, rely on action returned promises instead * Get rid of that = this in favor of arrow functions * Rename generic "p" promises to meaningful names * Add assert.expect for more solid tests (so that we don't skip assertions in the future if we change them) * Fix some assertions that weren't being run because of the incorrect promise being returned (p.then, and then just returning p) * Get rid of $.when stubbing in favor of waiting for the promise returned from the action * Get rid of hacky setTimeout(..., 0) to run assertions after the promises * integration.test.js * Get rid of wait(0) calls to hack around asynchronous actions * Use the action returned promises instead of the waitPromises/Deferreds * Remove unused "el" parameter being passed to this.abandon in several tests * Remove unnecessary test helper this.abandonPreview (it was the same as this.abandon) * Clarify a bit the last and more complex test with some comments and variable name changes * Get rid of that=this in favor of arrow functions * container.test.js * Get rid of that=this in favor of arrow functions * previewBehavior.test.js * Get rid of that=this in favor of arrow functions * Get rid of $.each in favor of .forEach Bug: T170807 Change-Id: I06fafd92a1712f143018e2ddff24fadf1a6882b3
2018-02-16 12:52:56 +00:00
el: this.el,
timestamp: 115
},
'The dispatcher was called with the correct arguments.'
);
} );
} );
QUnit.test( 'it should delay dispatching the FETCH_COMPLETE action', function ( assert ) {
const result = {},
Improve & fix action and integration tests This change fixes some issues with assertions not running, removes unnecessary promise dances, and improves legibility and some code patterns in the action and integration node tests mainly. Detailed changes: * actions.js * Fully migrate out of jQuery 1 promises (no done/fail) * Fix linkDwell action not returning the fetch action promise * actions.test.js * Simplify setupWait for the tests * It always autoresolves immediately the wait call to ensure speedy tests * No waitDeferreds or waitPromises array coordination, rely on action returned promises instead * Get rid of that = this in favor of arrow functions * Rename generic "p" promises to meaningful names * Add assert.expect for more solid tests (so that we don't skip assertions in the future if we change them) * Fix some assertions that weren't being run because of the incorrect promise being returned (p.then, and then just returning p) * Get rid of $.when stubbing in favor of waiting for the promise returned from the action * Get rid of hacky setTimeout(..., 0) to run assertions after the promises * integration.test.js * Get rid of wait(0) calls to hack around asynchronous actions * Use the action returned promises instead of the waitPromises/Deferreds * Remove unused "el" parameter being passed to this.abandon in several tests * Remove unnecessary test helper this.abandonPreview (it was the same as this.abandon) * Clarify a bit the last and more complex test with some comments and variable name changes * Get rid of that=this in favor of arrow functions * container.test.js * Get rid of that=this in favor of arrow functions * previewBehavior.test.js * Get rid of that=this in favor of arrow functions * Get rid of $.each in favor of .forEach Bug: T170807 Change-Id: I06fafd92a1712f143018e2ddff24fadf1a6882b3
2018-02-16 12:52:56 +00:00
fetched = this.fetch();
assert.strictEqual(
this.wait.getCall( 0 ).args[ 0 ],
350,
'It waits for FETCH_COMPLETE_TARGET_DELAY - FETCH_START_DELAY milliseconds.'
);
Improve & fix action and integration tests This change fixes some issues with assertions not running, removes unnecessary promise dances, and improves legibility and some code patterns in the action and integration node tests mainly. Detailed changes: * actions.js * Fully migrate out of jQuery 1 promises (no done/fail) * Fix linkDwell action not returning the fetch action promise * actions.test.js * Simplify setupWait for the tests * It always autoresolves immediately the wait call to ensure speedy tests * No waitDeferreds or waitPromises array coordination, rely on action returned promises instead * Get rid of that = this in favor of arrow functions * Rename generic "p" promises to meaningful names * Add assert.expect for more solid tests (so that we don't skip assertions in the future if we change them) * Fix some assertions that weren't being run because of the incorrect promise being returned (p.then, and then just returning p) * Get rid of $.when stubbing in favor of waiting for the promise returned from the action * Get rid of hacky setTimeout(..., 0) to run assertions after the promises * integration.test.js * Get rid of wait(0) calls to hack around asynchronous actions * Use the action returned promises instead of the waitPromises/Deferreds * Remove unused "el" parameter being passed to this.abandon in several tests * Remove unnecessary test helper this.abandonPreview (it was the same as this.abandon) * Clarify a bit the last and more complex test with some comments and variable name changes * Get rid of that=this in favor of arrow functions * container.test.js * Get rid of that=this in favor of arrow functions * previewBehavior.test.js * Get rid of that=this in favor of arrow functions * Get rid of $.each in favor of .forEach Bug: T170807 Change-Id: I06fafd92a1712f143018e2ddff24fadf1a6882b3
2018-02-16 12:52:56 +00:00
this.gatewayDeferred.resolve( result );
Improve & fix action and integration tests This change fixes some issues with assertions not running, removes unnecessary promise dances, and improves legibility and some code patterns in the action and integration node tests mainly. Detailed changes: * actions.js * Fully migrate out of jQuery 1 promises (no done/fail) * Fix linkDwell action not returning the fetch action promise * actions.test.js * Simplify setupWait for the tests * It always autoresolves immediately the wait call to ensure speedy tests * No waitDeferreds or waitPromises array coordination, rely on action returned promises instead * Get rid of that = this in favor of arrow functions * Rename generic "p" promises to meaningful names * Add assert.expect for more solid tests (so that we don't skip assertions in the future if we change them) * Fix some assertions that weren't being run because of the incorrect promise being returned (p.then, and then just returning p) * Get rid of $.when stubbing in favor of waiting for the promise returned from the action * Get rid of hacky setTimeout(..., 0) to run assertions after the promises * integration.test.js * Get rid of wait(0) calls to hack around asynchronous actions * Use the action returned promises instead of the waitPromises/Deferreds * Remove unused "el" parameter being passed to this.abandon in several tests * Remove unnecessary test helper this.abandonPreview (it was the same as this.abandon) * Clarify a bit the last and more complex test with some comments and variable name changes * Get rid of that=this in favor of arrow functions * container.test.js * Get rid of that=this in favor of arrow functions * previewBehavior.test.js * Get rid of that=this in favor of arrow functions * Get rid of $.each in favor of .forEach Bug: T170807 Change-Id: I06fafd92a1712f143018e2ddff24fadf1a6882b3
2018-02-16 12:52:56 +00:00
return fetched.then( () => {
assert.deepEqual(
this.dispatch.getCall( 2 ).args[ 0 ],
{
type: actionTypes.FETCH_COMPLETE,
Improve & fix action and integration tests This change fixes some issues with assertions not running, removes unnecessary promise dances, and improves legibility and some code patterns in the action and integration node tests mainly. Detailed changes: * actions.js * Fully migrate out of jQuery 1 promises (no done/fail) * Fix linkDwell action not returning the fetch action promise * actions.test.js * Simplify setupWait for the tests * It always autoresolves immediately the wait call to ensure speedy tests * No waitDeferreds or waitPromises array coordination, rely on action returned promises instead * Get rid of that = this in favor of arrow functions * Rename generic "p" promises to meaningful names * Add assert.expect for more solid tests (so that we don't skip assertions in the future if we change them) * Fix some assertions that weren't being run because of the incorrect promise being returned (p.then, and then just returning p) * Get rid of $.when stubbing in favor of waiting for the promise returned from the action * Get rid of hacky setTimeout(..., 0) to run assertions after the promises * integration.test.js * Get rid of wait(0) calls to hack around asynchronous actions * Use the action returned promises instead of the waitPromises/Deferreds * Remove unused "el" parameter being passed to this.abandon in several tests * Remove unnecessary test helper this.abandonPreview (it was the same as this.abandon) * Clarify a bit the last and more complex test with some comments and variable name changes * Get rid of that=this in favor of arrow functions * container.test.js * Get rid of that=this in favor of arrow functions * previewBehavior.test.js * Get rid of that=this in favor of arrow functions * Get rid of $.each in favor of .forEach Bug: T170807 Change-Id: I06fafd92a1712f143018e2ddff24fadf1a6882b3
2018-02-16 12:52:56 +00:00
el: this.el,
result,
Improve & fix action and integration tests This change fixes some issues with assertions not running, removes unnecessary promise dances, and improves legibility and some code patterns in the action and integration node tests mainly. Detailed changes: * actions.js * Fully migrate out of jQuery 1 promises (no done/fail) * Fix linkDwell action not returning the fetch action promise * actions.test.js * Simplify setupWait for the tests * It always autoresolves immediately the wait call to ensure speedy tests * No waitDeferreds or waitPromises array coordination, rely on action returned promises instead * Get rid of that = this in favor of arrow functions * Rename generic "p" promises to meaningful names * Add assert.expect for more solid tests (so that we don't skip assertions in the future if we change them) * Fix some assertions that weren't being run because of the incorrect promise being returned (p.then, and then just returning p) * Get rid of $.when stubbing in favor of waiting for the promise returned from the action * Get rid of hacky setTimeout(..., 0) to run assertions after the promises * integration.test.js * Get rid of wait(0) calls to hack around asynchronous actions * Use the action returned promises instead of the waitPromises/Deferreds * Remove unused "el" parameter being passed to this.abandon in several tests * Remove unnecessary test helper this.abandonPreview (it was the same as this.abandon) * Clarify a bit the last and more complex test with some comments and variable name changes * Get rid of that=this in favor of arrow functions * container.test.js * Get rid of that=this in favor of arrow functions * previewBehavior.test.js * Get rid of that=this in favor of arrow functions * Get rid of $.each in favor of .forEach Bug: T170807 Change-Id: I06fafd92a1712f143018e2ddff24fadf1a6882b3
2018-02-16 12:52:56 +00:00
token: this.token
},
'The dispatcher was called with the correct arguments.'
Improve & fix action and integration tests This change fixes some issues with assertions not running, removes unnecessary promise dances, and improves legibility and some code patterns in the action and integration node tests mainly. Detailed changes: * actions.js * Fully migrate out of jQuery 1 promises (no done/fail) * Fix linkDwell action not returning the fetch action promise * actions.test.js * Simplify setupWait for the tests * It always autoresolves immediately the wait call to ensure speedy tests * No waitDeferreds or waitPromises array coordination, rely on action returned promises instead * Get rid of that = this in favor of arrow functions * Rename generic "p" promises to meaningful names * Add assert.expect for more solid tests (so that we don't skip assertions in the future if we change them) * Fix some assertions that weren't being run because of the incorrect promise being returned (p.then, and then just returning p) * Get rid of $.when stubbing in favor of waiting for the promise returned from the action * Get rid of hacky setTimeout(..., 0) to run assertions after the promises * integration.test.js * Get rid of wait(0) calls to hack around asynchronous actions * Use the action returned promises instead of the waitPromises/Deferreds * Remove unused "el" parameter being passed to this.abandon in several tests * Remove unnecessary test helper this.abandonPreview (it was the same as this.abandon) * Clarify a bit the last and more complex test with some comments and variable name changes * Get rid of that=this in favor of arrow functions * container.test.js * Get rid of that=this in favor of arrow functions * previewBehavior.test.js * Get rid of that=this in favor of arrow functions * Get rid of $.each in favor of .forEach Bug: T170807 Change-Id: I06fafd92a1712f143018e2ddff24fadf1a6882b3
2018-02-16 12:52:56 +00:00
);
} );
} );
QUnit.test( 'it should dispatch the FETCH_FAILED action when the request fails', function ( assert ) {
const fetched = this.fetch();
this.gatewayDeferred.reject( new Error( 'API req failed' ) );
Improve & fix action and integration tests This change fixes some issues with assertions not running, removes unnecessary promise dances, and improves legibility and some code patterns in the action and integration node tests mainly. Detailed changes: * actions.js * Fully migrate out of jQuery 1 promises (no done/fail) * Fix linkDwell action not returning the fetch action promise * actions.test.js * Simplify setupWait for the tests * It always autoresolves immediately the wait call to ensure speedy tests * No waitDeferreds or waitPromises array coordination, rely on action returned promises instead * Get rid of that = this in favor of arrow functions * Rename generic "p" promises to meaningful names * Add assert.expect for more solid tests (so that we don't skip assertions in the future if we change them) * Fix some assertions that weren't being run because of the incorrect promise being returned (p.then, and then just returning p) * Get rid of $.when stubbing in favor of waiting for the promise returned from the action * Get rid of hacky setTimeout(..., 0) to run assertions after the promises * integration.test.js * Get rid of wait(0) calls to hack around asynchronous actions * Use the action returned promises instead of the waitPromises/Deferreds * Remove unused "el" parameter being passed to this.abandon in several tests * Remove unnecessary test helper this.abandonPreview (it was the same as this.abandon) * Clarify a bit the last and more complex test with some comments and variable name changes * Get rid of that=this in favor of arrow functions * container.test.js * Get rid of that=this in favor of arrow functions * previewBehavior.test.js * Get rid of that=this in favor of arrow functions * Get rid of $.each in favor of .forEach Bug: T170807 Change-Id: I06fafd92a1712f143018e2ddff24fadf1a6882b3
2018-02-16 12:52:56 +00:00
this.now += 115;
return fetched.then( () => {
assert.strictEqual(
Improve & fix action and integration tests This change fixes some issues with assertions not running, removes unnecessary promise dances, and improves legibility and some code patterns in the action and integration node tests mainly. Detailed changes: * actions.js * Fully migrate out of jQuery 1 promises (no done/fail) * Fix linkDwell action not returning the fetch action promise * actions.test.js * Simplify setupWait for the tests * It always autoresolves immediately the wait call to ensure speedy tests * No waitDeferreds or waitPromises array coordination, rely on action returned promises instead * Get rid of that = this in favor of arrow functions * Rename generic "p" promises to meaningful names * Add assert.expect for more solid tests (so that we don't skip assertions in the future if we change them) * Fix some assertions that weren't being run because of the incorrect promise being returned (p.then, and then just returning p) * Get rid of $.when stubbing in favor of waiting for the promise returned from the action * Get rid of hacky setTimeout(..., 0) to run assertions after the promises * integration.test.js * Get rid of wait(0) calls to hack around asynchronous actions * Use the action returned promises instead of the waitPromises/Deferreds * Remove unused "el" parameter being passed to this.abandon in several tests * Remove unnecessary test helper this.abandonPreview (it was the same as this.abandon) * Clarify a bit the last and more complex test with some comments and variable name changes * Get rid of that=this in favor of arrow functions * container.test.js * Get rid of that=this in favor of arrow functions * previewBehavior.test.js * Get rid of that=this in favor of arrow functions * Get rid of $.each in favor of .forEach Bug: T170807 Change-Id: I06fafd92a1712f143018e2ddff24fadf1a6882b3
2018-02-16 12:52:56 +00:00
this.dispatch.callCount, 3,
'dispatch called thrice, START, FAILED, and COMPLETE'
);
assert.deepEqual(
Improve & fix action and integration tests This change fixes some issues with assertions not running, removes unnecessary promise dances, and improves legibility and some code patterns in the action and integration node tests mainly. Detailed changes: * actions.js * Fully migrate out of jQuery 1 promises (no done/fail) * Fix linkDwell action not returning the fetch action promise * actions.test.js * Simplify setupWait for the tests * It always autoresolves immediately the wait call to ensure speedy tests * No waitDeferreds or waitPromises array coordination, rely on action returned promises instead * Get rid of that = this in favor of arrow functions * Rename generic "p" promises to meaningful names * Add assert.expect for more solid tests (so that we don't skip assertions in the future if we change them) * Fix some assertions that weren't being run because of the incorrect promise being returned (p.then, and then just returning p) * Get rid of $.when stubbing in favor of waiting for the promise returned from the action * Get rid of hacky setTimeout(..., 0) to run assertions after the promises * integration.test.js * Get rid of wait(0) calls to hack around asynchronous actions * Use the action returned promises instead of the waitPromises/Deferreds * Remove unused "el" parameter being passed to this.abandon in several tests * Remove unnecessary test helper this.abandonPreview (it was the same as this.abandon) * Clarify a bit the last and more complex test with some comments and variable name changes * Get rid of that=this in favor of arrow functions * container.test.js * Get rid of that=this in favor of arrow functions * previewBehavior.test.js * Get rid of that=this in favor of arrow functions * Get rid of $.each in favor of .forEach Bug: T170807 Change-Id: I06fafd92a1712f143018e2ddff24fadf1a6882b3
2018-02-16 12:52:56 +00:00
this.dispatch.getCall( 1 ).args[ 0 ],
{
type: actionTypes.FETCH_FAILED,
Fix action reducer forgetting *all* duplicate dwell actions I was able to track the issue down to the changes made in this patch: https://gerrit.wikimedia.org/r/331563 This patch is mostly a simplification of ce8a2d4 (I9a73b3086fc8fb0edd897a347b5497d5362e20ef): - Don't make wait#wait() abortable. This adds complexity and isn't needed since the token is rechecked in the .then() of actions#linkDwell(). The request is permitted to continue and fetch if that token still matches and is never issued otherwise. Once a request has been issued, that request is still abortable. However, note that calling XHR.abort() is just a request to abort and may not be granted. Whether or not XHR.catch() is invoked is what dispatches the FETCH_ABORTED action (or doesn't if the request to abort was denied). - Remove the abort tests for wait#wait() since the functionality is no longer provided. - Pass the preview token in the FETCH_ABORTED action and reduce FETCH_ABORTED the same way as ABANDON_COMPLETE in reducers/preview. The follow-up patch, I3597b8025f7a12db0cf5d83cce5a77abace9bae3, adds integration tests for the specific bug fix. Note that these Selenium tests are incompatible with the content proxy, so it is probably best to simply unset $wgPopupsRestGatewayEndpoint and $wgPopupsGateway and allow the defaults to be used. Also note that the tests are incompatible with recent versions of Node.js (so use NVM) and emit many deprecation warnings (so set deprecationWarnings to false in tests/selenium/wdio.conf.js) An example run of the tests looks something like: chromedriver --url-base=wd/hub --port=4444 & # If any changes are made locally, also run `npm -s start &`. MW_SERVER=http://localhost:8181 \ MEDIAWIKI_USER=foo \ MEDIAWIKI_PASSWORD=bar \ DISPLAY= \ npm -s run selenium-test Live testing may be performed as well. Remember that RESTBase requests are incompatible with MobileFrontend's content proxy hack so ensure to comment it out if $wgPopupsGateway is configured for RESTBase (see T218159). 1. Open the DevTools network tab. 2. Disable the browser cache. Chromium, at least, won't abort requests coming form the cache. 3. Hover back and forth quickly over a preview. In Chromium, canceled requests are labeled and appear red. This is a good scenario to test. With the patch, a preview should always be shown when ultimately resting on a link. Without the patch, it is possible to rest upon the link with no preview showing. This may require several attempts. Bug: T219434 Change-Id: I9da84b0296dd14e9ce69cb35f1ca475272fb249a
2019-02-05 17:38:22 +00:00
el: this.el,
token: this.token
},
'The dispatcher was called with the correct arguments.'
);
} );
} );
QUnit.test( 'it should dispatch the FETCH_FAILED action when the request fails even after the wait timeout', function ( assert ) {
Improve & fix action and integration tests This change fixes some issues with assertions not running, removes unnecessary promise dances, and improves legibility and some code patterns in the action and integration node tests mainly. Detailed changes: * actions.js * Fully migrate out of jQuery 1 promises (no done/fail) * Fix linkDwell action not returning the fetch action promise * actions.test.js * Simplify setupWait for the tests * It always autoresolves immediately the wait call to ensure speedy tests * No waitDeferreds or waitPromises array coordination, rely on action returned promises instead * Get rid of that = this in favor of arrow functions * Rename generic "p" promises to meaningful names * Add assert.expect for more solid tests (so that we don't skip assertions in the future if we change them) * Fix some assertions that weren't being run because of the incorrect promise being returned (p.then, and then just returning p) * Get rid of $.when stubbing in favor of waiting for the promise returned from the action * Get rid of hacky setTimeout(..., 0) to run assertions after the promises * integration.test.js * Get rid of wait(0) calls to hack around asynchronous actions * Use the action returned promises instead of the waitPromises/Deferreds * Remove unused "el" parameter being passed to this.abandon in several tests * Remove unnecessary test helper this.abandonPreview (it was the same as this.abandon) * Clarify a bit the last and more complex test with some comments and variable name changes * Get rid of that=this in favor of arrow functions * container.test.js * Get rid of that=this in favor of arrow functions * previewBehavior.test.js * Get rid of that=this in favor of arrow functions * Get rid of $.each in favor of .forEach Bug: T170807 Change-Id: I06fafd92a1712f143018e2ddff24fadf1a6882b3
2018-02-16 12:52:56 +00:00
// After the wait interval happens, resolve the gateway request
return this.waitPromise.then( () => {
this.gatewayDeferred.reject( new Error( 'API req failed' ) );
return this.fetch();
Improve & fix action and integration tests This change fixes some issues with assertions not running, removes unnecessary promise dances, and improves legibility and some code patterns in the action and integration node tests mainly. Detailed changes: * actions.js * Fully migrate out of jQuery 1 promises (no done/fail) * Fix linkDwell action not returning the fetch action promise * actions.test.js * Simplify setupWait for the tests * It always autoresolves immediately the wait call to ensure speedy tests * No waitDeferreds or waitPromises array coordination, rely on action returned promises instead * Get rid of that = this in favor of arrow functions * Rename generic "p" promises to meaningful names * Add assert.expect for more solid tests (so that we don't skip assertions in the future if we change them) * Fix some assertions that weren't being run because of the incorrect promise being returned (p.then, and then just returning p) * Get rid of $.when stubbing in favor of waiting for the promise returned from the action * Get rid of hacky setTimeout(..., 0) to run assertions after the promises * integration.test.js * Get rid of wait(0) calls to hack around asynchronous actions * Use the action returned promises instead of the waitPromises/Deferreds * Remove unused "el" parameter being passed to this.abandon in several tests * Remove unnecessary test helper this.abandonPreview (it was the same as this.abandon) * Clarify a bit the last and more complex test with some comments and variable name changes * Get rid of that=this in favor of arrow functions * container.test.js * Get rid of that=this in favor of arrow functions * previewBehavior.test.js * Get rid of that=this in favor of arrow functions * Get rid of $.each in favor of .forEach Bug: T170807 Change-Id: I06fafd92a1712f143018e2ddff24fadf1a6882b3
2018-02-16 12:52:56 +00:00
} ).then( () => {
assert.strictEqual(
Improve & fix action and integration tests This change fixes some issues with assertions not running, removes unnecessary promise dances, and improves legibility and some code patterns in the action and integration node tests mainly. Detailed changes: * actions.js * Fully migrate out of jQuery 1 promises (no done/fail) * Fix linkDwell action not returning the fetch action promise * actions.test.js * Simplify setupWait for the tests * It always autoresolves immediately the wait call to ensure speedy tests * No waitDeferreds or waitPromises array coordination, rely on action returned promises instead * Get rid of that = this in favor of arrow functions * Rename generic "p" promises to meaningful names * Add assert.expect for more solid tests (so that we don't skip assertions in the future if we change them) * Fix some assertions that weren't being run because of the incorrect promise being returned (p.then, and then just returning p) * Get rid of $.when stubbing in favor of waiting for the promise returned from the action * Get rid of hacky setTimeout(..., 0) to run assertions after the promises * integration.test.js * Get rid of wait(0) calls to hack around asynchronous actions * Use the action returned promises instead of the waitPromises/Deferreds * Remove unused "el" parameter being passed to this.abandon in several tests * Remove unnecessary test helper this.abandonPreview (it was the same as this.abandon) * Clarify a bit the last and more complex test with some comments and variable name changes * Get rid of that=this in favor of arrow functions * container.test.js * Get rid of that=this in favor of arrow functions * previewBehavior.test.js * Get rid of that=this in favor of arrow functions * Get rid of $.each in favor of .forEach Bug: T170807 Change-Id: I06fafd92a1712f143018e2ddff24fadf1a6882b3
2018-02-16 12:52:56 +00:00
this.dispatch.callCount, 3,
'dispatch called thrice, START, FAILED, and COMPLETE'
);
assert.deepEqual(
Improve & fix action and integration tests This change fixes some issues with assertions not running, removes unnecessary promise dances, and improves legibility and some code patterns in the action and integration node tests mainly. Detailed changes: * actions.js * Fully migrate out of jQuery 1 promises (no done/fail) * Fix linkDwell action not returning the fetch action promise * actions.test.js * Simplify setupWait for the tests * It always autoresolves immediately the wait call to ensure speedy tests * No waitDeferreds or waitPromises array coordination, rely on action returned promises instead * Get rid of that = this in favor of arrow functions * Rename generic "p" promises to meaningful names * Add assert.expect for more solid tests (so that we don't skip assertions in the future if we change them) * Fix some assertions that weren't being run because of the incorrect promise being returned (p.then, and then just returning p) * Get rid of $.when stubbing in favor of waiting for the promise returned from the action * Get rid of hacky setTimeout(..., 0) to run assertions after the promises * integration.test.js * Get rid of wait(0) calls to hack around asynchronous actions * Use the action returned promises instead of the waitPromises/Deferreds * Remove unused "el" parameter being passed to this.abandon in several tests * Remove unnecessary test helper this.abandonPreview (it was the same as this.abandon) * Clarify a bit the last and more complex test with some comments and variable name changes * Get rid of that=this in favor of arrow functions * container.test.js * Get rid of that=this in favor of arrow functions * previewBehavior.test.js * Get rid of that=this in favor of arrow functions * Get rid of $.each in favor of .forEach Bug: T170807 Change-Id: I06fafd92a1712f143018e2ddff24fadf1a6882b3
2018-02-16 12:52:56 +00:00
this.dispatch.getCall( 1 ).args[ 0 ],
{
type: actionTypes.FETCH_FAILED,
Fix action reducer forgetting *all* duplicate dwell actions I was able to track the issue down to the changes made in this patch: https://gerrit.wikimedia.org/r/331563 This patch is mostly a simplification of ce8a2d4 (I9a73b3086fc8fb0edd897a347b5497d5362e20ef): - Don't make wait#wait() abortable. This adds complexity and isn't needed since the token is rechecked in the .then() of actions#linkDwell(). The request is permitted to continue and fetch if that token still matches and is never issued otherwise. Once a request has been issued, that request is still abortable. However, note that calling XHR.abort() is just a request to abort and may not be granted. Whether or not XHR.catch() is invoked is what dispatches the FETCH_ABORTED action (or doesn't if the request to abort was denied). - Remove the abort tests for wait#wait() since the functionality is no longer provided. - Pass the preview token in the FETCH_ABORTED action and reduce FETCH_ABORTED the same way as ABANDON_COMPLETE in reducers/preview. The follow-up patch, I3597b8025f7a12db0cf5d83cce5a77abace9bae3, adds integration tests for the specific bug fix. Note that these Selenium tests are incompatible with the content proxy, so it is probably best to simply unset $wgPopupsRestGatewayEndpoint and $wgPopupsGateway and allow the defaults to be used. Also note that the tests are incompatible with recent versions of Node.js (so use NVM) and emit many deprecation warnings (so set deprecationWarnings to false in tests/selenium/wdio.conf.js) An example run of the tests looks something like: chromedriver --url-base=wd/hub --port=4444 & # If any changes are made locally, also run `npm -s start &`. MW_SERVER=http://localhost:8181 \ MEDIAWIKI_USER=foo \ MEDIAWIKI_PASSWORD=bar \ DISPLAY= \ npm -s run selenium-test Live testing may be performed as well. Remember that RESTBase requests are incompatible with MobileFrontend's content proxy hack so ensure to comment it out if $wgPopupsGateway is configured for RESTBase (see T218159). 1. Open the DevTools network tab. 2. Disable the browser cache. Chromium, at least, won't abort requests coming form the cache. 3. Hover back and forth quickly over a preview. In Chromium, canceled requests are labeled and appear red. This is a good scenario to test. With the patch, a preview should always be shown when ultimately resting on a link. Without the patch, it is possible to rest upon the link with no preview showing. This may require several attempts. Bug: T219434 Change-Id: I9da84b0296dd14e9ce69cb35f1ca475272fb249a
2019-02-05 17:38:22 +00:00
el: this.el,
token: this.token
},
'The dispatcher was called with the correct arguments.'
);
} );
} );
QUnit.test( 'it should dispatch the FETCH_ABORTED action when the request is aborted', function ( assert ) {
const fetched = this.fetch();
this.now += 115;
this.gatewayDeferred.reject( 'http', {
textStatus: 'abort',
exception: 'abort',
xhr: {
readyState: 0
}
} );
return fetched.then( () => {
assert.strictEqual(
this.dispatch.callCount, 2,
'dispatch called twice with START and ABORT'
);
assert.deepEqual(
this.dispatch.getCall( 1 ).args[ 0 ],
{
type: actionTypes.FETCH_ABORTED,
Fix action reducer forgetting *all* duplicate dwell actions I was able to track the issue down to the changes made in this patch: https://gerrit.wikimedia.org/r/331563 This patch is mostly a simplification of ce8a2d4 (I9a73b3086fc8fb0edd897a347b5497d5362e20ef): - Don't make wait#wait() abortable. This adds complexity and isn't needed since the token is rechecked in the .then() of actions#linkDwell(). The request is permitted to continue and fetch if that token still matches and is never issued otherwise. Once a request has been issued, that request is still abortable. However, note that calling XHR.abort() is just a request to abort and may not be granted. Whether or not XHR.catch() is invoked is what dispatches the FETCH_ABORTED action (or doesn't if the request to abort was denied). - Remove the abort tests for wait#wait() since the functionality is no longer provided. - Pass the preview token in the FETCH_ABORTED action and reduce FETCH_ABORTED the same way as ABANDON_COMPLETE in reducers/preview. The follow-up patch, I3597b8025f7a12db0cf5d83cce5a77abace9bae3, adds integration tests for the specific bug fix. Note that these Selenium tests are incompatible with the content proxy, so it is probably best to simply unset $wgPopupsRestGatewayEndpoint and $wgPopupsGateway and allow the defaults to be used. Also note that the tests are incompatible with recent versions of Node.js (so use NVM) and emit many deprecation warnings (so set deprecationWarnings to false in tests/selenium/wdio.conf.js) An example run of the tests looks something like: chromedriver --url-base=wd/hub --port=4444 & # If any changes are made locally, also run `npm -s start &`. MW_SERVER=http://localhost:8181 \ MEDIAWIKI_USER=foo \ MEDIAWIKI_PASSWORD=bar \ DISPLAY= \ npm -s run selenium-test Live testing may be performed as well. Remember that RESTBase requests are incompatible with MobileFrontend's content proxy hack so ensure to comment it out if $wgPopupsGateway is configured for RESTBase (see T218159). 1. Open the DevTools network tab. 2. Disable the browser cache. Chromium, at least, won't abort requests coming form the cache. 3. Hover back and forth quickly over a preview. In Chromium, canceled requests are labeled and appear red. This is a good scenario to test. With the patch, a preview should always be shown when ultimately resting on a link. Without the patch, it is possible to rest upon the link with no preview showing. This may require several attempts. Bug: T219434 Change-Id: I9da84b0296dd14e9ce69cb35f1ca475272fb249a
2019-02-05 17:38:22 +00:00
el: this.el,
token: this.token
},
'The dispatcher was called with the correct arguments.'
);
} );
} );
QUnit.module( 'ext.popups/actions#abandon', {
beforeEach() {
Improve & fix action and integration tests This change fixes some issues with assertions not running, removes unnecessary promise dances, and improves legibility and some code patterns in the action and integration node tests mainly. Detailed changes: * actions.js * Fully migrate out of jQuery 1 promises (no done/fail) * Fix linkDwell action not returning the fetch action promise * actions.test.js * Simplify setupWait for the tests * It always autoresolves immediately the wait call to ensure speedy tests * No waitDeferreds or waitPromises array coordination, rely on action returned promises instead * Get rid of that = this in favor of arrow functions * Rename generic "p" promises to meaningful names * Add assert.expect for more solid tests (so that we don't skip assertions in the future if we change them) * Fix some assertions that weren't being run because of the incorrect promise being returned (p.then, and then just returning p) * Get rid of $.when stubbing in favor of waiting for the promise returned from the action * Get rid of hacky setTimeout(..., 0) to run assertions after the promises * integration.test.js * Get rid of wait(0) calls to hack around asynchronous actions * Use the action returned promises instead of the waitPromises/Deferreds * Remove unused "el" parameter being passed to this.abandon in several tests * Remove unnecessary test helper this.abandonPreview (it was the same as this.abandon) * Clarify a bit the last and more complex test with some comments and variable name changes * Get rid of that=this in favor of arrow functions * container.test.js * Get rid of that=this in favor of arrow functions * previewBehavior.test.js * Get rid of that=this in favor of arrow functions * Get rid of $.each in favor of .forEach Bug: T170807 Change-Id: I06fafd92a1712f143018e2ddff24fadf1a6882b3
2018-02-16 12:52:56 +00:00
setupWait( this );
setupEl( this );
}
} );
QUnit.test( 'it should dispatch start and end actions', function ( assert ) {
const dispatch = this.sandbox.spy(),
token = '0123456789',
getState = () =>
( {
preview: {
Update: cancel unused HTTP requests in flight Whenever an HTTP request sequence is started, i.e. wait for the fetch start time, issue a network request, and return the result, abort the process if the results are known to no longer be needed. This occurs when a user has dwelt upon one link and then abandoned it either during the fetch start wait time or during the fetch network request itself. This change is accomplished by preserving the pending promises in two actions, LINK_DWELL and FETCH_START, and whenever the ABANDON_START action is issued, it now aborts any previously pending XHR-like promise, called a "AbortPromise" which is just a thenable with an abort() method. There is a similar concept in Core: https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/core/+/ecc812f06e7dff587b3f31dc18189adbf4616351/resources/src/mediawiki.api/index.js. Aborting pending requests has big implications for client and server logging as requests are quickly canceled, especially on slower connections. These differences can be observed on the network tab of DevTools and the log in Redux DevTools. Consider, for instance, the scenario of dwelling upon and quickly abandoning a single link prior to this patch: BOOT EVENT_LOGGED LINK_DWELL FETCH_START ABANDON_START FETCH_END STATSV_LOGGED ABANDON_END EVENT_LOGGED FETCH_COMPLETE And after this patch when the fetch timer is canceled (prior to an actual network request): BOOT EVENT_LOGGED LINK_DWELL ABANDON_START ABANDON_END EVENT_LOGGED In the above sequence, FETCH_* and STATSV_LOGGED actions never occur. And after this patch when the network request itself is canceled: BOOT EVENT_LOGGED LINK_DWELL FETCH_START ABANDON_START FETCH_FAILED STATSV_LOGGED FETCH_COMPLETE ABANDON_END EVENT_LOGGED FETCH_FAILED occurs intentionally, STATSV_LOGGED and FETCH_COMPLETE still happen even though the fetch didn't complete successfully, and FETCH_END doesn't. Additionally, since less data is transmitted, it's possible that the timing and success rate of logging will improve on low bandwidth connections. Also, this patch tries to revise the JSDocs where possible to support type checking and fix a call to the missing assert.fail() function in changeListener.test.js. Bug: T197700 Change-Id: I9a73b3086fc8fb0edd897a347b5497d5362e20ef
2018-06-25 13:26:11 +00:00
activeToken: token,
promise: $.Deferred().promise( { abort() {} } )
}
} );
Improve & fix action and integration tests This change fixes some issues with assertions not running, removes unnecessary promise dances, and improves legibility and some code patterns in the action and integration node tests mainly. Detailed changes: * actions.js * Fully migrate out of jQuery 1 promises (no done/fail) * Fix linkDwell action not returning the fetch action promise * actions.test.js * Simplify setupWait for the tests * It always autoresolves immediately the wait call to ensure speedy tests * No waitDeferreds or waitPromises array coordination, rely on action returned promises instead * Get rid of that = this in favor of arrow functions * Rename generic "p" promises to meaningful names * Add assert.expect for more solid tests (so that we don't skip assertions in the future if we change them) * Fix some assertions that weren't being run because of the incorrect promise being returned (p.then, and then just returning p) * Get rid of $.when stubbing in favor of waiting for the promise returned from the action * Get rid of hacky setTimeout(..., 0) to run assertions after the promises * integration.test.js * Get rid of wait(0) calls to hack around asynchronous actions * Use the action returned promises instead of the waitPromises/Deferreds * Remove unused "el" parameter being passed to this.abandon in several tests * Remove unnecessary test helper this.abandonPreview (it was the same as this.abandon) * Clarify a bit the last and more complex test with some comments and variable name changes * Get rid of that=this in favor of arrow functions * container.test.js * Get rid of that=this in favor of arrow functions * previewBehavior.test.js * Get rid of that=this in favor of arrow functions * Get rid of $.each in favor of .forEach Bug: T170807 Change-Id: I06fafd92a1712f143018e2ddff24fadf1a6882b3
2018-02-16 12:52:56 +00:00
this.sandbox.stub( mw, 'now' ).returns( new Date() );
const abandoned = actions.abandon()( dispatch, getState );
assert.true(
dispatch.calledWith( {
type: actionTypes.ABANDON_START,
timestamp: mw.now(),
token
} ),
'The dispatcher was called with the correct arguments.'
);
// ---
assert.true(
this.wait.calledWith( 300 ),
'Have you spoken with #Design about changing this value?'
);
return abandoned.then( () => {
assert.true(
dispatch.calledWith( {
type: actionTypes.ABANDON_END,
token
} ),
'ABANDON_* share the same token.'
);
} );
} );
QUnit.test( 'it shouldn\'t dispatch under certain conditions', function ( assert ) {
const dispatch = this.sandbox.spy(),
getState = () =>
( {
preview: {
activeToken: undefined
}
} );
return actions.abandon()( dispatch, getState )
.then( () => {
assert.strictEqual(
dispatch.callCount,
0,
'The dispatcher was not called.'
);
} );
} );
QUnit.module( 'ext.popups/actions#saveSettings' );
QUnit.test( 'it should dispatch an action with previous and current enabled state', function ( assert ) {
const dispatch = this.sandbox.spy(),
getState = this.sandbox.stub().returns( {
preview: {
enabled: { page: false }
}
} );
actions.saveSettings( { page: true } )( dispatch, getState );
assert.true(
getState.calledOnce,
'it should query the global state for the current state'
);
assert.true(
dispatch.calledWith( {
type: actionTypes.SETTINGS_CHANGE,
oldValue: { page: false },
newValue: { page: true }
} ),
'it should dispatch the action with the previous and next enabled state'
);
} );
QUnit.module( 'ext.popups/actions#previewShow', {
beforeEach() {
setupWait( this );
}
} );
QUnit.test( 'it should dispatch the PREVIEW_SHOW action and log a pageview', function ( assert ) {
const token = '1234567890',
dispatch = this.sandbox.spy(),
getState = this.sandbox.stub().returns( {
preview: {
activeToken: token,
fetchResponse: {
title: 'A',
pageId: 42,
type: 'page'
}
}
} );
this.sandbox.stub( mw, 'now' ).returns( new Date() );
const previewShow = actions
.previewShow( token )( dispatch, getState );
assert.true(
dispatch.calledWith( {
type: actionTypes.PREVIEW_SHOW,
token,
timestamp: mw.now()
} ),
'dispatches the preview show event'
);
assert.strictEqual(
this.wait.getCall( 0 ).args[ 0 ],
1000,
'It waits for PAGEVIEW_VISIBILITY_DURATION milliseconds before trigging a pageview.'
);
return previewShow.then( () => {
assert.true(
dispatch.calledTwice,
'Dispatch was called twice - once for PREVIEW_SHOW then for PREVIEW_SEEN'
);
assert.true(
dispatch.calledWith( {
type: actionTypes.PREVIEW_SEEN,
namespace: 0,
pageId: 42,
title: 'A'
} ),
'Dispatches virtual pageview'
);
} );
} );
QUnit.test( 'PREVIEW_SEEN action not called if activeToken changes', function ( assert ) {
const token = '1234567890',
dispatch = this.sandbox.spy(),
getState = this.sandbox.stub().returns( {
preview: {
activeToken: '911',
fetchResponse: {
title: 'A',
type: 'page'
}
}
} );
// dispatch event
const previewShow = actions
.previewShow( token )( dispatch, getState );
return previewShow.then( () => {
assert.true(
dispatch.calledOnce,
'Dispatch was only called for PREVIEW_SHOW'
);
} );
} );
QUnit.test( 'PREVIEW_SEEN action not called if preview type not page', function ( assert ) {
const token = '1234567890',
dispatch = this.sandbox.spy(),
getState = this.sandbox.stub().returns( {
preview: {
activeToken: token,
fetchResponse: {
title: 'A',
type: 'empty'
}
}
} );
// dispatch event
const previewShow = actions
.previewShow( token )( dispatch, getState );
return previewShow.then( () => {
assert.true(
dispatch.calledOnce,
'Dispatch was only called for PREVIEW_SHOW'
);
} );
} );