-
Notifications
You must be signed in to change notification settings - Fork 6k
feat: add more tests for browser/pages/vscode #3743
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -95,8 +95,64 @@ try { | |
console.error(error) | ||
} | ||
|
||
export function setBodyBackgroundToThemeBackgroundColor(document: Document, localStorage: Storage) { | ||
const errorMsgPrefix = "[vscode]" | ||
|
||
if (!document) { | ||
throw new Error(`${errorMsgPrefix} Could not set body background to theme background color. Document is undefined.`) | ||
} | ||
|
||
if (!localStorage) { | ||
throw new Error( | ||
`${errorMsgPrefix} Could not set body background to theme background color. localStorage is undefined.`, | ||
) | ||
} | ||
|
||
const colorThemeData = localStorage.getItem("colorThemeData") | ||
|
||
if (!colorThemeData) { | ||
throw new Error( | ||
`${errorMsgPrefix} Could not set body background to theme background color. Could not find colorThemeData in localStorage.`, | ||
) | ||
} | ||
|
||
let _colorThemeData | ||
try { | ||
// We wrap this JSON.parse logic in a try/catch | ||
// because it can throw if the JSON is invalid. | ||
// and instead of throwing a random error | ||
// we can throw our own error, which will be more helpful | ||
// to the end user. | ||
_colorThemeData = JSON.parse(colorThemeData) | ||
} catch { | ||
throw new Error( | ||
`${errorMsgPrefix} Could not set body background to theme background color. Could not parse colorThemeData from localStorage.`, | ||
) | ||
} | ||
|
||
const hasColorMapProperty = Object.prototype.hasOwnProperty.call(_colorThemeData, "colorMap") | ||
if (!hasColorMapProperty) { | ||
throw new Error( | ||
`${errorMsgPrefix} Could not set body background to theme background color. colorThemeData is missing colorMap.`, | ||
) | ||
} | ||
|
||
const editorBgColor = _colorThemeData.colorMap["editor.background"] | ||
|
||
if (!editorBgColor) { | ||
throw new Error( | ||
`${errorMsgPrefix} Could not set body background to theme background color. colorThemeData.colorMap["editor.background"] is undefined.`, | ||
) | ||
} | ||
|
||
document.body.style.background = editorBgColor | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here we assume that |
||
|
||
return null | ||
} | ||
|
||
try { | ||
document.body.style.background = JSON.parse(localStorage.getItem("colorThemeData")!).colorMap["editor.background"] | ||
setBodyBackgroundToThemeBackgroundColor(document, localStorage) | ||
} catch (error) { | ||
// Oh well. | ||
console.error("Something went wrong setting the body background to the theme background color.") | ||
console.error(error) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,11 @@ | |
* @jest-environment jsdom | ||
*/ | ||
import { JSDOM } from "jsdom" | ||
import { getNlsConfiguration, nlsConfigElementId } from "../../../src/browser/pages/vscode" | ||
import { | ||
getNlsConfiguration, | ||
nlsConfigElementId, | ||
setBodyBackgroundToThemeBackgroundColor, | ||
} from "../../../src/browser/pages/vscode" | ||
|
||
describe("vscode", () => { | ||
describe("getNlsConfiguration", () => { | ||
|
@@ -58,4 +62,117 @@ describe("vscode", () => { | |
document.body.removeChild(mockElement) | ||
}) | ||
}) | ||
describe("setBodyBackgroundToThemeBackgroundColor", () => { | ||
beforeEach(() => { | ||
// We need to set the url in the JSDOM constructor | ||
// to prevent this error "SecurityError: localStorage is not available for opaque origins" | ||
// See: https://github.com/jsdom/jsdom/issues/2304#issuecomment-622314949 | ||
const { window } = new JSDOM("", { url: "http://localhost" }) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess this is because Node.js doesn't have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Exactly! |
||
global.document = window.document | ||
global.localStorage = window.localStorage | ||
}) | ||
it("should return null", () => { | ||
const test = { | ||
colorMap: { | ||
[`editor.background`]: "#ff3270", | ||
}, | ||
} | ||
localStorage.setItem("colorThemeData", JSON.stringify(test)) | ||
|
||
expect(setBodyBackgroundToThemeBackgroundColor(document, localStorage)).toBeNull() | ||
|
||
localStorage.removeItem("colorThemeData") | ||
}) | ||
it("should throw an error if Document is undefined", () => { | ||
const errorMsgPrefix = "[vscode]" | ||
const errorMessage = `${errorMsgPrefix} Could not set body background to theme background color. Document is undefined.` | ||
|
||
expect(() => { | ||
// @ts-expect-error We need to test when document is undefined | ||
setBodyBackgroundToThemeBackgroundColor(undefined, localStorage) | ||
}).toThrowError(errorMessage) | ||
}) | ||
it("should throw an error if localStorage is undefined", () => { | ||
const errorMsgPrefix = "[vscode]" | ||
const errorMessage = `${errorMsgPrefix} Could not set body background to theme background color. localStorage is undefined.` | ||
|
||
expect(() => { | ||
// @ts-expect-error We need to test when localStorage is undefined | ||
setBodyBackgroundToThemeBackgroundColor(document, undefined) | ||
}).toThrowError(errorMessage) | ||
}) | ||
it("should throw an error if it can't find colorThemeData in localStorage", () => { | ||
const errorMsgPrefix = "[vscode]" | ||
const errorMessage = `${errorMsgPrefix} Could not set body background to theme background color. Could not find colorThemeData in localStorage.` | ||
|
||
expect(() => { | ||
setBodyBackgroundToThemeBackgroundColor(document, localStorage) | ||
}).toThrowError(errorMessage) | ||
}) | ||
it("should throw an error if there is an error parsing colorThemeData from localStorage", () => { | ||
const errorMsgPrefix = "[vscode]" | ||
const errorMessage = `${errorMsgPrefix} Could not set body background to theme background color. Could not parse colorThemeData from localStorage.` | ||
|
||
localStorage.setItem( | ||
"colorThemeData", | ||
'{"id":"vs-dark max-SS-Cyberpunk-themes-cyberpunk-umbra-color-theme-json","label":"Activate UMBRA protocol","settingsId":"Activate "errorForeground":"#ff3270","foreground":"#ffffff","sideBarTitle.foreground":"#bbbbbb"},"watch\\":::false}', | ||
) | ||
|
||
expect(() => { | ||
setBodyBackgroundToThemeBackgroundColor(document, localStorage) | ||
}).toThrowError(errorMessage) | ||
|
||
localStorage.removeItem("colorThemeData") | ||
}) | ||
it("should throw an error if there is no colorMap property", () => { | ||
const errorMsgPrefix = "[vscode]" | ||
const errorMessage = `${errorMsgPrefix} Could not set body background to theme background color. colorThemeData is missing colorMap.` | ||
|
||
const test = { | ||
id: "hey-joe", | ||
} | ||
localStorage.setItem("colorThemeData", JSON.stringify(test)) | ||
|
||
expect(() => { | ||
setBodyBackgroundToThemeBackgroundColor(document, localStorage) | ||
}).toThrowError(errorMessage) | ||
|
||
localStorage.removeItem("colorThemeData") | ||
}) | ||
it("should throw an error if there is no editor.background color", () => { | ||
const errorMsgPrefix = "[vscode]" | ||
const errorMessage = `${errorMsgPrefix} Could not set body background to theme background color. colorThemeData.colorMap["editor.background"] is undefined.` | ||
|
||
const test = { | ||
id: "hey-joe", | ||
colorMap: { | ||
editor: "#fff", | ||
}, | ||
} | ||
localStorage.setItem("colorThemeData", JSON.stringify(test)) | ||
|
||
expect(() => { | ||
setBodyBackgroundToThemeBackgroundColor(document, localStorage) | ||
}).toThrowError(errorMessage) | ||
|
||
localStorage.removeItem("colorThemeData") | ||
}) | ||
it("should set the body background to the editor background color", () => { | ||
const test = { | ||
colorMap: { | ||
[`editor.background`]: "#ff3270", | ||
}, | ||
} | ||
localStorage.setItem("colorThemeData", JSON.stringify(test)) | ||
|
||
setBodyBackgroundToThemeBackgroundColor(document, localStorage) | ||
|
||
// When the body.style.backgroundColor is set using hex | ||
// it is converted to rgb | ||
// which is why we use that in the assertion | ||
expect(document.body.style.backgroundColor).toBe("rgb(255, 50, 112)") | ||
|
||
localStorage.removeItem("colorThemeData") | ||
}) | ||
}) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At first, I didn't like the function signature — passing in
document
andlocalStorage
— but then I justified it because:document
orlocalStorage
is missingThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense to me, this also allows you to test this by mocking document/localStorage, and avoids global state