From c222fc5d0b8e1058ebfae93fb95aae9f56b6eeea Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Tue, 13 Oct 2020 22:02:46 -0400
Subject: [PATCH] tests: Change `waitFor()` to check before first sleep

There are a few problems with sleeping before checking the condition
for the first time:

  * It slows down tests.
  * The predicate is never checked if the interval duration is greater
    than the timeout.
  * 0 can't be used to test if the condition is currently true.

There is a minor disadvantage to sleeping before checking: It will
cause more tests to run without an asynchronous interruption, which
could theoretically mask some async bugs.
---
 tests/frontend/helper.js       | 13 +++++++++----
 tests/frontend/specs/helper.js | 20 ++++++++++++++++++++
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/tests/frontend/helper.js b/tests/frontend/helper.js
index c61a6213b..874248319 100644
--- a/tests/frontend/helper.js
+++ b/tests/frontend/helper.js
@@ -168,7 +168,7 @@ var helper = {};
       return _fail(...args);
     };
 
-    var intervalCheck = setInterval(function(){
+    const check = () => {
       try {
         if (!conditionFunc()) return;
         deferred.resolve();
@@ -177,9 +177,11 @@ var helper = {};
       }
       clearInterval(intervalCheck);
       clearTimeout(timeout);
-    }, intervalTime);
+    };
 
-    var timeout = setTimeout(function(){
+    const intervalCheck = setInterval(check, intervalTime);
+
+    const timeout = setTimeout(() => {
       clearInterval(intervalCheck);
       var error = new Error("wait for condition never became true " + conditionFunc.toString());
       deferred.reject(error);
@@ -189,8 +191,11 @@ var helper = {};
       }
     }, timeoutTime);
 
+    // Check right away to avoid an unnecessary sleep if the condition is already true.
+    check();
+
     return deferred;
-  }
+  };
 
   helper.selectLines = function($startLine, $endLine, startOffset, endOffset){
     // if no offset is provided, use beginning of start line and end of end line
diff --git a/tests/frontend/specs/helper.js b/tests/frontend/specs/helper.js
index 062b3f94d..14c7944b6 100644
--- a/tests/frontend/specs/helper.js
+++ b/tests/frontend/specs/helper.js
@@ -193,6 +193,26 @@ describe("the test helper", function(){
         },100);
       });
     });
+
+    describe('checks first then sleeps', function() {
+      it('resolves quickly if the predicate is immediately true', async function() {
+        const before = Date.now();
+        await helper.waitFor(() => true, 1000, 900);
+        expect(Date.now() - before).to.be.lessThan(800);
+      });
+
+      it('polls exactly once if timeout < interval', async function() {
+        let calls = 0;
+        await helper.waitFor(() => { calls++; }, 1, 1000)
+            .fail(() => {}) // Suppress the redundant uncatchable exception.
+            .catch(() => {}); // Don't throw an exception -- we know it rejects.
+        expect(calls).to.be(1);
+      });
+
+      it('resolves if condition is immediately true even if timeout is 0', async function() {
+        await helper.waitFor(() => true, 0);
+      });
+    });
   });
 
   describe("the selectLines method", function(){