Skip to content

test(core): raise behavioral Rust coverage for weak core contracts #360

Description

@DecisionNerd

Maintainer scope: before v1.0.0

Backward compatibility is out of scope for this issue and its repair sub-issues before the v1.0.0 release. Do not add or retain legacy readers, old API aliases, compatibility shims, mixed-version support, migration/backfill machinery, or old-version regression tests solely to preserve behavior or data from earlier GraphForge versions. Earlier backward-compatibility requirements in this issue are superseded by this maintainer instruction.

This does not authorize unrelated breaking changes or weaken the issue's current-version acceptance criteria. Exact current-format semantics, supported current-version API/binding and export/import interoperability, authentication, corruption/unsupported-format refusal, crash recovery, retry/idempotency, active snapshots, cancellation and resource budgets remain required where applicable. Never silently reinterpret unsupported old data. Document intentional format/API breaks and the supported current format; a migration implementation is not required. Current-version correctness tests and explicitly behavior-preserving refactors remain in scope. Compatibility guarantees for v1.0.0 and later are a separate release-policy decision.

Problem

Coverage floors existed but had no enforcement point on the merge path, so they did not hold and the numbers drifted without anyone being told.

PR CI deliberately does not run llvm-cov. The only coverage step in the Test Suite exercises the coverage tooling's own failure modes rather than this repository's coverage. The real gate lived in the heavy rust-tests-coverage-native stage of make pre-push, while the documented default loop is make pre-push-fast, which skips heavy stages. A floor enforced only when a developer opts in is not a floor.

The 2026-09-07 reopen was therefore arithmetic rather than decay. Between 2026-08-04 and 2026-09-14 the measured core surface grew from 98,219 to 184,015 lines, and the 85,796 added lines were covered at 87.1%. Adding code at 87% to a base at 95% carries the aggregate to 91.32% with no test removed. Production source grew from 276,795 to 478,167 lines over the same window, so this was real growth and not a change in measurement scope. An aggregate floor above the measured value requires every marginal line, in perpetuity, to be covered at or above it.

What remains is the original concern, which the drift obscured: concentrated gaps in specific crates that a healthy workspace aggregate conceals.

Objective

Close the per-crate coverage gaps with deterministic behavioral tests that prove real contracts, using measurement practices that reflect how coverage actually behaves in Rust rather than treating a line percentage as proof.

Debt / Regime

  • Debt type: test/proof
  • Quality regime: A — deterministic compute

Resolved 2026-09-16 by #1333

  • Core aggregate floor changed from 95 to a fixed 80, matching the per-crate floor. The per-crate 80, patch 90 and adapter 80 floors are unchanged.
  • A post-merge Coverage Baseline workflow runs the same ledger on every push to main and fails on a breached floor. Its patch total compares HEAD against the previous main commit, because post-merge the merge base with origin/main is HEAD itself and would measure an empty patch. It is not a required PR status and cannot block a pull request.
  • The acceptance criterion requiring a 95% core aggregate is superseded. Every other criterion stands.

Measured state

From the retained ledger at 8069cdae on 2026-09-14. A fresh run against current main supersedes these figures once available; the decomposition series merged after this measurement.

Surface Line coverage
Core 91.32% (168,048 / 184,015)
Python adapter 80.97%
Node adapter 80.92%
Merged workspace 93.71%

Crates below or near the 80 floor:

Crate Line coverage Standing
graphforge-cli 79.34% (2,853 / 3,596) Below floor by roughly 24 covered lines
graphforge-io 80.34% (282 / 351) Passing with no margin
graphforge-portable-oci 80.79% (534 / 661) Passing with no margin
graphforge-ontology 81.82% (3,547 / 4,335) Thin margin
graphforge-core 89.67% (885 / 987) Comfortable

graphforge-plan and graphforge-ast, named as the weakest crates when this issue was filed, are no longer among them.

Rust coverage practice

