chore: migrate core/common to jest#8746
Open
pearigee wants to merge 3 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Code Review
This pull request migrates the core/common package's test suite from Mocha, Sinon, and Node's assert module to Jest. The changes include updating configuration files, dependencies, and rewriting tests to use Jest assertions and mocking utilities. The review feedback highlights that the migration is incomplete in test/service-object.ts due to remaining assert imports and calls. Additionally, the reviewer points out an anti-pattern of executing expect assertions inside a hoisted jest.mock factory function, recommending capturing the options in a local variable to assert on them within the test block instead.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Migration to Jest & ts-jest
Successfully migrated the
@google-cloud/commontest suite from Mocha, Sinon, and c8 to Jest andts-jest. Removed legacy configuration files (.mocharc.js,.nycrc), addedcoverage/to.gitignore, and introducedjest.config.jsto compile and execute tests on TypeScript files directly.Key Unit Test Fixes & Isolation
ReferenceErrorwarnings due to Jest's top-level hoisting ofjest.mock().authClientobject (extend(true, {}, authClient, ...)) rather than mutating it directly.authorizeRequestfunction resolved toundefinedinstead of the request options. This originally caused aTypeErrorindecorateRequestthat was thrown asynchronously, bypassingdone()and leaking as a failure into subsequent tests.System Test Reliability & ESM Mocking
Note: System tests were broken on current versions of Node, this PR includes the following fixes:
8118,8119,8120) to mock servers insystem-test/common.tsto eliminateEADDRINUSEconflicts. Wrapped asynchronous callbacks in structuredtry...catchblocks to immediately close active servers and fail the test when assertions fail, preventing test runs from hanging.--experimental-vm-modulesCLI flag by mockinggaxiosandteeny-request(which dynamically importnode-fetchv3) at the top level of the system test file. The mocks redirect HTTP transport through Node's native globalfetchAPI, enabling standard CommonJS execution in Jest.