From 38d7718feb32d27c540d6f94ba753cf4a2bcc3eb Mon Sep 17 00:00:00 2001
From: Joe Previte <jjprevite@gmail.com>
Date: Thu, 11 Feb 2021 11:18:15 -0700
Subject: [PATCH] refactor: use promises for goHome test

---
 ci/dev/test.sh      |  3 +--
 test/goHome.test.ts | 45 +++++++++++++++++++--------------------------
 2 files changed, 20 insertions(+), 28 deletions(-)

diff --git a/ci/dev/test.sh b/ci/dev/test.sh
index 6520176d..82f6ad36 100755
--- a/ci/dev/test.sh
+++ b/ci/dev/test.sh
@@ -9,8 +9,7 @@ main() {
   # information. We must also run it from the root otherwise coverage will not
   # include our source files.
   cd "$OLDPWD"
-  # We use the same environment variables set in ci.yml in the test job
-  if [[ -z ${PASSWORD+x} ]] || [[ -z ${CODE_SERVER_ADDRESS+x} ]]; then
+  if [[ -z ${PASSWORD-} ]] || [[ -z ${CODE_SERVER_ADDRESS-} ]]; then
     echo "The end-to-end testing suites rely on your local environment"
     echo -e "\n"
     echo "Please set the following environment variables locally:"
diff --git a/test/goHome.test.ts b/test/goHome.test.ts
index 1c608875..f688df05 100644
--- a/test/goHome.test.ts
+++ b/test/goHome.test.ts
@@ -2,15 +2,23 @@ import { chromium, Page, Browser, BrowserContext, Cookie } from "playwright"
 import { createCookieIfDoesntExist } from "../src/common/util"
 import { hash } from "../src/node/util"
 
-describe("login", () => {
+async function setTimeoutPromise(milliseconds: number): Promise<void> {
+  return new Promise((resolve, _) => {
+    setTimeout(() => {
+      resolve()
+    }, milliseconds)
+  })
+}
+
+describe("go home", () => {
   let browser: Browser
   let page: Page
   let context: BrowserContext
 
-  beforeAll(async (done) => {
+  beforeAll(async () => {
     browser = await chromium.launch()
     // Create a new context with the saved storage state
-    const storageState = JSON.parse(process.env.STORAGE || "")
+    const storageState = JSON.parse(process.env.STORAGE || "{}")
 
     const cookieToStore = {
       sameSite: "Lax" as const,
@@ -40,37 +48,23 @@ describe("login", () => {
       storageState: { cookies: maybeUpdatedCookies },
       recordVideo: { dir: "./test/videos/" },
     })
-    done()
   })
 
-  afterAll(async (done) => {
+  afterAll(async () => {
     // Remove password from local storage
     await context.clearCookies()
 
     await browser.close()
     await context.close()
-    done()
   })
 
-  beforeEach(async (done) => {
+  beforeEach(async () => {
     page = await context.newPage()
-    done()
   })
 
   // NOTE: this test will fail if you do not run code-server with --home $CODE_SERVER_ADDRESS/healthz
-  it("should see a 'Go Home' button in the Application Menu that goes to /healthz", async (done) => {
+  it("should see a 'Go Home' button in the Application Menu that goes to /healthz", async () => {
     let requestedGoHomeUrl = false
-    // Ideally, this test should pass and finish before the timeout set in the Jest config
-    // However, if it doesn't, we don't want a memory leak so we set this backup timeout
-    // Otherwise Jest may throw this error
-    // "Jest did not exit one second after the test run has completed.
-    // This usually means that there are asynchronous operations that weren't stopped in your tests.
-    // Consider running Jest with `--detectOpenHandles` to troubleshoot this issue."
-    const backupTimeout = setTimeout(() => {
-      // If it's not true by this point then the test should fail
-      expect(requestedGoHomeUrl).toBeTruthy()
-      done()
-    }, 20000)
 
     const GO_HOME_URL = `${process.env.CODE_SERVER_ADDRESS}/healthz`
     page.on("request", (request) => {
@@ -80,11 +74,6 @@ describe("login", () => {
       // only that it was made
       if (request.url() === GO_HOME_URL) {
         requestedGoHomeUrl = true
-        expect(requestedGoHomeUrl).toBeTruthy()
-        clearTimeout(backupTimeout)
-
-        // This ensures Jest knows we're done here.
-        done()
       }
     })
     // Sometimes a dialog shows up when you navigate
@@ -105,6 +94,10 @@ describe("login", () => {
     // Click it and navigate to /healthz
     // NOTE: ran into issues of it failing intermittently
     // without having button: "middle"
-    await page.click(goHomeButton, { button: "middle" })
+    await Promise.all([ 
+      page.waitForNavigation(), 
+      page.click(goHomeButton, { button: "middle" }) 
+    ])
+    expect(page.url()).toBe(GO_HOME_URL)
   })
 })