Skip to content

coverage: 120 partial lines in include/morph are branches nothing takes both ways, and only lines are gated #404

Description

@Yaraslaut

What is wrong

The repository measures branch coverage and then gates on lines.

Branch data is produced and preserved — scripts/coverage.sh exports LCOV
BRDA records and runs scripts/aggregate_lcov_branches.py to collapse
llvm-cov's per-template-instantiation noise into one record per source branch,
deliberately keeping branch coverage rather than dropping it. Codecov receives
it: the report carries 903 branches for include/morph and 2,279
repo-wide.

Nothing anywhere states a branch target. Every status and every component in
codecov.yml scores lines. So a branch taken one way only is visible in the
report and gates nothing.

On master that is 120 partial lines inside include/morph — 120 places
where a condition has been evaluated but only ever come out one way. Inverting
the if at any of them cannot fail a test, because no test has ever observed
the other arm.

Worst files:

partials misses file
19 5 include/morph/core/bridge.hpp
19 5 include/morph/core/remote.hpp
15 2 include/morph/util/rational.hpp
12 36 include/morph/forms/forms.hpp
12 52 include/morph/offline/sqlite_offline_queue.hpp
6 1 include/morph/forms/instance_constraints.hpp
5 1 include/morph/forms/views.hpp
5 3 include/morph/core/registry.hpp

bridge.hpp and remote.hpp are the two files the whole framework's dispatch
and wire behaviour live in.

Why a line target cannot substitute

A line target is satisfied by executing a line. if (a && b) is one line and
counts as covered the moment control reaches it, whatever a and b were. The
condition this repository actually wants held — a wrong comparison, a dropped
!, a < that should be <= is caught by a test
— is a statement about
branches (and, at full strength, about MC/DC: each condition independently
shown to affect the outcome), not about lines.

This is the property SQLite states as its headline test claim, and it is the
reason it states it in terms of MC/DC rather than statements.

The constraint that makes the naive fix wrong

aggregate_lcov_branches.py exists because llvm-cov emits a BRDA record per
template instantiation, and morph is a header-only template library. A raw
branch percentage over un-aggregated records is meaningless here — dozens of
spurious partials for one source branch. Any branch target must be set on the
aggregated records that script produces
, and its aggregation is therefore
load-bearing for the gate rather than merely cosmetic.

Second constraint: llvm-cov reports branch coverage, not MC/DC. Clang has
-fcoverage-mcdc (LLVM 18+), which is a different instrumentation mode with its
own limits — notably a hard cap on conditions per decision. Whether morph adopts
it, and at what cost, is part of this ticket rather than assumed by it.

Verification status: reproduced

Codecov API, branch master: include/morph aggregates to lines 5,716,
branches 903, partial lines 120. Per-file partial counts above are from the same
response. aggregate_lcov_branches.py (199 lines) and the BRDA handling in
scripts/coverage.sh:158-172 read from master.

Not verified: how many of the 120 are genuinely unreachable rather than
untested. Some certainly are — codecov.yml already documents a class of
llvm-cov closing-brace artifacts — and the split between "no test wrote this"
and "no input can reach this" is the substance of the work, not a preliminary to
it.

What would close this

  1. Branch coverage for include/morph is reported as its own number and carries
    a target, set from a measured ceiling in the way every line target in
    codecov.yml already is.
  2. Each of the 120 partial lines is in exactly one of two states: covered by a
    test that exercises the other arm, or recorded as unreachable with the
    reason
    , in the manner codecov.yml's existing artifact audits use. A bare
    suppression is not a disposition.
  3. A decision on -fcoverage-mcdc is recorded either way — adopted, with its
    condition-count cap and its cost stated, or declined, with why branch
    coverage is judged sufficient here.

Blocked by morph#403: include/morph/net's 903-branch denominator excludes
a whole subsystem today, so both the count and the ceiling would be measured
against the wrong tree.

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

    area: ciSubsystem: cibugSomething isn't workingtriage: validWell-framed; implement as written

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions