test: metrics

[infiniteregrets]

LLM (codex) generated based on the test spec

[greptile-apps]

Greptile Overview

Greptile Summary

This PR adds new integration tests that exercise the metrics endpoints (account/basin/stream), including validation of metric units/interval defaults and server-side error handling for invalid time ranges. It also extends the test harness types in tests/common/mod.rs to expose basin and stream names required to call basin/stream metrics APIs.

The tests fit into the existing test_context-based integration test pattern by provisioning an S2Stream (via a shared basin + per-test stream), performing a small append/read to generate activity, and then calling the relevant get_*_metrics APIs under a timeout before asserting response shapes.

Confidence Score: 4/5

• This PR is likely safe to merge once the timestamp truncation in the new metrics tests is addressed.
• Changes are limited to tests and small accessor additions, but metrics_ops.rs uses unix_timestamp() as u32, which can wrap on pre-epoch clocks and make invalid-range assertions unreliable; fixing that would make the test suite more deterministic.
• tests/metrics_ops.rs

Important Files Changed

Filename	Overview
tests/common/mod.rs	Adds basin_name()/stream_name() accessors on test wrapper types to support new metrics integration tests; no behavioral changes beyond exposing names.
tests/metrics_ops.rs	Adds new integration tests for account/basin/stream metrics endpoints, including invalid time range cases; contains a likely bug from casting unix_timestamp() to u32 (wraps for pre-epoch clocks) which can make tests behave incorrectly.

Sequence Diagram

sequenceDiagram
    participant Test as metrics_ops.rs test
    participant Ctx as test_context(S2Stream)
    participant Common as tests/common/mod.rs
    participant Stream as s2_sdk::S2Stream
    participant Client as s2_sdk::S2
    participant MetricsAPI as S2 metrics endpoints

    Test->>Ctx: setup S2Stream
    Ctx->>Common: SharedS2Basin::setup()
    Common->>Client: S2::new(config)
    Common->>Client: create_basin()
    Common->>Client: create_stream()
    Ctx-->>Test: &S2Stream

    Test->>Stream: append_sample() / read_sample()
    Test->>Client: get_*_metrics(TimeRange/Interval)
    Client->>MetricsAPI: HTTP request
    MetricsAPI-->>Client: Vec<Metric> / ErrorResponse
    Client-->>Test: Result
    Test->>Test: assert units/intervals or invalid code

    Test->>Ctx: teardown
    Ctx->>Common: delete_stream() (best effort)
    Common->>Client: delete_basin() (when last user)

Loading

[greptile-apps]

_{2 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Time truncation bug

epoch_range casts unix_timestamp() (an i64) to u32 (as u32). If the system clock is before 1970, this wraps to a huge positive value, making start/end nonsensical and causing the “invalid time range” tests to behave incorrectly (they may pass/fail for the wrong reason). Consider keeping timestamps as i64 (or validating non-negative) until constructing the API types.

Prompt To Fix With AI

This is a comment left during a code review.
Path: tests/metrics_ops.rs
Line: 13:16

Comment:
**Time truncation bug**

`epoch_range` casts `unix_timestamp()` (an `i64`) to `u32` (`as u32`). If the system clock is before 1970, this wraps to a huge positive value, making `start/end` nonsensical and causing the “invalid time range” tests to behave incorrectly (they may pass/fail for the wrong reason). Consider keeping timestamps as `i64` (or validating non-negative) until constructing the API types.


How can I resolve this? If you propose a fix, please make it concise.

[quettabit]

LGTM!