Skip to content

tooling/c2pa-oracle: the branch enumeration is checked rows → tests only, so a branch added without a row stays green #616

Description

@justin13888

tooling/c2pa-oracle's #[cfg(test)] mod tests carries an enumeration of every discriminating
branch in find_jumbf_superbox, declared_store_len, jumbf_superbox_span and
is_jumbf_not_found, one row per branch, naming the test that pins each of its two directions.
Three tests in that module parse the table out of the doc comment and check it:

  • the_enumeration_is_a_table_with_no_blank_cell — no row has an empty cell, and the function and
    both direction cells are intra-doc links;
  • every_test_the_enumeration_names_exists_in_this_file — every name a row links resolves to a
    function this file defines;
  • every_test_file_the_enumeration_names_exists — every tests/*.rs path the prose points at
    exists.

All three run rows → tests. None runs tests → rows, and none looks at the code the rows
describe. So the table's completeness — the property it exists for — is held by review alone:

Adding a refusing arm to declared_store_len with no row leaves all 24 tests green.

That was measured on PR #535 by adding such an arm and running mise run test-c2pa.

What would close it

A fourth check that counts, per named function, the discriminating branches in the source and
compares that count against the rows the table gives that function. The source is available to the
test already — the module reads itself through include_str!("lib.rs") — so the shape is the same
as the three existing checks: parse, count, compare, fail with the function's name.

The hard part is what counts as a discriminating branch without re-implementing a parser: match
arms, if with an else path, and the combinator forms this file actually uses (ok_or,
filter(..).ok_or, and_then(..).ok_or) are the population, and a syntactic counter over the
four functions' bodies has to agree with the table's own reading of them or the check is noise.
A hand-maintained per-function expected count is the weaker alternative and would still be a drift
guard — it goes red when a branch is added and the count is not updated, which is the direction
that is unheld today.

Why it is filed rather than built

#535's round 7 judged the review loop on that PR to be generating its own findings; adding new
machinery at its close is how such a loop fails to terminate. The pull request instead narrows the
README.md sentence to what the checks actually enforce — "fail on a blank cell or on a name
nothing defines" — and states the asymmetry in the module doc. Implementing this guard is what
would make the original, stronger sentence true again.

Refs #447, #535.

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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions