feat(ci): improve test performance, fix failing tests by bshaffer · Pull Request #9021 · googleapis/google-cloud-node

bshaffer · 2026-07-29T21:43:16Z

Conditional Test Sharding: Dynamically shards tests when a large number of packages are modified. Adds a job that rolls up shard results to satisfy branch protection. For what this looks like with just ONE package modified, see chore: test sharding with one package #9027
Pipeline Speed: Reduces GitHub Action checkout fetch-depth to 2, which saves ~4 minutes for each shard
Clean CI Logs: Switches Mocha to use dot reporter and removes accidental console.log from the Firestore conformance tests.
Disambiguous Workflow Names: Renames conformance-test and conformance to bigtable-conformance and storage-conformance respectively.
Test Fixes:
1. Increases the pack-n-play test timeout to accommodate npm install
2. unwraps native fetch errors in gcp-metadata to suppress ECONNREFUSED warnings
3. replaces url.parse() with new URL() in nodejs-googleapis-common to fix Mocha test crashes
4. fixes race-condition in bigtable client-side exporter.
Confirm that it's okay to skip pack-and-play unit tests on Windows (as these alone add 5-10 minutes).

bshaffer · 2026-07-29T21:43:42Z

+    # echo "skipping trigger of tests for now: tracking in #7540"
+    echo "change detected in ci, we should test everything"
+    echo "result of git diff ${GIT_DIFF_ARG} ci:"
+    git diff ${GIT_DIFF_ARG} ci
+    GIT_DIFF_ARG=""


As tests now take ~30 minutes (down from 2 hours), we can reenable this and close #7540.

Once we add PNPM workspaces and turbo caching, this number goes down to ~15!!

gemini-code-assist

Code Review

This pull request parallelizes test execution and adds sharding support in the CI conditional test runner, configures the Mocha reporter and silences pnpm installation in the single test runner, and updates mock discovery URLs in the googleapis-common tests. A critical syntax error was identified in the conditional test runner script, where leftover loop and conditional control structures will cause the Bash script to fail.

This reverts commit 37e6bcf.

…re test output

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

bshaffer · 2026-07-30T15:40:56Z

@gemini-code-assist review

gemini-code-assist

Code Review

This pull request introduces test sharding and dynamic matrix generation for GitHub Actions in the CI scripts, updates pnpm install flags, and transitions from the deprecated url.parse to the standard URL API. Additionally, it increases test timeouts, skips certain tests on Windows, handles nested error causes in gcp-metadata, and ensures startTime does not exceed endTime in Bigtable metrics. Feedback focuses on improving shell script robustness by quoting variables (BUILD_TYPE, d, and test_script) to prevent syntax or word-splitting issues, and avoiding direct mutation of the options parameter in the OpenTelemetry test stub.

westarle

A few comments, I think this is a really great idea!

westarle · 2026-07-30T17:00:29Z

+
+    if [[ -n "${SHARD_TOTAL}" && -n "${SHARD_INDEX}" ]]; then
+        if (( SHARD_TOTAL > 0 && i % SHARD_TOTAL != SHARD_INDEX )); then
+            continue


We should just keep an eye on whether there are differences between the shard durations (in case multiple "heavy" targets land in a single shard consistently.)

Yes, I've already seen this happen that Shard 0 is a little heavier, probably because of the bigtable and pack-n-play tests. If a big discrepancy emerges (right now, it seems like just a few minutes, so I am not sure it's a big concern), we could take the known longer tests and manually separate them? What are your thoughts on the best way to accomplish this?

we could take the known longer tests and manually separate them

This it what I'd do.

right now, it seems like just a few minutes, so I am not sure it's a big concern

+1

westarle · 2026-07-30T17:03:23Z

-    # Then fetch enough history for finding the common commit.
-    git fetch origin main --deepen=300
+        # Then fetch enough history for finding the common commit.
+        git fetch origin main --deepen=300


Can we have the action fetch the right level of detail? I think we do 2 levels in setup then immediately fill in 300 more.

My understanding is that 2 is sufficient for github actions, but 300 might be needed for kokoro (which is why the conditional here exists). The 300 is just supposed to be a sufficiently large number to ensure a common commit ancestor is found.

So I guess Im not sure what your suggestion is here.

Thanks for handling the newbie questions Brent. TIL about actions/checkout synthetic commits (and also I missed the if [ -z... at the top of this diff).

I'd like all these scripts to be hermetic, and have things work like your actions work (operator fetches the right depth and passes the GIT_DIFF_ARG). Today, if I set GIT_DIFF_ARG but fail to fetch it fails, and if I don't set GIT_DIFF_ARG it can waste some time if I already fetched what I know I need.

Here's a practical fix for now:

Add a new "strict" flag or variable --strict/STRICT to the script and pass it from the action

If "strict" is set, GIT_DIFF_ARG must be set and git diff --quiet ${GIT_DIFF_ARG} must return 0 or 1.

The test on line 34 becomes "if not strict".

Then we can visit GCB builds later to the model you're introducing for actions here.

westarle · 2026-07-30T17:17:27Z

could this be in another PR?

The bigtable unit tests were failing, so I added these changes to fix them... however I'm not 100% sure THIS specific file update is needed (I suspect it is). I can verify!

westarle · 2026-07-30T17:18:48Z

+              run_id: context.runId,
+            });
+            const nodeVersion = '${{ matrix.node-version }}';
+            const shardJobs = jobs.filter(j => j.name.includes(`units (Node ${nodeVersion},`));


