Skip to content

A guard that exempts on substring presence will pass a module that imports a helper and never calls it #700

Description

@realmarcin

Summary

A guard that decides "is this module converted?" by checking whether a name
appears in the file will pass a module that imports the name and never calls
it. That happened in #689 and hid two incomplete conversions plus one I had
already claimed to have made.

What happened

tests/test_record_roots_are_shared.py::_sweeping_test_modules() skipped any
module whose text contained record_files, default_record_roots,
taxon_descriptor_roots or data/isolates.

test_no_vacuous_go_annotations.py was converted by a scripted pass that
replaced COMMUNITIES.glob("*.yaml"). Its sweep actually read

for path in sorted((corpus or COMMUNITIES).glob("*.yaml")):

which the pattern did not match, so only the import was added. ruff --fix
then removed the now-unused import as F401. Net result: a module that read as
converted, swept one root, and passed the scan — twice.

Fixed in #689 by making detection structural: a module that still globs a
constant bound to kb/communities is flagged whatever else it says.

Why this is worth its own issue

The specific instance is fixed. The pattern is not, and it is cheap to
repeat:

  • test_writers_leave_a_trace.py classifies writers by whether
    appends_curation_history == "yes", which comes from
    scripts/audit_writers.py — that one is safe, because it runs the audit
    rather than grepping.
  • Several guards in this repo do decide membership by substring
    (_SHARED_MARKERS was one; _SWEEP_MARKERS still is, though it only widens
    the candidate set rather than exempting from it).

The general rule worth writing down and checking: a guard may use a substring
to decide what to LOOK at, never to decide that something is FINE.
Exempting
on presence is how a rename, an auto-fix, or an unused import turns a gate off.

Suggested

  1. Audit the repo's guards for "membership by substring" used as an exemption.
    grep -rn "in text" tests/ is the starting point.
  2. Where one is found, either re-derive the property structurally (parse, call
    the tool, run the thing) or invert the check so the substring only widens the
    candidate set.
  3. Note the rule alongside the mutation-testing protocol in The mutation-testing protocol is undocumented, and three of its failure modes produce false greens #696 — both are
    about the same failure: evidence that looks like evidence and is not.

Acceptance test

A module that imports a shared helper without calling it is still flagged by
whatever guard claims to check that the helper is used.

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