Skip to content

Migrate CLI infra state-bucket tests to the GCS emulator - #310

Open
JyotinderSingh wants to merge 4 commits into
keras-team:mainfrom
JyotinderSingh:migrate-cli-infra-storage-tests
Open

Migrate CLI infra state-bucket tests to the GCS emulator#310
JyotinderSingh wants to merge 4 commits into
keras-team:mainfrom
JyotinderSingh:migrate-cli-infra-storage-tests

Conversation

@JyotinderSingh

@JyotinderSingh JyotinderSingh commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator

Description

Two things:

Migrate the last GCS mocks to the emulator. state_backend_test and state_test were the last two test files that wire-mocked storage.Client. They now run against fake-gcs-server like everything else: ensure_gcs_backend creates the real {project}-kinetic-state bucket (asserted by existence and versioning), the Conflict path is a genuine duplicate create, and list_clusters discovers real seeded stack files by real prefix query, with a new case proving the listing is scoped to .pulumi/stacks/kinetic/. Forbidden/PermissionDenied remain injected since the emulator has no IAM, and uniform bucket-level access is asserted on the request because the emulator does not persist that flag.

Document the testing tiers. The contributing guide now explains the test ladder (unit → emulator → docker → e2e), what each tier needs, how to run it, and that every tier skips cleanly when its prerequisite is missing. Also corrects the e2e prerequisites: with a profile from kinetic init, E2E_TESTS=1 is the only variable required — KINETIC_* are optional overrides.

Contributor Agreement

  • I am a human, and not a bot.
  • I will be responsible for responding to review comments in a timely manner.
  • I will work with the maintainers to push this PR forward until submission.
  • I will test the changes on my cloud setup and provide proof of successful validation.

@codecov-commenter

codecov-commenter commented Aug 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 66 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@a6adb41). Learn more about missing BASE report.

Files with missing lines Patch % Lines
kinetic/cli/infra/state_backend_test.py 0.00% 36 Missing ⚠️
kinetic/cli/infra/state_test.py 0.00% 30 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #310   +/-   ##
=======================================
  Coverage        ?   36.82%           
=======================================
  Files           ?       83           
  Lines           ?    15217           
  Branches        ?        0           
=======================================
  Hits            ?     5604           
  Misses          ?     9613           
  Partials        ?        0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@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 refactors the infrastructure state and state backend tests to run against a real GCS emulator (using FakeGcsTestCase) instead of relying on heavy mocking. This provides more realistic integration testing for bucket creation, versioning, and cluster listing. The reviewer feedback focuses on improving the mocking and patching strategy in these tests. Specifically, it is recommended to use mock.patch.object with wraps instead of custom spy functions, and to consistently patch the module-specific storage.Client (e.g., state_backend.storage.Client or state.storage.Client) rather than the globally imported class to avoid side effects and align with existing repository patterns.

Comment thread kinetic/cli/infra/state_backend_test.py
Comment thread kinetic/cli/infra/state_backend_test.py
Comment thread kinetic/cli/infra/state_backend_test.py
Comment thread kinetic/cli/infra/state_test.py
The last two test files that wire-mocked storage.Client now run
against fake-gcs-server, the canonical transport:

- state_backend_test: ensure_gcs_backend creates the derived
  {project}-kinetic-state bucket for real; the name is asserted by the
  bucket's existence and versioning by reading it back, and the
  Conflict path is a genuine duplicate create rather than an injected
  exception. Uniform bucket-level access is asserted on the request via
  a call-time snapshot, since the emulator does not persist that flag.
- state_test (ListClustersTest): cluster discovery lists real seeded
  stack files by real prefix query, and the missing-bucket case is a
  genuine NotFound collapsing to []. A new case proves the listing is
  scoped to the .pulumi/stacks/kinetic/ prefix (decoys under another
  Pulumi project and at the bucket root are excluded).

Forbidden/PermissionDenied remain injected — the emulator has no IAM —
which is fault injection, not transport replacement.
Replace the flat 'Code quality and testing' section with a tier ladder
(unit / 0 emulator / 1 docker / e2e), each with what it exercises, what
it needs, and how to run it. Covers the one non-pip prerequisite
(fake-gcs-server is a Go binary: brew, go install, or
FAKE_GCS_SERVER_BIN), the skip-when-missing semantics, the E2E_TESTS
process-isolation rule, the docker tier's PyPI-release dependency, and
guidance for writing new tests (emulator for anything touching GCS;
patch only for fault injection or spies).
Route every patch in state_backend_test/state_test through
state_backend.storage / state.storage rather than the directly imported
google.cloud.storage, matching repo convention.

The UBLA test keeps its call-time snapshot spy rather than a plain
wraps=: wraps on the unbound method loses self (TypeError), and even
with autospec the real create_bucket reloads the bucket from the
server's reply, so the flag reads False by the time call_args is
inspected. The comment now says so.
@JyotinderSingh
JyotinderSingh force-pushed the migrate-cli-infra-storage-tests branch from e8c2297 to 6ec300f Compare August 15, 2026 08:38
The e2e tests submit through @kinetic.run, which resolves project, zone,
and cluster via resolve_infra (explicit > KINETIC_* env > active profile
> default), and kinetic fetches the kubeconfig itself on first use. With
a profile from 'kinetic init', E2E_TESTS=1 is the only variable needed;
KINETIC_* are documented as optional overrides (how CI runs without a
profile). Drops the redundant manual get-credentials step.
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.

2 participants