From 3b26962d29674f435e638725b2fae2ca008824b2 Mon Sep 17 00:00:00 2001 From: Hugo Farajallah Date: Thu, 26 Mar 2026 11:44:56 +0100 Subject: [PATCH 1/3] fix: add guards in BlockUnit.run_trial() and fix SubInfo bare except - Raise RuntimeError when run_trial() is called before generate_conditions() - Raise TypeError when trial function returns non-dict - Fix logging_block_info() calling .count() on numpy array - Replace bare except: with except Exception: in SubInfo.validate() - Add test_BlockUnit.py and test_SubInfo.py covering these paths --- psyflow/BlockUnit.py | 19 ++++++++-- psyflow/SubInfo.py | 2 +- tests/test_BlockUnit.py | 77 +++++++++++++++++++++++++++++++++++++++++ tests/test_SubInfo.py | 69 ++++++++++++++++++++++++++++++++++++ 4 files changed, 164 insertions(+), 3 deletions(-) create mode 100644 tests/test_BlockUnit.py create mode 100644 tests/test_SubInfo.py diff --git a/psyflow/BlockUnit.py b/psyflow/BlockUnit.py index 9a1300b..5da1c5a 100644 --- a/psyflow/BlockUnit.py +++ b/psyflow/BlockUnit.py @@ -265,6 +265,12 @@ def run_trial(self, func: Callable, **kwargs) -> "BlockUnit": **kwargs : dict Additional keyword arguments forwarded to ``func``. """ + if self.conditions is None: + raise RuntimeError( + f"BlockUnit '{self.block_id}' has no conditions. " + "Call generate_conditions() before run_trial()." + ) + self.meta['block_start_time'] = core.getAbsTime() self.logging_block_info() @@ -273,6 +279,11 @@ def run_trial(self, func: Callable, **kwargs) -> "BlockUnit": for i, cond in enumerate(self.conditions): result = func(self.win, self.kb, self.settings, cond, **kwargs) + if not isinstance(result, dict): + raise TypeError( + f"Trial function {func.__name__!r} must return a dict, " + f"got {type(result).__name__!r}" + ) result.update({ "trial_index": i, "block_id": self.block_id, @@ -403,10 +414,14 @@ def logging_block_info(self) -> None: """ Log block metadata including ID, index, seed, trial count, and condition distribution. """ - dist = {c: self.conditions.count(c) for c in set(self.conditions)} if self.conditions else {} + if self.conditions is not None and len(self.conditions) > 0: + conds = self.conditions + dist = {c: int(np.sum(conds == c)) for c in set(conds)} + else: + dist = {} logging.data(f"[BlockUnit] Blockid: {self.block_id}") logging.data(f"[BlockUnit] Blockidx: {self.block_idx}") logging.data(f"[BlockUnit] Blockseed: {self.seed}") - logging.data(f"[BlockUnit] Blocktrial-N: {len(self.conditions)}") + logging.data(f"[BlockUnit] Blocktrial-N: {len(self.conditions) if self.conditions is not None else 0}") logging.data(f"[BlockUnit] Blockdist: {dist}") logging.data(f"[BlockUnit] Blockconditions: {self.conditions}") diff --git a/psyflow/SubInfo.py b/psyflow/SubInfo.py index ed471e5..48528e3 100644 --- a/psyflow/SubInfo.py +++ b/psyflow/SubInfo.py @@ -167,7 +167,7 @@ def validate(self, responses) -> bool: raise ValueError if digits is not None and len(str(val)) != digits: raise ValueError - except: + except Exception: infoDlg = gui.Dlg() infoDlg.addText( self._local("invalid_input").format(field=self._local(field['name'])) diff --git a/tests/test_BlockUnit.py b/tests/test_BlockUnit.py new file mode 100644 index 0000000..e92aded --- /dev/null +++ b/tests/test_BlockUnit.py @@ -0,0 +1,77 @@ +"""Tests for psyflow.BlockUnit.""" + +import sys +import unittest +from unittest.mock import MagicMock +from types import SimpleNamespace + +import numpy as np + +# Stub out PsychoPy before importing BlockUnit +_psychopy_stub = MagicMock() +_psychopy_stub.core.getAbsTime.return_value = 0.0 +sys.modules.setdefault("psychopy", _psychopy_stub) +sys.modules.setdefault("psychopy.core", _psychopy_stub.core) +sys.modules.setdefault("psychopy.logging", _psychopy_stub.logging) + +from psyflow.BlockUnit import BlockUnit # noqa: E402 + + +def _make_settings(**overrides): + """Create a minimal settings-like object.""" + defaults = { + "trials_per_block": 3, + "block_seed": [42], + } + defaults.update(overrides) + return SimpleNamespace(**defaults) + + +def _make_block(**overrides): + """Build a BlockUnit without calling __init__ (avoids PsychoPy window).""" + block = BlockUnit.__new__(BlockUnit) + defaults = dict( + block_id="test", + block_idx=0, + n_trials=3, + settings=_make_settings(), + win=MagicMock(), + kb=MagicMock(), + seed=42, + conditions=None, + results=[], + meta={}, + _on_start=[], + _on_end=[], + ) + defaults.update(overrides) + for k, v in defaults.items(): + setattr(block, k, v) + return block + + +class TestRunTrialGuards(unittest.TestCase): + """run_trial() should reject invalid state with clear errors.""" + + def test_conditions_none_raises_runtime_error(self): + block = _make_block(conditions=None) + + with self.assertRaises(RuntimeError) as ctx: + block.run_trial(lambda win, kb, s, c: {"rt": 0.5}) + + self.assertIn("conditions", str(ctx.exception).lower()) + + def test_func_returning_none_raises_type_error(self): + block = _make_block(conditions=np.array(["A"])) + + def bad_trial_func(win, kb, settings, cond): + return None + + with self.assertRaises(TypeError) as ctx: + block.run_trial(bad_trial_func) + + self.assertIn("dict", str(ctx.exception).lower()) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_SubInfo.py b/tests/test_SubInfo.py new file mode 100644 index 0000000..827f9da --- /dev/null +++ b/tests/test_SubInfo.py @@ -0,0 +1,69 @@ +"""Tests for psyflow.SubInfo.""" + +import sys +import unittest +from unittest.mock import MagicMock + +# Stub out PsychoPy before importing SubInfo. +# test_BlockUnit may have already registered a different stub, so we use +# setdefault and then grab whatever SubInfo actually sees. +_psychopy_stub = MagicMock() +sys.modules.setdefault("psychopy", _psychopy_stub) +sys.modules.setdefault("psychopy.gui", _psychopy_stub.gui) + +import psyflow.SubInfo as _subinfo_mod # noqa: E402 +from psyflow.SubInfo import SubInfo # noqa: E402 + +# The gui object SubInfo actually captured at import time +_gui = _subinfo_mod.gui + + +def _make_subinfo(**field_map_overrides): + """Build a SubInfo without calling __init__.""" + info = SubInfo.__new__(SubInfo) + info.fields = [ + {"name": "subject_id", "type": "int", + "constraints": {"min": 101, "max": 999, "digits": 3}} + ] + info.field_map = { + "Participant Information": "Info", + "registration_failed": "Failed", + "invalid_input": "Bad: {field}", + } + info.field_map.update(field_map_overrides) + info.subject_data = None + return info + + +class TestCollect(unittest.TestCase): + """SubInfo.collect() control-flow edge cases.""" + + def test_cancel_returns_none(self): + info = _make_subinfo() + + mock_dlg = MagicMock() + mock_dlg.show.return_value = None + _gui.Dlg.return_value = mock_dlg + + result = info.collect(exit_on_cancel=False) + self.assertIsNone(result) + + +class TestValidate(unittest.TestCase): + """SubInfo.validate() error handling.""" + + def test_keyboard_interrupt_propagates(self): + info = _make_subinfo() + + class ExplodingStr: + def __int__(self): + raise KeyboardInterrupt("simulated Ctrl+C") + def __str__(self): + return "boom" + + with self.assertRaises(KeyboardInterrupt): + info.validate([ExplodingStr()]) + + +if __name__ == "__main__": + unittest.main() From dd081c8aa7f00aeb4f12cd4f5f233e4db2329944 Mon Sep 17 00:00:00 2001 From: Hugo Farajallah Date: Thu, 26 Mar 2026 12:01:53 +0100 Subject: [PATCH 2/3] fix(tests): remove numpy dependency from test_BlockUnit CI environment does not have numpy installed. Use a plain list instead of np.array() since enumerate() works on both. --- tests/test_BlockUnit.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/test_BlockUnit.py b/tests/test_BlockUnit.py index e92aded..3601598 100644 --- a/tests/test_BlockUnit.py +++ b/tests/test_BlockUnit.py @@ -5,8 +5,6 @@ from unittest.mock import MagicMock from types import SimpleNamespace -import numpy as np - # Stub out PsychoPy before importing BlockUnit _psychopy_stub = MagicMock() _psychopy_stub.core.getAbsTime.return_value = 0.0 @@ -62,7 +60,7 @@ def test_conditions_none_raises_runtime_error(self): self.assertIn("conditions", str(ctx.exception).lower()) def test_func_returning_none_raises_type_error(self): - block = _make_block(conditions=np.array(["A"])) + block = _make_block(conditions=["A"]) def bad_trial_func(win, kb, settings, cond): return None From b2450cae37d47b4885bc4746e1d2b996da70a933 Mon Sep 17 00:00:00 2001 From: Hugo Farajallah Date: Thu, 26 Mar 2026 12:04:50 +0100 Subject: [PATCH 3/3] fix(tests): skip BlockUnit/SubInfo tests when deps are missing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The CI smoke job only installs pyyaml — numpy and psychopy are not available. Use skipUnless with a try-import guard instead of stubbing sys.modules, matching the existing pattern in test_cli_root.py. --- tests/test_BlockUnit.py | 16 +++++++++------- tests/test_SubInfo.py | 23 +++++++++++------------ 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/tests/test_BlockUnit.py b/tests/test_BlockUnit.py index 3601598..b2b462b 100644 --- a/tests/test_BlockUnit.py +++ b/tests/test_BlockUnit.py @@ -5,14 +5,15 @@ from unittest.mock import MagicMock from types import SimpleNamespace -# Stub out PsychoPy before importing BlockUnit -_psychopy_stub = MagicMock() -_psychopy_stub.core.getAbsTime.return_value = 0.0 -sys.modules.setdefault("psychopy", _psychopy_stub) -sys.modules.setdefault("psychopy.core", _psychopy_stub.core) -sys.modules.setdefault("psychopy.logging", _psychopy_stub.logging) +try: + import numpy # noqa: F401 + from psychopy import core, logging # noqa: F401 + _HAS_DEPS = True +except ImportError: + _HAS_DEPS = False -from psyflow.BlockUnit import BlockUnit # noqa: E402 +if _HAS_DEPS: + from psyflow.BlockUnit import BlockUnit def _make_settings(**overrides): @@ -48,6 +49,7 @@ def _make_block(**overrides): return block +@unittest.skipUnless(_HAS_DEPS, "requires numpy and psychopy") class TestRunTrialGuards(unittest.TestCase): """run_trial() should reject invalid state with clear errors.""" diff --git a/tests/test_SubInfo.py b/tests/test_SubInfo.py index 827f9da..cdb6d23 100644 --- a/tests/test_SubInfo.py +++ b/tests/test_SubInfo.py @@ -1,21 +1,18 @@ """Tests for psyflow.SubInfo.""" -import sys import unittest from unittest.mock import MagicMock -# Stub out PsychoPy before importing SubInfo. -# test_BlockUnit may have already registered a different stub, so we use -# setdefault and then grab whatever SubInfo actually sees. -_psychopy_stub = MagicMock() -sys.modules.setdefault("psychopy", _psychopy_stub) -sys.modules.setdefault("psychopy.gui", _psychopy_stub.gui) +try: + from psychopy import gui # noqa: F401 + _HAS_PSYCHOPY = True +except ImportError: + _HAS_PSYCHOPY = False -import psyflow.SubInfo as _subinfo_mod # noqa: E402 -from psyflow.SubInfo import SubInfo # noqa: E402 - -# The gui object SubInfo actually captured at import time -_gui = _subinfo_mod.gui +if _HAS_PSYCHOPY: + import psyflow.SubInfo as _subinfo_mod + from psyflow.SubInfo import SubInfo + _gui = _subinfo_mod.gui def _make_subinfo(**field_map_overrides): @@ -35,6 +32,7 @@ def _make_subinfo(**field_map_overrides): return info +@unittest.skipUnless(_HAS_PSYCHOPY, "requires psychopy") class TestCollect(unittest.TestCase): """SubInfo.collect() control-flow edge cases.""" @@ -49,6 +47,7 @@ def test_cancel_returns_none(self): self.assertIsNone(result) +@unittest.skipUnless(_HAS_PSYCHOPY, "requires psychopy") class TestValidate(unittest.TestCase): """SubInfo.validate() error handling."""