Skip to content

test(logging): pin compression_savings in the gcs pubsub spend log fixture - #34204

Merged
yuneng-berri merged 1 commit into
litellm_internal_stagingfrom
litellm_/gcs-pubsub-test-failure-1b8ed8
Jul 22, 2026
Merged

test(logging): pin compression_savings in the gcs pubsub spend log fixture#34204
yuneng-berri merged 1 commit into
litellm_internal_stagingfrom
litellm_/gcs-pubsub-test-failure-1b8ed8

Conversation

@yuneng-berri

Copy link
Copy Markdown
Collaborator

Relevant issues

Linear ticket

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have added meaningful tests
  • My PR passes all CI/CD checks (e.g., lint, format, unit tests)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review (Greptile reviews automatically once the PR is opened; only comment @greptileai to re-request a review after pushing changes)

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

Screenshots / Proof of Fix

This is a test-only change to a golden fixture, so there is no runtime behavior to exercise against a live proxy; the emitted payload is already correct on staging and only the expectation file was behind. The meaningful evidence is the test going red before and green after, at the two commits below

Before, fixture at e7b9357bc2 (staging tip merged into this branch, without the fix):

$ python -m pytest tests/logging_callback_tests/test_gcs_pub_sub.py -q

E   AssertionError: Dictionary mismatch: ['Extra key in actual: metadata.compression_savings']
E   assert False
=========================== short test summary info ============================
1 failed, 1 passed, 1 warning in 8.61s

After, fixture at 7ef380fa3c (this PR):

$ python -m pytest tests/logging_callback_tests/test_gcs_pub_sub.py -q

2 passed, 1 warning in 8.57s

Type

✅ Test

Changes

test_async_gcs_pub_sub_v1 has been failing on every run in the logging_testing CircleCI job. It is not flaky; the golden fixture is stale

#33810 added a compression_savings key to SpendLogsMetadata, which is the schema that defines the spend-log metadata JSON. _get_spend_logs_metadata builds clean_metadata from SpendLogsMetadata.__annotations__, so every field declared there is emitted by construction, and the GCS pubsub v1 path publishes the SpendLogsPayload verbatim. The new key therefore shows up in the published message by design. That PR updated the other spend-log fixtures, including the inline metadata strings in test_spend_management_endpoints.py, but missed gcs_pub_sub_body/spend_logs_payload.json. The comparator in this test flags any key present in the actual payload but absent from the expected file, so it fails deterministically

This pins compression_savings as null in the fixture rather than adding it to ignored_keys. The precedent in this file is the latter; metadata.litellm_call_id was ignored when it was introduced. That was the right call there because the value is a random UUID and cannot be pinned. compression_savings is deterministic on this path, so ignoring it would leave the assertion permanently blind to the field, whereas pinning it keeps both directions of the comparison meaningful: the test now fails if the key is dropped from the payload as well as if it changes shape

Final Attestation

  • The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR

…xture

The spend-log metadata schema gained a compression_savings key, so the
gcs pubsub v1 payload now carries it. The golden fixture was never
updated, and the comparator flags any key present in the payload but
absent from the fixture, so test_async_gcs_pub_sub_v1 failed on every
run. Pin the key as null rather than adding it to ignored_keys; the
value is deterministic on this path, so ignoring it would leave the
assertion blind to the field entirely.
@greptile-apps

greptile-apps Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR updates the GCS Pub/Sub v1 spend-log fixture to match the emitted metadata shape. The main change is:

  • Adds compression_savings: null to the expected metadata JSON

Confidence Score: 5/5

This looks safe to merge.

  • The fixture matches the no-compression request exercised by the test.
  • Parsed metadata is still checked for missing, extra, and mismatched values.
  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
tests/logging_callback_tests/gcs_pub_sub_body/spend_logs_payload.json Adds the deterministic null expectation for compression_savings without weakening the fixture comparison

Reviews (1): Last reviewed commit: "test(logging): pin compression_savings i..." | Re-trigger Greptile

@yuneng-berri
yuneng-berri enabled auto-merge (squash) July 22, 2026 00:49
@codecov

codecov Bot commented Jul 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@yuneng-berri
yuneng-berri merged commit 5081e0c into litellm_internal_staging Jul 22, 2026
77 checks passed
@yuneng-berri
yuneng-berri deleted the litellm_/gcs-pubsub-test-failure-1b8ed8 branch July 22, 2026 00:56
@codspeed-hq

codspeed-hq Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 31 untouched benchmarks


Comparing litellm_/gcs-pubsub-test-failure-1b8ed8 (7ef380f) with litellm_internal_staging (2b2ae4c)1

Open in CodSpeed

Footnotes

  1. No successful run was found on litellm_internal_staging (e967bc8) during the generation of this report, so 2b2ae4c was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

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