Skip to content

Commit 3edc8b9

Browse files
committed
Make streaming and git handler tests pass on Windows
Both suites failed only on the Windows CI, for reasons in the tests rather than the code under test. - Command runner streaming tests wrote through text-mode stdout/stderr, where Windows translates `\n` to `\r\n`. A literal `\r\n` therefore became `\r\r\n` and the buffered/CRLF assertions saw different bytes. Write bytes via `sys.stdout.buffer` / `sys.stderr.buffer` instead so the child emits exactly what the test asserts on. - fine_git handler tests used a POSIX literal `/repo` as the fake toplevel. It has no drive on Windows, so it is not absolute and cannot become a `file://` URI. Add a `repo_root` fixture backed by `tmp_path` and a `toplevel_result` helper that prints it with forward slashes, as Git for Windows does, and derive expected URIs from it.
1 parent 9f1e0fb commit 3edc8b9

4 files changed

Lines changed: 87 additions & 43 deletions

File tree

‎finecode_extension_runner/tests/test_command_runner_streaming.py‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,13 @@ async def test_subscribing_late_replays_what_was_already_produced() -> None:
135135
async def test_buffered_output_is_unchanged_for_callers_that_never_subscribe() -> None:
136136
"""The path every existing handler uses, including the trailing newline."""
137137
process = await _runner().run(
138-
_python("import sys\nprint('out')\nsys.stderr.write('err\\n')")
138+
_python(
139+
"import sys\n"
140+
"sys.stdout.buffer.write(b'out\\n')\n"
141+
"sys.stdout.buffer.flush()\n"
142+
"sys.stderr.buffer.write(b'err\\n')\n"
143+
"sys.stderr.buffer.flush()\n"
144+
)
139145
)
140146
await process.wait_for_end()
141147

@@ -233,8 +239,8 @@ async def test_stderr_is_complete_when_a_stdout_failure_surfaces() -> None:
233239
"import sys\n"
234240
f"sys.stdout.write('x' * {oversized})\n"
235241
"sys.stdout.flush()\n"
236-
"sys.stderr.write('the real error message\\n')\n"
237-
"sys.stderr.flush()\n"
242+
"sys.stderr.buffer.write(b'the real error message\\n')\n"
243+
"sys.stderr.buffer.flush()\n"
238244
)
239245
)
240246

@@ -366,10 +372,13 @@ async def test_a_trailing_carriage_return_without_a_newline_is_data() -> None:
366372

367373

368374
async def test_crlf_line_endings_are_still_stripped() -> None:
369-
"""The `\\r` that does precede a `\\n` is part of the terminator."""
375+
"""The `\\r` that does precede a `\\n` is part of the terminator. It writes
376+
bytes because a text-mode stdout on Windows would turn the literal `\\r\\n`
377+
into `\\r\\r\\n`."""
370378
process = await _runner().run(
371379
_python(
372-
"import sys\nsys.stdout.write('one\\r\\ntwo\\r\\n')\nsys.stdout.flush()"
380+
"import sys\nsys.stdout.buffer.write(b'one\\r\\ntwo\\r\\n')\n"
381+
"sys.stdout.buffer.flush()"
373382
)
374383
)
375384

‎presets/fine_git/tests/conftest.py‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
from collections.abc import AsyncIterator
55
from pathlib import Path
66

7+
import pytest
78
from finecode_extension_api.interfaces import icommandrunner
89

910

