node_tests: Don't remove require cache of module in zrequire.

There is good reason to do this (explanation is bit long!). With the
TypeScript migration, and the require and ES6 migrations that come
with it, we use require instead of set_global which loads the entire
module. Suppose we have a util module, which is used by some other
module, say message_store, and util is being required in message_store
since it is removed from window. Then, if a test zrequires
message_store first, and then zrequires the util module qand mocks one
of its methods, it will not be mocked for the message_store
module. The reason is:
 1. zrequire('message_store') leads to require('util').
 2. zrequire('util') removes the util module from cache and it is
    reloaded. Now the util module in message_store and the one in
    the test will be different and any updates to it in tests won't
    be reflected in the actual code.

Which can lead to confusion for folks writing tests. I'll mention this
can be avoided doing zrequire('util') first but...that is not ideal.

And, since there was one outlier test that relied on this behavior,
we add the namespace.reset_module function.

.eslintrc.json

@@ -347,6 +347,7 @@
             "globals": {
                 "assert": false,
                 "document": false,
+                "reset_module": false,
                 "set_global": false,
                 "window": false,
                 "with_field": false,

frontend_tests/node_tests/recent_topics.js

@@ -293,6 +293,8 @@ function verify_topic_data(all_topics, stream, topic, last_msg_id, participated)
     assert.equal(topic_data.participated, participated);
 }
 
+let rt = zrequire("recent_topics");
+
 run_test("test_recent_topics_launch", () => {
     // Note: unread count and urls are fake,
     // since they are generated in external libraries
@@ -314,7 +316,7 @@ run_test("test_recent_topics_launch", () => {
         return "<recent_topics table stub>";
     });
 
-    const rt = zrequire("recent_topics");
+    rt = reset_module("recent_topics");
     rt.process_messages(messages);
 
     rt.launch();
@@ -351,7 +353,7 @@ run_test("test_filter_all", () => {
     // topic is not muted
     row_data = generate_topic_data([[1, "topic-1", 0, false, true]]);
     i = row_data.length;
-    const rt = zrequire("recent_topics");
+    rt = reset_module("recent_topics");
     rt.set_filter("all");
     rt.process_messages([messages[0]]);
 
@@ -391,7 +393,7 @@ run_test("test_filter_unread", () => {
     ]);
     let i = 0;
 
-    const rt = zrequire("recent_topics");
+    rt = reset_module("recent_topics");
 
     global.stub_templates(() => "<recent_topics table stub>");
     rt.process_messages(messages);
@@ -455,8 +457,7 @@ run_test("test_filter_participated", () => {
     ]);
     let i = 0;
 
-    const rt = zrequire("recent_topics");
+    rt = reset_module("recent_topics");
-
     global.stub_templates(() => "<recent_topics table stub>");
     rt.process_messages(messages);
     assert.equal(rt.inplace_rerender("1:topic-4"), true);
@@ -495,7 +496,7 @@ run_test("test_filter_participated", () => {
 });
 
 run_test("test_update_unread_count", () => {
-    const rt = zrequire("recent_topics");
+    rt = reset_module("recent_topics");
     rt.set_filter("all");
     global.stub_templates(() => "<recent_topics table stub>");
     rt.process_messages(messages);
@@ -509,7 +510,7 @@ run_test("test_update_unread_count", () => {
 global.stub_templates(() => "<recent_topics table stub>");
 
 run_test("basic assertions", () => {
-    const rt = zrequire("recent_topics");
+    rt = reset_module("recent_topics");
     rt.set_filter("all");
     rt.process_messages(messages);
     let all_topics = rt.get();
@@ -586,7 +587,7 @@ run_test("basic assertions", () => {
 });
 
 run_test("test_reify_local_echo_message", () => {
-    const rt = zrequire("recent_topics");
+    rt = reset_module("recent_topics");
     rt.set_filter("all");
     rt.process_messages(messages);
 
@@ -633,7 +634,7 @@ run_test("test_reify_local_echo_message", () => {
 });
 
 run_test("test_delete_messages", () => {
-    const rt = zrequire("recent_topics");
+    rt = reset_module("recent_topics");
     rt.set_filter("all");
     rt.process_messages(messages);
 
@@ -690,7 +691,7 @@ run_test("test_topic_edit", () => {
         },
     });
     // NOTE: This test should always run in the end as it modified the messages data.
-    const rt = zrequire("recent_topics");
+    rt = reset_module("recent_topics");
     rt.set_filter("all");
     rt.process_messages(messages);
 
@@ -769,7 +770,7 @@ run_test("test_topic_edit", () => {
 });
 
 run_test("test_search", () => {
-    const rt = zrequire("recent_topics");
+    rt = reset_module("recent_topics");
     assert.equal(rt.topic_in_search_results("t", "general", "Recent Topic"), true);
     assert.equal(rt.topic_in_search_results("T", "general", "Recent Topic"), true);
     assert.equal(rt.topic_in_search_results("to", "general", "Recent Topic"), true);

frontend_tests/zjsunit/index.js

@@ -44,6 +44,7 @@ global.with_field = namespace.with_field;
 global.set_global = namespace.set_global;
 global.patch_builtin = namespace.set_global;
 global.zrequire = namespace.zrequire;
+global.reset_module = namespace.reset_module;
 global.stub_out_jquery = namespace.stub_out_jquery;
 global.with_overrides = namespace.with_overrides;
 

frontend_tests/zjsunit/namespace.js

@@ -17,18 +17,29 @@ exports.set_global = function (name, val) {
     return val;
 };
 
-exports.zrequire = function (name, fn) {
+function require_path(name, fn) {
     if (fn === undefined) {
         fn = "../../static/js/" + name;
     } else if (/^generated\/|^js\/|^shared\/|^third\//.test(fn)) {
         // FIXME: Stealing part of the NPM namespace is confusing.
         fn = "../../static/" + fn;
     }
-    delete require.cache[require.resolve(fn)];
+
+    return fn;
+}
+
+exports.zrequire = function (name, fn) {
+    fn = require_path(name, fn);
     requires.push(fn);
     return require(fn);
 };
 
+exports.reset_module = function (name, fn) {
+    fn = require_path(name, fn);
+    delete require.cache[require.resolve(fn)];
+    return require(fn);
+};
+
 exports.clear_zulip_refs = function () {
     /*
         This is a big hammer to make sure