These constrain how the remaining work is measured and are the reason a line percentage alone is not accepted as evidence.

  • Line coverage is the weakest metric cargo-llvm-cov reports. Prefer region coverage when judging whether a contract is actually exercised. The ? operator is the common failure: it compiles an error-propagation branch onto the same line as the success path, so line coverage marks that line covered when only the happy path ran. Single-line match arms and if let behave the same way. A crate can show a high line percentage with most of its error paths never executed.
  • Generic code is measured only where it is instantiated. Monomorphization means an uninstantiated generic function contributes nothing to either numerator or denominator, so a generic-heavy module can look well covered while whole instantiations are untested. Cover the concrete types that matter, not the generic definition.
  • Uncoverable arms should be deleted, not excluded, but count them correctly. There are 125 unreachable!, todo! and unimplemented! arms under crates/*/src. Only 65 are in production code and therefore suppress the measured percentage; 37 sit in #[cfg(test)] modules and 23 in test files that the decomposition moved under src/, and none of those 60 affect coverage at all. The two the compiler currently proves unreachable, in graphforge-exec/src/algorithm_paths_prize_steiner.rs, are test-module code and are a warning to silence rather than coverage work. Concentrations in production sit in graphforge-ir/src/binder/projection.rs (7), graphforge-cypher/src/parser/expr.rs (5) and graphforge-cli/src/lib.rs (4). Removing a compiler-proven unreachable production arm is the correct fix; adding an exclusion or a test that cannot run is not.
  • Doctests do not count. cargo llvm-cov does not instrument doctests on stable, so documentation examples prove nothing about these numbers. Public API examples still need real tests.
  • Know what the denominator excludes. Core, per-crate and patch totals exclude crate-level tests/, benches/ and examples/ sources and executable lines inside #[cfg(test)] items. Those tests still cover production lines when they run; they simply are not themselves measured. Adding tests never inflates the denominator.
  • Mutation testing is the honest check that a test asserts anything. cargo-mutants is the Rust-native tool and is not yet used here. It is a developer tool installed like cargo-llvm-cov, not a Cargo.toml dependency, so adopting it adds nothing to the build or to shipped artifacts. Run it only against the modules a tranche touches; a workspace-wide run rebuilds and retests per mutant and is impractical at this size. This replaces hand-reasoning about whether a test would catch a wrong result.
  • Property testing is out of scope here, deliberately. This issue required property tests when graphforge-plan and graphforge-ast were the weak crates, and pure invariant logic is exactly where proptest earns a dependency. Those crates are no longer weak. The remaining gaps are command behavior, interchange and ontology paths, where table-driven and boundary tests are the better instrument. Adding a test-only dependency to satisfy a requirement aimed at work that is already done is not justified; revisit it if planner or AST invariant work returns.
  • Unsafe code is not a coverage problem. There are 55 unsafe blocks under crates/*/src. Coverage says nothing useful about undefined behavior. Where unsafe invariants matter, Miri is the right instrument, and it runs separately from coverage.
  • Tests must be deterministic. No wall-clock dependence, no reliance on thread interleaving, no ordering assumptions that hold only on one machine. A flaky test that raises a percentage is worse than the gap it filled.

Requirements

  • Recompute the baseline from the current merged ledger before selecting exact uncovered lines. Figures in this issue predate the decomposition series.
  • Prioritize high-value executable contracts rather than maximizing counts:
    • logical-plan construction, validation, and malformed/boundary cases;
    • AST/token behavior with observable semantics;
    • public executor dispatch, empty inputs, invalid options, cancellation/resource boundaries, and structured errors;
    • public storage/API paths below the floor where they represent reachable behavior.
  • For each tranche, cover a success path, the actual failure mode, and one meaningful boundary or invariant.
  • Prefer public/integration behavior where it proves multiple internal lines honestly; use unit or table-driven tests for pure deterministic logic.
  • Judge error-path coverage by region coverage or by an explicit test of the error arm, never by a line percentage over ? expressions.
  • Adopt cargo-mutants as a documented developer tool alongside cargo-llvm-cov, recorded in docs/engineering/TESTING.md with the bounded per-module invocation. It is installed, not depended on. This lands with the first test tranche, not as separate tooling work.
  • Do not add test-only product branches, manufactured errors, fallback engines, blanket ignores, or assertions that accept missing results.
  • Do not test private implementation trivia solely to inflate coverage. Verified unreachable or dead code is removed, not silently excluded.
  • Report per-crate totals so highly covered algorithm modules cannot conceal a weak foundational crate.
  • Preserve the advisory openCypher TCK model; this issue does not convert upstream TCK expectations into the product API gate.
  • Use isolated build output and run no more than two heavy Rust builds concurrently.

