pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[IMP] mail: correctly patch mail status manager timers

Browse Source 
We want to avoid that a `setTimeout` started in one test has an impact on other
tests. This is especially true with the status update loop. Since mail_service
is used in multiple tests, the safest solution is to patch timeouts everytime
we make a call to getMailServices.
There is no need for an unpatch, the patch become the default behaviour until
the page is refresh at the end of tests.

It is possible to manually add the patch after calling `getMailServices`
```
this.services = mailTestUtils.getMailServices(this);
this.timeoutMock = mailTestUtils.patchMailTimeouts();
```
This can be usefull if we want to get the timeoutMock to simulate time changes.

Task: 1856205
This commit is contained in:
XavierDo
2019-02-07 12:53:14 +00:00
parent 2d45df1081
commit 5c3aaacd2e
4 changed files with 137 additions and 74 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/mail/static/src/js/services/mail_status_manager.js
+4 -23
View File
@@ -3,6 +3,7 @@ odoo.define('mail.Manager.Status', function (require) {
var core = require('web.core');
var MailManager = require('mail.Manager');
var mailUtils = require('mail.utils');
var QWeb = core.qweb;
/**
@@ -33,8 +34,8 @@ MailManager.include({
// Add to list to call it in next bus update or _fetchMissingImStatus
this._imStatus[partnerID] = undefined;
// fetch after some time if no other getImStatus occurs
this._clearStatusServiceTimeout(this._fetchStatusTimeout);
this._fetchStatusTimeout = this._setStatusServiceTimeout(function () {
mailUtils.clearTimeout(this._fetchStatusTimeout);
this._fetchStatusTimeout = mailUtils.setTimeout(function () {
self._fetchMissingImStatus();
}, 500);
}
@@ -67,15 +68,6 @@ MailManager.include({
// Private
//--------------------------------------------------------------------------
/**
* A simple clearTimeout, useful for test
*
* @private
* @param {integer} ids
*/
_clearStatusServiceTimeout: function (id) {
clearTimeout(id);
},
/**
* Fetch the list of im_status for partner with id in ids list and triggers
* an update.
@@ -168,17 +160,6 @@ MailManager.include({
}
});
},
/**
* A simple setTimeout, useful for test
*
* @private
* @param {function} func
* @param {integer} duration
* @return {integer}
*/
_setStatusServiceTimeout: function (func, duration) {
return setTimeout(func, duration);
},
/**
* Once initialised, this loop will update the im_status of registered
* users.
@@ -191,7 +172,7 @@ MailManager.include({
if (!_.isNumber(counter)) {
counter = 0;
}
this._setStatusServiceTimeout(function () {
mailUtils.setTimeout(function () {
if (counter >= self._UPDATE_INTERVAL && self._isTabFocused) {
self._fetchImStatus({ partnerIDs: self._getImStatusToUpdate() });
counter = 0;
addons/mail/static/src/js/utils.js
+10
View File
@@ -91,6 +91,14 @@ function timeFromNow(date) {
return date.fromNow();
}
function o_clearTimeout(id) {
return clearTimeout(id);
}
function o_setTimeout(func, delay) {
return setTimeout(func, delay);
}
return {
addLink: addLink,
getTextToHTML: getTextToHTML,
@@ -100,6 +108,8 @@ return {
parseEmail: parseEmail,
stripHTML: stripHTML,
timeFromNow: timeFromNow,
clearTimeout: o_clearTimeout,
setTimeout: o_setTimeout,
};
});
addons/mail/static/tests/helpers/test_utils.js
+108
View File
@@ -5,6 +5,7 @@ var BusService = require('bus.BusService');
var Discuss = require('mail.Discuss');
var MailService = require('mail.Service');
var mailUtils = require('mail.utils');
var AbstractStorageService = require('web.AbstractStorageService');
var Class = require('web.Class');
@@ -79,6 +80,111 @@ var MockMailService = Class.extend({
},
});
/**
* Patch all the mailUtils.clearTimeout and mailUtils.setTimeout.
*
* @return {Object} helper functions, including unpatch and time management tools.
*/
var patchMailTimeouts = function () {
var currentTime = 0;
var timeouts = {};
var countTimeout = 0;
mailUtils.clearTimeout = function (id) {
delete timeouts[id];
};
mailUtils.setTimeout = function (func, duration) {
duration = duration || 0;
var executeTime = currentTime + duration;
countTimeout++;
timeouts[countTimeout] = {
executeTime: executeTime,
func: func
};
return countTimeout;
};
/**
* @return {integer|boolean} id of the next timeout in queue, false if queue is empty
*/
function getNextTimeoutId() {
var minKey = false;
_.each(timeouts, function (value, key) {
if (minKey === false) {
minKey = Number(key);
return;
}
var minTime = timeouts[minKey].executeTime;
if (value.executeTime < minTime || (value.executeTime === minTime && key < minKey)) {
minKey = Number(key);
}
});
return minKey;
}
/**
* @return {integer|boolean} delay (time interval) before the next timeout in queue is executed.
* Useful to know how much time to advance to execute next timer.
*/
function getNextTimeoutDelay() {
var next = getNextTimeoutId();
if (next === false) {
return false;
}
return timeouts[next].executeTime - currentTime;
}
/**
* Set the current time to given time
*
* @param {integer} time
*/
function setTime(time) {
var next = getNextTimeoutId();
if (next !== false && timeouts[next].executeTime <= time) {
currentTime = timeouts[next].executeTime;
var func = timeouts[next].func;
// watch out setTimeout inside setTimeout (recursive)
delete timeouts[next];
func();
setTime(time);
}
else {
currentTime = time;
}
}
/**
* Add the given time to current time
*
* @param {integer} time
*/
function addTime(time) {
setTime(currentTime + time);
}
/**
* Set time to the max time in queue and execute all timeouts before this time.
*/
function runPendingTimeouts() {
var maxTimeInQueue = 0;
_.each(timeouts, function (value, key) {
if (value.executeTime > maxTimeInQueue) {
maxTimeInQueue = value.executeTime;
}
});
setTime(maxTimeInQueue);
}
return {
addTime: addTime,
getNextTimeoutDelay:getNextTimeoutDelay,
runPendingTimeouts: runPendingTimeouts,
setTime: setTime,
};
};
/**
* Returns the list of mail services required by the mail components: a
* mail_service, and its two dependencies bus_service and local_storage.
@@ -87,6 +193,7 @@ var MockMailService = Class.extend({
* and local_storage, in that order
*/
function getMailServices() {
patchMailTimeouts();
return new MockMailService().getServices();
}
@@ -94,6 +201,7 @@ return {
MockMailService: MockMailService,
createDiscuss: createDiscuss,
getMailServices: getMailServices,
patchMailTimeouts: patchMailTimeouts,
};
});
addons/mail/static/tests/mail_status_manager_tests.js
+15 -51
View File
@@ -13,42 +13,12 @@ QUnit.module('mail', {}, function () {
QUnit.module('service', {}, function () {
QUnit.module('Status manager', {
beforeEach: function () {
var self = this;
this.services = mailTestUtils.getMailServices();
this.timeouts = {};
this.countTimeout = 0;
this.defaultPatchData = {
_clearStatusServiceTimeout: function (id) {
self.timeouts[id] = false;
},
_setStatusServiceTimeout: function (func, duration) {
var id = self.countTimeout;
self.timeouts[id] = func;
self.countTimeout++;
return id;
},
_updateImStatusLoop: function () {}, // avoid spam and infinite loop
};
this.patchMailService = function (patch) {
testUtils.mock.patch(this.services.mail_service, _.extend({}, this.defaultPatchData, patch));
};
this.resolveTimeouts = function () {
var timeouts = _.extend({}, self.timeouts);
self.timeouts = {}; // empty timeout before looping to avoid to remove loop timeout
_.each(_.values(timeouts), function (func) {
if (func !== false) {
func();
}
});
};
},
afterEach: function () {
testUtils.mock.unpatch(this.services.mail_service);
this.services = mailTestUtils.getMailServices(this);
this.timeoutMock = mailTestUtils.patchMailTimeouts();
},
});
QUnit.test('simple set im_status', function (assert) {
assert.expect(1);
this.patchMailService();
var parent = testUtils.createParent({
services: this.services,
mockRPC: function (route, args) {
@@ -64,13 +34,12 @@ QUnit.test('simple set im_status', function (assert) {
}]);
assert.strictEqual(parent.call('mail_service', 'getImStatus', { partnerID: 1 }), 'online');
this.resolveTimeouts(); // alway resolve timeout
this.timeoutMock.runPendingTimeouts();
parent.destroy();
});
QUnit.test('multi get_im_status', function (assert) {
assert.expect(9);
this.patchMailService();
var readCount = 0;
var parent = testUtils.createParent({
//data: this.data,
@@ -99,21 +68,19 @@ QUnit.test('multi get_im_status', function (assert) {
assert.strictEqual(parent.call('mail_service', 'getImStatus', { partnerID: 2 }), undefined);
assert.strictEqual(parent.call('mail_service', 'getImStatus', { partnerID: 3 }), undefined);
this.resolveTimeouts();
this.timeoutMock.runPendingTimeouts();
assert.strictEqual(parent.call('mail_service', 'getImStatus', { partnerID: 1 }), 'online');
assert.strictEqual(parent.call('mail_service', 'getImStatus', { partnerID: 2 }), 'away');
assert.strictEqual(parent.call('mail_service', 'getImStatus', { partnerID: 3 }), 'im_partner');
this.resolveTimeouts();
this.timeoutMock.runPendingTimeouts();
assert.strictEqual(readCount, 1, 'Only one read on partner should have been performed');
parent.destroy();
});
QUnit.test('update loop', function (assert) {
assert.expect(11);
delete this.defaultPatchData['_updateImStatusLoop']; // we want to test default behaviour of loop
this.patchMailService();
assert.expect(12);
var readCount = 0;
var parent = testUtils.createParent({
services: this.services,
@@ -140,13 +107,10 @@ QUnit.test('update loop', function (assert) {
{ id: 3, im_status: 'im_partner' }, //shouldn't be updated !!!!
]);
//_updateImStatusLoop should be running at one second per iteration, lets make a minute pass.
// this test is strongly dependant on the fact that the update loop as 1 second tick and update every 50 seconds.
assert.strictEqual(readCount, 0);
for (var i = 0; i < 50; i++){
this.resolveTimeouts();
}
this.timeoutMock.addTime(50*1000);
assert.strictEqual(readCount, 0);
this.resolveTimeouts(); //one more second pass
this.timeoutMock.addTime(1000);
assert.strictEqual(readCount, 1, 'one call should have been made after 50 seconds' );
assert.strictEqual(parent.call('mail_service', 'getImStatus', { partnerID: 1 }), 'online');
assert.strictEqual(parent.call('mail_service', 'getImStatus', { partnerID: 2 }), 'away');
@@ -154,16 +118,17 @@ QUnit.test('update loop', function (assert) {
//simulate change of focus
//original listener: $(window).on("blur", this._onWindowFocusChange.bind(this, false); + unload, ...
parent.call('mail_service', '_onWindowFocusChange', false); // remove focus from tab
for (var i = 0; i < 200; i++){ // a lot of time has passed
this.resolveTimeouts();
}
assert.strictEqual(readCount, 1, 'No more call should have been performed');
this.timeoutMock.addTime(5*60*1000); // x minutes without focus, no rpc should be done during this time
assert.strictEqual(readCount, 1, 'No more call should have been performed');
//simulate change of focus
//original listener: $(window).on("focus", this._onWindowFocusChange.bind(this, true);
parent.call('mail_service', '_onWindowFocusChange', true); // give focus to tab
this.resolveTimeouts(); //one more second pass
var nextUpdateDelay = this.timeoutMock.getNextTimeoutDelay();
assert.strictEqual(nextUpdateDelay, 1000, "next update should be done in maximum one second");
this.timeoutMock.addTime(nextUpdateDelay); // one second should be enough
assert.strictEqual(readCount, 2, 'One more call should have been done once tab focused');
this.timeoutMock.runPendingTimeouts();
parent.destroy();
});
@@ -171,7 +136,6 @@ QUnit.test('update status', function (assert) {
// the current solution to look for updatable im_status in dom is not perfect, but waiting for
// ability to include widgets in views, this is the most simple solution
assert.expect(2);
this.patchMailService();
var StatusWidget = Widget.extend({
start: function () {
this.render();
@@ -204,7 +168,7 @@ QUnit.test('update status', function (assert) {
{ id: 1, im_status: 'offline' },
]);
assert.notOk(statusWidget.$('.o_updatable_im_status i').hasClass('o_user_online'));
this.resolveTimeouts();
this.timeoutMock.runPendingTimeouts();
statusWidget.destroy();
});