Skip to content

chore(test): improve diff detection for unit tests - #17998

Open
daniel-sanche wants to merge 8 commits into
mainfrom
fix-coverage-sharding-git-diff
Open

chore(test): improve diff detection for unit tests#17998
daniel-sanche wants to merge 8 commits into
mainfrom
fix-coverage-sharding-git-diff

Conversation

@daniel-sanche

Copy link
Copy Markdown
Contributor

The diff logic in the new unit test sharding system had an issue: when comparing against main to find the modified packages, any packages that changed on main would be treated as packages that changed in the PR

This could especially cause issues if a commit was added while a workflow was running, because the changed package list would shift mid-run

This PR fixes the issue by:

  • Uses ... and a depth of 200 to find the common ancestor commit to diff against, instead of diffing against main directly
  • To avoid repeated large fetches, calculates the package list once and passes the list of modified packages through the workflow

daniel-sanche and others added 4 commits August 4, 2026 14:57
Adds a **Related Libraries** table to the `google-cloud-bigquery`
[README.rst](file:///usr/local/google/home/shuowei/src/google-cloud-python/google-cloud-python/packages/google-cloud-bigquery/README.rst)
linking connector libraries for DataFrame operations:
- **BigQuery DataFrames (`bigframes`)**: pandas/scikit-learn-like API
powered by BigQuery engine.
- **`pandas-gbq`**: integration for loading and writing pandas
DataFrames.

Fixes #<522853190> 🦕
@daniel-sanche
daniel-sanche requested a review from a team as a code owner August 4, 2026 23:01

@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 updates CI scripts to use triple-dot diffs when comparing against the target branch in presubmits, adds support for a pre-determined package list in coverage reporting, and outputs a space-separated list of packages to GitHub Actions output. Feedback was provided to replace --deepen=200 with --depth=200 in ci/run_conditional_tests.sh to avoid potential failures on non-shallow repositories.

Comment thread ci/run_conditional_tests.sh Outdated
daniel-sanche and others added 3 commits August 4, 2026 16:04
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@parthea parthea added the unit_test:all_packages debug label used to trigger unit tests against all packages in a PR label Aug 5, 2026

@parthea parthea 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.

LGTM, but holding off on formal approval until the temporary changes are removed.

Also added unit_test:all_packages to start a thorough test

@parthea
parthea marked this pull request as draft August 5, 2026 13:21
@parthea

parthea commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Please mark this ready for review once the temporary changes are removed

executed the first time *any* client library is instantiated in your application, and only if the affected loggers have not been previously configured.
(This is the reason for 2.i. above.)

.. test 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.

Remove temporary changes

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done!

@parthea

parthea commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

LGTM, but holding off on formal approval until the temporary changes are removed.

Also added unit_test:all_packages to start a thorough test

Following up on this, the results look good.

@daniel-sanche
daniel-sanche marked this pull request as ready for review August 5, 2026 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

unit_test:all_packages debug label used to trigger unit tests against all packages in a PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants