From e70756c3ff886c21f2ed1d2f8f535db9b9a313bc Mon Sep 17 00:00:00 2001 From: bhuvan-somisetty Date: Mon, 14 Sep 2026 09:20:56 +0530 Subject: [PATCH] fix: use distinct READ_ERROR status for non-missing-file read failures read() was reporting FILE_NOT_FOUND for any file I/O error other than FileNotFoundError, e.g. permission errors or reading a directory. This made read_with_status() unreliable for telling callers why a read actually failed. Also removed EMPTY_DATA from the status docstring since nothing in the file ever sets it. Fixes #578 --- concore_base.py | 4 ++-- tests/test_read_status.py | 31 ++++++++++++++++++++++++++++++- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/concore_base.py b/concore_base.py index 9468b48..0676bf7 100644 --- a/concore_base.py +++ b/concore_base.py @@ -247,7 +247,7 @@ def read(mod, port_identifier, name, initstr_val): Also sets ``concore.last_read_status`` (and ``concore_base.last_read_status``) to one of: SUCCESS, FILE_NOT_FOUND, TIMEOUT, PARSE_ERROR, - EMPTY_DATA, RETRIES_EXCEEDED. + READ_ERROR, RETRIES_EXCEEDED. Notes: This low-level helper always returns ``(data, success_flag)``. @@ -317,7 +317,7 @@ def read(mod, port_identifier, name, initstr_val): mod.s += ins # Update s to break unchanged() loop except Exception as e: logger.error(f"Error reading {file_path}: {e}. Using default value.") - last_read_status = "FILE_NOT_FOUND" + last_read_status = "READ_ERROR" return default_return_val, False # Retry logic if file is empty diff --git a/tests/test_read_status.py b/tests/test_read_status.py index c58bc22..aec32d7 100644 --- a/tests/test_read_status.py +++ b/tests/test_read_status.py @@ -110,6 +110,35 @@ def test_last_read_status_is_parse_error(self): assert self.concore.last_read_status == "PARSE_ERROR" +class TestReadFileGenericIOError: + """read_with_status() returns (default, False) with READ_ERROR on + file I/O failures that are not a missing file (e.g. the target + path is a directory instead of a file).""" + + @pytest.fixture(autouse=True) + def setup(self, temp_dir, monkeypatch): + import concore + + self.concore = concore + monkeypatch.setattr(concore, "delay", 0) + + in_dir = os.path.join(temp_dir, "in1") + # "ym" is a directory here instead of a file, so open() raises + # IsADirectoryError rather than FileNotFoundError. + os.makedirs(os.path.join(in_dir, "ym"), exist_ok=True) + + monkeypatch.setattr(concore, "inpath", os.path.join(temp_dir, "in")) + + def test_returns_default_and_false(self): + data, ok = self.concore.read_with_status(1, "ym", "[0, 0.0]") + assert ok is False + assert data == [0, 0.0] + + def test_last_read_status_is_read_error(self): + self.concore.read(1, "ym", "[0, 0.0]") + assert self.concore.last_read_status == "READ_ERROR" + + class TestReadFileTraversalBlocked: """read_with_status() rejects traversal names and returns PARSE_ERROR.""" @@ -294,6 +323,6 @@ def test_initial_value_is_success(self): "FILE_NOT_FOUND", "TIMEOUT", "PARSE_ERROR", - "EMPTY_DATA", + "READ_ERROR", "RETRIES_EXCEEDED", )