test: do pre-PR A/B-test checkout into temporary directory

Otherwise, they clash with the directories used for the pre-compiled
binaries. Since each pipeline only uses git_ab_test in precisely one
test, loosing the re-use is not a big deal.

Signed-off-by: Patrick Roy <[email protected]>

Author: roypat

tests/framework/ab_test.py

@@ -25,14 +25,14 @@
 import os
 import statistics
 from pathlib import Path
+from tempfile import TemporaryDirectory
 from typing import Callable, List, Optional, TypeVar
 
 import scipy
 
 from framework import utils
 from framework.microvm import Microvm
 from framework.utils import CommandReturn
-from framework.with_filelock import with_filelock
 from host_tools.cargo_build import get_binary, get_firecracker_binaries
 
 # Locally, this will always compare against main, even if we try to merge into, say, a feature branch.
@@ -82,19 +82,20 @@ def git_ab_test(
              (alternatively, your comparator can perform any required assertions and not return anything).
     """
 
-    dir_a = git_clone(Path("../build") / a_revision, a_revision)
-    result_a = test_runner(dir_a, True)
+    with TemporaryDirectory() as tmp_dir:
+        dir_a = git_clone(Path(tmp_dir) / a_revision, a_revision)
+        result_a = test_runner(dir_a, True)
 
-    if b_revision:
-        dir_b = git_clone(Path("../build") / b_revision, b_revision)
-    else:
-        # By default, pytest execution happens inside the `tests` subdirectory. Pass the repository root, as
-        # documented.
-        dir_b = Path.cwd().parent
-    result_b = test_runner(dir_b, False)
+        if b_revision:
+            dir_b = git_clone(Path(tmp_dir) / b_revision, b_revision)
+        else:
+            # By default, pytest execution happens inside the `tests` subdirectory. Pass the repository root, as
+            # documented.
+            dir_b = Path.cwd().parent
+        result_b = test_runner(dir_b, False)
 
-    comparison = comparator(result_a, result_b)
-    return result_a, result_b, comparison
+        comparison = comparator(result_a, result_b)
+        return result_a, result_b, comparison
 
 
 def precompiled_binary_ab_test(
@@ -249,26 +250,22 @@ def check_regression(
     )
 
 
-@with_filelock
 def git_clone(clone_path, commitish):
     """Clone the repository at `commit`.
 
     :return: the working copy directory.
     """
-    if not clone_path.exists():
-        ret, _, _ = utils.run_cmd(f"git cat-file -t {commitish}")
-        if ret != 0:
-            # git didn't recognize this object; qualify it if it is a branch
-            commitish = f"origin/{commitish}"
-        # make a temp branch for that commit so we can directly check it out
-        branch_name = f"tmp-{commitish}"
-        utils.check_output(f"git branch {branch_name} {commitish}")
-        _, git_root, _ = utils.run_cmd("git rev-parse --show-toplevel")
-        # split off the '\n' at the end of the stdout
-        utils.check_output(
-            f"git clone -b {branch_name} {git_root.strip()} {clone_path}"
-        )
-        utils.check_output(f"git branch -D {branch_name}")
+    ret, _, _ = utils.run_cmd(f"git cat-file -t {commitish}")
+    if ret != 0:
+        # git didn't recognize this object; qualify it if it is a branch
+        commitish = f"origin/{commitish}"
+    # make a temp branch for that commit so we can directly check it out
+    branch_name = f"tmp-{commitish}"
+    utils.check_output(f"git branch {branch_name} {commitish}")
+    _, git_root, _ = utils.run_cmd("git rev-parse --show-toplevel")
+    # split off the '\n' at the end of the stdout
+    utils.check_output(f"git clone -b {branch_name} {git_root.strip()} {clone_path}")
+    utils.check_output(f"git branch -D {branch_name}")
     return clone_path