Skip to content

Test all packages#17629

Closed
parthea wants to merge 17 commits into
mainfrom
test-all-packages
Closed

Test all packages#17629
parthea wants to merge 17 commits into
mainfrom
test-all-packages

Conversation

@parthea

@parthea parthea commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> 🦕

parthea and others added 17 commits July 2, 2026 18:35
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

@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 introduces a new batching script ci/get_batches.py to group packages for parallelized testing, updates ci/run_conditional_tests.sh to support testing specific directories and force-testing all packages, and lowers the test coverage threshold in packages/django-google-spanner/noxfile.py. Feedback highlights that the coverage threshold should not be disabled (set to 0), the shebang in ci/get_batches.py needs to be moved to the first line to execute correctly, and a potential double-slash issue in the glob expansion of ci/run_conditional_tests.sh should be resolved.

"--cov-config=.coveragerc",
"--cov-report=",
"--cov-fail-under=75",
"--cov-fail-under=0",

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.

high

Lowering the coverage threshold to 0 disables coverage enforcement for this package. If this was done temporarily for testing or debugging, please revert it to the original value of 75 to ensure code coverage standards are maintained.

Suggested change
"--cov-fail-under=0",
"--cov-fail-under=75",

Comment thread ci/get_batches.py
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#!/usr/bin/env python3

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 shebang line #!/usr/bin/env python3 is currently placed on line 14, after the copyright header. For the operating system to recognize the shebang and execute the script using the Python interpreter when run directly (e.g., ./ci/get_batches.py), the shebang must be the very first line of the file (line 1), preceding even the copyright comments.

fi

if [[ "${subdir%/}" == "packages" ]]; then
loop_dirs=("${subdir}"/*/)

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 ${subdir} directly when it has a trailing slash (e.g., packages/) results in a double slash in the expanded glob (e.g., packages//google-cloud-storage/). Using ${subdir%/} strips any trailing slash and prevents double slashes.

Suggested change
loop_dirs=("${subdir}"/*/)
loop_dirs=("${subdir%/}"/*/)

@parthea parthea closed this Jul 6, 2026
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.

1 participant