Merge pull request #3866 from cdr/jsjoeio-reorg-test-files

refactor: clean up test file structure

ci/dev/test-unit.sh

@@ -3,7 +3,7 @@ set -euo pipefail
 
 main() {
   cd "$(dirname "$0")/../.."
-  cd test/unit/test-plugin
+  cd test/unit/node/test-plugin
   make -s out/index.js
   # We must keep jest in a sub-directory. See ../../test/package.json for more
   # information. We must also run it from the root otherwise coverage will not

docs/CONTRIBUTING.md

@@ -143,7 +143,7 @@ Our unit tests are written in TypeScript and run using
 
 These live under [test/unit](../test/unit).
 
-We use unit tests for functions and things that can be tested in isolation.
+We use unit tests for functions and things that can be tested in isolation. The file structure is modeled closely after `/src` so it's easy for people to know where test files should live.
 
 ### Integration tests
 

test/unit/browser/pages/login.test.ts

@@ -1,5 +1,5 @@
 import { JSDOM } from "jsdom"
-import { LocationLike } from "../../util.test"
+import { LocationLike } from "../../common/util.test"
 
 describe("login", () => {
   describe("there is an element with id 'base'", () => {

test/unit/browser/register.test.ts

@@ -1,7 +1,7 @@
 import { JSDOM } from "jsdom"
 import { registerServiceWorker } from "../../../src/browser/register"
 import { createLoggerMock } from "../../utils/helpers"
-import { LocationLike } from "../util.test"
+import { LocationLike } from "../common/util.test"
 
 describe("register", () => {
   describe("when navigator and serviceWorker are defined", () => {

test/unit/common/emitter.test.ts

@@ -1,8 +1,8 @@
 // Note: we need to import logger from the root
 // because this is the logger used in logError in ../src/common/util
-import { logger } from "../../node_modules/@coder/logger"
+import { logger } from "@coder/logger"
 
-import { Emitter } from "../../src/common/emitter"
+import { Emitter } from "../../../src/common/emitter"
 
 describe("emitter", () => {
   let spy: jest.SpyInstance

test/unit/common/http.test.ts

@@ -1,4 +1,4 @@
-import { HttpCode, HttpError } from "../../src/common/http"
+import { HttpCode, HttpError } from "../../../src/common/http"
 
 describe("http", () => {
   describe("HttpCode", () => {

test/unit/common/util.test.ts

@@ -1,6 +1,6 @@
 import { JSDOM } from "jsdom"
-import * as util from "../../src/common/util"
-import { createLoggerMock } from "../utils/helpers"
+import * as util from "../../../src/common/util"
+import { createLoggerMock } from "../../utils/helpers"
 
 const dom = new JSDOM()
 global.document = dom.window.document

test/unit/node/cli.test.ts

@@ -3,9 +3,9 @@ import { promises as fs } from "fs"
 import * as net from "net"
 import * as os from "os"
 import * as path from "path"
-import { Args, parse, setDefaults, shouldOpenInExistingInstance, splitOnFirstEquals } from "../../src/node/cli"
-import { tmpdir } from "../../src/node/constants"
-import { paths } from "../../src/node/util"
+import { Args, parse, setDefaults, shouldOpenInExistingInstance, splitOnFirstEquals } from "../../../src/node/cli"
+import { tmpdir } from "../../../src/node/constants"
+import { paths } from "../../../src/node/util"
 
 type Mutable<T> = {
   -readonly [P in keyof T]: T[P]

test/unit/node/constants.test.ts

@@ -1,7 +1,7 @@
-import { createLoggerMock } from "../utils/helpers"
+import { createLoggerMock } from "../../utils/helpers"
 
 describe("constants", () => {
-  let constants: typeof import("../../src/node/constants")
+  let constants: typeof import("../../../src/node/constants")
 
   describe("with package.json defined", () => {
     const loggerModule = createLoggerMock()
@@ -15,8 +15,8 @@ describe("constants", () => {
 
     beforeAll(() => {
       jest.mock("@coder/logger", () => loggerModule)
-      jest.mock("../../package.json", () => mockPackageJson, { virtual: true })
-      constants = require("../../src/node/constants")
+      jest.mock("../../../package.json", () => mockPackageJson, { virtual: true })
+      constants = require("../../../src/node/constants")
     })
 
     afterAll(() => {
@@ -57,8 +57,8 @@ describe("constants", () => {
     }
 
     beforeAll(() => {
-      jest.mock("../../package.json", () => mockPackageJson, { virtual: true })
-      constants = require("../../src/node/constants")
+      jest.mock("../../../package.json", () => mockPackageJson, { virtual: true })
+      constants = require("../../../src/node/constants")
     })
 
     afterAll(() => {

test/unit/node/plugin.test.ts

@@ -2,11 +2,11 @@ import { logger } from "@coder/logger"
 import * as express from "express"
 import * as fs from "fs"
 import * as path from "path"
-import { HttpCode } from "../../src/common/http"
-import { AuthType } from "../../src/node/cli"
-import { codeServer, PluginAPI } from "../../src/node/plugin"
-import * as apps from "../../src/node/routes/apps"
-import * as httpserver from "../utils/httpserver"
+import { HttpCode } from "../../../src/common/http"
+import { AuthType } from "../../../src/node/cli"
+import { codeServer, PluginAPI } from "../../../src/node/plugin"
+import * as apps from "../../../src/node/routes/apps"
+import * as httpserver from "../../utils/httpserver"
 const fsp = fs.promises
 
 // Jest overrides `require` so our usual override doesn't work.

test/unit/node/proxy.test.ts

@@ -1,7 +1,7 @@
 import bodyParser from "body-parser"
 import * as express from "express"
-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("proxy", () => {
   const nhooyrDevServer = new httpserver.HttpServer()

test/unit/node/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/node/routes/login.test.ts

@@ -1,6 +1,6 @@
-import { RateLimiter } from "../../../src/node/routes/login"
-import * as httpserver from "../../utils/httpserver"
-import * as integration from "../../utils/integration"
+import { RateLimiter } from "../../../../src/node/routes/login"
+import * as httpserver from "../../../utils/httpserver"
+import * as integration from "../../../utils/integration"
 
 describe("login", () => {
   describe("RateLimiter", () => {

test/unit/node/routes/static.test.ts

@@ -1,8 +1,8 @@
 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"
+import { tmpdir } from "../../../utils/helpers"
+import * as httpserver from "../../../utils/httpserver"
+import * as integration from "../../../utils/integration"
 
 describe("/static", () => {
   let _codeServer: httpserver.HttpServer | undefined

test/unit/node/socket.test.ts

@@ -3,10 +3,10 @@ import { promises as fs } from "fs"
 import * as net from "net"
 import * as path from "path"
 import * as tls from "tls"
-import { Emitter } from "../../src/common/emitter"
-import { tmpdir } from "../../src/node/constants"
-import { SocketProxyProvider } from "../../src/node/socket"
-import { generateCertificate } from "../../src/node/util"
+import { Emitter } from "../../../src/common/emitter"
+import { tmpdir } from "../../../src/node/constants"
+import { SocketProxyProvider } from "../../../src/node/socket"
+import { generateCertificate } from "../../../src/node/util"
 
 describe("SocketProxyProvider", () => {
   const provider = new SocketProxyProvider()

test/unit/node/test-plugin/tsconfig.json

@@ -44,7 +44,7 @@
     // "moduleResolution": "node",            /* Specify module resolution strategy: 'node' (Node.js) or 'classic' (TypeScript pre-1.6). */
     "baseUrl": "./" /* Base directory to resolve non-absolute module names. */,
     "paths": {
-      "code-server": ["../../../typings/pluginapi"]
+      "code-server": ["../../../../typings/pluginapi"]
     } /* A series of entries which re-map imports to lookup locations relative to the 'baseUrl'. */,
     // "rootDirs": [],                        /* List of root folders whose combined content represents the structure of the project at runtime. */
     // "typeRoots": [],                       /* List of folders to include type definitions from. */

test/unit/node/update.test.ts

@@ -1,9 +1,9 @@
 import { promises as fs } from "fs"
 import * as http from "http"
 import * as path from "path"
-import { tmpdir } from "../../src/node/constants"
-import { SettingsProvider, UpdateSettings } from "../../src/node/settings"
-import { LatestResponse, UpdateProvider } from "../../src/node/update"
+import { tmpdir } from "../../../src/node/constants"
+import { SettingsProvider, UpdateSettings } from "../../../src/node/settings"
+import { LatestResponse, UpdateProvider } from "../../../src/node/update"
 
 describe("update", () => {
   let version = "1.0.0"

test/unit/node/util.test.ts

@@ -1,9 +1,9 @@
 import * as cp from "child_process"
-import * as path from "path"
 import { promises as fs } from "fs"
+import * as path from "path"
 import { generateUuid } from "../../../src/common/util"
-import * as util from "../../../src/node/util"
 import { tmpdir } from "../../../src/node/constants"
+import * as util from "../../../src/node/util"
 
 describe("getEnvPaths", () => {
   describe("on darwin", () => {