You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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:
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.
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.
scripts/tests/conftest.pyresolves the repo root with_repo_layout.find_repo_root, which walks up for a.gitmarker and — findingnone — falls back to
start.parent. That fallback is:scripts/), andscripts/devkit/), the layout/adoptdefaults to.Nothing distinguishes the two, because the only signal that would (
.git) isexactly what is absent. This is
#60's defect, already pinned bytest_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_onlymarker that skips a test when a path itneeds 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:
Round 1: disable skipping entirely when no
.gitis found. Preserves theloud 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.
Round 2: search both
REPO_ROOTand 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/andinit.shat the trueroot got a confident
not vendored in this treeabout a file that was rightthere, 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.
Round 3 (shipped): no guess at all. One root is searched, and when no
.gitwas found the skip reason says so: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-
.gittree a test may skip when it could have run; the reason text iswhat 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:paths.enginesfromconfig/dev-model.yamland walk up that manysegments 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
kitconfigis not importable at conftest time withoutthe same path arithmetic being solved first.
Related:
#60(the resolver limit itself),#203(three modules still carryprivate copies of the walk-up),
#134,#232.