feat(cli): add test for readSocketPath (#4284)

* fix: update isNodeJSErrnoException

* refactor(cli): export and purify readSocketPath

* feat: add tests for readSocketPath

* fix(ci): temporarily disable install deps from cache
This commit is contained in:
Joe Previte 2021-10-29 16:03:57 -07:00 committed by GitHub
parent 49c9c191b9
commit 946e4e8843
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 80 additions and 26 deletions

View File

@ -31,14 +31,18 @@ jobs:
- name: Install helm - name: Install helm
uses: azure/setup-helm@v1.1 uses: azure/setup-helm@v1.1
- name: Fetch dependencies from cache # NOTE@jsjoeio
id: cache-yarn # disabling this until we can audit the build process
uses: actions/cache@v2 # and the usefulness of this step
with: # See: https://github.com/cdr/code-server/issues/4287
path: "**/node_modules" # - name: Fetch dependencies from cache
key: yarn-build-${{ hashFiles('**/yarn.lock') }} # id: cache-yarn
restore-keys: | # uses: actions/cache@v2
yarn-build- # with:
# path: "**/node_modules"
# key: yarn-build-${{ hashFiles('**/yarn.lock') }}
# restore-keys: |
# yarn-build-
- name: Install dependencies - name: Install dependencies
# if: steps.cache-yarn.outputs.cache-hit != 'true' # if: steps.cache-yarn.outputs.cache-hit != 'true'

View File

@ -3,7 +3,9 @@ import { promises as fs } from "fs"
import yaml from "js-yaml" import yaml from "js-yaml"
import * as os from "os" import * as os from "os"
import * as path from "path" import * as path from "path"
import { canConnect, generateCertificate, generatePassword, humanPath, paths } from "./util" import { canConnect, generateCertificate, generatePassword, humanPath, paths, isNodeJSErrnoException } from "./util"
const DEFAULT_SOCKET_PATH = path.join(os.tmpdir(), "vscode-ipc")
export enum Feature { export enum Feature {
// No current experimental features! // No current experimental features!
@ -657,6 +659,27 @@ function bindAddrFromAllSources(...argsConfig: Args[]): Addr {
return addr return addr
} }
/**
* Reads the socketPath based on path passed in.
*
* The one usually passed in is the DEFAULT_SOCKET_PATH.
*
* If it can't read the path, it throws an error and returns undefined.
*/
export async function readSocketPath(path: string): Promise<string | undefined> {
try {
return await fs.readFile(path, "utf8")
} catch (error) {
// If it doesn't exist, we don't care.
// But if it fails for some reason, we should throw.
// We want to surface that to the user.
if (!isNodeJSErrnoException(error) || error.code !== "ENOENT") {
throw error
}
}
return undefined
}
/** /**
* Determine if it looks like the user is trying to open a file or folder in an * Determine if it looks like the user is trying to open a file or folder in an
* existing instance. The arguments here should be the arguments the user * existing instance. The arguments here should be the arguments the user
@ -668,24 +691,13 @@ export const shouldOpenInExistingInstance = async (args: Args): Promise<string |
return process.env.VSCODE_IPC_HOOK_CLI return process.env.VSCODE_IPC_HOOK_CLI
} }
const readSocketPath = async (): Promise<string | undefined> => {
try {
return await fs.readFile(path.join(os.tmpdir(), "vscode-ipc"), "utf8")
} catch (error: any) {
if (error.code !== "ENOENT") {
throw error
}
}
return undefined
}
// If these flags are set then assume the user is trying to open in an // If these flags are set then assume the user is trying to open in an
// existing instance since these flags have no effect otherwise. // existing instance since these flags have no effect otherwise.
const openInFlagCount = ["reuse-window", "new-window"].reduce((prev, cur) => { const openInFlagCount = ["reuse-window", "new-window"].reduce((prev, cur) => {
return args[cur as keyof Args] ? prev + 1 : prev return args[cur as keyof Args] ? prev + 1 : prev
}, 0) }, 0)
if (openInFlagCount > 0) { if (openInFlagCount > 0) {
return readSocketPath() return readSocketPath(DEFAULT_SOCKET_PATH)
} }
// It's possible the user is trying to spawn another instance of code-server. // It's possible the user is trying to spawn another instance of code-server.
@ -693,7 +705,7 @@ export const shouldOpenInExistingInstance = async (args: Args): Promise<string |
// exists), that a file or directory was passed, and that the socket is // exists), that a file or directory was passed, and that the socket is
// active. // active.
if (Object.keys(args).length === 1 && args._.length > 0) { if (Object.keys(args).length === 1 && args._.length > 0) {
const socketPath = await readSocketPath() const socketPath = await readSocketPath(DEFAULT_SOCKET_PATH)
if (socketPath && (await canConnect(socketPath))) { if (socketPath && (await canConnect(socketPath))) {
return socketPath return socketPath
} }

View File

@ -490,7 +490,7 @@ export function escapeHtml(unsafe: string): string {
* it has a .code property. * it has a .code property.
*/ */
export function isNodeJSErrnoException(error: unknown): error is NodeJS.ErrnoException { export function isNodeJSErrnoException(error: unknown): error is NodeJS.ErrnoException {
return error instanceof Error && (error as NodeJS.ErrnoException).code !== undefined return error !== undefined && (error as NodeJS.ErrnoException).code !== undefined
} }
// TODO: Replace with proper templating system. // TODO: Replace with proper templating system.

View File

@ -11,11 +11,11 @@ import {
setDefaults, setDefaults,
shouldOpenInExistingInstance, shouldOpenInExistingInstance,
splitOnFirstEquals, splitOnFirstEquals,
readSocketPath,
} from "../../../src/node/cli" } from "../../../src/node/cli"
import { tmpdir } from "../../../src/node/constants"
import { shouldSpawnCliProcess } from "../../../src/node/main" import { shouldSpawnCliProcess } from "../../../src/node/main"
import { generatePassword, paths } from "../../../src/node/util" import { generatePassword, paths } from "../../../src/node/util"
import { useEnv } from "../../utils/helpers" import { useEnv, tmpdir } from "../../utils/helpers"
type Mutable<T> = { type Mutable<T> = {
-readonly [P in keyof T]: T[P] -readonly [P in keyof T]: T[P]
@ -390,10 +390,11 @@ describe("parser", () => {
describe("cli", () => { describe("cli", () => {
let args: Mutable<Args> = { _: [] } let args: Mutable<Args> = { _: [] }
const testDir = path.join(tmpdir, "tests/cli") let testDir: string
const vscodeIpcPath = path.join(os.tmpdir(), "vscode-ipc") const vscodeIpcPath = path.join(os.tmpdir(), "vscode-ipc")
beforeAll(async () => { beforeAll(async () => {
testDir = await tmpdir("cli")
await fs.rmdir(testDir, { recursive: true }) await fs.rmdir(testDir, { recursive: true })
await fs.mkdir(testDir, { recursive: true }) await fs.mkdir(testDir, { recursive: true })
}) })
@ -667,3 +668,40 @@ password: ${password}
cert: false`) cert: false`)
}) })
}) })
describe("readSocketPath", () => {
const fileContents = "readSocketPath file contents"
let tmpDirPath: string
let tmpFilePath: string
beforeEach(async () => {
tmpDirPath = await tmpdir("readSocketPath")
tmpFilePath = path.join(tmpDirPath, "readSocketPath.txt")
await fs.writeFile(tmpFilePath, fileContents)
})
afterEach(async () => {
await fs.rmdir(tmpDirPath, { recursive: true })
})
it("should throw an error if it can't read the file", async () => {
// TODO@jsjoeio - implement
// Test it on a directory.... ESDIR
// TODO@jsjoeio - implement
expect(() => readSocketPath(tmpDirPath)).rejects.toThrow("EISDIR")
})
it("should return undefined if it can't read the file", async () => {
// TODO@jsjoeio - implement
const socketPath = await readSocketPath(path.join(tmpDirPath, "not-a-file"))
expect(socketPath).toBeUndefined()
})
it("should return the file contents", async () => {
const contents = await readSocketPath(tmpFilePath)
expect(contents).toBe(fileContents)
})
it("should return the same file contents for two different calls", async () => {
const contents1 = await readSocketPath(tmpFilePath)
const contents2 = await readSocketPath(tmpFilePath)
expect(contents2).toBe(contents1)
})
})