From 036ce6fe7d6719b3cfffa63684e97a5c92ddeb5c Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Wed, 30 Sep 2026 15:46:30 +0000 Subject: [PATCH 1/3] perf(tooling): instantiate array with length instead of Array.from in assert.deepStrictEqual shim --- bin/proxyquire-bun-shim.cjs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/proxyquire-bun-shim.cjs b/bin/proxyquire-bun-shim.cjs index b8061bf1588..287653cff87 100644 --- a/bin/proxyquire-bun-shim.cjs +++ b/bin/proxyquire-bun-shim.cjs @@ -486,7 +486,7 @@ if ( expected.constructor === Array ) { try { - const copyA = Array.from(actual); + const copyA = new Array(actual.length); for (const k of Reflect.ownKeys(actual)) { if (k !== 'length') { Object.defineProperty( @@ -496,7 +496,7 @@ if ( ); } } - const copyB = Array.from(expected); + const copyB = new Array(expected.length); for (const k of Reflect.ownKeys(expected)) { if (k !== 'length') { Object.defineProperty( From bf2edc45d0ac3299dfaa79747998f3a32c6c631c Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Wed, 30 Sep 2026 16:00:29 +0000 Subject: [PATCH 2/3] fix(tooling): coerce array length to uint32 via >>> 0 in assert.deepStrictEqual shim --- bin/proxyquire-bun-shim.cjs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/proxyquire-bun-shim.cjs b/bin/proxyquire-bun-shim.cjs index 287653cff87..66f1cf37b9f 100644 --- a/bin/proxyquire-bun-shim.cjs +++ b/bin/proxyquire-bun-shim.cjs @@ -486,7 +486,7 @@ if ( expected.constructor === Array ) { try { - const copyA = new Array(actual.length); + const copyA = new Array(actual.length >>> 0); for (const k of Reflect.ownKeys(actual)) { if (k !== 'length') { Object.defineProperty( @@ -496,7 +496,7 @@ if ( ); } } - const copyB = new Array(expected.length); + const copyB = new Array(expected.length >>> 0); for (const k of Reflect.ownKeys(expected)) { if (k !== 'length') { Object.defineProperty( From b2fa1151fd15dc49e72b0950d311fad4cd54a08b Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Wed, 30 Sep 2026 17:38:52 +0000 Subject: [PATCH 3/3] refactor(spanner): replace assert shim overrides with direct test updates --- bin/proxyquire-bun-shim.cjs | 179 +----------------- design.md | 110 ----------- handwritten/spanner/test/codec.ts | 2 +- handwritten/spanner/test/helper.ts | 2 +- .../spanner/test/partial-result-stream.ts | 10 +- 5 files changed, 18 insertions(+), 285 deletions(-) delete mode 100644 design.md diff --git a/bin/proxyquire-bun-shim.cjs b/bin/proxyquire-bun-shim.cjs index 66f1cf37b9f..9fd8d4a6ce8 100644 --- a/bin/proxyquire-bun-shim.cjs +++ b/bin/proxyquire-bun-shim.cjs @@ -355,180 +355,17 @@ if ( try { const assert = require('assert'); const origDeepEqual = assert.deepEqual; - function looseDeepEqual(a, b) { - if (a === b) return true; - // In Node.js, assert.deepEqual performs abstract equality (==) on primitives - // (e.g. assert.deepEqual('1', 1) passes). In Bun, native assert.deepEqual('1', 1) - // throws. Allow loose primitive equality only when both operands are non-null - // primitives to avoid false positives like null == undefined or [] == false. - if ( - typeof a !== 'object' && - typeof b !== 'object' && - a !== null && - b !== null - ) { - return a == b; - } - if ( - a === null || - b === null || - typeof a !== 'object' || - typeof b !== 'object' - ) { - return false; - } - if (a instanceof Date || b instanceof Date) { - return ( - a instanceof Date && - b instanceof Date && - a.getTime() === b.getTime() - ); - } - if (a instanceof RegExp || b instanceof RegExp) { - return ( - a instanceof RegExp && - b instanceof RegExp && - a.toString() === b.toString() - ); - } - if ( - typeof Buffer !== 'undefined' && - (Buffer.isBuffer(a) || Buffer.isBuffer(b)) - ) { - return Buffer.isBuffer(a) && Buffer.isBuffer(b) && a.equals(b); - } - if (a instanceof Map || b instanceof Map) { - return ( - a instanceof Map && - b instanceof Map && - a.size === b.size && - looseDeepEqual(Array.from(a.entries()), Array.from(b.entries())) - ); - } - if (a instanceof Set || b instanceof Set) { - return ( - a instanceof Set && - b instanceof Set && - a.size === b.size && - looseDeepEqual(Array.from(a.values()), Array.from(b.values())) - ); - } - if (Array.isArray(a) !== Array.isArray(b)) return false; - const keysA = Object.keys(a); - const keysB = Object.keys(b); - if (keysA.length !== keysB.length) return false; - for (const k of keysA) { - if (!Object.prototype.hasOwnProperty.call(b, k)) return false; - if (!looseDeepEqual(a[k], b[k])) return false; - } - return true; - } - if (typeof origDeepEqual === 'function') { + if (typeof origDeepEqual === 'function' && typeof Headers !== 'undefined') { assert.deepEqual = function (actual, expected, message) { - if ( - typeof Headers !== 'undefined' && - actual instanceof Headers && - expected instanceof Headers - ) { - actual = Object.fromEntries(actual.entries()); - expected = Object.fromEntries(expected.entries()); - } - try { - return origDeepEqual.call(this, actual, expected, message); - } catch (err) { - if (looseDeepEqual(actual, expected)) return; - throw err; - } - }; - } - const origDeepStrictEqual = assert.deepStrictEqual; - if (typeof origDeepStrictEqual === 'function') { - // Bun <= 1.4 strictly requires prototype reference equality in - // assert.deepStrictEqual. In Node 20+, Array subclasses whose constructor - // is Array (such as RowImpl) are compared by contents and properties against - // plain arrays. In affected Bun versions, retry only for arrays where both - // constructors are Array, transferring own properties so custom prototypes - // or property mismatches continue to fail strictly. - const [bunMajor, bunMinor] = ( - (typeof process !== 'undefined' && process.versions?.bun) || - '0.0' - ) - .split('.') - .map(Number); - const hasBunArrayProtoIssue = - bunMajor === 1 && - bunMinor <= 4 && - (() => { - try { - class TestArr extends Array {} - Object.defineProperty(TestArr.prototype, 'constructor', { - value: Array, - writable: true, - configurable: true, - enumerable: false, - }); - origDeepStrictEqual(new TestArr(), []); - return false; - } catch { - return true; - } - })(); - - assert.deepStrictEqual = function (actual, expected, message) { - try { - return origDeepStrictEqual.call(this, actual, expected, message); - } catch (err) { - if ( - hasBunArrayProtoIssue && - Array.isArray(actual) && - Array.isArray(expected) && - actual.constructor === Array && - expected.constructor === Array - ) { - try { - const copyA = new Array(actual.length >>> 0); - for (const k of Reflect.ownKeys(actual)) { - if (k !== 'length') { - Object.defineProperty( - copyA, - k, - Object.getOwnPropertyDescriptor(actual, k), - ); - } - } - const copyB = new Array(expected.length >>> 0); - for (const k of Reflect.ownKeys(expected)) { - if (k !== 'length') { - Object.defineProperty( - copyB, - k, - Object.getOwnPropertyDescriptor(expected, k), - ); - } - } - return origDeepStrictEqual.call(this, copyA, copyB, message); - } catch { - throw err; - } - } - throw err; - } - }; - } - const origThrows = assert.throws; - if (typeof origThrows === 'function') { - assert.throws = function (block, error, message) { - if ( - error instanceof RegExp && - /Cannot assign to read only property/.test(error.source) - ) { - const adapted = new RegExp( - '(?:' + error.source + '|Attempted to assign to readonly property)', - error.flags, + if (actual instanceof Headers && expected instanceof Headers) { + return origDeepEqual.call( + this, + Object.fromEntries(actual.entries()), + Object.fromEntries(expected.entries()), + message, ); - return origThrows.call(this, block, adapted, message); } - return origThrows.call(this, block, error, message); + return origDeepEqual.call(this, actual, expected, message); }; } } catch { diff --git a/design.md b/design.md deleted file mode 100644 index 8e08ba10b90..00000000000 --- a/design.md +++ /dev/null @@ -1,110 +0,0 @@ -# Design Doc: Opt-In Bun Fetch Transport for `@google-cloud/storage` - -**Context:** [PR #9458 (bun-runtime/1-test-runner-handwritten-libraries-4)](https://github.com/googleapis/google-cloud-node/pull/9458) -**Target:** `@google-cloud/storage`, `gaxios`, `teeny-request` - ---- - -## Objective - -Provide an opt-in HTTP fetch transport shim in the Bun test runner so `@google-cloud/storage` unit tests can use legacy `nock` mocks while system tests run directly against live Google Cloud endpoints using Bun's native `fetch`. - ---- - -## Background - -`@google-cloud/storage` is an HTTP/REST client that executes network operations via `gaxios` and `teeny-request`. In Bun, these libraries default to `globalThis.fetch`, a native C++/Zig implementation that connects directly to OS sockets and bypasses Node's `http` and `https` modules. - -Because our unit test suites rely on `nock` (which monkeypatches Node's `http.ClientRequest`), running Storage unit tests under Bun bypasses `nock` entirely, triggering connection timeouts (`ETIMEOUT fake.local:80`), authentication errors (`invalid_grant`), and failed mock assertions. Conversely, system tests run against live Google Cloud endpoints, where customer applications in production will execute Bun's native `fetch` without shims. Masking native `fetch` during system tests would prevent us from verifying critical production behaviors like chunked streaming uploads, download streams, and TLS negotiation. - ---- - -## Overview - -The test runner will run unshimmed using Bun's native `globalThis.fetch` by default. We introduce an explicit `--fetch-shim` flag in `bin/run-test.cjs` that activates `__googleCloudBunFetch` to route `fetch` calls through Node's `http.request` stack specifically for test suites requiring `nock`. This allows Storage unit tests to pass without rewriting legacy mocks, while ensuring system tests validate authentic Bun native `fetch` execution against live services. - ---- - -## Detailed Design - -### 1. Test Runner Opt-In Flag (`bin/run-test.cjs`) - -`bin/run-test.cjs` inspects CLI arguments for `--fetch-shim` (or `BUN_FETCH_SHIM=true`). By default, the fetch shim is **disabled**: - -```javascript -// bin/run-test.cjs -const enableFetchShim = rawArgs.includes('--fetch-shim') || process.env.BUN_FETCH_SHIM === 'true'; -const args = rawArgs.filter(a => a !== '--fetch-shim'); - -if (wantsBunRuntime) { - process.env.BUN_ENABLE_FETCH_SHIM = enableFetchShim ? 'true' : 'false'; - // ... -} -``` - -### 2. Transport Shim Gating (`bin/proxyquire-bun-shim.cjs`) - -Module cache snapshots (Sections 1 & 2) and `proxyquire` emulation (Section 4) remain active unconditionally for module stubbing. Section 3 (`__googleCloudBunFetch`) is gated behind `BUN_ENABLE_FETCH_SHIM`: - -```javascript -// bin/proxyquire-bun-shim.cjs -const enableFetchShim = process.env.BUN_ENABLE_FETCH_SHIM === 'true'; - -if (enableFetchShim) { - // --------------------------------------------------------------------------- - // 3. Nock-Compatible HTTP/HTTPS Fetch Transport (__googleCloudBunFetch) - // --------------------------------------------------------------------------- - globalThis.__googleCloudBunFetch = async (url, init = {}) => { ... }; - - // Register Bun.plugin for ESM gaxios, Module._extensions for teeny-request, - // and hook Gaxios.prototype._defaultAdapter -} -``` - -### 3. Package Script Updates (`handwritten/storage/package.json`) - -Unit tests opt into the shim; system tests run untouched: - -```json -{ - "scripts": { - "test": "node ../../bin/run-test.cjs --fetch-shim build/cjs/test", - "system-test": "node ../../bin/run-test.cjs build/cjs/system-test --timeout 600000 --exit" - } -} -``` - -### 4. Storage System Test Gotcha: Kokoro GCE Metadata Mock - -`handwritten/storage/system-test/storage.ts` currently uses `nock` to block connections to `http://metadata.google.internal` so Kokoro GCE workers do not use the host VM's service account. Because system tests will run without the fetch shim, native `fetch` will bypass this mock. - -**Resolution:** Replace the `nock` call with runtime environment variables: - -```typescript -// handwritten/storage/system-test/storage.ts -process.env.GCE_METADATA_HOST = '169.254.169.254.invalid'; -process.env.DETECT_GCP_RETRIES = '0'; -``` - ---- - -## Comparison Matrix - -| Aspect | Unit Tests (`pnpm test`) | System Tests (`pnpm system-test`) | -| :--- | :--- | :--- | -| **Command** | `run-test.cjs --fetch-shim` | `run-test.cjs` | -| **HTTP Transport** | `http.request` / `https.request` bridge | Native Bun `globalThis.fetch` | -| **Nock Compatibility** | Supported (`nock` intercepts traffic) | Unsupported (not used in real system tests) | -| **Production Parity** | Tests library logic & Node compatibility | Tests live Bun C++ networking against GCP | - ---- - -## Alternatives Considered - -1. **Unconditional Fetch Shim (PR #9458 as written)**: Route all Bun HTTP calls through `http.request`. - *Rejected*: Completely masks Bun's native `fetch` in system tests, leaving production networking behavior unverified. -2. **Opt-Out Flag (`--no-fetch-shim`)**: Enable the shim by default and pass an opt-out flag in system tests. - *Rejected*: Violates the principle of being native and safe by default; forces system test commands to carry negative flags. -3. **Rewrite Unit Tests to MSW / Fetch Mocks**: Replace `nock` across all test files with a fetch-compatible interceptor. - *Rejected*: High implementation cost; requires touching hundreds of test files across multiple repositories. - diff --git a/handwritten/spanner/test/codec.ts b/handwritten/spanner/test/codec.ts index 43b3e5a2ac5..733aff5cbc2 100644 --- a/handwritten/spanner/test/codec.ts +++ b/handwritten/spanner/test/codec.ts @@ -990,7 +990,7 @@ describe('codec', () => { value: music.Genre.JAZZ, fullName: 'examples.spanner.music.Genre', }).toJSON(), - 1, + '1', ); }); }); diff --git a/handwritten/spanner/test/helper.ts b/handwritten/spanner/test/helper.ts index 668df900db9..30d04e189f7 100644 --- a/handwritten/spanner/test/helper.ts +++ b/handwritten/spanner/test/helper.ts @@ -125,7 +125,7 @@ describe('helper', () => { assert.throws(() => { replaceProjectIdToken(frozenObj, projectId); - }, /Cannot assign to read only property/); + }, /Cannot assign to read only property|Attempted to assign to readonly property/); }); it('should replace more than one {{projectId}}', () => { diff --git a/handwritten/spanner/test/partial-result-stream.ts b/handwritten/spanner/test/partial-result-stream.ts index 032f4e1db3d..dc1938c2057 100644 --- a/handwritten/spanner/test/partial-result-stream.ts +++ b/handwritten/spanner/test/partial-result-stream.ts @@ -196,7 +196,10 @@ describe('PartialResultStream', () => { // Node 18's assert.deepStrictEqual strictly requires prototype equality, // which fails when comparing RowImpl (an Array subclass) with a plain Array literal. // Node 20+ relaxed this for Array subclasses with constructor = Array. - if (parseInt(process.versions.node.split('.')[0], 10) < 20) { + if ( + parseInt(process.versions.node.split('.')[0], 10) < 20 || + process.versions.bun + ) { assert.deepStrictEqual([...row], EXPECTED_ROW); } else { assert.deepStrictEqual(row, EXPECTED_ROW); @@ -260,7 +263,10 @@ describe('PartialResultStream', () => { // Node 18's assert.deepStrictEqual strictly requires prototype equality, // which fails when comparing RowImpl (an Array subclass) with a plain Array literal. // Node 20+ relaxed this for Array subclasses with constructor = Array. - if (parseInt(process.versions.node.split('.')[0], 10) < 20) { + if ( + parseInt(process.versions.node.split('.')[0], 10) < 20 || + process.versions.bun + ) { assert.deepStrictEqual([...row], EXPECTED_ROW); } else { assert.deepStrictEqual(row, EXPECTED_ROW);