Repository navigation
chore: remove proxyquire-bun-shim.cjs and runner flags - #9646
danieljbruce wants to merge 3 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
This branch has not been deployed
1 participant
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.