Skip to content

tests: test_coverage_gaps.cpp's SubscriberState marker names a type that does not exist and three line ranges that moved #419

Description

@Yaraslaut

Found while fixing morph#355 and reported to the sprint manager. Same defect
class, one file over.

What is wrong

tests/test_coverage_gaps.cpp:397 is a section marker naming a type that does
not exist and three line ranges that no longer say what it claims:

// ── bridge.hpp: SubscriberState weak-lock fails when handler dies mid-flight (489-490, 518-519, 532-533)

SubscriberState does not exist. git grep -c SubscriberState over master
e675253a returns exactly one hit — this comment. There is no such type in the
tree.

All three line ranges are stale. What include/morph/core/bridge.hpp holds
at them today:

Cited Actually
489-490 mid doc-comment — "are not deduplicated: the guard below reads binding->primary/currentId…"
518-519 auto const previous = ::morph::exec::detail::ModelId{binding->currentId.load()};
532-533 if (detail::parkIfInFrame(*handoff, true, newId, nullptr)) { return; …

None is a weak-lock failure path.

Why this one needs repointing, not deletion

morph#355 deleted the sibling marker at :575 (tryFireImpl returns when draft is absent) because it was a bare header with no test body under it — it
named coverage that was never written. This one is different: there are tests
under it. Deleting the marker would orphan real tests from the explanation of
what they cover, which is the opposite of what morph#355 was for.

So the work is to find what the tests below actually exercise, name it, and cite
the line range it lives at now.

The general shape

This is the third instance of one pattern — morph#349 (dispatchDecoded),
morph#355 (tryFireImpl, SubscriberEntry), and this. A comment citing a
symbol plus a line range has two independent ways to rot and nothing checks
either. scripts/check_spec_citations.sh verifies cited paths and sections;
it does not verify that a cited identifier resolves, nor that a cited line range
still contains what the comment says. Worth considering whether the lint should
grow that, which would close the class rather than the instance — but that is a
larger decision and belongs in its own ticket if it is wanted.

Verification status: reproduced

git grep -c "SubscriberState" origin/master → 1 (this comment).
The three line ranges read from git show origin/master:include/morph/core/bridge.hpp.

Not verified: what the tests under the marker actually cover. That is the
substance of the fix and needs someone to read them against the current
bridge.hpp, not a grep.

What would close this

  1. The marker names a type that exists and a line range that contains the
    behaviour the tests below exercise — verified by reading, not by assuming the
    old range shifted by a fixed amount.
  2. git grep "SubscriberState" returns nothing tree-wide.
  3. If the tests below turn out to cover nothing that still exists, that is the
    finding and it is reported rather than resolved by deleting them.

Blocked by morph#355 — its PR #414 edits this same file (deleting the :575
marker).

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