Repository navigation
chore: remove proxyquire-bun-shim.cjs and runner flags - #9646
danieljbruce wants to merge 6 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the Bun compatibility shim for proxyquire (proxyquire-bun-shim.cjs) and cleans up all associated configuration, flags, and preloading logic across the test runner (bin/run-test.cjs) and various package .mocharc.js files. I have no feedback to provide as there are no review comments and the changes are straightforward cleanups.
danieljbruce
left a comment
danieljbruce
left a comment
There was a problem hiding this comment.
This LGTM. Just an isolated removal of the Shim. All the other PRs can remove unnecessary references to the shim on top of this. I probably would have removed the references first, but it likely doesn't matter too much as long as we follow through with removing everything.
Nevermind. This PR actually is meant to be reviewed/merged last so this comment doesn't apply. LGTM then. |
|
My main concern with this PR stack is that it is creating exact local replicas of the proxyquire patch in different locations. I think it could be a better option to replace proxyquire with sinon completely. |
…translate (#9642) ## Description Replaces proxyquire with sinon and direct stubs across four packages. ## Impact Removes --proxyquire-shim from gax, googleapis-common, error-reporting, and translate unit tests. ## Changes - **Part 1 of 5 in stacked PR series**: **#9642** -> #9643 -> #9644 -> #9645 -> #9646 (b/570680750). - `core/packages/gax`: Replace `proxyquire` in `test/unit/grpc.ts` with `sinon.stub(fs, 'existsSync')` and remove `--proxyquire-shim` from `package.json`. - `core/packages/nodejs-googleapis-common`: Replace `proxyquire` in `test/test.authplus.ts` and `test/test.http2.ts` with `sinon.stub(auth.GoogleAuth.prototype, 'getClient')` and `sinon.stub(coreHttp2, 'connect')`, and remove `--proxyquire-shim` from `package.json`. - `handwritten/error-reporting`: Stub `util.makeAuthenticatedRequestFactory` and `RequestHandler.prototype.request` directly in `test/unit/google-apis/auth-client.ts` and remove `--proxyquire-shim` from `package.json`. - `packages/google-cloud-translate`: Intercept `Module.prototype.require` directly in `test/index.ts` `before()` hook and remove `--proxyquire-shim` from `package.json`. ## Testing - Executed `pnpm test` under both Node.js and Bun (`JS_RUNTIME=bun`) in `core/packages/gax`, `core/packages/nodejs-googleapis-common`, `handwritten/error-reporting`, and `packages/google-cloud-translate`. - Verified strict lint and type checks pass via `bin/linter.mjs --strict`. ## Alternatives - Not merging this pull request was considered, which would leave these four packages dependent on the global `bin/proxyquire-bun-shim.cjs` preload shim.
937810c to
e404d9a
Compare
…to fix/remove-proxyquire-bun-shim
…to fix/remove-proxyquire-bun-shim
…to fix/remove-proxyquire-bun-shim
|
[Generated] Per #9643 (comment), we opened #9759 ( We are waiting for #9759 to be merged and a new release of |
## Description Adds runtime-agnostic `createProxyquire` and `proxyquire` helpers to `google-test-utils`. ## Impact Releasing `createProxyquire` in `google-test-utils` avoids extra YAML flags across stacked PRs. ## Changes - Per #9643 (comment), extracts just the `createProxyquire` and `proxyquire` implementation in `core/test-utils` (`google-test-utils`) into a standalone PR. - We want to merge and release `google-test-utils` (`core/test-utils`) with `createProxyquire` first so that the rest of the stacked PRs (#9643, #9644, #9645, #9646) can consume the published `google-test-utils` package and do not need the extra `--filter "./core/test-utils" --ignore-scripts`, `pnpm --dir core/test-utils run compile`, `ensureWorkspacePackagesLinked()`, and `run prepare --if-present` flags in the `.yaml` files (`cloudbuild.yaml` and `.github/actions/pnpm-lockfile-check/action.yaml`). - `core/test-utils/src/proxyquire.ts`: Adds `createProxyquire`, `proxyquire`, `Proxyquire`, and `ProxyquireStubs` built on `Module.prototype.require` and `require.cache` with automatic caller path resolution (`Error.prepareStackTrace`) for Node.js and Bun (`JS_RUNTIME=bun`). - `core/test-utils/src/index.ts`: Exports `createProxyquire`, `proxyquire`, `Proxyquire`, and `ProxyquireStubs`. - `core/test-utils/test/proxyquire.test.ts`: Adds unit tests covering direct/transitive `@global` stubbing, `@noCallThru` / `noCallThru()` / `callThru()`, `noPreserveCache()` / `preserveCache()`, `null` stub `MODULE_NOT_FOUND` handling, and bare module stubs. ## Testing - Executed `pnpm --dir core/test-utils test` 5x on Node.js and `JS_RUNTIME=bun MOCHA_PARALLEL=false bun --bun run --cwd core/test-utils test` 5x on Bun ($\ge 95\%$ confidence, 25/25 unit tests passing). - Verified strict lint and TypeScript checks pass via `node ./bin/linter.mjs --strict` and `ci/run_conditional_tests.sh --strict`. ## Alternatives - Including `createProxyquire` directly inside #9643 alongside the `common`, `bigquery`, `datastore`, and `google-cloud-dns` migrations was considered, but because `.npmrc` sets `link-workspace-packages=false` and `enable-pre-post-scripts=true`, doing so before releasing `google-test-utils` requires extra `--filter "./core/test-utils" --ignore-scripts` and workspace-linking flags across the YAML files in the stack. - Not merging this PR would leave those temporary YAML workarounds in the stack PRs. --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…to fix/remove-proxyquire-bun-shim
…eAllConnections on Bun
Description
Removes bin/proxyquire-bun-shim.cjs and all remaining Mocha and runner shim configuration.
Impact
Eliminates the global Bun proxyquire preload shim from the test runner.
Changes
bin/proxyquire-bun-shim.cjs.--proxyquire-shim,BUN_ENABLE_PROXYQUIRE_SHIM, andproxyquire-bun-shim.cjspreloading frombin/run-test.cjs.proxyquire-bun-shim.cjsfrom.mocharc.cjsand all 11core/packages/*/.mocharc.jsconfiguration files.Testing
JS_RUNTIME=bun) withoutbin/proxyquire-bun-shim.cjs.bin/linter.mjs --strict.Alternatives
--proxyquire-shimrunner flags in the repository.