diff --git a/tests/conftest.py b/tests/conftest.py index 6a2b35a..3e878e8 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -22,9 +22,9 @@ def pytest_html_report_title(report): def _readable_test_name(item): """ - Build a human-readable name for the test report from the test's - docstring. Returns None when there is no docstring, so the default - nodeid is kept. + Use the test's docstring (first line) as its name in the report. Every + test is expected to have a docstring; falls back to the default node id + if one is missing. """ test_fn = getattr(item, "obj", None) docstring = inspect.getdoc(test_fn) if test_fn else None @@ -33,18 +33,18 @@ def _readable_test_name(item): # First non-empty line, whitespace collapsed — docstrings are often # multi-line and indented, which would render badly as a node id. - first_line = next((line.strip() for line in docstring.splitlines() if line.strip()), "") - if not first_line: + base = next((line.strip() for line in docstring.splitlines() if line.strip()), "") + if not base: return None # Keep parametrised cases distinct (they share one docstring). param_id = getattr(getattr(item, "callspec", None), "id", None) - return f"{first_line} [{param_id}]" if param_id else first_line + return f"{base} [{param_id}]" if param_id else base @pytest.hookimpl(hookwrapper=True) def pytest_runtest_makereport(item, call): - """Use the test's docstring as its name in the HTML report, when present.""" + """Use the test's docstring as its name in the report.""" outcome = yield report = outcome.get_result() diff --git a/tests/integration/test_c_find_returns_worklist_items.py b/tests/integration/test_c_find_returns_worklist_items.py index 3f4aa86..0834cd8 100644 --- a/tests/integration/test_c_find_returns_worklist_items.py +++ b/tests/integration/test_c_find_returns_worklist_items.py @@ -63,6 +63,7 @@ def event(self): return event def test_cfind_returns_scheduled_items(self, event, storage): + """C-FIND returns scheduled items.""" results = list(CFind(storage).call(event)) assert len(results) == 3 @@ -97,6 +98,7 @@ def test_cfind_returns_scheduled_items(self, event, storage): assert ds is None def test_cfind_filters_by_scheduled_date_range(self, event, storage): + """C-FIND filters by scheduled date range.""" event.identifier.ScheduledProcedureStepSequence[0].ScheduledProcedureStepStartDate = "20240101-20240201" results = list(CFind(storage).call(event)) @@ -117,6 +119,7 @@ def test_cfind_filters_by_scheduled_date_range(self, event, storage): assert ds is None def test_cfind_filters_by_accession_number(self, event, storage): + """C-FIND filters by accession number.""" event.identifier.AccessionNumber = "ACC234567" results = list(CFind(storage).call(event)) assert len(results) == 2 @@ -128,6 +131,7 @@ def test_cfind_filters_by_accession_number(self, event, storage): assert ds.AccessionNumber == "ACC234567" def test_cfind_filters_by_before_scheduled_date(self, event, storage): + """C-FIND filters by before scheduled date.""" event.identifier.ScheduledProcedureStepSequence[0].ScheduledProcedureStepStartDate = "-20240101" results = list(CFind(storage).call(event)) @@ -148,6 +152,7 @@ def test_cfind_filters_by_before_scheduled_date(self, event, storage): assert ds is None def test_cfind_filters_by_after_scheduled_date(self, event, storage): + """C-FIND filters by after scheduled date.""" event.identifier.ScheduledProcedureStepSequence[0].ScheduledProcedureStepStartDate = "20240201-" results = list(CFind(storage).call(event)) @@ -167,6 +172,7 @@ def test_cfind_filters_by_after_scheduled_date(self, event, storage): assert ds is None def test_cfind_filters_by_scheduled_time_range(self, event, storage): + """C-FIND filters by scheduled time range.""" event.identifier.ScheduledProcedureStepSequence[0].ScheduledProcedureStepStartTime = "090000-093000" results = list(CFind(storage).call(event)) @@ -187,6 +193,7 @@ def test_cfind_filters_by_scheduled_time_range(self, event, storage): assert ds is None def test_cfind_filters_by_before_scheduled_time(self, event, storage): + """C-FIND filters by before scheduled time.""" event.identifier.ScheduledProcedureStepSequence[0].ScheduledProcedureStepStartTime = "-093000" results = list(CFind(storage).call(event)) @@ -207,6 +214,7 @@ def test_cfind_filters_by_before_scheduled_time(self, event, storage): assert ds is None def test_cfind_filters_by_after_scheduled_time(self, event, storage): + """C-FIND filters by after scheduled time.""" event.identifier.ScheduledProcedureStepSequence[0].ScheduledProcedureStepStartTime = "093000-" results = list(CFind(storage).call(event)) @@ -227,6 +235,7 @@ def test_cfind_filters_by_after_scheduled_time(self, event, storage): assert ds is None def test_cfind_filters_by_date_and_time_range(self, event, storage): + """C-FIND filters by date and time range.""" event.identifier.ScheduledProcedureStepSequence[0].ScheduledProcedureStepStartDate = "20240101-20240201" event.identifier.ScheduledProcedureStepSequence[0].ScheduledProcedureStepStartTime = "090000-093000" @@ -249,6 +258,7 @@ def test_cfind_filters_by_date_and_time_range(self, event, storage): assert ds is None def test_cfind_filters_by_modality(self, event, storage): + """C-FIND filters by modality.""" storage.store_worklist_item( WorklistItem( accession_number="ACC999999", @@ -284,6 +294,7 @@ def test_cfind_filters_by_modality(self, event, storage): assert ds is None def test_cfind_filters_by_patient_id(self, event, storage): + """C-FIND filters by patient id.""" event.identifier.PatientID = "999234567" results = list(CFind(storage).call(event)) diff --git a/tests/integration/test_c_store_saves_metadata.py b/tests/integration/test_c_store_saves_metadata.py index 6e29ad8..5e7ac4a 100644 --- a/tests/integration/test_c_store_saves_metadata.py +++ b/tests/integration/test_c_store_saves_metadata.py @@ -42,6 +42,7 @@ def mwl_storage(self, tmp_dir): return MWLStorage(f"{tmp_dir}/worklist.db") def test_existing_sop_instance_uid(self, storage, mock_event): + """Existing SOP instance UID.""" sop_instance_uid = "1.2.3.4.5.6" # gitleaks:allow subject = CStore(storage) mock_event.dataset.file_meta = mock_event.file_meta @@ -67,6 +68,7 @@ def test_existing_sop_instance_uid(self, storage, mock_event): assert len(results) == 1 def test_valid_event_is_stored(self, storage, mock_event): + """Valid event is stored.""" subject = CStore(storage) assert subject.call(mock_event) == SUCCESS @@ -91,6 +93,7 @@ def test_valid_event_is_stored(self, storage, mock_event): assert Path(f"{storage.storage_root}/{storage_path}").is_file() def test_c_store_marks_worklist_in_progress(self, storage, mwl_storage, mock_event): + """C-STORE marks worklist in progress.""" item = WorklistItem( accession_number="ABC123", modality="MG", diff --git a/tests/integration/test_mwl_storage_patient_name_search.py b/tests/integration/test_mwl_storage_patient_name_search.py index 11a14d3..d86d3f0 100644 --- a/tests/integration/test_mwl_storage_patient_name_search.py +++ b/tests/integration/test_mwl_storage_patient_name_search.py @@ -41,31 +41,38 @@ def items(storage): class TestPatientNameSearch: def test_trailing_wildcard(self, storage): + """Patient name search: Trailing wildcard.""" results = storage.find_worklist_items(patient_name="SMITH*") assert {r.patient_name for r in results} == {"SMITH^SARITA", "SMITH^JANE"} def test_wildcard_on_given_name(self, storage): + """Wildcard on given name.""" results = storage.find_worklist_items(patient_name="*SARITA") assert {r.patient_name for r in results} == {"SMITH^SARITA", "JONES^SARITA"} def test_single_character_wildcard(self, storage): + """Single character wildcard.""" results = storage.find_worklist_items(patient_name="SMITH^J?NE") assert {r.patient_name for r in results} == {"SMITH^JANE"} def test_exact_match(self, storage): + """Patient name search: Exact match.""" results = storage.find_worklist_items(patient_name="JONES^SARITA") assert len(results) == 1 assert results[0].patient_name == "JONES^SARITA" def test_no_match(self, storage): + """Patient name search: No match.""" results = storage.find_worklist_items(patient_name="BROWN*") assert results == [] def test_case_insensitive_match(self, storage): + """Case insensitive match.""" results = storage.find_worklist_items(patient_name="smith*") assert {r.patient_name for r in results} == {"SMITH^SARITA", "SMITH^JANE"} @pytest.mark.xfail(reason="SQLite's UPPER() is ASCII-only; non-ASCII case folding requires ICU compilation") def test_case_insensitive_non_ascii(self, storage): + """Case insensitive non ascii.""" results = storage.find_worklist_items(patient_name="müller*") assert {r.patient_name for r in results} == {"MÜLLER^DILMA"} diff --git a/tests/integration/test_n_create_updates_worklist_status.py b/tests/integration/test_n_create_updates_worklist_status.py index 89d567f..6cbd046 100644 --- a/tests/integration/test_n_create_updates_worklist_status.py +++ b/tests/integration/test_n_create_updates_worklist_status.py @@ -39,6 +39,7 @@ def worklist_item(self): ) def test_n_create_updates_worklist_status(self, tmp_dir, worklist_item): + """N-CREATE updates worklist status.""" storage = MWLStorage(f"{tmp_dir}/test.db") study_instance_uid = generate_uid() accession_number = storage.store_worklist_item(worklist_item) diff --git a/tests/integration/test_n_set_updates_worklist_status.py b/tests/integration/test_n_set_updates_worklist_status.py index b59a6cf..15c154a 100644 --- a/tests/integration/test_n_set_updates_worklist_status.py +++ b/tests/integration/test_n_set_updates_worklist_status.py @@ -43,6 +43,7 @@ def worklist_item(self): ) def test_n_set_updates_worklist_status(self, tmp_dir, worklist_item, mpps_instance_uid): + """N-SET updates worklist status.""" storage = MWLStorage(f"{tmp_dir}/test.db") accession_number = storage.store_worklist_item(worklist_item) storage.update_status(accession_number, "IN PROGRESS", mpps_instance_uid) diff --git a/tests/integration/test_relay_listener_processes_actions.py b/tests/integration/test_relay_listener_processes_actions.py index 857dc79..44a30b6 100644 --- a/tests/integration/test_relay_listener_processes_actions.py +++ b/tests/integration/test_relay_listener_processes_actions.py @@ -18,6 +18,7 @@ def update_payload(self): @pytest.mark.asyncio async def test_relay_listener_creates_worklist_items(self, listener_payload, tmp_dir, fake_relay): + """Relay listener creates worklist items.""" storage = MWLStorage(f"{tmp_dir}/test_worklist.db") listener = RelayListener(storage) relay_message = json.dumps({"accept": {"address": "wss://accept-url"}}) @@ -34,6 +35,7 @@ async def test_relay_listener_creates_worklist_items(self, listener_payload, tmp @pytest.mark.asyncio async def test_relay_listener_updates_worklist_item_status(self, update_payload, tmp_dir, fake_relay): + """Relay listener updates worklist item status.""" storage = MWLStorage(f"{tmp_dir}/test_worklist.db") listener = RelayListener(storage) relay_message = json.dumps({"accept": {"address": "wss://accept-url"}}) diff --git a/tests/integration/test_request_cfind_on_worklist.py b/tests/integration/test_request_cfind_on_worklist.py index 698c068..ccd2e2d 100644 --- a/tests/integration/test_request_cfind_on_worklist.py +++ b/tests/integration/test_request_cfind_on_worklist.py @@ -54,6 +54,7 @@ def with_pacs_server(self, tmp_dir): server.stop() def test_cfind_request_to_worklist_server(self): + """C-FIND request to worklist server.""" ae = AE(ae_title="LOCAL_AE_TITLE") ae.add_requested_context(ModalityWorklistInformationFind) assoc = ae.associate("0.0.0.0", 4243, ae_title="MWL_SCP_AE_TITLE") @@ -82,6 +83,7 @@ def test_cfind_request_to_worklist_server(self): assert ds is None def test_cfind_with_filters_request_to_worklist_server(self): + """C-FIND with filters request to worklist server.""" ae = AE(ae_title="LOCAL_AE_TITLE") ae.add_requested_context(ModalityWorklistInformationFind) assoc = ae.associate("0.0.0.0", 4243, ae_title="MWL_SCP_AE_TITLE") diff --git a/tests/integration/test_send_c_store_to_gateway.py b/tests/integration/test_send_c_store_to_gateway.py index 8e7c69a..f47f84b 100644 --- a/tests/integration/test_send_c_store_to_gateway.py +++ b/tests/integration/test_send_c_store_to_gateway.py @@ -22,6 +22,7 @@ def with_pacs_server(self, tmp_dir): server.stop() def test_send_dicom_series_to_gateway(self, tmp_dir): + """Send DICOM series to gateway.""" number_of_instances = 5 storage = PACSStorage(f"{tmp_dir}/test.db", str(tmp_dir)) send_random_dicom_series( diff --git a/tests/scripts/test_database.py b/tests/scripts/test_database.py index 8c1990d..7751240 100644 --- a/tests/scripts/test_database.py +++ b/tests/scripts/test_database.py @@ -25,6 +25,7 @@ def worklist_db(tmp_dir, monkeypatch): # Tests for backup_database def test_backup_creates_file(tmp_dir): + """Backup creates file.""" db_path = f"{tmp_dir}/test.db" sqlite3.connect(db_path).close() @@ -34,6 +35,7 @@ def test_backup_creates_file(tmp_dir): def test_backup_returns_timestamped_path(tmp_dir): + """Backup returns timestamped path.""" db_path = f"{tmp_dir}/test.db" sqlite3.connect(db_path).close() @@ -43,6 +45,7 @@ def test_backup_returns_timestamped_path(tmp_dir): def test_backup_creates_backup_dir_if_missing(tmp_dir): + """Backup creates backup dir if missing.""" db_path = f"{tmp_dir}/test.db" sqlite3.connect(db_path).close() backup_dir = f"{tmp_dir}/backups/nested" @@ -53,12 +56,14 @@ def test_backup_creates_backup_dir_if_missing(tmp_dir): def test_backup_database_creates_backup(worklist_db): + """Backup database creates backup.""" backup_path = backup_database(worklist_db, str(Path(worklist_db).parent / "backups")) assert Path(backup_path).exists() # Tests for reset_worklist_database def test_reset_worklist_database_deletes_all_rows(worklist_db): + """Reset worklist database deletes all rows.""" reset_worklist_database() with sqlite3.connect(worklist_db) as conn: @@ -67,10 +72,12 @@ def test_reset_worklist_database_deletes_all_rows(worklist_db): def test_reset_worklist_database_returns_row_count(worklist_db): + """Reset worklist database returns row count.""" assert reset_worklist_database() == 2 def test_reset_worklist_database_returns_zero_when_empty(tmp_dir, monkeypatch): + """Reset worklist database returns zero when empty.""" db_path = f"{tmp_dir}/worklist.db" with sqlite3.connect(db_path) as conn: conn.execute("CREATE TABLE worklist_items (accession_number TEXT PRIMARY KEY)") diff --git a/tests/services/dicom/test_c_echo.py b/tests/services/dicom/test_c_echo.py index 09e059c..6ac5138 100644 --- a/tests/services/dicom/test_c_echo.py +++ b/tests/services/dicom/test_c_echo.py @@ -8,5 +8,6 @@ class TestCEcho: def test_call(self): + """C-ECHO: Call.""" event = MagicMock(spec=Event) assert CEcho().call(event) == SUCCESS diff --git a/tests/services/dicom/test_c_store.py b/tests/services/dicom/test_c_store.py index a35c5b6..eabd252 100644 --- a/tests/services/dicom/test_c_store.py +++ b/tests/services/dicom/test_c_store.py @@ -38,18 +38,21 @@ def mock_storage(self, mock_pacs_storage): return mock_pacs_storage.return_value def test_no_sop_instance_uid_fails(self, mock_storage, mock_event): + """No SOP instance UID fails.""" subject = CStore(mock_storage) mock_event.dataset.SOPInstanceUID = None assert subject.call(mock_event) == FAILURE def test_no_patient_id_fails(self, mock_storage, mock_event): + """No patient id fails.""" subject = CStore(mock_storage) mock_event.dataset.PatientID = None assert subject.call(mock_event) == FAILURE def test_existing_sop_instance_uid(self, mock_storage, mock_event): + """Existing SOP instance UID.""" mock_storage.store_instance.side_effect = pydicom.uid.generate_uid() subject = CStore(mock_storage) @@ -57,6 +60,7 @@ def test_existing_sop_instance_uid(self, mock_storage, mock_event): mock_storage.store_instance.assert_called_once() def test_valid_event_is_stored(self, mock_storage, mock_event): + """Valid event is stored.""" mock_storage.instance_exists.return_value = False subject = CStore(mock_storage) @@ -74,18 +78,22 @@ def test_valid_event_is_stored(self, mock_storage, mock_event): assert call_args[0][3] == "ae-title" # AE Title def test_storage_error_fails(self, mock_storage, mock_event): + """Storage error fails.""" mock_storage.store_instance.side_effect = Exception("Nooooo!") subject = CStore(mock_storage) assert subject.call(mock_event) == FAILURE def test_failure_hexcode(self): + """C-STORE: Failure hexcode.""" assert FAILURE == 0xC000 def test_success_hexcode(self): + """C-STORE: Success hexcode.""" assert SUCCESS == 0x0000 def test_compressor_is_called(self, mock_storage, mock_event): + """Compressor is called.""" mock_storage.instance_exists.return_value = False mock_compressor = Mock(spec=ImageCompressor) mock_compressor.compress.return_value = mock_event.dataset @@ -125,6 +133,7 @@ def test_validation_failure_notifies_manage(self, mock_storage, mock_event): mock_mwl.get_source_message_id.assert_called_once_with("ABC123") def test_worklist_marked_in_progress_on_success(self, mock_storage, mock_event): + """Worklist marked in progress on success.""" mock_mwl = Mock(spec=MWLStorage) subject = CStore(mock_storage, mwl_storage=mock_mwl) @@ -133,6 +142,7 @@ def test_worklist_marked_in_progress_on_success(self, mock_storage, mock_event): mock_mwl.update_status.assert_called_once_with("ABC123", "IN PROGRESS") def test_worklist_not_updated_on_store_failure(self, mock_storage, mock_event): + """Worklist not updated on store failure.""" mock_storage.store_instance.side_effect = Exception("store failed") mock_mwl = Mock(spec=MWLStorage) subject = CStore(mock_storage, mwl_storage=mock_mwl) @@ -142,6 +152,7 @@ def test_worklist_not_updated_on_store_failure(self, mock_storage, mock_event): mock_mwl.update_status.assert_not_called() def test_worklist_update_error_does_not_fail_store(self, mock_storage, mock_event): + """Worklist update error does not fail store.""" mock_mwl = Mock(spec=MWLStorage) mock_mwl.update_status.side_effect = Exception("db error") subject = CStore(mock_storage, mwl_storage=mock_mwl) diff --git a/tests/services/dicom/test_dicom_uploader.py b/tests/services/dicom/test_dicom_uploader.py index 65ce504..d08d9bb 100644 --- a/tests/services/dicom/test_dicom_uploader.py +++ b/tests/services/dicom/test_dicom_uploader.py @@ -19,6 +19,7 @@ def dicom_file(self): yield tf.name def test_upload_success(self, mock_put, dicom_file): + """DICOM uploader: Upload success.""" mock_response = Mock() mock_response.status_code = 201 mock_put.return_value = mock_response @@ -57,6 +58,7 @@ def test_upload_without_action_id(self, _, dicom_file): assert result is False def test_upload_failure_status_code(self, mock_put, dicom_file): + """DICOM uploader: Upload failure status code.""" mock_response = Mock() mock_response.status_code = 500 mock_response.text = "Internal server error" @@ -68,6 +70,7 @@ def test_upload_failure_status_code(self, mock_put, dicom_file): assert result is False def test_upload_timeout(self, mock_put, dicom_file): + """DICOM uploader: Upload timeout.""" mock_put.side_effect = requests.exceptions.Timeout() uploader = DICOMUploader(timeout=5) @@ -76,6 +79,7 @@ def test_upload_timeout(self, mock_put, dicom_file): assert result is False def test_upload_network_error(self, mock_put, dicom_file): + """DICOM uploader: Upload network error.""" mock_put.side_effect = requests.exceptions.ConnectionError() uploader = DICOMUploader() diff --git a/tests/services/dicom/test_image_compressor.py b/tests/services/dicom/test_image_compressor.py index 26a800c..1330a40 100644 --- a/tests/services/dicom/test_image_compressor.py +++ b/tests/services/dicom/test_image_compressor.py @@ -11,6 +11,7 @@ class TestImageCompressor: def test_compress_applies_jpeg2000_compression(self, dataset_with_pixels): + """Compress applies JPEG 2000 compression.""" subject = ImageCompressor() compressed_ds = subject.compress(dataset_with_pixels) @@ -42,6 +43,7 @@ def test_compress_dataset_with_none_pixel_data(self, dataset_with_pixels): assert result.file_meta.TransferSyntaxUID in UNCOMPRESSED_TRANSFER_SYNTAXES def test_compress_already_compressed_dataset(self, dataset_with_pixels): + """Compress already compressed dataset.""" subject = ImageCompressor(compression_ratio=100) compressed_once = subject.compress(dataset_with_pixels) @@ -52,6 +54,7 @@ def test_compress_already_compressed_dataset(self, dataset_with_pixels): @patch("services.dicom.image_compressor.compress") def test_compress_failure_returns_resized_uncompressed(self, mock_compress, dataset_with_pixels): + """Compress failure returns resized uncompressed.""" mock_compress.side_effect = Exception("Compression failed!") dataset_with_pixels.Rows = 1000 @@ -67,6 +70,7 @@ def test_compress_failure_returns_resized_uncompressed(self, mock_compress, data assert result.file_meta.TransferSyntaxUID in UNCOMPRESSED_TRANSFER_SYNTAXES def test_compress_preserves_metadata(self, dataset_with_pixels): + """Compress preserves metadata.""" dataset_with_pixels.PatientID = "123456" dataset_with_pixels.PatientName = "TEST^PATIENT" dataset_with_pixels.StudyDescription = "Test Study" @@ -79,6 +83,7 @@ def test_compress_preserves_metadata(self, dataset_with_pixels): assert compressed_ds.StudyDescription == "Test Study" def test_resizer_is_called(self, dataset_with_pixels): + """Resizer is called.""" mock_resizer = Mock(spec=ImageResizer) mock_resizer.resize.return_value = dataset_with_pixels @@ -88,6 +93,7 @@ def test_resizer_is_called(self, dataset_with_pixels): mock_resizer.resize.assert_called_once() def test_compress_with_real_resizer(self, dataset_with_pixels): + """Compress with real resizer.""" dataset_with_pixels.Rows = 3000 dataset_with_pixels.Columns = 3000 dataset_with_pixels.PixelData = np.zeros((3000, 3000), dtype=np.uint16).tobytes() @@ -101,6 +107,7 @@ def test_compress_with_real_resizer(self, dataset_with_pixels): assert compressed_ds.file_meta.TransferSyntaxUID == JPEG2000 def test_resize_failure_still_compresses(self, dataset_with_pixels): + """Resize failure still compresses.""" mock_resizer = Mock(spec=ImageResizer) mock_resizer.resize.side_effect = Exception("Resize failed!") @@ -112,6 +119,7 @@ def test_resize_failure_still_compresses(self, dataset_with_pixels): assert result.file_meta.TransferSyntaxUID == JPEG2000 def test_resize_and_compression_both_fail(self, dataset_with_pixels): + """Resize and compression both fail.""" mock_resizer = Mock(spec=ImageResizer) mock_resizer.resize.side_effect = Exception("Resize failed!") diff --git a/tests/services/dicom/test_image_resizer.py b/tests/services/dicom/test_image_resizer.py index 13d8f52..299418d 100644 --- a/tests/services/dicom/test_image_resizer.py +++ b/tests/services/dicom/test_image_resizer.py @@ -28,6 +28,7 @@ def test_resize_skips_small_images(self, dataset_with_pixels): assert resized_ds.Columns == 256 def test_resize_preserves_bit_depth(self, dataset_with_pixels): + """Resize preserves bit depth.""" dataset_with_pixels.Rows = 1000 dataset_with_pixels.Columns = 1000 dataset_with_pixels.BitsAllocated = 16 diff --git a/tests/services/dicom/test_upload_listener.py b/tests/services/dicom/test_upload_listener.py index 459120f..2126353 100644 --- a/tests/services/dicom/test_upload_listener.py +++ b/tests/services/dicom/test_upload_listener.py @@ -14,6 +14,7 @@ def mock_processor(): class TestUploadListener: def test_start_calls_process_batch(self, mock_processor): + """Upload listener: Start calls process batch.""" listener = UploadListener( processor=mock_processor, poll_interval=0.01, @@ -36,6 +37,7 @@ def stop_after_calls(*args, **kwargs): mock_processor.process_batch.assert_called_with(limit=10) def test_stop_sets_running_false(self, mock_processor): + """Upload listener: Stop sets running false.""" listener = UploadListener(processor=mock_processor) listener._running = True diff --git a/tests/services/dicom/test_upload_processor.py b/tests/services/dicom/test_upload_processor.py index 46193b7..bc17763 100644 --- a/tests/services/dicom/test_upload_processor.py +++ b/tests/services/dicom/test_upload_processor.py @@ -39,6 +39,7 @@ def processor(mock_pacs_storage, mock_mwl_storage, mock_uploader): class TestUploadProcessor: def test_process_batch_with_no_pending(self, processor, mock_pacs_storage): + """Process batch with no pending.""" mock_pacs_storage.get_pending_uploads.return_value = [] result = processor.process_batch(limit=10) @@ -47,6 +48,7 @@ def test_process_batch_with_no_pending(self, processor, mock_pacs_storage): mock_pacs_storage.get_pending_uploads.assert_called_once_with(limit=10, max_retries=3) def test_process_batch_processes_all_instances(self, processor, mock_pacs_storage, mock_uploader): + """Process batch processes all instances.""" mock_pacs_storage.get_pending_uploads.return_value = [ { "sop_instance_uid": "1.2.3.1", # gitleaks:allow @@ -71,6 +73,7 @@ def test_process_batch_processes_all_instances(self, processor, mock_pacs_storag assert mock_uploader.upload_dicom.call_count == 2 def test_upload_instance_success(self, processor, mock_pacs_storage, mock_mwl_storage, mock_uploader): + """Upload processor: Upload instance success.""" instance = { "sop_instance_uid": "1.2.3.4", # gitleaks:allow "storage_path": "ab/cd/file.dcm", @@ -90,6 +93,7 @@ def test_upload_instance_success(self, processor, mock_pacs_storage, mock_mwl_st mock_uploader.upload_dicom.assert_called_once_with("1.2.3.4", mo(), "ACTION123") # gitleaks:allow def test_upload_instance_file_not_found(self, processor, mock_pacs_storage): + """Upload processor: Upload instance file not found.""" instance = { "sop_instance_uid": "1.2.3.4", # gitleaks:allow "storage_path": "missing/file.dcm", @@ -108,6 +112,7 @@ def test_upload_instance_file_not_found(self, processor, mock_pacs_storage): assert "not found" in args[0][1] def test_upload_instance_upload_failure(self, processor, mock_pacs_storage, mock_mwl_storage, mock_uploader): + """Upload processor: Upload instance upload failure.""" instance = { "sop_instance_uid": "1.2.3.4", # gitleaks:allow "storage_path": "ab/cd/file.dcm", @@ -126,6 +131,7 @@ def test_upload_instance_upload_failure(self, processor, mock_pacs_storage, mock assert mock_pacs_storage.mark_upload_failed.call_args[1]["permanent"] is False def test_upload_instance_handles_exception(self, processor, mock_pacs_storage): + """Upload processor: Upload instance handles exception.""" instance = { "sop_instance_uid": "1.2.3.4", # gitleaks:allow "storage_path": "ab/cd/file.dcm", @@ -144,6 +150,7 @@ def test_upload_instance_handles_exception(self, processor, mock_pacs_storage): class TestBackoff: def test_backoff_increases_on_failure(self, processor, mock_pacs_storage, mock_uploader): + """Backoff increases on failure.""" mock_pacs_storage.get_pending_uploads.return_value = [ { "sop_instance_uid": "1.2.3.4", # gitleaks:allow @@ -160,6 +167,7 @@ def test_backoff_increases_on_failure(self, processor, mock_pacs_storage, mock_u assert processor.backoff_delay == 1.0 def test_backoff_capped_at_max(self, mock_pacs_storage, mock_mwl_storage, mock_uploader): + """Backoff capped at max.""" processor = UploadProcessor( pacs_storage=mock_pacs_storage, mwl_storage=mock_mwl_storage, diff --git a/tests/services/dicom/test_validator.py b/tests/services/dicom/test_validator.py index d0f16cc..1a4d6e7 100644 --- a/tests/services/dicom/test_validator.py +++ b/tests/services/dicom/test_validator.py @@ -29,10 +29,12 @@ def valid_image_dataset(self, valid_dataset): return valid_dataset def test_validate_dataset_success(self, valid_dataset): + """Validate dataset success.""" validator = DicomValidator() validator.validate_dataset(valid_dataset) # Should not raise def test_validate_dataset_missing_sop_instance_uid(self): + """Validate dataset missing SOP instance UID.""" ds = Dataset() ds.PatientID = TEST_PATIENT_ID ds.StudyInstanceUID = TEST_STUDY_INSTANCE_UID @@ -43,6 +45,7 @@ def test_validate_dataset_missing_sop_instance_uid(self): validator.validate_dataset(ds) def test_validate_dataset_missing_patient_id(self): + """Validate dataset missing patient id.""" ds = Dataset() ds.SOPInstanceUID = TEST_SOP_INSTANCE_UID ds.StudyInstanceUID = TEST_STUDY_INSTANCE_UID @@ -53,6 +56,7 @@ def test_validate_dataset_missing_patient_id(self): validator.validate_dataset(ds) def test_validate_dataset_missing_study_instance_uid(self): + """Validate dataset missing study instance UID.""" ds = Dataset() ds.SOPInstanceUID = TEST_SOP_INSTANCE_UID ds.PatientID = TEST_PATIENT_ID @@ -63,6 +67,7 @@ def test_validate_dataset_missing_study_instance_uid(self): validator.validate_dataset(ds) def test_validate_dataset_missing_sop_class_uid(self): + """Validate dataset missing SOP class UID.""" ds = Dataset() ds.SOPInstanceUID = TEST_SOP_INSTANCE_UID ds.PatientID = TEST_PATIENT_ID @@ -74,6 +79,7 @@ def test_validate_dataset_missing_sop_class_uid(self): def test_validate_bytes_valid_preamble(self): # 128 bytes preamble + DICM + minimal content + """Validate bytes valid preamble.""" data = b"\x00" * 128 + b"DICM" + b"\x00" * 100 validator = DicomValidator() @@ -81,6 +87,7 @@ def test_validate_bytes_valid_preamble(self): def test_validate_bytes_missing_preamble(self): # DICM at wrong position (no 128-byte preamble before it) + """Validate bytes missing preamble.""" data = b"DICM" + b"\x00" * 200 validator = DicomValidator() @@ -88,6 +95,7 @@ def test_validate_bytes_missing_preamble(self): validator.validate_bytes(data) def test_validate_bytes_too_small(self): + """Validate bytes too small.""" data = b"\x00" * 50 validator = DicomValidator() @@ -95,6 +103,7 @@ def test_validate_bytes_too_small(self): validator.validate_bytes(data) def test_validate_bytes_wrong_magic(self): + """Validate bytes wrong magic.""" data = b"\x00" * 128 + b"XXXX" + b"\x00" * 100 validator = DicomValidator() @@ -102,10 +111,12 @@ def test_validate_bytes_wrong_magic(self): validator.validate_bytes(data) def test_validate_pixel_data_valid(self, valid_image_dataset): + """Validate pixel data valid.""" validator = DicomValidator() validator.validate_pixel_data(valid_image_dataset) # Should not raise def test_validate_pixel_data_missing_rows(self): + """Validate pixel data missing rows.""" ds = Dataset() ds.PixelData = b"\x00" * 100 ds.Columns = 10 @@ -116,6 +127,7 @@ def test_validate_pixel_data_missing_rows(self): validator.validate_pixel_data(ds) def test_validate_pixel_data_missing_columns(self): + """Validate pixel data missing columns.""" ds = Dataset() ds.PixelData = b"\x00" * 100 ds.Rows = 10 @@ -126,6 +138,7 @@ def test_validate_pixel_data_missing_columns(self): validator.validate_pixel_data(ds) def test_validate_pixel_data_missing_bits_allocated(self): + """Validate pixel data missing bits allocated.""" ds = Dataset() ds.PixelData = b"\x00" * 100 ds.Rows = 10 @@ -136,12 +149,14 @@ def test_validate_pixel_data_missing_bits_allocated(self): validator.validate_pixel_data(ds) def test_validate_pixel_data_no_pixel_data(self): + """Validate pixel data no pixel data.""" ds = Dataset() # No PixelData validator = DicomValidator() validator.validate_pixel_data(ds) # Should not raise def test_validate_pixel_data_none_pixel_data(self): + """Validate pixel data none pixel data.""" ds = Dataset() ds.PixelData = None diff --git a/tests/services/mwl/test_c_find.py b/tests/services/mwl/test_c_find.py index 5df524f..6b963a5 100644 --- a/tests/services/mwl/test_c_find.py +++ b/tests/services/mwl/test_c_find.py @@ -50,6 +50,7 @@ class TestCFind: """Tests for CFind class.""" def test_call_with_no_results(self, handler, mock_storage, mock_event): + """C-FIND: Call with no results.""" mock_storage.find_worklist_items.return_value = [] results = list(handler.call(mock_event)) @@ -61,6 +62,7 @@ def test_call_with_no_results(self, handler, mock_storage, mock_event): mock_storage.find_worklist_items.assert_called_once() def test_call_with_single_result(self, handler, mock_storage, mock_event, sample_worklist_item): + """C-FIND: Call with single result.""" worklist_item = WorklistItem(**sample_worklist_item) mock_storage.find_worklist_items.return_value = [worklist_item] @@ -80,6 +82,7 @@ def test_call_with_single_result(self, handler, mock_storage, mock_event, sample assert ds is None def test_call_with_multiple_results(self, handler, mock_storage, mock_event): + """C-FIND: Call with multiple results.""" items = [ WorklistItem( accession_number=f"ACC00{i}", @@ -109,6 +112,7 @@ def test_call_with_multiple_results(self, handler, mock_storage, mock_event): assert status == SUCCESS def test_call_with_accession_number_filter(self, handler, mock_storage, mock_event): + """C-FIND: Call with accession number filter.""" mock_event.identifier.AccessionNumber = "ACC12345" mock_storage.find_worklist_items.return_value = [] @@ -125,6 +129,7 @@ def test_call_with_accession_number_filter(self, handler, mock_storage, mock_eve def test_call_with_modality_filter(self, handler, mock_storage, mock_event): # Add modality to query + """C-FIND: Call with modality filter.""" sps_item = Dataset() sps_item.Modality = "MG" mock_event.identifier.ScheduledProcedureStepSequence = [sps_item] @@ -142,6 +147,7 @@ def test_call_with_modality_filter(self, handler, mock_storage, mock_event): ) def test_call_with_date_filter(self, handler, mock_storage, mock_event): + """C-FIND: Call with date filter.""" sps_item = Dataset() sps_item.ScheduledProcedureStepStartDate = "20260107" mock_event.identifier.ScheduledProcedureStepSequence = [sps_item] @@ -159,6 +165,7 @@ def test_call_with_date_filter(self, handler, mock_storage, mock_event): ) def test_call_with_time_filter(self, handler, mock_storage, mock_event): + """C-FIND: Call with time filter.""" sps_item = Dataset() sps_item.ScheduledProcedureStepStartTime = "100000" mock_event.identifier.ScheduledProcedureStepSequence = [sps_item] @@ -176,6 +183,7 @@ def test_call_with_time_filter(self, handler, mock_storage, mock_event): ) def test_call_with_patient_id_filter(self, handler, mock_storage, mock_event): + """C-FIND: Call with patient id filter.""" mock_event.identifier.PatientID = "9876543210" mock_storage.find_worklist_items.return_value = [] @@ -191,6 +199,7 @@ def test_call_with_patient_id_filter(self, handler, mock_storage, mock_event): ) def test_call_with_patient_name_filter(self, handler, mock_storage, mock_event): + """C-FIND: Call with patient name filter.""" mock_event.identifier.PatientName = "Smith*" mock_storage.find_worklist_items.return_value = [] @@ -206,6 +215,7 @@ def test_call_with_patient_name_filter(self, handler, mock_storage, mock_event): ) def test_call_handles_storage_exception(self, handler, mock_storage, mock_event): + """C-FIND: Call handles storage exception.""" mock_storage.find_worklist_items.side_effect = Exception("Database error") results = list(handler.call(mock_event)) @@ -216,6 +226,7 @@ def test_call_handles_storage_exception(self, handler, mock_storage, mock_event) assert ds is None def test_call_return_key_attributes_present(self, handler, mock_storage, mock_event, sample_worklist_item): + """C-FIND: Call return key attributes present.""" worklist_item = WorklistItem(**sample_worklist_item) mock_storage.find_worklist_items.return_value = [worklist_item] diff --git a/tests/services/mwl/test_create_worklist_item.py b/tests/services/mwl/test_create_worklist_item.py index 5981454..bbd5f34 100644 --- a/tests/services/mwl/test_create_worklist_item.py +++ b/tests/services/mwl/test_create_worklist_item.py @@ -16,12 +16,14 @@ def mwl_storage(self, db_file): return MWLStorage(str(db_file)) def test_call_success(self, mwl_storage, listener_payload): + """Create worklist item: Call success.""" subject = CreateWorklistItem(mwl_storage) response = subject.call(listener_payload) assert response == {"action_id": "action-12345", "status": "created"} def test_call_missing_action_id(self, mwl_storage, listener_payload): + """Create worklist item: Call missing action id.""" subject = CreateWorklistItem(mwl_storage) del listener_payload["action_id"] @@ -31,6 +33,7 @@ def test_call_missing_action_id(self, mwl_storage, listener_payload): assert response["message"] == "Missing key: 'action_id'" def test_call_missing_accession_number(self, mwl_storage, listener_payload): + """Create worklist item: Call missing accession number.""" subject = CreateWorklistItem(mwl_storage) del listener_payload["parameters"]["worklist_item"]["accession_number"] @@ -40,6 +43,7 @@ def test_call_missing_accession_number(self, mwl_storage, listener_payload): assert response["message"] == "Missing key: 'accession_number'" def test_call_existing_worklist_item(self, mwl_storage, listener_payload): + """Create worklist item: Call existing worklist item.""" CreateWorklistItem(mwl_storage).call(listener_payload) subject = CreateWorklistItem(mwl_storage) @@ -49,6 +53,7 @@ def test_call_existing_worklist_item(self, mwl_storage, listener_payload): @patch(f"{CreateWorklistItem.__module__}.MWLStorage.store_worklist_item", side_effect=Exception("DB error")) def test_call_storage_exception(self, _, mwl_storage, listener_payload): + """Create worklist item: Call storage exception.""" subject = CreateWorklistItem(mwl_storage) response = subject.call(listener_payload) diff --git a/tests/services/mwl/test_n_create.py b/tests/services/mwl/test_n_create.py index e765d21..add178f 100644 --- a/tests/services/mwl/test_n_create.py +++ b/tests/services/mwl/test_n_create.py @@ -51,6 +51,7 @@ def event(self, sop_instance_uid): return event def test_ncreate_success(self, storage, event, sop_instance_uid): + """N-CREATE: N-CREATE success.""" status, ds = NCreate(storage).call(event) assert status == SUCCESS @@ -62,6 +63,7 @@ def test_ncreate_success(self, storage, event, sop_instance_uid): storage.update_status.assert_called_once_with("ACC123", "IN PROGRESS", sop_instance_uid) def test_ncreate_missing_sop_instance_uid(self, storage, event): + """N-CREATE missing SOP instance UID.""" event.request.AffectedSOPInstanceUID = None status, ds = NCreate(storage).call(event) @@ -70,6 +72,7 @@ def test_ncreate_missing_sop_instance_uid(self, storage, event): assert ds is None def test_ncreate_duplicate_sop_instance(self, storage, event): + """N-CREATE duplicate SOP instance.""" storage.mpps_instance_exists.return_value = True status, ds = NCreate(storage).call(event) @@ -78,6 +81,7 @@ def test_ncreate_duplicate_sop_instance(self, storage, event): assert ds is None def test_ncreate_missing_pps_status(self, storage, event): + """N-CREATE missing pps status.""" del event.attribute_list.PerformedProcedureStepStatus status, ds = NCreate(storage).call(event) @@ -86,6 +90,7 @@ def test_ncreate_missing_pps_status(self, storage, event): assert ds is None def test_ncreate_invalid_pps_status(self, storage, event): + """N-CREATE invalid pps status.""" event.attribute_list.PerformedProcedureStepStatus = "COMPLETED" status, ds = NCreate(storage).call(event) @@ -94,6 +99,7 @@ def test_ncreate_invalid_pps_status(self, storage, event): assert ds is None def test_ncreate_missing_scheduled_step_sequence(self, storage, event): + """N-CREATE missing scheduled step sequence.""" del event.attribute_list.ScheduledStepAttributesSequence status, ds = NCreate(storage).call(event) @@ -102,6 +108,7 @@ def test_ncreate_missing_scheduled_step_sequence(self, storage, event): assert ds is None def test_ncreate_processing_failure(self, storage, event): + """N-CREATE processing failure.""" storage.mpps_instance_exists.side_effect = Exception("Nooooo!") status, ds = NCreate(storage).call(event) @@ -110,6 +117,7 @@ def test_ncreate_processing_failure(self, storage, event): assert ds is None def test_ncreate_missing_accession_number(self, storage, event): + """N-CREATE missing accession number.""" del event.attribute_list.ScheduledStepAttributesSequence[0].AccessionNumber status, ds = NCreate(storage).call(event) diff --git a/tests/services/mwl/test_n_set.py b/tests/services/mwl/test_n_set.py index c29f804..0fd203c 100644 --- a/tests/services/mwl/test_n_set.py +++ b/tests/services/mwl/test_n_set.py @@ -29,6 +29,7 @@ def requested_sop_instance_uid(self): return generate_uid() def test_missing_status_returns_processing_failure(self, mock_storage, event): + """Missing status returns processing failure.""" event.request.RequestedSOPInstanceUID = generate_uid() # No PerformedProcedureStepStatus set @@ -38,6 +39,7 @@ def test_missing_status_returns_processing_failure(self, mock_storage, event): assert ds is None def test_invalid_status_returns_invalid_attribute(self, mock_storage, event, requested_sop_instance_uid): + """Invalid status returns invalid attribute.""" event.request.RequestedSOPInstanceUID = requested_sop_instance_uid event.attribute_list.PerformedProcedureStepStatus = "INVALID_STATUS" @@ -47,6 +49,7 @@ def test_invalid_status_returns_invalid_attribute(self, mock_storage, event, req assert ds is None def test_unknown_sop_instance_returns_unknown(self, mock_storage, event, requested_sop_instance_uid): + """Unknown SOP instance returns unknown.""" event.request.RequestedSOPInstanceUID = requested_sop_instance_uid event.attribute_list.PerformedProcedureStepStatus = "COMPLETED" @@ -59,6 +62,7 @@ def test_unknown_sop_instance_returns_unknown(self, mock_storage, event, request mock_storage.get_worklist_item_by_mpps_instance_uid.assert_called_once_with(requested_sop_instance_uid) def test_database_update_failure_returns_processing_failure(self, mock_storage, event, requested_sop_instance_uid): + """Database update failure returns processing failure.""" event.request.RequestedSOPInstanceUID = requested_sop_instance_uid event.attribute_list.PerformedProcedureStepStatus = "COMPLETED" @@ -75,6 +79,7 @@ def test_database_update_failure_returns_processing_failure(self, mock_storage, mock_storage.update_status.assert_called_once_with("ACC123", "COMPLETED") def test_successful_nset_returns_success_and_dataset(self, mock_storage, event, requested_sop_instance_uid): + """Successful N-SET returns success and dataset.""" event.request.RequestedSOPInstanceUID = requested_sop_instance_uid event.attribute_list.PerformedProcedureStepStatus = "COMPLETED" @@ -95,6 +100,7 @@ def test_successful_nset_returns_success_and_dataset(self, mock_storage, event, mock_storage.update_status.assert_called_once_with("ACC123", "COMPLETED") def test_exception_returns_processing_failure(self, mock_storage, event): + """Exception returns processing failure.""" event.request.RequestedSOPInstanceUID = generate_uid() event.attribute_list.PerformedProcedureStepStatus = "COMPLETED" diff --git a/tests/services/mwl/test_update_worklist_item_status.py b/tests/services/mwl/test_update_worklist_item_status.py index 8dfca41..8f17340 100644 --- a/tests/services/mwl/test_update_worklist_item_status.py +++ b/tests/services/mwl/test_update_worklist_item_status.py @@ -36,6 +36,7 @@ def mwl_storage(self, db_file: str): return MWLStorage(db_file) def test_call_success(self, mwl_storage, worklist_item_data, status_update_payload): + """Update worklist item status: Call success.""" mwl_storage.store_worklist_item(WorklistItem(**worklist_item_data)) mwl_storage.update_status("ACC123", "IN PROGRESS") @@ -44,6 +45,7 @@ def test_call_success(self, mwl_storage, worklist_item_data, status_update_paylo assert response == {"accession_number": "ACC123", "status": "updated"} def test_call_missing_keys(self, mwl_storage, status_update_payload): + """Update worklist item status: Call missing keys.""" subject = UpdateWorklistItemStatus(mwl_storage) del status_update_payload["parameters"]["worklist_item"]["status"] @@ -53,6 +55,7 @@ def test_call_missing_keys(self, mwl_storage, status_update_payload): assert "Missing key" in response["message"] def test_call_nonexistent_item(self, mwl_storage, status_update_payload): + """Update worklist item status: Call nonexistent item.""" subject = UpdateWorklistItemStatus(mwl_storage) response = subject.call(status_update_payload) @@ -60,6 +63,7 @@ def test_call_nonexistent_item(self, mwl_storage, status_update_payload): assert response["message"] == "Worklist item 'ACC123' not found" def test_call_invalid_status_transition(self, mwl_storage, worklist_item_data, status_update_payload): + """Update worklist item status: Call invalid status transition.""" subject = UpdateWorklistItemStatus(mwl_storage) mwl_storage.store_worklist_item(WorklistItem(**worklist_item_data)) diff --git a/tests/services/test_storage.py b/tests/services/test_storage.py index adde3a6..210ac8d 100644 --- a/tests/services/test_storage.py +++ b/tests/services/test_storage.py @@ -53,6 +53,7 @@ def result(): class TestPACSStorage: def test_init(self, pacs_storage, db_file): + """PACS storage: Init.""" assert Path(db_file).exists() conn = sqlite3.connect(db_file) @@ -61,6 +62,7 @@ def test_init(self, pacs_storage, db_file): assert table is not None def test_instance_exists_returns_true(self, pacs_storage): + """Instance exists returns true.""" uid = generate_uid() with pacs_storage._get_connection() as conn: @@ -76,9 +78,11 @@ def test_instance_exists_returns_true(self, pacs_storage): assert pacs_storage.instance_exists(uid) is True def test_instance_exists_returns_false(self, pacs_storage): + """Instance exists returns false.""" assert pacs_storage.instance_exists("1.2.3") is False def test_store_instance_saves_to_filesystem(self, pacs_storage): + """Store instance saves to filesystem.""" uid = generate_uid() metadata = {"patient_id": "9990001112", "patient_name": "SMITH^JANE"} @@ -93,6 +97,7 @@ def test_store_instance_saves_to_filesystem(self, pacs_storage): assert expected_rel in filepath def test_store_instance_saves_to_db(self, pacs_storage): + """Store instance saves to db.""" uid = generate_uid() metadata = { @@ -123,12 +128,14 @@ def _insert_item(self, storage, result): return item def test_init(self, mwl_storage, db_file): + """MWL storage: Init.""" conn = sqlite3.connect(db_file) table = conn.execute("SELECT name FROM sqlite_master WHERE type='table' AND name='worklist_items'").fetchone() assert table is not None def test_store_worklist_item(self, mwl_storage, result): + """Store worklist item.""" item = WorklistItem(**result) mwl_storage.store_worklist_item(item) @@ -143,6 +150,7 @@ def test_store_worklist_item(self, mwl_storage, result): assert row["patient_id"] == item.patient_id def test_store_worklist_item_already_exists(self, mwl_storage, result): + """Store worklist item already exists.""" item = WorklistItem(**result) mwl_storage.store_worklist_item(item) @@ -150,6 +158,7 @@ def test_store_worklist_item_already_exists(self, mwl_storage, result): mwl_storage.store_worklist_item(item) def test_find_worklist_items(self, mwl_storage, result): + """Find worklist items.""" item = self._insert_item(mwl_storage, result) results = mwl_storage.find_worklist_items() @@ -167,6 +176,7 @@ def test_find_worklist_items(self, mwl_storage, result): ], ) def test_find_worklist_items_with_filters(self, mwl_storage, result, query_param_name, query_param_value): + """Find worklist items with filters.""" self._insert_item(mwl_storage, result) results = mwl_storage.find_worklist_items(**{query_param_name: query_param_value}) @@ -174,6 +184,7 @@ def test_find_worklist_items_with_filters(self, mwl_storage, result, query_param assert len(results) == 1 def test_find_worklist_items_with_multiple_filters(self, mwl_storage, result): + """Find worklist items with multiple filters.""" self._insert_item(mwl_storage, result) results = mwl_storage.find_worklist_items( @@ -186,6 +197,7 @@ def test_find_worklist_items_with_multiple_filters(self, mwl_storage, result): assert len(results) == 1 def test_find_worklist_items_with_date_range(self, mwl_storage, result): + """Find worklist items with date range.""" self._insert_item(mwl_storage, result) results = mwl_storage.find_worklist_items(scheduled_date="20240101 - 20240131") @@ -193,12 +205,14 @@ def test_find_worklist_items_with_date_range(self, mwl_storage, result): assert len(results) == 1 def test_find_worklist_items_with_open_ended_date_range(self, mwl_storage, result): + """Find worklist items with open ended date range.""" self._insert_item(mwl_storage, result) assert len(mwl_storage.find_worklist_items(scheduled_date="20240101 -")) == 1 assert len(mwl_storage.find_worklist_items(scheduled_date="-20240101")) == 1 def test_find_worklist_items_with_time_range(self, mwl_storage, result): + """Find worklist items with time range.""" self._insert_item(mwl_storage, result) results = mwl_storage.find_worklist_items(scheduled_time="090000 - 170000") @@ -206,11 +220,13 @@ def test_find_worklist_items_with_time_range(self, mwl_storage, result): assert len(results) == 1 def test_find_worklist_items_with_open_ended_time_range(self, mwl_storage, result): + """Find worklist items with open ended time range.""" self._insert_item(mwl_storage, result) assert len(mwl_storage.find_worklist_items(scheduled_time="090000 -")) == 1 def test_find_worklist_items_with_date_and_time_range(self, mwl_storage, result): + """Find worklist items with date and time range.""" self._insert_item(mwl_storage, result) results = mwl_storage.find_worklist_items( @@ -222,6 +238,7 @@ def test_find_worklist_items_with_date_and_time_range(self, mwl_storage, result) @pytest.mark.parametrize("wildcard_param", ["Smith*", "*Smith*", "Sm?th*", "Smith^Jane"]) def test_find_worklist_items_patient_name_wildcard_conversion(self, mwl_storage, result, wildcard_param): + """Find worklist items patient name wildcard conversion.""" self._insert_item(mwl_storage, result) results = mwl_storage.find_worklist_items(patient_name=wildcard_param) @@ -230,6 +247,7 @@ def test_find_worklist_items_patient_name_wildcard_conversion(self, mwl_storage, assert results[0].patient_name == "SMITH^JANE" def test_find_worklist_items_with_filters_completed_items(self, mwl_storage, result): + """Find worklist items with filters completed items.""" item = self._insert_item(mwl_storage, result) mwl_storage.update_status(item.accession_number, "IN PROGRESS") mwl_storage.update_status(item.accession_number, "COMPLETED") @@ -244,6 +262,7 @@ def test_find_worklist_items_with_filters_completed_items(self, mwl_storage, res assert len(results) == 0 def test_find_worklist_items_with_filters_discontinued_items(self, mwl_storage, result): + """Find worklist items with filters discontinued items.""" item = self._insert_item(mwl_storage, result) mwl_storage.update_status(item.accession_number, "IN PROGRESS") mwl_storage.update_status(item.accession_number, "DISCONTINUED") @@ -258,6 +277,7 @@ def test_find_worklist_items_with_filters_discontinued_items(self, mwl_storage, assert len(results) == 0 def test_get_worklist_item(self, mwl_storage, result): + """Get worklist item.""" item = self._insert_item(mwl_storage, result) fetched = mwl_storage.get_worklist_item(item.accession_number) @@ -265,9 +285,11 @@ def test_get_worklist_item(self, mwl_storage, result): assert fetched == item def test_get_worklist_item_returns_none(self, mwl_storage): + """Get worklist item returns none.""" assert mwl_storage.get_worklist_item("DOES_NOT_EXIST") is None def test_update_status(self, mwl_storage, result): + """MWL storage: Update status.""" item = self._insert_item(mwl_storage, result) mwl_storage.update_status(item.accession_number, "IN PROGRESS") @@ -277,9 +299,11 @@ def test_update_status(self, mwl_storage, result): assert mwl_storage.get_worklist_item(item.accession_number).status == "COMPLETED" def test_update_status_returns_none_when_not_found(self, mwl_storage): + """MWL storage: Update status returns none when not found.""" assert mwl_storage.update_status("DOES_NOT_EXIST", "IN PROGRESS") is None def test_update_status_with_mpps(self, mwl_storage, result): + """MWL storage: Update status with MPPS.""" item = self._insert_item(mwl_storage, result) mwl_storage.update_status(item.accession_number, "IN PROGRESS") @@ -293,27 +317,32 @@ def test_update_status_with_mpps(self, mwl_storage, result): assert mwl_storage.get_worklist_item(item.accession_number).mpps_instance_uid == "some-uid" def test_update_status_raises_on_invalid_target(self, mwl_storage, result): + """MWL storage: Update status raises on invalid target.""" item = self._insert_item(mwl_storage, result) with pytest.raises(InvalidStatusTransitionError): mwl_storage.update_status(item.accession_number, "SCHEDULED") # SCHEDULED is never a valid target def test_update_status_returns_none_on_wrong_state(self, mwl_storage, result): + """MWL storage: Update status returns none on wrong state.""" item = self._insert_item(mwl_storage, result) mwl_storage.update_status(item.accession_number, "IN PROGRESS") assert mwl_storage.update_status(item.accession_number, "IN PROGRESS") is None def test_update_study_instance_uid(self, mwl_storage, result): + """MWL storage: Update study instance UID.""" item = self._insert_item(mwl_storage, result) assert mwl_storage.update_study_instance_uid(item.accession_number, "new-uid") is True def test_update_study_instance_uid_raises(self, mwl_storage): + """MWL storage: Update study instance UID raises.""" with pytest.raises(WorklistItemNotFoundError): mwl_storage.update_study_instance_uid("DOES_NOT_EXIST", "uid") def test_delete_worklist_item(self, mwl_storage, result): + """Delete worklist item.""" item = self._insert_item(mwl_storage, result) assert mwl_storage.delete_worklist_item(item.accession_number) is True @@ -327,10 +356,12 @@ def test_delete_worklist_item(self, mwl_storage, result): assert row is None def test_delete_worklist_item_raises(self, mwl_storage): + """Delete worklist item raises.""" with pytest.raises(WorklistItemNotFoundError): mwl_storage.delete_worklist_item("DOES_NOT_EXIST") def test_mpps_instance_exists(self, mwl_storage, result): + """MPPS instance exists.""" uid = generate_uid() item = self._insert_item(mwl_storage, result) mwl_storage.update_status(item.accession_number, "IN PROGRESS") @@ -340,9 +371,11 @@ def test_mpps_instance_exists(self, mwl_storage, result): assert mwl_storage.mpps_instance_exists(uid) is True def test_mpps_instance_not_exists(self, mwl_storage): + """MPPS instance not exists.""" assert mwl_storage.mpps_instance_exists("nope") is False def test_get_worklist_item_by_mpps_instance_uid(self, mwl_storage, result): + """Get worklist item by MPPS instance UID.""" uid = generate_uid() item = self._insert_item(mwl_storage, result) mwl_storage.update_status(item.accession_number, "IN PROGRESS") @@ -358,9 +391,11 @@ def test_get_worklist_item_by_mpps_instance_uid(self, mwl_storage, result): assert fetched.patient_id == item.patient_id def test_get_worklist_item_by_mpps_instance_uid_returns_none(self, mwl_storage): + """Get worklist item by MPPS instance UID returns none.""" assert mwl_storage.get_worklist_item_by_mpps_instance_uid("nope") is None def test_update_status_scheduled_to_in_progress(self, mwl_storage, result): + """MWL storage: Update status scheduled to in progress.""" item = self._insert_item(mwl_storage, result) mwl_storage.update_status(item.accession_number, "IN PROGRESS") @@ -368,6 +403,7 @@ def test_update_status_scheduled_to_in_progress(self, mwl_storage, result): assert mwl_storage.get_worklist_item(item.accession_number).status == "IN PROGRESS" def test_update_status_in_progress_to_discontinued(self, mwl_storage, result): + """MWL storage: Update status in progress to discontinued.""" item = self._insert_item(mwl_storage, result) mwl_storage.update_status(item.accession_number, "IN PROGRESS") diff --git a/tests/test_environment.py b/tests/test_environment.py index 1cc2196..c7bc06d 100644 --- a/tests/test_environment.py +++ b/tests/test_environment.py @@ -3,7 +3,7 @@ class TestEnvironment: def test_environment(self, monkeypatch): - + """Environment: Environment.""" env = Environment() # Default should be development diff --git a/tests/test_modality_emulator.py b/tests/test_modality_emulator.py index 4dce4b8..9eebe08 100644 --- a/tests/test_modality_emulator.py +++ b/tests/test_modality_emulator.py @@ -21,6 +21,7 @@ def test_generate_dicom_creates_valid_dataset( mock_generate_uid, mock_image_open, ): + """Generate DICOM creates valid dataset.""" study_instance_uid = generate_uid() sop_instance_uid = generate_uid() implementation_class_uid = generate_uid() @@ -104,6 +105,7 @@ def test_process_worklist_items_sends_all_dicoms( pending_status, success_status, ): + """Process worklist items sends all dicoms.""" study_instance_uid = generate_uid() mock_generate_uid.return_value = study_instance_uid @@ -162,6 +164,7 @@ def test_process_worklist_items_returns_when_no_items( mock_sleep, success_status, ): + """Process worklist items returns when no items.""" mwl_storage = MagicMock() emulator = ModalityEmulator(mwl_storage) ae = MagicMock() @@ -189,6 +192,7 @@ def test_process_worklist_items_handles_failed_association( self, _, ): + """Process worklist items handles failed association.""" mwl_storage = MagicMock() emulator = ModalityEmulator(mwl_storage) @@ -211,5 +215,6 @@ def test_process_worklist_items_handles_failed_association( @patch("modality_emulator.Environment", production=True) def test_main_raises_in_production(self, _): + """Main raises in production.""" with pytest.raises(RuntimeError, match="Modality Emulator should not be run in production environment"): main() diff --git a/tests/test_relay_listener.py b/tests/test_relay_listener.py index 38d2ee9..0867d11 100644 --- a/tests/test_relay_listener.py +++ b/tests/test_relay_listener.py @@ -24,6 +24,7 @@ def storage_instance(self, mock_mwl_storage): return mock_mwl_storage.return_value def test_relay_listener_initialization(self, storage_instance): + """Relay listener initialization.""" subject = RelayListener(storage_instance) assert subject.storage == storage_instance @@ -33,6 +34,7 @@ def test_relay_listener_initialization(self, storage_instance): @pytest.mark.asyncio async def test_relay_listener_listen_echo(self, storage_instance, fake_relay): + """Relay listener listen echo.""" subject = RelayListener(storage_instance) relay_message = json.dumps({"accept": {"address": "wss://accept-url"}}) @@ -47,6 +49,7 @@ async def test_relay_listener_listen_echo(self, storage_instance, fake_relay): @pytest.mark.asyncio async def test_relay_listener_listen(self, storage_instance, listener_payload, fake_relay): + """Relay listener listen.""" storage_instance.store_worklist_action.return_value = {"action_id": "action-12345", "status": "created"} subject = RelayListener(storage_instance) @@ -73,6 +76,7 @@ async def test_relay_listener_listen(self, storage_instance, listener_payload, f ) def test_process_create_item_action(self, storage_instance, listener_payload): + """Process create item action.""" subject = RelayListener(storage_instance) response = subject.process_action(listener_payload) @@ -94,6 +98,7 @@ def test_process_create_item_action(self, storage_instance, listener_payload): ) def test_process_update_item_status_action(self, storage_instance, listener_payload): + """Process update item status action.""" subject = RelayListener(storage_instance) subject.process_action(listener_payload) @@ -110,6 +115,7 @@ def test_process_update_item_status_action(self, storage_instance, listener_payl storage_instance.update_status.assert_called_once_with("ACC999999", "IN PROGRESS") def test_process_action_missing_keys(self, storage_instance, listener_payload): + """Process action missing keys.""" subject = RelayListener(storage_instance) del listener_payload["parameters"]["worklist_item"]["accession_number"] @@ -121,6 +127,7 @@ def test_process_action_missing_keys(self, storage_instance, listener_payload): storage_instance.store_worklist_item.assert_not_called() def test_process_action_invalid_type(self, storage_instance, listener_payload): + """Process action invalid type.""" subject = RelayListener(storage_instance) listener_payload["action_type"] = "worklist.unknown_action" @@ -147,12 +154,14 @@ def setup(self, monkeypatch): yield def test_connection_url(self, mock_azure_credential): + """Relay URI with default azure credential: Connection url.""" subject = RelayURI() url = subject.connection_url() assert url.startswith("wss://test-namespace/$hc/test-connection?sb-hc-action=listen") assert "sb-hc-token=Bearer+test-token" in url def test_uses_default_azure_credential(self, mock_azure_credential): + """Uses default azure credential.""" with patch("relay_listener.DefaultAzureCredential") as mock_dac: mock_dac.return_value = mock_azure_credential subject = RelayURI() @@ -171,12 +180,14 @@ def setup(self, monkeypatch): yield def test_connection_url_includes_sas_token(self): + """Connection url includes SAS token.""" subject = RelayURI() url = subject.connection_url() assert url.startswith("wss://test-namespace/$hc/test-connection?sb-hc-action=listen") assert "sb-hc-token=SharedAccessSignature" in url def test_no_credential_is_created(self): + """No credential is created.""" with patch("relay_listener.DefaultAzureCredential") as mock_dac: with patch("relay_listener.ManagedIdentityCredential") as mock_mic: RelayURI() @@ -195,12 +206,14 @@ def setup(self, monkeypatch): yield def test_uses_managed_identity_credential(self, mock_azure_credential): + """Uses managed identity credential.""" with patch("relay_listener.ManagedIdentityCredential") as mock_mic: mock_mic.return_value = mock_azure_credential subject = RelayURI() assert subject._credential is mock_mic.return_value def test_sas_key_is_ignored(self, mock_azure_credential, monkeypatch): + """SAS key is ignored.""" monkeypatch.setenv("AZURE_RELAY_SHARED_ACCESS_KEY", "some-key") subject = RelayURI() assert not subject._use_sas() @@ -209,22 +222,26 @@ def test_sas_key_is_ignored(self, mock_azure_credential, monkeypatch): class TestVerifyCredentials: def test_logs_sas_when_key_present(self, monkeypatch): + """Logs SAS when key present.""" monkeypatch.setenv("AZURE_RELAY_SHARED_ACCESS_KEY", "test-key") with patch("relay_listener.logger") as mock_logger: verify_credentials() mock_logger.info.assert_called_with("Using SAS token authentication for Azure Relay.") def test_verifies_default_azure_credential_when_no_key(self, mock_azure_credential, monkeypatch): + """Verifies default azure credential when no key.""" monkeypatch.delenv("AZURE_RELAY_SHARED_ACCESS_KEY", raising=False) verify_credentials() mock_azure_credential.get_token.assert_called_with("https://relay.azure.net/.default") def test_verifies_managed_identity_in_production(self, mock_azure_credential, monkeypatch): + """Verifies managed identity in production.""" monkeypatch.setenv("ENVIRONMENT", "prod") verify_credentials() mock_azure_credential.get_token.assert_called_with("https://relay.azure.net/.default") def test_raises_client_authentication_error_on_credential_failure(self, monkeypatch): + """Raises client authentication error on credential failure.""" monkeypatch.delenv("AZURE_RELAY_SHARED_ACCESS_KEY", raising=False) with patch("relay_listener.DefaultAzureCredential") as mock: mock.return_value.get_token.side_effect = ClientAuthenticationError("no credentials") @@ -240,6 +257,7 @@ def test_raises_client_authentication_error_on_credential_failure(self, monkeypa async def test_main_handles_connection_closed_and_keyboard_interrupt( mock_sleep, mock_relay_listener, mock_mwl_storage, mock_logger ): + """Main handles connection closed and keyboard interrupt.""" relay_listener_instance = mock_relay_listener.return_value relay_listener_instance.listen = AsyncMock() diff --git a/tests/test_server.py b/tests/test_server.py index 19f583a..ba22fe3 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -17,6 +17,7 @@ @patch(f"{PACSServer.__module__}.PACSStorage") class TestPACSServer: def test_init(self, mock_pacs_storage, mock_mwl_storage, tmp_dir): + """PACS server: Init.""" subject = PACSServer( "Custom AE Title", 2222, tmp_dir, f"{tmp_dir}/test.db", False, mwl_db_path=f"{tmp_dir}/worklist.db" ) @@ -32,6 +33,7 @@ def test_init(self, mock_pacs_storage, mock_mwl_storage, tmp_dir): mock_mwl_storage.assert_called_once_with(f"{tmp_dir}/worklist.db") def test_init_defaults(self, mock_pacs_storage, mock_mwl_storage): + """PACS server: Init defaults.""" subject = PACSServer() assert subject.ae_title == "SCREENING_PACS" @@ -48,6 +50,7 @@ def test_init_defaults(self, mock_pacs_storage, mock_mwl_storage): @patch(f"{PACSServer.__module__}.CEcho") @patch(f"{PACSServer.__module__}.CStore") def test_start(self, mock_c_store, mock_c_echo, mock_ae, _mock_pacs_storage, _mock_mwl_storage): + """PACS server: Start.""" subject = PACSServer() subject.start() @@ -69,6 +72,7 @@ def test_start(self, mock_c_store, mock_c_echo, mock_ae, _mock_pacs_storage, _mo @patch(f"{PACSServer.__module__}.AE") def test_stop(self, *_): + """PACS server: Stop.""" subject = PACSServer() subject.start() subject.stop() @@ -80,6 +84,7 @@ def test_stop(self, *_): @patch(f"{MWLServer.__module__}.MWLStorage") class TestMWLServer: def test_init(self, mock_storage): + """MWL server: Init.""" subject = MWLServer("CUSTOM_MWL", 11112, "/custom/path/worklist.db", False) assert subject.ae_title == "CUSTOM_MWL" @@ -91,6 +96,7 @@ def test_init(self, mock_storage): mock_storage.assert_called_once_with("/custom/path/worklist.db") def test_init_defaults(self, mock_storage): + """MWL server: Init defaults.""" subject = MWLServer() assert subject.ae_title == "MWL_SCP" @@ -104,6 +110,7 @@ def test_init_defaults(self, mock_storage): @patch(f"{MWLServer.__module__}.AE") @patch(f"{MWLServer.__module__}.CEcho") def test_start(self, mock_c_echo, mock_ae, _): + """MWL server: Start.""" subject = MWLServer() mock_ae_instance = MagicMock() mock_ae.return_value = mock_ae_instance @@ -130,6 +137,7 @@ def test_start(self, mock_c_echo, mock_ae, _): @patch(f"{MWLServer.__module__}.AE") def test_stop(self, *_): + """MWL server: Stop.""" subject = MWLServer() subject.start() subject.stop()