From b70bd6be30c31f87fbd6068d64b73166000360f8 Mon Sep 17 00:00:00 2001 From: Priyank Patel Date: Fri, 7 Jun 2019 01:17:35 +0000 Subject: [PATCH] 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. --- frontend_tests/node_tests/typing_status.js | 43 ++++++++++++++++++++++ static/js/typing.js | 3 +- static/js/typing_status.js | 24 ++++++------ 3 files changed, 57 insertions(+), 13 deletions(-) diff --git a/frontend_tests/node_tests/typing_status.js b/frontend_tests/node_tests/typing_status.js index 7a323d64a7..aef577f55f 100644 --- a/frontend_tests/node_tests/typing_status.js +++ b/frontend_tests/node_tests/typing_status.js @@ -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); }); diff --git a/static/js/typing.js b/static/js/typing.js index 5bd1cc0f0c..ecb94cae88 100644 --- a/static/js/typing.js +++ b/static/js/typing.js @@ -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, diff --git a/static/js/typing_status.js b/static/js/typing_status.js index 84a7a89dd1..256b583837 100644 --- a/static/js/typing_status.js +++ b/static/js/typing_status.js @@ -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); } };