Skip to content

The test conftest cannot resolve the repo root without .git, and the marker turns that into a false 'not vendored' claim #233

Description

@topij

scripts/tests/conftest.py resolves the repo root with
_repo_layout.find_repo_root, which walks up for a .git marker and — finding
none — falls back to start.parent. That fallback is:

  • correct when the engines sit directly under the root (scripts/), and
  • one level short when they are nested (scripts/devkit/), the layout
    /adopt defaults to.

Nothing distinguishes the two, because the only signal that would (.git) is
exactly what is absent. This is #60's defect, already pinned by
test_repo_layout.py::test_the_fallback_is_wrong_in_a_nested_layout_and_that_is_known.

Why it needed handling in the test conftest specifically

PR #232 added a kit_repo_only marker that skips a test when a path it
needs is absent. That turns a wrong root from a loud failure into a statement:
not vendored in this tree, about a file that may be sitting right there.

Two attempts, and what each cost — recorded so a third does not repeat them:

  1. Round 1: disable skipping entirely when no .git is found. Preserves the
    loud failure in the nested case. But the fallback root is correct for the
    flat layout, so this broke a case that worked: a tarball export of a
    genuinely sized-down tree went from an accurate skip to a failure — Kit tests hardcode parents[2] as the repo root, so they fail or error in the scripts/devkit/ layout /adopt defaults to #134's
    own harm class, for a different population. Withdrawn.

  2. Round 2: search both REPO_ROOT and its parent. Withdrawn in round 3,
    for two reasons found by execution. It is still wrong at nesting depth > 1 —
    a tree with engines at tools/internal/devkit/ and init.sh at the true
    root got a confident not vendored in this tree about a file that was right
    there, which is worse than round 1's behaviour on the same tree. And in the
    flat layout the second candidate sits outside the repo entirely, so a
    same-named file in the parent directory — a tarball unpacked inside another
    checkout — suppressed a skip that should have fired.

  3. Round 3 (shipped): no guess at all. One root is searched, and when no
    .git was found the skip reason says so:

    not vendored in this tree: init.sh (repo root unresolved — no .git above <dir>; see #233)
    

    This makes no false claim — it states exactly what was searched — and keeps
    the clean run a sized-down adopter is owed. The cost is that in a nested
    no-.git tree a test may skip when it could have run; the reason text is
    what makes that legible rather than misleading.

Three consecutive review rounds each found a defect in the previous round's
guess.
That is the signal, not any single defect: the root is not derivable
from the information available, and every approximation has leaked in a
different tree shape. A fourth approximation should not be attempted here.

What a resolution needs

Something that identifies the repo root without .git. Options, none costed:

  • Read paths.engines from config/dev-model.yaml and walk up that many
    segments from the engines directory. The config is the value being hardcoded
    today; it is also the thing the layout is defined by. Cost: the conftest
    gains a YAML read, and kitconfig is not importable at conftest time without
    the same path arithmetic being solved first.
  • A repo-root sentinel the kit writes and adopters keep.
  • Accept the widening permanently and document it as the answer.

Related: #60 (the resolver limit itself), #203 (three modules still carry
private copies of the walk-up), #134, #232.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions