OCram85/code-server
Archived
1
0
Fork 0
Code Pull Requests 2 Activity

Implement last opened functionality (#4633)

Browse Source 
* Implement last opened functionality

Fixes https://github.com/cdr/code-server/issues/4619

* Fix test temp dirs not being cleaned up

* Mock logger everywhere

This suppresses all the error and debug output we generate which makes
it hard to actually find which test has failed.  It also gives us a
standard way to test logging for the few places we do that.

* Use separate data directories for unit test instances

Exactly as we do for the e2e tests.

* Add integration tests for vscode route

* Make settings use --user-data-dir

Without this test instances step on each other feet and they also
clobber your own non-test settings.

* Make redirects consistent

They will preserve the trailing slash if there is one.

* Remove compilation check

If you do a regular non-watch build there are no compilation stats so
this bricks VS Code in CI when running the unit tests.

I am not sure how best to fix this for the case where you have a build
that has not been packaged yet so I just removed it for now and added a
message to check if VS Code is compiling when in dev mode.

* Update code-server update endpoint name
This commit is contained in:
Asher
2021-12-17 13:06:52 -06:00
committed by GitHub
parent b990dabed1
commit c4c480a068
31 changed files with 406 additions and 241 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
test/unit/node/__snapshots__/app.test.ts.snap
View File

@ -1,3 +0,0 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`handleServerError should log an error if resolved is true 1`] = `"Cannot read property 'handle' of undefined"`;

64
test/unit/node/app.test.ts
View File

