Add comprehensive unit tests for RequestTelemetry#3
Open
devin-ai-integration[bot] wants to merge 1 commit intomasterfrom
Open
Add comprehensive unit tests for RequestTelemetry#3devin-ai-integration[bot] wants to merge 1 commit intomasterfrom
devin-ai-integration[bot] wants to merge 1 commit intomasterfrom
Conversation
The RequestTelemetry class manages the X-Stripe-Client-Telemetry header used to report request metrics back to Stripe. While functional telemetry is tested end-to-end in TelemetryTest, the class itself had no direct unit tests covering its internal logic. This adds 11 focused unit tests covering: Enqueue/poll semantics: - pollPayload returns empty when nothing is enqueued - Enqueue followed by poll returns valid JSON payload - Poll drains the queue (second poll returns empty) Usage field handling: - Usage list is included in the payload when provided - Empty usage list is normalized to null (absent in JSON) Telemetry toggle: - Enqueue is silently ignored when telemetry is disabled - Poll returns empty when telemetry is disabled at poll time (but still consumes the queue entry) Edge cases: - Null requestId causes enqueue to be silently skipped - Queue is bounded at MAX_REQUEST_METRICS_QUEUE_SIZE (100) Deprecated API: - getHeaderValue returns empty when header is already present - getHeaderValue returns telemetry when header is absent Co-Authored-By: Jason Kelley <kllyjsn@gmail.com>
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why?
RequestTelemetrymanages theX-Stripe-Client-Telemetryheader used to report request duration metrics back to Stripe. While the existingTelemetryTestexercises telemetry end-to-end through the HTTP layer (viaMockWebServer), theRequestTelemetryclass itself had no direct unit tests covering its internal enqueue/poll logic, toggle behavior, queue bounds, or edge cases.What?
Adds
RequestTelemetryTest.javain thecom.stripe.netpackage (same package as the class under test, sinceRequestTelemetryis package-private) with 11 unit tests:null(omitted from JSON)Stripe.enableTelemetry = false; poll returns empty when disabled at poll time (but still consumes the queue entry)MAX_REQUEST_METRICS_QUEUE_SIZE)getHeaderValue— returns empty when header already present; returns telemetry when absentNo production code changes.
Reviewer notes:
RequestTelemetryuses a shared staticConcurrentLinkedQueue. ThesetUp()method drains it before each test to isolate state, but this is worth verifying is sufficient if the test runner parallelizes within-class tests.testQueueBoundedAtMaxSizehardcodes100to match the privateMAX_REQUEST_METRICS_QUEUE_SIZEconstant — if that constant changes, this test will fail (which is arguably desirable).See Also
src/test/java/com/stripe/functional/TelemetryTest.javaClassCastExceptionin error parsing — separate change)Link to Devin session: https://app.devin.ai/sessions/c1b674151bf44bf5beb6dff50488e9d3
Requested by: @kllyjsn