From fd69f2db88b778eee39528c9fe3a3086ac456b1f Mon Sep 17 00:00:00 2001
From: Joe Previte <jjprevite@gmail.com>
Date: Fri, 2 Apr 2021 12:32:24 -0700
Subject: [PATCH] refactor: logout test

---
 test/e2e/globalSetup.test.ts |  2 +-
 test/e2e/logout.test.ts      | 12 ++++++--
 test/unit/util.test.ts       | 59 +-----------------------------------
 test/utils/helpers.ts        | 36 ----------------------
 4 files changed, 11 insertions(+), 98 deletions(-)

diff --git a/test/e2e/globalSetup.test.ts b/test/e2e/globalSetup.test.ts
index ee4b64433..6d4982b63 100644
--- a/test/e2e/globalSetup.test.ts
+++ b/test/e2e/globalSetup.test.ts
@@ -12,7 +12,7 @@ describe("globalSetup", () => {
     await page.goto(CODE_SERVER_ADDRESS, { waitUntil: "networkidle" })
   })
 
-  it("should keep us logged in if we don't reset the browser", async () => {
+  it("should keep us logged in using the storageState", async () => {
     // See the editor
     const codeServerEditor = await page.isVisible(".monaco-workbench")
     expect(codeServerEditor).toBeTruthy()
diff --git a/test/e2e/logout.test.ts b/test/e2e/logout.test.ts
index da364fe76..6ffce5ca9 100644
--- a/test/e2e/logout.test.ts
+++ b/test/e2e/logout.test.ts
@@ -25,10 +25,16 @@ describe("logout", () => {
     expect(await page.isVisible(logoutButton))
 
     await page.hover(logoutButton)
+    // TODO(@jsjoeio)
+    // Look into how we're attaching the handlers for the logout feature
+    // We need to see how it's done upstream and add logging to the
+    // handlers themselves.
+    // They may be attached too slowly, hence why we need this timeout
+    await page.waitForTimeout(2000)
 
-    await page.click(logoutButton)
-    // it takes a couple seconds for url to change
-    await page.waitForLoadState("networkidle")
+    // Recommended by Playwright for async navigation
+    // https://github.com/microsoft/playwright/issues/1987#issuecomment-620182151
+    await Promise.all([page.waitForNavigation(), page.click(logoutButton)])
     const currentUrl = page.url()
     expect(currentUrl).toBe(`${CODE_SERVER_ADDRESS}/login`)
   })
diff --git a/test/unit/util.test.ts b/test/unit/util.test.ts
index 31eace762..02f3fdc5f 100644
--- a/test/unit/util.test.ts
+++ b/test/unit/util.test.ts
@@ -11,10 +11,7 @@ import {
   trimSlashes,
   normalize,
 } from "../../src/common/util"
-import { Cookie as CookieEnum } from "../../src/node/routes/login"
-import { hash } from "../../src/node/util"
-import { PASSWORD } from "../utils/constants"
-import { checkForCookie, createCookieIfDoesntExist, loggerModule, Cookie } from "../utils/helpers"
+import { loggerModule } from "../utils/helpers"
 
 const dom = new JSDOM()
 global.document = dom.window.document
@@ -252,58 +249,4 @@ describe("util", () => {
       expect(loggerModule.logger.error).toHaveBeenCalledWith("api: oh no")
     })
   })
-
-  describe("checkForCookie", () => {
-    it("should check if the cookie exists and has a value", () => {
-      const fakeCookies: Cookie[] = [
-        {
-          name: CookieEnum.Key,
-          value: hash(PASSWORD),
-          domain: "localhost",
-          secure: false,
-          sameSite: "Lax",
-          httpOnly: false,
-          expires: 18000,
-          path: "/",
-        },
-      ]
-      expect(checkForCookie(fakeCookies, CookieEnum.Key)).toBe(true)
-    })
-    it("should return false if there are no cookies", () => {
-      const fakeCookies: Cookie[] = []
-      expect(checkForCookie(fakeCookies, "key")).toBe(false)
-    })
-  })
-
-  describe("createCookieIfDoesntExist", () => {
-    it("should create a cookie if it doesn't exist", () => {
-      const cookies: Cookie[] = []
-      const cookieToStore = {
-        name: CookieEnum.Key,
-        value: hash(PASSWORD),
-        domain: "localhost",
-        secure: false,
-        sameSite: "Lax" as const,
-        httpOnly: false,
-        expires: 18000,
-        path: "/",
-      }
-      expect(createCookieIfDoesntExist(cookies, cookieToStore)).toStrictEqual([cookieToStore])
-    })
-    it("should return the same cookies if the cookie already exists", () => {
-      const PASSWORD = "123supersecure"
-      const cookieToStore = {
-        name: CookieEnum.Key,
-        value: hash(PASSWORD),
-        domain: "localhost",
-        secure: false,
-        sameSite: "Lax" as const,
-        httpOnly: false,
-        expires: 18000,
-        path: "/",
-      }
-      const cookies: Cookie[] = [cookieToStore]
-      expect(createCookieIfDoesntExist(cookies, cookieToStore)).toStrictEqual(cookies)
-    })
-  })
 })
diff --git a/test/utils/helpers.ts b/test/utils/helpers.ts
index 2aeb82df6..b45804014 100644
--- a/test/utils/helpers.ts
+++ b/test/utils/helpers.ts
@@ -1,39 +1,3 @@
-// Borrowed from playwright
-export interface Cookie {
-  name: string
-  value: string
-  domain: string
-  path: string
-  /**
-   * Unix time in seconds.
-   */
-  expires: number
-  httpOnly: boolean
-  secure: boolean
-  sameSite: "Strict" | "Lax" | "None"
-}
-
-/**
- * Checks if a cookie exists in array of cookies
- */
-export function checkForCookie(cookies: Array<Cookie>, key: string): boolean {
-  // Check for a cookie where the name is equal to key
-  return Boolean(cookies.find((cookie) => cookie.name === key))
-}
-
-/**
- * Creates a login cookie if one doesn't already exist
- */
-export function createCookieIfDoesntExist(cookies: Array<Cookie>, cookieToStore: Cookie): Array<Cookie> {
-  const cookieName = cookieToStore.name
-  const doesCookieExist = checkForCookie(cookies, cookieName)
-  if (!doesCookieExist) {
-    const updatedCookies = [...cookies, cookieToStore]
-    return updatedCookies
-  }
-  return cookies
-}
-
 export const loggerModule = {
   field: jest.fn(),
   level: 2,