tests: Add tests for the logic of typing_status.handle_text_input.

This tests was added to make sure we catch subtle bug related to
comparing new_recipient and current_recipient. When we changed the
recipient to use arrays instead of string to use new user IDs based
api we encoured this bug and out testing suite couldn't detect this.
This commit is contained in:
Priyank Patel 2019-06-07 01:17:35 +00:00 committed by Tim Abbott
parent b8250fc61e
commit b70bd6be30
3 changed files with 57 additions and 13 deletions

View File

@ -1,3 +1,6 @@
zrequire('typing');
zrequire('people');
zrequire('compose_pm_pill');
zrequire('typing_status');
function return_false() { return false; }
@ -277,4 +280,44 @@ run_test('basics', () => {
});
assert(events.idle_callback);
// test that we correctly detect if worker.get_recipient
// and typing_status.state.current_recipient are the same
worker.get_recipient = typing.get_recipient;
worker.is_valid_conversation = () => false;
compose_pm_pill.get_user_ids_string = () => '1,2,3';
typing_status.state.current_recipient = typing.get_recipient();
const call_count = {
maybe_ping_server: 0,
start_or_extend_idle_timer: 0,
stop_last_notification: 0,
};
// stub functions to see how may time they are called
for (const method in call_count) {
if (!call_count.hasOwnProperty(method)) { continue; }
typing_status[method] = function () {
call_count[method] += 1;
};
}
// User ids of poeple in compose narrow doesn't change and is same as stat.current_recipent
// so counts of function should increase except stop_last_notification
typing_status.handle_text_input(worker);
assert.deepEqual(call_count.maybe_ping_server, 1);
assert.deepEqual(call_count.start_or_extend_idle_timer, 1);
assert.deepEqual(call_count.stop_last_notification, 0);
typing_status.handle_text_input(worker);
assert.deepEqual(call_count.maybe_ping_server, 2);
assert.deepEqual(call_count.start_or_extend_idle_timer, 2);
assert.deepEqual(call_count.stop_last_notification, 0);
// change in recipient and new_recipient should make us
// call typing_status.stop_last_notification
compose_pm_pill.get_user_ids_string = () => '2,3,4';
typing_status.handle_text_input(worker);
assert.deepEqual(call_count.maybe_ping_server, 2);
assert.deepEqual(call_count.start_or_extend_idle_timer, 2);
assert.deepEqual(call_count.stop_last_notification, 1);
});

View File

@ -70,9 +70,10 @@ function notify_server_stop(user_ids_string) {
send_typing_notification_ajax(user_ids_string, "stop");
}
exports.get_recipient = get_user_ids_array;
exports.initialize = function () {
var worker = {
get_recipient: get_user_ids_array,
get_recipient: exports.get_recipient,
is_valid_conversation: is_valid_conversation,
get_current_time: get_current_time,
notify_server_start: notify_server_start,

View File

@ -42,23 +42,23 @@ exports.initialize_state = function () {
exports.initialize_state();
function stop_last_notification(worker) {
exports.stop_last_notification = function stop_last_notification(worker) {
var state = exports.state;
if (state.idle_timer) {
clearTimeout(state.idle_timer);
}
worker.notify_server_stop(state.current_recipient);
exports.initialize_state();
}
};
function start_or_extend_idle_timer(worker) {
exports.start_or_extend_idle_timer = function start_or_extend_idle_timer(worker) {
var state = exports.state;
function on_idle_timeout() {
// We don't do any real error checking here, because
// if we've been idle, we need to tell folks, and if
// our current recipient has changed, previous code will
// have stopped the timer.
stop_last_notification(worker);
exports.stop_last_notification(worker);
}
if (state.idle_timer) {
@ -68,7 +68,7 @@ function start_or_extend_idle_timer(worker) {
on_idle_timeout,
TYPING_STOPPED_WAIT_PERIOD
);
}
};
function set_next_start_time(current_time) {
exports.state.next_send_start_time = current_time + TYPING_STARTED_WAIT_PERIOD;
@ -79,12 +79,12 @@ function actually_ping_server(worker, recipient, current_time) {
set_next_start_time(current_time);
}
function maybe_ping_server(worker, recipient) {
exports.maybe_ping_server = function maybe_ping_server(worker, recipient) {
var current_time = worker.get_current_time();
if (current_time > exports.state.next_send_start_time) {
actually_ping_server(worker, recipient, current_time);
}
}
};
exports.handle_text_input = function (worker) {
var new_recipient = worker.get_recipient();
@ -96,10 +96,10 @@ exports.handle_text_input = function (worker) {
if (_.isEqual(new_recipient, current_recipient)) {
// Nothing has really changed, except we may need
// to send a ping to the server.
maybe_ping_server(worker, new_recipient);
exports.maybe_ping_server(worker, new_recipient);
// We can also extend out our idle time.
start_or_extend_idle_timer(worker);
exports.start_or_extend_idle_timer(worker);
return;
}
@ -107,7 +107,7 @@ exports.handle_text_input = function (worker) {
// We apparently stopped talking to our old recipient,
// so we must stop the old notification. Don't return
// yet, because we may have a new recipient.
stop_last_notification(worker);
exports.stop_last_notification(worker);
}
if (!worker.is_valid_conversation(new_recipient)) {
@ -121,7 +121,7 @@ exports.handle_text_input = function (worker) {
exports.state.current_recipient = new_recipient;
var current_time = worker.get_current_time();
actually_ping_server(worker, new_recipient, current_time);
start_or_extend_idle_timer(worker);
exports.start_or_extend_idle_timer(worker);
};
exports.stop = function (worker) {
@ -129,7 +129,7 @@ exports.stop = function (worker) {
// it doesn't necessarily mean we had typing indicators
// active before this.
if (exports.state.current_recipient) {
stop_last_notification(worker);
exports.stop_last_notification(worker);
}
};