@@ -57,3 +58,21 @@ async def run(
5758
icommandrunner.check_argv(cmd)
5859
self.commands.append(list(cmd))
5960
return self._results.pop(0)
61+
62+
63+
@pytest.fixture
64+
def repo_root(tmp_path: Path) -> Path:
65+
"""A repository toplevel that is absolute on every OS.
66+
67+
A POSIX literal such as ``/repo`` has no drive on Windows, so it is not
68+
absolute there and cannot become a ``file://`` URI.
69+
"""
70+
return tmp_path
71+
72+
73+
def toplevel_result(repo_root: Path) -> FakeCommandResult:
74+
"""``git rev-parse --show-toplevel`` output for *repo_root*.
75+
76+
Git prints forward slashes on every OS, Git for Windows included.
77+
"""
78+
return FakeCommandResult(exit_code=0, stdout=f"{repo_root.as_posix()}\n")

‎presets/fine_git/tests/test_get_git_diff_handler.py‎

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
from pathlib import Path
44

55
import pytest
6-
from conftest import FakeCommandResult, FakeCommandRunner
6+
from conftest import FakeCommandResult, FakeCommandRunner, toplevel_result
77
from finecode_extension_api.interfaces.icommandrunner import ICommandRunner
88
from finecode_extension_runner.testing import run_handler
99

@@ -43,14 +43,16 @@
4343

4444

4545
@pytest.mark.asyncio
46-
async def test_two_file_diff_parses_change_kind_and_added_removed_lines() -> None:
46+
async def test_two_file_diff_parses_change_kind_and_added_removed_lines(
47+
repo_root: Path,
48+
) -> None:
4749
"""A modified file and an added file must each decode to the right
4850
change_kind, with added_lines/removed_lines stripped of their +/- markers
4951
and the +++/--- header lines excluded, while patch keeps the diff --git
5052
header verbatim."""
5153
command_runner = FakeCommandRunner(
5254
results=[
53-
FakeCommandResult(exit_code=0, stdout="/repo\n"),
55+
toplevel_result(repo_root),
5456
FakeCommandResult(exit_code=0, stdout=_MODIFIED_SECTION + _ADDED_SECTION),
5557
]
5658
)
@@ -67,27 +69,29 @@ async def test_two_file_diff_parses_change_kind_and_added_removed_lines() -> Non
6769

6870
modified, added = result.files
6971

70-
assert modified.path == "file:///repo/mod.txt"
72+
assert modified.path == (repo_root / "mod.txt").as_uri()
7173
assert modified.change_kind == GitChangeKind.MODIFIED
7274
assert modified.added_lines == ["new line"]
7375
assert modified.removed_lines == ["old line"]
7476
assert modified.patch.startswith("diff --git a/mod.txt b/mod.txt\n")
7577
assert modified.is_binary is False
7678

77-
assert added.path == "file:///repo/new.txt"
79+
assert added.path == (repo_root / "new.txt").as_uri()
7880
assert added.change_kind == GitChangeKind.ADDED
7981
assert added.added_lines == ["new content"]
8082
assert added.removed_lines == []
8183

8284

8385
@pytest.mark.asyncio
84-
async def test_binary_diff_section_is_reported_without_added_removed_lines() -> None:
86+
async def test_binary_diff_section_is_reported_without_added_removed_lines(
87+
repo_root: Path,
88+
) -> None:
8589
"""A `Binary files ... differ` section carries no hunks to scan for +/-
8690
lines, so it must set is_binary and leave added/removed lines empty
8791
rather than the parser tripping over the absence of a `@@` marker."""
8892
command_runner = FakeCommandRunner(
8993
results=[
90-
FakeCommandResult(exit_code=0, stdout="/repo\n"),
94+
toplevel_result(repo_root),
9195
FakeCommandResult(exit_code=0, stdout=_BINARY_SECTION),
9296
]
9397
)
@@ -107,13 +111,15 @@ async def test_binary_diff_section_is_reported_without_added_removed_lines() ->
107111

108112

109113
@pytest.mark.asyncio
110-
async def test_staged_source_and_context_lines_reach_the_diff_command() -> None:
114+
async def test_staged_source_and_context_lines_reach_the_diff_command(
115+
repo_root: Path,
116+
) -> None:
111117
"""`source=STAGED` must add `--cached` and `context_lines=0` must add
112118
`-U0` to the `git diff` invocation, or a caller's request is silently
113119
ignored."""
114120
command_runner = FakeCommandRunner(
115121
results=[
116-
FakeCommandResult(exit_code=0, stdout="/repo\n"),
122+
toplevel_result(repo_root),
117123
FakeCommandResult(exit_code=0, stdout=""),
118124
]
119125
)
@@ -133,14 +139,15 @@ async def test_staged_source_and_context_lines_reach_the_diff_command() -> None:
133139
@pytest.mark.asyncio
134140
async def test_paths_none_scopes_diff_to_the_project_directory(
135141
tmp_path: Path,
142+
repo_root: Path,
136143
) -> None:
137144
"""`paths=None` means the project directory, not the repository. `git diff`
138145
ignores cwd and covers the whole repository unless a pathspec says otherwise,
139146
so the handler must name the project directory itself -- without it, a nested
140147
project's diff would include every other project in the same repository."""
141148
command_runner = FakeCommandRunner(
142149
results=[
143-
FakeCommandResult(exit_code=0, stdout="/repo\n"),
150+
toplevel_result(repo_root),
144151
FakeCommandResult(exit_code=0, stdout=""),
145152
]
146153
)
@@ -176,7 +183,9 @@ async def test_paths_none_scopes_diff_to_the_project_directory(
176183

177184

178185
@pytest.mark.asyncio
179-
async def test_content_lines_starting_with_dashes_or_pluses_are_not_dropped() -> None:
186+
async def test_content_lines_starting_with_dashes_or_pluses_are_not_dropped(
187+
repo_root: Path,
188+
) -> None:
180189
"""Inside a hunk, a removed `---` arrives as `----` and an added `+++more` as
181190
`++++more` (verified against real git). Excluding those prefixes as if they
182191
were file headers loses real content, and `added_lines`/`removed_lines` then
@@ -185,7 +194,7 @@ async def test_content_lines_starting_with_dashes_or_pluses_are_not_dropped() ->
185194
corner one."""
186195
command_runner = FakeCommandRunner(
187196
results=[
188-
FakeCommandResult(exit_code=0, stdout="/repo\n"),
197+
toplevel_result(repo_root),
189198
FakeCommandResult(exit_code=0, stdout=_MARKDOWN_RULE_SECTION),
190199
]
191200
)
@@ -203,13 +212,15 @@ async def test_content_lines_starting_with_dashes_or_pluses_are_not_dropped() ->
203212

204213

205214
@pytest.mark.asyncio
206-
async def test_section_naming_no_path_is_skipped_not_fatal_for_the_whole_diff() -> None:
215+
async def test_section_naming_no_path_is_skipped_not_fatal_for_the_whole_diff(
216+
repo_root: Path,
217+
) -> None:
207218
"""A mode-only change on a path git had to quote carries no `---`/`+++` lines
208219
and a header the parser cannot match (real git output). One unparseable
209220
section must cost that one file, not the whole answer."""
210221
command_runner = FakeCommandRunner(
211222
results=[
212-
FakeCommandResult(exit_code=0, stdout="/repo\n"),
223+
toplevel_result(repo_root),
213224
FakeCommandResult(
214225
exit_code=0,
215226
stdout=_MODE_CHANGE_QUOTED_PATH_SECTION + _MODIFIED_SECTION,
@@ -225,4 +236,4 @@ async def test_section_naming_no_path_is_skipped_not_fatal_for_the_whole_diff()
225236
)
226237

227238
assert result.error is None
228-
assert [file.path for file in result.files] == ["file:///repo/mod.txt"]
239+
assert [file.path for file in result.files] == [(repo_root / "mod.txt").as_uri()]

‎presets/fine_git/tests/test_get_git_status_handler.py‎

Lines changed: 28 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
from pathlib import Path
44

55
import pytest
6-
from conftest import FakeCommandResult, FakeCommandRunner
6+
from conftest import FakeCommandResult, FakeCommandRunner, toplevel_result
77
from finecode_extension_api.interfaces.icommandrunner import ICommandRunner
88
from finecode_extension_runner.testing import run_handler
99

@@ -13,7 +13,9 @@
1313

1414

1515
@pytest.mark.asyncio
16-
async def test_porcelain_output_is_parsed_including_the_two_record_rename() -> None:
16+
async def test_porcelain_output_is_parsed_including_the_two_record_rename(
17+
repo_root: Path,
18+
) -> None:
1719
"""A modified file, a staged-and-modified file, an untracked file, and a rename
1820
(whose original path arrives as a second, unprefixed -z record) must all decode
1921
to the right FileStatus -- the rename encoding is the easy part to get wrong."""
@@ -27,7 +29,7 @@ async def test_porcelain_output_is_parsed_including_the_two_record_rename() -> N
2729
status_stdout = "\0".join(records) + "\0"
2830
command_runner = FakeCommandRunner(
2931
results=[
30-
FakeCommandResult(exit_code=0, stdout="/repo\n"),
32+
toplevel_result(repo_root),
3133
FakeCommandResult(exit_code=0, stdout=status_stdout),
3234
]
3335
)
@@ -39,31 +41,31 @@ async def test_porcelain_output_is_parsed_including_the_two_record_rename() -> N
3941
service_overrides={ICommandRunner: command_runner},
4042
)
4143

42-
assert result.repo_root == "file:///repo"
44+
assert result.repo_root == repo_root.as_uri()
4345
assert result.error is None
4446
assert len(result.changes) == 4
4547

4648
modified, staged_and_modified, untracked, renamed = result.changes
4749

48-
assert modified.path == "file:///repo/modified.txt"
50+
assert modified.path == (repo_root / "modified.txt").as_uri()
4951
assert modified.index_status == GitChangeKind.UNMODIFIED
5052
assert modified.worktree_status == GitChangeKind.MODIFIED
5153
assert modified.original_path is None
5254

53-
assert staged_and_modified.path == "file:///repo/staged_and_modified.txt"
55+
assert staged_and_modified.path == (repo_root / "staged_and_modified.txt").as_uri()
5456
assert staged_and_modified.index_status == GitChangeKind.MODIFIED
5557
assert staged_and_modified.worktree_status == GitChangeKind.MODIFIED
5658
assert staged_and_modified.original_path is None
5759

58-
assert untracked.path == "file:///repo/untracked.txt"
60+
assert untracked.path == (repo_root / "untracked.txt").as_uri()
5961
assert untracked.index_status == GitChangeKind.UNTRACKED
6062
assert untracked.worktree_status == GitChangeKind.UNTRACKED
6163
assert untracked.original_path is None
6264

63-
assert renamed.path == "file:///repo/new_name.txt"
65+
assert renamed.path == (repo_root / "new_name.txt").as_uri()
6466
assert renamed.index_status == GitChangeKind.RENAMED
6567
assert renamed.worktree_status == GitChangeKind.UNMODIFIED
66-
assert renamed.original_path == "file:///repo/old_name.txt"
68+
assert renamed.original_path == (repo_root / "old_name.txt").as_uri()
6769

6870

6971
@pytest.mark.asyncio
@@ -88,12 +90,12 @@ async def test_not_a_git_repository_is_a_result_state_not_an_error() -> None:
8890

8991

9092
@pytest.mark.asyncio
91-
async def test_empty_paths_list_skips_status_but_still_reports_repo_root() -> None:
93+
async def test_empty_paths_list_skips_status_but_still_reports_repo_root(
94+
repo_root: Path,
95+
) -> None:
9296
"""`paths=[]` means nothing was requested -- the handler reports the repo root
9397
it already resolved without spending a `git status` call on an empty request."""
94-
command_runner = FakeCommandRunner(
95-
results=[FakeCommandResult(exit_code=0, stdout="/repo\n")]
96-
)
98+
command_runner = FakeCommandRunner(results=[toplevel_result(repo_root)])
9799

98100
result = await run_handler(
99101
GitGetGitStatusHandler,
@@ -102,23 +104,23 @@ async def test_empty_paths_list_skips_status_but_still_reports_repo_root() -> No
102104
service_overrides={ICommandRunner: command_runner},
103105
)
104106

105-
assert result.repo_root == "file:///repo"
107+
assert result.repo_root == repo_root.as_uri()
106108
assert result.changes == []
107109
assert len(command_runner.commands) == 1
108110

109111

110112
@pytest.mark.asyncio
111-
async def test_include_ignored_without_untracked_asks_git_for_both_and_filters() -> (
112-
None
113-
):
113+
async def test_include_ignored_without_untracked_asks_git_for_both_and_filters(
114+
repo_root: Path,
115+
) -> None:
114116
"""`include_ignored=True` with `include_untracked=False` is not expressible as
115117
git flags: `-uno --ignored=matching` is `fatal: Unsupported combination of
116118
ignored and untracked-files arguments`, and `-uno --ignored=traditional`
117119
reports no ignored files at all. The handler therefore asks with `-uall` and
118120
drops the untracked entries itself."""
119121
command_runner = FakeCommandRunner(
120122
results=[
121-
FakeCommandResult(exit_code=0, stdout="/repo\n"),
123+
toplevel_result(repo_root),
122124
FakeCommandResult(
123125
exit_code=0, stdout="!! build.log\0?? scratch.txt\0 M tracked.py\0"
124126
),
@@ -138,18 +140,20 @@ async def test_include_ignored_without_untracked_asks_git_for_both_and_filters()
138140
assert "-uno" not in status_cmd
139141

140142
assert [change.path for change in result.changes] == [
141-
"file:///repo/build.log",
142-
"file:///repo/tracked.py",
143+
(repo_root / "build.log").as_uri(),
144+
(repo_root / "tracked.py").as_uri(),
143145
]
144146

145147

146148
@pytest.mark.asyncio
147-
async def test_excluding_untracked_alone_still_asks_git_to_suppress_them() -> None:
149+
async def test_excluding_untracked_alone_still_asks_git_to_suppress_them(
150+
repo_root: Path,
151+
) -> None:
148152
"""Without `include_ignored` there is nothing to widen for, so the cheaper
149153
`-uno` is used and git never enumerates untracked files."""
150154
command_runner = FakeCommandRunner(
151155
results=[
152-
FakeCommandResult(exit_code=0, stdout="/repo\n"),
156+
toplevel_result(repo_root),
153157
FakeCommandResult(exit_code=0, stdout=""),
154158
]
155159
)
@@ -169,14 +173,15 @@ async def test_excluding_untracked_alone_still_asks_git_to_suppress_them() -> No
169173
@pytest.mark.asyncio
170174
async def test_paths_none_scopes_status_to_the_project_directory(
171175
tmp_path: Path,
176+
repo_root: Path,
172177
) -> None:
173178
"""`paths=None` means the project directory, not the repository. `git status`
174179
ignores cwd and reports the whole repository unless a pathspec says otherwise,
175180
so the handler must name the project directory itself -- without it, a nested
176181
project's status would include every other project in the same repository."""
177182
command_runner = FakeCommandRunner(
178183
results=[
179-
FakeCommandResult(exit_code=0, stdout="/repo\n"),
184+
toplevel_result(repo_root),
180185
FakeCommandResult(exit_code=0, stdout=""),
181186
]
182187
)

0 commit comments

Comments
 (0)