From b7f5de03c265e55afe8cc03046658e550e69cc08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20R=C4=85czy?= Date: Mon, 7 Apr 2025 15:47:56 -0700 Subject: [PATCH 1/5] Add test_log_compat --- selfdrive/test/fuzzy_generation.py | 7 +++-- selfdrive/test/process_replay/test_fuzzy.py | 2 +- .../test/process_replay/test_log_compat.py | 27 +++++++++++++++++++ 3 files changed, 31 insertions(+), 5 deletions(-) create mode 100644 selfdrive/test/process_replay/test_log_compat.py diff --git a/selfdrive/test/fuzzy_generation.py b/selfdrive/test/fuzzy_generation.py index 94eb0dfaa6..c9f2353713 100644 --- a/selfdrive/test/fuzzy_generation.py +++ b/selfdrive/test/fuzzy_generation.py @@ -1,11 +1,10 @@ import capnp import hypothesis.strategies as st from typing import Any +from types import ModuleType from collections.abc import Callable from functools import cache -from cereal import log - DrawType = Callable[[st.SearchStrategy], Any] @@ -76,6 +75,6 @@ class FuzzyGenerator: return data @classmethod - def get_random_event_msg(cls, draw: DrawType, events: list[str], real_floats: bool = False) -> list[dict[str, Any]]: + def get_random_event_msg(cls, draw: DrawType, log_schema: ModuleType, events: list[str], real_floats: bool = False) -> list[dict[str, Any]]: fg = cls(draw, real_floats=real_floats) - return [draw(fg.generate_struct(log.Event.schema, e)) for e in sorted(events)] + return [draw(fg.generate_struct(log_schema.Event.schema, e)) for e in sorted(events)] diff --git a/selfdrive/test/process_replay/test_fuzzy.py b/selfdrive/test/process_replay/test_fuzzy.py index 723112163e..b6ee5a7b3f 100644 --- a/selfdrive/test/process_replay/test_fuzzy.py +++ b/selfdrive/test/process_replay/test_fuzzy.py @@ -25,7 +25,7 @@ class TestFuzzProcesses: @settings(phases=[Phase.generate, Phase.target], max_examples=MAX_EXAMPLES, deadline=1000, suppress_health_check=[HealthCheck.too_slow, HealthCheck.data_too_large]) def test_fuzz_process(self, proc_name, cfg, data): - msgs = FuzzyGenerator.get_random_event_msg(data.draw, events=cfg.pubs, real_floats=True) + msgs = FuzzyGenerator.get_random_event_msg(data.draw, log_schema=log, events=cfg.pubs, real_floats=True) lr = [log.Event.new_message(**m).as_reader() for m in msgs] cfg.timeout = 5 pr.replay_process(cfg, lr, fingerprint=TOYOTA.TOYOTA_COROLLA_TSS2, disable_progress=True) diff --git a/selfdrive/test/process_replay/test_log_compat.py b/selfdrive/test/process_replay/test_log_compat.py new file mode 100644 index 0000000000..d95a0cd622 --- /dev/null +++ b/selfdrive/test/process_replay/test_log_compat.py @@ -0,0 +1,27 @@ +import os +import capnp +import hypothesis.strategies as st +from hypothesis import given, settings, HealthCheck + +from openpilot.selfdrive.test.fuzzy_generation import FuzzyGenerator +from openpilot.tools.lib.logreader import LogReader + +MAX_EXAMPLES = int(os.environ.get("MAX_EXAMPLES", "10")) + + +@given(st.data()) +@settings(max_examples=MAX_EXAMPLES, suppress_health_check=[HealthCheck.large_base_example]) +def test_log_backwards_compatibility(schema_path, data): + # capnp global parser needs to be cleaned up to avoid schema/struct ID conflicts + capnp.cleanup_global_schema_parser() + old_log = capnp.load(schema_path) + capnp.cleanup_global_schema_parser() + + msgs_dicts = FuzzyGenerator.get_random_event_msg(data.draw, log_schema=old_log, events=old_log.Event.schema.union_fields, real_floats=True) + msgs = [old_log.Event.new_message(**m).as_reader() for m in msgs_dicts] + dat = b"".join(msg.as_builder().to_bytes() for msg in msgs) + + lr = list(LogReader.from_bytes(dat)) + assert len(lr) == len(msgs) + # calling which() on a removed union type will raise an exception + assert set([m.which() for m in lr]) == set([msg.which() for msg in msgs]) From 154fb5ee822b110ceecb8d5dd4ced7806d5b2928 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20R=C4=85czy?= Date: Mon, 7 Apr 2025 15:51:33 -0700 Subject: [PATCH 2/5] Set comprehentions --- selfdrive/test/process_replay/test_log_compat.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/selfdrive/test/process_replay/test_log_compat.py b/selfdrive/test/process_replay/test_log_compat.py index d95a0cd622..1856886813 100644 --- a/selfdrive/test/process_replay/test_log_compat.py +++ b/selfdrive/test/process_replay/test_log_compat.py @@ -24,4 +24,4 @@ def test_log_backwards_compatibility(schema_path, data): lr = list(LogReader.from_bytes(dat)) assert len(lr) == len(msgs) # calling which() on a removed union type will raise an exception - assert set([m.which() for m in lr]) == set([msg.which() for msg in msgs]) + assert {m.which() for m in lr} == {msg.which() for msg in msgs} From b43c1c7176b717983039145f95b1a6b9f50c1e9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20R=C4=85czy?= Date: Mon, 7 Apr 2025 20:35:19 -0700 Subject: [PATCH 3/5] Get log.capnp from target branch --- .../test/process_replay/test_log_compat.py | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/selfdrive/test/process_replay/test_log_compat.py b/selfdrive/test/process_replay/test_log_compat.py index 1856886813..db6ca74573 100644 --- a/selfdrive/test/process_replay/test_log_compat.py +++ b/selfdrive/test/process_replay/test_log_compat.py @@ -1,21 +1,38 @@ import os import capnp +import pytest +import shutil import hypothesis.strategies as st from hypothesis import given, settings, HealthCheck +from cereal import CEREAL_PATH from openpilot.selfdrive.test.fuzzy_generation import FuzzyGenerator from openpilot.tools.lib.logreader import LogReader +from openpilot.common.run import run_cmd MAX_EXAMPLES = int(os.environ.get("MAX_EXAMPLES", "10")) +TARGET_REMOTE = os.environ.get("TARGET_REMOTE", "origin") +TARGET_BRANCH = os.environ.get("TARGET_BRANCH", "master") + + +@pytest.fixture(scope="module") +def parent_schema_file(tmp_path_factory): + commit = run_cmd(["git", "merge-base", f"{TARGET_REMOTE}/{TARGET_BRANCH}", "HEAD"]) + log_capnp_url = f"https://raw.githubusercontent.com/commaai/openpilot/{commit}/cereal/log.capnp" + tmp_dir = tmp_path_factory.mktemp("capnp") + tmp_log_capnp_path = tmp_dir / f"{commit}-log.capnp" + if not tmp_log_capnp_path.exists(): + run_cmd(["curl", "-o", str(tmp_log_capnp_path), log_capnp_url]) + + return str(tmp_log_capnp_path) @given(st.data()) @settings(max_examples=MAX_EXAMPLES, suppress_health_check=[HealthCheck.large_base_example]) -def test_log_backwards_compatibility(schema_path, data): +def test_log_backwards_compatibility(parent_schema_file, data): # capnp global parser needs to be cleaned up to avoid schema/struct ID conflicts - capnp.cleanup_global_schema_parser() - old_log = capnp.load(schema_path) - capnp.cleanup_global_schema_parser() + capnp_parser = capnp.SchemaParser() + old_log = capnp_parser.load(parent_schema_file, imports=[CEREAL_PATH]) msgs_dicts = FuzzyGenerator.get_random_event_msg(data.draw, log_schema=old_log, events=old_log.Event.schema.union_fields, real_floats=True) msgs = [old_log.Event.new_message(**m).as_reader() for m in msgs_dicts] From 419363c97a59e4a0533c2bd55848cfe266425f1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20R=C4=85czy?= Date: Tue, 8 Apr 2025 15:25:25 -0700 Subject: [PATCH 4/5] Make it work --- selfdrive/test/process_replay/test_log_compat.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/selfdrive/test/process_replay/test_log_compat.py b/selfdrive/test/process_replay/test_log_compat.py index db6ca74573..cfc75b34a4 100644 --- a/selfdrive/test/process_replay/test_log_compat.py +++ b/selfdrive/test/process_replay/test_log_compat.py @@ -17,9 +17,11 @@ TARGET_BRANCH = os.environ.get("TARGET_BRANCH", "master") @pytest.fixture(scope="module") def parent_schema_file(tmp_path_factory): + tmp_dir = tmp_path_factory.mktemp("cereal") + shutil.copytree(CEREAL_PATH, tmp_dir, dirs_exist_ok=True) + commit = run_cmd(["git", "merge-base", f"{TARGET_REMOTE}/{TARGET_BRANCH}", "HEAD"]) log_capnp_url = f"https://raw.githubusercontent.com/commaai/openpilot/{commit}/cereal/log.capnp" - tmp_dir = tmp_path_factory.mktemp("capnp") tmp_log_capnp_path = tmp_dir / f"{commit}-log.capnp" if not tmp_log_capnp_path.exists(): run_cmd(["curl", "-o", str(tmp_log_capnp_path), log_capnp_url]) @@ -32,7 +34,7 @@ def parent_schema_file(tmp_path_factory): def test_log_backwards_compatibility(parent_schema_file, data): # capnp global parser needs to be cleaned up to avoid schema/struct ID conflicts capnp_parser = capnp.SchemaParser() - old_log = capnp_parser.load(parent_schema_file, imports=[CEREAL_PATH]) + old_log = capnp_parser.load(os.path.abspath(parent_schema_file)) msgs_dicts = FuzzyGenerator.get_random_event_msg(data.draw, log_schema=old_log, events=old_log.Event.schema.union_fields, real_floats=True) msgs = [old_log.Event.new_message(**m).as_reader() for m in msgs_dicts] From f160e5a555e53e22b5891be462b3cc452757beb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20R=C4=85czy?= Date: Tue, 8 Apr 2025 15:28:42 -0700 Subject: [PATCH 5/5] Add comment --- selfdrive/test/process_replay/test_log_compat.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/selfdrive/test/process_replay/test_log_compat.py b/selfdrive/test/process_replay/test_log_compat.py index cfc75b34a4..fdc3b79653 100644 --- a/selfdrive/test/process_replay/test_log_compat.py +++ b/selfdrive/test/process_replay/test_log_compat.py @@ -18,6 +18,8 @@ TARGET_BRANCH = os.environ.get("TARGET_BRANCH", "master") @pytest.fixture(scope="module") def parent_schema_file(tmp_path_factory): tmp_dir = tmp_path_factory.mktemp("cereal") + # FIXME this is an ugly way to do this, but for some reason capnp.load ignores the `imports``, and only looks at dir where the file is + # how it supposed to work is: capnp.load(my_custom_log_capnp, imports=[CEREAL_PATH]) shutil.copytree(CEREAL_PATH, tmp_dir, dirs_exist_ok=True) commit = run_cmd(["git", "merge-base", f"{TARGET_REMOTE}/{TARGET_BRANCH}", "HEAD"])