Merge pull request #3297 from code-asher/test-static

test/e2e/terminal.test.ts

@@ -1,10 +1,10 @@
-import { test, expect } from "@playwright/test"
+import { expect, test } from "@playwright/test"
 import * as cp from "child_process"
 import * as fs from "fs"
-// import { tmpdir } from "os"
 import * as path from "path"
 import util from "util"
-import { STORAGE, tmpdir } from "../utils/constants"
+import { STORAGE } from "../utils/constants"
+import { tmpdir } from "../utils/helpers"
 import { CodeServer } from "./models/CodeServer"
 
 test.describe("Integrated Terminal", () => {

test/unit/constants.test.ts

@@ -1,31 +1,22 @@
-import * as fs from "fs"
-import { tmpdir } from "../../test/utils/constants"
-import { loggerModule } from "../utils/helpers"
-
-// jest.mock is hoisted above the imports so we must use `require` here.
-jest.mock("@coder/logger", () => require("../utils/helpers").loggerModule)
+import { createLoggerMock } from "../utils/helpers"
 
 describe("constants", () => {
-  beforeAll(() => {
-    jest.clearAllMocks()
-    jest.resetModules()
-  })
+  let constants: typeof import("../../src/node/constants")
+
   describe("with package.json defined", () => {
-    const { getPackageJson } = require("../../src/node/constants")
-    let mockPackageJson = {
+    const loggerModule = createLoggerMock()
+    const mockPackageJson = {
       name: "mock-code-server",
       description: "Run VS Code on a remote server.",
       repository: "https://github.com/cdr/code-server",
       version: "1.0.0",
       commit: "f6b2be2838f4afb217c2fd8f03eafedd8d55ef9b",
     }
-    let version = ""
-    let commit = ""
 
-    beforeEach(() => {
+    beforeAll(() => {
+      jest.mock("@coder/logger", () => loggerModule)
       jest.mock("../../package.json", () => mockPackageJson, { virtual: true })
-      commit = require("../../src/node/constants").commit
-      version = require("../../src/node/constants").version
+      constants = require("../../src/node/constants")
     })
 
     afterAll(() => {
@@ -34,18 +25,18 @@ describe("constants", () => {
     })
 
     it("should provide the commit", () => {
-      expect(commit).toBe("f6b2be2838f4afb217c2fd8f03eafedd8d55ef9b")
+      expect(constants.commit).toBe(mockPackageJson.commit)
     })
 
     it("should return the package.json version", () => {
-      expect(version).toBe(mockPackageJson.version)
+      expect(constants.version).toBe(mockPackageJson.version)
     })
 
     describe("getPackageJson", () => {
       it("should log a warning if package.json not found", () => {
         const expectedErrorMessage = "Cannot find module './package.json' from 'src/node/constants.ts'"
 
-        getPackageJson("./package.json")
+        constants.getPackageJson("./package.json")
 
         expect(loggerModule.logger.warn).toHaveBeenCalled()
         expect(loggerModule.logger.warn).toHaveBeenCalledWith(expectedErrorMessage)
@@ -54,51 +45,32 @@ describe("constants", () => {
       it("should find the package.json", () => {
         // the function calls require from src/node/constants
         // so to get the root package.json we need to use ../../
-        const packageJson = getPackageJson("../../package.json")
-        expect(Object.keys(packageJson).length).toBeGreaterThan(0)
-        expect(packageJson.name).toBe("mock-code-server")
-        expect(packageJson.description).toBe("Run VS Code on a remote server.")
-        expect(packageJson.repository).toBe("https://github.com/cdr/code-server")
+        const packageJson = constants.getPackageJson("../../package.json")
+        expect(packageJson).toStrictEqual(mockPackageJson)
       })
     })
   })
 
   describe("with incomplete package.json", () => {
-    let mockPackageJson = {
+    const mockPackageJson = {
       name: "mock-code-server",
     }
-    let version = ""
-    let commit = ""
 
-    beforeEach(() => {
+    beforeAll(() => {
       jest.mock("../../package.json", () => mockPackageJson, { virtual: true })
-      version = require("../../src/node/constants").version
-      commit = require("../../src/node/constants").commit
+      constants = require("../../src/node/constants")
     })
 
-    afterEach(() => {
+    afterAll(() => {
       jest.clearAllMocks()
       jest.resetModules()
     })
 
     it("version should return 'development'", () => {
-      expect(version).toBe("development")
+      expect(constants.version).toBe("development")
     })
     it("commit should return 'development'", () => {
-      expect(commit).toBe("development")
-    })
-  })
-})
-
-describe("test constants", () => {
-  describe("tmpdir", () => {
-    it("should return a temp directory", async () => {
-      const testName = "temp-dir"
-      const pathToTempDir = await tmpdir(testName)
-
-      expect(pathToTempDir).toContain(testName)
-
-      await fs.promises.rmdir(pathToTempDir)
+      expect(constants.commit).toBe("development")
     })
   })
 })

test/unit/helpers.test.ts

@@ -0,0 +1,14 @@
+import { promises as fs } from "fs"
+import { tmpdir } from "../../test/utils/helpers"
+
+/**
+ * This file is for testing test helpers (not core code).
+ */
+describe("test helpers", () => {
+  it("should return a temp directory", async () => {
+    const testName = "temp-dir"
+    const pathToTempDir = await tmpdir(testName)
+    expect(pathToTempDir).toContain(testName)
+    expect(fs.access(pathToTempDir)).resolves.toStrictEqual(undefined)
+  })
+})

test/unit/register.test.ts

@@ -1,6 +1,6 @@
 import { JSDOM } from "jsdom"
 import { registerServiceWorker } from "../../src/browser/register"
-import { loggerModule } from "../utils/helpers"
+import { createLoggerMock } from "../utils/helpers"
 import { LocationLike } from "./util.test"
 
 describe("register", () => {
@@ -21,6 +21,7 @@ describe("register", () => {
       })
     })
 
+    const loggerModule = createLoggerMock()
     beforeEach(() => {
       jest.clearAllMocks()
       jest.mock("@coder/logger", () => loggerModule)
@@ -75,6 +76,7 @@ describe("register", () => {
   })
 
   describe("when navigator and serviceWorker are NOT defined", () => {
+    const loggerModule = createLoggerMock()
     beforeEach(() => {
       jest.clearAllMocks()
       jest.mock("@coder/logger", () => loggerModule)

test/unit/routes/health.test.ts

@@ -1,5 +1,5 @@
-import * as httpserver from "../utils/httpserver"
-import * as integration from "../utils/integration"
+import * as httpserver from "../../utils/httpserver"
+import * as integration from "../../utils/integration"
 
 describe("health", () => {
   let codeServer: httpserver.HttpServer | undefined

test/unit/routes/static.test.ts

@@ -0,0 +1,136 @@
+import { promises as fs } from "fs"
+import * as path from "path"
+import { tmpdir } from "../../utils/helpers"
+import * as httpserver from "../../utils/httpserver"
+import * as integration from "../../utils/integration"
+
+describe("/static", () => {
+  let _codeServer: httpserver.HttpServer | undefined
+  function codeServer(): httpserver.HttpServer {
+    if (!_codeServer) {
+      throw new Error("tried to use code-server before setting it up")
+    }
+    return _codeServer
+  }
+
+  let testFile: string | undefined
+  let testFileContent: string | undefined
+  let nonExistentTestFile: string | undefined
+
+  // The static endpoint expects a commit and then the full path of the file.
+  // The commit is just for cache busting so we can use anything we want. `-`
+  // and `development` are specially recognized in that they will cause the
+  // static endpoint to avoid sending cache headers.
+  const commit = "-"
+
+  beforeAll(async () => {
+    const testDir = await tmpdir("static")
+    testFile = path.join(testDir, "test")
+    testFileContent = "static file contents"
+    nonExistentTestFile = path.join(testDir, "i-am-not-here")
+    await fs.writeFile(testFile, testFileContent)
+  })
+
+  afterEach(async () => {
+    if (_codeServer) {
+      await _codeServer.close()
+      _codeServer = undefined
+    }
+  })
+
+  function commonTests() {
+    it("should return a 404 when a commit and file are not provided", async () => {
+      const resp = await codeServer().fetch("/static")
+      expect(resp.status).toBe(404)
+
+      const content = await resp.json()
+      expect(content).toStrictEqual({ error: "Not Found" })
+    })
+
+    it("should return a 404 when a file is not provided", async () => {
+      const resp = await codeServer().fetch(`/static/${commit}`)
+      expect(resp.status).toBe(404)
+
+      const content = await resp.json()
+      expect(content).toStrictEqual({ error: "Not Found" })
+    })
+  }
+
+  describe("disabled authentication", () => {
+    beforeEach(async () => {
+      _codeServer = await integration.setup(["--auth=none"], "")
+    })
+
+    commonTests()
+
+    it("should return a 404 for a nonexistent file", async () => {
+      const resp = await codeServer().fetch(`/static/${commit}/${nonExistentTestFile}`)
+      expect(resp.status).toBe(404)
+
+      const content = await resp.json()
+      expect(content.error).toMatch("ENOENT")
+    })
+
+    it("should return a 200 and file contents for an existent file", async () => {
+      const resp = await codeServer().fetch(`/static/${commit}${testFile}`)
+      expect(resp.status).toBe(200)
+
+      const content = await resp.text()
+      expect(content).toStrictEqual(testFileContent)
+    })
+  })
+
+  describe("enabled authentication", () => {
+    // Store whatever might be in here so we can restore it afterward.
+    // TODO: We should probably pass this as an argument somehow instead of
+    // manipulating the environment.
+    const previousEnvPassword = process.env.PASSWORD
+
+    beforeEach(async () => {
+      process.env.PASSWORD = "test"
+      _codeServer = await integration.setup(["--auth=password"], "")
+    })
+
+    afterEach(() => {
+      process.env.PASSWORD = previousEnvPassword
+    })
+
+    commonTests()
+
+    describe("inside code-server root", () => {
+      it("should return a 404 for a nonexistent file", async () => {
+        const resp = await codeServer().fetch(`/static/${commit}/${__filename}-does-not-exist`)
+        expect(resp.status).toBe(404)
+
+        const content = await resp.json()
+        expect(content.error).toMatch("ENOENT")
+      })
+
+      it("should return a 200 and file contents for an existent file", async () => {
+        const resp = await codeServer().fetch(`/static/${commit}${__filename}`)
+        expect(resp.status).toBe(200)
+
+        const content = await resp.text()
+        expect(content).toStrictEqual(await fs.readFile(__filename, "utf8"))
+      })
+    })
+
+    describe("outside code-server root", () => {
+      it("should return a 401 for a nonexistent file", async () => {
+        const resp = await codeServer().fetch(`/static/${commit}/${nonExistentTestFile}`)
+        expect(resp.status).toBe(401)
+
+        const content = await resp.json()
+        expect(content).toStrictEqual({ error: "Unauthorized" })
+      })
+
+      it("should return a 401 for an existent file", async () => {
+        const resp = await codeServer().fetch(`/static/${commit}${testFile}`)
+        expect(resp.status).toBe(401)
+
+        const content = await resp.json()
+        expect(content).toStrictEqual({ error: "Unauthorized" })
+      })
+    })
+  })
+})

test/unit/util.test.ts

@@ -11,7 +11,7 @@ import {
   trimSlashes,
   normalize,
 } from "../../src/common/util"
-import { loggerModule } from "../utils/helpers"
+import { createLoggerMock } from "../utils/helpers"
 
 const dom = new JSDOM()
 global.document = dom.window.document
@@ -229,6 +229,8 @@ describe("util", () => {
       jest.restoreAllMocks()
     })
 
+    const loggerModule = createLoggerMock()
+
     it("should log an error with the message and stack trace", () => {
       const message = "You don't have access to that folder."
       const error = new Error(message)

test/utils/constants.ts

@@ -1,14 +1,3 @@
-import * as fs from "fs"
-import * as os from "os"
-import * as path from "path"
-
 export const CODE_SERVER_ADDRESS = process.env.CODE_SERVER_ADDRESS || "http://localhost:8080"
 export const PASSWORD = process.env.PASSWORD || "e45432jklfdsab"
 export const STORAGE = process.env.STORAGE || ""
-
-export async function tmpdir(testName: string): Promise<string> {
-  const dir = path.join(os.tmpdir(), "code-server")
-  await fs.promises.mkdir(dir, { recursive: true })
-
-  return await fs.promises.mkdtemp(path.join(dir, `test-${testName}-`), { encoding: "utf8" })
-}

test/utils/helpers.ts

@@ -1,4 +1,12 @@
-export const loggerModule = {
+import * as fs from "fs"
+import * as os from "os"
+import * as path from "path"
+
+/**
+ * Return a mock of @coder/logger.
+ */
+export function createLoggerMock() {
+  return {
     field: jest.fn(),
     level: 2,
     logger: {
@@ -8,4 +16,17 @@ export const loggerModule = {
       trace: jest.fn(),
       warn: jest.fn(),
     },
+  }
+}
+
+/**
+ * Create a uniquely named temporary directory.
+ *
+ * These directories are placed under a single temporary code-server directory.
+ */
+export async function tmpdir(testName: string): Promise<string> {
+  const dir = path.join(os.tmpdir(), "code-server/tests")
+  await fs.promises.mkdir(dir, { recursive: true })
+
+  return await fs.promises.mkdtemp(path.join(dir, `${testName}-`), { encoding: "utf8" })
 }

test/utils/integration.ts

@@ -3,7 +3,7 @@ import { runCodeServer } from "../../src/node/main"
 import * as httpserver from "./httpserver"
 
 export async function setup(argv: string[], configFile?: string): Promise<httpserver.HttpServer> {
-  argv = ["--bind-addr=localhost:0", ...argv]
+  argv = ["--bind-addr=localhost:0", "--log=warn", ...argv]
 
   const cliArgs = parse(argv)
   const configArgs = parseConfigFile(configFile || "", "test/integration.ts")