From 71cf459ececd572e32e0ac5f6695809db852fed4 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Wed, 3 Feb 2021 11:22:13 -0700 Subject: [PATCH 1/4] feat: add tests for common/util --- test/util.test.ts | 107 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 106 insertions(+), 1 deletion(-) diff --git a/test/util.test.ts b/test/util.test.ts index d5eb37e46..d6883e103 100644 --- a/test/util.test.ts +++ b/test/util.test.ts @@ -1,4 +1,7 @@ -import { normalize } from "../src/common/util" +import { logger as l } from "@coder/logger" +import { arrayify, getFirstString, normalize, plural, resolveBase, split, trimSlashes } from "../src/common/util" + +type LocationLike = Pick describe("util", () => { describe("normalize", () => { @@ -15,4 +18,106 @@ describe("util", () => { expect(normalize("qux", true)).toBe("qux") }) }) + + describe("split", () => { + it("should split at a comma", () => { + expect(split("Hello,world", ",")).toStrictEqual(["Hello", "world"]) + }) + + it("shouldn't split if the delimiter doesn't exist", () => { + expect(split("Hello world", ",")).toStrictEqual(["Hello world", ""]) + }) + }) + + describe("plural", () => { + it("should add an s if count is greater than 1", () => { + expect(plural(2, "dog")).toBe("dogs") + }) + it("should NOT add an s if the count is 1", () => { + expect(plural(1, "dog")).toBe("dog") + }) + }) + + describe("trimSlashes", () => { + it("should remove leading slashes", () => { + expect(trimSlashes("/hello-world")).toBe("hello-world") + }) + + it("should remove trailing slashes", () => { + expect(trimSlashes("hello-world/")).toBe("hello-world") + }) + + it("should remove both leading and trailing slashes", () => { + expect(trimSlashes("/hello-world/")).toBe("hello-world") + }) + + it("should remove multiple leading and trailing slashes", () => { + expect(trimSlashes("///hello-world////")).toBe("hello-world") + }) + }) + + describe("resolveBase", () => { + beforeEach(() => { + const location: LocationLike = { + pathname: "/healthz", + origin: "http://localhost:8080", + } + + // Because resolveBase is not a pure function + // and relies on the global location to be set + // we set it before all the tests + // and tell TS that our location should be looked at + // as Location (even though it's missing some properties) + global.location = location as Location + }) + + it("should resolve a base", () => { + expect(resolveBase("localhost:8080")).toBe("/localhost:8080") + }) + + it("should resolve a base with a forward slash at the beginning", () => { + expect(resolveBase("/localhost:8080")).toBe("/localhost:8080") + }) + + it("should resolve a base with query params", () => { + expect(resolveBase("localhost:8080?folder=hello-world")).toBe("/localhost:8080") + }) + + it("should resolve a base with a path", () => { + expect(resolveBase("localhost:8080/hello/world")).toBe("/localhost:8080/hello/world") + }) + + it("should resolve a base to an empty string when not provided", () => { + expect(resolveBase()).toBe("") + }) + }) + + describe("arrayify", () => { + it("should return value it's already an array", () => { + expect(arrayify(["hello", "world"])).toStrictEqual(["hello", "world"]) + }) + it("should wrap the value in an array if not an array", () => { + expect( + arrayify({ + name: "Coder", + version: "3.8", + }), + ).toStrictEqual([{ name: "Coder", version: "3.8" }]) + }) + it("should return an empty array if the value is undefined", () => { + expect(arrayify(undefined)).toStrictEqual([]) + }) + }) + + describe("getFirstString", () => { + it("should return the string if passed a string", () => { + expect(getFirstString("Hello world!")).toBe("Hello world!") + }) + it("should get the first string from an array", () => { + expect(getFirstString(["Hello", "World"])).toBe("Hello") + }) + it("should return undefined if the value isn't an array or a string", () => { + expect(getFirstString({ name: "Coder" })).toBe(undefined) + }) + }) }) From 3cebfcd447cb97a43c1fdf71704edbb16e5ddea4 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Wed, 3 Feb 2021 16:10:17 -0700 Subject: [PATCH 2/4] feat: add tests for logError --- test/util.test.ts | 52 +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 50 insertions(+), 2 deletions(-) diff --git a/test/util.test.ts b/test/util.test.ts index d6883e103..45179bdcd 100644 --- a/test/util.test.ts +++ b/test/util.test.ts @@ -1,5 +1,16 @@ -import { logger as l } from "@coder/logger" -import { arrayify, getFirstString, normalize, plural, resolveBase, split, trimSlashes } from "../src/common/util" +// 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 { + arrayify, + getFirstString, + logError, + normalize, + plural, + resolveBase, + split, + trimSlashes, +} from "../src/common/util" type LocationLike = Pick @@ -96,6 +107,7 @@ describe("util", () => { it("should return value it's already an array", () => { expect(arrayify(["hello", "world"])).toStrictEqual(["hello", "world"]) }) + it("should wrap the value in an array if not an array", () => { expect( arrayify({ @@ -104,6 +116,7 @@ describe("util", () => { }), ).toStrictEqual([{ name: "Coder", version: "3.8" }]) }) + it("should return an empty array if the value is undefined", () => { expect(arrayify(undefined)).toStrictEqual([]) }) @@ -113,11 +126,46 @@ describe("util", () => { it("should return the string if passed a string", () => { expect(getFirstString("Hello world!")).toBe("Hello world!") }) + it("should get the first string from an array", () => { expect(getFirstString(["Hello", "World"])).toBe("Hello") }) + it("should return undefined if the value isn't an array or a string", () => { expect(getFirstString({ name: "Coder" })).toBe(undefined) }) }) + + describe("logError", () => { + let spy: jest.SpyInstance + + beforeEach(() => { + spy = jest.spyOn(logger, "error") + }) + + afterEach(() => { + jest.clearAllMocks() + }) + + afterAll(() => { + jest.restoreAllMocks() + }) + + it("should log an error with the message and stack trace", () => { + const message = "You don't have access to that folder." + const error = new Error(message) + + logError("ui", error) + + expect(spy).toHaveBeenCalled() + expect(spy).toHaveBeenCalledWith(`ui: ${error.message} ${error.stack}`) + }) + + it("should log an error, even if not an instance of error", () => { + logError("api", "oh no") + + expect(spy).toHaveBeenCalled() + expect(spy).toHaveBeenCalledWith("api: oh no") + }) + }) }) From 323339d15aea8dde24869f3d4b769bea523cc275 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Wed, 3 Feb 2021 16:52:06 -0700 Subject: [PATCH 3/4] feat: add jsdom for browser-ish tests --- test/package.json | 3 ++- test/yarn.lock | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/test/package.json b/test/package.json index 2a5e96138..e3cb94f61 100644 --- a/test/package.json +++ b/test/package.json @@ -1,11 +1,12 @@ { - "#": "We must put jest in a sub-directory otherwise VS Code somehow picks up", "#": "the types and generates conflicts with mocha.", "devDependencies": { "@types/jest": "^26.0.20", + "@types/jsdom": "^16.2.6", "@types/node-fetch": "^2.5.8", "@types/supertest": "^2.0.10", "jest": "^26.6.3", + "jsdom": "^16.4.0", "node-fetch": "^2.6.1", "playwright": "^1.8.0", "supertest": "^6.1.1", diff --git a/test/yarn.lock b/test/yarn.lock index a8bc09050..4f6ae7ab3 100644 --- a/test/yarn.lock +++ b/test/yarn.lock @@ -551,6 +551,15 @@ jest-diff "^26.0.0" pretty-format "^26.0.0" +"@types/jsdom@^16.2.6": + version "16.2.6" + resolved "https://registry.yarnpkg.com/@types/jsdom/-/jsdom-16.2.6.tgz#9ddf0521e49be5365797e690c3ba63148e562c29" + integrity sha512-yQA+HxknGtW9AkRTNyiSH3OKW5V+WzO8OPTdne99XwJkYC+KYxfNIcoJjeiSqP3V00PUUpFP6Myoo9wdIu78DQ== + dependencies: + "@types/node" "*" + "@types/parse5" "*" + "@types/tough-cookie" "*" + "@types/node-fetch@^2.5.8": version "2.5.8" resolved "https://registry.yarnpkg.com/@types/node-fetch/-/node-fetch-2.5.8.tgz#e199c835d234c7eb0846f6618012e558544ee2fb" @@ -569,6 +578,11 @@ resolved "https://registry.yarnpkg.com/@types/normalize-package-data/-/normalize-package-data-2.4.0.tgz#e486d0d97396d79beedd0a6e33f4534ff6b4973e" integrity sha512-f5j5b/Gf71L+dbqxIpQ4Z2WlmI/mPJ0fOkGGmFgtb6sAu97EPczzbS3/tJKxmcYDj55OX6ssqwDAWOHIYDRDGA== +"@types/parse5@*": + version "6.0.0" + resolved "https://registry.yarnpkg.com/@types/parse5/-/parse5-6.0.0.tgz#38590dc2c3cf5717154064e3ee9b6947ee21b299" + integrity sha512-oPwPSj4a1wu9rsXTEGIJz91ISU725t0BmSnUhb57sI+M8XEmvUop84lzuiYdq0Y5M6xLY8DBPg0C2xEQKLyvBA== + "@types/prettier@^2.0.0": version "2.1.6" resolved "https://registry.yarnpkg.com/@types/prettier/-/prettier-2.1.6.tgz#f4b1efa784e8db479cdb8b14403e2144b1e9ff03" @@ -594,6 +608,11 @@ dependencies: "@types/superagent" "*" +"@types/tough-cookie@*": + version "4.0.0" + resolved "https://registry.yarnpkg.com/@types/tough-cookie/-/tough-cookie-4.0.0.tgz#fef1904e4668b6e5ecee60c52cc6a078ffa6697d" + integrity sha512-I99sngh224D0M7XgW1s120zxCt3VYQ3IQsuw3P3jbq5GG4yc79+ZjyKznyOGIQrflfylLgcfekeZW/vk0yng6A== + "@types/yargs-parser@*": version "20.2.0" resolved "https://registry.yarnpkg.com/@types/yargs-parser/-/yargs-parser-20.2.0.tgz#dd3e6699ba3237f0348cd085e4698780204842f9" From 4f6efced683c9d66facf7b231a43d1f4a0bf1841 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Wed, 3 Feb 2021 16:52:17 -0700 Subject: [PATCH 4/4] feat: add tests for getOptions --- test/package.json | 2 +- test/util.test.ts | 91 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 1 deletion(-) diff --git a/test/package.json b/test/package.json index e3cb94f61..4a408464c 100644 --- a/test/package.json +++ b/test/package.json @@ -1,5 +1,5 @@ { - "#": "the types and generates conflicts with mocha.", + "#": "We must put jest in a sub-directory otherwise VS Code somehow picks up the types and generates conflicts with mocha.", "devDependencies": { "@types/jest": "^26.0.20", "@types/jsdom": "^16.2.6", diff --git a/test/util.test.ts b/test/util.test.ts index 45179bdcd..418756a58 100644 --- a/test/util.test.ts +++ b/test/util.test.ts @@ -1,9 +1,12 @@ +import { JSDOM } from "jsdom" // 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 { arrayify, + generateUuid, getFirstString, + getOptions, logError, normalize, plural, @@ -12,6 +15,10 @@ import { trimSlashes, } from "../src/common/util" +const dom = new JSDOM() +global.document = dom.window.document +// global.window = (dom.window as unknown) as Window & typeof globalThis + type LocationLike = Pick describe("util", () => { @@ -49,6 +56,20 @@ describe("util", () => { }) }) + describe("generateUuid", () => { + it("should generate a unique uuid", () => { + const uuid = generateUuid() + const uuid2 = generateUuid() + expect(uuid).toHaveLength(24) + expect(typeof uuid).toBe("string") + expect(uuid).not.toBe(uuid2) + }) + it("should generate a uuid of a specific length", () => { + const uuid = generateUuid(10) + expect(uuid).toHaveLength(10) + }) + }) + describe("trimSlashes", () => { it("should remove leading slashes", () => { expect(trimSlashes("/hello-world")).toBe("hello-world") @@ -103,6 +124,76 @@ describe("util", () => { }) }) + describe("getOptions", () => { + // Things to mock + // logger + // location + // document + beforeEach(() => { + const location: LocationLike = { + pathname: "/healthz", + origin: "http://localhost:8080", + // search: "?environmentId=600e0187-0909d8a00cb0a394720d4dce", + } + + // Because resolveBase is not a pure function + // and relies on the global location to be set + // we set it before all the tests + // and tell TS that our location should be looked at + // as Location (even though it's missing some properties) + global.location = location as Location + }) + + afterEach(() => { + jest.restoreAllMocks() + }) + + it("should return options with base and cssStaticBase even if it doesn't exist", () => { + expect(getOptions()).toStrictEqual({ + base: "", + csStaticBase: "", + }) + }) + + it("should return options when they do exist", () => { + // Mock getElementById + const spy = jest.spyOn(document, "getElementById") + // Create a fake element and set the attribute + const mockElement = document.createElement("div") + mockElement.setAttribute( + "data-settings", + '{"base":".","csStaticBase":"./static/development/Users/jp/Dev/code-server","logLevel":2,"disableTelemetry":false,"disableUpdateCheck":false}', + ) + // Return mockElement from the spy + // this way, when we call "getElementById" + // it returns the element + spy.mockImplementation(() => mockElement) + + expect(getOptions()).toStrictEqual({ + base: "", + csStaticBase: "/static/development/Users/jp/Dev/code-server", + disableTelemetry: false, + disableUpdateCheck: false, + logLevel: 2, + }) + }) + + it("should include queryOpts", () => { + // Trying to understand how the implementation works + // 1. It grabs the search params from location.search (i.e. ?) + // 2. it then grabs the "options" param if it exists + // 3. then it creates a new options object + // spreads the original options + // then parses the queryOpts + location.search = '?options={"logLevel":2}' + expect(getOptions()).toStrictEqual({ + base: "", + csStaticBase: "", + logLevel: 2, + }) + }) + }) + describe("arrayify", () => { it("should return value it's already an array", () => { expect(arrayify(["hello", "world"])).toStrictEqual(["hello", "world"])