From 9aac80dafc1e54e90a46d3707a1d627f3cb0cf13 Mon Sep 17 00:00:00 2001
From: Brian Gan <brgan@google.com>
Date: Fri, 23 Oct 2026 01:57:39 +0000
Subject: [PATCH] tests: e2e: fix harness on Windows, CI & inside git checkouts

Normalize the `\r\n` that Python on Windows writes to pipes in repo's
output so tests can match multi-line messages.  Pass a str to
`os.readlink`, which only accepts Path objects on Windows since Python
3.8.  Set `GIT_CEILING_DIRECTORIES` for the client so git never finds a
repo above the test dir (e.g. when running with `--basetemp=./tmp` from
a checkout).

Run the real git rather than a `git` wrapper that's first on PATH: the
real git's exec dir (from `git --exec-path`) has a copy of git, so put
it first on PATH.  LUCI's wrapper, for one, retries server errors for
minutes, which hangs tests that simulate them.

Bug: 566270944
Change-Id: Ia0ed5285852638e533632a7272f154239f1e834f
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/648543
Reviewed-by: Gavin Mak <gavinmak@google.com>
Tested-by: Brian Gan <brgan@google.com>
Commit-Queue: Brian Gan <brgan@google.com>
---
 tests/e2e/conftest.py | 10 ++++++++--
 tests/e2e/harness.py  | 43 ++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/tests/e2e/conftest.py b/tests/e2e/conftest.py
index 531510c7c..8159824d1 100644
--- a/tests/e2e/conftest.py
+++ b/tests/e2e/conftest.py
@@ -113,8 +113,14 @@ def client_env(tmp_path: Path, git_defaults: str) -> Dict[str, str]:
         pytest.skip("git is too old to emulate Git 3 defaults")
     settings.update(harness.GIT_DEFAULTS[git_defaults])
     harness.write_config(home / ".gitconfig", settings)
-    # Python resolves `~` via USERPROFILE on Windows.
-    return dict(harness.clean_environ(), HOME=str(home), USERPROFILE=str(home))
+    # Python resolves `~` via USERPROFILE on Windows.  The ceiling keeps git
+    # from finding a repo above the test dir (e.g. with `--basetemp=./tmp`).
+    return dict(
+        harness.clean_environ(),
+        HOME=str(home),
+        USERPROFILE=str(home),
+        GIT_CEILING_DIRECTORIES=str(tmp_path),
+    )
 
 
 @pytest.fixture
diff --git a/tests/e2e/harness.py b/tests/e2e/harness.py
index c5e1511b6..0cd1d319a 100644
--- a/tests/e2e/harness.py
+++ b/tests/e2e/harness.py
@@ -161,9 +161,43 @@ def clean_environ() -> Dict[str, str]:
     env["GIT_CONFIG_NOSYSTEM"] = "1"
     # Tests match on English git & repo output.
     env["LC_ALL"] = "C"
+    exec_dir = _git_exec_dir()
+    if exec_dir:
+        env["PATH"] = os.pathsep.join([exec_dir, env.get("PATH", "")])
     return env
 
 
+@functools.lru_cache(maxsize=None)
+def _git_exec_dir() -> Optional[str]:
+    """Return git's exec dir if it has a `git` binary, to put first on PATH.
+
+    `git` on PATH may be a wrapper, e.g. LUCI's, which retries the server
+    errors tests simulate for minutes.  The real git answers `--exec-path`
+    (wrappers pass it through) & installs a copy of itself there, so putting
+    that dir first makes the harness, repo & the git that repo runs all use the
+    real git.
+    """
+    env = {k: v for k, v in os.environ.items() if not k.startswith("GIT_")}
+    try:
+        proc = subprocess.run(
+            ["git", "--exec-path"],
+            env=env,
+            stdout=subprocess.PIPE,
+            stderr=subprocess.DEVNULL,
+            check=False,
+            timeout=COMMAND_TIMEOUT,
+        )
+    except OSError:
+        return None
+    exec_dir = proc.stdout.decode("utf-8").strip()
+    if proc.returncode or not any(
+        os.path.isfile(os.path.join(exec_dir, name))
+        for name in ("git", "git.exe")
+    ):
+        return None
+    return exec_dir
+
+
 def run_git(
     *args: str,
     cwd: Optional[Union[str, Path]] = None,
@@ -759,7 +793,8 @@ def _snapshot_worktree(root: Path) -> Dict[str, FileContent]:
             # Deleted in the worktree but not (yet) in the index.
             continue
         if stat.S_ISLNK(st_mode):
-            files[name] = Blob(os.fsencode(os.readlink(path)), "120000")
+            # Python <3.8 on Windows rejects Path objects here.
+            files[name] = Blob(os.fsencode(os.readlink(str(path))), "120000")
         elif stat.S_ISREG(st_mode):
             mode = "100755" if st_mode & 0o111 else "100644"
             files[name] = Blob(path.read_bytes(), mode)
@@ -853,11 +888,13 @@ class RepoClient:
             timeout=COMMAND_TIMEOUT,
         )
         elapsed = time.monotonic() - start
+        # Python on Windows writes `\r\n` to pipes; normalize so tests can match
+        # multi-line output.
         result = RepoResult(
             list(args),
             proc.returncode,
-            proc.stdout.decode("utf-8", "replace"),
-            proc.stderr.decode("utf-8", "replace"),
+            proc.stdout.decode("utf-8", "replace").replace("\r\n", "\n"),
+            proc.stderr.decode("utf-8", "replace").replace("\r\n", "\n"),
         )
         print(f"$ {cmd}  # exit {result.returncode} after {elapsed:.1f}s")
         if result.output: