Merge pull request #414 from mdboom/add-unit-tests

Improve test coverage

Author: mdboom

.github/workflows/ci.yml

@@ -32,9 +32,40 @@ jobs:
         with:
           python-version: ${{ matrix.python_version }}
           cache: pip
-      - name: Install dependencies
+      - name: install dependencies
+        run: |
+          python -m pip install .[test]
+      - name: run tests
+        run: |
+          python -m pytest tests -n auto -m "not long_running"
+
+  long_test_linux:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-python@v5
+        with:
+          python-version: 3.13
+          cache: pip
+      - name: install dependencies
         run: |
           python -m pip install .[test]
-      - name: Run tests
+      - name: run tests
         run: |
-          python -m pytest tests -n auto
+          python -m pytest tests -n auto -m long_running
+
+  long_test_windows:
+    runs-on: windows-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-python@v5
+        with:
+          python-version: 3.13
+          cache: pip
+      - name: install dependencies
+        run: |
+          python -m pip install .[test]
+      - name: run tests
+        run: |
+          python -m pytest tests -n auto -m long_running
+

bench_runner/config.py

@@ -9,20 +9,24 @@
 
 
 from . import runners
+from .util import PathLike
 
 
 @functools.cache
-def get_bench_runner_config(
-    filepath: Path | str = Path("bench_runner.toml"),
-):
-    with Path(filepath).open("rb") as fd:
+def get_bench_runner_config(filepath: PathLike | None = None):
+    if filepath is None:
+        filepath = Path("bench_runner.toml")
+    else:
+        filepath = Path(filepath)
+
+    with filepath.open("rb") as fd:
         return tomllib.load(fd)
 
 
-def get_config_for_current_runner() -> dict[str, Any]:
-    config = get_bench_runner_config()
-    runner = runners.get_runner_for_hostname()
-    all_runners = config.get("runners", [])
+def get_config_for_current_runner(filepath: PathLike | None = None) -> dict[str, Any]:
+    config = get_bench_runner_config(filepath)
+    runner = runners.get_runner_for_hostname(cfgpath=filepath)
+    all_runners = config.get("runners", {})
     if len(all_runners) >= 1:
-        return all_runners[0].get(runner.nickname, {})
+        return all_runners.get(runner.nickname, {})
     return {}

bench_runner/runners.py

@@ -7,6 +7,7 @@
 
 
 from . import config
+from .util import PathLike
 
 
 class Runner:
@@ -47,8 +48,8 @@ def display_name(self) -> str:
 
 
 @functools.cache
