Skip to content

test(fixtures): run-tests.sh reports every fixture green when python3 is missing #1066

Description

@BryanFRD

tests/fixtures/run-tests.sh reports every fixture as passing, while asserting nothing, on any machine where python3 is not on PATH.

Reproduced

Point a fixture at an expectation that cannot possibly hold:

description = "x"
check_contains = [
    "ZZZ_IMPOSSIBLE_STRING",
]
$ FERRFLOW_BIN=... bash tests/fixtures/run-tests.sh ../fxgen
  ok   multi-versioned-files
Results: 121 passed, 0 failed, 0 skipped (total: 121)

Running ferrflow check by hand in that same fixture directory errors out, so the runner is not merely lenient, it is reading no expectations at all.

Cause

parse_toml_array shells out to python3 and swallows every failure:

parse_toml_array() {
    local file="$1" key="$2"
    python3 -c "
...
" 2>/dev/null || true
}

On Windows, python3 resolves to the Microsoft Store stub, which prints an install prompt to stderr and exits non-zero. 2>/dev/null || true turns that into an empty result, so check_contains, check_not_contains and output_order are all empty lists and every while read loop over them runs zero times. failed stays false, the fixture prints ok.

The same applies to any environment where python is installed only as python, or not at all.

Why it matters

The failure mode is the worst available one: a contributor runs the fixture suite locally, sees 121 passed, and concludes their change is safe. I hit exactly that while working on #983, where two fixtures were genuinely broken by the change and the local run reported them green. CI caught it, so the suite is not worthless, but the local run is actively misleading rather than merely absent.

It also means nobody can tell how long the local suite has been vacuous, or whether a fixture expectation added in that window was ever exercised outside CI.

Fix

Fail loudly instead of silently:

  • Resolve an interpreter once at startup (python3, then python, checking it actually runs rather than just that the name resolves, since the Store stub resolves fine), and exit with a clear message if none works.
  • Drop the 2>/dev/null || true from parse_toml_array, parse_toml_int and parse_toml_bool so a parse failure is an error rather than an empty value.

A self-check would be cheap insurance: before running anything, parse a known-good expectation and assert the result is non-empty. That catches this whole class of failure rather than just the python case.

Rewriting the parsing in bash would remove the dependency, but the expectation files are TOML with multi-line arrays, so that trades one fragile thing for another. Detecting the missing interpreter is the smaller and more honest fix.

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