diff --git a/src/openflexure_microscope_server/scan_directories.py b/src/openflexure_microscope_server/scan_directories.py index a5001fe5..25cccd2a 100644 --- a/src/openflexure_microscope_server/scan_directories.py +++ b/src/openflexure_microscope_server/scan_directories.py @@ -35,7 +35,7 @@ class ScanInfo(BaseModel): class ScanDirectoryManager: """ - A class for managinging interactions with scan directories + A class for managing interactions with scan directories """ _base_scan_dir: str @@ -57,7 +57,7 @@ class ScanDirectoryManager: def path_for(self, scan_name: str) -> str: """Return the path for a given scan name - Returns the path even if it doesn't exist) + Returns the path even if it doesn't exist """ return os.path.join(self._base_scan_dir, scan_name) @@ -97,7 +97,7 @@ class ScanDirectoryManager: return file_path @property - def all_scans(self): + def all_scans(self) -> list[str]: """Return a list of the scan names in the base directory""" return [f.name for f in os.scandir(self._base_scan_dir) if f.is_dir()] @@ -138,11 +138,11 @@ class ScanDirectoryManager: return f"{scan_name}_{scan_num:0{SCAN_ZERO_PAD_DIGITS}d}" - def new_scan_dir(self, scan_name: str) -> str: + def new_scan_dir(self, scan_name: str) -> "ScanDirectory": """Get a unique name for this scan and create a directory for it The scan will be named `{scan_name}_0001` where the number is - zero-padded to be 4 digits long (to allow correct sorting if the + zero-padded to be SCAN_ZERO_PAD_DIGITS digits long (to allow correct sorting if the scans are ordered alphanumerically). Creates a new empty folder, into which scans are saved @@ -161,11 +161,11 @@ class ScanDirectoryManager: os.makedirs(self.img_dir_for(full_scan_name)) return ScanDirectory(full_scan_name, self.base_dir) - def delete_scan(self, scan_name: str): + def delete_scan(self, scan_name: str) -> None: """Delete a scan""" shutil.rmtree(self.path_for(scan_name)) - def zip_scan(self, scan_name: str, final_version: bool = False) -> str: + def zip_scan(self, scan_name: str, final_version: bool = False) -> "ScanDirectory": """Zips any images from the scan not yet zipped, return full path to zip `final_version` Set true to stitch all files not just the scan images @@ -213,7 +213,7 @@ class ScanDirectory: ) @property - def name(self): + def name(self) -> str: """The name of the scan""" return self._name @@ -249,7 +249,7 @@ class ScanDirectory: return max(os.stat(root).st_mtime for root, _, _ in os.walk(self.dir_path)) def scan_info(self): - """Return the inforomation for the scan directory as a ScanInfo object""" + """Return the information for the scan directory as a ScanInfo object""" folder_contents = self.get_scan_files() if folder_contents: scan_images = [i for i in folder_contents if IMAGE_REGEX.search(i)] diff --git a/src/openflexure_microscope_server/things/smart_scan.py b/src/openflexure_microscope_server/things/smart_scan.py index c7dea601..9af232e4 100644 --- a/src/openflexure_microscope_server/things/smart_scan.py +++ b/src/openflexure_microscope_server/things/smart_scan.py @@ -155,7 +155,7 @@ class SmartScanThing(Thing): if self._scan_data is not None: self._return_to_starting_position() if not isinstance(e, scan_directories.NotEnoughFreeSpaceError): - # Don't stich if drive is full (already logged) + # Don't stitch if drive is full (already logged) self._perform_final_stitch() # Error must be raised so UI gives correct output raise e @@ -671,7 +671,7 @@ class SmartScanThing(Thing): "scans/{scan_name}", responses={ 200: {"description": "Successfully deleted scan"}, - 400: {"description": "Scan not deleted does"}, + 400: {"description": "An error occurred while trying to delete scan"}, }, ) def delete_scan(self, scan_name: str, logger: InvocationLogger) -> None: @@ -744,7 +744,7 @@ class SmartScanThing(Thing): This will return None (`null` to JS) if there is no preview image to return. - This is used for two things reasons: + This is used for two reasons: 1. If all caching was turned off this stitch would be sent over the network repeatedly 2. If caching was is on, then the stitch will not update when needed. diff --git a/tests/mock_things/mock_autofocus.py b/tests/mock_things/mock_autofocus.py index 6db47aa9..d32b5a50 100644 --- a/tests/mock_things/mock_autofocus.py +++ b/tests/mock_things/mock_autofocus.py @@ -12,6 +12,6 @@ class MockAutoFocusThing: # Counter for checking functions were called mock_call_count = {"looping_autofocus": 0} - def looping_autofocus(self, dz=2000, start="centre"): # noqa: ARG002 + def looping_autofocus(self, dz: int = 2000, start: str = "centre") -> None: # noqa: ARG002 """This function mocks autofocus with no return""" self.mock_call_count["looping_autofocus"] += 1 diff --git a/tests/mock_things/mock_background_detect.py b/tests/mock_things/mock_background_detect.py index a7eb5c5b..bd57ee61 100644 --- a/tests/mock_things/mock_background_detect.py +++ b/tests/mock_things/mock_background_detect.py @@ -1,4 +1,4 @@ -"""This testing submodule contains mock autofocus things. +"""This testing submodule contains mock background detect things. These mocks are designed to be inserted as dependencies to give specific functionality and returns. diff --git a/tests/test_scan_directories.py b/tests/test_scan_directories.py index 1816035e..e9bd63c9 100644 --- a/tests/test_scan_directories.py +++ b/tests/test_scan_directories.py @@ -24,13 +24,13 @@ LOGGER = logging.getLogger("mock-invocation_logger") BASE_SCAN_DIR = os.path.join(tempfile.gettempdir(), "scans") -def _clear_scan_dir(): +def _clear_scan_dir() -> None: """Delete the scan dir""" if os.path.exists(BASE_SCAN_DIR): shutil.rmtree(BASE_SCAN_DIR) -def _add_fake_image(scan_dir: ScanDirectory): +def _add_fake_image(scan_dir: ScanDirectory) -> None: """Make a fake image on disk in the scan directory""" x_pos = random.randint(-100, 100) y_pos = random.randint(-100, 100) @@ -40,8 +40,14 @@ def _add_fake_image(scan_dir: ScanDirectory): f_obj.write("fake") -def _add_fake_file(scan_dir: ScanDirectory, filename: str, in_im_dir=False): - """Make a fake file on disk in the scan directory""" +def _add_fake_file( + scan_dir: ScanDirectory, filename: str, in_im_dir: bool = False +) -> None: + """Make a fake file on disk in the scan directory. + + :param in_im_dir: Boolean, if set True the fake file is created in the images + directory of the scan not the root directory. + """ if in_im_dir: filepath = os.path.join(scan_dir.images_dir, filename) else: @@ -53,7 +59,12 @@ def _add_fake_file(scan_dir: ScanDirectory, filename: str, in_im_dir=False): def test_basic_directory_operations(): """Test some basic operations - Rather a long test but systematically iterates through some basics. + Test some basic operations, including: + - ScanDirectoryManager creates a scan directory + - For a fake (dummy) scan, a path can be created and retrieved + - For a fake (dummy) file, a path can be created and retrieved (for both a .zip and .img file) + - When a scan directory is created, the directory exists as does the image directory + - Scan directories can be deleted and scans can be cleared """ _clear_scan_dir() assert not os.path.isdir(BASE_SCAN_DIR) @@ -66,20 +77,20 @@ def test_basic_directory_operations(): scan_path = os.path.join(BASE_SCAN_DIR, scan_name) scan_im_dir = os.path.join(BASE_SCAN_DIR, scan_name, "images") - # It doesn't exitist but we can check its paths + # It doesn't exist but we can check its paths assert not scan_dir_manager.exists(scan_name) assert not os.path.isdir(scan_path) assert scan_dir_manager.path_for(scan_name) == scan_path assert scan_dir_manager.img_dir_for(scan_name) == scan_im_dir - # or a get the path of a fake file + # Get the path of a fake file fake_file = scan_dir_manager.get_file_from(scan_name, "foo.zip") assert fake_file == os.path.join(scan_path, "foo.zip") # But this is none if we check it exists fake_file = scan_dir_manager.get_file_from(scan_name, "foo.zip", check_exists=True) assert fake_file is None - # or a get the path of a fake file + # Get the path of another fake file fake_file = scan_dir_manager.get_file_from_img_dir(scan_name, "bar.img") assert fake_file == os.path.join(scan_im_dir, "bar.img") # But this is none if we check it exists @@ -97,6 +108,7 @@ def test_basic_directory_operations(): assert os.path.isdir(scan_path) assert os.path.isdir(scan_im_dir) + # Test cleanup. Delete the created scan and scan directory scan_dir_manager.delete_scan(scan_name) assert not scan_dir_manager.exists(scan_name) assert not os.path.isdir(scan_path) @@ -116,7 +128,7 @@ def test_scan_sequence_and_listing(): # Check they exist and are numbered sequentially all_scans = scan_dir_manager.all_scans - assert len(all_scans) == 4 + assert len(set(all_scans)) == 4 assert "fake_scan_0001" in all_scans assert "fake_scan_0002" in all_scans assert "fake_scan_0003" in all_scans @@ -149,7 +161,7 @@ def test_scan_name_non_sequential(): assert scan_dir.name == "fake_scan_0012" all_scans = scan_dir_manager.all_scans - assert len(all_scans) == 7 + assert len(set(all_scans)) == 7 assert "fake_scan_0001" in all_scans assert "fake_scan_0002" in all_scans assert "fake_scan_0003" in all_scans @@ -192,15 +204,14 @@ def test_scan_info(): now = time.time() assert info.name == "fake_scan_0001" - # Created and modifies in the last 5 seconds - assert now - 5 < info.created < now - # Created and modifies in the last 5 seconds + # Created and modified in the last 5 seconds assert now - 5 < info.created < now + assert now - 5 < info.modified < now assert info.number_of_images == 17 assert not info.stitch_available assert info.dzi is None - # Add a fake scan and check this recognised as a stitch not a scan image + # Add a fake scan and check this is recognised as a stitch not a scan image _add_fake_file(scan_dir, "fake_scan_0001_stitched.jpg", in_im_dir=True) info = scan_dir.scan_info() assert info.number_of_images == 17 @@ -217,10 +228,9 @@ def test_empty_scan_info(): now = time.time() assert info.name == "fake_scan_0001" - # Created and modifies in the last 5 seconds - assert now - 5 < info.created < now - # Created and modifies in the last 5 seconds + # Created and modified in the last 5 seconds assert now - 5 < info.created < now + assert now - 5 < info.modified < now assert info.number_of_images == 0 assert not info.stitch_available assert info.dzi is None @@ -236,7 +246,7 @@ def test_zipping_scan_data(): scan_dir_manager = ScanDirectoryManager(BASE_SCAN_DIR) scan_dir = scan_dir_manager.new_scan_dir("fake_scan") - # Create 21 fake scan files, a fake stitch, and a fake zip + # Create 21 fake scan images, a fake stitch, and a fake zip for i in range(21): _add_fake_image(scan_dir) _add_fake_file(scan_dir, "fake_scan_0001_stitched.jpg", in_im_dir=True) @@ -249,7 +259,7 @@ def test_zipping_scan_data(): else: zip_fname = scan_dir_manager.zip_scan("fake_scan_0001") zip_files = get_files_in_zip(zip_fname) - assert len(zip_files) == 21 + assert len(set(zip_files)) == 21 # Zip again with final version on and there should be 22 images if caller == "scan_dir": @@ -257,7 +267,7 @@ def test_zipping_scan_data(): else: scan_dir_manager.zip_scan("fake_scan_0001", final_version=True) zip_files = get_files_in_zip(zip_fname) - assert len(get_files_in_zip(zip_fname)) == 22 + assert len(set(get_files_in_zip(zip_fname))) == 22 # Check the zips are not in the zip for file in zip_files: assert not file.endswith(".zip") diff --git a/tests/test_smart_scan.py b/tests/test_smart_scan.py index 44ffcf70..da23c4b9 100644 --- a/tests/test_smart_scan.py +++ b/tests/test_smart_scan.py @@ -7,7 +7,7 @@ directly poll any properties and to start any methods. Any methods with LabThings dependency injections will require dependencies to be passed in manually. Rather than passing in real LabThings clients it is possible to create test objects that mock these clients. Thus, entirely -isolating one Thing for testing, at the expense of neededing to create detailed +isolating one Thing for testing, at the expense of needing to create detailed mock objects. For these tests to reliably represent real behaviour the mock Things will need to @@ -41,7 +41,7 @@ LOGGER = logging.getLogger("mock-invocation_logger") SCAN_DIR = os.path.join(tempfile.gettempdir(), "scans") -def _clear_scan_dir(): +def _clear_scan_dir() -> None: """Delete the scan dir""" if os.path.exists(SCAN_DIR): shutil.rmtree(SCAN_DIR) @@ -77,6 +77,7 @@ def test_private_delete_scan(smart_scan_thing, caplog): # Make the outer scan dir, but not the one to delete os.makedirs(SCAN_DIR) + # Attempt to delete the fake scan. Expect it to fail and provide a warning deleted = smart_scan_thing._delete_scan(fake_scan_name, LOGGER) assert not deleted assert len(caplog.records) == 1 @@ -104,9 +105,10 @@ def test_public_delete_scan(smart_scan_thing, caplog): # Make the outer scan dir, but not the one to delete os.makedirs(SCAN_DIR) - with pytest.raises(HTTPException) as exc_info: - smart_scan_thing.delete_scan(fake_scan_name, LOGGER) - # Should raise a 400 error if the scan doesn't exists, not a 404 as the server + # Attempt to delete the fake scan. Expect it to fail + with pytest.raises(HTTPException) as exc_info: + smart_scan_thing.delete_scan(fake_scan_name, LOGGER) + # Should raise a 400 error if the scan doesn't exist, not a 404 as the server # was not expecting to receive the scan files assert exc_info.value.status_code == 400 assert len(caplog.records) == 1 @@ -151,7 +153,7 @@ def _run_only_outer_scan(adjust_inital_state: Optional[Callable] = None): _run_scan method where this can be tested. Once this is done the final scan behaviour can be tested too. - adjust_initial_scan is a callable which accepts the mocked smart scan thing + adjust_initial_state is a callable which accepts the mocked smart scan thing as the only variable. It can be used to adjust the initial state of the test @@ -177,7 +179,7 @@ def _run_only_outer_scan(adjust_inital_state: Optional[Callable] = None): # Counter for checking functions were called mock_call_count = {"_run_scan": 0} - # Mock thing settngs as a dictionary + # Mock thing settings as a dictionary thing_settings = {"skip_background": True} def _run_scan(self): @@ -244,7 +246,7 @@ def test_outer_scan(): assert mock_ss_thing.mock_call_count["_run_scan"] == 1 -def test_outer_scan_wo_scample_skip(): +def test_outer_scan_wo_sample_skip(): """Test setup and teardown of the scan.""" def _set_skip_background(mock_ss_thing): diff --git a/webapp/src/components/tabContentComponents/scanListContent.vue b/webapp/src/components/tabContentComponents/scanListContent.vue index 6b342d4b..79741558 100644 --- a/webapp/src/components/tabContentComponents/scanListContent.vue +++ b/webapp/src/components/tabContentComponents/scanListContent.vue @@ -255,7 +255,7 @@ export default { formatDate(timestamp) { // Multiply by 1000 as JS uses ms not s let d = new Date(timestamp*1000); - // Convert to a sting in a very javascript way! + // Convert to a string in a very javascript way! let yyyy = d.getFullYear(); let mm = d.getMonth() + 1; let dd = d.getDate();