FIX: flaky javascript tests with fake timers (#13235)

FIX: flaky javascript tests with fake timers (#13235)

The problem was happening in component integration tests on the rendering stage, sometimes the rendering would never finish.

Using time moments in the future when faking time solves the problem. Unfortunately, I don’t know why exactly it helps. It was just a lucky guess after some hours I spent trying to figure out what’s going on. But I’ve done a lot of testings, so looks like it really works. I’ll be monitoring builds for some time after merging this anyway.

Unit tests seem to work alright with moments in the past. And we don’t fake time in acceptance tests at the moment but I guess they would very likely be flaky with time moments from the past since they also do rendering.

I’m actually thinking of moving all fake time moments to the future (including moments in unit tests) to decrease the chances of flakiness. But I don’t want to do everything in one PR, because I can accidentally introduce new flakiness.

A pretty easy way of picking time moments in the future for tests is to use the 2100 year. It has the same calendar as 2021. If a day is Monday in 2021 it’s Monday in 2100 too.

diff --git a/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js b/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js
index 86ec6c0..6c7ae49 100644
--- a/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js
+++ b/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js
@@ -76,6 +76,15 @@ export function fakeTime(timeString, timezone = null, advanceTime = false) {
   });
 }
 
+export function withFrozenTime(timeString, timezone, callback) {
+  const clock = fakeTime(timeString, timezone, false);
+  try {
+    callback();
+  } finally {
+    clock.restore();
+  }
+}
+
 let _pretenderCallbacks = {};
 
 export function resetSite(siteSettings, extras) {
diff --git a/app/assets/javascripts/discourse/tests/integration/components/bookmark-test.js b/app/assets/javascripts/discourse/tests/integration/components/bookmark-test.js
index b7ac876..ba6d71d 100644
--- a/app/assets/javascripts/discourse/tests/integration/components/bookmark-test.js
+++ b/app/assets/javascripts/discourse/tests/integration/components/bookmark-test.js
@@ -6,13 +6,6 @@ import {
   fakeTime,
   query,
 } from "discourse/tests/helpers/qunit-helpers";
-import sinon from "sinon";
-
-let clock = null;
-
-function mockMomentTz(dateString, timezone) {
-  clock = fakeTime(dateString, timezone, true);
-}
 
 discourseModule("Integration | Component | bookmark", function (hooks) {
   setupRenderingTest(hooks);
@@ -32,18 +25,17 @@ discourseModule("Integration | Component | bookmark", function (hooks) {
   });
 
   hooks.afterEach(function () {
-    if (clock) {
-      clock.restore();
+    if (this.clock) {
+      this.clock.restore();
     }
-    sinon.restore();
   });
 
   componentTest("show later this week option if today is < Thursday", {
     template,
-    skip: true,
 
     beforeEach() {
-      mockMomentTz("2019-12-10T08:00:00", this.currentUser._timezone);
+      const monday = "2100-06-07T08:00:00";
+      this.clock = fakeTime(monday, this.currentUser._timezone, true);
     },
 
     test(assert) {
@@ -55,10 +47,10 @@ discourseModule("Integration | Component | bookmark", function (hooks) {
     "does not show later this week option if today is >= Thursday",
     {
       template,
-      skip: true,
 
       beforeEach() {
-        mockMomentTz("2019-12-13T08:00:00", this.currentUser._timezone);
+        const thursday = "2100-06-10T08:00:00";
+        this.clock = fakeTime(thursday, this.currentUser._timezone, true);
       },
 
       test(assert) {
@@ -72,10 +64,13 @@ discourseModule("Integration | Component | bookmark", function (hooks) {
 
   componentTest("later today does not show if later today is tomorrow", {
     template,
-    skip: true,
 
     beforeEach() {
-      mockMomentTz("2019-12-11T22:00:00", this.currentUser._timezone);
+      this.clock = fakeTime(
+        "2100-12-11T22:00:00",
+        this.currentUser._timezone,
+        true
+      );
     },
 
     test(assert) {
@@ -88,10 +83,13 @@ discourseModule("Integration | Component | bookmark", function (hooks) {
 
   componentTest("later today shows if it is after 5pm but before 6pm", {
     template,
-    skip: true,
 
     beforeEach() {
-      mockMomentTz("2019-12-11T14:30:00", this.currentUser._timezone);
+      this.clock = fakeTime(
+        "2100-12-11T14:30:00",
+        this.currentUser._timezone,
+        true
+      );
     },
 
     test(assert) {
@@ -101,10 +99,13 @@ discourseModule("Integration | Component | bookmark", function (hooks) {
 
   componentTest("later today does not show if it is after 5pm", {
     template,
-    skip: true,
 
     beforeEach() {
-      mockMomentTz("2019-12-11T17:00:00", this.currentUser._timezone);
+      this.clock = fakeTime(
+        "2100-12-11T17:00:00",
+        this.currentUser._timezone,
+        true
+      );
     },
 
     test(assert) {
@@ -117,10 +118,13 @@ discourseModule("Integration | Component | bookmark", function (hooks) {
 
   componentTest("later today does show if it is before the end of the day", {
     template,
-    skip: true,
 
     beforeEach() {
-      mockMomentTz("2019-12-11T13:00:00", this.currentUser._timezone);
+      this.clock = fakeTime(
+        "2100-12-11T13:00:00",
+        this.currentUser._timezone,
+        true
+      );
     },
 
     test(assert) {
@@ -130,7 +134,6 @@ discourseModule("Integration | Component | bookmark", function (hooks) {
 
   componentTest("prefills the custom reminder type date and time", {
     template,
-    skip: true,
 
     beforeEach() {
       let name = "test";
@@ -147,7 +150,6 @@ discourseModule("Integration | Component | bookmark", function (hooks) {
 
   componentTest("defaults to 08:00 for custom time", {
     template,
-    skip: true,
 
     async test(assert) {
       await click("#tap_tile_custom");
diff --git a/app/assets/javascripts/discourse/tests/integration/components/select-kit/future-date-input-selector-test.js b/app/assets/javascripts/discourse/tests/integration/components/select-kit/future-date-input-selector-test.js
index 0bc0e7c..344463e 100644
--- a/app/assets/javascripts/discourse/tests/integration/components/select-kit/future-date-input-selector-test.js
+++ b/app/assets/javascripts/discourse/tests/integration/components/select-kit/future-date-input-selector-test.js
@@ -17,7 +17,6 @@ discourseModule(
 
     hooks.beforeEach(function () {
       this.set("subject", selectKit());
-      this.clock = fakeTime("2021-05-03T08:00:00", "UTC", true); // Monday
     });
 
     hooks.afterEach(function () {
@@ -25,9 +24,13 @@ discourseModule(
     });
 
     componentTest("shows default options", {
-      skip: true,
       template: hbs`{{future-date-input-selector}}`,
 
+      beforeEach() {
+        const monday = fakeTime("2100-06-07T08:00:00", "UTC", true);
+        this.clock = monday;
+      },
+
       async test(assert) {
         await this.subject.expand();
 
@@ -48,11 +51,11 @@ discourseModule(
     });
 
     componentTest("doesn't show 'Next Week' on Sundays", {
-      skip: true,
       template: hbs`{{future-date-input-selector}}`,
 
       beforeEach() {
-        this.clock = fakeTime("2021-05-02T08:00:00", "UTC", true); // Sunday
+        const sunday = fakeTime("2100-06-13T08:00:00", "UTC", true);
+        this.clock = sunday;
       },
 
       async test(assert) {
diff --git a/app/assets/javascripts/discourse/tests/unit/lib/bookmark-test.js b/app/assets/javascripts/discourse/tests/unit/lib/bookmark-test.js
index 79dcb4a..68c83e3 100644
--- a/app/assets/javascripts/discourse/tests/unit/lib/bookmark-test.js
+++ b/app/assets/javascripts/discourse/tests/unit/lib/bookmark-test.js
@@ -1,15 +1,14 @@
 import { module, test } from "qunit";
 import { fakeTime } from "discourse/tests/helpers/qunit-helpers";
 import { formattedReminderTime } from "discourse/lib/bookmark";
-import sinon from "sinon";
 
 module("Unit | Utility | bookmark", function (hooks) {
   hooks.beforeEach(function () {
-    fakeTime("2020-04-11 08:00:00", "Australia/Brisbane");
+    this.clock = fakeTime("2020-04-11 08:00:00", "Australia/Brisbane");
   });
 
   hooks.afterEach(function () {
-    sinon.restore();
+    this.clock.restore();
   });
 
   test("formattedReminderTime works when the reminder time is tomorrow", function (assert) {
diff --git a/app/assets/javascripts/discourse/tests/unit/lib/time-utils-test.js b/app/assets/javascripts/discourse/tests/unit/lib/time-utils-test.js
index ce174c0..6cfca1c 100644
--- a/app/assets/javascripts/discourse/tests/unit/lib/time-utils-test.js
+++ b/app/assets/javascripts/discourse/tests/unit/lib/time-utils-test.js
@@ -1,6 +1,6 @@
 import {
   discourseModule,
-  fakeTime,
+  withFrozenTime,
 } from "discourse/tests/helpers/qunit-helpers";
 
 import {
@@ -15,33 +15,29 @@ import { test } from "qunit";
 
 const timezone = "Australia/Brisbane";
 
-function mockMomentTz(dateString) {
-  fakeTime(dateString, timezone);
-}
-
 discourseModule("Unit | lib | timeUtils", function () {

[... diff too long, it was truncated ...]

GitHub sha: 178b294a629cb7fa6970c45be07c71d5305910a9

This commit appears in #13235 which was approved by ZogStriP. It was merged by AndrewPrigorshnev.