Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
f14703f
Add cloudbuild.yaml
danieljbruce May 6, 2026
248638f
Shorten the script
danieljbruce May 6, 2026
cdb2ad1
remove the timeout
danieljbruce May 6, 2026
372a03b
feat: add new Bigtable feature
danieljbruce May 6, 2026
7caf8a0
Merge branch 'migrate-bigtable-to-gcp-cloud-build' of https://github.…
danieljbruce May 6, 2026
55b0dec
Merge branch 'main' into migrate-bigtable-to-gcp-cloud-build
danieljbruce May 6, 2026
7a3fadd
Merge branch 'main' into migrate-bigtable-to-gcp-cloud-build
danieljbruce May 6, 2026
5f930f9
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] May 6, 2026
f3772ea
Merge branch 'migrate-bigtable-to-gcp-cloud-build' of https://github.…
gcf-owl-bot[bot] May 6, 2026
83f73dd
feat: add new Bigtable feature
danieljbruce May 6, 2026
d7782b2
Merge branch 'migrate-bigtable-to-gcp-cloud-build' of https://github.…
danieljbruce May 6, 2026
6be7893
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] May 6, 2026
1f71bd1
Merge branch 'migrate-bigtable-to-gcp-cloud-build' of https://github.…
gcf-owl-bot[bot] May 6, 2026
e6cc411
Changed to `ubuntu` in the yaml file
danieljbruce May 6, 2026
c836303
Merge branch 'migrate-bigtable-to-gcp-cloud-build' of https://github.…
danieljbruce May 6, 2026
1ab42bc
change the dir to handwritten/bigtable
danieljbruce May 6, 2026
8e6ff1d
Change the pre-system test script
danieljbruce May 7, 2026
051f019
Fix the unique id error
danieljbruce May 7, 2026
20b11db
Add pre system tests to the first step
danieljbruce May 7, 2026
a74e61e
Remove the comment changed to
danieljbruce May 7, 2026
0aae31a
Eliminate the first pack and play error
danieljbruce May 8, 2026
af443d1
Fix `require` error with node dependencies
danieljbruce May 8, 2026
57bbdad
Modify the pack and play tests by adding a header
danieljbruce May 8, 2026
f6d43fe
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] May 8, 2026
8dd7d70
Eliminate unnecessary comments
danieljbruce May 8, 2026
fd18c1a
Merge branch 'migrate-bigtable-to-gcp-cloud-build' of https://github.…
danieljbruce May 8, 2026
bfe5155
Restore the system test script
danieljbruce May 8, 2026
e51a2e8
Increase the timeout to 3 hours for the system tes
danieljbruce May 8, 2026
11f861b
Merge branch 'main' into migrate-bigtable-to-gcp-cloud-build
danieljbruce May 8, 2026
88a19c7
Add dependency comment
danieljbruce May 8, 2026
c6a65a8
Merge branch 'migrate-bigtable-to-gcp-cloud-build' of https://github.…
danieljbruce May 11, 2026
83539cc
Do a conditional diff check
danieljbruce May 11, 2026
9f6587c
check against main removed
danieljbruce May 11, 2026
9916905
Add a comment about the packnplay tests
danieljbruce May 11, 2026
3ce9c3f
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] May 11, 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
98 changes: 98 additions & 0 deletions handwritten/bigtable/cloudbuild.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
steps:
# 0. Check for changes in handwritten/bigtable
- name: 'gcr.io/cloud-builders/git'
id: 'check-changes'
entrypoint: 'bash'
args:
- '-c'
- |
git fetch origin main --quiet
# Compare with origin/main to detect changes.
if git diff --quiet origin/main... -- . ; then
echo "No changes in handwritten/bigtable. Skipping tests."
echo "skip" > /workspace/skip_tests
else
echo "Changes detected in handwritten/bigtable. Proceeding with tests."
fi
dir: 'handwritten/bigtable'

# 1. Set up Node.js environment
- name: 'gcr.io/cloud-builders/npm'
entrypoint: 'bash'
args:
- '-c'
- |
if [ -f /workspace/skip_tests ]; then exit 0; fi
npm install -g npm@latest
npm config set prefix /usr/local
npm install
npm run presystem-test
Comment on lines +25 to +29
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

Updating npm globally and changing the configuration prefix in a CI environment is generally discouraged as it can lead to non-deterministic builds and potential permission issues. The gcr.io/cloud-builders/npm image should already provide a functional npm version. If a specific version of Node.js or npm is required, it is better to use a specific image tag or pin the version explicitly.

    if [ -f /workspace/skip_tests ]; then exit 0; fi
    npm install
    npm run presystem-test

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.

