From 279bd020dc3e5ba942aa82616b89348b0f7a1c9c Mon Sep 17 00:00:00 2001 From: Justin Newberry Date: Tue, 29 Aug 2023 13:36:26 -0700 Subject: [PATCH] Tests: logmessaged reduce global state (#29680) * reduce global state * use a decorator here too * use that one * use base imports * ipchandler in setup * move to common dir * move to helpers * wip * fix the decorator old-commit-hash: 5dc7028f91079dfa07347a4d24ff56b72c24bd20 --- selfdrive/test/helpers.py | 19 +++++++++++++++++++ system/swaglog.py | 7 ++++++- system/tests/test_logmessaged.py | 23 ++++++++++++++--------- tools/lib/tests/__init__.py | 12 ------------ tools/lib/tests/test_caching.py | 6 +++--- 5 files changed, 42 insertions(+), 25 deletions(-) diff --git a/selfdrive/test/helpers.py b/selfdrive/test/helpers.py index f5ba67b407..3f4e4e1661 100644 --- a/selfdrive/test/helpers.py +++ b/selfdrive/test/helpers.py @@ -1,5 +1,8 @@ import os import time +import tempfile + +from unittest import mock from functools import wraps import cereal.messaging as messaging @@ -67,3 +70,19 @@ def with_processes(processes, init_time=0, ignore_stopped=None): return wrap return wrapper + + +def temporary_mock_dir(mock_path): + def wrapper(func): + @wraps(func) + def wrap(*args, **kwargs): + with tempfile.TemporaryDirectory() as temp_dir: + cache_dir_patch = mock.patch(mock_path, temp_dir) + cache_dir_patch.start() + func(*args, **kwargs, temp_dir=temp_dir) + cache_dir_patch.stop() + return wrap + return wrapper + +temporary_cache_dir = temporary_mock_dir("openpilot.tools.lib.url_file.CACHE_DIR") +temporary_swaglog_dir = temporary_mock_dir("openpilot.system.swaglog.SWAGLOG_DIR") \ No newline at end of file diff --git a/system/swaglog.py b/system/swaglog.py index 5fe1c2f27f..3d45ad9826 100644 --- a/system/swaglog.py +++ b/system/swaglog.py @@ -77,6 +77,9 @@ class UnixDomainSocketHandler(logging.Handler): self.sock = None def __del__(self): + self.close() + + def close(self): if self.sock is not None: self.sock.close() if self.zctx is not None: @@ -129,6 +132,8 @@ elif print_level == 'info': elif print_level == 'warning': outhandler.setLevel(logging.WARNING) +ipchandler = UnixDomainSocketHandler(SwagFormatter(log)) + log.addHandler(outhandler) # logs are sent through IPC before writing to disk to prevent disk I/O blocking -log.addHandler(UnixDomainSocketHandler(SwagFormatter(log))) +log.addHandler(ipchandler) diff --git a/system/tests/test_logmessaged.py b/system/tests/test_logmessaged.py index 955272d403..330f151368 100755 --- a/system/tests/test_logmessaged.py +++ b/system/tests/test_logmessaged.py @@ -1,21 +1,22 @@ #!/usr/bin/env python3 import glob import os -import shutil import time import unittest import cereal.messaging as messaging from openpilot.selfdrive.manager.process_config import managed_processes -from openpilot.system.swaglog import cloudlog, SWAGLOG_DIR +from openpilot.system.swaglog import cloudlog, ipchandler +from selfdrive.test.helpers import temporary_swaglog_dir class TestLogmessaged(unittest.TestCase): + def _setup(self, temp_dir): + # clear the IPC buffer in case some other tests used cloudlog and filled it + ipchandler.close() + ipchandler.connect() - def setUp(self): - if os.path.exists(SWAGLOG_DIR): - shutil.rmtree(SWAGLOG_DIR) - + self.temp_dir = temp_dir managed_processes['logmessaged'].start() self.sock = messaging.sub_sock("logMessage", timeout=1000, conflate=False) self.error_sock = messaging.sub_sock("logMessage", timeout=1000, conflate=False) @@ -31,9 +32,11 @@ class TestLogmessaged(unittest.TestCase): managed_processes['logmessaged'].stop(block=True) def _get_log_files(self): - return list(glob.glob(os.path.join(SWAGLOG_DIR, "swaglog.*"))) + return list(glob.glob(os.path.join(self.temp_dir, "swaglog.*"))) - def test_simple_log(self): + @temporary_swaglog_dir + def test_simple_log(self, temp_dir): + self._setup(temp_dir) msgs = [f"abc {i}" for i in range(10)] for m in msgs: cloudlog.error(m) @@ -42,7 +45,9 @@ class TestLogmessaged(unittest.TestCase): assert len(m) == len(msgs) assert len(self._get_log_files()) >= 1 - def test_big_log(self): + @temporary_swaglog_dir + def test_big_log(self, temp_dir): + self._setup(temp_dir) n = 10 msg = "a"*3*1024*1024 for _ in range(n): diff --git a/tools/lib/tests/__init__.py b/tools/lib/tests/__init__.py index 32219c6ce9..e69de29bb2 100644 --- a/tools/lib/tests/__init__.py +++ b/tools/lib/tests/__init__.py @@ -1,12 +0,0 @@ -import tempfile - -from unittest import mock - -def temporary_cache_dir(func): - def wrapper(*args, **kwargs): - with tempfile.TemporaryDirectory() as temp_dir: - cache_dir_patch = mock.patch("openpilot.tools.lib.url_file.CACHE_DIR", temp_dir) - cache_dir_patch.start() - func(*args, **kwargs) - cache_dir_patch.stop() - return wrapper \ No newline at end of file diff --git a/tools/lib/tests/test_caching.py b/tools/lib/tests/test_caching.py index 0cb7d66d54..bd93cd93ec 100644 --- a/tools/lib/tests/test_caching.py +++ b/tools/lib/tests/test_caching.py @@ -2,7 +2,7 @@ import os import unittest from openpilot.tools.lib.url_file import URLFile -from tools.lib.tests import temporary_cache_dir +from openpilot.selfdrive.test.helpers import temporary_cache_dir class TestFileDownload(unittest.TestCase): @@ -32,7 +32,7 @@ class TestFileDownload(unittest.TestCase): self.assertEqual(response_cached, response_downloaded) @temporary_cache_dir - def test_small_file(self): + def test_small_file(self, temp_dir): # Make sure we don't force cache os.environ["FILEREADER_CACHE"] = "0" small_file_url = "https://raw.githubusercontent.com/commaai/openpilot/master/docs/SAFETY.md" @@ -53,7 +53,7 @@ class TestFileDownload(unittest.TestCase): self.compare_loads(small_file_url, 100 * i, 100) @temporary_cache_dir - def test_large_file(self): + def test_large_file(self, temp_dir): large_file_url = "https://commadataci.blob.core.windows.net/openpilotci/0375fdf7b1ce594d/2019-06-13--08-32-25/3/qlog.bz2" # Load the end 100 bytes of both files file_large = URLFile(large_file_url)