Repository navigation
tests/conftest.py and tests/test_adopt_plan_paths_spelled.py: under a one-byte locale such as Latin-1 the 0xff --work-dir row skips and the two U+00E9 rows read the child's UTF-8, so none of the three fails on a tool that is right, and the bytes helper starts its child as run_script does (#1196) - #1209
Draft
brettheap wants to merge 1 commit into
Draft
brettheap wants to merge 1 commit into
brettheap wants to merge 1 commit into
Conversation
… one-byte locale such as Latin-1 the 0xff `--work-dir` row skips and the two U+00E9 rows read the child's UTF-8, so none of the three fails on a tool that is right, and the bytes helper starts its child as `run_script` does (#1196) Follows #1185 (for #1167), from RV1185's review of it (#1185 issuecomment-6356497748): its N1 and N2. No change to adopt-project.py, and the tool is right in every case below. Under a Latin-1 locale built as #1153's test_adopt_one_byte_locale.py builds one, the new file and the two non-UTF-8 `--out` rows of test_stdout_encoding.py gave 5 failed, 15 passed, 13 skipped; they give 0 failed, 17 passed, 16 skipped now. N1, the three rows that failed under a one-byte locale: - `skip_where_a_name_cannot_be_not_utf_8` (tests/conftest.py) also skips where `os.fsdecode(b"\xff")` is not U+DCFF, before the disk is touched. Latin-1 reads the byte as U+00FF, a printable character, so `_shown` prints `wd<U+00FF>k` as typed, and the 0xff row's premise, that Python reads it in as U+DCFF, does not hold. The disk and `os.fsencode` take the byte in every locale, so neither could say. UTF-8 and ASCII give U+DCFF and skip no more than before: the 1086 tests of the 22 files that ask the helper, and of test_repo_hygiene.py, have under UTF-8 the outcome they had. The two non-UTF-8 `--out` rows of test_stdout_encoding.py ask it too, and skip under Latin-1 where they failed. - `run_script` takes `encoding`, None unless named, and the four runs of tests/test_adopt_plan_paths_spelled.py name "utf-8": each child is told `PYTHONIOENCODING=utf-8`, and a parent in Latin-1 read the two bytes of U+00E9 as two characters (`..._non_ascii_out_path_...` and `..._non_ascii_work_dir_...`). Both rows pass there now. - The rows of test_non_utf8_name_callers.py (its fixture, which test_non_utf8_git_failures.py and test_non_utf8_oserror_sites.py use) and test_non_utf8_name_helpers.py that let the probe through to the disk skip where the helper skips on the new question, as they skip where it skips on the first: 28 of them failed under Latin-1 without it. N2: `plan_of_a_repository_on_github_as_bytes` builds its child's environment with `blank_unnamed_pin_sources` and the git identity defaults, as `run_script` and test_stdout_encoding.py's `run_tool_bytes` do. It had neither; the run ends at the clone refusal, so nothing it says changes, and no row sees it. Five rows are added and no other row changes: three in test_non_utf8_name_helpers.py (the new question asked of an emulated Latin-1 `os.fsdecode`, before the disk, and the UTF-8 and ASCII controls) and two in the new tests/test_run_script_encoding.py (the keyword reads stdout and stderr in the encoding named). The 31 rows of the spelled file are unchanged: 31 pass under UTF-8, and under ASCII 16 pass and 15 skip. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Reviewer's GuideTests are corrected for one-byte locales and explicit child output encodings: invalid non-UTF-8 filename premises now skip before filesystem access, File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
This was referenced Oct 23, 2026
Contributor
Author
|
RV1209: independent review of #1209 at Independent adversarial review for lane 4. No MUST and no SHOULD; five NITs for follow-ups. The PR does what #1196 asks. Under UTF-8 no test of the 25 files that reach the changed helpers changes its outcome; only the five new rows are added. Under a real Latin-1 locale the three rows of the spelled file no longer fail. What I measured against
1. UTF-8: nothing that ran before skips now
2. Latin-1
3.
|
| Name | d193617c7 |
13a2c58cb |
child_environ |
|---|---|---|---|
SHAPE_PIN_SOURCE_OPENXDOX |
/somewhere/else |
'' |
'' |
Shape_Pin_Source_Mixed |
/x |
'' |
'' |
LANES_LANE |
'' |
'' |
'' |
the four GIT_* identity names |
absent | the suite's IDENTITY_NAME and IDENTITY_EMAIL |
the same |
PYTHONIOENCODING |
utf-8:surrogateescape |
the same | the same |
AGENT_PROTOCOL_ROOT |
absent | absent | <tmp>/agent-protocol |
At the head, apart from the stand-in gh (PATH, GH_STUB_LOG), the only key child_environ has and the helper lacks is AGENT_PROTOCOL_ROOT. See N5.
5. Mutants
I wrote these myself, and applied each in a separate worktree at the head. The UTF-8 and Latin-1 runs used the helpers file, test_run_script_encoding.py, the spelled file and test_stdout_encoding.py's two non-UTF-8 --out rows (57 tests), unless the table says otherwise.
| Mutant | UTF-8 | Latin-1 |
|---|---|---|
| none (control) | 57 passed | 54 passed, 39 skipped (with the callers file) |
| G1: locale question removed | killed, 1: ..._skips_before_the_disk |
killed, 4: it, the 0xff row and both --out rows (callers file included) |
| G2: comparison inverted | killed, 8 helper rows | (not run) |
| G3: locale asked after the disk probe | killed, 1: ..._skips_before_the_disk |
killed, 1: the same row |
G4: locale asked BEFORE the system's os.fsencode question |
survives, 57 passed | (not run). See N3 |
G5: narrower, skip only where sys.getfilesystemencoding() is Latin-1 |
killed, 1: ..._skips_before_the_disk |
killed, 2: both ..._is_not_skipped controls |
G6: os.fsdecode(os.fsencode(NOT_UTF_8_PROBE)) |
survives | survives. Equivalent, as the body says |
| F1: callers fixture without the locale half | (not run) | killed, 23 rows of the callers, oserror-sites and git-failures files |
| F2: helpers guard without the locale half | (not run) | killed, 5 helper rows |
E1: run_script drops encoding |
killed, 1: the latin-1-reads-two row |
killed, 3: the utf-8 row and both U+00E9 rows |
E3: plan_of_a_local_source drops encoding |
(not run) | killed, 1: the --out U+00E9 row |
E4: plan_of_a_repository_on_github drops encoding |
(not run) | killed, 1: the --work-dir U+00E9 row |
After each run I restored the tree with git checkout -- tests/ and checked the sha256 of the four files the mutants touch against the head's. All 18 restores matched, and the worktree is clean. The encoding="utf-8" added to test_each_of_the_two_paths_is_spelled_in_one_run and a_command_on_github is equivalent in every locale those rows run in: they type U+202E, which Latin-1 and ASCII skip. It is harmless consistency.
6. Gate
- On
origin/mainef619c74a. The scratch merge with13a2c58cbis tree1c5710d09. The 25 files give 1118 passed and 1 skipped (the Windows default-stdout row):test_repo_hygiene.pyhas 101 rows there, and every other file has the head's counts. - On
origin/maindea964aa1. Main moved again (validate-family.py:gitmodules-path-twiceand the check for values git dies on read the.gitmodulesgit reads, the index's where the work tree's is gone, and every section's lines endgit add -- .gitmodules, so a plaingit commitafter them takes the section out of HEAD (#1077) #1124:templates/family-root/scripts/validate-family.pyand family tests). The scratch merge is tree854f04dec. I re-ran the 14 files that run the family tools, plus the PR's own files and the hygiene file: 498 passed and 1 skipped. Those 14 are:test_bump_leg_branch_and_push_line,test_cli_reference,test_clone_url_bytes;test_family_head_by_hand_bytes,test_family_member_branch_not_utf8,test_family_tracking_branch;test_local_remote_dir_urls,test_stdout_encoding,test_update_shape_apply_lines_and_headers;- the spelled, callers, helpers,
run_script_encodingand hygiene files.
- Disk. Each run started with 17G or more free. It never fell below 42G.
7. Merge-trees
- Against tests/test_adopt_plan_paths_spelled.py: seven mutants of the
plan written toandcloning ... intolines that no row killed now fail one, so a printable non-ASCII path prints as typed, ESC, CR and a non-UTF-8--work-dirare spelled, a relative--outis not made absolute, an--outthat ends in a space or is wrapped in backticks is spelled, andexecutespells the clone line asplandoes (#1167) #1185's headd193617c7and adopt-project.py:plan written toandcloning <repository> (read only) intoname the caller's--outand--work-dirwith_shown, as the other lines of the report name a path, so a bidi control, a line feed or a tab in either no longer reaches the terminal as it was typed (#1115) #1154's head8a7828332. Both exit 0. The tree is98ee3463, the PR head's own tree. - Against
origin/maindea964aa1. Exit 0, with tree854f04dec. That tree differs from main in seven files, the stack's:adopt-project.py,tests/conftest.py, the spelled, callers and helpers files,test_run_script_encoding.pyandtest_stdout_encoding.py. Each one's blob is the PR head's: main has not touched any of them since the stack's merge-base4ed7c7bf5. - Open PRs that touch
tests/conftest.py. None but tests/conftest.py and tests/test_adopt_plan_paths_spelled.py: under a one-byte locale such as Latin-1 the 0xff--work-dirrow skips and the two U+00E9 rows read the child's UTF-8, so none of the three fails on a tool that is right, and the bytes helper starts its child asrun_scriptdoes (#1196) #1209. I checked all 54 open PRs by the three-dot diff of each head from its merge-base with main. Only adopt-project.py:plan written toandcloning <repository> (read only) intoname the caller's--outand--work-dirwith_shown, as the other lines of the report name a path, so a bidi control, a line feed or a tab in either no longer reaches the terminal as it was typed (#1115) #1154 and tests/test_adopt_plan_paths_spelled.py: seven mutants of theplan written toandcloning ... intolines that no row killed now fail one, so a printable non-ASCII path prints as typed, ESC, CR and a non-UTF-8--work-dirare spelled, a relative--outis not made absolute, an--outthat ends in a space or is wrapped in backticks is spelled, andexecutespells the clone line asplandoes (#1167) #1185 touch any of the five files. - Every open PR against the scratch merge. I merged each of the 54 heads with the scratch merge of
dea964aa1. All exit 0, except four that conflict with main itself the same way, in files tests/conftest.py and tests/test_adopt_plan_paths_spelled.py: under a one-byte locale such as Latin-1 the 0xff--work-dirrow skips and the two U+00E9 rows read the child's UTF-8, so none of the three fails on a tool that is right, and the bytes helper starts its child asrun_scriptdoes (#1196) #1209 does not touch:- adopt-project.py:
check's rehearsal and_stored_as_writtentake a parent's-csettings,attr.treetoo, and the source's objects,GIT_ATTR_SOURCEis refused by name before any question, and each throwaway reads its environment first (#1136) #1169, adopt-project.py:checkandexecuterun withGIT_NO_LAZY_FETCH=1and refuse a partial-clone source that lacks an object its branches, tags or HEAD reach assource-partial-clone, so neither fetches into the source any more (#1145) #1180 and adopt-project.py:planruns withGIT_NO_LAZY_FETCH=1, refuses a partial-clone--sourcethat lacks objects assource-partial-clone, and names a partial--pin-sourceit could read only by fetching and the fetch that makes it whole (#1162) #1190 attests/test_adopt_check_throwaway_repos.py, after tests/test_adopt_check_throwaway_repos.py: rows pin thatrefused_pathsholds each case twin under its own name and blob, or names it, withcore.ignorecasetrue or false, and thatchecknames the twins git refuses and none it holds (#1033) #1158; - family.py: the
add,bumpandremovelines its remedies name begin with the interpreter and the script's path, quote the row's name, fill in the family's org, say the branch they run on, andmember-already-presentprints itsremove(#1127) #1201 atdocs/handbook.html.
- adopt-project.py:
Scope: the three files past the issue's text
Each change is needed once the shared helper asks the locale, and each is right:
- The callers fixture. Without it, 23 rows fail under Latin-1 (F1).
- The helpers guard. Without it, 5 rows fail (F2).
test_run_script_encoding.py. It is the only row that holdsrun_script's keyword under UTF-8, the locale CI runs (E1).
Whether the shared helper should ask the locale at all is N1.
Findings
- N1 (NIT): under a one-byte locale the shared helper's new question also skips rows that can judge the tool.
- What. 80 rows passed under Latin-1 before and skip there now:
- 35 on the helper's own new reason. Most are branch rows (
on_a_branch_that_is_not_utf_8and thetest_adopt_git_reads.pydefault-branch rows), for exampletest_plan_refuses_a_default_branch_that_is_not_utf8[head],test_family_tracking_branch.py's..._is_refused_by_name[add]andtest_family_head_by_hand_bytes.py's 8test_the_printed_line_puts_the_member_back_on_a_branch_that_is_not_utf8rows. Their names come from git's bytes, which are not UTF-8 in any locale. Each passed there, so the tool was judged and was right. - 45 on the two guards, the callers fixture and the helpers guard. Their subject is how a caller or the helper treats the disk's EILSEQ, EACCES, ENOSPC and EEXIST, emulated. That has nothing to do with the locale.
- 35 on the helper's own new reason. Most are branch rows (
- The docstring overstates it. "so no file name here is one that is not UTF-8" is not true of a git branch. The disk and git still hold the byte 0xFF in every locale; only Python's reading of it changes.
- Why only a NIT. No row the skip catches was failing on a wrong tool (section 2). Neither CI nor the bench runs a one-byte locale, and 52 rows of these files still fail there after. tests/test_adopt_plan_paths_spelled.py: follow-up of PR #1185 (#1167) from its review: three of its rows fail under a one-byte locale where the tool is right (the 0xff
--work-dirand the two U+00E9 paths), and its new helper builds the child's environment withoutblank_unnamed_pin_sourcesor the git identity defaults #1196 asked for exactly this change to the shared helper. - Follow-up.
- Put the locale question in its own helper, for example
skip_where_the_locale_reads_the_byte_as_text()intests/conftest.py. - Call it from the rows whose premise is that Python reads 0xFF in as U+DCFF: the 0xff
--work-dirrow andtest_stdout_encoding.py's two non-UTF-8--outrows. - Leave
skip_where_a_name_cannot_be_not_utf_8with its two questions, so the callers-fixture and helpers-guard halves are not needed. - Or keep this design, and say in the helper's docstring that under a one-byte locale it also skips the git-branch rows.
- Put the locale question in its own helper, for example
- What. 80 rows passed under Latin-1 before and skip there now:
- N2 (NIT): the new Latin-1 helper row fails on a real Windows.
- What.
test_a_locale_that_reads_the_byte_as_a_character_skips_before_the_diskis notLINUX_ONLY, and it patches onlyos.fsdecode. On Windows the helper's first question skips first. I emulated it withos.fsencode = windows_fsencodeand the row's Latin-1fsdecode. The reason is then "this system'sos.fsencodemakes no byte of a surrogate...", which holds no0xFF, soassert "0xFF" in reasonfails. - Why only a NIT. CI is Linux only (ci: run the test suite on Linux only #597), but this file is written to hold the Windows answers too.
- Fix. In that row, also patch
os.fsencodewith a POSIX one,lambda name: name if isinstance(name := os.fspath(name), bytes) else name.encode("utf-8", "surrogateescape"). Or skip it wherelen(os.fsencode(NOT_UTF_8_PROBE)) != 1.
- What.
- N3 (NIT): no row holds that the locale is asked after the system's question.
- What. G4 (the two swapped) survives on Linux. But Windows'
os.fsdecodeis UTF-8 withsurrogatepass, andb"\xff".decode("utf-8", "surrogatepass")raisesUnicodeDecodeError. With Windows'os.fsencodeandos.fsdecodeboth emulated, the head's helper skips on theos.fsencodereason, and G4's helper raisesUnicodeDecodeError. The conftest comment atNOT_UTF_8_BRANCHalready saysos.fsdecode"raises on Windows". - Fix. In
test_where_fsencode_makes_three_bytes_the_disk_is_not_asked, also patchos.fsdecodewith Windows' (lambda name: name if isinstance(name := os.fspath(name), str) else name.decode("utf-8", "surrogatepass")). G4 then fails there.
- What. G4 (the two swapped) survives on Linux. But Windows'
- N4 (NIT): the two-question predicate is written three times.
- What. The same test,
len(os.fsencode(NOT_UTF_8_PROBE)) != 1 or os.fsdecode(b"\xff") != NOT_UTF_8_PROBE, is in the callers fixturea_system_that_carries_the_byteand in the helpers file'sskip_where_an_earlier_question_skips. Its two halves are also the helper's own first two steps. A third question added to the helper later would have to be added in all three, or those rows fail again, as 28 did here. - Fix. Define one predicate in
tests/conftest.py, for examplean_earlier_question_skips() -> bool, beside the helper, and use it from both test files.
- What. The same test,
- N5 (NIT): the new helper's docstring claims more parity than it has.
- What.
the_environment_of_a_child_read_as_bytessays the child is started "astest_stdout_encoding.py'srun_tool_bytesstarts its own".run_tool_bytes(throughchild_environ) also sets an emptyAGENT_PROTOCOL_ROOT, and this helper does not (section 4). It has no effect here:shape_advisory.workspace_pathsis read only after a clone, and the run ends at the clone refusal. - Fix. Say "as
run_scriptstarts every other child", which is exact. Or add"AGENT_PROTOCOL_ROOT": str(tmp_path / "agent-protocol"), with the directory made, and keep the sentence.
- What.
Notes, not findings
- The closing keyword. It is not live while the base is tests/test_adopt_plan_paths_spelled.py: seven mutants of the
plan written toandcloning ... intolines that no row killed now fail one, so a printable non-ASCII path prints as typed, ESC, CR and a non-UTF-8--work-dirare spelled, a relative--outis not made absolute, an--outthat ends in a space or is wrapped in backticks is spelled, andexecutespells the clone line asplandoes (#1167) #1185's branch:closingIssuesReferencesis empty. It becomes live on the retarget tomain. - CI and Sonar. CI on the head:
tests,tests-git-bash,packageand SonarCloud all pass. The quality gate passed, with 0 new issues. - Scratch. Worktrees and refs are under
_lane4-scratch/rv1209-*andrefs/rv1209/*in my own clone. Nothing was pushed, and nothing in the PR was changed.
Lane: openreposhape-4 (openRepoShape-4)
6 tasks
This was referenced Nov 8, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1196
This PR lands after #1185. It is stacked on #1185's branch (
fix/1167-plan-paths-spelled-rows, headd193617c7), which is itself stacked on #1154's. It is retargeted tomainonce #1185 has landed, by the lane's stacked-PR procedure. Tests only: no change toadopt-project.py(sha256efe97bf7...b3d2, #1154's, byte for byte), and the tool is right in every case below.From RV1185's review of #1185 (#1185 issuecomment-6356497748): its N1 and N2.
N1: three rows failed under a one-byte locale
Latin-1 built with
localedef -c, as #1153'stest_adopt_one_byte_locale.pybuilds it (LOCPATH,LC_ALL=en_US.ISO-8859-1,PYTHONUTF8=0).--work-dirrow.skip_where_a_name_cannot_be_not_utf_8(tests/conftest.py) also skips, before the disk is touched, whereos.fsdecode(b"\xff")is not U+DCFF. Latin-1 reads the byte as U+00FF, which is printable, so_shownrightly prints it as typed. The disk andos.fsencodetake the byte in every locale, so onlyos.fsdecodecan say.run_scripttakesencoding(None unless named, so no other caller changes), and the fourrun_scriptcalls of the spelled file name"utf-8": every child there is toldPYTHONIOENCODING=utf-8.test_non_utf8_name_callers.py's fixturea_system_that_carries_the_byte(whichtest_non_utf8_git_failures.pyandtest_non_utf8_oserror_sites.pyuse) and three rows oftest_non_utf8_name_helpers.pyguard the rows that let the probe through to the disk. They skip where the helper skips on the new question, as they do for the first. Without it, 28 of them failed under Latin-1 where they passed.N2: the bytes helper's environment
plan_of_a_repository_on_github_as_bytesbuilds its child's environment withblank_unnamed_pin_sourcesand the git identity defaults, asrun_scriptandtest_stdout_encoding.py'srun_tool_bytesdo. Read from the helper with a pin-source override and a lane set, and no identity, in this process: before,SHAPE_PIN_SOURCE_OPENXDOXwas'/somewhere/else'and the fourGIT_*names were absent; now it is''and they are the suite'sIDENTITY_NAMEandIDENTITY_EMAIL. The run ends at the clone refusal, so nothing it says changes, and no row can see it.Rows added
Five, and no other row changes: three in
test_non_utf8_name_helpers.py(the new question asked of an emulated Latin-1os.fsdecode, before the disk; UTF-8 and ASCII controls) and two in the newtests/test_run_script_encoding.py(stdout and stderr read in the encoding named).Counts
d193617c7)LC_ALL=C.UTF-8)LC_ALL=C PYTHONUTF8=0 PYTHONCOERCECLOCALE=0)--outrows oftest_stdout_encoding.py, Latin-1 (RV1185's run)test_repo_hygiene.py(23 files; 24 after, with the new one), UTF-8, on the headorigin/main961640c68and the headUnder UTF-8 every one of the 1086 tests that ran before has the outcome it had (compared by test id from the junit files): no new skip. The one skip is the Windows default-stdout row. Under Latin-1, over the same files: 0 tests went from passed to failed, 25 from failed to passed or skipped, 80 from passed to skipped (they ask for a name that is not UTF-8, which Latin-1 has none of), and 50 fail there before and after (not these files' defect: a one-byte locale is not a configuration they run under).
Mutants
Each restored, sha256 checked. "UTF-8" and "Latin-1" name the locale the run was made in.
..._skips_before_the_diskrow; Latin-1: it, the 0xff row and the 2test_stdout_encoding.pyrows..._skips_before_the_disktest_non_utf8_name_helpers.py; Latin-1: 4run_scriptdropsencodinglatin-1row oftest_run_script_encoding.py; Latin-1: both its rows and both U+00E9 rowsplan_of_a_local_source/plan_of_a_repository_on_githubdropsencoding--out/--work-dirU+00E9 rowadopt-project.pyblank_unnamed_pin_sources, or without the identity defaultsos.fsdecode(os.fsencode(probe))Merge-trees
git merge-tree --write-treeof the head againstorigin/main, #1185's head, #1154's head and each of the 55 open PRs' heads: clean, except #1117 and #1201, which conflict atdocs/handbook.htmlagainstorigin/mainitself, with or without this branch. No open PR but #1154 and #1185 touches any of the five files.Left
Outside this issue: 50 tests of these 24 files fail under Latin-1 before and after, as do #1154's own
test_stdout_encoding.pyrows that name U+65E5 U+672C (now skipped by this change where they ask the helper).🤖 Generated with Claude Code
Lane: openreposhape-4 (openRepoShape-4)
Summary by Sourcery
Make the test suite correctly account for locale-dependent filename decoding and subprocess output encoding.
Bug Fixes:
Enhancements:
Tests: