pytest: use a clean environment for all tests (#29788)

* pytest: use a clean environment for all tests

* rm that

* fix pj

* put build back

* fix params

* fix that

* handle no key

* that was removed

---------

Co-authored-by: Justin Newberry <justin@comma.ai>

.github/workflows/selfdrive_tests.yaml

@@ -249,7 +249,7 @@ jobs:
       timeout-minutes: 40
       run: |
         ${{ env.RUN }} "export SKIP_LONG_TESTS=1 && \
-                        $PYTEST --rootdir . -c selfdrive/test/pytest-ci.ini && \
+                        $PYTEST && \
                         selfdrive/locationd/test/_test_locationd_lib.py && \
                         ./system/ubloxd/tests/test_glonass_runner && \
                         ./selfdrive/ui/tests/create_test_translations.sh && \

.github/workflows/tools_tests.yaml

@@ -33,11 +33,10 @@ jobs:
     - uses: actions/checkout@v3
       with:
         submodules: true
-    - name: Build Docker image
-      run: eval "$BUILD"
+    - uses: ./.github/workflows/setup
     - name: Build openpilot
       timeout-minutes: 5
-      run: ${{ env.RUN }} "scons -j$(nproc) --directory=/tmp/openpilot/cereal --minimal"
+      run: ${{ env.RUN }} "scons -j$(nproc) cereal/ common/ --minimal"
     - name: Test PlotJuggler
       timeout-minutes: 2
       run: |

common/prefix.py

@@ -2,11 +2,11 @@ import os
 import shutil
 import uuid
 
-from typing import List, Optional
+from typing import Optional
 
 from openpilot.common.params import Params
 
-class OpenpilotPrefix(object):
+class OpenpilotPrefix:
   def __init__(self, prefix: Optional[str] = None, clean_dirs_on_exit: bool = True):
     self.prefix = prefix if prefix else str(uuid.uuid4())
     self.msgq_path = os.path.join('/dev/shm', self.prefix)
@@ -24,7 +24,10 @@ class OpenpilotPrefix(object):
   def __exit__(self, exc_type, exc_obj, exc_tb):
     if self.clean_dirs_on_exit:
       self.clean_dirs()
-    del os.environ['OPENPILOT_PREFIX']
+    try:
+      del os.environ['OPENPILOT_PREFIX']
+    except KeyError:
+      pass
     return False
 
   def clean_dirs(self):
@@ -33,17 +36,3 @@ class OpenpilotPrefix(object):
       shutil.rmtree(os.path.realpath(symlink_path), ignore_errors=True)
       os.remove(symlink_path)
     shutil.rmtree(self.msgq_path, ignore_errors=True)
-
-
-class DummySocket:
-  def __init__(self):
-    self.data: List[bytes] = []
-
-  def receive(self, non_blocking: bool = False) -> Optional[bytes]:
-    if non_blocking:
-      return None
-
-    return self.data.pop()
-
-  def send(self, data: bytes):
-    self.data.append(data)

common/tests/test_params.py

@@ -1,8 +1,6 @@
 import os
 import threading
 import time
-import tempfile
-import shutil
 import uuid
 import unittest
 
@@ -10,12 +8,7 @@ from openpilot.common.params import Params, ParamKeyType, UnknownKeyName, put_no
 
 class TestParams(unittest.TestCase):
   def setUp(self):
-    self.tmpdir = tempfile.mkdtemp()
-    print("using", self.tmpdir)
-    self.params = Params(self.tmpdir)
-
-  def tearDown(self):
-    shutil.rmtree(self.tmpdir)
+    self.params = Params()
 
   def test_params_put_and_get(self):
     self.params.put("DongleId", "cb38263377b873ee")
@@ -90,19 +83,19 @@ class TestParams(unittest.TestCase):
     self.assertFalse(self.params.get_bool("IsMetric"))
 
   def test_put_non_blocking_with_get_block(self):
-    q = Params(self.tmpdir)
+    q = Params()
     def _delayed_writer():
       time.sleep(0.1)
-      put_nonblocking("CarParams", "test", self.tmpdir)
+      put_nonblocking("CarParams", "test")
     threading.Thread(target=_delayed_writer).start()
     assert q.get("CarParams") is None
     assert q.get("CarParams", True) == b"test"
 
   def test_put_bool_non_blocking_with_get_block(self):
-    q = Params(self.tmpdir)
+    q = Params()
     def _delayed_writer():
       time.sleep(0.1)
-      put_bool_nonblocking("CarParams", True, self.tmpdir)
+      put_bool_nonblocking("CarParams", True)
     threading.Thread(target=_delayed_writer).start()
     assert q.get("CarParams") is None
     assert q.get("CarParams", True) == b"1"

conftest.py

@@ -0,0 +1,10 @@
+import pytest
+
+from openpilot.common.prefix import OpenpilotPrefix
+
+
+@pytest.fixture(scope="function", autouse=True)
+def global_setup_and_teardown():
+  # setup a clean environment for each test
+  with OpenpilotPrefix():
+    yield

pyproject.toml

@@ -6,6 +6,22 @@ python_files = "test_*.py"
 markers = [
   "parallel: mark tests as parallelizable (tests with no global state, so can be run in parallel)"
 ]
+testpaths = [
+  "common",
+  "selfdrive/athena",
+  "selfdrive/boardd",
+  "selfdrive/car",
+  "selfdrive/controls",
+  "selfdrive/locationd",
+  "selfdrive/monitoring",
+  "selfdrive/thermald",
+  "selfdrive/test/longitudinal_maneuvers",
+  "system/hardware/tici",
+  "system/loggerd",
+  "system/tests",
+  "system/ubloxd",
+  "tools/lib/tests"
+]
 
 [tool.mypy]
 python_version = "3.11"

selfdrive/test/process_replay/process_replay.py

@@ -17,12 +17,12 @@ from cereal import car
 from cereal.services import service_list
 from cereal.visionipc import VisionIpcServer, get_endpoint_name as vipc_get_endpoint_name
 from openpilot.common.params import Params
+from openpilot.common.prefix import OpenpilotPrefix
 from openpilot.common.timeout import Timeout
 from openpilot.common.realtime import DT_CTRL
 from panda.python import ALTERNATIVE_EXPERIENCE
 from openpilot.selfdrive.car.car_helpers import get_car, interfaces
 from openpilot.selfdrive.manager.process_config import managed_processes
-from openpilot.selfdrive.test.process_replay.helpers import OpenpilotPrefix, DummySocket
 from openpilot.selfdrive.test.process_replay.vision_meta import meta_from_camera_state, available_streams
 from openpilot.selfdrive.test.process_replay.migration import migrate_all
 from openpilot.selfdrive.test.process_replay.capture import ProcessOutputCapture
@@ -33,6 +33,18 @@ NUMPY_TOLERANCE = 1e-7
 PROC_REPLAY_DIR = os.path.dirname(os.path.abspath(__file__))
 FAKEDATA = os.path.join(PROC_REPLAY_DIR, "fakedata/")
 
+class DummySocket:
+  def __init__(self):
+    self.data: List[bytes] = []
+
+  def receive(self, non_blocking: bool = False) -> Optional[bytes]:
+    if non_blocking:
+      return None
+
+    return self.data.pop()
+
+  def send(self, data: bytes):
+    self.data.append(data)
 
 class LauncherWithCapture:
   def __init__(self, capture: ProcessOutputCapture, launcher: Callable):

selfdrive/test/process_replay/regen_all.py

@@ -6,7 +6,7 @@ import random
 import traceback
 from tqdm import tqdm
 
-from openpilot.selfdrive.test.process_replay.helpers import OpenpilotPrefix
+from openpilot.common.prefix import OpenpilotPrefix
 from openpilot.selfdrive.test.process_replay.regen import regen_and_save
 from openpilot.selfdrive.test.process_replay.test_processes import FAKEDATA, source_segments as segments
 from openpilot.tools.lib.route import SegmentName

selfdrive/test/pytest-ci.ini

@@ -1,18 +0,0 @@
-[pytest]
-testpaths = 
-    common
-    selfdrive/athena
-    selfdrive/boardd
-    selfdrive/car
-    selfdrive/controls
-    selfdrive/locationd
-    selfdrive/monitoring
-    selfdrive/thermald
-    selfdrive/test/longitudinal_maneuvers
-    system/hardware/tici
-    system/loggerd
-    system/tests
-    system/ubloxd
-    tools/lib/tests
-markers =
-    parallel: mark tests as parallelizable (tests with no global state, so can be run in parallel)

selfdrive/thermald/tests/test_power_monitoring.py

@@ -25,12 +25,9 @@ def pm_patch(name, value, constant=False):
 
 
 @patch("time.monotonic", new=mock_time_monotonic)
-@patch("openpilot.selfdrive.thermald.power_monitoring.put_nonblocking", new=lambda x, y: Params().put(x, y))
 class TestPowerMonitoring(unittest.TestCase):
   def setUp(self):
     self.params = Params()
-    self.params.remove("CarBatteryCapacity")
-    self.params.remove("DisablePowerDown")
 
   # Test to see that it doesn't do anything when pandaState is None
   def test_pandaState_present(self):