Skip to content

test: coverage for indexes list and get_none_if_not_found#62

Merged
eddietejeda merged 1 commit intomainfrom
feat/test-coverage-indexes-api
Apr 27, 2026
Merged

test: coverage for indexes list and get_none_if_not_found#62
eddietejeda merged 1 commit intomainfrom
feat/test-coverage-indexes-api

Conversation

@eddietejeda
Copy link
Copy Markdown
Contributor

Summary

Adds unit and mockito-backed tests to improve Codecov on the workspace indexes list flow and ApiClient::get_none_if_not_found.

Changes

  • ApiClient::test_new (#[cfg(test)]): construct a client against a mock server without loading profile config.
  • api: tests for get_none_if_not_found on 404 (returns None) and 200 (parses JSON), including X-Workspace-Id when a workspace is set.
  • indexes: extract information_schema_followup, connection_label_to_id_map, and sort_info_tables for focused tests; mock collect_tables, list_one_table, and list_one_table_scan.
  • Behavior (aligned with prior review): information_schema pagination stops when has_more is true but next_cursor is missing; connection lookup maps display name → id only (catalog values that are already connection IDs still work via the existing fallback).

Testing

cargo test (all tests pass).

Add ApiClient::test_new for mock HTTP tests. Cover get_none_if_not_found
404 and 200 paths with mockito. Extract information_schema_followup,
connection_label_to_id_map, and sort_info_tables for unit tests plus
mocked collect_tables and list_one_table scan paths. Connection map now
stores display name to id only; catalog rows that already carry a
connection id still resolve via the existing fallback.
Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Two nits inline, neither blocking.

@sentry
Copy link
Copy Markdown

sentry Bot commented Apr 27, 2026

Codecov Report

❌ Patch coverage is 96.98795% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/indexes.rs 96.18% 5 Missing ⚠️

📢 Thoughts on this report? Let us know!

Comment thread src/indexes.rs
}

#[test]
fn sort_info_tables_orders_by_connection_schema_table() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: The test only exercises connection-level ordering (entries differ at connection). Schema and table tiebreaking are never reached, so the test name oversells the coverage. Consider adding a second pair that shares the same connection but differs on schema or table to actually verify the composite key. (not blocking)

Comment thread src/indexes.rs
}

#[test]
fn list_one_table_scan_returns_indexes_on_200() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super nit: Unlike the other mockito tests in this file, this one doesn't call .match_header("Authorization", "Bearer k"), so the header is not verified. Consistent header matching would make the test stronger. (not blocking)

@eddietejeda eddietejeda merged commit a7f54c7 into main Apr 27, 2026
9 checks passed
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.

1 participant