Test all packages#17629
Conversation
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>
There was a problem hiding this comment.
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", |
There was a problem hiding this comment.
| # 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 |
There was a problem hiding this comment.
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}"/*/) |
There was a problem hiding this comment.
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.
| loop_dirs=("${subdir}"/*/) | |
| loop_dirs=("${subdir%/}"/*/) |
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:
Fixes #<issue_number_goes_here> 🦕