Skip to content

fix: use legacy SCP for Testnet IAP upload#207

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

Open
JOY (JOY) wants to merge 1 commit into
base:main
Choose a base branch
from
Open

fix: use legacy SCP for Testnet IAP upload #207

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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 .github/scripts/tests/test_validate_testnet_bens.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -886,6 +886,9 @@ def test_testnet_iap_upload_hard_caps_and_retries(self):
for call in success_gcloud_calls
)
)
self.assertTrue(
all("--scp-flag=-O" in call for call in success_gcloud_calls)
)
Comment on lines +889 to +891

@gemini-code-assistgemini-code-assistBotAug 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

It appears that the actual implementation changes to .github/workflows/deploy-config.yml (adding the --scp-flag=-O flag to the gcloud compute scp command) are missing from this pull request. Since this test reads directly from that workflow file, the test will fail in CI unless the workflow file is also updated in this PR.

Additionally, using all() inside self.assertTrue() will only produce a generic False is not True error message on failure. Consider asserting each call individually to get a more descriptive failure message showing the actual command executed.

forcallinsuccess_gcloud_calls:
self.assertIn("--scp-flag=-O", call)

self.assertNotEqual(0, failure_result.returncode)
self.assertEqual(3, len(failure_timeout_calls))
self.assertEqual(3, len(failure_gcloud_calls))
Expand All@@ -895,6 +898,9 @@ def test_testnet_iap_upload_hard_caps_and_retries(self):
for call in failure_gcloud_calls
)
)
self.assertTrue(
all("--scp-flag=-O" in call for call in failure_gcloud_calls)
)
Comment on lines +901 to +903

@gemini-code-assistgemini-code-assistBotAug 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Similar to the success path assertion, using all() inside self.assertTrue() will only produce a generic False is not True error message on failure. Consider asserting each call individually to get a more descriptive failure message showing the actual command executed.

forcallinfailure_gcloud_calls:
self.assertIn("--scp-flag=-O", call)

self.assertEqual(["5", "10"], failure_sleep_calls)
self.assertIn("failed after 3 attempts", failure_result.stderr)
self.assertTrue(
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/deploy-config.yml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -960,6 +960,7 @@ jobs:
--project="${GCP_PROJECT_ID}" \
--zone="${GCP_TESTNET_ZONE}" \
--tunnel-through-iap \
--scp-flag=-O \
/tmp/doscan-testnet-config.tgz \
"${GCP_TESTNET_INSTANCE}:/tmp/doscan-testnet-config-${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}.tgz"; then
uploaded=1
Expand Down