From e37e0db6284de6c701259c76a1486a77789bf2cc Mon Sep 17 00:00:00 2001 From: bruno-f-cruz <7049351+bruno-f-cruz@users.noreply.github.com> Date: Tue, 30 Jun 2026 12:52:37 -0700 Subject: [PATCH 1/3] Update action dependencies --- .github/workflows/contraqctor.yml | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/workflows/contraqctor.yml b/.github/workflows/contraqctor.yml index 3eedd30..13bf82d 100644 --- a/.github/workflows/contraqctor.yml +++ b/.github/workflows/contraqctor.yml @@ -28,9 +28,9 @@ jobs: python-version: [ 3.11, 3.12, 3.13 ] fail-fast: false steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - - uses: astral-sh/setup-uv@v8.1.0 + - uses: astral-sh/setup-uv@v8.2.0 with: enable-cache: true @@ -81,12 +81,12 @@ jobs: version: ${{ steps.get_version.outputs.version }} steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 with: fetch-depth: 0 ref: main - - uses: astral-sh/setup-uv@v8.1.0 + - uses: astral-sh/setup-uv@v8.2.0 with: enable-cache: true @@ -108,7 +108,7 @@ jobs: run: uv build - name: Upload wheels as artifact - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: dist path: dist/ @@ -133,12 +133,12 @@ jobs: needs: prepare-release steps: - name: Download wheels artifact - uses: actions/download-artifact@v4 + uses: actions/download-artifact@v8.0.1 with: name: dist path: dist/ - - uses: astral-sh/setup-uv@v8.1.0 + - uses: astral-sh/setup-uv@v8.2.0 with: enable-cache: true @@ -165,12 +165,12 @@ jobs: if: github.event_name == 'release' && !github.event.release.prerelease steps: - name: Checkout - uses: actions/checkout@v6 + uses: actions/checkout@v7 with: ref: main - name: Install uv - uses: astral-sh/setup-uv@v8.1.0 + uses: astral-sh/setup-uv@v8.2.0 with: enable-cache: true From b3778ee9c621d454d23fec5b59a6026a73408800 Mon Sep 17 00:00:00 2001 From: bruno-f-cruz <7049351+bruno-f-cruz@users.noreply.github.com> Date: Tue, 30 Jun 2026 12:53:03 -0700 Subject: [PATCH 2/3] Honor strict=False when a collection's own load fails --- src/contraqctor/contract/base.py | 25 +++++++++--- tests/test_contract/test_core.py | 69 +++++++++++++++++++++++++++++++- tests/test_contract/test_mux.py | 5 ++- 3 files changed, 91 insertions(+), 8 deletions(-) diff --git a/src/contraqctor/contract/base.py b/src/contraqctor/contract/base.py index 703deb3..2ae8e74 100644 --- a/src/contraqctor/contract/base.py +++ b/src/contraqctor/contract/base.py @@ -386,7 +386,10 @@ def collect_errors(self) -> List[_typing.ErrorOnLoad]: """ errors = [] if self.has_error: + # A stream that errored on load has no valid data to traverse into, + # so we report its own error and stop here (iterating would re-raise). errors.append(cast(_typing.ErrorOnLoad, self._data)) + return errors for stream in self: if stream is None: continue @@ -418,6 +421,13 @@ def load_all(self, strict: bool = False) -> Self: ``` """ self.load() + if self.has_error: + if strict: + cast(_typing.ErrorOnLoad, self._data).raise_from_error() + # In non-strict mode the collection failed to load its own children, + # so there is nothing further to recurse into; the error is preserved + # and surfaced via collect_errors(). + return self for stream in self: if stream is None: continue @@ -539,16 +549,21 @@ def load(self) -> Self: Overrides the base method to add validation that loaded data is a list of DataStreams. + If the underlying read raised, the error is preserved as an ``ErrorOnLoad`` (as in the + base implementation) rather than re-raised, so that non-strict loading can collect it. + A non-list result from a *successful* read is a genuine contract violation and is captured + as an ``ErrorOnLoad`` as well. + Returns: Self: The collection instance for method chaining. - - Raises: - ValueError: If loaded data is not a list of DataStreams. """ super().load() + if self.has_error: + # read() raised and was captured as an ErrorOnLoad; leave it in place. + return self if not isinstance(self._data, list): - self._data = _typing.UnsetData - raise ValueError("Data must be a list of DataStreams.") + self._data = _typing.ErrorOnLoad(self, exception=ValueError("Data must be a list of DataStreams.")) + return self self._update_data_stream_mapping() return self diff --git a/tests/test_contract/test_core.py b/tests/test_contract/test_core.py index 3d2c0ad..7107499 100644 --- a/tests/test_contract/test_core.py +++ b/tests/test_contract/test_core.py @@ -2,7 +2,24 @@ from conftest import SimpleDataStream, SimpleParams from contraqctor import _typing -from contraqctor.contract.base import DataStream, DataStreamCollection, implicit_loading +from contraqctor.contract.base import ( + DataStream, + DataStreamCollection, + DataStreamCollectionBase, + implicit_loading, +) + + +class _FailingCollection(DataStreamCollectionBase[DataStream, SimpleParams]): + """A collection whose own reader raises, mimicking e.g. a HarpDevice pointed at a missing folder.""" + + @staticmethod + def _reader(params: SimpleParams) -> list: + with open(params.path, "r") as f: + f.read() + return [] + + make_params = SimpleParams class TestDataStream: @@ -435,3 +452,53 @@ def test_load_all_strict(self, text_file, temp_dir): with pytest.raises(FileNotFoundError): collection.load_all(strict=True) + + def test_load_all_non_strict_with_failing_collection(self, text_file, temp_dir): + """Non-strict load_all should not raise when a collection's own read() fails.""" + working = SimpleDataStream(name="working", reader_params=SimpleParams(path=text_file)) + + nonexistent_path = temp_dir / "nonexistent_folder" / "device.yml" + failing_collection = _FailingCollection(name="failing", reader_params=SimpleParams(path=nonexistent_path)) + + root = DataStreamCollection(name="root", data_streams=[working, failing_collection]) + + # Should not raise, even though the sub-collection's own read() fails. + result = root.load_all(strict=False) + + errors = result.collect_errors() + assert len(errors) == 1 + assert errors[0].data_stream == failing_collection + assert isinstance(errors[0].exception, FileNotFoundError) + + assert failing_collection.has_error + assert root.at("working").has_data + + def test_load_all_strict_with_failing_collection(self, temp_dir): + """Strict load_all should raise the original error when a collection's own read() fails.""" + nonexistent_path = temp_dir / "nonexistent_folder" / "device.yml" + failing_collection = _FailingCollection(name="failing", reader_params=SimpleParams(path=nonexistent_path)) + + root = DataStreamCollection(name="root", data_streams=[failing_collection]) + + with pytest.raises(FileNotFoundError): + root.load_all(strict=True) + + def test_load_collection_non_list_result_is_captured(self, temp_dir): + """A successful read returning a non-list is captured as an ErrorOnLoad, not raised.""" + bad_file = temp_dir / "bad.txt" + bad_file.write_text("not a list") + + class _BadResultCollection(DataStreamCollectionBase[DataStream, SimpleParams]): + @staticmethod + def _reader(params: SimpleParams) -> list: + with open(params.path, "r") as f: + return f.read() # returns a str, not a list + + make_params = SimpleParams + + collection = _BadResultCollection(name="bad", reader_params=SimpleParams(path=bad_file)) + + collection.load() + assert collection.has_error + with pytest.raises(ValueError, match="Data must be a list of DataStreams."): + collection.collect_errors()[0].raise_from_error() diff --git a/tests/test_contract/test_mux.py b/tests/test_contract/test_mux.py index a952bbd..cf4782e 100644 --- a/tests/test_contract/test_mux.py +++ b/tests/test_contract/test_mux.py @@ -61,9 +61,10 @@ def test_multiple_paths(self, temp_dir): # grab temp_dir from conftest ), ) + mux.load() + assert mux.has_error with pytest.raises(ValueError): - # Should throw since we have duplicate file names - mux.load() + _ = mux.data def test_include_exclude_patterns(self, temp_dir): """Test include and exclude patterns.""" From a8a9ece5eb482afa84a1e8ead71dc2b40e549f53 Mon Sep 17 00:00:00 2001 From: bruno-f-cruz <7049351+bruno-f-cruz@users.noreply.github.com> Date: Tue, 30 Jun 2026 14:02:20 -0700 Subject: [PATCH 3/3] Add suite-level setup_suite/teardown_suite and defer QC data loading into it --- src/contraqctor/qc/base.py | 159 ++++++++++++++++-- src/contraqctor/qc/harp/environment_sensor.py | 16 +- src/contraqctor/qc/harp/lickety_split.py | 16 +- src/contraqctor/qc/harp/sniff_detector.py | 19 ++- src/contraqctor/qc/harp/treadmill.py | 16 +- tests/test_qc/harp/test_environment_sensor.py | 6 + .../test_qc/harp/test_harp_sniff_detector.py | 5 + tests/test_qc/harp/test_lickety_split.py | 8 + tests/test_qc/harp/test_treadmill.py | 7 + tests/test_qc/test_base.py | 116 +++++++++++++ 10 files changed, 341 insertions(+), 27 deletions(-) diff --git a/src/contraqctor/qc/base.py b/src/contraqctor/qc/base.py index c780ccf..386bb97 100644 --- a/src/contraqctor/qc/base.py +++ b/src/contraqctor/qc/base.py @@ -292,6 +292,33 @@ class Suite(abc.ABC): All test suites should inherit from this class and implement test methods that start with 'test'. + .. important:: + **Keep ``__init__`` free of data I/O; load data in :meth:`setup_suite` + instead.** A suite's constructor should only store references to the data + streams it needs; it must not read a stream's ``.data`` (or otherwise + perform I/O that can fail). The :class:`Runner` only wraps the execution of + ``test_*`` methods — together with the lifecycle hooks + :meth:`setup_suite`/:meth:`teardown_suite` and :meth:`setup`/:meth:`teardown` + — in exception handling. Any exception raised while *constructing* a suite + (e.g. accessing a register whose ``.bin`` file is missing on an incomplete + dataset) propagates out and aborts the whole suite-assembly step before a + single test runs. + + The rule: anything that performs I/O (or any other work that can fail) + belongs in a lifecycle hook, not ``__init__``. The hooks mirror + :mod:`unittest`: + + * :meth:`setup_suite` / :meth:`teardown_suite` run **once per suite** + (like ``setUpClass`` / ``tearDownClass``). This is the right place to + load data: it happens a single time, and if it fails every test in the + suite is reported as an error rather than crashing assembly. + * :meth:`setup` / :meth:`teardown` run **once per test** (like ``setUp`` / + ``tearDown``). + + Note these hooks run at execution time, not at construction or collection. + When invoking a ``test_*`` method directly (e.g. in unit tests) call + :meth:`setup_suite` first to populate the data it relies on. + Examples: ```python from contraqctor.qc.base import Suite @@ -684,22 +711,88 @@ def skip_test(self, message: t.Optional[str] = None, *, context: t.Optional[t.An description=description, ) + def setup_suite(self) -> None: + """Run once before any test method in the suite. + + Mimics :meth:`unittest.TestCase.setUpClass`. Override this to perform + expensive or failure-prone preparation — e.g. loading a data stream's + ``.data`` — a single time for the whole suite, rather than in ``__init__`` + (which is not protected by exception handling) or in :meth:`setup` (which + re-runs before every test). + + The :class:`Runner` invokes this inside an exception-handling block. If it + raises, every test in the suite is reported as an error instead of being + run, and :meth:`teardown_suite` is *not* called. Suite construction and + test discovery are unaffected. + """ + pass + + def teardown_suite(self) -> None: + """Run once after all test methods in the suite have run. + + Mimics :meth:`unittest.TestCase.tearDownClass`. Only invoked if + :meth:`setup_suite` completed successfully. + """ + pass + def setup(self) -> None: """Run before each test method. - This method can be overridden by subclasses to implement - setup logic that runs before each test. + Mimics :meth:`unittest.TestCase.setUp`. This method can be overridden by + subclasses to implement setup logic that runs before each test. For work + that should happen only once for the whole suite, override + :meth:`setup_suite` instead. """ pass def teardown(self) -> None: """Run after each test method. - This method can be overridden by subclasses to implement - teardown logic that runs after each test. + Mimics :meth:`unittest.TestCase.tearDown`. This method can be overridden + by subclasses to implement teardown logic that runs after each test. For + work that should happen only once for the whole suite, override + :meth:`teardown_suite` instead. """ pass + def _try_setup_suite(self) -> t.Optional[t.Tuple[Exception, str]]: + """Run suite-level setup, capturing any exception. + + Returns: + None if :meth:`setup_suite` succeeded, otherwise a tuple of the raised + exception and its formatted traceback. + """ + try: + self.setup_suite() + return None + except Exception as e: + return e, traceback.format_exc() + + def _suite_setup_error_result(self, test_method: ITest, exception: Exception, tb: str) -> Result: + """Build an ERROR result for a test that could not run due to suite-setup failure. + + Args: + test_method: The test method that was skipped. + exception: The exception raised by :meth:`setup_suite`. + tb: The formatted traceback for the exception. + + Returns: + Result: An ERROR result attributed to the skipped test. + """ + test_name = test_method.__name__ + return Result( + status=Status.ERROR, + result=None, + test_name=test_name, + suite_name=self.name, + description=getattr(test_method, "__doc__", None), + message=f"Error during suite setup: {str(exception)}", + exception=exception, + traceback=tb, + test_reference=test_method, + suite_reference=self, + ) + def _process_test_result( self, result: t.Optional[Result], test_method: ITest, test_name: str, description: t.Optional[str] ) -> Result: @@ -787,14 +880,26 @@ def run_test(self, test_method: ITest) -> t.Generator[Result, None, None]: def run_all(self) -> t.Generator[Result, None, None]: """Run all test methods in the suite. - Finds all test methods and runs them in sequence. + Runs :meth:`setup_suite` once, then all test methods in sequence, then + :meth:`teardown_suite`. If :meth:`setup_suite` raises, every test is + yielded as an error result and no tests are run. Yields: Result: Result objects produced by all test methods. """ + tests = list(self.get_tests()) + setup_failure = self._try_setup_suite() + if setup_failure is not None: + exception, tb = setup_failure + for test in tests: + yield self._suite_setup_error_result(test, exception, tb) + return - for test in self.get_tests(): - yield from self.run_test(test) + try: + for test in tests: + yield from self.run_test(test) + finally: + self.teardown_suite() @dataclasses.dataclass @@ -1213,17 +1318,26 @@ def _run_suite_tests( suite_task = progress.add_task(f"[cyan]{suite_name}".ljust(suite_name_width + 5), total=len(tests)) suite_results = [] - for test in tests: - test_name = test.__name__ - test_desc = f"[cyan]{suite_name:<{suite_name_width}} • {test_name:<{test_name_width}}" - progress.update(suite_task, description=test_desc) + setup_failure = suite._try_setup_suite() + try: + for test in tests: + test_name = test.__name__ + test_desc = f"[cyan]{suite_name:<{suite_name_width}} • {test_name:<{test_name_width}}" + progress.update(suite_task, description=test_desc) - test_results = list(suite.run_test(test)) - suite_results.extend(test_results) + if setup_failure is None: + test_results = list(suite.run_test(test)) + else: + exception, tb = setup_failure + test_results = [suite._suite_setup_error_result(test, exception, tb)] + suite_results.extend(test_results) - progress.advance(total_task) - progress.advance(group_task) - progress.advance(suite_task) + progress.advance(total_task) + progress.advance(group_task) + progress.advance(suite_task) + finally: + if setup_failure is None: + suite.teardown_suite() if tests: self._update_suite_progress(progress, suite_task, suite_name, suite_results, suite_name_width) @@ -1271,8 +1385,17 @@ def run_all(self) -> t.Dict[t.Optional[str], t.List[Result]]: for group, tests_in_group in _TaggedTest.group_by_group(collected_tests): for suite, tests_in_suite in _TaggedTest.group_by_suite(tests_in_group): results: t.List[Result] = [] - for test in tests_in_suite: - results.extend(suite.run_test(test.test)) + setup_failure = suite._try_setup_suite() + try: + for test in tests_in_suite: + if setup_failure is None: + results.extend(suite.run_test(test.test)) + else: + exception, tb = setup_failure + results.append(suite._suite_setup_error_result(test.test, exception, tb)) + finally: + if setup_failure is None: + suite.teardown_suite() for result in results: collected_results.append( _TaggedResult(suite=suite, group=group, result=result, test=result.test_reference) diff --git a/src/contraqctor/qc/harp/environment_sensor.py b/src/contraqctor/qc/harp/environment_sensor.py index b5d0635..573096a 100644 --- a/src/contraqctor/qc/harp/environment_sensor.py +++ b/src/contraqctor/qc/harp/environment_sensor.py @@ -55,8 +55,20 @@ def __init__( """ super().__init__(harp_device) self.harp_device = harp_device - self.data: pd.DataFrame = self.harp_device["SensorData"].data.copy() - self.data = self.data[self.data["MessageType"] == "EVENT"] + self.data: pd.DataFrame + + @override + def setup_suite(self) -> None: + """Load the environment sensor event data. + + Data reads are deferred out of ``__init__`` and into ``setup_suite`` (which + the runner calls once per suite inside its exception-handling block) so that + a missing/corrupt ``SensorData`` register surfaces as a failed test rather + than crashing suite construction. See :class:`contraqctor.qc.base.Suite`. + """ + super().setup_suite() + data = self.harp_device["SensorData"].data.copy() + self.data = data[data["MessageType"] == "EVENT"] def test_sampling_rate(self): """Tests if the sampling rate of the environment sensor is within nominal values""" diff --git a/src/contraqctor/qc/harp/lickety_split.py b/src/contraqctor/qc/harp/lickety_split.py index 075c1ef..7200e6f 100644 --- a/src/contraqctor/qc/harp/lickety_split.py +++ b/src/contraqctor/qc/harp/lickety_split.py @@ -74,9 +74,21 @@ def __init__( """ super().__init__(harp_device) self.harp_device = harp_device - self.data: pd.DataFrame = self.harp_device["LickState"].data.copy() - self.data = self.data[self.data["MessageType"] == "EVENT"] self.lick_refractory_period = lick_refractory_period + self.data: pd.DataFrame + + @override + def setup_suite(self) -> None: + """Load the lick-state event data. + + Data reads are deferred out of ``__init__`` and into ``setup_suite`` (which + the runner calls once per suite inside its exception-handling block) so that + a missing/corrupt ``LickState`` register surfaces as a failed test rather + than crashing suite construction. See :class:`contraqctor.qc.base.Suite`. + """ + super().setup_suite() + data = self.harp_device["LickState"].data.copy() + self.data = data[data["MessageType"] == "EVENT"] @staticmethod def _get_distinct_from_channel(data: pd.DataFrame, channel: str): diff --git a/src/contraqctor/qc/harp/sniff_detector.py b/src/contraqctor/qc/harp/sniff_detector.py index 04d4090..b432bcd 100644 --- a/src/contraqctor/qc/harp/sniff_detector.py +++ b/src/contraqctor/qc/harp/sniff_detector.py @@ -74,14 +74,27 @@ def __init__( """ super().__init__(harp_device) self.harp_device = harp_device - self.data: pd.DataFrame = self.harp_device["RawVoltage"].data.copy() - self.data = self.data[self.data["MessageType"] == "EVENT"]["RawVoltage"] - self.fs: float = self.harp_device["RawVoltageDispatchRate"].data.iloc[-1].values[0] self.quantization_ratio_thr = quantization_ratio_thr self.clustering_thr = clustering_thr self.clipping_thr = clipping_thr self.sudden_jumps_thr = sudden_jumps_thr self.notch_filter_freq = notch_filter_freq + self.data: pd.Series + self.fs: float + + @override + def setup_suite(self) -> None: + """Load the raw-voltage signal and sampling frequency. + + Data reads are deferred out of ``__init__`` and into ``setup_suite`` (which + the runner calls once per suite inside its exception-handling block) so that + a missing/corrupt register surfaces as a failed test rather than crashing + suite construction. See :class:`contraqctor.qc.base.Suite`. + """ + super().setup_suite() + data = self.harp_device["RawVoltage"].data.copy() + self.data = data[data["MessageType"] == "EVENT"]["RawVoltage"] + self.fs = self.harp_device["RawVoltageDispatchRate"].data.iloc[-1].values[0] def test_sampling_rate(self): """Tests if the sampling rate of the sniff detector is within nominal values""" diff --git a/src/contraqctor/qc/harp/treadmill.py b/src/contraqctor/qc/harp/treadmill.py index 52c0e8b..b2f1aab 100644 --- a/src/contraqctor/qc/harp/treadmill.py +++ b/src/contraqctor/qc/harp/treadmill.py @@ -46,10 +46,22 @@ def __init__(self, _harp_device: HarpDevice, *, adc_mid_tol_percent: float = 0.0 """ super().__init__(_harp_device) self._harp_device = _harp_device - self._data: pd.DataFrame = self._harp_device["SensorData"].data.copy() - self._data = self._data[self._data["MessageType"] == "EVENT"] self._adc_mid_tol_percent = adc_mid_tol_percent self._max_tick_jump = max_tick_jump + self._data: pd.DataFrame + + @override + def setup_suite(self) -> None: + """Load the treadmill sensor event data. + + Data reads are deferred out of ``__init__`` and into ``setup_suite`` (which + the runner calls once per suite inside its exception-handling block) so that + a missing/corrupt ``SensorData`` register surfaces as a failed test rather + than crashing suite construction. See :class:`contraqctor.qc.base.Suite`. + """ + super().setup_suite() + data = self._harp_device["SensorData"].data.copy() + self._data = data[data["MessageType"] == "EVENT"] def test_sampling_rate(self): """Tests if the sampling rate of the treadmill is within nominal values""" diff --git a/tests/test_qc/harp/test_environment_sensor.py b/tests/test_qc/harp/test_environment_sensor.py index 41c1950..9201157 100644 --- a/tests/test_qc/harp/test_environment_sensor.py +++ b/tests/test_qc/harp/test_environment_sensor.py @@ -148,17 +148,20 @@ def mock_env_sensor_device_bad_humidity(): class TestHarpEnvironmentSensorTestSuite: def test_init(self, mock_env_sensor_device): suite = HarpEnvironmentSensorTestSuite(mock_env_sensor_device) + suite.setup_suite() assert suite.harp_device == mock_env_sensor_device assert "Temperature" in suite.data.columns assert "Humidity" in suite.data.columns def test_sampling_rate(self, mock_env_sensor_device): suite = HarpEnvironmentSensorTestSuite(mock_env_sensor_device) + suite.setup_suite() result = suite.test_sampling_rate() assert result.status == Status.PASSED def test_temperature_within_expected_limits(self, mock_env_sensor_device, mock_env_sensor_device_bad_temp): suite = HarpEnvironmentSensorTestSuite(mock_env_sensor_device) + suite.setup_suite() result = suite.test_temperature_within_expected_limits() assert result.status == Status.PASSED assert result.context is not None @@ -167,11 +170,13 @@ def test_temperature_within_expected_limits(self, mock_env_sensor_device, mock_e assert "mean" in result.context suite = HarpEnvironmentSensorTestSuite(mock_env_sensor_device_bad_temp) + suite.setup_suite() result = suite.test_temperature_within_expected_limits() assert result.status == Status.WARNING def test_humidity_within_expected_limits(self, mock_env_sensor_device, mock_env_sensor_device_bad_humidity): suite = HarpEnvironmentSensorTestSuite(mock_env_sensor_device) + suite.setup_suite() result = suite.test_humidity_within_expected_limits() assert result.status == Status.PASSED assert result.context is not None @@ -180,5 +185,6 @@ def test_humidity_within_expected_limits(self, mock_env_sensor_device, mock_env_ assert "mean" in result.context suite = HarpEnvironmentSensorTestSuite(mock_env_sensor_device_bad_humidity) + suite.setup_suite() result = suite.test_humidity_within_expected_limits() assert result.status == Status.WARNING diff --git a/tests/test_qc/harp/test_harp_sniff_detector.py b/tests/test_qc/harp/test_harp_sniff_detector.py index 0e874e5..07139a5 100644 --- a/tests/test_qc/harp/test_harp_sniff_detector.py +++ b/tests/test_qc/harp/test_harp_sniff_detector.py @@ -144,6 +144,7 @@ class TestHarpSniffDetectorTestSuite: def test_init(self, mock_sniff_device): """Test initializing the HarpSniffDetectorTestSuite.""" suite = HarpSniffDetectorTestSuite(mock_sniff_device) + suite.setup_suite() # Check defaults assert suite.harp_device == mock_sniff_device @@ -184,11 +185,13 @@ def test_whoami(self, mock_sniff_device, mock_sniff_device_wrong_whoami): def test_sniff_detector_sampling_rate(self, mock_sniff_device, mock_sniff_device_bad_rate): """Test test_sniff_detector_sampling_rate method.""" suite = HarpSniffDetectorTestSuite(mock_sniff_device) + suite.setup_suite() result = suite.test_sampling_rate() assert result.status == Status.PASSED assert "Sampling rate is" in result.message suite = HarpSniffDetectorTestSuite(mock_sniff_device_bad_rate) + suite.setup_suite() result = suite.test_sampling_rate() assert result.status == Status.FAILED assert "not within nominal values" in result.message @@ -196,12 +199,14 @@ def test_sniff_detector_sampling_rate(self, mock_sniff_device, mock_sniff_device def test_sniff_detector_signal_quality(self, mock_sniff_device, mock_sniff_device_bad_quality): """Test test_sniff_detector_signal_quality method.""" suite = HarpSniffDetectorTestSuite(mock_sniff_device) + suite.setup_suite() result = suite.test_signal_quality() assert result.status == Status.PASSED assert "All quality checks passed." in result.message assert "context" in dir(result) suite = HarpSniffDetectorTestSuite(mock_sniff_device_bad_quality) + suite.setup_suite() result = suite.test_signal_quality() assert result.status == Status.FAILED assert "Some quality checks failed" in result.message diff --git a/tests/test_qc/harp/test_lickety_split.py b/tests/test_qc/harp/test_lickety_split.py index d967860..8b9abf9 100644 --- a/tests/test_qc/harp/test_lickety_split.py +++ b/tests/test_qc/harp/test_lickety_split.py @@ -115,17 +115,20 @@ def mock_lickety_split_device_duration_violations(): class TestHarpLicketySplitTestSuite: def test_init(self, mock_lickety_split_device): suite = HarpLicketySplitTestSuite(mock_lickety_split_device) + suite.setup_suite() assert suite.harp_device == mock_lickety_split_device assert "Channel0" in suite.data.columns def test_refractory_period_violations(self, mock_lickety_split_device, mock_lickety_split_device_many_violations): suite = HarpLicketySplitTestSuite(mock_lickety_split_device) + suite.setup_suite() result = suite.test_refractory_period_violations() assert result.status == Status.PASSED or result.status == Status.WARNING assert result.message is not None assert result.context is not None suite = HarpLicketySplitTestSuite(mock_lickety_split_device_many_violations) + suite.setup_suite() result = suite.test_refractory_period_violations() assert result.status in (Status.WARNING, Status.FAILED) assert result.message is not None @@ -133,12 +136,14 @@ def test_refractory_period_violations(self, mock_lickety_split_device, mock_lick def test_minimum_lick_rate(self, mock_lickety_split_device, mock_lickety_split_device_low_rate): suite = HarpLicketySplitTestSuite(mock_lickety_split_device) + suite.setup_suite() result = suite.test_minimum_lick_rate() assert result.status == Status.PASSED assert result.message is not None assert result.context is not None suite = HarpLicketySplitTestSuite(mock_lickety_split_device_low_rate) + suite.setup_suite() result = suite.test_minimum_lick_rate() assert result.status == Status.FAILED assert result.message is not None @@ -146,12 +151,14 @@ def test_minimum_lick_rate(self, mock_lickety_split_device, mock_lickety_split_d def test_lick_duration(self, mock_lickety_split_device, mock_lickety_split_device_duration_violations): suite = HarpLicketySplitTestSuite(mock_lickety_split_device) + suite.setup_suite() result = suite.test_lick_duration() assert result.status == Status.PASSED or result.status == Status.WARNING assert result.message is not None assert result.context is not None suite = HarpLicketySplitTestSuite(mock_lickety_split_device_duration_violations) + suite.setup_suite() result = suite.test_lick_duration() assert result.status == Status.WARNING assert result.message is not None @@ -159,6 +166,7 @@ def test_lick_duration(self, mock_lickety_split_device, mock_lickety_split_devic def test_lick_duration_no_licks(self, mock_lickety_split_device_low_rate): suite = HarpLicketySplitTestSuite(mock_lickety_split_device_low_rate) + suite.setup_suite() result = suite.test_lick_duration() assert result.status == Status.FAILED assert result.message is not None diff --git a/tests/test_qc/harp/test_treadmill.py b/tests/test_qc/harp/test_treadmill.py index 41d1551..7bca72c 100644 --- a/tests/test_qc/harp/test_treadmill.py +++ b/tests/test_qc/harp/test_treadmill.py @@ -153,41 +153,48 @@ def mock_treadmill_device_tripwire(): class TestHarpTreadmillTestSuite: def test_init(self, mock_treadmill_device): suite = HarpTreadmillTestSuite(mock_treadmill_device) + suite.setup_suite() assert suite.harp_device == mock_treadmill_device assert "Encoder" in suite._data.columns assert "Torque" in suite._data.columns def test_sampling_rate(self, mock_treadmill_device, mock_treadmill_device_bad_rate): suite = HarpTreadmillTestSuite(mock_treadmill_device) + suite.setup_suite() result = suite.test_sampling_rate() assert result.status == Status.PASSED assert result.message is not None and "Sampling rate is" in result.message suite = HarpTreadmillTestSuite(mock_treadmill_device_bad_rate) + suite.setup_suite() result = suite.test_sampling_rate() assert result.status == Status.FAILED assert result.message is not None and "not within nominal values" in result.message def test_encoder(self, mock_treadmill_device, mock_treadmill_device_zero_ticks): suite = HarpTreadmillTestSuite(mock_treadmill_device) + suite.setup_suite() result = suite.test_encoder() assert result.status == Status.PASSED assert result.message is not None and "All encoder metrics" in result.message assert "total_ticks" in result.result suite = HarpTreadmillTestSuite(mock_treadmill_device_zero_ticks) + suite.setup_suite() result = suite.test_encoder() assert result.status == Status.FAILED assert result.message is not None and "Total ticks is zero" in result.message def test_torque_range(self, mock_treadmill_device, mock_treadmill_device_bad_torque): suite = HarpTreadmillTestSuite(mock_treadmill_device) + suite.setup_suite() result = suite.test_torque_range() assert result.status == Status.PASSED assert result.message is not None assert result.result is not None suite = HarpTreadmillTestSuite(mock_treadmill_device_bad_torque) + suite.setup_suite() result = suite.test_torque_range() assert result.status == Status.FAILED assert result.message is not None diff --git a/tests/test_qc/test_base.py b/tests/test_qc/test_base.py index 5a4b75d..921f2cc 100644 --- a/tests/test_qc/test_base.py +++ b/tests/test_qc/test_base.py @@ -286,3 +286,119 @@ def test_exception(self): exception_suite = ExceptionSuite() exception_result = list(exception_suite.run_test(exception_suite.test_exception))[0] assert exception_result.suite_reference is exception_suite + + def test_setup_suite_not_called_at_construction_or_collection(self): + """Data loading deferred to setup_suite must not run at construction or collection. + + Regression test: I/O belongs in ``setup_suite`` (not ``__init__``) so that a + load error is captured by the runner. Neither constructing the suite nor + discovering its tests should trigger ``setup_suite``. + """ + + class LazySuite(Suite): + def __init__(self): + self.setup_suite_calls = 0 + + def setup_suite(self): + self.setup_suite_calls += 1 + raise FileNotFoundError("register .bin missing") + + def test_uses_data(self): + return self.pass_test(True) + + def test_independent(self): + return self.pass_test(True) + + # Construction must be side-effect free. + suite = LazySuite() + assert suite.setup_suite_calls == 0 + + # Collecting tests must not call setup_suite either. + test_names = sorted(m.__name__ for m in suite.get_tests()) + assert test_names == ["test_independent", "test_uses_data"] + assert suite.setup_suite_calls == 0 + + def test_setup_suite_runs_once_per_suite(self): + """setup_suite/teardown_suite run once per suite, not once per test.""" + + class LifecycleSuite(Suite): + def __init__(self): + self.setup_suite_calls = 0 + self.teardown_suite_calls = 0 + self.setup_calls = 0 + + def setup_suite(self): + self.setup_suite_calls += 1 + + def teardown_suite(self): + self.teardown_suite_calls += 1 + + def setup(self): + self.setup_calls += 1 + + def test_a(self): + return self.pass_test(True) + + def test_b(self): + return self.pass_test(True) + + suite = LifecycleSuite() + results = list(suite.run_all()) + + assert len(results) == 2 + assert all(r.status == Status.PASSED for r in results) + # Suite-level hooks run exactly once; per-test setup runs per test. + assert suite.setup_suite_calls == 1 + assert suite.teardown_suite_calls == 1 + assert suite.setup_calls == 2 + + def test_failing_setup_suite_surfaces_as_test_results(self): + """A failing setup_suite errors every test in the suite, not a construction crash.""" + + class LazySuite(Suite): + def __init__(self): + self.teardown_suite_calls = 0 + + def setup_suite(self): + raise FileNotFoundError("register .bin missing") + + def teardown_suite(self): + self.teardown_suite_calls += 1 + + def test_uses_data(self): + return self.pass_test(True) + + def test_independent(self): + return self.pass_test(True) + + # Construction must not raise even though setup_suite would fail. + suite = LazySuite() + results = {r.test_name: r for r in suite.run_all()} + + # Every test errors because suite setup failed before any test ran. + assert results["test_independent"].status == Status.ERROR + assert results["test_uses_data"].status == Status.ERROR + assert isinstance(results["test_uses_data"].exception, FileNotFoundError) + # teardown_suite must NOT run when setup_suite failed (mirrors unittest). + assert suite.teardown_suite_calls == 0 + + def test_failing_setup_suite_via_runner(self): + """The Runner also captures a failing setup_suite as errored tests.""" + + class LazySuite(Suite): + def setup_suite(self): + raise FileNotFoundError("register .bin missing") + + def test_a(self): + return self.pass_test(True) + + def test_b(self): + return self.pass_test(True) + + from contraqctor.qc.base import Runner + + runner = Runner().add_suite(LazySuite()) + results = runner.run_all() + flat = [r for group in results.values() for r in group] + assert len(flat) == 2 + assert all(r.status == Status.ERROR for r in flat)