refactor: clean up

Author: jsjoeio

src/node/cli.ts

@@ -6,6 +6,8 @@ import * as path from "path"
 import { Args as VsArgs } from "../../typings/ipc"
 import { canConnect, generateCertificate, generatePassword, humanPath, isNodeJSErrnoException, paths } from "./util"
 
+const DEFAULT_SOCKET_PATH = path.join(os.tmpdir(), "vscode-ipc")
+
 export enum Feature {
   /** Web socket compression. */
   PermessageDeflate = "permessage-deflate",
@@ -662,18 +664,20 @@ export const shouldRunVsCodeCli = (args: Args): boolean => {
  *
  * If it can't read the path, it throws an error and returns undefined.
  */
-export async function readSocketPath(): Promise<string | undefined> {
-  // TODO@jsjoeio - we should make this function pure and pass in the path
-  // to the file it reads
+export async function readSocketPath(path: string): Promise<string | undefined> {
   try {
-    return await fs.readFile(path.join(os.tmpdir(), "vscode-ipc"), "utf8")
+    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
  * existing instance. The arguments here should be the arguments the user
@@ -691,15 +695,15 @@ export const shouldOpenInExistingInstance = async (args: Args): Promise<string |
     return args[cur as keyof Args] ? prev + 1 : prev
   }, 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.
   // Check if any unrelated flags are set (check against one because `_` always
   // exists), that a file or directory was passed, and that the socket is
   // active.
   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))) {
       return socketPath
     }

src/node/util.ts

@@ -531,5 +531,5 @@ export function escapeHtml(unsafe: string): string {
  * it has a .code property.
  */
 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
 }

test/unit/node/cli.test.ts

@@ -14,9 +14,8 @@ import {
   shouldRunVsCodeCli,
   splitOnFirstEquals,
 } from "../../../src/node/cli"
-import { tmpdir } from "../../../src/node/constants"
 import { generatePassword, paths } from "../../../src/node/util"
-import { useEnv } from "../../utils/helpers"
+import { tmpdir, useEnv } from "../../utils/helpers"
 
 type Mutable<T> = {
   -readonly [P in keyof T]: T[P]
@@ -379,10 +378,11 @@ describe("parser", () => {
 
 describe("cli", () => {
   let args: Mutable<Args> = { _: [] }
-  const testDir = path.join(tmpdir, "tests/cli")
   const vscodeIpcPath = path.join(os.tmpdir(), "vscode-ipc")
+  let testDir: string
 
   beforeAll(async () => {
+    testDir = await tmpdir("cli")
     await fs.rmdir(testDir, { recursive: true })
     await fs.mkdir(testDir, { recursive: true })
   })
@@ -657,32 +657,39 @@ cert: false`)
   })
 })
 
-describe.only("readSocketPath", () => {
-  const vscodeIpcPath = path.join(os.tmpdir(), "vscode-ipc")
+describe("readSocketPath", () => {
   const fileContents = "readSocketPath file contents"
+  let tmpDirPath: string
+  let tmpFilePath: string
 
   beforeEach(async () => {
-    await fs.writeFile(vscodeIpcPath, fileContents)
+    tmpDirPath = await tmpdir("readSocketPath")
+    tmpFilePath = path.join(tmpDirPath, "readSocketPath.txt")
+    await fs.writeFile(tmpFilePath, fileContents)
   })
 
   afterEach(async () => {
-    await fs.rmdir(vscodeIpcPath, { recursive: true })
+    await fs.rmdir(tmpDirPath, { recursive: true })
   })
 
-  it.skip("should throw an error if it can't read the file", async () => {
+  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
-    const p = await readSocketPath()
-    console.log(p)
-    expect(p).toThrowError("oops")
+    expect(() => readSocketPath(tmpDirPath)).rejects.toThrow("EISDIR")
   })
-  it.skip("should return undefined if it can't read the file", async () => {
+  it("should return undefined if it can't read the file", async () => {
     // TODO@jsjoeio - implement
-    const socketPath = await readSocketPath()
+    const socketPath = await readSocketPath(path.join(tmpDirPath, "not-a-file"))
     expect(socketPath).toBeUndefined()
   })
   it("should return the file contents", async () => {
-    const contents = await readSocketPath()
+    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)
+  })
 })