Skip to content

feat: add tests for shouldRunVsCodeCli and bindAddrFromArgs #4211

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions src/node/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,11 @@ interface Addr {
port: number
}

function bindAddrFromArgs(addr: Addr, args: Args): Addr {
/**
* This function creates the bind address
* using the CLI args.
*/
export function bindAddrFromArgs(addr: Addr, args: Args): Addr {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Exported so that we can unit test. Added short description for good measure.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool!

addr = { ...addr }
if (args["bind-addr"]) {
addr = parseBindAddr(args["bind-addr"])
Expand Down Expand Up @@ -626,7 +630,18 @@ function bindAddrFromAllSources(...argsConfig: Args[]): Addr {
}

export const shouldRunVsCodeCli = (args: Args): boolean => {
return !!args["list-extensions"] || !!args["install-extension"] || !!args["uninstall-extension"]
// Create new interface with only Arg keys
// keyof Args
// Turn that into an array
// Array<...>
type ExtensionArgs = Array<keyof Args>
const extensionRelatedArgs: ExtensionArgs = ["list-extensions", "install-extension", "uninstall-extension"]

const argKeys = Object.keys(args)

// If any of the extensionRelatedArgs are included in args
// then we don't want to run the vscode cli
return extensionRelatedArgs.some((arg) => argKeys.includes(arg))
}

/**
Expand Down
181 changes: 180 additions & 1 deletion test/unit/node/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,18 @@ 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 {
Args,
bindAddrFromArgs,
parse,
setDefaults,
shouldOpenInExistingInstance,
shouldRunVsCodeCli,
splitOnFirstEquals,
} from "../../../src/node/cli"
import { tmpdir } from "../../../src/node/constants"
import { paths } from "../../../src/node/util"
import { useEnv } from "../../utils/helpers"

type Mutable<T> = {
-readonly [P in keyof T]: T[P]
Expand Down Expand Up @@ -463,3 +472,173 @@ describe("splitOnFirstEquals", () => {
expect(actual).toEqual(expect.arrayContaining(expected))
})
})

describe("shouldRunVsCodeCli", () => {
it("should return false if no 'extension' related args passed in", () => {
const args = {
_: [],
}
const actual = shouldRunVsCodeCli(args)
const expected = false

expect(actual).toBe(expected)
})

it("should return true if 'list-extensions' passed in", () => {
const args = {
_: [],
["list-extensions"]: true,
}
const actual = shouldRunVsCodeCli(args)
const expected = true

expect(actual).toBe(expected)
})

it("should return true if 'install-extension' passed in", () => {
const args = {
_: [],
["install-extension"]: ["hello.world"],
}
const actual = shouldRunVsCodeCli(args)
const expected = true

expect(actual).toBe(expected)
})

it("should return true if 'uninstall-extension' passed in", () => {
const args = {
_: [],
["uninstall-extension"]: ["hello.world"],
}
const actual = shouldRunVsCodeCli(args)
const expected = true

expect(actual).toBe(expected)
})
})

describe("bindAddrFromArgs", () => {
it("should return the bind address", () => {
const args = {
_: [],
}

const addr = {
host: "localhost",
port: 8080,
}

const actual = bindAddrFromArgs(addr, args)
const expected = addr

expect(actual).toStrictEqual(expected)
})

it("should use the bind-address if set in args", () => {
const args = {
_: [],
["bind-addr"]: "localhost:3000",
}

const addr = {
host: "localhost",
port: 8080,
}

const actual = bindAddrFromArgs(addr, args)
const expected = {
host: "localhost",
port: 3000,
}

expect(actual).toStrictEqual(expected)
})

it("should use the host if set in args", () => {
const args = {
_: [],
["host"]: "coder",
}

const addr = {
host: "localhost",
port: 8080,
}

const actual = bindAddrFromArgs(addr, args)
const expected = {
host: "coder",
port: 8080,
}

expect(actual).toStrictEqual(expected)
})

it("should use process.env.PORT if set", () => {
const [setValue, resetValue] = useEnv("PORT")
setValue("8000")

const args = {
_: [],
}

const addr = {
host: "localhost",
port: 8080,
}

const actual = bindAddrFromArgs(addr, args)
const expected = {
host: "localhost",
port: 8000,
}

expect(actual).toStrictEqual(expected)
resetValue()
})

it("should set port if in args", () => {
const args = {
_: [],
port: 3000,
}

const addr = {
host: "localhost",
port: 8080,
}

const actual = bindAddrFromArgs(addr, args)
const expected = {
host: "localhost",
port: 3000,
}

expect(actual).toStrictEqual(expected)
})

it("should use the args.port over process.env.PORT if both set", () => {
const [setValue, resetValue] = useEnv("PORT")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice set of tests 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

setValue("8000")

const args = {
_: [],
port: 3000,
}

const addr = {
host: "localhost",
port: 8080,
}

const actual = bindAddrFromArgs(addr, args)
const expected = {
host: "localhost",
port: 3000,
}

expect(actual).toStrictEqual(expected)
resetValue()
})
})