test: insta snapshot#1103
Conversation
📝 WalkthroughWalkthroughThis PR introduces comprehensive snapshot-based test coverage using the ChangesSnapshot Test Coverage
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
rust/crates/ccusage/src/config_schema.rs (1)
1249-1251: ⚡ Quick winAdd a comment explaining why snapshot testing is skipped in the schema-generator context.
The early return based on binary name is a pragmatic solution for snapshot testing across different execution contexts—it prevents snapshot assertions from running when the
generate_config_schemabinary is regenerating schema definitions. This pattern is intentional and appropriate.To improve maintainability, add a doc comment above the helper function explaining why the skip is necessary:
/// Detects if running in the `generate_config_schema` binary context. /// Returns true to skip snapshot tests, which would conflict with snapshot /// regeneration during schema generation. fn running_in_schema_generator_test_binary() -> bool {🤖 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 `@rust/crates/ccusage/src/config_schema.rs` around lines 1249 - 1251, Add a short doc comment above the helper function running_in_schema_generator_test_binary explaining that it detects when tests are running inside the generate_config_schema binary and returns true to skip snapshot assertions because snapshot tests would conflict with snapshot regeneration during schema generation; then keep the existing early return that uses this helper (the if running_in_schema_generator_test_binary() { return; } block) so snapshot tests are skipped in that execution context.
🤖 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.
Nitpick comments:
In `@rust/crates/ccusage/src/config_schema.rs`:
- Around line 1249-1251: Add a short doc comment above the helper function
running_in_schema_generator_test_binary explaining that it detects when tests
are running inside the generate_config_schema binary and returns true to skip
snapshot assertions because snapshot tests would conflict with snapshot
regeneration during schema generation; then keep the existing early return that
uses this helper (the if running_in_schema_generator_test_binary() { return; }
block) so snapshot tests are skipped in that execution context.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: badcc1bd-f958-4500-b1c2-76c3da16daea
⛔ Files ignored due to path filters (24)
rust/Cargo.lockis excluded by!**/*.lockrust/crates/ccusage-terminal/src/snapshots/ccusage_terminal__tests__snapshots_ansi_truncation_boundary.snapis excluded by!**/*.snaprust/crates/ccusage-terminal/src/snapshots/ccusage_terminal__tests__snapshots_full_table_with_multiline_cells_and_separators.snapis excluded by!**/*.snaprust/crates/ccusage-terminal/src/snapshots/ccusage_terminal__tests__snapshots_multiline_box_title_layout.snapis excluded by!**/*.snaprust/crates/ccusage-terminal/src/snapshots/ccusage_terminal__tests__snapshots_narrow_table_with_wrapping_truncation_and_compact_dates.snapis excluded by!**/*.snaprust/crates/ccusage/src/adapter/opencode/snapshots/ccusage__adapter__opencode__parser__tests__snapshots_message_to_entry_variants_and_model_candidates.snapis excluded by!**/*.snaprust/crates/ccusage/src/adapter/opencode/snapshots/ccusage__adapter__opencode__tests__snapshots_agent_summary_json_period_keys_and_session_metadata.snapis excluded by!**/*.snaprust/crates/ccusage/src/adapter/snapshots/ccusage__adapter__codex__tests__snapshots_codex_reports_for_periods_sessions_costs_and_fallback_models.snapis excluded by!**/*.snaprust/crates/ccusage/src/snapshots/ccusage__cli__tests__claude_agent_help.snapis excluded by!**/*.snaprust/crates/ccusage/src/snapshots/ccusage__cli__tests__codex_daily_help.snapis excluded by!**/*.snaprust/crates/ccusage/src/snapshots/ccusage__cli__tests__root_help.snapis excluded by!**/*.snaprust/crates/ccusage/src/snapshots/ccusage__cli__tests__snapshots_cli_parse_error_guidance.snapis excluded by!**/*.snaprust/crates/ccusage/src/snapshots/ccusage__cli__tests__snapshots_representative_cli_parse_shapes.snapis excluded by!**/*.snaprust/crates/ccusage/src/snapshots/ccusage__cli__tests__statusline_help.snapis excluded by!**/*.snaprust/crates/ccusage/src/snapshots/ccusage__config_schema__tests__snapshots_schema_agent_specific_option_edges.snapis excluded by!**/*.snaprust/crates/ccusage/src/snapshots/ccusage__output__tests__snapshots_group_project_output_orders_named_projects_and_unknown_bucket.snapis excluded by!**/*.snaprust/crates/ccusage/src/snapshots/ccusage__output__tests__snapshots_model_multiline_formatting_sorts_dedupes_and_shortens_names.snapis excluded by!**/*.snaprust/crates/ccusage/src/snapshots/ccusage__output__tests__snapshots_session_summary_json_with_present_and_missing_options.snapis excluded by!**/*.snaprust/crates/ccusage/src/snapshots/ccusage__output__tests__snapshots_summary_json_with_optional_fields_and_model_breakdowns.snapis excluded by!**/*.snaprust/crates/ccusage/src/snapshots/ccusage__output__tests__snapshots_totals_json_with_extra_tokens_credits_and_zero_credit_omission.snapis excluded by!**/*.snaprust/crates/ccusage/src/snapshots/ccusage__summary__tests__snapshots_bucket_aggregation_for_week_boundaries_invalid_dates_and_model_merging.snapis excluded by!**/*.snaprust/crates/ccusage/src/snapshots/ccusage__summary__tests__snapshots_filter_and_sort_summaries_since_until_inclusive.snapis excluded by!**/*.snaprust/crates/ccusage/src/snapshots/ccusage__summary__tests__snapshots_session_accumulator_latest_metadata_versions_and_timezone.snapis excluded by!**/*.snaprust/crates/ccusage/src/snapshots/ccusage__summary__tests__snapshots_summarize_by_key_aggregates_counts_costs_and_breakdowns.snapis excluded by!**/*.snap
📒 Files selected for processing (10)
rust/crates/ccusage-terminal/Cargo.tomlrust/crates/ccusage-terminal/src/lib.rsrust/crates/ccusage/Cargo.tomlrust/crates/ccusage/src/adapter/codex.rsrust/crates/ccusage/src/adapter/opencode/mod.rsrust/crates/ccusage/src/adapter/opencode/parser.rsrust/crates/ccusage/src/cli.rsrust/crates/ccusage/src/config_schema.rsrust/crates/ccusage/src/output.rsrust/crates/ccusage/src/summary.rs
|
@coderabbitai review this |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 135024
|
Screen.Recording.2026-05-20.at.21.53.16.mov
Summary by cubic
Adds comprehensive
instasnapshot tests acrossccusageandccusage-terminalto lock down CLI help, JSON report shapes, and terminal table rendering. Introducesrender_lines()in the terminal crate for deterministic output; runtime behavior is unchanged.Refactors
render_lines()toccusage-terminal::SimpleTableand madeprint()delegate to it for stable, testable output.Dependencies
instatoccusage-terminal.insta(withjsonfeature) toccusage.Written for commit 45e5543. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
Tests
Refactor