-def get_runners() -> list[Runner]:
-    conf = config.get_bench_runner_config().get("runners", {})
+def get_runners(cfgpath: PathLike | None = None) -> list[Runner]:
+    conf = config.get_bench_runner_config(cfgpath).get("runners", {})
     runners = []
     for nickname, section in conf.items():
         runners.append(
@@ -73,27 +74,31 @@ def get_runners() -> list[Runner]:
     return runners
 
 
-def get_runners_by_hostname() -> dict[str, Runner]:
-    return {x.hostname: x for x in get_runners()}
+def get_runners_by_hostname(cfgpath: PathLike | None = None) -> dict[str, Runner]:
+    return {x.hostname: x for x in get_runners(cfgpath)}
 
 
-def get_runners_by_nickname() -> dict[str, Runner]:
-    return {x.nickname: x for x in get_runners()}
+def get_runners_by_nickname(cfgpath: PathLike | None = None) -> dict[str, Runner]:
+    return {x.nickname: x for x in get_runners(cfgpath)}
 
 
-def get_nickname_for_hostname(hostname: str | None = None) -> str:
+def get_nickname_for_hostname(
+    hostname: str | None = None, cfgpath: PathLike | None = None
+) -> str:
     # The envvar BENCHMARK_MACHINE_NICKNAME is used to override the machine that
     # results are reported for.
     if "BENCHMARK_MACHINE_NICKNAME" in os.environ:
         return os.environ["BENCHMARK_MACHINE_NICKNAME"]
-    return get_runner_for_hostname(hostname).nickname
+    return get_runner_for_hostname(hostname, cfgpath).nickname
 
 
-def get_runner_by_nickname(nickname: str) -> Runner:
-    return get_runners_by_nickname().get(nickname, unknown_runner)
+def get_runner_by_nickname(nickname: str, cfgpath: PathLike | None = None) -> Runner:
+    return get_runners_by_nickname(cfgpath).get(nickname, unknown_runner)
 
 
-def get_runner_for_hostname(hostname: str | None = None) -> Runner:
+def get_runner_for_hostname(
+    hostname: str | None = None, cfgpath: PathLike | None = None
+) -> Runner:
     if hostname is None:
         hostname = socket.gethostname()
-    return get_runners_by_hostname().get(hostname, unknown_runner)
+    return get_runners_by_hostname(cfgpath).get(hostname, unknown_runner)

bench_runner/scripts/install.py

@@ -308,18 +308,29 @@ def generate_generic(dst: Any) -> Any:
 def _main(check: bool) -> None:
     WORKFLOW_PATH.mkdir(parents=True, exist_ok=True)
 
+    # Copy the bench_runner.toml file first, because it might be needed by the
+    # other steps
+
+    if not (ROOT_PATH / "bench_runner.toml").is_file():
+        shutil.copyfile(
+            TEMPLATE_PATH / "bench_runner.toml",
+            ROOT_PATH / "bench_runner.toml",
+        )
+
     for src_path in TEMPLATE_PATH.glob("*"):
         if not src_path.is_file():
             continue
 
+        print(f"Processing {src_path}...")
+
         if src_path.name.endswith(".src.yml"):
             dst_path = WORKFLOW_PATH / (src_path.name[:-8] + ".yml")
             generator = GENERATORS.get(src_path.name, generate_generic)
             src = load_yaml(src_path)
             dst = generator(src)
             write_yaml(dst_path, dst, check)
         elif src_path.name.endswith(".src.py"):
-            dst_path = WORKFLOW_PATH / (src_path.name[:-7] + ".py")
+            dst_path = ROOT_PATH / (src_path.name[:-7] + ".py")
             write_python(dst_path, src_path.read_text(), check)
         else:
             dst_path = ROOT_PATH / src_path.name

bench_runner/scripts/workflow.py

@@ -254,6 +254,7 @@ def _main(
     pystats: bool,
     force_32bit: bool,
     run_id: str | None = None,
+    fast: bool = False,
 ):
     venv = Path("venv")
     cpython = Path("cpython")
@@ -270,7 +271,9 @@ def _main(
         checkout_cpython(fork, ref, cpython)
 
     with log_group("Determining if we need to run benchmarks"):
-        if not should_run(force, fork, ref, machine, False, flags, cpython=cpython):
+        if not fast and not should_run(
+            force, fork, ref, machine, False, flags, cpython=cpython
+        ):
             print("No need to run benchmarks.  Skipping...")
             return
 
@@ -291,8 +294,9 @@ def _main(
     with log_group("Installing pyperformance"):
         install_pyperformance(venv)
 
-    with log_group("Tuning system"):
-        tune_system(venv, perf)
+    if not fast:
+        with log_group("Tuning system"):
+            tune_system(venv, perf)
 
     try:
         if Path(".debug").exists():
@@ -319,11 +323,12 @@ def _main(
                 benchmarks,
                 flags=flags,
                 run_id=run_id,
-                test_mode=False,
+                test_mode=fast,
                 individual=pystats,
             )
     finally:
-        reset_system(venv)
+        if not fast:
+            reset_system(venv)
 
 
 def main():
@@ -364,6 +369,9 @@ def main():
         help="Do a 32-bit build (Windows only)",
     )
     parser.add_argument("--run_id", default=None, type=str, help="The github run id")
+    parser.add_argument(
+        "--_fast", action="store_true", help="Use fast mode, for testing"
+    )
     args = parser.parse_args()
 
     _main(
@@ -378,6 +386,7 @@ def main():
         args.pystats,
         args.force_32bit,
         args.run_id,
+        args._fast,
     )
 
 

bench_runner/util.py

@@ -9,9 +9,6 @@
 from typing import Iterable, Iterator, Literal, TypeAlias, Union
 
 
-from . import config
-
-
 PathLike: TypeAlias = Union[str, os.PathLike]
 
 
@@ -32,6 +29,8 @@ def apply_suffix(path: PathLike, suffix: str) -> Path:
 
 @functools.cache
 def get_excluded_benchmarks() -> set[str]:
+    from . import config
+
     conf = config.get_bench_runner_config()
     benchmarks_section = conf.get("benchmarks", {})
     for key in ("excluded", "excluded_benchmarks"):

pyproject.toml

@@ -42,3 +42,8 @@ include-package-data = true
 
 [tool.setuptools.package-data]
 bench_runner = ["templates/*"]
+
+[tool.pytest.ini_options]
+markers = [
+    "long_running: marks tests as long_running (deselect with '-m \"not running\"')",
+]

tests/data/bench_runner.toml

@@ -25,3 +25,11 @@ markers = ["s", "s", "s", "^", ".", "."]
 
 [publish_mirror]
 skip = false
+
+[weekly.default]
+runners = ["linux"]
+flags = []
+
+[weekly.jit]
+runners = ["linux"]
+flags = ["JIT"]

tests/test_config.py

@@ -0,0 +1,18 @@
+from pathlib import Path
+import socket
+
+
+from bench_runner import config
+
+
+DATA_PATH = Path(__file__).parent / "data"
+
+
+def test_get_runner_for_hostname(monkeypatch):
+    monkeypatch.setattr(socket, "gethostname", lambda: "pyperf")
+
+    runner = config.get_config_for_current_runner(DATA_PATH / "bench_runner.toml")
+
+    assert runner["os"] == "linux"
+    assert runner["arch"] == "x86_64"
+    assert runner["hostname"] == "pyperf"

tests/test_flags.py

@@ -0,0 +1,20 @@
+import pytest
+
+
+from bench_runner import flags
+
+
+def test_parse_flags():
+    assert flags.parse_flags("tier2,jit") == ["PYTHON_UOPS", "JIT"]
+    assert flags.parse_flags("jit,tier2") == ["JIT", "PYTHON_UOPS"]
+    assert flags.parse_flags("jit,tier2,") == ["JIT", "PYTHON_UOPS"]
+    assert flags.parse_flags(",jit,tier2,") == ["JIT", "PYTHON_UOPS"]
+    assert flags.parse_flags(",") == []
+    assert flags.parse_flags("") == []
+
+    with pytest.raises(ValueError):
+        flags.parse_flags("tier2,jit,foo")
+
+
+def test_flags_to_human():
+    assert list(flags.flags_to_human(["PYTHON_UOPS", "JIT"])) == ["T2", "JIT"]