Acceptance Criteria

  • The current merged baseline identifies uncovered executable contracts by file/module and maps each selected gap to a behavioral, integration, unit, or table-driven oracle.
  • graphforge-cli reaches at least 80% line coverage through tests of real command behavior, not argument-parsing trivia.
  • Every in-scope non-binding production Rust crate reaches at least 80% line coverage.
  • The non-binding core aggregate holds at or above its fixed 80% floor.
  • Changed Rust source is protected by the mechanically enforced 90% patch-coverage floor, now running post-merge on every push to main.
  • Per-crate and aggregate gates fail closed on missing, malformed, or stale reports.
  • Each tranche records a cargo-mutants run scoped to the modules it adds tests for, reporting the mutant score and listing every surviving mutant. A surviving mutant is either killed by a further test or justified individually; a blanket statement that mutation testing was impractical does not satisfy this.
  • Compiler-proven unreachable arms encountered in touched modules are removed rather than excluded.
  • Existing public API BDD, openCypher TCK, persistence/recovery, concurrency, and Python/Node parity suites remain green.
  • No production behavior or compatibility stub is introduced solely to satisfy coverage.

BDD Completion Scenarios

Scenario: Planner success and rejection paths are proved

Given valid and malformed logical-plan inputs at documented boundaries
When the Rust planner APIs execute
Then valid inputs produce exact deterministic plans
And invalid inputs return the exact structured error without partial output.

Scenario: Executor dispatch cannot disappear behind aggregate coverage

Given a supported public invocation and a representative invalid or empty boundary
When executor dispatch runs
Then the real Rust path returns the exact Arrow result or structured error
And a missing, manufactured, or wrong dispatch result fails its regression test.

Scenario: A weak crate fails independently

Given the core aggregate remains above its floor but one in-scope production crate falls below 80%
When the coverage gate runs
Then the gate fails with that crate's measured result
And coverage from unrelated algorithm modules cannot average it away.

Scenario: An error path is not proved by a covered line

Given a function whose error path is reached only through ? propagation
When line coverage marks the expression covered after a success-only test
Then the contract is not accepted as proved
And an explicit test of the error arm, or region coverage, is required instead.

Scenario: Changed Rust code lacks proof

Given a merge to main changes executable Rust lines without exercising at least 90% of them
When the post-merge coverage baseline evaluates patch coverage against the previous main commit
Then the workflow fails with the uncovered changed lines
And generated files or unrelated historical gaps do not distort the patch result.

Implementation Notes

Likely initial focus, subject to the recomputed baseline:

  • crates/graphforge-cli/src/ command behavior, the one crate currently below floor
  • thin-margin crates graphforge-io, graphforge-portable-oci and graphforge-ontology
  • uncovered public dispatch and boundary paths in crates/graphforge-exec/src/
  • removal of compiler-proven unreachable arms encountered along the way

Split implementation into focused PRs only if independently reviewable test tranches justify separate CI cycles. Do not mix product fixes into a test-only PR; file or link a focused product issue when a test exposes incorrect behavior.

Observability

No product telemetry is required. The coverage ledger retains aggregate and per-crate counts, source SHA, toolchain identity, and deterministic failure diagnostics without graph data, query contents, properties, UUIDs, or unrestricted local paths.

Security And Privacy

Use synthetic local fixtures. Do not add secrets, external services, sensitive graph data, or unrestricted filesystem fixtures. Structured-error assertions must not require logging user graph or query contents.

Testing

  • Table-driven and boundary tests for pure AST and planner invariants.
  • Integration tests through the real planner and executor path for dispatch and Arrow contracts.
  • Empty, malformed, overflow/resource, and structured-error boundaries where reachable.
  • cargo-mutants scoped to touched modules, covering wrong values, wrong error classes and absent results, with the score and surviving mutants recorded in the PR.
  • Scenario-to-evidence mapping in the issue or PR test plan.
  • Targeted iteration followed by cargo fmt --all -- --check, cargo clippy --workspace -- -D warnings, cargo test --workspace, make pre-push, and exact-head required CI.

Documentation

docs/engineering/TESTING.md already documents the fixed floors, the post-merge enforcement workflow and what the production filter excludes. Extend it with what line coverage does and does not prove in Rust if this issue's practice notes prove useful beyond it.

Non-Goals

  • Testing private implementation details solely to improve a percentage.
  • Changing GraphForge runtime behavior under the guise of test work.
  • Replacing behavioral tests with snapshots of unstable diagnostics.
  • Treating xfail, pending, skip, missing results, or NotImplementedError as coverage success.
  • Changing Python or Node adapter behavior.
  • Making the advisory openCypher TCK a release-tested public API suite.
  • Restoring an aggregate floor above the measured value.

Related Issues

Open Questions

None. The property-testing question was resolved on 2026-09-16 by removing the requirement; see the Rust coverage practice section for the reason.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    coreCore source code changesexecutorChanges to query executorplannerChanges to query plannerrelease:noneNo release note or version impacttestingTest coverage and testing infrastructure

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions