Skip to content

feat(testing): add unit tests for constants #2701

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 5 commits into from
Feb 10, 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
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
},
"main": "out/node/entry.js",
"devDependencies": {
"@schemastore/package": "^0.0.6",
"@types/body-parser": "^1.19.0",
"@types/cookie-parser": "^1.4.2",
"@types/express": "^4.17.8",
Expand Down Expand Up @@ -62,8 +63,8 @@
"stylelint": "^13.0.0",
"stylelint-config-recommended": "^3.0.0",
"ts-node": "^9.0.0",
"wtfnode": "^0.8.4",
"typescript": "^4.1.3"
"typescript": "^4.1.3",
"wtfnode": "^0.8.4"
Comment on lines -65 to +67
Copy link

Choose a reason for hiding this comment

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

I wonder if this was a manual change or something done automatically? Not particularly an issue, just curious 👀

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that happens when you add a new dependency to the top? Not sure, I did see it in one of Asher's PRs

Copy link
Member

Choose a reason for hiding this comment

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

Yeah yarn resorts the dependencies. 😄

Copy link
Member

Choose a reason for hiding this comment

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

The real question is how it got out of order in the first place lol

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LOL I have no idea - could have been me 😂

Copy link
Member

Choose a reason for hiding this comment

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

Haha idk I say we just blame yarn.

},
"resolutions": {
"@types/node": "^12.12.7",
Expand Down
17 changes: 12 additions & 5 deletions src/node/constants.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,20 @@
import { logger } from "@coder/logger"
import { JSONSchemaForNPMPackageJsonFiles } from "@schemastore/package"
import * as path from "path"

let pkg: { version?: string; commit?: string } = {}
try {
pkg = require("../../package.json")
} catch (error) {
logger.warn(error.message)
export function getPackageJson(relativePath: string): JSONSchemaForNPMPackageJsonFiles {
let pkg = {}
try {
pkg = require(relativePath)
} catch (error) {
logger.warn(error.message)
}

return pkg
}

const pkg = getPackageJson("../../package.json")

export const version = pkg.version || "development"
export const commit = pkg.commit || "development"
export const rootPath = path.resolve(__dirname, "../..")
58 changes: 58 additions & 0 deletions test/constants.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// 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 { commit, getPackageJson, version } from "../src/node/constants"

describe("constants", () => {
describe("getPackageJson", () => {
let spy: jest.SpyInstance

beforeEach(() => {
spy = jest.spyOn(logger, "warn")
})

afterEach(() => {
jest.clearAllMocks()
})

afterAll(() => {
jest.restoreAllMocks()
})

it("should log a warning if package.json not found", () => {
const expectedErrorMessage = "Cannot find module './package.json' from 'src/node/constants.ts'"

getPackageJson("./package.json")

expect(spy).toHaveBeenCalled()
expect(spy).toHaveBeenCalledWith(expectedErrorMessage)
})

it("should find the package.json", () => {
// the function calls require from src/node/constants
// so to get the root package.json we need to use ../../
const packageJson = getPackageJson("../../package.json")
expect(Object.keys(packageJson).length).toBeGreaterThan(0)
expect(packageJson.name).toBe("code-server")
expect(packageJson.description).toBe("Run VS Code on a remote server.")
expect(packageJson.repository).toBe("https://github.com/cdr/code-server")
})
})
describe("version", () => {
it("should return the package.json version", () => {
// Source: https://gist.github.com/jhorsman/62eeea161a13b80e39f5249281e17c39#gistcomment-2896416
const validSemVar = new RegExp("^(0|[1-9]d*).(0|[1-9]d*).(0|[1-9]d*)")
const isValidSemVar = validSemVar.test(version)
expect(version).not.toBe(null)
expect(isValidSemVar).toBe(true)
})
})

describe("commit", () => {
it("should return 'development' if commit is undefined", () => {
// In development, the commit is not stored in our package.json
// But when we build code-server and release it, it is
expect(commit).toBe("development")
})
})
})
35 changes: 35 additions & 0 deletions test/http.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { HttpCode, HttpError } from "../src/common/http"

describe("http", () => {
describe("HttpCode", () => {
it("should return the correct HTTP codes", () => {
expect(HttpCode.Ok).toBe(200)
expect(HttpCode.Redirect).toBe(302)
expect(HttpCode.NotFound).toBe(404)
expect(HttpCode.BadRequest).toBe(400)
expect(HttpCode.Unauthorized).toBe(401)
expect(HttpCode.LargePayload).toBe(413)
expect(HttpCode.ServerError).toBe(500)
})
})

describe("HttpError", () => {
it("should work as expected", () => {
const message = "Bad request from client"
const httpError = new HttpError(message, HttpCode.BadRequest)

expect(httpError.message).toBe(message)
expect(httpError.status).toBe(400)
expect(httpError.details).toBeUndefined()
})
it("should have details if provided", () => {
const details = {
message: "User needs to be signed-in in order to perform action",
}
const message = "Unauthorized"
const httpError = new HttpError(message, HttpCode.BadRequest, details)

expect(httpError.details).toStrictEqual(details)
})
})
})
4 changes: 0 additions & 4 deletions test/util.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,6 @@ describe("util", () => {
})

describe("getOptions", () => {
// Things to mock
// logger
// location
// document
Comment on lines -128 to -131
Copy link

Choose a reason for hiding this comment

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

Comment disappeared! no other changes in this file; curious about the change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was intentional! See commit: 164d11e

beforeEach(() => {
const location: LocationLike = {
pathname: "/healthz",
Expand Down
5 changes: 5 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -980,6 +980,11 @@
"@parcel/utils" "^1.11.0"
physical-cpu-count "^2.0.0"

"@schemastore/package@^0.0.6":
version "0.0.6"
resolved "https://registry.yarnpkg.com/@schemastore/package/-/package-0.0.6.tgz#9a76713da1c7551293b7e72e4f387f802bfd5d81"
integrity sha512-uNloNHoyHttSSdeuEkkSC+mdxJXMKlcUPOMb//qhQbIQijXg8x54VmAw3jm6GJZQ5DBtIqGBd66zEQCDCChQVA==

"@stylelint/postcss-css-in-js@^0.37.2":
version "0.37.2"
resolved "https://registry.yarnpkg.com/@stylelint/postcss-css-in-js/-/postcss-css-in-js-0.37.2.tgz#7e5a84ad181f4234a2480803422a47b8749af3d2"
Expand Down