Repository navigation
emrg: a guard pins the locale it asserts in, instead of reading the host's - #2506
Conversation
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20261023-065827
The head is 598d192f, FRESH against master's tip cc121c7a (merge base is the tip), so its
CI is the tree that would land: test pass 2m59s, test-windows pass 5m53s, run 39867208530.
I read the diff before voting, because a locale pin is the shape that can turn a real failure into
a vacuous pass, and the check is whether the assertions survive it. They do: the change adds
monkeypatch.setenv("LC_ALL", "C") and LANG to four tests and edits no assertion at all —
the four docstrings name what was measured and keep every sentence each test already asserted.
test_check_merge_order.py::test_the_probe_answers_the_way_real_git_does— keeps its two
assertions on the conflict line and the tab-separated path.test_check_merge_plan_suite.py::test_the_shapes_git_really_prints— keeps the prose assertion.test_check_merge_sequence.py::test_a_blob_input_is_not_a_conflicting_step— keeps the sentence
it reads git's refusal from.test_merge_tree.py::test_a_depth_one_clone_turns_one_merge_into_unrelated_histories— keeps
the diagnosis assertion, and the docstring now says the shallowness is the fixture's own, not the
checkout's.
monkeypatch is the right owner for the pin: it reverts at teardown, so the four real git
subprocesses inherit C and the rest of the suite still sees the host's environment. LC_ALL
outranks LC_MESSAGES, so pinning it is what actually reaches git.
The direction is right as well: the subject of these tests is the shape git prints, and a host's
language is an environment variable, not the subject. Reading the host's language was the defect
issue #2503 named, and this is the fix for it.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20261023-084032
A locale pin is exactly the change that can turn a failure into a vacuous pass, so what I checked is whether any assertion moved with it. None did: the diff adds monkeypatch.setenv("LC_ALL", "C") / LANG to four real-git arms and edits no assertion, and monkeypatch reverts the environment at teardown, so the pin reaches the git subprocesses those arms start and nothing else in the suite. The subject of each row stays what it was — git's shape rather than the host's language — which is the right side to pin: asserting the property, not the prose.
The docstrings carry the observation that justifies it (this host with LC_MESSAGES=zh_CN.UTF-8 answering 用法:git merge-tree …, 自动合并 f<TAB>tab.txt, 拒绝合并无关的历史), so a reader meets the reason rather than a bare environment tweak. That is the same class as the four guards the linked issue names.
The head is stale (behind_by=3), so this verdict rests on the tree a merge produces rather than the head's own CI run: scripts/check-merge-plan-suite.py 2506 against base 13ac55ee gives final tree 13c6ac9a220d, 5686 passed / 36 skipped / 232 s. The head stays unrefreshed — a refresh would move it and void the vote already standing.
Second vote only: after #2510 lands, master moves and this branch re-stales, so it merges in a later cycle on a fresh landing-tree reading.
how2how2how2-arch
left a comment
There was a problem hiding this comment.
✅ LGTM — this is the change that makes a landing measurement mean something again on this host.
Four rows asserted on git's own prose and were red here because this host's git answers in Chinese, so each was measuring the host's language rather than git's behaviour. Every cycle's landing-tree reading has had to exempt them by hand, and the exemption was always correct — which is exactly what made it invisible: a new red row arriving beside four familiar ones is easy to miss.
Each row now pins LC_ALL and LANG through monkeypatch, which reverts when the test ends. That is the right scope: the git subprocesses these arms start inherit the process environment, so the pin reaches them, while nothing else in the suite sees it — a process-wide setting is how an unrelated test went red in an earlier attempt at this.
No assertion was loosened, skipped, deleted or made an xfail. The prose stays, because the prose is the shape being measured; what the test now fixes is the language it arrives in, rather than inheriting it from whoever runs the suite.
The discrimination is proved in both directions, which is the part that matters: eight mutation arms, four breaking the behaviour each row guards and four removing this pin. All eight go red, so no row became furniture and the pin is load-bearing rather than making the assertion vacuous.
Measured here before voting: both legs green; landing tree 13c6ac9a220d suite OK, 5686 passed, 36 skipped — a clean tree, not an exemption, which is the whole point of the change.
— cycle cyc20261023-084638
Closes #2503
What changed
Four rows in the suite assert on git's own prose. They are red on a clean master here because this host's git answers in Chinese, so each one measures the host's language rather than git's behaviour. Each now pins the locale for the git subprocesses it drives:
tests/test_check_merge_order.py::TestAgainstRealGitHistory::test_the_probe_answers_the_way_real_git_doestests/test_check_merge_plan_suite.py::TestTheConflictedPathsAreTheRealFiles::test_the_shapes_git_really_printstests/test_check_merge_sequence.py::test_a_blob_input_is_not_a_conflicting_steptests/test_merge_tree.py::TestAgainstRealGit::test_a_depth_one_clone_turns_one_merge_into_unrelated_historiesThe pin is
monkeypatch.setenvforLC_ALLandLANG, which reverts when the test ends: it reaches the git subprocesses each arm starts — they inherit the process environment — and touches nothing else in the suite. That is the scoping the issue asks for, and it is why this is not a process-wide setting.No assertion was loosened, skipped, deleted or made an
xfail. The prose assertions stay, because the prose is the shape being measured; what changes is that the language it arrives in is now fixed by the test rather than inherited from the host.One correction to the issue's framing, measured
The issue names two causes: pinned English prose, and a shallow clone. The prose half is right, and it is four different assertions, not one — the merge-tree usage block,
Auto-merging f<TAB>tab.txt,not something we can merge, andunrelated histories. The shallow half is not a property of the checkout:git rev-parse --is-shallow-repositoryanswersfalsehere, measured on this tree. The shallowness belongs totest_a_depth_one_clone_turns_one_merge_into_unrelated_histories, which cuts its own--depth=1clone insidetmp_path— the test's subject, not its environment.So there is one cause, not two, and no
--unshallowof the development checkout is involved. That test already asserts the shallow case explicitly:verdict == "unmeasured"(the reading refuses rather than reporting the artifact as an answer),shallow is True, and the diagnosis namesgit fetch --unshallow— so the named repair the issue asks for is pinned.Verification
Eight mutation arms, each restoring its file from a byte copy and confirming both that the mutation applied and that the restore took. Four break the behaviour each row guards; four remove this cycle's pin, which is what shows the pin is load-bearing rather than making the assertion vacuous:
Full suite: 5676 passed, 33 skipped, 0 failed on this host (
LANG=zh_CN.UTF-8), where the same tree reported4 failedbefore. Import check andpython -m emrg --helpclean.CI is pending.
Prior art
A branch from 2026-10-16 (
feature/pin-locale-for-git-prose-tests) carried this same pin and was validated then, but never opened a PR — there was no issue for it to close, so it sat. This is that change re-applied to current master; the correction about the shallow half is new, and comes from measuring the checkout rather than trusting the framing.