From 320f0350ac9c1a387de85ced9ae3727a2d4152b2 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Mon, 19 Dec 2022 09:29:52 -0700 Subject: [PATCH 01/31] chore: upgrade Code to 1.74.1 --- lib/vscode | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/vscode b/lib/vscode index 6261075646f0..1ad8d514439d 160000 --- a/lib/vscode +++ b/lib/vscode @@ -1 +1 @@ -Subproject commit 6261075646f055b99068d3688932416f2346dd3b +Subproject commit 1ad8d514439d5077d2b0b7ee64d2ce82a9308e5a From ec4177c99ddc28ba815b2eb0c74799f2464687a1 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Mon, 19 Dec 2022 09:49:28 -0700 Subject: [PATCH 02/31] chore: remove require in integration.diff I don't know what the impact of this is but in https://github.com/microsoft/vscode/commit/192c67db71e8c261f26e2f34c86a4791ae428b2f they removed the usage of `require` in `server.main.ts`. More details in PR: https://github.com/microsoft/vscode/pull/165831 --- patches/integration.diff | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/patches/integration.diff b/patches/integration.diff index dc585f4edf67..ae9c779b0c2c 100644 --- a/patches/integration.diff +++ b/patches/integration.diff @@ -38,7 +38,7 @@ Index: code-server/lib/vscode/src/vs/server/node/server.main.ts -const LOCAL_HISTORY_HOME = join(APP_SETTINGS_HOME, 'History'); -const MACHINE_SETTINGS_HOME = join(USER_DATA_PATH, 'Machine'); -args['user-data-dir'] = USER_DATA_PATH; --const APP_ROOT = dirname(FileAccess.asFileUri('', require).fsPath); +-const APP_ROOT = dirname(FileAccess.asFileUri('').fsPath); -const BUILTIN_EXTENSIONS_FOLDER_PATH = join(APP_ROOT, 'extensions'); -args['builtin-extensions-dir'] = BUILTIN_EXTENSIONS_FOLDER_PATH; -args['extensions-dir'] = args['extensions-dir'] || join(REMOTE_DATA_FOLDER, 'extensions'); @@ -58,7 +58,7 @@ Index: code-server/lib/vscode/src/vs/server/node/server.main.ts + const LOCAL_HISTORY_HOME = join(APP_SETTINGS_HOME, 'History'); + const MACHINE_SETTINGS_HOME = join(USER_DATA_PATH, 'Machine'); + args['user-data-dir'] = USER_DATA_PATH; -+ const APP_ROOT = dirname(FileAccess.asFileUri('', require).fsPath); ++ const APP_ROOT = dirname(FileAccess.asFileUri('').fsPath); + const BUILTIN_EXTENSIONS_FOLDER_PATH = args['builtin-extensions-dir'] || join(APP_ROOT, 'extensions'); + args['builtin-extensions-dir'] = BUILTIN_EXTENSIONS_FOLDER_PATH; + args['extensions-dir'] = args['extensions-dir'] || join(REMOTE_DATA_FOLDER, 'extensions'); @@ -107,7 +107,7 @@ Index: code-server/lib/vscode/src/vs/workbench/browser/parts/dialogs/dialogHandl =================================================================== --- code-server.orig/lib/vscode/src/vs/workbench/browser/parts/dialogs/dialogHandler.ts +++ code-server/lib/vscode/src/vs/workbench/browser/parts/dialogs/dialogHandler.ts -@@ -143,8 +143,11 @@ export class BrowserDialogHandler implem +@@ -145,8 +145,11 @@ export class BrowserDialogHandler implem async about(): Promise { const detailString = (useAgo: boolean): string => { @@ -184,7 +184,7 @@ Index: code-server/lib/vscode/src/vs/workbench/browser/web.main.ts import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry'; import { IProgressService } from 'vs/platform/progress/common/progress'; import { DelayedLogChannel } from 'vs/workbench/services/output/common/delayedLogChannel'; -@@ -117,6 +118,9 @@ export class BrowserMain extends Disposa +@@ -118,6 +119,9 @@ export class BrowserMain extends Disposa // Startup const instantiationService = workbench.startup(); From 8afa5c221df26a17c1b77a0d55e8742bcca6e035 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Mon, 19 Dec 2022 09:51:54 -0700 Subject: [PATCH 03/31] chore: update marketplace.diff --- patches/marketplace.diff | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patches/marketplace.diff b/patches/marketplace.diff index 72e652bbd1b5..0fb72b62facd 100644 --- a/patches/marketplace.diff +++ b/patches/marketplace.diff @@ -19,7 +19,7 @@ Index: code-server/lib/vscode/src/vs/platform/product/common/product.ts =================================================================== --- code-server.orig/lib/vscode/src/vs/platform/product/common/product.ts +++ code-server/lib/vscode/src/vs/platform/product/common/product.ts -@@ -53,6 +53,16 @@ else if (typeof require?.__$__nodeRequir +@@ -47,6 +47,16 @@ else if (globalThis._VSCODE_PRODUCT_JSON version: pkg.version }); } From be1a56011429a8a43ba15f6aada6c4e9428cbf47 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Mon, 19 Dec 2022 10:17:38 -0700 Subject: [PATCH 04/31] chore: update sha hash in webview.diff --- patches/webview.diff | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/patches/webview.diff b/patches/webview.diff index 24607004567f..ec638d05f9c0 100644 --- a/patches/webview.diff +++ b/patches/webview.diff @@ -41,7 +41,7 @@ Index: code-server/lib/vscode/src/vs/workbench/services/environment/browser/envi =================================================================== --- code-server.orig/lib/vscode/src/vs/workbench/services/environment/browser/environmentService.ts +++ code-server/lib/vscode/src/vs/workbench/services/environment/browser/environmentService.ts -@@ -210,7 +210,7 @@ export class BrowserWorkbenchEnvironment +@@ -207,7 +207,7 @@ export class BrowserWorkbenchEnvironment @memoize get webviewExternalEndpoint(): string { @@ -70,12 +70,12 @@ Index: code-server/lib/vscode/src/vs/workbench/contrib/webview/browser/pre/index -+ content="default-src 'none'; script-src 'sha256-/9/YQU12wvTeVXCsIGB4shLwdWrMceCpKojfkloNjPU=' 'self'; frame-src 'self'; style-src 'unsafe-inline';"> +- content="default-src 'none'; script-src 'sha256-6s2fEapj0jmA7ZDjzz23Uv4xLlM7KX3p9DYidJX7Zmk=' 'self'; frame-src 'self'; style-src 'unsafe-inline';"> ++ content="default-src 'none'; script-src 'sha256-6/HBKMr5Cr24xXtQ+U/BxvVfCvBLYE55u8Jq3j/nzcI=' 'self'; frame-src 'self'; style-src 'unsafe-inline';"> Date: Mon, 19 Dec 2022 10:23:15 -0700 Subject: [PATCH 05/31] chore: update disable-builtin-ext-update.diff If my logic is right, then this patch is now simplified thanks to this: https://github.com/microsoft/vscode/blob/1.74.1/src/vs/workbench/contrib/extensions/browser/extensionsWorkbenchService.ts#L1238 --- patches/disable-builtin-ext-update.diff | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/patches/disable-builtin-ext-update.diff b/patches/disable-builtin-ext-update.diff index 2b4cb2942c07..eea53da0d3e6 100644 --- a/patches/disable-builtin-ext-update.diff +++ b/patches/disable-builtin-ext-update.diff @@ -18,14 +18,3 @@ Index: code-server/lib/vscode/src/vs/workbench/contrib/extensions/browser/extens if (!this.local.preRelease && this.gallery.properties.isPreReleaseVersion) { return false; } -@@ -1237,6 +1241,10 @@ export class ExtensionsWorkbenchService - // Skip if check updates only for builtin extensions and current extension is not builtin. - continue; - } -+ if (installed.type !== ExtensionType.User) { -+ // Never update builtin extensions. -+ continue; -+ } - if (installed.isBuiltin && (!installed.local?.identifier.uuid || (!isWeb && this.productService.quality === 'stable'))) { - // Skip checking updates for a builtin extension if it does not has Marketplace identifier or the current product is VS Code Desktop stable. - continue; From 6acfb0bd95fc2b9ebbda78486808cea86fcc4fbc Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Mon, 19 Dec 2022 10:25:19 -0700 Subject: [PATCH 06/31] chore: refresh proxy-uri patch --- patches/proxy-uri.diff | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patches/proxy-uri.diff b/patches/proxy-uri.diff index 1ae84e63fe33..5ec0bbdc7bae 100644 --- a/patches/proxy-uri.diff +++ b/patches/proxy-uri.diff @@ -92,7 +92,7 @@ Index: code-server/lib/vscode/src/vs/workbench/browser/web.main.ts =================================================================== --- code-server.orig/lib/vscode/src/vs/workbench/browser/web.main.ts +++ code-server/lib/vscode/src/vs/workbench/browser/web.main.ts -@@ -248,7 +248,7 @@ export class BrowserMain extends Disposa +@@ -247,7 +247,7 @@ export class BrowserMain extends Disposa // Remote const connectionToken = environmentService.options.connectionToken || getCookieValue(connectionTokenCookieName); From bb2a29b1cf94a3f0c351d7bcd5711e809fa235e0 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Mon, 19 Dec 2022 10:40:19 -0700 Subject: [PATCH 07/31] chore: refresh local-storage.diff --- patches/local-storage.diff | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patches/local-storage.diff b/patches/local-storage.diff index 53df415805b7..343d140861f0 100644 --- a/patches/local-storage.diff +++ b/patches/local-storage.diff @@ -32,7 +32,7 @@ Index: code-server/lib/vscode/src/vs/workbench/browser/web.api.ts =================================================================== --- code-server.orig/lib/vscode/src/vs/workbench/browser/web.api.ts +++ code-server/lib/vscode/src/vs/workbench/browser/web.api.ts -@@ -266,6 +266,11 @@ export interface IWorkbenchConstructionO +@@ -259,6 +259,11 @@ export interface IWorkbenchConstructionO */ readonly configurationDefaults?: Record; From 4c00ad438af89fb82cbc73cd8b8754b8080c7db1 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Mon, 19 Dec 2022 10:40:45 -0700 Subject: [PATCH 08/31] chore: refresh sourcemaps.diff --- patches/sourcemaps.diff | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patches/sourcemaps.diff b/patches/sourcemaps.diff index a992ea55bd9d..a267d8dd7919 100644 --- a/patches/sourcemaps.diff +++ b/patches/sourcemaps.diff @@ -32,7 +32,7 @@ Index: code-server/lib/vscode/build/gulpfile.reh.js let version = packageJson.version; const quality = product.quality; -@@ -388,7 +387,7 @@ function tweakProductForServerWeb(produc +@@ -389,7 +388,7 @@ function tweakProductForServerWeb(produc const minifyTask = task.define(`minify-vscode-${type}`, task.series( optimizeTask, util.rimraf(`out-vscode-${type}-min`), From 661e1de13c63d11463988b6e707d870b40b2b862 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Mon, 19 Dec 2022 10:41:05 -0700 Subject: [PATCH 09/31] chore: refresh disable-downloads.diff --- patches/disable-downloads.diff | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patches/disable-downloads.diff b/patches/disable-downloads.diff index 450510340640..fd79bb5a8214 100644 --- a/patches/disable-downloads.diff +++ b/patches/disable-downloads.diff @@ -12,7 +12,7 @@ Index: code-server/lib/vscode/src/vs/workbench/browser/web.api.ts =================================================================== --- code-server.orig/lib/vscode/src/vs/workbench/browser/web.api.ts +++ code-server/lib/vscode/src/vs/workbench/browser/web.api.ts -@@ -271,6 +271,11 @@ export interface IWorkbenchConstructionO +@@ -264,6 +264,11 @@ export interface IWorkbenchConstructionO */ readonly userDataPath?: string From da4f4c3496b2e123d65b58f640a6e6bfead7ea74 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Mon, 19 Dec 2022 10:41:32 -0700 Subject: [PATCH 10/31] chore: refresh display-language.diff --- patches/display-language.diff | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/patches/display-language.diff b/patches/display-language.diff index 120c1674d261..916cc91f86f4 100644 --- a/patches/display-language.diff +++ b/patches/display-language.diff @@ -42,7 +42,7 @@ Index: code-server/lib/vscode/src/vs/base/common/platform.ts export const LANGUAGE_DEFAULT = 'en'; let _isWindows = false; -@@ -83,17 +81,19 @@ if (typeof navigator === 'object' && !is +@@ -86,17 +84,19 @@ if (typeof navigator === 'object' && !is _isMobile = _userAgent?.indexOf('Mobi') >= 0; _isWeb = true; @@ -125,7 +125,7 @@ Index: code-server/lib/vscode/src/vs/platform/environment/common/environmentServ =================================================================== --- code-server.orig/lib/vscode/src/vs/platform/environment/common/environmentService.ts +++ code-server/lib/vscode/src/vs/platform/environment/common/environmentService.ts -@@ -110,7 +110,7 @@ export abstract class AbstractNativeEnvi +@@ -107,7 +107,7 @@ export abstract class AbstractNativeEnvi return URI.file(join(vscodePortable, 'argv.json')); } From 63fb5343353db3b6cb4a15f1a4ebaa6028506500 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Mon, 19 Dec 2022 10:41:51 -0700 Subject: [PATCH 11/31] chore: refresh getting-started.diff --- patches/getting-started.diff | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/patches/getting-started.diff b/patches/getting-started.diff index 5115848c4bfd..20df51655111 100644 --- a/patches/getting-started.diff +++ b/patches/getting-started.diff @@ -10,7 +10,7 @@ Index: code-server/lib/vscode/src/vs/workbench/contrib/welcomeGettingStarted/bro =================================================================== --- code-server.orig/lib/vscode/src/vs/workbench/contrib/welcomeGettingStarted/browser/gettingStarted.ts +++ code-server/lib/vscode/src/vs/workbench/contrib/welcomeGettingStarted/browser/gettingStarted.ts -@@ -62,7 +62,7 @@ import { GettingStartedIndexList } from +@@ -59,7 +59,7 @@ import { GettingStartedIndexList } from import { StandardKeyboardEvent } from 'vs/base/browser/keyboardEvent'; import { KeyCode } from 'vs/base/common/keyCodes'; import { getTelemetryLevel } from 'vs/platform/telemetry/common/telemetryUtils'; @@ -19,7 +19,7 @@ Index: code-server/lib/vscode/src/vs/workbench/contrib/welcomeGettingStarted/bro import { OpenFolderViaWorkspaceAction } from 'vs/workbench/browser/actions/workspaceActions'; import { OpenRecentAction } from 'vs/workbench/browser/actions/windowActions'; import { Toggle } from 'vs/base/browser/ui/toggle/toggle'; -@@ -758,6 +758,72 @@ export class GettingStartedPage extends +@@ -756,6 +756,72 @@ export class GettingStartedPage extends $('p.subtitle.description', {}, localize({ key: 'gettingStarted.editingEvolved', comment: ['Shown as subtitle on the Welcome page.'] }, "Editing evolved")) ); @@ -92,7 +92,7 @@ Index: code-server/lib/vscode/src/vs/workbench/contrib/welcomeGettingStarted/bro const leftColumn = $('.categories-column.categories-column-left', {},); const rightColumn = $('.categories-column.categories-column-right', {},); -@@ -775,13 +841,23 @@ export class GettingStartedPage extends +@@ -773,13 +839,23 @@ export class GettingStartedPage extends const layoutLists = () => { if (gettingStartedList.itemCount) { this.container.classList.remove('noWalkthroughs'); @@ -143,7 +143,7 @@ Index: code-server/lib/vscode/src/vs/workbench/browser/web.api.ts =================================================================== --- code-server.orig/lib/vscode/src/vs/workbench/browser/web.api.ts +++ code-server/lib/vscode/src/vs/workbench/browser/web.api.ts -@@ -276,6 +276,11 @@ export interface IWorkbenchConstructionO +@@ -269,6 +269,11 @@ export interface IWorkbenchConstructionO */ readonly isEnabledFileDownloads?: boolean From 093bbc081e42829347b7121c8a8e41be9252b749 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Mon, 19 Dec 2022 11:31:51 -0700 Subject: [PATCH 12/31] docs: update testing notes for cli-window-open --- patches/cli-window-open.diff | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patches/cli-window-open.diff b/patches/cli-window-open.diff index 16d2ebc2b160..27c6192865be 100644 --- a/patches/cli-window-open.diff +++ b/patches/cli-window-open.diff @@ -8,7 +8,7 @@ To test: 2. Open code-server 3. Open terminal 4. Open another code-server window -5. Run code-server with a file or directory argument +5. Run node ./out/node/entry.js with a file or directory argument The file or directory should only open from the instance attached to that terminal. From b1b82057d12d241ac5b0ef586d29cf28ff7abd3d Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Mon, 19 Dec 2022 12:09:46 -0700 Subject: [PATCH 13/31] docs: update telemetry testing instructions --- patches/telemetry.diff | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patches/telemetry.diff b/patches/telemetry.diff index 8a8297a38ed8..e19cc4f40e5b 100644 --- a/patches/telemetry.diff +++ b/patches/telemetry.diff @@ -1,7 +1,7 @@ Add support for telemetry endpoint To test: -1. Create a RequestBin - https://requestbin.io/ +1. Create a mock API using [RequestBin](https://requestbin.io/) or [Beeceptor](https://beeceptor.com/) 2. Run code-server with `CS_TELEMETRY_URL` set: i.e. `CS_TELEMETRY_URL="https://requestbin.io/1ebub9z1" ./code-server--macos-amd64/bin/code-server` NOTE: it has to be a production build. From 0aecf9e108f8d6387f6cb71ccce9b55c47a03fdf Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Mon, 19 Dec 2022 13:34:58 -0700 Subject: [PATCH 14/31] fix: add GITHUB_TOKEN to build code-server job Downloading @vscode/ripgrep is failing only in CI so adding this environment variable to see if it increases the rate limit. Ref: https://github.com/microsoft/vscode-ripgrep#github-api-limit-note --- .github/workflows/build.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index ff12ff2f54c7..1a58e2b7be42 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -179,6 +179,8 @@ jobs: run: yarn --frozen-lockfile - name: Build code-server + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: yarn build # Get Code's git hash. When this changes it means the content is From d00ab199f485874053381992ebb0c22c17b372f6 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Mon, 19 Dec 2022 14:35:25 -0700 Subject: [PATCH 15/31] refactor: use own cache key build code-server job --- .github/workflows/build.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 1a58e2b7be42..2eacba567b3b 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -170,9 +170,9 @@ jobs: uses: actions/cache@v3 with: path: "**/node_modules" - key: yarn-build-${{ hashFiles('**/yarn.lock') }} + key: yarn-build-code-server-${{ hashFiles('**/yarn.lock') }} restore-keys: | - yarn-build- + yarn-build-code-server- - name: Install dependencies if: steps.cache-node-modules.outputs.cache-hit != 'true' From 4deb15673bb1c75eada4af54fab5ecaacccf2281 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Tue, 20 Dec 2022 08:55:11 -0700 Subject: [PATCH 16/31] temp: disable vscode test --- test/unit/node/routes/vscode.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/node/routes/vscode.test.ts b/test/unit/node/routes/vscode.test.ts index 105d732ebd28..5d3dbb63d944 100644 --- a/test/unit/node/routes/vscode.test.ts +++ b/test/unit/node/routes/vscode.test.ts @@ -6,7 +6,7 @@ import * as integration from "../../../utils/integration" // TODO@jsjoeio - move these to integration tests since they rely on Code // to be built -describe("vscode", () => { +describe.skip("vscode", () => { let codeServer: httpserver.HttpServer | undefined const testName = "vscode" From 3999279b73c3519c7dbb03dfc7076bf26f717e13 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Tue, 20 Dec 2022 10:55:12 -0700 Subject: [PATCH 17/31] refactor: delete wrapper test --- test/unit/node/routes/vscode.test.ts | 2 +- test/unit/node/wrapper.test.ts | 14 -------------- 2 files changed, 1 insertion(+), 15 deletions(-) delete mode 100644 test/unit/node/wrapper.test.ts diff --git a/test/unit/node/routes/vscode.test.ts b/test/unit/node/routes/vscode.test.ts index 5d3dbb63d944..105d732ebd28 100644 --- a/test/unit/node/routes/vscode.test.ts +++ b/test/unit/node/routes/vscode.test.ts @@ -6,7 +6,7 @@ import * as integration from "../../../utils/integration" // TODO@jsjoeio - move these to integration tests since they rely on Code // to be built -describe.skip("vscode", () => { +describe("vscode", () => { let codeServer: httpserver.HttpServer | undefined const testName = "vscode" diff --git a/test/unit/node/wrapper.test.ts b/test/unit/node/wrapper.test.ts deleted file mode 100644 index 4aa14bda5536..000000000000 --- a/test/unit/node/wrapper.test.ts +++ /dev/null @@ -1,14 +0,0 @@ -import { ChildProcess, ParentProcess, isChild } from "../../../src/node/wrapper" - -describe("wrapper", () => { - describe("isChild", () => { - it("should return false for parent process", () => { - const p = new ParentProcess("1") - expect(isChild(p)).toBe(false) - }) - }) - it("should return false for parent process", () => { - const childProc = new ChildProcess(1) - expect(isChild(childProc)).toBe(true) - }) -}) From fc54c4451c693c3b927945233901f88cf90250a0 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Tue, 20 Dec 2022 13:53:17 -0700 Subject: [PATCH 18/31] Revert "refactor: delete wrapper test" This reverts commit 3999279b73c3519c7dbb03dfc7076bf26f717e13. --- test/unit/node/routes/vscode.test.ts | 2 +- test/unit/node/wrapper.test.ts | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 test/unit/node/wrapper.test.ts diff --git a/test/unit/node/routes/vscode.test.ts b/test/unit/node/routes/vscode.test.ts index 105d732ebd28..5d3dbb63d944 100644 --- a/test/unit/node/routes/vscode.test.ts +++ b/test/unit/node/routes/vscode.test.ts @@ -6,7 +6,7 @@ import * as integration from "../../../utils/integration" // TODO@jsjoeio - move these to integration tests since they rely on Code // to be built -describe("vscode", () => { +describe.skip("vscode", () => { let codeServer: httpserver.HttpServer | undefined const testName = "vscode" diff --git a/test/unit/node/wrapper.test.ts b/test/unit/node/wrapper.test.ts new file mode 100644 index 000000000000..4aa14bda5536 --- /dev/null +++ b/test/unit/node/wrapper.test.ts @@ -0,0 +1,14 @@ +import { ChildProcess, ParentProcess, isChild } from "../../../src/node/wrapper" + +describe("wrapper", () => { + describe("isChild", () => { + it("should return false for parent process", () => { + const p = new ParentProcess("1") + expect(isChild(p)).toBe(false) + }) + }) + it("should return false for parent process", () => { + const childProc = new ChildProcess(1) + expect(isChild(childProc)).toBe(true) + }) +}) From b3f7a209043e01ef469e1cb69e7b8d9024b36ddf Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Wed, 21 Dec 2022 12:04:59 -0700 Subject: [PATCH 19/31] refactor: move vscode tests to e2e (#5911) * wip: migrate vscode tests to e2e * feat: add codeWorkspace to global setup * refactor: only use dir in spawn when we should * wip: migrate more tests * refactor: move all vscode tests to e2e * refactor(ci): move unit to own job * fixup: add codecov to unit test step * Update test/e2e/models/CodeServer.ts * Update test/e2e/models/CodeServer.ts * docs: add note about intercept requests * refactor: rm unused clean() calls * refactor: delete duplicate test * refactor: update 'should not redirect' test --- .github/workflows/build.yaml | 62 +++++++++--- ci/dev/test-unit.sh | 16 ---- test/e2e/models/CodeServer.ts | 4 +- test/e2e/routes.test.ts | 118 +++++++++++++++++++++++ test/unit/node/routes/vscode.test.ts | 137 --------------------------- test/utils/globalE2eSetup.ts | 13 ++- 6 files changed, 181 insertions(+), 169 deletions(-) create mode 100644 test/e2e/routes.test.ts delete mode 100644 test/unit/node/routes/vscode.test.ts diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 2eacba567b3b..101b7577f6cb 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -139,6 +139,55 @@ jobs: if: steps.changed-files.outputs.any_changed == 'true' run: yarn lint:ts + test-unit: + name: Run unit tests + runs-on: ubuntu-20.04 + timeout-minutes: 5 + steps: + - name: Checkout repo + uses: actions/checkout@v3 + with: + fetch-depth: 2 + + - name: Get changed files + id: changed-files + uses: tj-actions/changed-files@v26.1 + with: + files: | + **/*.ts + files_ignore: | + lib/vscode/** + + - name: Install Node.js v16 + if: steps.changed-files.outputs.any_changed == 'true' + uses: actions/setup-node@v3 + with: + node-version: "16" + + - name: Fetch dependencies from cache + if: steps.changed-files.outputs.any_changed == 'true' + id: cache-node-modules + uses: actions/cache@v3 + with: + path: "**/node_modules" + key: yarn-build-${{ hashFiles('**/yarn.lock') }} + restore-keys: | + yarn-build- + + - name: Install dependencies + if: steps.changed-files.outputs.any_changed == 'true' && steps.cache-node-modules.outputs.cache-hit != 'true' + run: SKIP_SUBMODULE_DEPS=1 yarn --frozen-lockfile + + - name: Run unit tests + if: steps.changed-files.outputs.any_changed == 'true' + run: yarn test:unit + + - name: Upload coverage report to Codecov + uses: codecov/codecov-action@v3 + with: + token: ${{ secrets.CODECOV_TOKEN }} + if: success() + build: name: Build code-server runs-on: ubuntu-20.04 @@ -206,19 +255,6 @@ jobs: if: steps.cache-vscode.outputs.cache-hit != 'true' run: yarn build:vscode - # Our code imports code from VS Code's `out` directory meaning VS Code - # must be built before running these tests. - # TODO: Move to its own step? - - name: Run code-server unit tests - run: yarn test:unit - if: success() - - - name: Upload coverage report to Codecov - uses: codecov/codecov-action@v3 - with: - token: ${{ secrets.CODECOV_TOKEN }} - if: success() - # The release package does not contain any native modules # and is neutral to architecture/os/libc version. - name: Create release package diff --git a/ci/dev/test-unit.sh b/ci/dev/test-unit.sh index b3e0b14c908c..e312c073e4ef 100755 --- a/ci/dev/test-unit.sh +++ b/ci/dev/test-unit.sh @@ -11,22 +11,6 @@ main() { make -s out/index.js popd - # Our code imports from `out` in order to work during development but if you - # have only built for production you will have not have this directory. In - # that case symlink `out` to a production build directory. - if [[ ! -e lib/vscode/out ]]; then - pushd lib - local out=(vscode-reh-web-*) - if [[ -d "${out[0]}" ]]; then - ln -s "../${out[0]}/out" ./vscode/out - else - echo "Could not find lib/vscode/out or lib/vscode-reh-web-*" - echo "Code must be built before running unit tests" - exit 1 - fi - popd - fi - # We must keep jest in a sub-directory. See ../../test/package.json for more # information. We must also run it from the root otherwise coverage will not # include our source files. diff --git a/test/e2e/models/CodeServer.ts b/test/e2e/models/CodeServer.ts index f2c2e0cc431d..2fb625627451 100644 --- a/test/e2e/models/CodeServer.ts +++ b/test/e2e/models/CodeServer.ts @@ -128,6 +128,8 @@ export class CodeServer { path.join(dir, "extensions"), "--auth", "none", + // The workspace to open. + ...(this.args.includes("--ignore-last-opened") ? [] : [dir]), ...this.args, // Using port zero will spawn on a random port. "--bind-addr", @@ -139,8 +141,6 @@ export class CodeServer { path.join(dir, "config.yaml"), "--user-data-dir", dir, - // The last argument is the workspace to open. - dir, ] this.logger.debug("spawning `node " + args.join(" ") + "`") const proc = cp.spawn("node", args, { diff --git a/test/e2e/routes.test.ts b/test/e2e/routes.test.ts new file mode 100644 index 000000000000..70e2632ba622 --- /dev/null +++ b/test/e2e/routes.test.ts @@ -0,0 +1,118 @@ +import { describe, test, expect } from "./baseFixture" +import { clean, tmpdir } from "../utils/helpers" +import * as path from "path" +import { promises as fs } from "fs" + +const routes = ["/", "/vscode", "/vscode/"] + +describe("VS Code Routes", ["--disable-workspace-trust"], {}, async () => { + const testName = "vscode-routes-default" + test.beforeAll(async () => { + await clean(testName) + }) + + test("should load all route variations", async ({ codeServerPage }) => { + for (const route of routes) { + await codeServerPage.navigate(route) + + // Check there were no redirections + const url = new URL(codeServerPage.page.url()) + expect(url.pathname).toBe(route) + + // TODO@jsjoeio + // now that we are in a proper browser instead of scraping the HTML we + // could possibly intercept requests to make sure assets are loading from + // the right spot. + // + // Check that page loaded from correct route + const html = await codeServerPage.page.innerHTML("html") + switch (route) { + case "/": + case "/vscode/": + expect(html).toMatch(/src="\.\/[a-z]+-[0-9a-z]+\/static\//) + break + case "/vscode": + expect(html).toMatch(/src="\.\/vscode\/[a-z]+-[0-9a-z]+\/static\//) + break + } + } + }) +}) + +const CODE_WORKSPACE_DIR = process.env.CODE_WORKSPACE_DIR || "" +describe("VS Code Routes with code-workspace", ["--disable-workspace-trust", CODE_WORKSPACE_DIR], {}, async () => { + test("should redirect to the passed in workspace using human-readable query", async ({ codeServerPage }) => { + const url = new URL(codeServerPage.page.url()) + expect(url.pathname).toBe("/") + expect(url.search).toBe(`?workspace=${CODE_WORKSPACE_DIR}`) + }) +}) + +const CODE_FOLDER_DIR = process.env.CODE_FOLDER_DIR || "" +describe("VS Code Routes with code-workspace", ["--disable-workspace-trust", CODE_FOLDER_DIR], {}, async () => { + test("should redirect to the passed in folder using human-readable query", async ({ codeServerPage }) => { + const url = new URL(codeServerPage.page.url()) + expect(url.pathname).toBe("/") + expect(url.search).toBe(`?folder=${CODE_FOLDER_DIR}`) + }) +}) + +describe( + "VS Code Routes with ignore-last-opened", + ["--disable-workspace-trust", "--ignore-last-opened"], + {}, + async () => { + test("should not redirect", async ({ codeServerPage }) => { + const folder = process.env.CODE_FOLDER_DIR + + await codeServerPage.navigate(`/`) + await codeServerPage.navigate(`/?folder=${folder}`) + await codeServerPage.navigate(`/`) + + const url = new URL(codeServerPage.page.url()) + expect(url.pathname).toBe("/") + expect(url.search).toBe("") + }) + }, +) + +describe( + "VS Code Routes with no workspace or folder", + ["--disable-workspace-trust"], + {}, + async () => { + test("should redirect to last query folder/workspace", async ({ codeServerPage }) => { + const folder = process.env.CODE_FOLDER_DIR + const workspace = process.env.CODE_WORKSPACE_DIR + await codeServerPage.navigate(`/?folder=${folder}&workspace=${workspace}`) + + // If you visit again without query parameters it will re-attach them by + // redirecting. It should always redirect to the same route. + for (const route of routes) { + await codeServerPage.navigate(route) + const url = new URL(codeServerPage.page.url()) + expect(url.pathname).toBe(route) + expect(url.search).toBe(`?folder=${folder}&workspace=${workspace}`) + } + }) + }, +) + +describe( + "VS Code Routes with no workspace or folder", + ["--disable-workspace-trust"], + {}, + async () => { + test("should not redirect if ew passed in", async ({ codeServerPage }) => { + const folder = process.env.CODE_FOLDER_DIR + const workspace = process.env.CODE_WORKSPACE_DIR + await codeServerPage.navigate(`/?folder=${folder}&workspace=${workspace}`) + + // Closing the folder should stop the redirecting. + await codeServerPage.navigate("/?ew=true") + let url = new URL(codeServerPage.page.url()) + expect(url.pathname).toBe("/") + expect(url.search).toBe("?ew=true") + }) + }, +) \ No newline at end of file diff --git a/test/unit/node/routes/vscode.test.ts b/test/unit/node/routes/vscode.test.ts deleted file mode 100644 index 5d3dbb63d944..000000000000 --- a/test/unit/node/routes/vscode.test.ts +++ /dev/null @@ -1,137 +0,0 @@ -import { promises as fs } from "fs" -import * as path from "path" -import { clean, tmpdir } from "../../../utils/helpers" -import * as httpserver from "../../../utils/httpserver" -import * as integration from "../../../utils/integration" - -// TODO@jsjoeio - move these to integration tests since they rely on Code -// to be built -describe.skip("vscode", () => { - let codeServer: httpserver.HttpServer | undefined - - const testName = "vscode" - beforeAll(async () => { - await clean(testName) - }) - - afterEach(async () => { - if (codeServer) { - await codeServer.dispose() - codeServer = undefined - } - }) - - const routes = ["/", "/vscode", "/vscode/"] - - it("should load all route variations", async () => { - codeServer = await integration.setup(["--auth=none"], "") - - for (const route of routes) { - const resp = await codeServer.fetch(route) - expect(resp.status).toBe(200) - const html = await resp.text() - const url = new URL(resp.url) // Check there were no redirections. - expect(url.pathname + url.search).toBe(route) - switch (route) { - case "/": - case "/vscode/": - expect(html).toMatch(/src="\.\/[a-z]+-[0-9a-z]+\/static\//) - break - case "/vscode": - expect(html).toMatch(/src="\.\/vscode\/[a-z]+-[0-9a-z]+\/static\//) - break - } - } - }) - - it("should redirect to the passed in workspace using human-readable query", async () => { - const workspace = path.join(await tmpdir(testName), "test.code-workspace") - await fs.writeFile(workspace, "") - codeServer = await integration.setup(["--auth=none", workspace], "") - - const resp = await codeServer.fetch("/") - const url = new URL(resp.url) - expect(url.pathname).toBe("/") - expect(url.search).toBe(`?workspace=${workspace}`) - }) - - it("should redirect to the passed in folder using human-readable query", async () => { - const folder = await tmpdir(testName) - codeServer = await integration.setup(["--auth=none", folder], "") - - const resp = await codeServer.fetch("/") - const url = new URL(resp.url) - expect(url.pathname).toBe("/") - expect(url.search).toBe(`?folder=${folder}`) - }) - - it("should redirect to last query folder/workspace", async () => { - codeServer = await integration.setup(["--auth=none"], "") - - const folder = await tmpdir(testName) - const workspace = path.join(await tmpdir(testName), "test.code-workspace") - await fs.writeFile(workspace, "") - let resp = await codeServer.fetch("/", undefined, { - folder, - workspace, - }) - expect(resp.status).toBe(200) - await resp.text() - - // If you visit again without query parameters it will re-attach them by - // redirecting. It should always redirect to the same route. - for (const route of routes) { - resp = await codeServer.fetch(route) - const url = new URL(resp.url) - expect(url.pathname).toBe(route) - expect(url.search).toBe(`?folder=${folder}&workspace=${workspace}`) - await resp.text() - } - - // Closing the folder should stop the redirecting. - resp = await codeServer.fetch("/", undefined, { ew: "true" }) - let url = new URL(resp.url) - expect(url.pathname).toBe("/") - expect(url.search).toBe("?ew=true") - await resp.text() - - resp = await codeServer.fetch("/") - url = new URL(resp.url) - expect(url.pathname).toBe("/") - expect(url.search).toBe("") - await resp.text() - }) - - it("should do nothing when nothing is passed in", async () => { - codeServer = await integration.setup(["--auth=none"], "") - - const resp = await codeServer.fetch("/", undefined) - - expect(resp.status).toBe(200) - const url = new URL(resp.url) - expect(url.search).toBe("") - await resp.text() - }) - - it("should not redirect when last opened is ignored", async () => { - codeServer = await integration.setup(["--auth=none", "--ignore-last-opened"], "") - - const folder = await tmpdir(testName) - const workspace = path.join(await tmpdir(testName), "test.code-workspace") - await fs.writeFile(workspace, "") - - let resp = await codeServer.fetch("/", undefined, { - folder, - workspace, - }) - expect(resp.status).toBe(200) - await resp.text() - - // No redirections. - resp = await codeServer.fetch("/") - const url = new URL(resp.url) - expect(url.pathname).toBe("/") - expect(url.search).toBe("") - await resp.text() - }) -}) diff --git a/test/utils/globalE2eSetup.ts b/test/utils/globalE2eSetup.ts index 3596ed33f34e..107dc85fa0d7 100644 --- a/test/utils/globalE2eSetup.ts +++ b/test/utils/globalE2eSetup.ts @@ -1,6 +1,8 @@ import { workspaceDir } from "./constants" -import { clean } from "./helpers" +import { clean, tmpdir } from "./helpers" import * as wtfnode from "./wtfnode" +import * as path from "path" +import { promises as fs } from "fs" /** * Perform workspace cleanup and authenticate. This should be ran before e2e @@ -17,5 +19,14 @@ export default async function () { wtfnode.setup() } + // Create dummy code-workspace for routes.test.ts + const codeWorkspace = path.join(await tmpdir(workspaceDir), "test.code-workspace") + await fs.writeFile(codeWorkspace, "") + process.env.CODE_WORKSPACE_DIR = codeWorkspace + + // Create dummy folder for routes.test.ts + const folder = await tmpdir(workspaceDir) + process.env.CODE_FOLDER_DIR = folder + console.log("✅ Global Setup for Playwright End-to-End Tests is now complete.") } From 10ebf1bcd9950df6c1e96f7528bf89e654fdbccc Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Wed, 21 Dec 2022 12:10:29 -0700 Subject: [PATCH 20/31] refactor: rm unused imports --- test/e2e/routes.test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/e2e/routes.test.ts b/test/e2e/routes.test.ts index 70e2632ba622..6931af878393 100644 --- a/test/e2e/routes.test.ts +++ b/test/e2e/routes.test.ts @@ -1,7 +1,5 @@ import { describe, test, expect } from "./baseFixture" -import { clean, tmpdir } from "../utils/helpers" -import * as path from "path" -import { promises as fs } from "fs" +import { clean } from "../utils/helpers" const routes = ["/", "/vscode", "/vscode/"] From 0fabb3a81da866441837a6215b22d0d8454461e4 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Wed, 21 Dec 2022 12:11:23 -0700 Subject: [PATCH 21/31] refactor: rm unnecessary navigate call in test --- test/e2e/routes.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/e2e/routes.test.ts b/test/e2e/routes.test.ts index 6931af878393..c15b9f71206d 100644 --- a/test/e2e/routes.test.ts +++ b/test/e2e/routes.test.ts @@ -63,7 +63,6 @@ describe( test("should not redirect", async ({ codeServerPage }) => { const folder = process.env.CODE_FOLDER_DIR - await codeServerPage.navigate(`/`) await codeServerPage.navigate(`/?folder=${folder}`) await codeServerPage.navigate(`/`) From cbf00ae7aa11162c0ac4d4c3f687cdcb461ceaa8 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Wed, 21 Dec 2022 12:18:38 -0700 Subject: [PATCH 22/31] fixup: formatting --- test/e2e/routes.test.ts | 66 +++++++++++++++++------------------------ 1 file changed, 28 insertions(+), 38 deletions(-) diff --git a/test/e2e/routes.test.ts b/test/e2e/routes.test.ts index c15b9f71206d..b93618bc2c9d 100644 --- a/test/e2e/routes.test.ts +++ b/test/e2e/routes.test.ts @@ -17,7 +17,7 @@ describe("VS Code Routes", ["--disable-workspace-trust"], {}, async () => { const url = new URL(codeServerPage.page.url()) expect(url.pathname).toBe(route) - // TODO@jsjoeio + // TODO@jsjoeio // now that we are in a proper browser instead of scraping the HTML we // could possibly intercept requests to make sure assets are loading from // the right spot. @@ -73,43 +73,33 @@ describe( }, ) -describe( - "VS Code Routes with no workspace or folder", - ["--disable-workspace-trust"], - {}, - async () => { - test("should redirect to last query folder/workspace", async ({ codeServerPage }) => { - const folder = process.env.CODE_FOLDER_DIR - const workspace = process.env.CODE_WORKSPACE_DIR - await codeServerPage.navigate(`/?folder=${folder}&workspace=${workspace}`) +describe("VS Code Routes with no workspace or folder", ["--disable-workspace-trust"], {}, async () => { + test("should redirect to last query folder/workspace", async ({ codeServerPage }) => { + const folder = process.env.CODE_FOLDER_DIR + const workspace = process.env.CODE_WORKSPACE_DIR + await codeServerPage.navigate(`/?folder=${folder}&workspace=${workspace}`) - // If you visit again without query parameters it will re-attach them by - // redirecting. It should always redirect to the same route. - for (const route of routes) { - await codeServerPage.navigate(route) - const url = new URL(codeServerPage.page.url()) - expect(url.pathname).toBe(route) - expect(url.search).toBe(`?folder=${folder}&workspace=${workspace}`) - } - }) - }, -) + // If you visit again without query parameters it will re-attach them by + // redirecting. It should always redirect to the same route. + for (const route of routes) { + await codeServerPage.navigate(route) + const url = new URL(codeServerPage.page.url()) + expect(url.pathname).toBe(route) + expect(url.search).toBe(`?folder=${folder}&workspace=${workspace}`) + } + }) +}) -describe( - "VS Code Routes with no workspace or folder", - ["--disable-workspace-trust"], - {}, - async () => { - test("should not redirect if ew passed in", async ({ codeServerPage }) => { - const folder = process.env.CODE_FOLDER_DIR - const workspace = process.env.CODE_WORKSPACE_DIR - await codeServerPage.navigate(`/?folder=${folder}&workspace=${workspace}`) +describe("VS Code Routes with no workspace or folder", ["--disable-workspace-trust"], {}, async () => { + test("should not redirect if ew passed in", async ({ codeServerPage }) => { + const folder = process.env.CODE_FOLDER_DIR + const workspace = process.env.CODE_WORKSPACE_DIR + await codeServerPage.navigate(`/?folder=${folder}&workspace=${workspace}`) - // Closing the folder should stop the redirecting. - await codeServerPage.navigate("/?ew=true") - let url = new URL(codeServerPage.page.url()) - expect(url.pathname).toBe("/") - expect(url.search).toBe("?ew=true") - }) - }, -) \ No newline at end of file + // Closing the folder should stop the redirecting. + await codeServerPage.navigate("/?ew=true") + let url = new URL(codeServerPage.page.url()) + expect(url.pathname).toBe("/") + expect(url.search).toBe("?ew=true") + }) +}) From e8ef8bf8efa22add5ef7eda086a17e464a60ca1b Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Wed, 21 Dec 2022 12:53:36 -0700 Subject: [PATCH 23/31] wip: update test --- test/e2e/routes.test.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/test/e2e/routes.test.ts b/test/e2e/routes.test.ts index b93618bc2c9d..056b2d3b2bd4 100644 --- a/test/e2e/routes.test.ts +++ b/test/e2e/routes.test.ts @@ -15,7 +15,17 @@ describe("VS Code Routes", ["--disable-workspace-trust"], {}, async () => { // Check there were no redirections const url = new URL(codeServerPage.page.url()) - expect(url.pathname).toBe(route) + let expected = route + if (process.env.USE_PROXY === "1") { + // TODO@jsjoeio if running behind proxy + // we need to modify expected value + // instead of / it should be //ide + expected = "something else " + // could also modify left side and stripe //ide... + // in url.pathname + + } + expect(url.pathname).toBe(expected) // TODO@jsjoeio // now that we are in a proper browser instead of scraping the HTML we From b6928faf449186a15a192da55a7a894c62796d8d Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Wed, 21 Dec 2022 14:11:33 -0700 Subject: [PATCH 24/31] refactor: modify assertion for proxy --- test/e2e/routes.test.ts | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/test/e2e/routes.test.ts b/test/e2e/routes.test.ts index 056b2d3b2bd4..ac2829d34147 100644 --- a/test/e2e/routes.test.ts +++ b/test/e2e/routes.test.ts @@ -15,17 +15,13 @@ describe("VS Code Routes", ["--disable-workspace-trust"], {}, async () => { // Check there were no redirections const url = new URL(codeServerPage.page.url()) - let expected = route if (process.env.USE_PROXY === "1") { - // TODO@jsjoeio if running behind proxy - // we need to modify expected value - // instead of / it should be //ide - expected = "something else " - // could also modify left side and stripe //ide... - // in url.pathname - + // Behind proxy, path will be / Date: Wed, 21 Dec 2022 14:19:53 -0700 Subject: [PATCH 25/31] fixup: use REVERSE_PROXY_BASE_PATH --- test/e2e/routes.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/e2e/routes.test.ts b/test/e2e/routes.test.ts index ac2829d34147..6412704941bf 100644 --- a/test/e2e/routes.test.ts +++ b/test/e2e/routes.test.ts @@ -1,5 +1,6 @@ import { describe, test, expect } from "./baseFixture" import { clean } from "../utils/helpers" +import { REVERSE_PROXY_BASE_PATH } from "../utils/constants" const routes = ["/", "/vscode", "/vscode/"] @@ -17,7 +18,7 @@ describe("VS Code Routes", ["--disable-workspace-trust"], {}, async () => { const url = new URL(codeServerPage.page.url()) if (process.env.USE_PROXY === "1") { // Behind proxy, path will be / Date: Wed, 21 Dec 2022 15:13:59 -0700 Subject: [PATCH 26/31] refactor: add helper fn getMaybeProxiedPathname --- test/e2e/routes.test.ts | 26 +++++++++++++------------- test/unit/helpers.test.ts | 22 +++++++++++++++++++++- test/utils/helpers.ts | 14 ++++++++++++++ 3 files changed, 48 insertions(+), 14 deletions(-) diff --git a/test/e2e/routes.test.ts b/test/e2e/routes.test.ts index 6412704941bf..b79f5ba16605 100644 --- a/test/e2e/routes.test.ts +++ b/test/e2e/routes.test.ts @@ -1,5 +1,5 @@ import { describe, test, expect } from "./baseFixture" -import { clean } from "../utils/helpers" +import { clean, getMaybeProxiedPathname } from "../utils/helpers" import { REVERSE_PROXY_BASE_PATH } from "../utils/constants" const routes = ["/", "/vscode", "/vscode/"] @@ -16,13 +16,8 @@ describe("VS Code Routes", ["--disable-workspace-trust"], {}, async () => { // Check there were no redirections const url = new URL(codeServerPage.page.url()) - if (process.env.USE_PROXY === "1") { - // Behind proxy, path will be / { test("should redirect to the passed in workspace using human-readable query", async ({ codeServerPage }) => { const url = new URL(codeServerPage.page.url()) - expect(url.pathname).toBe("/") + const pathname = getMaybeProxiedPathname(url) + expect(pathname).toBe("/") expect(url.search).toBe(`?workspace=${CODE_WORKSPACE_DIR}`) }) }) @@ -57,7 +53,8 @@ const CODE_FOLDER_DIR = process.env.CODE_FOLDER_DIR || "" describe("VS Code Routes with code-workspace", ["--disable-workspace-trust", CODE_FOLDER_DIR], {}, async () => { test("should redirect to the passed in folder using human-readable query", async ({ codeServerPage }) => { const url = new URL(codeServerPage.page.url()) - expect(url.pathname).toBe("/") + const pathname = getMaybeProxiedPathname(url) + expect(pathname).toBe("/") expect(url.search).toBe(`?folder=${CODE_FOLDER_DIR}`) }) }) @@ -74,7 +71,8 @@ describe( await codeServerPage.navigate(`/`) const url = new URL(codeServerPage.page.url()) - expect(url.pathname).toBe("/") + const pathname = getMaybeProxiedPathname(url) + expect(pathname).toBe("/") expect(url.search).toBe("") }) }, @@ -91,7 +89,8 @@ describe("VS Code Routes with no workspace or folder", ["--disable-workspace-tru for (const route of routes) { await codeServerPage.navigate(route) const url = new URL(codeServerPage.page.url()) - expect(url.pathname).toBe(route) + const pathname = getMaybeProxiedPathname(url) + expect(pathname).toBe(route) expect(url.search).toBe(`?folder=${folder}&workspace=${workspace}`) } }) @@ -106,7 +105,8 @@ describe("VS Code Routes with no workspace or folder", ["--disable-workspace-tru // Closing the folder should stop the redirecting. await codeServerPage.navigate("/?ew=true") let url = new URL(codeServerPage.page.url()) - expect(url.pathname).toBe("/") + const pathname = getMaybeProxiedPathname(url) + expect(pathname).toBe("/") expect(url.search).toBe("?ew=true") }) }) diff --git a/test/unit/helpers.test.ts b/test/unit/helpers.test.ts index ba3a54d2810d..28420b060806 100644 --- a/test/unit/helpers.test.ts +++ b/test/unit/helpers.test.ts @@ -1,5 +1,6 @@ import { promises as fs } from "fs" -import { clean, getAvailablePort, tmpdir, useEnv } from "../../test/utils/helpers" +import { clean, getAvailablePort, getMaybeProxiedPathname, tmpdir, useEnv } from "../../test/utils/helpers" +import { REVERSE_PROXY_BASE_PATH } from "../utils/constants" /** * This file is for testing test helpers (not core code). @@ -56,3 +57,22 @@ describe("getAvailablePort", () => { expect(portOne).not.toEqual(portTwo) }) }) + +describe("getMaybeProxiedPathname", () => { + it("should return the route", () => { + const route = "/vscode" + const url = new URL(`http://localhost:3000${route}`) + const actual = getMaybeProxiedPathname(url) + expect(actual).toBe(route) + }) + it("should strip proxy if env var set", () => { + const envKey = "USE_PROXY" + const [setValue, resetValue] = useEnv(envKey) + setValue("1") + const route = "/vscode" + const url = new URL(`http://localhost:3000/8000/${REVERSE_PROXY_BASE_PATH}${route}`) + const actual = getMaybeProxiedPathname(url) + expect(actual).toBe(route) + resetValue() + }) +}) \ No newline at end of file diff --git a/test/utils/helpers.ts b/test/utils/helpers.ts index 889ea48ce792..cd54be486f13 100644 --- a/test/utils/helpers.ts +++ b/test/utils/helpers.ts @@ -136,3 +136,17 @@ export async function getMaybeProxiedCodeServer(codeServer: CodeServerPage | Cod return address } + +/** + * Stripes proxy base from url.pathname + * i.e. //ide + route returns just route + */ +export function getMaybeProxiedPathname(url: URL): string { + if (process.env.USE_PROXY === "1") { + // Behind proxy, path will be //ide + route + const pathWithoutProxy = url.pathname.split(`/${REVERSE_PROXY_BASE_PATH}`)[1] + return pathWithoutProxy + } + + return url.pathname +} From 02d0aec553af8b3cd2cf0412b1cd1a8f64b9f201 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Wed, 21 Dec 2022 15:15:02 -0700 Subject: [PATCH 27/31] fixup: formatting --- test/unit/helpers.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/helpers.test.ts b/test/unit/helpers.test.ts index 28420b060806..033449e9b160 100644 --- a/test/unit/helpers.test.ts +++ b/test/unit/helpers.test.ts @@ -75,4 +75,4 @@ describe("getMaybeProxiedPathname", () => { expect(actual).toBe(route) resetValue() }) -}) \ No newline at end of file +}) From 96812433a438c1c1fa63bec87c812ee73acaa957 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Wed, 21 Dec 2022 15:16:50 -0700 Subject: [PATCH 28/31] fixup: rm unused import --- test/e2e/routes.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/e2e/routes.test.ts b/test/e2e/routes.test.ts index b79f5ba16605..253a60e7b640 100644 --- a/test/e2e/routes.test.ts +++ b/test/e2e/routes.test.ts @@ -1,6 +1,5 @@ import { describe, test, expect } from "./baseFixture" import { clean, getMaybeProxiedPathname } from "../utils/helpers" -import { REVERSE_PROXY_BASE_PATH } from "../utils/constants" const routes = ["/", "/vscode", "/vscode/"] From a059129252216c5f5cba83e9bca3d90cf658b7be Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Wed, 21 Dec 2022 16:07:18 -0700 Subject: [PATCH 29/31] chore: increase playwright timeout --- test/playwright.config.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/playwright.config.ts b/test/playwright.config.ts index c555f0960909..fa3743eb474e 100644 --- a/test/playwright.config.ts +++ b/test/playwright.config.ts @@ -11,7 +11,7 @@ import path from "path" // PWDEBUG=1 yarn test:e2e # Run Playwright inspector const config: PlaywrightTestConfig = { testDir: path.join(__dirname, "e2e"), // Search for tests in this directory. - timeout: 60000, // Each test is given 60 seconds. + timeout: 120000, // Each test is given 120 seconds. retries: process.env.CI ? 2 : 1, // Retry in CI due to flakiness. // Limit the number of failures on CI to save resources maxFailures: process.env.CI ? 3 : undefined, From 3b0215162920ad1e525f130910e20ff2ac767132 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Wed, 21 Dec 2022 16:16:32 -0700 Subject: [PATCH 30/31] Revert "chore: increase playwright timeout" This reverts commit a059129252216c5f5cba83e9bca3d90cf658b7be. --- test/playwright.config.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/playwright.config.ts b/test/playwright.config.ts index fa3743eb474e..c555f0960909 100644 --- a/test/playwright.config.ts +++ b/test/playwright.config.ts @@ -11,7 +11,7 @@ import path from "path" // PWDEBUG=1 yarn test:e2e # Run Playwright inspector const config: PlaywrightTestConfig = { testDir: path.join(__dirname, "e2e"), // Search for tests in this directory. - timeout: 120000, // Each test is given 120 seconds. + timeout: 60000, // Each test is given 60 seconds. retries: process.env.CI ? 2 : 1, // Retry in CI due to flakiness. // Limit the number of failures on CI to save resources maxFailures: process.env.CI ? 3 : undefined, From c0d59c886fd30270256268eb9f96c0c3436a5dc0 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Wed, 21 Dec 2022 16:17:49 -0700 Subject: [PATCH 31/31] chore: rm timeout --- .github/workflows/build.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 101b7577f6cb..ecb67980c0d5 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -374,7 +374,7 @@ jobs: ./test/node_modules/.bin/playwright install - name: Run end-to-end tests - run: CODE_SERVER_TEST_ENTRY=./release yarn test:e2e --global-timeout 840000 + run: CODE_SERVER_TEST_ENTRY=./release yarn test:e2e - name: Upload test artifacts if: always()