From 5f5b4178be39284c7a8c46b58ccbfe40fdda637f Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Wed, 4 Dec 2024 13:55:55 -0600 Subject: [PATCH 01/14] add persistence and hangup functionality to ocap serve --- packages/cli/package.json | 2 + packages/cli/src/app.ts | 64 +++++++++++++++++++------ packages/cli/src/commands/serve.test.ts | 18 +++++-- packages/cli/src/commands/serve.ts | 11 ++++- packages/cli/src/utils.test.ts | 17 +++++++ packages/cli/src/utils.ts | 50 +++++++++++++++++++ vitest.config.ts | 10 ++-- yarn.lock | 2 + 8 files changed, 149 insertions(+), 25 deletions(-) create mode 100644 packages/cli/src/utils.test.ts create mode 100644 packages/cli/src/utils.ts diff --git a/packages/cli/package.json b/packages/cli/package.json index c22494e34..8cce95edd 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -35,6 +35,7 @@ "dependencies": { "@endo/bundle-source": "^3.5.0", "@endo/init": "^1.1.6", + "@endo/promise-kit": "^1.1.6", "@metamask/snaps-utils": "^8.3.0", "@ocap/shims": "workspace:^", "@ocap/utils": "workspace:^", @@ -50,6 +51,7 @@ "@metamask/eslint-config-nodejs": "^14.0.0", "@metamask/eslint-config-typescript": "^14.0.0", "@metamask/utils": "^9.3.0", + "@ocap/test-utils": "workspace:^", "@ts-bridge/cli": "^0.5.1", "@ts-bridge/shims": "^0.1.1", "@types/serve-handler": "^6", diff --git a/packages/cli/src/app.ts b/packages/cli/src/app.ts index 02180bdc2..ba2b6f73b 100755 --- a/packages/cli/src/app.ts +++ b/packages/cli/src/app.ts @@ -1,9 +1,12 @@ +import path from 'node:path'; import yargs from 'yargs'; import { hideBin } from 'yargs/helpers'; import { createBundle } from './commands/bundle.js'; import { getServer } from './commands/serve.js'; import { defaultConfig } from './config.js'; +import type { Config } from './config.js'; +import { makeTimeoutWithReset, withTimeout } from './utils.js'; await yargs(hideBin(process.argv)) .usage('$0 [options]') @@ -26,33 +29,66 @@ await yargs(hideBin(process.argv)) }, ) .command( - 'serve [-p port]', + 'serve [-p port] [options]', 'Serve bundled user code by filename', (_yargs) => _yargs - .option('port', { - alias: 'p', - type: 'number', - default: defaultConfig.server.port, - }) .option('dir', { - alias: 'd', type: 'string', dir: true, required: true, describe: 'A directory of files to bundle', + }) + .option('port', { + alias: 'p', + type: 'number', + default: defaultConfig.server.port, + }) + .option('hangup', { + alias: 'h', + type: 'number', + array: false, + default: 3000, + describe: + 'How long the server keeps running after receiving its last request', + }) + .option('no-hangup', { + type: 'boolean', + default: false, }), async (args) => { - console.info(`serving ${args.dir} on localhost:${args.port}`); - const server = getServer({ + const appName = 'bundle server'; + const url = `http://localhost:${args.port}`; + const resolvedDir = path.resolve(args.dir); + const config: Config = { server: { port: args.port, }, - dir: args.dir, - }); - await server.listen(); + dir: resolvedDir, + }; + console.info(`starting ${appName} in ${resolvedDir} on ${url}`); + + if (args.hangup) { + const parsedHangup = Number(args.hangup?.toString().split(',').at(-1)); + const { promise: hangup, reset: resetHangup } = + makeTimeoutWithReset(parsedHangup); + console.info( + `${appName} will auto hangup after ${parsedHangup}ms without a request`, + ); + + const server = getServer(config, resetHangup); + const { close } = await server.listen(); + + await hangup; + console.log( + `terminating ${appName} after ${parsedHangup}ms without a request`, + ); + await withTimeout(close(), 400).catch(console.error); + } else { + const server = getServer(config); + await server.listen(); + } }, ) - .help('h') - .alias('h', 'help') + .help('help') .parse(); diff --git a/packages/cli/src/commands/serve.test.ts b/packages/cli/src/commands/serve.test.ts index 7c665d160..fc1034b03 100644 --- a/packages/cli/src/commands/serve.test.ts +++ b/packages/cli/src/commands/serve.test.ts @@ -11,6 +11,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { getServer } from './serve.js'; import { getTestBundles } from '../../test/bundles.js'; import { defaultConfig } from '../config.js'; +import { withTimeout } from '../utils.js'; const isBundleSourceResult = ( value: unknown, @@ -61,19 +62,29 @@ describe('serve', async () => { }, dir: root, }); + const url = `http://localhost:${port}`; const requestBundle = async (path: string): Promise => { - const resp = await nodeFetch(`http://localhost:${port}/${path}`); + const resp = await nodeFetch(`${url}/${path}`); if (resp.ok) { return resp.json(); } throw new Error(resp.statusText, { cause: resp.status }); }; return { + url, listen, requestBundle, }; }; + it('responds to ping', async () => { + const { listen, url } = makeServer(testBundleRoot); + const { close } = await listen(); + const response = await nodeFetch(`${url}/ping`); + expect(response.ok).toBe(true); + await withTimeout(close(), 400).catch(console.error); + }); + it('serves bundles', async () => { const bundleName = 'test.bundle'; const bundleRoot = join(testBundleRoot, '..'); @@ -107,10 +118,7 @@ describe('serve', async () => { expect(receivedBundleHash).toStrictEqual(expectedBundleHash); } finally { - await Promise.race([ - new Promise((_resolve) => setTimeout(_resolve, 400)), - close(), - ]); + await withTimeout(close(), 400).catch(console.error); } }); diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index aa4fc7472..b1c71c44b 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -12,11 +12,12 @@ import type { Config } from '../config.js'; * Get a static server for development purposes. * * @param config - The config object. + * @param onGetResponse - An optional hook to call when getResponse() is called. * @returns An object with a `listen` method that returns a promise that * resolves when the server is listening. */ // eslint-disable-next-line @typescript-eslint/explicit-function-return-type -export function getServer(config: Config) { +export function getServer(config: Config, onGetResponse?: () => void) { if (!config.dir) { throw new Error(`Config option 'dir' must be specified.`); } @@ -37,12 +38,20 @@ export function getServer(config: Config) { request: IncomingMessage, response: ServerResponse, ): Promise { + onGetResponse?.(); + const pathname = request.url && request.headers.host && new URL(request.url, `http://${request.headers.host}`).pathname; const path = pathname?.slice(1); + if (path === 'ping') { + response.statusCode = 200; + response.end(); + return; + } + if (!isAllowedPath(path)) { response.statusCode = 404; response.end(); diff --git a/packages/cli/src/utils.test.ts b/packages/cli/src/utils.test.ts new file mode 100644 index 000000000..213d51651 --- /dev/null +++ b/packages/cli/src/utils.test.ts @@ -0,0 +1,17 @@ +import '@ocap/shims/endoify'; +import { delay } from '@ocap/test-utils'; +import { describe, it, expect } from 'vitest'; + +import { withTimeout } from './utils.js'; + +describe('utils', async () => { + describe('withTimeout', () => { + it('times out within the specified duration', async () => { + const duration = 300; + const delta = 100; + const timeout = withTimeout(new Promise(() => undefined), duration); + await delay(duration + delta); + await expect(async () => await timeout).rejects.toThrow(/timed out/u); + }); + }); +}); diff --git a/packages/cli/src/utils.ts b/packages/cli/src/utils.ts new file mode 100644 index 000000000..0d5b68cb6 --- /dev/null +++ b/packages/cli/src/utils.ts @@ -0,0 +1,50 @@ +import { makePromiseKit } from '@endo/promise-kit'; + +/** + * Wrap a promise with a timeout rejection. + * + * @param promise - The promise to wrap with a timeout. + * @param timeout - How many ms to wait before rejecting. + * @returns A wrapped promise which rejects after timeout miliseconds. + */ +export async function withTimeout( + promise: Promise, + timeout: number, +): Promise { + return Promise.race([ + promise, + new Promise((_resolve, reject) => + setTimeout( + () => + reject( + new Error(`promise timed out after ${timeout}ms`, { + cause: promise, + }), + ), + timeout, + ), + ), + ]) as Promise; +} + +/** + * Make a promise which resolves after a timeout and a reset method which resets the timeout. + * + * @param timeout How many ms to wait before the timeout completes. + * @returns A reset method and a promise which resolves timeout ms after the last reset call. + */ +export function makeTimeoutWithReset(timeout: number): { + reset: () => void; + promise: Promise; +} { + const { promise, resolve } = makePromiseKit(); + let timeoutId = setTimeout(() => resolve(), timeout); + const reset = (): void => { + clearTimeout(timeoutId); + timeoutId = setTimeout(() => resolve(), timeout); + }; + return { + promise, + reset, + }; +} diff --git a/vitest.config.ts b/vitest.config.ts index 9e825de6f..7a4a474be 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -35,10 +35,10 @@ export default defineConfig({ thresholds: { autoUpdate: true, 'packages/cli/**': { - statements: 75.47, - functions: 72.22, - branches: 61.11, - lines: 75.47, + statements: 58.53, + functions: 64, + branches: 59.09, + lines: 60, }, 'packages/errors/**': { statements: 100, @@ -79,4 +79,4 @@ export default defineConfig({ }, }, }, -}); +}); \ No newline at end of file diff --git a/yarn.lock b/yarn.lock index b7f1a878c..49eaa7867 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1606,6 +1606,7 @@ __metadata: "@arethetypeswrong/cli": "npm:^0.16.4" "@endo/bundle-source": "npm:^3.5.0" "@endo/init": "npm:^1.1.6" + "@endo/promise-kit": "npm:^1.1.6" "@metamask/auto-changelog": "npm:^3.4.4" "@metamask/eslint-config": "npm:^14.0.0" "@metamask/eslint-config-nodejs": "npm:^14.0.0" @@ -1613,6 +1614,7 @@ __metadata: "@metamask/snaps-utils": "npm:^8.3.0" "@metamask/utils": "npm:^9.3.0" "@ocap/shims": "workspace:^" + "@ocap/test-utils": "workspace:^" "@ocap/utils": "workspace:^" "@ts-bridge/cli": "npm:^0.5.1" "@ts-bridge/shims": "npm:^0.1.1" From 9df3c48953e461add165e65279cbec56e84436e5 Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Thu, 5 Dec 2024 14:56:38 -0600 Subject: [PATCH 02/14] lint --- vitest.config.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vitest.config.ts b/vitest.config.ts index 7a4a474be..bf6822f37 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -79,4 +79,4 @@ export default defineConfig({ }, }, }, -}); \ No newline at end of file +}); From 2ae21b8f6c96cd78b402ae9b482b3a9ff6f1a66b Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Thu, 5 Dec 2024 15:33:03 -0600 Subject: [PATCH 03/14] test(extension): use ocap serve for e2e tests --- packages/extension/package.json | 5 ++--- packages/extension/playwright.config.ts | 5 ----- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/packages/extension/package.json b/packages/extension/package.json index ec96d8beb..c1f39b7e1 100644 --- a/packages/extension/package.json +++ b/packages/extension/package.json @@ -19,7 +19,6 @@ "build:dev": "yarn build:vite:dev && yarn test:build", "build:vite": "vite build --config vite.config.ts", "build:vite:dev": "yarn build:vite --mode development", - "bundle": "node ../../scripts/bundle-vat.js", "changelog:validate": "../../scripts/validate-changelog.sh @ocap/extension", "clean": "rimraf --glob ./dist './*.tsbuildinfo'", "lint": "yarn lint:ts && yarn lint:eslint && yarn lint:misc --check && yarn constraints && yarn lint:dependencies", @@ -37,10 +36,10 @@ "test:dev": "yarn test --coverage false", "test:verbose": "yarn test --reporter verbose", "test:watch": "vitest --config vitest.config.ts", - "test:e2e": "playwright test", + "test:e2e": "yarn start:server & playwright test", "test:e2e:ui": "playwright test --ui", "test:e2e:debug": "playwright test --debug", - "start:server": "npx http-server ./src/vats -p 3000 -a localhost --cors -y" + "start:server": "yarn ocap serve ./src/vats -p 3000 -h 10000" }, "dependencies": { "@endo/eventual-send": "^1.2.6", diff --git a/packages/extension/playwright.config.ts b/packages/extension/playwright.config.ts index 7735dd424..52415db8a 100644 --- a/packages/extension/playwright.config.ts +++ b/packages/extension/playwright.config.ts @@ -14,9 +14,4 @@ export default defineConfig({ }, }, ], - webServer: { - command: 'yarn start:server', - url: 'http://localhost:3000', - reuseExistingServer: true, - }, }); From 9f37c082a4900fa149799f5cf42dd313da3ef9ce Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Thu, 5 Dec 2024 15:37:40 -0600 Subject: [PATCH 04/14] use 'ocap bundle' in CI setup --- .github/workflows/lint-build-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/lint-build-test.yml b/.github/workflows/lint-build-test.yml index 56d2a11e6..475e171d5 100644 --- a/.github/workflows/lint-build-test.yml +++ b/.github/workflows/lint-build-test.yml @@ -174,7 +174,7 @@ jobs: cache: yarn - run: yarn --immutable - run: yarn build - - run: yarn bundle ./packages/extension/src/vats/sample-vat.js + - run: yarn ocap bundle ./packages/extension/src/vats/sample-vat.js - run: yarn test:e2e - name: Require clean working directory shell: bash From 5dc6f5bddf6d49751f803a0c27c43ea3e49ca4bb Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Thu, 5 Dec 2024 15:51:28 -0600 Subject: [PATCH 05/14] remove unused ping command --- packages/cli/src/commands/serve.test.ts | 8 -------- packages/cli/src/commands/serve.ts | 6 ------ 2 files changed, 14 deletions(-) diff --git a/packages/cli/src/commands/serve.test.ts b/packages/cli/src/commands/serve.test.ts index fc1034b03..cccd0c89f 100644 --- a/packages/cli/src/commands/serve.test.ts +++ b/packages/cli/src/commands/serve.test.ts @@ -77,14 +77,6 @@ describe('serve', async () => { }; }; - it('responds to ping', async () => { - const { listen, url } = makeServer(testBundleRoot); - const { close } = await listen(); - const response = await nodeFetch(`${url}/ping`); - expect(response.ok).toBe(true); - await withTimeout(close(), 400).catch(console.error); - }); - it('serves bundles', async () => { const bundleName = 'test.bundle'; const bundleRoot = join(testBundleRoot, '..'); diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index b1c71c44b..75c7e9d64 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -46,12 +46,6 @@ export function getServer(config: Config, onGetResponse?: () => void) { new URL(request.url, `http://${request.headers.host}`).pathname; const path = pathname?.slice(1); - if (path === 'ping') { - response.statusCode = 200; - response.end(); - return; - } - if (!isAllowedPath(path)) { response.statusCode = 404; response.end(); From 5225ecc6f31f5add18645a702e1c3c3818948a25 Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Thu, 5 Dec 2024 15:58:07 -0600 Subject: [PATCH 06/14] simplify hangup args --- packages/cli/src/app.ts | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/packages/cli/src/app.ts b/packages/cli/src/app.ts index ba2b6f73b..196867ee1 100755 --- a/packages/cli/src/app.ts +++ b/packages/cli/src/app.ts @@ -48,13 +48,9 @@ await yargs(hideBin(process.argv)) alias: 'h', type: 'number', array: false, - default: 3000, + default: 0, describe: - 'How long the server keeps running after receiving its last request', - }) - .option('no-hangup', { - type: 'boolean', - default: false, + 'How long the server keeps running after receiving its last request. Set to 0 to disable.', }), async (args) => { const appName = 'bundle server'; @@ -68,7 +64,10 @@ await yargs(hideBin(process.argv)) }; console.info(`starting ${appName} in ${resolvedDir} on ${url}`); - if (args.hangup) { + if (args.hangup === 0) { + const server = getServer(config); + await server.listen(); + } else { const parsedHangup = Number(args.hangup?.toString().split(',').at(-1)); const { promise: hangup, reset: resetHangup } = makeTimeoutWithReset(parsedHangup); @@ -84,9 +83,6 @@ await yargs(hideBin(process.argv)) `terminating ${appName} after ${parsedHangup}ms without a request`, ); await withTimeout(close(), 400).catch(console.error); - } else { - const server = getServer(config); - await server.listen(); } }, ) From dab1a4dbd7f9400dfb08464e0b1e4316fd8652d4 Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Thu, 5 Dec 2024 16:05:17 -0600 Subject: [PATCH 07/14] tweak cli feedback --- packages/cli/src/app.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/app.ts b/packages/cli/src/app.ts index 196867ee1..73604f29a 100755 --- a/packages/cli/src/app.ts +++ b/packages/cli/src/app.ts @@ -29,7 +29,7 @@ await yargs(hideBin(process.argv)) }, ) .command( - 'serve [-p port] [options]', + 'serve [options]', 'Serve bundled user code by filename', (_yargs) => _yargs @@ -37,7 +37,7 @@ await yargs(hideBin(process.argv)) type: 'string', dir: true, required: true, - describe: 'A directory of files to bundle', + describe: 'A directory containing bundle files to serve', }) .option('port', { alias: 'p', @@ -50,7 +50,7 @@ await yargs(hideBin(process.argv)) array: false, default: 0, describe: - 'How long the server keeps running after receiving its last request. Set to 0 to disable.', + 'How long the server keeps running after receiving its last request. Set to 0 to disable', }), async (args) => { const appName = 'bundle server'; From 5371e83760b036200f0bd0c19797b29bc1063882 Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Thu, 5 Dec 2024 16:08:41 -0600 Subject: [PATCH 08/14] update coverage thresholds --- vitest.config.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/vitest.config.ts b/vitest.config.ts index bf6822f37..bc3b46394 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -35,10 +35,10 @@ export default defineConfig({ thresholds: { autoUpdate: true, 'packages/cli/**': { - statements: 58.53, + statements: 56.41, functions: 64, - branches: 59.09, - lines: 60, + branches: 55, + lines: 57.89, }, 'packages/errors/**': { statements: 100, From 54d69c7e927f830e1411a90a3b6911573fdfdace Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Fri, 6 Dec 2024 09:00:52 -0600 Subject: [PATCH 09/14] parse hangupTime before reading --- packages/cli/src/app.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/app.ts b/packages/cli/src/app.ts index 73604f29a..cb5afe1a1 100755 --- a/packages/cli/src/app.ts +++ b/packages/cli/src/app.ts @@ -64,15 +64,16 @@ await yargs(hideBin(process.argv)) }; console.info(`starting ${appName} in ${resolvedDir} on ${url}`); - if (args.hangup === 0) { + const hangupTime = Number(args.hangup?.toString().split(',').at(-1)); + + if (hangupTime === 0) { const server = getServer(config); await server.listen(); } else { - const parsedHangup = Number(args.hangup?.toString().split(',').at(-1)); const { promise: hangup, reset: resetHangup } = - makeTimeoutWithReset(parsedHangup); + makeTimeoutWithReset(hangupTime); console.info( - `${appName} will auto hangup after ${parsedHangup}ms without a request`, + `${appName} will auto hangup after ${hangupTime}ms without a request`, ); const server = getServer(config, resetHangup); @@ -80,7 +81,7 @@ await yargs(hideBin(process.argv)) await hangup; console.log( - `terminating ${appName} after ${parsedHangup}ms without a request`, + `terminating ${appName} after ${hangupTime}ms without a request`, ); await withTimeout(close(), 400).catch(console.error); } From cde52c167e480c7726a7c85bc099e4c52ef71178 Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Fri, 6 Dec 2024 09:23:40 -0600 Subject: [PATCH 10/14] organize test bundle setup routine --- packages/cli/test/bundles.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/cli/test/bundles.ts b/packages/cli/test/bundles.ts index 9cfaced4f..733775ce0 100644 --- a/packages/cli/test/bundles.ts +++ b/packages/cli/test/bundles.ts @@ -7,10 +7,11 @@ import { cp } from '../src/file.js'; const makeTestBundleRoot = async (): Promise => { const testRoot = resolve(import.meta.url.split(':')[1] as string, '..'); + const stageRoot = resolve(tmpdir(), 'test'); // copy bundle targets to staging area const testBundleRoot = resolve(testRoot, 'bundles'); - const stageBundleRoot = resolve(tmpdir(), 'test/bundles'); + const stageBundleRoot = resolve(stageRoot, 'bundles'); await mkdir(stageBundleRoot, { recursive: true }); for (const ext of ['.js', '.expected']) { await Promise.all( @@ -20,10 +21,7 @@ const makeTestBundleRoot = async (): Promise => { }), ); } - await cp( - join(testRoot, 'test.bundle'), - join(stageBundleRoot, '../test.bundle'), - ); + await cp(join(testRoot, 'test.bundle'), join(stageRoot, 'test.bundle')); // return the staging area, ready for testing return stageBundleRoot; From b2b01fcda3791c38ebc969e1a5961e8c035a8a2c Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Fri, 6 Dec 2024 09:32:22 -0600 Subject: [PATCH 11/14] remove unused url from makeServer return --- packages/cli/src/commands/serve.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/cli/src/commands/serve.test.ts b/packages/cli/src/commands/serve.test.ts index cccd0c89f..b7902bc01 100644 --- a/packages/cli/src/commands/serve.test.ts +++ b/packages/cli/src/commands/serve.test.ts @@ -71,7 +71,6 @@ describe('serve', async () => { throw new Error(resp.statusText, { cause: resp.status }); }; return { - url, listen, requestBundle, }; From 032b36ddfd657b17ee8b97fa2832101ea1bc16d4 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Fri, 6 Dec 2024 10:40:42 -0800 Subject: [PATCH 12/14] test(extension): Refactor e2e test script (#270) Refactors the e2e test invocation to avoid a timed hangup of the server. We still start the server in parallel with Playwright, but we always close the server by killing its process after the tests end. --- .github/workflows/lint-build-test.yml | 3 +-- packages/extension/package.json | 6 +++--- packages/extension/scripts/test-e2e-ci.sh | 22 ++++++++++++++++++++++ 3 files changed, 26 insertions(+), 5 deletions(-) create mode 100755 packages/extension/scripts/test-e2e-ci.sh diff --git a/.github/workflows/lint-build-test.yml b/.github/workflows/lint-build-test.yml index 475e171d5..e9d6c0563 100644 --- a/.github/workflows/lint-build-test.yml +++ b/.github/workflows/lint-build-test.yml @@ -174,8 +174,7 @@ jobs: cache: yarn - run: yarn --immutable - run: yarn build - - run: yarn ocap bundle ./packages/extension/src/vats/sample-vat.js - - run: yarn test:e2e + - run: yarn test:e2e:ci - name: Require clean working directory shell: bash run: | diff --git a/packages/extension/package.json b/packages/extension/package.json index c1f39b7e1..ea1be7226 100644 --- a/packages/extension/package.json +++ b/packages/extension/package.json @@ -36,10 +36,10 @@ "test:dev": "yarn test --coverage false", "test:verbose": "yarn test --reporter verbose", "test:watch": "vitest --config vitest.config.ts", - "test:e2e": "yarn start:server & playwright test", + "test:e2e": "yarn playwright test", + "test:e2e:ci": "./scripts/test-e2e-ci.sh", "test:e2e:ui": "playwright test --ui", - "test:e2e:debug": "playwright test --debug", - "start:server": "yarn ocap serve ./src/vats -p 3000 -h 10000" + "test:e2e:debug": "playwright test --debug" }, "dependencies": { "@endo/eventual-send": "^1.2.6", diff --git a/packages/extension/scripts/test-e2e-ci.sh b/packages/extension/scripts/test-e2e-ci.sh new file mode 100755 index 000000000..a7d3222f1 --- /dev/null +++ b/packages/extension/scripts/test-e2e-ci.sh @@ -0,0 +1,22 @@ +#!/usr/bin/env bash + +set -x +set -e +set -o pipefail + +yarn ocap bundle "./src/vats" + +# Start the server in background and capture its PID +yarn ocap serve "./src/vats" & +SERVER_PID=$! + +function cleanup() { + # Kill the server if it's still running + if kill -0 $SERVER_PID 2>/dev/null; then + kill $SERVER_PID + fi +} +# Ensure we always close the server +trap cleanup EXIT + +yarn test:e2e From 2f36095110986cdd53c37732fcc61f526c83498c Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Fri, 6 Dec 2024 12:47:09 -0600 Subject: [PATCH 13/14] remove unused hangup functionality --- packages/cli/src/app.ts | 33 ++---------------------------- packages/cli/src/commands/serve.ts | 5 +---- vitest.config.ts | 6 +++--- 3 files changed, 6 insertions(+), 38 deletions(-) diff --git a/packages/cli/src/app.ts b/packages/cli/src/app.ts index cb5afe1a1..5c764dc83 100755 --- a/packages/cli/src/app.ts +++ b/packages/cli/src/app.ts @@ -6,7 +6,6 @@ import { createBundle } from './commands/bundle.js'; import { getServer } from './commands/serve.js'; import { defaultConfig } from './config.js'; import type { Config } from './config.js'; -import { makeTimeoutWithReset, withTimeout } from './utils.js'; await yargs(hideBin(process.argv)) .usage('$0 [options]') @@ -43,14 +42,6 @@ await yargs(hideBin(process.argv)) alias: 'p', type: 'number', default: defaultConfig.server.port, - }) - .option('hangup', { - alias: 'h', - type: 'number', - array: false, - default: 0, - describe: - 'How long the server keeps running after receiving its last request. Set to 0 to disable', }), async (args) => { const appName = 'bundle server'; @@ -63,28 +54,8 @@ await yargs(hideBin(process.argv)) dir: resolvedDir, }; console.info(`starting ${appName} in ${resolvedDir} on ${url}`); - - const hangupTime = Number(args.hangup?.toString().split(',').at(-1)); - - if (hangupTime === 0) { - const server = getServer(config); - await server.listen(); - } else { - const { promise: hangup, reset: resetHangup } = - makeTimeoutWithReset(hangupTime); - console.info( - `${appName} will auto hangup after ${hangupTime}ms without a request`, - ); - - const server = getServer(config, resetHangup); - const { close } = await server.listen(); - - await hangup; - console.log( - `terminating ${appName} after ${hangupTime}ms without a request`, - ); - await withTimeout(close(), 400).catch(console.error); - } + const server = getServer(config); + await server.listen(); }, ) .help('help') diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index 75c7e9d64..aa4fc7472 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -12,12 +12,11 @@ import type { Config } from '../config.js'; * Get a static server for development purposes. * * @param config - The config object. - * @param onGetResponse - An optional hook to call when getResponse() is called. * @returns An object with a `listen` method that returns a promise that * resolves when the server is listening. */ // eslint-disable-next-line @typescript-eslint/explicit-function-return-type -export function getServer(config: Config, onGetResponse?: () => void) { +export function getServer(config: Config) { if (!config.dir) { throw new Error(`Config option 'dir' must be specified.`); } @@ -38,8 +37,6 @@ export function getServer(config: Config, onGetResponse?: () => void) { request: IncomingMessage, response: ServerResponse, ): Promise { - onGetResponse?.(); - const pathname = request.url && request.headers.host && diff --git a/vitest.config.ts b/vitest.config.ts index bc3b46394..84f6c22cc 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -35,10 +35,10 @@ export default defineConfig({ thresholds: { autoUpdate: true, 'packages/cli/**': { - statements: 56.41, + statements: 63.23, functions: 64, - branches: 55, - lines: 57.89, + branches: 61.11, + lines: 65.15, }, 'packages/errors/**': { statements: 100, From 5300ff3c8a6f23f10e2d82287d0e1dbf4a7ebc07 Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Fri, 6 Dec 2024 12:51:56 -0600 Subject: [PATCH 14/14] remove unused makeTimeoutWithReset --- packages/cli/package.json | 1 - packages/cli/src/utils.ts | 24 ------------------------ vitest.config.ts | 6 +++--- yarn.lock | 1 - 4 files changed, 3 insertions(+), 29 deletions(-) diff --git a/packages/cli/package.json b/packages/cli/package.json index 8cce95edd..acd20a3ad 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -35,7 +35,6 @@ "dependencies": { "@endo/bundle-source": "^3.5.0", "@endo/init": "^1.1.6", - "@endo/promise-kit": "^1.1.6", "@metamask/snaps-utils": "^8.3.0", "@ocap/shims": "workspace:^", "@ocap/utils": "workspace:^", diff --git a/packages/cli/src/utils.ts b/packages/cli/src/utils.ts index 0d5b68cb6..6acbf95bc 100644 --- a/packages/cli/src/utils.ts +++ b/packages/cli/src/utils.ts @@ -1,5 +1,3 @@ -import { makePromiseKit } from '@endo/promise-kit'; - /** * Wrap a promise with a timeout rejection. * @@ -26,25 +24,3 @@ export async function withTimeout( ), ]) as Promise; } - -/** - * Make a promise which resolves after a timeout and a reset method which resets the timeout. - * - * @param timeout How many ms to wait before the timeout completes. - * @returns A reset method and a promise which resolves timeout ms after the last reset call. - */ -export function makeTimeoutWithReset(timeout: number): { - reset: () => void; - promise: Promise; -} { - const { promise, resolve } = makePromiseKit(); - let timeoutId = setTimeout(() => resolve(), timeout); - const reset = (): void => { - clearTimeout(timeoutId); - timeoutId = setTimeout(() => resolve(), timeout); - }; - return { - promise, - reset, - }; -} diff --git a/vitest.config.ts b/vitest.config.ts index 84f6c22cc..9e15f370a 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -35,10 +35,10 @@ export default defineConfig({ thresholds: { autoUpdate: true, 'packages/cli/**': { - statements: 63.23, - functions: 64, + statements: 71.66, + functions: 76.19, branches: 61.11, - lines: 65.15, + lines: 71.66, }, 'packages/errors/**': { statements: 100, diff --git a/yarn.lock b/yarn.lock index 49eaa7867..92ff3d15b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1606,7 +1606,6 @@ __metadata: "@arethetypeswrong/cli": "npm:^0.16.4" "@endo/bundle-source": "npm:^3.5.0" "@endo/init": "npm:^1.1.6" - "@endo/promise-kit": "npm:^1.1.6" "@metamask/auto-changelog": "npm:^3.4.4" "@metamask/eslint-config": "npm:^14.0.0" "@metamask/eslint-config-nodejs": "npm:^14.0.0"