Skip to content

test(cli): cover the streaming query result sink - #1335

Merged
DecisionNerd merged 7 commits into
mainfrom
test/360-cli-query-sink
Sep 16, 2026
Merged

DecisionNerd merged 7 commits into
mainfrom
test/360-cli-query-sink

Conversation

@DecisionNerd

@DecisionNerd DecisionNerd commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

Why

graphforge-cli measured 79.34% against its 80% per-crate floor and was the only crate below floor. The largest coherent uncovered block was the gf query command wrapper: the underlying execute_to_result_sink_with_evidence contract is covered at the API level, but the wrapper that selects sink options, maps the format, and renders the two receipt shapes was not exercised at all.

Coverage

Measured with the same production filter coverage_rust_ledger.py applies, so these are production lines and not raw LCOV counts.

Covered / measured Percent
Before 2,853 / 3,596 79.34%
After 2,902 / 3,596 80.70%

49 previously uncovered production lines, 44 in portable_cli.rs and 5 in the crate root dispatch. The floor needed 24, so this clears it with margin rather than scraping past.

Tests

Success path, both receipt forms, a boundary, and a real failure:

  • the JSON receipt reports the sink contract, exact row and byte counts, and a result fingerprint;
  • the human receipt reports the same outcome in its own format;
  • --format arrow-ipc selects the non-default sink;
  • explicit batch and row-group bounds change how the sink is written without changing the result fingerprint;
  • a query the binder rejects fails, writes no partial sink, and emits a diagnostic.

Mutation evidence

cargo mutants --file crates/graphforge-cli/src/portable_cli.rs --package graphforge-cli

22 mutants: 14 caught, 3 missed, 5 unviable. The run_query whole-body stub is caught, so the tests fail when the function stops doing its work.

All three survivors are the same function, oci_credential at line 208, which this tranche does not target. It reads GRAPHFORGE_OCI_CREDENTIAL and feeds the OCI publish and pull paths. Killing them needs either a registry or process-global environment mutation, which is unsafe under parallel test execution in current Rust. The behaviour they guard is worth a follow-up, because a mutant that fabricates a credential rather than reading the environment currently goes undetected. Justified individually rather than waived in bulk, per #360.

Also

Adopts cargo-mutants in docs/engineering/TESTING.md as a developer tool, with the bounded per-module invocation. It is installed, not depended on, and adds nothing to the build.

Note for anyone running these locally: project tests need TMPDIR on a filesystem the admission policy accepts. The default temporary directory is refused as filesystem_class_unproven.

Refs #360

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

DecisionNerd and others added 2 commits September 16, 2026 17:10
graphforge-cli measured 79.34% against its 80% per-crate floor, and the largest
coherent uncovered block was the gf query command wrapper. The underlying
execute_to_result_sink_with_evidence contract is covered at the API level; the
command wrapper that selects sink options, maps the format, and renders the two
receipt shapes was not exercised at all.

Cover a success path, both receipt forms, an explicit-bounds boundary, and the
rejection path. The bounds test pins the invariant that batch and row-group
bounds change how the sink is written without changing the result fingerprint.

Measured against the same production filter the ledger uses, this covers 49
previously uncovered production lines and takes graphforge-cli from 79.34% to a
projected 80.70%.

Refs #360

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Line coverage proves a line executed, not that anything asserted its result.
Record the bounded per-module invocation, the TMPDIR requirement that project
tests carry, and the rule that every surviving mutant is killed or justified
individually.

Refs #360

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: e8ef5186-a78e-4858-84a6-aab1bbf13601

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Warning

Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption.


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.

❤️ Share

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

DecisionNerd and others added 5 commits September 16, 2026 17:44
The migration ledger asserts that every Cargo target appears in the checked-in
target map, so a new integration test fails Bazel Bootstrap until it is mapped.
Add the rust_test target, register it, and raise cargo_target_count to 127.

The target needs graphforge-storage explicitly, the same as multi_ontology,
because the test initializes a project and that crate is a Cargo dev-dependency
rather than part of the shared CLI dep list.

Verified with bazelisk test //crates/graphforge-cli:query_sink.

Refs #360

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The parity inventory requires every mapped integration test to be reachable
from //:ci_rust_tests. Registering the target in the migration map satisfies
the ledger but not the inventory; the target must also be a member of the
crate suite that the root aggregate pulls in.

Verified with bazelisk test //crates/graphforge-cli:cli_tests, 7 tests pass.

Refs #360

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@DecisionNerd
DecisionNerd merged commit 74ee569 into main Sep 16, 2026
23 checks passed
@DecisionNerd
DecisionNerd deleted the test/360-cli-query-sink branch September 16, 2026 18:33
IgorOffline pushed a commit to IgorOffline/CurateLabs-graphforge that referenced this pull request Oct 10, 2026
PR CurateLabs#1335 committed 29 files of mutants.out/ to main. I used a catch-all add in
that worktree and swept in the mutation tool's output directory. Nothing
gitignores it, so any local mutation run now dirties the working tree, and the
committed results go stale the moment the code they measured changes.

Nothing reads the directory: no script, workflow or make target references it.
The scores belong in the pull request that produced them, where they are dated
and attached to a diff, not checked into the tree as a snapshot nobody
refreshes.

Untrack it and ignore mutants.out*/ so the tool's working and backup
directories both stay out of the repository.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core source code changes documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant