From 28ebbb3f20453ba62aba3eaa159ef877a63e8c26 Mon Sep 17 00:00:00 2001 From: Zack Radisic <56137411+zackradisic@users.noreply.github.com> Date: Wed, 12 Mar 2025 14:16:03 -0700 Subject: [PATCH] Fix node:vm test (#18081) --- CONTRIBUTING.md | 9 ++ cmake/tools/SetupWebKit.cmake | 2 +- src/bun.js/bindings/NodeVM.cpp | 2 +- test/bundler/esbuild/default.test.ts | 2 +- test/bundler/expectBundled.ts | 2 +- test/exports/bun-exports.bun-v0.6.11.json | 2 +- test/exports/node-exports.bun-v0.6.11.json | 2 +- test/js/bun/net/socket.test.ts | 4 +- .../parallel/test-vm-new-script-context.js | 107 ++++++++++++++++++ test/napi/napi.test.ts | 76 +++++++------ 10 files changed, 164 insertions(+), 44 deletions(-) create mode 100644 test/js/node/test/parallel/test-vm-new-script-context.js diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9e0a74c415..b7e8130e2a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -213,10 +213,19 @@ $ make jsc-debug $ bun run build:local ``` +Using `bun run build:local` will build Bun in the `./build/debug-local` directory (instead of `./build/debug`), you'll have to change a couple of places to use this new directory: + +- The first line in [`src/js/builtins.d.ts`](/src/js/builtins.d.ts) +- The `CompilationDatabase` line in [`.clangd` config](/.clangd) should be `CompilationDatabase: build/debug-local` +- In [`build.zig`](/build.zig), the `codegen_path` option should be `build/debug-local/codegen` (instead of `build/debug/codegen`) +- In [`.vscode/launch.json`](/.vscode/launch.json), many configurations use `./build/debug/`, change them as you see fit + Note that the WebKit folder, including build artifacts, is 8GB+ in size. If you are using a JSC debug build and using VScode, make sure to run the `C/C++: Select a Configuration` command to configure intellisense to find the debug headers. +Note that if you change make changes to our [WebKit fork](https://github.com/oven-sh/WebKit), you will also have to change [`SetupWebKit.cmake`](/cmake/tools/SetupWebKit.cmake) to point to the commit hash. + ## Troubleshooting ### 'span' file not found on Ubuntu diff --git a/cmake/tools/SetupWebKit.cmake b/cmake/tools/SetupWebKit.cmake index f20e7270a1..ab4166fcdd 100644 --- a/cmake/tools/SetupWebKit.cmake +++ b/cmake/tools/SetupWebKit.cmake @@ -2,7 +2,7 @@ option(WEBKIT_VERSION "The version of WebKit to use") option(WEBKIT_LOCAL "If a local version of WebKit should be used instead of downloading") if(NOT WEBKIT_VERSION) - set(WEBKIT_VERSION abe5e808a649db1182333f65bef55e65bb374616) + set(WEBKIT_VERSION 2ffd4be9f9513c30333d9e23c175cab4760584e3) endif() string(SUBSTRING ${WEBKIT_VERSION} 0 16 WEBKIT_VERSION_PREFIX) diff --git a/src/bun.js/bindings/NodeVM.cpp b/src/bun.js/bindings/NodeVM.cpp index 34ec76d3eb..89a98009ed 100644 --- a/src/bun.js/bindings/NodeVM.cpp +++ b/src/bun.js/bindings/NodeVM.cpp @@ -607,7 +607,7 @@ JSC_DEFINE_HOST_FUNCTION(scriptRunInNewContext, (JSGlobalObject * globalObject, auto scope = DECLARE_THROW_SCOPE(vm); if (!script) { - throwTypeError(globalObject, scope, "Script.prototype.runInNewContext can only be called on a Script object"_s); + throwTypeError(globalObject, scope, "this.runInContext is not a function"_s); return {}; } diff --git a/test/bundler/esbuild/default.test.ts b/test/bundler/esbuild/default.test.ts index 785a5eca75..83a689eaa0 100644 --- a/test/bundler/esbuild/default.test.ts +++ b/test/bundler/esbuild/default.test.ts @@ -1835,7 +1835,7 @@ describe("bundler", () => { assert(!api.readFile("/out.js").includes("var bar"), 'bundle shouldnt include "var bar"'); }, run: { - error: "ReferenceError: Can't find variable: bar", + error: "ReferenceError: bar is not defined", }, }); itBundled("default/ArgumentDefaultValueScopeNoBundle", { diff --git a/test/bundler/expectBundled.ts b/test/bundler/expectBundled.ts index ba04cc93fb..f416fda26d 100644 --- a/test/bundler/expectBundled.ts +++ b/test/bundler/expectBundled.ts @@ -344,7 +344,7 @@ export interface BundlerTestRunOptions { stderr?: string; /** partial match stdout (toContain()) */ partialStdout?: string; - /** match exact error message, example "ReferenceError: Can't find variable: bar" */ + /** match exact error message, example "ReferenceError: bar is not defined" */ error?: string; /** * for extra confidence the error is correctly tested for, a regex for the line it was diff --git a/test/exports/bun-exports.bun-v0.6.11.json b/test/exports/bun-exports.bun-v0.6.11.json index 7a876ecdde..2b6dc4aa6b 100644 --- a/test/exports/bun-exports.bun-v0.6.11.json +++ b/test/exports/bun-exports.bun-v0.6.11.json @@ -8597,7 +8597,7 @@ "errors": { "node:wasi": { "name": "ReferenceError", - "message": "Can't find variable: constants" + "message": "constants is not defined" } } } diff --git a/test/exports/node-exports.bun-v0.6.11.json b/test/exports/node-exports.bun-v0.6.11.json index d324c88245..3ba74b3158 100644 --- a/test/exports/node-exports.bun-v0.6.11.json +++ b/test/exports/node-exports.bun-v0.6.11.json @@ -8563,7 +8563,7 @@ }, "node:wasi": { "name": "ReferenceError", - "message": "Can't find variable: constants" + "message": "constants is not defined" } } } diff --git a/test/js/bun/net/socket.test.ts b/test/js/bun/net/socket.test.ts index 693ce9e808..e136f31129 100644 --- a/test/js/bun/net/socket.test.ts +++ b/test/js/bun/net/socket.test.ts @@ -392,7 +392,7 @@ it("it should not crash when getting a ReferenceError on client socket open", as hostname: server.hostname, socket: { open(socket) { - // ReferenceError: Can't find variable: bytes + // ReferenceError: bytes is not defined // @ts-expect-error socket.write(bytes); }, @@ -409,7 +409,7 @@ it("it should not crash when getting a ReferenceError on client socket open", as }); const result: any = await promise; - expect(result?.message).toBe("Can't find variable: bytes"); + expect(result?.message).toBe("bytes is not defined"); } }); diff --git a/test/js/node/test/parallel/test-vm-new-script-context.js b/test/js/node/test/parallel/test-vm-new-script-context.js new file mode 100644 index 0000000000..e301185e98 --- /dev/null +++ b/test/js/node/test/parallel/test-vm-new-script-context.js @@ -0,0 +1,107 @@ +// Copyright Joyent, Inc. and other Node contributors. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and associated documentation files (the +// "Software"), to deal in the Software without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Software, and to permit +// persons to whom the Software is furnished to do so, subject to the +// following conditions: +// +// The above copyright notice and this permission notice shall be included +// in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN +// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE +// USE OR OTHER DEALINGS IN THE SOFTWARE. + +'use strict'; +require('../common'); + +const assert = require('assert'); + +const Script = require('vm').Script; + +{ + const script = new Script('\'passed\';'); + const result1 = script.runInNewContext(); + const result2 = script.runInNewContext(); + assert.strictEqual(result1, 'passed'); + assert.strictEqual(result2, 'passed'); +} + +{ + const script = new Script('throw new Error(\'test\');'); + assert.throws(() => { + script.runInNewContext(); + }, /^Error: test$/); +} + +{ + const script = new Script('foo.bar = 5;'); + assert.throws(() => { + script.runInNewContext(); + }, /^ReferenceError: foo is not defined$/); +} + +{ + global.hello = 5; + const script = new Script('hello = 2'); + script.runInNewContext(); + assert.strictEqual(global.hello, 5); + + // Cleanup + delete global.hello; +} + +{ + global.code = 'foo = 1;' + + 'bar = 2;' + + 'if (baz !== 3) throw new Error(\'test fail\');'; + global.foo = 2; + global.obj = { foo: 0, baz: 3 }; + const script = new Script(global.code); + /* eslint-disable no-unused-vars */ + const baz = script.runInNewContext(global.obj); + /* eslint-enable no-unused-vars */ + assert.strictEqual(global.obj.foo, 1); + assert.strictEqual(global.obj.bar, 2); + assert.strictEqual(global.foo, 2); + + // cleanup + delete global.code; + delete global.foo; + delete global.obj; +} + +{ + const script = new Script('f()'); + function changeFoo() { global.foo = 100; } + script.runInNewContext({ f: changeFoo }); + assert.strictEqual(global.foo, 100); + + // cleanup + delete global.foo; +} + +{ + const script = new Script('f.a = 2'); + const f = { a: 1 }; + script.runInNewContext({ f }); + assert.strictEqual(f.a, 2); + + assert.throws(() => { + script.runInNewContext(); + }, /^ReferenceError: f is not defined$/); +} + +{ + const script = new Script(''); + assert.throws(() => { + script.runInNewContext.call('\'hello\';'); + }, /^TypeError: this\.runInContext is not a function$/); +} \ No newline at end of file diff --git a/test/napi/napi.test.ts b/test/napi/napi.test.ts index 773e12efe1..f7920b5f91 100644 --- a/test/napi/napi.test.ts +++ b/test/napi/napi.test.ts @@ -62,44 +62,48 @@ describe("napi", () => { }); if (target === "bun") { - it("should work with --compile", async () => { - const dir = tempDirWithFiles("napi-app-compile-" + format, { - "package.json": JSON.stringify({ - name: "napi-app", - version: "1.0.0", - type: format === "esm" ? "module" : "commonjs", - }), - }); + it( + "should work with --compile", + async () => { + const dir = tempDirWithFiles("napi-app-compile-" + format, { + "package.json": JSON.stringify({ + name: "napi-app", + version: "1.0.0", + type: format === "esm" ? "module" : "commonjs", + }), + }); - const exe = join(dir, "main" + (process.platform === "win32" ? ".exe" : "")); - const build = spawnSync({ - cmd: [ - bunExe(), - "build", - "--target=" + target, - "--format=" + format, - "--compile", - join(__dirname, "napi-app", "main.js"), - ], - cwd: dir, - env: bunEnv, - stdout: "inherit", - stderr: "inherit", - }); - expect(build.success).toBeTrue(); + const exe = join(dir, "main" + (process.platform === "win32" ? ".exe" : "")); + const build = spawnSync({ + cmd: [ + bunExe(), + "build", + "--target=" + target, + "--format=" + format, + "--compile", + join(__dirname, "napi-app", "main.js"), + ], + cwd: dir, + env: bunEnv, + stdout: "inherit", + stderr: "inherit", + }); + expect(build.success).toBeTrue(); - const result = spawnSync({ - cmd: [exe, "self"], - env: bunEnv, - stdin: "inherit", - stderr: "inherit", - stdout: "pipe", - }); - const stdout = result.stdout.toString().trim(); + const result = spawnSync({ + cmd: [exe, "self"], + env: bunEnv, + stdin: "inherit", + stderr: "inherit", + stdout: "pipe", + }); + const stdout = result.stdout.toString().trim(); - expect(stdout).toBe("hello world!"); - expect(result.success).toBeTrue(); - }); + expect(stdout).toBe("hello world!"); + expect(result.success).toBeTrue(); + }, + 10 * 1000, + ); } it("`bun build`", async () => { @@ -283,7 +287,7 @@ describe("napi", () => { // remove all debug logs bunResult = bunResult.replaceAll(/^\[\w+\].+$/gm, "").trim(); expect(bunResult).toBe( - `synchronously threw ReferenceError: message "Can't find variable: shouldNotExist", code undefined`, + `synchronously threw ReferenceError: message "shouldNotExist is not defined", code undefined`, ); }); });