From b61a8addcf186e38bc0d6e44d475af4ddd73b826 Mon Sep 17 00:00:00 2001 From: Asher Date: Thu, 3 Mar 2022 12:32:43 -0600 Subject: [PATCH] feat: migrate state to new database name (#4938) * Merge setup and navigate functions Whenever we navigate we probably want to make sure the editor is ready so might as well just have one function. * Add customizable entry and workspace directory * Add test for state db migration * Update Code This contains the state migrations. --- test/e2e/baseFixture.ts | 4 +- test/e2e/codeServer.test.ts | 93 +++++++++++++++++++++++----- test/e2e/models/CodeServer.ts | 112 ++++++++++++++++++---------------- vendor/package.json | 2 +- vendor/yarn.lock | 4 +- 5 files changed, 142 insertions(+), 73 deletions(-) diff --git a/test/e2e/baseFixture.ts b/test/e2e/baseFixture.ts index 3778593d6..1efad7f7a 100644 --- a/test/e2e/baseFixture.ts +++ b/test/e2e/baseFixture.ts @@ -70,8 +70,8 @@ export const test = base.extend({ // made too). In these cases just accept. page.on("dialog", (d) => d.accept()) - const codeServerPage = new CodeServerPage(codeServer, page) - await codeServerPage.setup(authenticated) + const codeServerPage = new CodeServerPage(codeServer, page, authenticated) + await codeServerPage.navigate() await use(codeServerPage) }, }) diff --git a/test/e2e/codeServer.test.ts b/test/e2e/codeServer.test.ts index 5d09f3331..30e8cc6c5 100644 --- a/test/e2e/codeServer.test.ts +++ b/test/e2e/codeServer.test.ts @@ -1,8 +1,45 @@ +import * as cp from "child_process" import { promises as fs } from "fs" +import * as os from "os" import * as path from "path" +import * as util from "util" import { describe, test, expect } from "./baseFixture" +import { CodeServer } from "./models/CodeServer" + +describe("code-server", true, [], {}, () => { + // TODO@asher: Generalize this? Could be nice if we were to ever need + // multiple migration tests in other suites. + const instances = new Map() + test.afterAll(async () => { + const procs = Array.from(instances.values()) + instances.clear() + await Promise.all(procs.map((cs) => cs.close())) + }) + + /** + * Spawn a specific version of code-server using the install script. + */ + const spawn = async (version: string, dir?: string): Promise => { + let instance = instances.get(version) + if (!instance) { + await util.promisify(cp.exec)(`./install.sh --method standalone --version ${version}`, { + cwd: path.join(__dirname, "../.."), + }) + + instance = new CodeServer( + "code-server@" + version, + ["--auth=none"], + { VSCODE_DEV: "" }, + dir, + `${os.homedir()}/.local/lib/code-server-${version}`, + ) + + instances.set(version, instance) + } + + return instance + } -describe("CodeServer", true, [], {}, () => { test("should navigate to home page", async ({ codeServerPage }) => { // We navigate codeServer before each test // and we start the test with a storage state @@ -34,24 +71,50 @@ describe("CodeServer", true, [], {}, () => { await codeServerPage.openFile(file) }) - test("should not share state with other paths", async ({ codeServerPage }) => { + test("should migrate state to avoid collisions", async ({ codeServerPage }) => { + // This can take a very long time in development because of how long pages + // take to load and we are doing a lot of that here. + test.slow() + const dir = await codeServerPage.workspaceDir - const file = path.join(dir, "foo") - await fs.writeFile(file, "bar") + const files = [path.join(dir, "foo"), path.join(dir, "bar")] + await Promise.all( + files.map((file) => { + return fs.writeFile(file, path.basename(file)) + }), + ) - await codeServerPage.openFile(file) + // Open a file in the latest instance. + await codeServerPage.openFile(files[0]) + await codeServerPage.stateFlush() - // If we reload now VS Code will be unable to save the state changes so wait - // until those have been written to the database. It flushes every five - // seconds so we need to wait at least that long. - await codeServerPage.page.waitForTimeout(5500) + // Open a file in an older version of code-server. It should not see the + // file opened in the new instance since the database has a different + // name. This must be accessed through the proxy so it shares the same + // domain and can write to the same database. + const cs = await spawn("4.0.2", dir) + const address = new URL(await cs.address()) + await codeServerPage.navigate("/proxy/" + address.port + "/") + await codeServerPage.openFile(files[1]) + expect(await codeServerPage.tabIsVisible(files[0])).toBe(false) + await codeServerPage.stateFlush() - // The tab should re-open on refresh. - await codeServerPage.page.reload() - await codeServerPage.waitForTab(file) + // Move back to latest code-server. We should see the file we previously + // opened with it but not the old code-server file because the new instance + // already created its own database on this path and will avoid migrating. + await codeServerPage.navigate() + await codeServerPage.waitForTab(files[0]) + expect(await codeServerPage.tabIsVisible(files[1])).toBe(false) - // The tab should not re-open on a different path. - await codeServerPage.setup(true, "/vscode") - expect(await codeServerPage.tabIsVisible(file)).toBe(false) + // Open a new path in latest code-server. This one should migrate the + // database from old code-server but see nothing from the new database + // created on the root. + await codeServerPage.navigate("/vscode") + await codeServerPage.waitForTab(files[1]) + expect(await codeServerPage.tabIsVisible(files[0])).toBe(false) + // Should still be open after a reload. + await codeServerPage.navigate("/vscode") + await codeServerPage.waitForTab(files[1]) + expect(await codeServerPage.tabIsVisible(files[0])).toBe(false) }) }) diff --git a/test/e2e/models/CodeServer.ts b/test/e2e/models/CodeServer.ts index f6c55cb67..004fd99f0 100644 --- a/test/e2e/models/CodeServer.ts +++ b/test/e2e/models/CodeServer.ts @@ -3,7 +3,7 @@ import * as cp from "child_process" import { promises as fs } from "fs" import * as path from "path" import { Page } from "playwright" -import util from "util" +import * as util from "util" import { logError, plural } from "../../../src/common/util" import { onLine } from "../../../src/node/util" import { PASSWORD, workspaceDir } from "../../utils/constants" @@ -38,12 +38,13 @@ export class CodeServer { private process: Promise | undefined public readonly logger: Logger private closed = false - private _workspaceDir: Promise | undefined constructor( name: string, - private readonly codeServerArgs: string[], - private readonly codeServerEnv: NodeJS.ProcessEnv, + private readonly args: string[], + private readonly env: NodeJS.ProcessEnv, + private readonly _workspaceDir: Promise | string | undefined, + private readonly entry = process.env.CODE_SERVER_TEST_ENTRY || ".", ) { this.logger = logger.named(name) } @@ -75,7 +76,7 @@ export class CodeServer { */ private async createWorkspace(): Promise { const dir = await this.workspaceDir - await fs.mkdir(path.join(dir, "User")) + await fs.mkdir(path.join(dir, "User"), { recursive: true }) await fs.writeFile( path.join(dir, "User/settings.json"), JSON.stringify({ @@ -96,36 +97,33 @@ export class CodeServer { const dir = await this.createWorkspace() return new Promise((resolve, reject) => { - this.logger.debug("spawning") - const proc = cp.spawn( - "node", - [ - process.env.CODE_SERVER_TEST_ENTRY || ".", - "--extensions-dir", - path.join(dir, "extensions"), - ...this.codeServerArgs, - // Using port zero will spawn on a random port. - "--bind-addr", - "127.0.0.1:0", - // Setting the XDG variables would be easier and more thorough but the - // modules we import ignores those variables for non-Linux operating - // systems so use these flags instead. - "--config", - path.join(dir, "config.yaml"), - "--user-data-dir", - dir, - // The last argument is the workspace to open. - dir, - ], - { - cwd: path.join(__dirname, "../../.."), - env: { - ...process.env, - ...this.codeServerEnv, - PASSWORD, - }, + const args = [ + this.entry, + "--extensions-dir", + path.join(dir, "extensions"), + ...this.args, + // Using port zero will spawn on a random port. + "--bind-addr", + "127.0.0.1:0", + // Setting the XDG variables would be easier and more thorough but the + // modules we import ignores those variables for non-Linux operating + // systems so use these flags instead. + "--config", + 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, { + cwd: path.join(__dirname, "../../.."), + env: { + ...process.env, + ...this.env, + PASSWORD, }, - ) + }) const timer = idleTimer("Failed to extract address; did the format change?", reject) @@ -136,7 +134,7 @@ export class CodeServer { }) proc.on("close", (code) => { - const error = new Error("closed unexpectedly") + const error = new Error("code-server closed unexpectedly") if (!this.closed) { this.logger.error(error.message, field("code", code)) } @@ -153,7 +151,7 @@ export class CodeServer { timer.reset() // Log the line without the timestamp. - this.logger.trace(line.replace(/\[.+\]/, "")) + this.logger.debug(line.replace(/\[.+\]/, "")) if (resolved) { return } @@ -194,7 +192,11 @@ export class CodeServer { export class CodeServerPage { private readonly editorSelector = "div.monaco-workbench" - constructor(private readonly codeServer: CodeServer, public readonly page: Page) { + constructor( + private readonly codeServer: CodeServer, + public readonly page: Page, + private readonly authenticated: boolean, + ) { this.page.on("console", (message) => { this.codeServer.logger.debug(message) }) @@ -215,11 +217,18 @@ export class CodeServerPage { } /** - * Navigate to a code-server endpoint. By default go to the root. + * Navigate to a code-server endpoint (root by default). Then wait for the + * editor to become available. */ async navigate(endpoint = "/") { const to = new URL(endpoint, await this.codeServer.address()) await this.page.goto(to.toString(), { waitUntil: "networkidle" }) + + // Only reload editor if authenticated. Otherwise we'll get stuck + // reloading the login page. + if (this.authenticated) { + await this.reloadUntilEditorIsReady() + } } /** @@ -456,21 +465,7 @@ export class CodeServerPage { } /** - * Navigates to code-server then reloads until the editor is ready. - * - * It is recommended to run setup before using this model in any tests. - */ - async setup(authenticated: boolean, endpoint = "/") { - await this.navigate(endpoint) - // If we aren't authenticated we'll see a login page so we can't wait until - // the editor is ready. - if (authenticated) { - await this.reloadUntilEditorIsReady() - } - } - - /** - * Execute a command in t root of the instance's workspace directory. + * Execute a command in the root of the instance's workspace directory. */ async exec(command: string): Promise { await util.promisify(cp.exec)(command, { @@ -488,4 +483,15 @@ export class CodeServerPage { cwd: path.join(__dirname, "../../.."), }) } + + /** + * Wait for state to be flushed to the database. + */ + async stateFlush(): Promise { + // If we reload too quickly VS Code will be unable to save the state changes + // so wait until those have been written to the database. It flushes every + // five seconds so we need to wait at least that long. + // TODO@asher: There must be a better way. + await this.page.waitForTimeout(5500) + } } diff --git a/vendor/package.json b/vendor/package.json index 9d00927be..86f905adf 100644 --- a/vendor/package.json +++ b/vendor/package.json @@ -7,6 +7,6 @@ "postinstall": "./postinstall.sh" }, "devDependencies": { - "code-oss-dev": "coder/vscode#bd734e3d9f21b1bce4dabab2514177e90c090ee6" + "code-oss-dev": "coder/vscode#94384412221f432c15bb679315c49964925090be" } } diff --git a/vendor/yarn.lock b/vendor/yarn.lock index 9a5eff2e1..14b8b6f42 100644 --- a/vendor/yarn.lock +++ b/vendor/yarn.lock @@ -274,9 +274,9 @@ clone-response@^1.0.2: dependencies: mimic-response "^1.0.0" -code-oss-dev@coder/vscode#bd734e3d9f21b1bce4dabab2514177e90c090ee6: +code-oss-dev@coder/vscode#94384412221f432c15bb679315c49964925090be: version "1.63.0" - resolved "https://codeload.github.com/coder/vscode/tar.gz/bd734e3d9f21b1bce4dabab2514177e90c090ee6" + resolved "https://codeload.github.com/coder/vscode/tar.gz/94384412221f432c15bb679315c49964925090be" dependencies: "@microsoft/applicationinsights-web" "^2.6.4" "@parcel/watcher" "2.0.3"