Skip to content

[WIP] chore(bigframes): add specific build script for doctest to restrict execution #16711

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
chalmerlowe wants to merge 10 commits into
base: main
Choose a base branch
from
+63 −5
Draft

[WIP] chore(bigframes): add specific build script for doctest to restrict execution #16711

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
596ce5c
feat(bigframes): add specific build script for doctest to control exe…
chalmerlowe Apr 17, 2026
397fdd8
fix(bigframes): apply reviewer feedback to run_doctest.sh
chalmerlowe Apr 17, 2026
0b369bc
feat(bigframes): enable doctest session by removing skip
chalmerlowe Apr 17, 2026
4f1854c
fix(bigframes): add credential setup and separate nox sessions in run…
chalmerlowe Apr 17, 2026
4c593a8
fix(bigframes): make cleanup non-fatal and use professional language …
chalmerlowe Apr 17, 2026
7d63adb
fix(bigframes): ignore dtype in divmod tests due to pandas instability
chalmerlowe Apr 17, 2026
b94c5bb
testing an element of the configuration
chalmerlowe Apr 20, 2026
c23c949
Adds comment to noxfile to confirm non-activation of non-bigframes pa…
chalmerlowe Apr 20, 2026
637b4af
Remove comments from two lines in script.
chalmerlowe Apr 21, 2026
b60e703
updates linting
chalmerlowe Apr 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .kokoro/presubmit/presubmit-doctest-bigframes.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,10 @@ env_vars: {
env_vars: {
key: "GOOGLE_CLOUD_PROJECT"
value: "bigframes-testing"
}

# Override the build file to only run for bigframes.
env_vars: {
key: "TRAMPOLINE_BUILD_FILE"
value: "github/google-cloud-python/packages/bigframes/scripts/run_doctest.sh"
}
42 changes: 42 additions & 0 deletions packages/bigframes/scripts/run_doctest.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
#!/bin/bash
# `-e` enables the script to automatically fail when a command fails
# `-o pipefail` sets the exit code to non-zero if any command fails,
# or zero if all commands in the pipeline exit successfully.
set -eo pipefail

# Disable buffering, so that the logs stream through.
export PYTHONUNBUFFERED=1

# Assume we are running from the repo root or we need to find it.
# If this script is in packages/bigframes/scripts/run_doctest.sh,
# then repo root is 3 levels up.
export PROJECT_ROOT=$(realpath $(dirname "${BASH_SOURCE[0]}")/../../..)
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 17, 2026

Choose a reason for hiding this comment

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

medium

The argument to realpath should be quoted to correctly handle directory paths that may contain spaces.

Suggested change
export PROJECT_ROOT=$(realpath $(dirname "${BASH_SOURCE[0]}")/../../..)
export PROJECT_ROOT=$(realpath "$(dirname "${BASH_SOURCE[0]}")/../../..")

cd "$PROJECT_ROOT"

# This is needed in order for `git diff` to succeed
git config --global --add safe.directory $(realpath .)
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 17, 2026

Choose a reason for hiding this comment

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

medium

The output of realpath . should be quoted to ensure the command works correctly if the current directory path contains spaces.

Suggested change
git config --global --add safe.directory $(realpath .)
git config --global --add safe.directory "$(realpath .)"


package_name="bigframes"
package_path="packages/${package_name}"

# Determine if we should skip based on git diff
files_to_check="${package_path}"

echo "checking changes with 'git diff ${KOKORO_GITHUB_PULL_REQUEST_TARGET_BRANCH}...${KOKORO_GITHUB_PULL_REQUEST_COMMIT} -- ${files_to_check}'"
set +e
package_modified=$(git diff "${KOKORO_GITHUB_PULL_REQUEST_TARGET_BRANCH}...${KOKORO_GITHUB_PULL_REQUEST_COMMIT}" -- ${files_to_check} | wc -l)
set -e
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 17, 2026

Choose a reason for hiding this comment

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

medium

The git diff command relies on environment variables that are typically only available in pull request builds. In other contexts (like continuous builds), these variables will be empty, leading to a git syntax error. While set +e prevents the script from terminating, it's cleaner to check for the presence of these variables. Additionally, the file path should be quoted.

Suggested change
echo "checking changes with 'git diff ${KOKORO_GITHUB_PULL_REQUEST_TARGET_BRANCH}...${KOKORO_GITHUB_PULL_REQUEST_COMMIT} -- ${files_to_check}'"
set +e
package_modified=$(git diff "${KOKORO_GITHUB_PULL_REQUEST_TARGET_BRANCH}...${KOKORO_GITHUB_PULL_REQUEST_COMMIT}" -- ${files_to_check} | wc -l)
set -e
if [[ -n "${KOKORO_GITHUB_PULL_REQUEST_TARGET_BRANCH}" && -n "${KOKORO_GITHUB_PULL_REQUEST_COMMIT}" ]]; then
echo "checking changes with 'git diff ${KOKORO_GITHUB_PULL_REQUEST_TARGET_BRANCH}...${KOKORO_GITHUB_PULL_REQUEST_COMMIT} -- ${files_to_check}'"
set +e
package_modified=$(git diff "${KOKORO_GITHUB_PULL_REQUEST_TARGET_BRANCH}...${KOKORO_GITHUB_PULL_REQUEST_COMMIT}" -- "${files_to_check}" | wc -l)
set -e
else
package_modified=0
fi


if [[ "${package_modified}" -gt 0 || "$KOKORO_BUILD_ARTIFACTS_SUBDIR" == *"continuous"* ]]; then
echo "------------------------------------------------------------"
echo "Running doctest for: ${package_name}"
echo "------------------------------------------------------------"

export GOOGLE_CLOUD_PROJECT="bigframes-testing"
export NOX_SESSION="cleanup doctest"

cd "${package_path}"
python3 -m nox -s "${NOX_SESSION}"
Comment thread
chalmerlowe marked this conversation as resolved.
Outdated
else
echo "No changes in ${package_name} and not a continuous build, skipping."
fi
Loading