Skip to content

chore(pubsub): factor out the pub/sub pieces to use the test-utils library - #9339

Open
feywind wants to merge 4 commits into
googleapis:mainfrom
feywind:reporx/test-resources-pubsub
Open

feywind wants to merge 4 commits into
googleapis:mainfrom
feywind:reporx/test-resources-pubsub

Conversation

@feywind

@feywind feywind commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

After adding the test-utils library, this PR factors it out of the Pub/Sub library so that it's properly shared.

This currently contains the commits of #9229 - once that's merged, this one will get updated.

b/554057686

@feywind
feywind requested a review from a team as a code owner September 15, 2026 16:29
@github-actions
github-actions Bot requested a review from shivanee-p September 15, 2026 16:29
@feywind feywind changed the title Reporx/test resources pubsub chore(pubsub): factor out the pub/sub pieces to use the test-utils library Sep 15, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request extracts and centralizes testing utilities into a new core/test-utils package, updating the handwritten/pubsub package to use it and removing duplicate local files. Feedback on the changes suggests simplifying the shortUUID implementation with slice to avoid unnecessary array allocations, resolving a missing .jsdoc.js configuration file referenced in the docs script, and using recursive wildcards in tsconfig.json to ensure nested files are compiled.

Comment on lines +19 to 21
function shortUUID(): string {
return crypto.randomUUID().split('-').shift()!;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The shortUUID function can be simplified and made more efficient by using slice instead of splitting the UUID string into an array and shifting it. This avoids unnecessary array allocation and the non-null assertion operator.

Suggested change
function shortUUID(): string {
return crypto.randomUUID().split('-').shift()!;
}
function shortUUID(): string {
return crypto.randomUUID().slice(0, 8);
}

"lint": "gts check",
"prepare": "npm run compile",
"pretest": "npm run compile",
"docs": "jsdoc -c .jsdoc.js",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The docs script references .jsdoc.js, but this configuration file is not present in the core/test-utils directory. Running npm run docs will fail. Please add the missing .jsdoc.js file or update the script to use compodoc (since .compodocrc is present).

Comment on lines +8 to +9
"src/*.ts",
"test/*.ts"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using flat wildcards like src/*.ts and test/*.ts only includes files directly in those directories. To ensure any nested files or subdirectories created in the future are also compiled and tested, use recursive wildcards.

Suggested change
"src/*.ts",
"test/*.ts"
"src/**/*.ts",
"test/**/*.ts"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants