[testbot] Add tests for streaming.py

[svc-osmo-ci]

Summary

AI-generated tests targeting file(s) with low coverage.

Issue - None

Targets selected

• src/lib/data/storage/streaming.py

Why this file was targeted

src/lib/data/storage/streaming.py — 44.9% coverage, 54 uncovered lines.

This file owns the public object-streaming contract of the storage SDK — stream_object and its three dispatch modes (full, byte-offset range, last-N-lines) plus the two-pass byte-offset seek in _seek_last_n_lines_byte_start/_stream_last_n_lines that backs workflow log tailing in the CLI and log-viewer UI. Tests here catch off-by-one and boundary regressions in the last-N-lines seek (empty file, file with exactly N lines, missing trailing newline, N larger than the file) that would silently serve users the wrong log window or the entire object, and catch mode-dispatch/summary regressions where size, lines, and retries in StreamSummary stop reflecting the actual transfer. It beats the higher-scored candidates because mux.py currently has no callers in the repo (a test would pin unused code) and notify.py is thin wiring around smtplib/Slack whose failures are already swallowed into warnings, whereas streaming.py has 10 reverse imports, is a documented library entry point other services depend on, and its uncovered lines are exactly the non-obvious offset math rather than defensive plumbing.

Generator summary

All 11 targets in the package pass.

COVERAGE REPORT
- src/lib/data/storage/streaming.py: 54/54 listed lines hit (100%) — pass
  still uncovered: none

What was added

src/lib/data/storage/tests/test_streaming.py (36 tests, wired via a new osmo_py_test(name = "test_streaming") in src/lib/data/storage/tests/BUILD). Tests go through the public stream_object API only, backed by an in-memory ResumableStream / fake StorageClient / real CacheableClientProvider, so the provider lifecycle is exercised rather than mocked out.

Coverage by area:

• Mode dispatch (_stream_response, lines 304–332): asserts the exact get_object(bucket, key, offset, length) calls issued for FullStream, OffsetStream (with and without length), and LastNLinesStream. The assert_never guard at 328/332 is reached by injecting an unrecognized option, verifying it fails loudly instead of silently streaming.
• Last-N-lines seek (lines 207–292): the boundary math is pinned — n=2 of 4 lines seeks to byte 12; n == line count, n > line count, and an empty object all fall back to a full second fetch (offset None) instead of raising IndexError; an unterminated final line seeks to byte 6 of alpha\nbeta\ngamma.
• Output shapes (lines 166–197, 376–412): bytes stream chunking, lines-mode decoding (replace / ignore / strict error strategies), and as_io=True full and partial reads, including summary is None before close and the last-N-lines offset applied through the file-like path.
• StreamSummary fidelity: size, lines, retries, and failures asserted against the underlying API context.

Verification: bazel test passes, mypy clean, pylint 10.00/10, and verify_coverage.py reports hit_fraction: 1.0 with an empty still_uncovered_ranges.

Coverage gain on listed uncovered ranges

✅ src/lib/data/storage/streaming.py — 54/54 listed lines hit (100%)

• ✅ line 166 — 1/1 hit
• ✅ lines 183-186 — 4/4 hit
• ✅ lines 195-197 — 3/3 hit
• ✅ line 207 — 1/1 hit
• ✅ lines 209-211 — 3/3 hit
• ✅ line 213 — 1/1 hit
• ✅ lines 216-218 — 3/3 hit
• ✅ line 220 — 1/1 hit
• ✅ line 222 — 1/1 hit
• ✅ line 225 — 1/1 hit
• ✅ line 272 — 1/1 hit
• ✅ line 277 — 1/1 hit
• ✅ line 283 — 1/1 hit
• ✅ line 285 — 1/1 hit
• ✅ line 292 — 1/1 hit
• ✅ line 304 — 1/1 hit
• ✅ lines 306-307 — 2/2 hit
• ✅ lines 312-313 — 2/2 hit
• ✅ lines 320-321 — 2/2 hit
• ✅ line 328 — 1/1 hit
• ✅ line 332 — 1/1 hit
• ✅ line 376 — 1/1 hit
• ✅ lines 378-381 — 4/4 hit
• ✅ line 385 — 1/1 hit
• ✅ line 387 — 1/1 hit
• ✅ line 389 — 1/1 hit
• ✅ lines 391-396 — 6/6 hit
• ✅ line 398 — 1/1 hit
• ✅ lines 404-407 — 4/4 hit
• ✅ line 410 — 1/1 hit
• ✅ line 412 — 1/1 hit

