Skip to content

feat(testing): add tests for src/browser/pages/vscode to hit 100% coverage #4055

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.

Sign up for GitHub

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 1 commit into from
Aug 30, 2021
Merged

feat(testing): add tests for src/browser/pages/vscode to hit 100% coverage #4055

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
26 changes: 14 additions & 12 deletions src/browser/pages/vscode.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ type NlsConfiguration = {
* Helper function to create the path to the bundle
* for getNlsConfiguration.
*/
export function createBundlePath(_resolvedLanguagePackCoreLocation: string, bundle: string) {
export function createBundlePath(_resolvedLanguagePackCoreLocation: string | undefined, bundle: string) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

🧹 updated the signature to match the actual value passed in _resolvedLanguagePackCoreLocation's type is string | undefined

// NOTE@jsjoeio - this comment was here before me
// Refers to operating systems that use a different path separator.
// Probably just Windows but we're not sure if "/" breaks on Windows
// so we'll leave it alone for now.
// FIXME: Only works if path separators are /.
return _resolvedLanguagePackCoreLocation + "/" + bundle.replace(/\//g, "!") + ".nls.json"
return (_resolvedLanguagePackCoreLocation || "") + "/" + bundle.replace(/\//g, "!") + ".nls.json"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

🧹 Originally, we were using _resolvedLanguagePackCoreLocation or passing in an empty string. We moved the logic from outside the function into the function since the caller shouldn't have to change the argument functions if the value is undefined. Instead, the function should handle this.

}

/**
Expand Down Expand Up @@ -72,20 +72,22 @@ export function getNlsConfiguration(_document: Document, base: string) {

type LoadBundleCallback = (_: undefined, result?: string) => void

nlsConfig.loadBundle = (bundle: string, _language: string, cb: LoadBundleCallback): void => {
nlsConfig.loadBundle = async (bundle: string, _language: string, cb: LoadBundleCallback): Promise<void> => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

🧹 Originally, this function was not using async/await. I then realized we had the fetch call and the .then chain. I refactored this so it was easy to test. Now it's clear to the caller this function has asynchronous logic.

Copy link
Contributor

Choose a reason for hiding this comment

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

nice 👍

const result = bundles[bundle]

if (result) {
return cb(undefined, result)
}
// FIXME: Only works if path separators are /.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

🧹 Removed this comment because it's related to createBundlePath and explained in a comment near that function with more details.

const path = createBundlePath(nlsConfig._resolvedLanguagePackCoreLocation || "", bundle)
fetch(`${base}/vscode/resource/?path=${encodeURIComponent(path)}`)
.then((response) => response.json())
.then((json) => {
bundles[bundle] = json
cb(undefined, json)
})
.catch(cb)

try {
const path = createBundlePath(nlsConfig._resolvedLanguagePackCoreLocation, bundle)
const response = await fetch(`${base}/vscode/resource/?path=${encodeURIComponent(path)}`)
const json = await response.json()
bundles[bundle] = json
return cb(undefined, json)
} catch (error) {
return cb(error)
}
}
}

Expand Down
3 changes: 2 additions & 1 deletion test/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,15 @@
"@types/jsdom": "^16.2.6",
"@types/node-fetch": "^2.5.8",
"@types/supertest": "^2.0.10",
"@types/wtfnode": "^0.7.0",
"argon2": "^0.28.0",
"jest": "^26.6.3",
"jest-fetch-mock": "^3.0.3",
"jsdom": "^16.4.0",
"node-fetch": "^2.6.1",
"playwright": "^1.12.1",
"supertest": "^6.1.1",
"ts-jest": "^26.4.4",
"@types/wtfnode": "^0.7.0",
"wtfnode": "^0.9.0"
},
"resolutions": {
Expand Down
51 changes: 45 additions & 6 deletions test/unit/browser/pages/vscode.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/**
* @jest-environment jsdom
*/
import fetchMock from "jest-fetch-mock"
import { JSDOM } from "jsdom"
import {
getNlsConfiguration,
Expand All @@ -20,6 +21,11 @@ describe("vscode", () => {
// We use underscores to not confuse with global values
const { window: _window } = new JSDOM()
_document = _window.document
fetchMock.enableMocks()
})

afterEach(() => {
fetchMock.resetMocks()
})

it("should throw an error if no nlsConfigElement", () => {
Expand Down Expand Up @@ -60,7 +66,7 @@ describe("vscode", () => {

_document.body.removeChild(mockElement)
})
it("should return have loadBundle property if _resolvedLangaugePackCoreLocation", () => {
it("should return and have a loadBundle property if _resolvedLangaugePackCoreLocation", async () => {
const mockElement = _document.createElement("div")
const dataSettings = {
locale: "en",
Expand All @@ -76,6 +82,32 @@ describe("vscode", () => {
expect(nlsConfig._resolvedLanguagePackCoreLocation).not.toBe(undefined)
expect(nlsConfig.loadBundle).not.toBe(undefined)

const mockCallbackFn = jest.fn((_, bundle) => {
return bundle
})

fetchMock.mockOnce(JSON.stringify({ key: "hello world" }))
// Ensure that load bundle works as expected
// by mocking the fetch response and checking that the callback
// had the expected value
await nlsConfig.loadBundle("hello", "en", mockCallbackFn)
expect(mockCallbackFn).toHaveBeenCalledTimes(1)
expect(mockCallbackFn).toHaveBeenCalledWith(undefined, { key: "hello world" })

// Call it again to ensure it loads from the cache
// it should return the same value
await nlsConfig.loadBundle("hello", "en", mockCallbackFn)
expect(mockCallbackFn).toHaveBeenCalledTimes(2)
expect(mockCallbackFn).toHaveBeenCalledWith(undefined, { key: "hello world" })

fetchMock.mockReject(new Error("fake error message"))
const mockCallbackFn2 = jest.fn((error) => error)
// Call it for a different bundle and mock a failed fetch call
// to ensure we get the expected error
const error = await nlsConfig.loadBundle("goodbye", "es", mockCallbackFn2)
expect(error.message).toEqual("fake error message")

// Clean up
_document.body.removeChild(mockElement)
})
})
Expand All @@ -87,6 +119,13 @@ describe("vscode", () => {
const actual = createBundlePath(_resolvedLangaugePackCoreLocation, bundle)
expect(actual).toBe(expected)
})
it("should return the correct path (even if _resolvedLangaugePackCoreLocation is undefined)", () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

✅ This check was added after modifying the createBundlePath function signature.

const _resolvedLangaugePackCoreLocation = undefined
const bundle = "/bundle.js"
const expected = "/!bundle.js.nls.json"
const actual = createBundlePath(_resolvedLangaugePackCoreLocation, bundle)
expect(actual).toBe(expected)
})
})
describe("setBodyBackgroundToThemeBackgroundColor", () => {
let _document: Document
Expand Down Expand Up @@ -228,11 +267,6 @@ describe("vscode", () => {
},
recordStats: true,

// TODO@jsjoeio address trustedTypesPolicy part
// might need to look up types
// and find a way to test the function
// maybe extract function into function
// and test manually
trustedTypesPolicy: undefined,
"vs/nls": {
availableLanguages: {},
Expand Down Expand Up @@ -280,6 +314,11 @@ describe("vscode", () => {

expect(loader.trustedTypesPolicy).not.toBe(undefined)
expect(loader.trustedTypesPolicy.name).toBe("amdLoader")

// Check that we can actually create a script URL
// using the createScriptURL on the loader object
const scriptUrl = loader.trustedTypesPolicy.createScriptURL("http://localhost/foo.js")
expect(scriptUrl).toBe("http://localhost/foo.js")
})
})
describe("_createScriptURL", () => {
Expand Down
22 changes: 21 additions & 1 deletion test/yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1757,6 +1757,13 @@ [email protected], core-util-is@~1.0.0:
resolved "https://registry.yarnpkg.com/core-util-is/-/core-util-is-1.0.2.tgz#b5fd54220aa2bc5ab57aab7140c940754503c1a7"
integrity sha1-tf1UIgqivFq1eqtxQMlAdUUDwac=

cross-fetch@^3.0.4:
version "3.1.4"
resolved "https://registry.yarnpkg.com/cross-fetch/-/cross-fetch-3.1.4.tgz#9723f3a3a247bf8b89039f3a380a9244e8fa2f39"
integrity sha512-1eAtFWdIubi6T4XPy6ei9iUFoKpUkIF971QLN8lIvvvwueI65+Nw5haMNKUwfJxabqlIIDODJKGrQ66gxC0PbQ==
dependencies:
node-fetch "2.6.1"

cross-spawn@^6.0.0:
version "6.0.5"
resolved "https://registry.yarnpkg.com/cross-spawn/-/cross-spawn-6.0.5.tgz#4a5ec7c64dfae22c3a14124dbacdee846d80cbc4"
Expand Down Expand Up @@ -2800,6 +2807,14 @@ jest-environment-node@^26.6.2:
jest-mock "^26.6.2"
jest-util "^26.6.2"

jest-fetch-mock@^3.0.3:
version "3.0.3"
resolved "https://registry.yarnpkg.com/jest-fetch-mock/-/jest-fetch-mock-3.0.3.tgz#31749c456ae27b8919d69824f1c2bd85fe0a1f3b"
integrity sha512-Ux1nWprtLrdrH4XwE7O7InRY6psIi3GOsqNESJgMJ+M5cv4A8Lh7SN9d2V2kKRZ8ebAfcd1LNyZguAOb6JiDqw==
dependencies:
cross-fetch "^3.0.4"
promise-polyfill "^8.1.3"

jest-get-type@^26.3.0:
version "26.3.0"
resolved "https://registry.yarnpkg.com/jest-get-type/-/jest-get-type-26.3.0.tgz#e97dc3c3f53c2b406ca7afaed4493b1d099199e0"
Expand Down Expand Up @@ -3418,7 +3433,7 @@ node-addon-api@^3.0.2:
resolved "https://registry.yarnpkg.com/node-addon-api/-/node-addon-api-3.2.1.tgz#81325e0a2117789c0128dab65e7e38f07ceba161"
integrity sha512-mmcei9JghVNDYydghQmeDX8KoAm0FAiYyIcUt/N4nhyAipB17pllZQDOJD2fotxABnt4Mdz+dKTO7eftLg4d0A==

node-fetch@^2.6.1:
node-fetch@2.6.1, node-fetch@^2.6.1:
version "2.6.1"
resolved "https://registry.yarnpkg.com/node-fetch/-/node-fetch-2.6.1.tgz#045bd323631f76ed2e2b55573394416b639a0052"
integrity sha512-V4aYg89jEoVRxRb2fJdAg8FHvI7cEyYdVAh94HH0UIK8oJxUfkjlDQN9RbMx+bEjP7+ggMiFRprSti032Oipxw==
Expand Down Expand Up @@ -3762,6 +3777,11 @@ progress@^2.0.3:
resolved "https://registry.yarnpkg.com/progress/-/progress-2.0.3.tgz#7e8cf8d8f5b8f239c1bc68beb4eb78567d572ef8"
integrity sha512-7PiHtLll5LdnKIMw100I+8xJXR5gW2QwWYkT6iJva0bXitZKa/XMrSbdmg3r2Xnaidz9Qumd0VPaMrZlF9V9sA==

promise-polyfill@^8.1.3:
version "8.2.0"
resolved "https://registry.yarnpkg.com/promise-polyfill/-/promise-polyfill-8.2.0.tgz#367394726da7561457aba2133c9ceefbd6267da0"
integrity sha512-k/TC0mIcPVF6yHhUvwAp7cvL6I2fFV7TzF1DuGPI8mBh4QQazf36xCKEHKTZKRysEoTQoQdKyP25J8MPJp7j5g==

prompts@^2.0.1:
version "2.4.0"
resolved "https://registry.yarnpkg.com/prompts/-/prompts-2.4.0.tgz#4aa5de0723a231d1ee9121c40fdf663df73f61d7"
Expand Down