@ -1,27 +1,29 @@
import { logger } from "@coder/logger"
import { promises, rmdirSync } from "fs"
import { promises } from "fs"
import * as http from "http"
import * as https from "https"
import * as path from "path"
import { createApp, ensureAddress, handleArgsSocketCatchError, handleServerError } from "../../../src/node/app"
import { OptionalString, setDefaults } from "../../../src/node/cli"
import { generateCertificate } from "../../../src/node/util"
import { getAvailablePort, tmpdir } from "../../utils/helpers"
import { clean, mockLogger, getAvailablePort, tmpdir } from "../../utils/helpers"
describe("createApp", () => {
let spy: jest.SpyInstance
let unlinkSpy: jest.SpyInstance
let port: number
let tmpDirPath: string
let tmpFilePath: string
beforeAll(async () => {
tmpDirPath = await tmpdir("unlink-socket")
mockLogger()
const testName = "unlink-socket"
await clean(testName)
tmpDirPath = await tmpdir(testName)
tmpFilePath = path.join(tmpDirPath, "unlink-socket-file")
})
beforeEach(async () => {
spy = jest.spyOn(logger, "error")
// NOTE:@jsjoeio
// Be mindful when spying.
// You can't spy on fs functions if you do import * as fs
@ -36,12 +38,6 @@ describe("createApp", () => {
jest.clearAllMocks()
})
afterAll(() => {
jest.restoreAllMocks()
// Ensure directory was removed
rmdirSync(tmpDirPath, { recursive: true })
})
it("should return an Express app, a WebSockets Express app and an http server", async () => {
const defaultArgs = await setDefaults({
port,
@ -70,8 +66,8 @@ describe("createApp", () => {
// By emitting an error event
// Ref: https://stackoverflow.com/a/33872506/3015595
app.server.emit("error", testError)
expect(spy).toHaveBeenCalledTimes(1)
expect(spy).toHaveBeenCalledWith(`http server error: ${testError.message} ${testError.stack}`)
expect(logger.error).toHaveBeenCalledTimes(1)
expect(logger.error).toHaveBeenCalledWith(`http server error: ${testError.message} ${testError.stack}`)
// Cleanup
app.dispose()
@ -152,20 +148,14 @@ describe("ensureAddress", () => {
})
describe("handleServerError", () => {
let spy: jest.SpyInstance
beforeEach(() => {
spy = jest.spyOn(logger, "error")
beforeAll(() => {
mockLogger()
})
afterEach(() => {
jest.clearAllMocks()
})
afterAll(() => {
jest.restoreAllMocks()
})
it("should call reject if resolved is false", async () => {
const resolved = false
const reject = jest.fn((err: Error) => undefined)
@ -184,33 +174,27 @@ describe("handleServerError", () => {
handleServerError(resolved, error, reject)
expect(spy).toHaveBeenCalledTimes(1)
expect(spy).toThrowErrorMatchingSnapshot()
expect(logger.error).toHaveBeenCalledTimes(1)
expect(logger.error).toHaveBeenCalledWith(`http server error: ${error.message} ${error.stack}`)
})
})
describe("handleArgsSocketCatchError", () => {
let spy: jest.SpyInstance
beforeEach(() => {
spy = jest.spyOn(logger, "error")
beforeAll(() => {
mockLogger()
})
afterEach(() => {
jest.clearAllMocks()
})
afterAll(() => {
jest.restoreAllMocks()
})
it("should log an error if its not an NodeJS.ErrnoException", () => {
const error = new Error()
handleArgsSocketCatchError(error)
expect(spy).toHaveBeenCalledTimes(1)
expect(spy).toHaveBeenCalledWith(error)
expect(logger.error).toHaveBeenCalledTimes(1)
expect(logger.error).toHaveBeenCalledWith(error)
})
it("should log an error if its not an NodeJS.ErrnoException (and the error has a message)", () => {
@ -219,8 +203,8 @@ describe("handleArgsSocketCatchError", () => {
handleArgsSocketCatchError(error)
expect(spy).toHaveBeenCalledTimes(1)
expect(spy).toHaveBeenCalledWith(errorMessage)
expect(logger.error).toHaveBeenCalledTimes(1)
expect(logger.error).toHaveBeenCalledWith(errorMessage)
})
it("should not log an error if its a iNodeJS.ErrnoException", () => {
@ -229,7 +213,7 @@ describe("handleArgsSocketCatchError", () => {
handleArgsSocketCatchError(error)
expect(spy).toHaveBeenCalledTimes(0)
expect(logger.error).toHaveBeenCalledTimes(0)
})
it("should log an error if the code is not ENOENT (and the error has a message)", () => {
@ -240,8 +224,8 @@ describe("handleArgsSocketCatchError", () => {
handleArgsSocketCatchError(error)
expect(spy).toHaveBeenCalledTimes(1)
expect(spy).toHaveBeenCalledWith(errorMessage)
expect(logger.error).toHaveBeenCalledTimes(1)
expect(logger.error).toHaveBeenCalledWith(errorMessage)
})
it("should log an error if the code is not ENOENT", () => {
@ -250,7 +234,7 @@ describe("handleArgsSocketCatchError", () => {
handleArgsSocketCatchError(error)
expect(spy).toHaveBeenCalledTimes(1)
expect(spy).toHaveBeenCalledWith(error)
expect(logger.error).toHaveBeenCalledTimes(1)
expect(logger.error).toHaveBeenCalledWith(error)
})
})

29
test/unit/node/cli.test.ts
View File

@ -361,13 +361,11 @@ describe("parser", () => {
})
describe("cli", () => {
let testDir: string
const testName = "cli"
const vscodeIpcPath = path.join(os.tmpdir(), "vscode-ipc")
beforeAll(async () => {
testDir = await tmpdir("cli")
await fs.rmdir(testDir, { recursive: true })
await fs.mkdir(testDir, { recursive: true })
await clean(testName)
})
beforeEach(async () => {
@ -416,6 +414,7 @@ describe("cli", () => {
args._ = ["./file"]
expect(await shouldOpenInExistingInstance(args)).toStrictEqual(undefined)
const testDir = await tmpdir(testName)
const socketPath = path.join(testDir, "socket")
await fs.writeFile(vscodeIpcPath, socketPath)
expect(await shouldOpenInExistingInstance(args)).toStrictEqual(undefined)
@ -635,14 +634,15 @@ describe("readSocketPath", () => {
let tmpDirPath: string
let tmpFilePath: string
beforeEach(async () => {
tmpDirPath = await tmpdir("readSocketPath")
tmpFilePath = path.join(tmpDirPath, "readSocketPath.txt")
await fs.writeFile(tmpFilePath, fileContents)
const testName = "readSocketPath"
beforeAll(async () => {
await clean(testName)
})
afterEach(async () => {
await fs.rmdir(tmpDirPath, { recursive: true })
beforeEach(async () => {
tmpDirPath = await tmpdir(testName)
tmpFilePath = path.join(tmpDirPath, "readSocketPath.txt")
await fs.writeFile(tmpFilePath, fileContents)
})
it("should throw an error if it can't read the file", async () => {
@ -677,9 +677,10 @@ describe("toVsCodeArgs", () => {
version: false,
}
const testName = "vscode-args"
beforeAll(async () => {
// Clean up temporary directories from the previous run.
await clean("vscode-args")
await clean(testName)
})
it("should convert empty args", async () => {
@ -691,7 +692,7 @@ describe("toVsCodeArgs", () => {
})
it("should convert with workspace", async () => {
const workspace = path.join(await tmpdir("vscode-args"), "test.code-workspace")
const workspace = path.join(await tmpdir(testName), "test.code-workspace")
await fs.writeFile(workspace, "foobar")
expect(await toVsCodeArgs(await setDefaults(parse([workspace])))).toStrictEqual({
...vscodeDefaults,
@ -702,7 +703,7 @@ describe("toVsCodeArgs", () => {
})
it("should convert with folder", async () => {
const folder = await tmpdir("vscode-args")
const folder = await tmpdir(testName)
expect(await toVsCodeArgs(await setDefaults(parse([folder])))).toStrictEqual({
...vscodeDefaults,
folder,
@ -712,7 +713,7 @@ describe("toVsCodeArgs", () => {
})
it("should ignore regular file", async () => {
const file = path.join(await tmpdir("vscode-args"), "file")
const file = path.join(await tmpdir(testName), "file")
await fs.writeFile(file, "foobar")
expect(await toVsCodeArgs(await setDefaults(parse([file])))).toStrictEqual({
...vscodeDefaults,

10
test/unit/node/constants.test.ts
View File

@ -1,10 +1,10 @@
import { createLoggerMock } from "../../utils/helpers"
import { logger } from "@coder/logger"
import { mockLogger } from "../../utils/helpers"
describe("constants", () => {
let constants: typeof import("../../../src/node/constants")
describe("with package.json defined", () => {
const loggerModule = createLoggerMock()
const mockPackageJson = {
name: "mock-code-server",
description: "Run VS Code on a remote server.",
@ -14,7 +14,7 @@ describe("constants", () => {
}
beforeAll(() => {
jest.mock("@coder/logger", () => loggerModule)
mockLogger()
jest.mock("../../../package.json", () => mockPackageJson, { virtual: true })
constants = require("../../../src/node/constants")
})
@ -38,8 +38,8 @@ describe("constants", () => {
constants.getPackageJson("./package.json")
expect(loggerModule.logger.warn).toHaveBeenCalled()
expect(loggerModule.logger.warn).toHaveBeenCalledWith(expectedErrorMessage)
expect(logger.warn).toHaveBeenCalled()
expect(logger.warn).toHaveBeenCalledWith(expectedErrorMessage)
})
it("should find the package.json", () => {

6
test/unit/node/proxy_agent.test.ts
View File

@ -1,11 +1,15 @@
import { shouldEnableProxy } from "../../../src/node/proxy_agent"
import { useEnv } from "../../utils/helpers"
import { mockLogger, useEnv } from "../../utils/helpers"
describe("shouldEnableProxy", () => {
const [setHTTPProxy, resetHTTPProxy] = useEnv("HTTP_PROXY")
const [setHTTPSProxy, resetHTTPSProxy] = useEnv("HTTPS_PROXY")
const [setNoProxy, resetNoProxy] = useEnv("NO_PROXY")
beforeAll(() => {
mockLogger()
})
beforeEach(() => {
jest.resetModules() // Most important - it clears the cache
resetHTTPProxy()

5
test/unit/node/routes/login.test.ts
View File

@ -1,8 +1,13 @@
import { RateLimiter } from "../../../../src/node/routes/login"
import { mockLogger } from "../../../utils/helpers"
import * as httpserver from "../../../utils/httpserver"
import * as integration from "../../../utils/integration"
describe("login", () => {
beforeAll(() => {
mockLogger()
})
describe("RateLimiter", () => {
it("should allow one try ", () => {
const limiter = new RateLimiter()

6
test/unit/node/routes/static.test.ts
View File

@ -1,7 +1,7 @@
import { promises as fs } from "fs"
import * as path from "path"
import { rootPath } from "../../../../src/node/constants"
import { tmpdir } from "../../../utils/helpers"
import { clean, tmpdir } from "../../../utils/helpers"
import * as httpserver from "../../../utils/httpserver"
import * as integration from "../../../utils/integration"
@ -23,8 +23,10 @@ describe("/_static", () => {
let testFileContent: string | undefined
let nonExistentTestFile: string | undefined
const testName = "_static"
beforeAll(async () => {
const testDir = await tmpdir("_static")
await clean(testName)
const testDir = await tmpdir(testName)
testFile = path.join(testDir, "test")
testFileContent = "static file contents"
nonExistentTestFile = path.join(testDir, "i-am-not-here")

158
test/unit/node/routes/vscode.test.ts Normal file
View File

@ -0,0 +1,158 @@
import { promises as fs } from "fs"
import { Response } from "node-fetch"
import * as path from "path"
import { clean, tmpdir } from "../../../utils/helpers"
import * as httpserver from "../../../utils/httpserver"
import * as integration from "../../../utils/integration"
interface WorkbenchConfig {
folderUri?: {
path: string
}
workspaceUri?: {
path: string
}
}
describe("vscode", () => {
let codeServer: httpserver.HttpServer | undefined
const testName = "vscode"
beforeAll(async () => {
await clean(testName)
})
afterEach(async () => {
if (codeServer) {
await codeServer.dispose()
codeServer = undefined
}
})
const routes = ["/", "/vscode", "/vscode/"]
it("should load all route variations", async () => {
codeServer = await integration.setup(["--auth=none"], "")
for (const route of routes) {
const resp = await codeServer.fetch(route)
expect(resp.status).toBe(200)
const html = await resp.text()
const url = new URL(resp.url) // Check there were no redirections.
expect(url.pathname + decodeURIComponent(url.search)).toBe(route)
switch (route) {
case "/":
case "/vscode/":
expect(html).toContain(`src="./static/`)
break
case "/vscode":
expect(html).toContain(`src="./vscode/static/`)
break
}
}
})
/**
* Get the workbench config from the provided response.
*/
const getConfig = async (resp: Response): Promise<WorkbenchConfig> => {
expect(resp.status).toBe(200)
const html = await resp.text()
const match = html.match(/<meta id="vscode-workbench-web-configuration" data-settings="(.+)">/)
if (!match || !match[1]) {
throw new Error("Unable to find workbench configuration")
}
const config = match[1].replace(/&quot;/g, '"')
try {
return JSON.parse(config)
} catch (error) {
console.error("Failed to parse workbench configuration", config)
throw error
}
}
it("should have no default folder or workspace", async () => {
codeServer = await integration.setup(["--auth=none"], "")
const config = await getConfig(await codeServer.fetch("/"))
expect(config.folderUri).toBeUndefined()
expect(config.workspaceUri).toBeUndefined()
})
it("should have a default folder", async () => {
const defaultDir = await tmpdir(testName)
codeServer = await integration.setup(["--auth=none", defaultDir], "")
// At first it will load the directory provided on the command line.
const config = await getConfig(await codeServer.fetch("/"))
expect(config.folderUri?.path).toBe(defaultDir)
expect(config.workspaceUri).toBeUndefined()
})
it("should have a default workspace", async () => {
const defaultWorkspace = path.join(await tmpdir(testName), "test.code-workspace")
await fs.writeFile(defaultWorkspace, "")
codeServer = await integration.setup(["--auth=none", defaultWorkspace], "")
// At first it will load the workspace provided on the command line.
const config = await getConfig(await codeServer.fetch("/"))
expect(config.folderUri).toBeUndefined()
expect(config.workspaceUri?.path).toBe(defaultWorkspace)
})
it("should redirect to last query folder/workspace", async () => {
codeServer = await integration.setup(["--auth=none"], "")
const folder = await tmpdir(testName)
const workspace = path.join(await tmpdir(testName), "test.code-workspace")
let resp = await codeServer.fetch("/", undefined, {
folder,
workspace,
})
expect(resp.status).toBe(200)
await resp.text()
// If you visit again without query parameters it will re-attach them by
// redirecting. It should always redirect to the same route.
for (const route of routes) {
resp = await codeServer.fetch(route)
const url = new URL(resp.url)
expect(url.pathname).toBe(route)
expect(decodeURIComponent(url.search)).toBe(`?folder=${folder}&workspace=${workspace}`)
await resp.text()
}
// Closing the folder should stop the redirecting.
resp = await codeServer.fetch("/", undefined, { ew: "true" })
let url = new URL(resp.url)
expect(url.pathname).toBe("/")
expect(decodeURIComponent(url.search)).toBe("?ew=true")
await resp.text()
resp = await codeServer.fetch("/")
url = new URL(resp.url)
expect(url.pathname).toBe("/")
expect(decodeURIComponent(url.search)).toBe("")
await resp.text()
})
it("should not redirect when last opened is ignored", async () => {
codeServer = await integration.setup(["--auth=none", "--ignore-last-opened"], "")
const folder = await tmpdir(testName)
const workspace = path.join(await tmpdir(testName), "test.code-workspace")
let resp = await codeServer.fetch("/", undefined, {
folder,
workspace,
})
expect(resp.status).toBe(200)
await resp.text()
// No redirections.
resp = await codeServer.fetch("/")
const url = new URL(resp.url)
expect(url.pathname).toBe("/")
expect(decodeURIComponent(url.search)).toBe("")
await resp.text()
})
})

49
test/unit/node/update.test.ts
View File

@ -1,9 +1,8 @@
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 { clean, mockLogger, tmpdir } from "../../utils/helpers"
describe("update", () => {
let version = "1.0.0"
@ -29,22 +28,31 @@ describe("update", () => {
response.end("not found")
})
const jsonPath = path.join(tmpdir, "tests/updates/update.json")
const settings = new SettingsProvider<UpdateSettings>(jsonPath)
let _settings: SettingsProvider<UpdateSettings> | undefined
const settings = (): SettingsProvider<UpdateSettings> => {
if (!_settings) {
throw new Error("Settings provider has not been created")
}
return _settings
}
let _provider: UpdateProvider | undefined
const provider = (): UpdateProvider => {
if (!_provider) {
const address = server.address()
if (!address || typeof address === "string" || !address.port) {
throw new Error("unexpected address")
}
_provider = new UpdateProvider(`http://${address.address}:${address.port}/latest`, settings)
throw new Error("Update provider has not been created")
}
return _provider
}
beforeAll(async () => {
mockLogger()
const testName = "update"
await clean(testName)
const testDir = await tmpdir(testName)
const jsonPath = path.join(testDir, "update.json")
_settings = new SettingsProvider<UpdateSettings>(jsonPath)
await new Promise((resolve, reject) => {
server.on("error", reject)
server.on("listening", resolve)
@ -53,8 +61,13 @@ describe("update", () => {
host: "localhost",
})
})
await fs.rmdir(path.join(tmpdir, "tests/updates"), { recursive: true })
await fs.mkdir(path.join(tmpdir, "tests/updates"), { recursive: true })
const address = server.address()
if (!address || typeof address === "string" || !address.port) {
throw new Error("unexpected address")
}
_provider = new UpdateProvider(`http://${address.address}:${address.port}/latest`, _settings)
})
afterAll(() => {
@ -72,7 +85,7 @@ describe("update", () => {
const now = Date.now()
const update = await p.getUpdate()
await expect(settings.read()).resolves.toEqual({ update })
await expect(settings().read()).resolves.toEqual({ update })
expect(isNaN(update.checked)).toEqual(false)
expect(update.checked < Date.now() && update.checked >= now).toEqual(true)
expect(update.version).toStrictEqual("2.1.0")
@ -86,7 +99,7 @@ describe("update", () => {
const now = Date.now()
const update = await p.getUpdate()
await expect(settings.read()).resolves.toEqual({ update })
await expect(settings().read()).resolves.toEqual({ update })
expect(isNaN(update.checked)).toStrictEqual(false)
expect(update.checked < now).toBe(true)
expect(update.version).toStrictEqual("2.1.0")
@ -100,7 +113,7 @@ describe("update", () => {
const now = Date.now()
const update = await p.getUpdate(true)
await expect(settings.read()).resolves.toEqual({ update })
await expect(settings().read()).resolves.toEqual({ update })
expect(isNaN(update.checked)).toStrictEqual(false)
expect(update.checked < Date.now() && update.checked >= now).toStrictEqual(true)
expect(update.version).toStrictEqual("4.1.1")
@ -113,12 +126,12 @@ describe("update", () => {
expect(spy).toEqual([])
let checked = Date.now() - 1000 * 60 * 60 * 23
await settings.write({ update: { checked, version } })
await settings().write({ update: { checked, version } })
await p.getUpdate()
expect(spy).toEqual([])
checked = Date.now() - 1000 * 60 * 60 * 25
await settings.write({ update: { checked, version } })
await settings().write({ update: { checked, version } })
const update = await p.getUpdate()
expect(update.checked).not.toStrictEqual(checked)
@ -143,14 +156,14 @@ describe("update", () => {
})
it("should not reject if unable to fetch", async () => {
let provider = new UpdateProvider("invalid", settings)
let provider = new UpdateProvider("invalid", settings())
let now = Date.now()
let update = await provider.getUpdate(true)
expect(isNaN(update.checked)).toStrictEqual(false)
expect(update.checked < Date.now() && update.checked >= now).toEqual(true)
expect(update.version).toStrictEqual("unknown")
provider = new UpdateProvider("http://probably.invalid.dev.localhost/latest", settings)
provider = new UpdateProvider("http://probably.invalid.dev.localhost/latest", settings())
now = Date.now()
update = await provider.getUpdate(true)
expect(isNaN(update.checked)).toStrictEqual(false)