diff --git a/system/loggerd/config.py b/system/loggerd/config.py index 168c9fba91..df187a48c0 100644 --- a/system/loggerd/config.py +++ b/system/loggerd/config.py @@ -5,7 +5,7 @@ from system.hardware import PC if os.environ.get('LOG_ROOT', False): ROOT = os.environ['LOG_ROOT'] elif PC: - ROOT = os.path.join(str(Path.home()), ".comma", "media", "0", "realdata") + ROOT = str(Path.home() / ".comma" / "media" / "0" / "realdata") else: ROOT = '/data/media/0/realdata/' @@ -16,7 +16,7 @@ SEGMENT_LENGTH = 60 STATS_DIR_FILE_LIMIT = 10000 STATS_SOCKET = "ipc:///tmp/stats" if PC: - STATS_DIR = os.path.join(str(Path.home()), ".comma", "stats") + STATS_DIR = str(Path.home() / ".comma" / "stats") else: STATS_DIR = "/data/stats/" STATS_FLUSH_TIME_S = 60 diff --git a/system/loggerd/tests/fill_eon.py b/system/loggerd/tests/fill.py similarity index 57% rename from system/loggerd/tests/fill_eon.py rename to system/loggerd/tests/fill.py index e0c52ea0d7..4bf4f73604 100755 --- a/system/loggerd/tests/fill_eon.py +++ b/system/loggerd/tests/fill.py @@ -1,7 +1,7 @@ #!/usr/bin/env python3 -"""Script to fill up EON with fake data""" +"""Script to fill up storage with fake data""" -import os +from pathlib import Path from system.loggerd.config import ROOT, get_available_percent from system.loggerd.tests.loggerd_tests_common import create_random_file @@ -10,13 +10,13 @@ from system.loggerd.tests.loggerd_tests_common import create_random_file if __name__ == "__main__": segment_idx = 0 while True: - seg_name = "1970-01-01--00-00-00--%d" % segment_idx - seg_path = os.path.join(ROOT, seg_name) + seg_name = f"1970-01-01--00-00-00--{segment_idx}" + seg_path = Path(ROOT) / seg_name print(seg_path) - create_random_file(os.path.join(seg_path, 'fcamera.hevc'), 36) - create_random_file(os.path.join(seg_path, 'rlog.bz2'), 2) + create_random_file(seg_path / "fcamera.hevc", 36) + create_random_file(seg_path / "rlog.bz2", 2) segment_idx += 1 diff --git a/system/loggerd/tests/loggerd_tests_common.py b/system/loggerd/tests/loggerd_tests_common.py index 780316ad0c..6d1303ca6c 100644 --- a/system/loggerd/tests/loggerd_tests_common.py +++ b/system/loggerd/tests/loggerd_tests_common.py @@ -4,29 +4,29 @@ import shutil import random import tempfile import unittest +from pathlib import Path +from typing import Optional import system.loggerd.uploader as uploader -def create_random_file(file_path, size_mb, lock=False, xattr=None): - try: - os.mkdir(os.path.dirname(file_path)) - except OSError: - pass - lock_path = file_path + ".lock" +def create_random_file(file_path: Path, size_mb: float, lock: bool = False, xattr: Optional[bytes] = None) -> None: + file_path.parent.mkdir(parents=True, exist_ok=True) + if lock: + lock_path = str(file_path) + ".lock" os.close(os.open(lock_path, os.O_CREAT | os.O_EXCL)) chunks = 128 chunk_bytes = int(size_mb * 1024 * 1024 / chunks) data = os.urandom(chunk_bytes) - with open(file_path, 'wb') as f: + with open(file_path, "wb") as f: for _ in range(chunks): f.write(data) if xattr is not None: - uploader.setxattr(file_path, uploader.UPLOAD_ATTR_NAME, xattr) + uploader.setxattr(str(file_path), uploader.UPLOAD_ATTR_NAME, xattr) class MockResponse(): def __init__(self, text, status_code): @@ -75,12 +75,18 @@ class MockParams(): class UploaderTestCase(unittest.TestCase): f_type = "UNKNOWN" + root: Path + seg_num: int + seg_format: str + seg_format2: str + seg_dir: str + def set_ignore(self): uploader.Api = MockApiIgnore def setUp(self): - self.root = tempfile.mkdtemp() - uploader.ROOT = self.root # Monkey patch root dir + self.root = Path(tempfile.mkdtemp()) + uploader.ROOT = str(self.root) # Monkey patch root dir uploader.Api = MockApi uploader.Params = MockParams uploader.fake_upload = True @@ -98,8 +104,9 @@ class UploaderTestCase(unittest.TestCase): if e.errno != errno.ENOENT: raise - def make_file_with_data(self, f_dir, fn, size_mb=.1, lock=False, xattr=None): - file_path = os.path.join(self.root, f_dir, fn) + def make_file_with_data(self, f_dir: str, fn: str, size_mb: float = .1, lock: bool = False, + xattr: Optional[bytes] = None) -> Path: + file_path = self.root / f_dir / fn create_random_file(file_path, size_mb, lock, xattr) return file_path diff --git a/system/loggerd/tests/test_deleter.py b/system/loggerd/tests/test_deleter.py index 5b54a43f3b..596545cdeb 100755 --- a/system/loggerd/tests/test_deleter.py +++ b/system/loggerd/tests/test_deleter.py @@ -1,5 +1,4 @@ #!/usr/bin/env python3 -import os import time import threading import unittest @@ -21,7 +20,7 @@ class TestDeleter(UploaderTestCase): super().setUp() self.fake_stats = Stats(f_bavail=0, f_blocks=10, f_frsize=4096) deleter.os.statvfs = self.fake_statvfs - deleter.ROOT = self.root + deleter.ROOT = str(self.root) def start_thread(self): self.end_event = threading.Event() @@ -39,11 +38,11 @@ class TestDeleter(UploaderTestCase): self.start_thread() with Timeout(5, "Timeout waiting for file to be deleted"): - while os.path.exists(f_path): + while f_path.exists(): time.sleep(0.01) self.join_thread() - self.assertFalse(os.path.exists(f_path), "File not deleted") + self.assertFalse(f_path.exists(), "File not deleted") def test_delete_files_in_create_order(self): f_path_1 = self.make_file_with_data(self.seg_dir, self.f_type) @@ -55,14 +54,13 @@ class TestDeleter(UploaderTestCase): self.start_thread() with Timeout(5, "Timeout waiting for file to be deleted"): - while os.path.exists(f_path_1) and os.path.exists(f_path_2): + while f_path_1.exists() and f_path_2.exists(): time.sleep(0.01) self.join_thread() - self.assertFalse(os.path.exists(f_path_1), "Older file not deleted") - - self.assertTrue(os.path.exists(f_path_2), "Newer file deleted before older file") + self.assertFalse(f_path_1.exists(), "Older file not deleted") + self.assertTrue(f_path_2.exists(), "Newer file deleted before older file") def test_no_delete_when_available_space(self): f_path = self.make_file_with_data(self.seg_dir, self.f_type) @@ -75,14 +73,14 @@ class TestDeleter(UploaderTestCase): try: with Timeout(2, "Timeout waiting for file to be deleted"): - while os.path.exists(f_path): + while f_path.exists(): time.sleep(0.01) except TimeoutException: pass finally: self.join_thread() - self.assertTrue(os.path.exists(f_path), "File deleted with available space") + self.assertTrue(f_path.exists(), "File deleted with available space") def test_no_delete_with_lock_file(self): f_path = self.make_file_with_data(self.seg_dir, self.f_type, lock=True) @@ -91,14 +89,14 @@ class TestDeleter(UploaderTestCase): try: with Timeout(2, "Timeout waiting for file to be deleted"): - while os.path.exists(f_path): + while f_path.exists(): time.sleep(0.01) except TimeoutException: pass finally: self.join_thread() - self.assertTrue(os.path.exists(f_path), "File deleted when locked") + self.assertTrue(f_path.exists(), "File deleted when locked") if __name__ == "__main__": diff --git a/system/loggerd/tests/test_uploader.py b/system/loggerd/tests/test_uploader.py index df99651447..9346b770a9 100755 --- a/system/loggerd/tests/test_uploader.py +++ b/system/loggerd/tests/test_uploader.py @@ -5,6 +5,8 @@ import threading import unittest import logging import json +from pathlib import Path +from typing import List, Optional from system.swaglog import cloudlog from system.loggerd.uploader import uploader_fn, UPLOAD_ATTR_NAME, UPLOAD_ATTR_VALUE @@ -50,8 +52,8 @@ class TestUploader(UploaderTestCase): self.end_event.set() self.up_thread.join() - def gen_files(self, lock=False, xattr=None, boot=True): - f_paths = list() + def gen_files(self, lock=False, xattr: Optional[bytes] = None, boot=True) -> List[Path]: + f_paths = [] for t in ["qlog", "rlog", "dcamera.hevc", "fcamera.hevc"]: f_paths.append(self.make_file_with_data(self.seg_dir, t, 1, lock=lock, xattr=xattr)) @@ -59,7 +61,7 @@ class TestUploader(UploaderTestCase): f_paths.append(self.make_file_with_data("boot", f"{self.seg_dir}", 1, lock=lock, xattr=xattr)) return f_paths - def gen_order(self, seg1, seg2, boot=True): + def gen_order(self, seg1: List[int], seg2: List[int], boot=True) -> List[str]: keys = [] if boot: keys += [f"boot/{self.seg_format.format(i)}.bz2" for i in seg1] @@ -82,7 +84,7 @@ class TestUploader(UploaderTestCase): self.assertFalse(len(log_handler.upload_order) < len(exp_order), "Some files failed to upload") self.assertFalse(len(log_handler.upload_order) > len(exp_order), "Some files were uploaded twice") for f_path in exp_order: - self.assertEqual(os.getxattr(os.path.join(self.root, f_path.replace('.bz2', '')), UPLOAD_ATTR_NAME), UPLOAD_ATTR_VALUE, "All files not uploaded") + self.assertEqual(os.getxattr((self.root / f_path).with_suffix(""), UPLOAD_ATTR_NAME), UPLOAD_ATTR_VALUE, "All files not uploaded") self.assertTrue(log_handler.upload_order == exp_order, "Files uploaded in wrong order") @@ -100,7 +102,7 @@ class TestUploader(UploaderTestCase): self.assertFalse(len(log_handler.upload_order) < len(exp_order), "Some files failed to upload") self.assertFalse(len(log_handler.upload_order) > len(exp_order), "Some files were uploaded twice") for f_path in exp_order: - self.assertEqual(os.getxattr(os.path.join(self.root, f_path.replace('.bz2', '')), UPLOAD_ATTR_NAME), UPLOAD_ATTR_VALUE, "All files not uploaded") + self.assertEqual(os.getxattr((self.root / f_path).with_suffix(""), UPLOAD_ATTR_NAME), UPLOAD_ATTR_VALUE, "All files not uploaded") self.assertTrue(log_handler.upload_order == exp_order, "Files uploaded in wrong order") @@ -119,7 +121,7 @@ class TestUploader(UploaderTestCase): self.assertFalse(len(log_handler.upload_ignored) < len(exp_order), "Some files failed to ignore") self.assertFalse(len(log_handler.upload_ignored) > len(exp_order), "Some files were ignored twice") for f_path in exp_order: - self.assertEqual(os.getxattr(os.path.join(self.root, f_path.replace('.bz2', '')), UPLOAD_ATTR_NAME), UPLOAD_ATTR_VALUE, "All files not ignored") + self.assertEqual(os.getxattr((self.root / f_path).with_suffix(""), UPLOAD_ATTR_NAME), UPLOAD_ATTR_VALUE, "All files not ignored") self.assertTrue(log_handler.upload_ignored == exp_order, "Files ignored in wrong order") @@ -144,7 +146,7 @@ class TestUploader(UploaderTestCase): self.assertFalse(len(log_handler.upload_order) < len(exp_order), "Some files failed to upload") self.assertFalse(len(log_handler.upload_order) > len(exp_order), "Some files were uploaded twice") for f_path in exp_order: - self.assertEqual(os.getxattr(os.path.join(self.root, f_path.replace('.bz2', '')), UPLOAD_ATTR_NAME), UPLOAD_ATTR_VALUE, "All files not uploaded") + self.assertEqual(os.getxattr((self.root / f_path).with_suffix(""), UPLOAD_ATTR_NAME), UPLOAD_ATTR_VALUE, "All files not uploaded") self.assertTrue(log_handler.upload_order == exp_order, "Files uploaded in wrong order") @@ -159,7 +161,7 @@ class TestUploader(UploaderTestCase): self.join_thread() for f_path in f_paths: - fn = f_path.replace('.bz2', '') + fn = f_path.with_suffix(f_path.suffix.replace(".bz2", "")) uploaded = UPLOAD_ATTR_NAME in os.listxattr(fn) and os.getxattr(fn, UPLOAD_ATTR_NAME) == UPLOAD_ATTR_VALUE self.assertFalse(uploaded, "File upload when locked") @@ -180,7 +182,8 @@ class TestUploader(UploaderTestCase): self.join_thread() for f_path in f_paths: - self.assertFalse(os.path.isfile(f_path + ".lock"), "File lock not cleared on startup") + lock_path = f_path.with_suffix(f_path.suffix + ".lock") + self.assertFalse(lock_path.is_file(), "File lock not cleared on startup") if __name__ == "__main__": diff --git a/system/loggerd/xattr_cache.py b/system/loggerd/xattr_cache.py index 95e39f2032..5feeff34d2 100644 --- a/system/loggerd/xattr_cache.py +++ b/system/loggerd/xattr_cache.py @@ -1,6 +1,6 @@ import os import errno -from typing import Dict, Tuple, Optional +from typing import Dict, Optional, Tuple _cached_attributes: Dict[Tuple, Optional[bytes]] = {}