Checklist

• I am familiar with the Contributing Guidelines
• New or existing tests cover these changes
• The documentation is up to date with these changes

---

Generated by testbot pipeline

Summary by CodeRabbit

• Tests
  • Added comprehensive coverage for storage streaming across full-object, byte-offset, and last-N-lines requests.
  • Verified byte, decoded-line, and file-like reading modes, including partial reads and boundary conditions.
  • Added tests for seeking, summaries, decoding errors, client lifecycle handling, and invalid options.
  • Added a public build target for running the streaming test suite.

[coderabbitai]

📝 Walkthrough

Walkthrough

The change adds a Bazel target and comprehensive unit tests for storage streaming. Tests cover byte, decoded-line, offset, last-N-lines, and file-like modes, including summaries, errors, client lifecycle, boundaries, and option validation.

Changes

Storage streaming test coverage

Layer / File(s)	Summary
Test target and streaming fixtures src/lib/data/storage/tests/BUILD, src/lib/data/storage/tests/test_streaming.py	Adds the test_streaming Bazel target and fixtures for ranged requests, stream statistics, client lifecycle, parameters, and summaries.
Basic streaming modes src/lib/data/storage/tests/test_streaming.py	Tests bytes, decoded lines, and byte-offset streaming, including delivery, summaries, decoding behavior, failures, closure, and boundary cases.
Advanced and file-like streaming src/lib/data/storage/tests/test_streaming.py	Tests last-N-lines selection, two-request seeking, file-like reads, deferred summaries, client lifetime, option combinations, and unknown-option handling.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

• NVIDIA/OSMO#1277: Adds the same test_streaming target and streaming tests.

Suggested reviewers: fernandol-nvidia, jiaenren

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: adding tests for streaming.py.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch testbot/20260810-2023

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/lib/data/storage/tests/test_streaming.py (1)

108-108: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Make the generic result types explicit.

client.APIResponse, Iterator, and List omit their item types. This prevents type checking from detecting fixture and stream-output mismatches. Use client.APIResponse[client.GetObjectResponse] and a TypeVar for _drain.

As per coding guidelines, use type annotations where they improve clarity. As per path instructions, keep strict type annotations.

Also applies to: 157-158

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/data/storage/tests/test_streaming.py` at line 108, Make the generic
annotations explicit in the affected streaming test helpers: change the API
response annotation to client.APIResponse[client.GetObjectResponse], and update
_drain to use a TypeVar so its Iterator and List item types are preserved. Apply
the same explicit item typing to the related annotations at the additionally
referenced lines, keeping strict type checking throughout.

Sources: Coding guidelines, Path instructions

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/lib/data/storage/tests/test_streaming.py`:
- Around line 542-543: Extend the test around stream_io.close() to assert that
stream_io.summary is not None after closing, while preserving the existing
assertion that it starts as None.

---

Nitpick comments:
In `@src/lib/data/storage/tests/test_streaming.py`:
- Line 108: Make the generic annotations explicit in the affected streaming test
helpers: change the API response annotation to
client.APIResponse[client.GetObjectResponse], and update _drain to use a TypeVar
so its Iterator and List item types are preserved. Apply the same explicit item
typing to the related annotations at the additionally referenced lines, keeping
strict type checking throughout.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: e373423f-8dcd-40b1-b610-1b51199c192e

📥 Commits

Reviewing files that changed from the base of the PR and between 0575ea6 and d6b015a.

📒 Files selected for processing (2)

• src/lib/data/storage/tests/BUILD
• src/lib/data/storage/tests/test_streaming.py

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.51%. Comparing base (0575ea6) to head (d6b015a).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1280      +/-   ##
==========================================
+ Coverage   70.16%   70.51%   +0.34%     
==========================================
  Files         238      238              
  Lines       28325    28325              
  Branches     4272     4272              
==========================================
+ Hits        19873    19972      +99     
+ Misses       7640     7538     -102     
- Partials      812      815       +3     

Flag	Coverage Δ
backend	73.19% <ø> (+0.37%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 4 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.