can we reuse test_dirs from setup?

westarle · 2026-07-30T17:21:35Z

-  it('should run tests', async () => {
+  it('should run tests', async function () {
+    this.timeout(120000); // 2 minutes
+    if (process.platform === 'win32') {


I'd be more comfortable doing this after we set up a postsubmit for the coverage. It seems like Windows would cause more problems in packaging...

honestly after testing, this did not affect performance that much, and windows STILL took 30-40 minutes, so I think removing it is the right call

westarle · 2026-07-30T17:35:54Z

-echo "pnpm install --ignore-scripts --engine-strict --prod --pnpmfile \"${PNPMFILE_PATH}\"; pnpm install --pnpmfile \"${PNPMFILE_PATH}\""
-pnpm install --ignore-scripts --engine-strict --prod --pnpmfile "${PNPMFILE_PATH}"; pnpm install --pnpmfile "${PNPMFILE_PATH}"
+echo "pnpm install --reporter=silent --engine-strict --pnpmfile \"${PNPMFILE_PATH}\""
+pnpm install --reporter=silent --engine-strict --pnpmfile "${PNPMFILE_PATH}"


Are errors still reported with --reporter=silent?

westarle

Also, would you be OK with breaking the test fixes out to their own little PRs?

bshaffer added 6 commits July 29, 2026 21:42

feat(ci): run unit-tests in parallel to speed them up

5da7791

chore(ci): reduce unit-test output due to log truncation

0d9eb53

chore(ci): rename conformance tests for clarity

6a14237

chore: run all ci tests

740552b

fix(ci): failing unit-tests

3d37a2c

feat(ci): add matrix sharding

24763d2

bshaffer commented Jul 29, 2026

View reviewed changes

gemini-code-assist Bot reviewed Jul 29, 2026

View reviewed changes

Comment thread ci/run_conditional_tests.sh Outdated

bshaffer added 7 commits July 29, 2026 21:55

fix bash syntax error, better job names

0c5f19a

reduce fetch-depth for github actions runs, remove concurrency

5b0a4a4

remove c8 text output

37e6bcf

feat: add conditional sharding using DRY_RUN_SHARDS envvar

dd49119

fix(ci): common and pack-and-play tests

90754d4

Revert "remove c8 text output" - we will handle this in a future PR

e42dd27

This reverts commit 37e6bcf.

add parent job for unit test shards (by node version), remove firesto…

f12544e

…re test output

This was referenced Jul 29, 2026

chore: run all ci tests #9017

Closed

chore: run all system and conformance tests #9022

Draft

bshaffer added 3 commits July 30, 2026 13:39

add dummy shards and extend test timeout for windows

9d358d3

improve bigtable unit-test performance

127374f

increase timeout to 10 minutes for windows tests

9ebd7db

bshaffer marked this pull request as ready for review July 30, 2026 15:12

bshaffer requested review from a team as code owners July 30, 2026 15:12

bshaffer changed the title ~~feat(ci): improve test performance~~ feat(ci): improve test performance, fix failing tests Jul 30, 2026

bshaffer added 2 commits July 30, 2026 15:14

fix bigtable-system-tests

ce4850e

skip pack-n-play tests on windows

cb703c9

bshaffer mentioned this pull request Jul 30, 2026

chore: test sharding with one package #9027

Closed

gcf-owl-bot Bot and others added 2 commits July 30, 2026 15:26

🦉 Updates from OwlBot post-processor

da39ebe

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

suggestions from gemini review

d01121e

gemini-code-assist Bot reviewed Jul 30, 2026

View reviewed changes

Comment thread ci/run_conditional_tests.sh Outdated

Comment thread ci/run_conditional_tests.sh Outdated

Comment thread handwritten/bigtable/test/metrics-collector/gcp-metrics-handler.ts

bshaffer added 2 commits July 30, 2026 16:11

address another round of gemini code review

b4d53f1

chore: pin actions/github-script to v8 SHA

4063b52

westarle reviewed Jul 30, 2026

View reviewed changes

Uh oh!

Conversation

bshaffer commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bshaffer Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

bshaffer commented Jul 30, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

westarle left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bshaffer Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

westarle left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bshaffer commented Jul 29, 2026 •

edited

Loading

bshaffer Jul 29, 2026 •

edited

Loading

bshaffer Jul 30, 2026 •

edited

Loading