From b7383d45329d144401874ce80e889362b05fadca Mon Sep 17 00:00:00 2001 From: Sam Tygier Date: Tue, 13 Feb 2024 15:45:23 +0000 Subject: [PATCH 01/13] Add first screenshot test for live view Co-authored-by: JackEAllen Co-authored-by: Mike Sullivan Co-authored-by: Ashley Meigh --- .../eyes_tests/live_viewer_window_test.py | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 mantidimaging/eyes_tests/live_viewer_window_test.py diff --git a/mantidimaging/eyes_tests/live_viewer_window_test.py b/mantidimaging/eyes_tests/live_viewer_window_test.py new file mode 100644 index 00000000000..4e80008b84b --- /dev/null +++ b/mantidimaging/eyes_tests/live_viewer_window_test.py @@ -0,0 +1,23 @@ +# Copyright (C) 2024 ISIS Rutherford Appleton Laboratory UKRI +# SPDX - License - Identifier: GPL-3.0-or-later +from __future__ import annotations + +from mantidimaging.core.operations.loader import load_filter_packages +from mantidimaging.test_helpers.unit_test_helper import FakeFSTestCase +from pathlib import Path + +from mantidimaging.eyes_tests.base_eyes import BaseEyesTest + +filters = {f.filter_name: f for f in load_filter_packages()} # Needed for pyfakefs + + +class LiveViewerWindowTest(FakeFSTestCase, BaseEyesTest): + + def setUp(self) -> None: + super().setUp() + self.fs.add_real_directory(Path(__file__).parent.parent) + + def test_live_view_opens_without_data(self): + self.fs.create_dir("/live_dir") + self.imaging.show_live_viewer(Path("/live_dir")) + self.check_target(widget=self.imaging.live_viewer) From 2dd4adc2eb64e8cdee4c9fc9293bfe20414da308 Mon Sep 17 00:00:00 2001 From: Sam Tygier Date: Tue, 13 Feb 2024 16:23:41 +0000 Subject: [PATCH 02/13] LiveViewerWindowPresenter: refactor load_and_display_image for easier mocking Co-authored-by: JackEAllen Co-authored-by: Mike Sullivan Co-authored-by: Ashley Meigh --- .../gui/windows/live_viewer/presenter.py | 32 +++++++++++++------ 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/mantidimaging/gui/windows/live_viewer/presenter.py b/mantidimaging/gui/windows/live_viewer/presenter.py index dcb0de0e386..baf8bce9c71 100644 --- a/mantidimaging/gui/windows/live_viewer/presenter.py +++ b/mantidimaging/gui/windows/live_viewer/presenter.py @@ -79,16 +79,14 @@ def select_image(self, index: int) -> None: self.selected_image = self.model.images[index] self.view.label_active_filename.setText(self.selected_image.image_name) - self.load_and_display_image(self.selected_image.image_path) + self.display_image(self.selected_image.image_path) - def load_and_display_image(self, image_path: Path): + def display_image(self, image_path: Path): + """ + Display image in the view after validating contents + """ try: - if image_path.suffix.lower() in [".tif", ".tiff"]: - with tifffile.TiffFile(image_path) as tif: - image_data = tif.asarray() - elif image_path.suffix.lower() == ".fits": - with fits.open(image_path.__str__()) as fit: - image_data = fit[0].data + image_data = self.load_image(image_path) image_data = self.perform_operations(image_data) except (IOError, KeyError, ValueError, TiffFileError, DeflateError) as error: @@ -107,18 +105,32 @@ def load_and_display_image(self, image_path: Path): self.view.show_most_recent_image(image_data) self.view.live_viewer.show_error(None) + @staticmethod + def load_image(image_path: Path) -> np.ndarray: + """ + Load a .Tif, .Tiff or .Fits file only if it exists + and returns as an ndarray + """ + if image_path.suffix.lower() in [".tif", ".tiff"]: + with tifffile.TiffFile(image_path) as tif: + image_data = tif.asarray() + elif image_path.suffix.lower() == ".fits": + with fits.open(image_path.__str__()) as fit: + image_data = fit[0].data + return image_data + def update_image_modified(self, image_path: Path): """ Update the displayed image when the file is modified """ if self.selected_image and image_path == self.selected_image.image_path: - self.load_and_display_image(image_path) + self.display_image(image_path) def update_image_operation(self): """ Reload the current image if an operation has been performed on the current image """ - self.load_and_display_image(self.selected_image.image_path) + self.display_image(self.selected_image.image_path) def convert_image_to_imagestack(self, image_data): """ From 8ca51efc6c26a2a8b1ce615bc4113abc9c9eb880 Mon Sep 17 00:00:00 2001 From: Sam Tygier Date: Tue, 13 Feb 2024 17:23:39 +0000 Subject: [PATCH 03/13] Test for live view with data Co-authored-by: JackEAllen Co-authored-by: Mike Sullivan Co-authored-by: Ashley Meigh --- .../eyes_tests/live_viewer_window_test.py | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/mantidimaging/eyes_tests/live_viewer_window_test.py b/mantidimaging/eyes_tests/live_viewer_window_test.py index 4e80008b84b..39115fad5f8 100644 --- a/mantidimaging/eyes_tests/live_viewer_window_test.py +++ b/mantidimaging/eyes_tests/live_viewer_window_test.py @@ -2,7 +2,12 @@ # SPDX - License - Identifier: GPL-3.0-or-later from __future__ import annotations +from unittest import mock + +import numpy as np + from mantidimaging.core.operations.loader import load_filter_packages +from mantidimaging.gui.windows.live_viewer.model import Image_Data from mantidimaging.test_helpers.unit_test_helper import FakeFSTestCase from pathlib import Path @@ -17,7 +22,33 @@ def setUp(self) -> None: super().setUp() self.fs.add_real_directory(Path(__file__).parent.parent) + def _generate_image(self): + image = np.zeros((10, 10)) + image[5, :] = np.arange(10) + return image + + def _make_simple_dir(self, directory: Path): + file_list = [directory / f"abc_{i:06d}.tif" for i in range(5)] + if not directory.exists(): + self.fs.create_dir(directory) + + for file in file_list: + self.fs.create_file(file) + + return file_list + def test_live_view_opens_without_data(self): self.fs.create_dir("/live_dir") self.imaging.show_live_viewer(Path("/live_dir")) self.check_target(widget=self.imaging.live_viewer) + + @mock.patch('mantidimaging.gui.windows.live_viewer.presenter.LiveViewerWindowPresenter.load_image') + @mock.patch('mantidimaging.gui.windows.live_viewer.model.ImageWatcher') + def test_live_view_opens_with_data(self, _mock_image_watcher, mock_load_image): + self.fs.create_dir("/live_dir") + file_list = self._make_simple_dir(Path("/live_dir")) + image_list = [Image_Data(path) for path in file_list] + mock_load_image.return_value = self._generate_image() + self.imaging.show_live_viewer(Path("/live_dir")) + self.imaging.live_viewer.presenter.model._handle_image_changed_in_list(image_list) + self.check_target(widget=self.imaging.live_viewer) From 492b6c94cc2b1bbebe928f545a57b5c27d55e465 Mon Sep 17 00:00:00 2001 From: Sam Tygier Date: Tue, 13 Feb 2024 17:35:40 +0000 Subject: [PATCH 04/13] Test for live view with bad data Co-authored-by: JackEAllen Co-authored-by: Mike Sullivan Co-authored-by: Ashley Meigh --- mantidimaging/eyes_tests/live_viewer_window_test.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/mantidimaging/eyes_tests/live_viewer_window_test.py b/mantidimaging/eyes_tests/live_viewer_window_test.py index 39115fad5f8..be492af8f6d 100644 --- a/mantidimaging/eyes_tests/live_viewer_window_test.py +++ b/mantidimaging/eyes_tests/live_viewer_window_test.py @@ -52,3 +52,14 @@ def test_live_view_opens_with_data(self, _mock_image_watcher, mock_load_image): self.imaging.show_live_viewer(Path("/live_dir")) self.imaging.live_viewer.presenter.model._handle_image_changed_in_list(image_list) self.check_target(widget=self.imaging.live_viewer) + + @mock.patch('mantidimaging.gui.windows.live_viewer.presenter.LiveViewerWindowPresenter.load_image') + @mock.patch('mantidimaging.gui.windows.live_viewer.model.ImageWatcher') + def test_live_view_opens_with_bad_data(self, _mock_image_watcher, mock_load_image): + self.fs.create_dir("/live_dir") + file_list = self._make_simple_dir(Path("/live_dir")) + image_list = [Image_Data(path) for path in file_list] + mock_load_image.side_effect = ValueError + self.imaging.show_live_viewer(Path("/live_dir")) + self.imaging.live_viewer.presenter.model._handle_image_changed_in_list(image_list) + self.check_target(widget=self.imaging.live_viewer) From 858e7d339dccd0ab5850c432da294944ec967a14 Mon Sep 17 00:00:00 2001 From: Sam Tygier Date: Tue, 13 Feb 2024 17:41:57 +0000 Subject: [PATCH 05/13] Make variable for live_directory --- .../eyes_tests/live_viewer_window_test.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/mantidimaging/eyes_tests/live_viewer_window_test.py b/mantidimaging/eyes_tests/live_viewer_window_test.py index be492af8f6d..5042389f555 100644 --- a/mantidimaging/eyes_tests/live_viewer_window_test.py +++ b/mantidimaging/eyes_tests/live_viewer_window_test.py @@ -21,6 +21,8 @@ class LiveViewerWindowTest(FakeFSTestCase, BaseEyesTest): def setUp(self) -> None: super().setUp() self.fs.add_real_directory(Path(__file__).parent.parent) + self.live_directory = Path("/live_dir") + self.fs.create_dir(self.live_directory) def _generate_image(self): image = np.zeros((10, 10)) @@ -38,28 +40,25 @@ def _make_simple_dir(self, directory: Path): return file_list def test_live_view_opens_without_data(self): - self.fs.create_dir("/live_dir") - self.imaging.show_live_viewer(Path("/live_dir")) + self.imaging.show_live_viewer(self.live_directory) self.check_target(widget=self.imaging.live_viewer) @mock.patch('mantidimaging.gui.windows.live_viewer.presenter.LiveViewerWindowPresenter.load_image') @mock.patch('mantidimaging.gui.windows.live_viewer.model.ImageWatcher') def test_live_view_opens_with_data(self, _mock_image_watcher, mock_load_image): - self.fs.create_dir("/live_dir") - file_list = self._make_simple_dir(Path("/live_dir")) + file_list = self._make_simple_dir(self.live_directory) image_list = [Image_Data(path) for path in file_list] mock_load_image.return_value = self._generate_image() - self.imaging.show_live_viewer(Path("/live_dir")) + self.imaging.show_live_viewer(self.live_directory) self.imaging.live_viewer.presenter.model._handle_image_changed_in_list(image_list) self.check_target(widget=self.imaging.live_viewer) @mock.patch('mantidimaging.gui.windows.live_viewer.presenter.LiveViewerWindowPresenter.load_image') @mock.patch('mantidimaging.gui.windows.live_viewer.model.ImageWatcher') def test_live_view_opens_with_bad_data(self, _mock_image_watcher, mock_load_image): - self.fs.create_dir("/live_dir") - file_list = self._make_simple_dir(Path("/live_dir")) + file_list = self._make_simple_dir(self.live_directory) image_list = [Image_Data(path) for path in file_list] mock_load_image.side_effect = ValueError - self.imaging.show_live_viewer(Path("/live_dir")) + self.imaging.show_live_viewer(self.live_directory) self.imaging.live_viewer.presenter.model._handle_image_changed_in_list(image_list) self.check_target(widget=self.imaging.live_viewer) From f58fd9109a2557f01c4a377b4d77225595f799db Mon Sep 17 00:00:00 2001 From: Sam Tygier Date: Tue, 13 Feb 2024 17:48:03 +0000 Subject: [PATCH 06/13] Cleanup pyfakefs hacks Some files need to be loaded from disk after the point of setting up the pyfakefs --- mantidimaging/eyes_tests/live_viewer_window_test.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/mantidimaging/eyes_tests/live_viewer_window_test.py b/mantidimaging/eyes_tests/live_viewer_window_test.py index 5042389f555..608bb722bea 100644 --- a/mantidimaging/eyes_tests/live_viewer_window_test.py +++ b/mantidimaging/eyes_tests/live_viewer_window_test.py @@ -13,14 +13,17 @@ from mantidimaging.eyes_tests.base_eyes import BaseEyesTest -filters = {f.filter_name: f for f in load_filter_packages()} # Needed for pyfakefs - class LiveViewerWindowTest(FakeFSTestCase, BaseEyesTest): + @classmethod + def setUpClass(cls) -> None: + super().setUpClass() + load_filter_packages() # Needs to be called before pyfakefs hides the filesystem + def setUp(self) -> None: super().setUp() - self.fs.add_real_directory(Path(__file__).parent.parent) + self.fs.add_real_directory(Path(__file__).parent.parent) # Allow ui file to be found self.live_directory = Path("/live_dir") self.fs.create_dir(self.live_directory) From 522edabfd040d5104994def6645db244806a3b53 Mon Sep 17 00:00:00 2001 From: Sam Tygier Date: Tue, 13 Feb 2024 18:04:30 +0000 Subject: [PATCH 07/13] Mock ImageWatcher in test_live_view_opens_without_data to avoid warning --- mantidimaging/eyes_tests/live_viewer_window_test.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mantidimaging/eyes_tests/live_viewer_window_test.py b/mantidimaging/eyes_tests/live_viewer_window_test.py index 608bb722bea..feebb931b45 100644 --- a/mantidimaging/eyes_tests/live_viewer_window_test.py +++ b/mantidimaging/eyes_tests/live_viewer_window_test.py @@ -42,7 +42,8 @@ def _make_simple_dir(self, directory: Path): return file_list - def test_live_view_opens_without_data(self): + @mock.patch('mantidimaging.gui.windows.live_viewer.model.ImageWatcher') + def test_live_view_opens_without_data(self, _mock_image_watcher): self.imaging.show_live_viewer(self.live_directory) self.check_target(widget=self.imaging.live_viewer) From 48d3076e270645efe9b5e5023b4cd3e0f8a0049c Mon Sep 17 00:00:00 2001 From: Mike Sullivan Date: Wed, 14 Feb 2024 14:52:01 +0000 Subject: [PATCH 08/13] windows screenshot tests working --- Makefile | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Makefile b/Makefile index 8b8b4ec6b7e..cffd26fd75f 100644 --- a/Makefile +++ b/Makefile @@ -10,6 +10,8 @@ CHANNELS=$(shell cat environment.yml | sed -ne '/channels:/,/dependencies:/{//!p ifeq ($(OS),Windows_NT) XVFBRUN= TEST_RESULT_DIR:=$(TEMP)\mantidimaging_tests + export APPLITOOLS_API_KEY=local + export APPLITOOLS_IMAGE_DIR:=${TEST_RESULT_DIR} else XVFBRUN=xvfb-run --auto-servernum TEST_RESULT_DIR:=$(shell mktemp -d) @@ -54,6 +56,11 @@ test-screenshots: APPLITOOLS_API_KEY=local APPLITOOLS_IMAGE_DIR=${TEST_RESULT_DIR} ${XVFBRUN} pytest -p no:xdist -p no:randomly -p no:cov mantidimaging/eyes_tests/ -vs @echo "Screenshots writen to" ${TEST_RESULT_DIR} +test-screenshots-win: + -mkdir ${TEST_RESULT_DIR} + ${XVFBRUN} pytest -p no:xdist -p no:randomly -p no:cov mantidimaging/eyes_tests/ -vs + @echo "Screenshots writen to" ${TEST_RESULT_DIR} + mypy: python -m mypy --ignore-missing-imports --no-site-packages ${SOURCE_DIRS} From 7bfc934e9b2a5fd0caadb358052abfe3f4eaaf56 Mon Sep 17 00:00:00 2001 From: Mike Sullivan Date: Wed, 14 Feb 2024 14:52:52 +0000 Subject: [PATCH 09/13] Live Viewer rotate operation screenshot test --- mantidimaging/eyes_tests/live_viewer_window_test.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/mantidimaging/eyes_tests/live_viewer_window_test.py b/mantidimaging/eyes_tests/live_viewer_window_test.py index feebb931b45..fc2c4ddb3da 100644 --- a/mantidimaging/eyes_tests/live_viewer_window_test.py +++ b/mantidimaging/eyes_tests/live_viewer_window_test.py @@ -66,3 +66,14 @@ def test_live_view_opens_with_bad_data(self, _mock_image_watcher, mock_load_imag self.imaging.show_live_viewer(self.live_directory) self.imaging.live_viewer.presenter.model._handle_image_changed_in_list(image_list) self.check_target(widget=self.imaging.live_viewer) + + @mock.patch('mantidimaging.gui.windows.live_viewer.presenter.LiveViewerWindowPresenter.load_image') + @mock.patch('mantidimaging.gui.windows.live_viewer.model.ImageWatcher') + def test_rotate_operation_rotates_image(self, _mock_image_watcher, mock_load_image): + file_list = self._make_simple_dir(self.live_directory) + image_list = [Image_Data(path) for path in file_list] + mock_load_image.return_value = self._generate_image() + self.imaging.show_live_viewer(self.live_directory) + self.imaging.live_viewer.filter_params = {"Rotate Stack": {"params": {"angle": 90}}} + self.imaging.live_viewer.presenter.model._handle_image_changed_in_list(image_list) + self.check_target(widget=self.imaging.live_viewer) From 325c0c8d20e86b6331abcd14d25a3425adcea568 Mon Sep 17 00:00:00 2001 From: MikeSullivan7 <30868085+MikeSullivan7@users.noreply.github.com> Date: Wed, 14 Feb 2024 15:58:13 +0000 Subject: [PATCH 10/13] Update live_viewer_window_test --- mantidimaging/eyes_tests/live_viewer_window_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mantidimaging/eyes_tests/live_viewer_window_test.py b/mantidimaging/eyes_tests/live_viewer_window_test.py index fc2c4ddb3da..81de5ccdeaf 100644 --- a/mantidimaging/eyes_tests/live_viewer_window_test.py +++ b/mantidimaging/eyes_tests/live_viewer_window_test.py @@ -74,6 +74,6 @@ def test_rotate_operation_rotates_image(self, _mock_image_watcher, mock_load_ima image_list = [Image_Data(path) for path in file_list] mock_load_image.return_value = self._generate_image() self.imaging.show_live_viewer(self.live_directory) - self.imaging.live_viewer.filter_params = {"Rotate Stack": {"params": {"angle": 90}}} self.imaging.live_viewer.presenter.model._handle_image_changed_in_list(image_list) + self.imaging.live_viewer.rotate_angles_group.actions()[1].trigger() self.check_target(widget=self.imaging.live_viewer) From 07445c1c8fa88c012770788c5e5ca28ebda6ac43 Mon Sep 17 00:00:00 2001 From: Sam Tygier Date: Thu, 15 Feb 2024 15:50:28 +0000 Subject: [PATCH 11/13] Move perform_operations out of try Avoid errors in operations being absorbed --- mantidimaging/gui/windows/live_viewer/presenter.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/mantidimaging/gui/windows/live_viewer/presenter.py b/mantidimaging/gui/windows/live_viewer/presenter.py index baf8bce9c71..5cabbfa79e1 100644 --- a/mantidimaging/gui/windows/live_viewer/presenter.py +++ b/mantidimaging/gui/windows/live_viewer/presenter.py @@ -87,14 +87,13 @@ def display_image(self, image_path: Path): """ try: image_data = self.load_image(image_path) - - image_data = self.perform_operations(image_data) except (IOError, KeyError, ValueError, TiffFileError, DeflateError) as error: message = f"{type(error).__name__} reading image: {image_path}: {error}" logger.error(message) self.view.remove_image() self.view.live_viewer.show_error(message) return + image_data = self.perform_operations(image_data) if image_data.size == 0: message = "reading image: {image_path}: Image has zero size" logger.error("reading image: %s: Image has zero size", image_path) From 0c9ebafb95ce044c891abd27e84239d33ca64a76 Mon Sep 17 00:00:00 2001 From: Sam Tygier Date: Thu, 15 Feb 2024 15:52:32 +0000 Subject: [PATCH 12/13] On Linux psutil reads information from /proc/meminfo --- mantidimaging/test_helpers/unit_test_helper.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mantidimaging/test_helpers/unit_test_helper.py b/mantidimaging/test_helpers/unit_test_helper.py index e2da1f607bb..4090cd7617f 100644 --- a/mantidimaging/test_helpers/unit_test_helper.py +++ b/mantidimaging/test_helpers/unit_test_helper.py @@ -165,6 +165,8 @@ class FakeFSTestCase(pyfakefs.fake_filesystem_unittest.TestCase): def setUp(self) -> None: super().setUp() self.setUpPyfakefs() + if sys.platform == 'linux': + self.fs.add_real_file("/proc/meminfo", read_only=True) def _files_equal(self, file1, file2) -> None: self.assertIsNotNone(file1) From 1e416a8a543ebb7eeac8fd1566e6c55a0ba87fd6 Mon Sep 17 00:00:00 2001 From: Sam Tygier Date: Thu, 15 Feb 2024 15:55:25 +0000 Subject: [PATCH 13/13] On linux shared arrays don't work with pyfakefs https://github.com/pytest-dev/pyfakefs/issues/949 --- mantidimaging/test_helpers/unit_test_helper.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/mantidimaging/test_helpers/unit_test_helper.py b/mantidimaging/test_helpers/unit_test_helper.py index 4090cd7617f..f4335ac266b 100644 --- a/mantidimaging/test_helpers/unit_test_helper.py +++ b/mantidimaging/test_helpers/unit_test_helper.py @@ -17,6 +17,7 @@ from mantidimaging.core.data import ImageStack from mantidimaging.core.parallel import utility as pu +from mantidimaging.core.parallel.utility import SharedArray from mantidimaging.core.utility.data_containers import ProjectionAngles backup_mp_avail = None @@ -168,6 +169,16 @@ def setUp(self) -> None: if sys.platform == 'linux': self.fs.add_real_file("/proc/meminfo", read_only=True) + #COMPAT: work around https://github.com/pytest-dev/pyfakefs/issues/949 + self._mock_create_shared_array = mock.patch( + "mantidimaging.core.parallel.utility._create_shared_array", + side_effect=lambda s, d: SharedArray(array=np.zeros(s, dtype=d), shared_memory=None)) + self._mock_create_shared_array.start() + + def tearDown(self) -> None: + if sys.platform == 'linux': + self._mock_create_shared_array.stop() + def _files_equal(self, file1, file2) -> None: self.assertIsNotNone(file1) self.assertIsNotNone(file2)