On the other PR I committed a code change to fix the node version.

dir: 'handwritten/bigtable'
id: 'install-dependencies'
waitFor: ['check-changes']

# 2. Configure environment variables for the tests and run system tests
# - GOOGLE_APPLICATION_CREDENTIALS: GCB steps run as a service account
# that is typically granted permissions directly. Explicitly setting
# GOOGLE_APPLICATION_CREDENTIALS might not be needed if the GCB service
# account has the right roles (e.g., Bigtable Admin, Bigtable User).
# If you need to use specific service account key JSON, you'd store it
# in Secret Manager and mount it here. For simplicity, we'll rely on
# the GCB service account's inherent permissions.
# - GCLOUD_PROJECT: Can be passed as a build variable.
- name: 'gcr.io/cloud-builders/npm'
entrypoint: 'bash'
args:
- '-c'
- |
if [ -f /workspace/skip_tests ]; then exit 0; fi
npm run system-test
dir: 'handwritten/bigtable'
env:
- 'GCLOUD_PROJECT=${_GCP_PROJECT_ID}' # Pass project ID via build variable
# If you need specific credentials from Secret Manager, uncomment these:
# - 'GOOGLE_APPLICATION_CREDENTIALS=/secrets/sa-key.json'
id: 'run-system-tests'
waitFor: ['install-dependencies']
# For Secret Manager, uncomment these (adjust secret name and volume path as needed):
# secretEnv: ['SA_KEY']
# volumes:
# - name: 'sa-keys'
# path: '/secrets'

# 3. (Optional) Code Coverage Reporting
- name: 'gcr.io/cloud-builders/npm'
entrypoint: 'bash'
args:
- '-c'
- |
if [ -f /workspace/skip_tests ]; then exit 0; fi
# Check if nyc is installed and run report
if [ -f ./node_modules/nyc/bin/nyc.js ]; then
./node_modules/nyc/bin/nyc.js report || true # `|| true` prevents build failure if nyc report itself exits non-zero
Comment on lines +71 to +72
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

Avoid reaching into the internal directory structure of a package to execute its binary. It is more robust to use the binary located in node_modules/.bin/ or use npx.

    if [ -f ./node_modules/.bin/nyc ]; then
      ./node_modules/.bin/nyc report || true # || true prevents build failure if nyc report itself exits non-zero

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.

Maybe, but not a serious issue since the old scripts did it this way anyways.

else
echo "nyc not found, skipping coverage report."
fi
# The original codecov.sh script from Kokoro needs to be made available to GCB.
# Options:
# a) Commit codecov.sh into your repo (e.g., .kokoro/codecov.sh) and call it:
# if [ -f .kokoro/codecov.sh ]; then . ./.kokoro/codecov.sh; fi
# b) Replicate its functionality directly in this step.
# c) Store it in a GCS bucket and fetch it.
echo "Codecov reporting (if desired) would be integrated here."
dir: 'handwritten/bigtable'
id: 'coverage-report'
waitFor: ['run-system-tests']

# If you use Secret Manager for credentials, uncomment and configure:
# availableSecrets:
# secretManager:
# - versionName: projects/${PROJECT_ID}/secrets/YOUR_SERVICE_ACCOUNT_KEY_SECRET_NAME/versions/latest
# env: 'SA_KEY' # This env var will hold the secret value. Use it as GOOGLE_APPLICATION_CREDENTIALS in step 3 if needed.

# Define a substitution variable for your project ID
# Replace 'long-door-651' with the actual GCP Project ID your system tests should run against.
substitutions:
_GCP_PROJECT_ID: 'long-door-651'
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

Hardcoding a specific GCP Project ID (long-door-651) in the configuration makes the build script less reusable and can leak internal infrastructure details. Consider using a placeholder or leaving it empty if it's intended to be provided via a build trigger.

  _GCP_PROJECT_ID: 'your-project-id'

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.

This value was shared before therefore sharing the specific id has no downside.


timeout: '10800s'
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 timeout is set to 10800 seconds (3 hours), which is quite high for system tests. If tests hang, this could lead to significant resource consumption. Consider reducing this to a more reasonable value (e.g., 3600s) based on the expected execution time.

timeout: '3600s'

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.

No. We need this much time.

Loading
Loading