Merge pull request #44 from lukaspanni/test

Add essential unit tests

Author: lukaspanni

.github/workflows/python-app.yml

@@ -15,7 +15,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        python-version: [3.8]
+        python-version: [3.8, 3.9]
 
     steps:
     - uses: actions/checkout@v2
@@ -26,7 +26,7 @@ jobs:
     - name: Install dependencies
       run: |
         python -m pip install --upgrade pip
-        pip install flake8 pytest
+        pip install flake8 pytest pytest-cov
         if [ -f requirements.txt ]; then pip install -r requirements.txt; fi
     - name: Lint with flake8
       run: |
@@ -35,4 +35,4 @@ jobs:
         flake8 . --count --exit-zero --max-complexity=10 --max-line-length=120 --statistics
     - name: Test with pytest
       run: |
-        pytest
+        pytest --cov=imagecopy

ImageCopy/Transformers/raw_separate_transform.py

@@ -19,6 +19,8 @@ def _load_config(self, config):
                     self.raw_dir_name += "/"
             else:
                 self.raw_dir_name = "RAW/"
+            return
+        self.raw_dir_name = None
 
     def transform(self, input_dict: dict):
         if self.raw_dir_name is None:

ImageCopy/Transformers/rename_transform.py

@@ -16,14 +16,13 @@ def _load_config(self, config: dict):
         """
         Load rename config
         """
-        if "in" in config:
-            self.in_str = config["in"]
-            if "out" in config:
-                self.out_str = config["out"]
-            else:
-                raise ValueError("Invalid rename-config, regex_out missing")
-        else:
-            raise ValueError("Invalid rename-config, regex_in missing")
+        if "in" not in config or "out" not in config:
+            raise ValueError("Invalid rename-config, regex_in/regex_out missing")
+        if not isinstance(config["in"], str) or not isinstance(config["out"], str):
+            raise ValueError("Invalid rename-config, regex_in/regex_out has wrong type")
+
+        self.in_str = config["in"]
+        self.out_str = config["out"]
 
     def transform(self, input_dict: dict):
         """

ImageCopy/action_runner.py

@@ -18,15 +18,17 @@ class ActionRunner:
     """
 
     def __init__(self, config: Config):
+        if config is None or not isinstance(config, Config):
+            raise ValueError("No valid Config-Object provided")
         self.config = config
         self.path_transformers = []
         self.after_actions = []
         if self.config.grouping is not None:
             self.path_transformers.append(GroupingTransform(self.config.grouping))
-        if self.config.raw_separate:
-            self.path_transformers.append(RawSeparateTransform(config.raw_separate))
-        if self.config.rename:
-            self.path_transformers.append(RenameTransform(config.rename))
+        if self.config.raw_separate is not None:
+            self.path_transformers.append(RawSeparateTransform(self.config.raw_separate))
+        if self.config.rename is not None:
+            self.path_transformers.append(RenameTransform(self.config.rename))
         if self.config.exif is not None:
             self.after_actions.append(ExifEditing(self.config.exif))
         # if self.config.greyscale is not None:
@@ -64,4 +66,4 @@ def after_action_process(after_actions: list, image_queue: Queue, feedback_queue
                     action.execute(command)
                 counter += 1
                 feedback_queue.put(counter)
-        feedback_queue.put("END")
+        feedback_queue.put("END")

ImageCopy/copier.py

@@ -0,0 +1,62 @@
+import os
+import shutil
+import time
+from enum import Enum
+
+from ImageCopy.config import Config
+from ImageCopy.image_file import ImageFile
+
+
+class Copier:
+    class OverwriteOptions(Enum):
+        OVERWRITE = "always-overwrite"
+        WARN = "warn-before-overwrite"
+        NO_OVERWRITE = "never-overwrite"
+        APPEND_SUFFIX = "append-suffix"
+
+    def __init__(self, config: Config):
+        self.mode = Copier.OverwriteOptions.OVERWRITE
+        if config.copy is not None:
+            if (mode := config.copy["mode"]) is not None:
+                try:
+                    self.mode = Copier.OverwriteOptions(mode)
+                    # if self.mode == Copier.OverwriteOptions.APPEND_SUFFIX:
+                    #     if config.copy["suffix"] is not None:
+                    #         self.suffix = config.copy["suffix"]
+                    #     else:
+                    #         self.suffix = str(int(time.time()))
+                except ValueError:
+                    print("Provided Copy-Mode", config.copy["mode"], "is not valid. Using 'always-overwrite'")
+                    pass
+
+    def copy(self, image: ImageFile, destination: str):
+        """
+        Copy the given image to its new location.
+
+        :param image: image file to copy.
+        :param destination: destination directory with or without new filename
+        """
+        split_path = destination.split("/")
+        # Workaround to allow rename
+        if "." in split_path[-1]:
+            target_dir = "/".join(split_path[:-1])
+        else:
+            target_dir = destination
+        if not os.path.exists(target_dir):
+            os.makedirs(target_dir, exist_ok=True)
+
+        # Check Overwrite Mode, TODO: Consider polymorphic implementation (especially if implementation changes)
+        if self.mode == Copier.OverwriteOptions.WARN:
+            # WARN
+            if os.path.exists(destination):
+                print("\nOverwriting Image", destination)
+
+        if self.mode == Copier.OverwriteOptions.APPEND_SUFFIX:
+            # TODO: Add Incremental Suffixes
+            if os.path.exists(destination):
+                destination = destination[:-len(image.extension)] + "_" + str(int(time.time())) + image.extension
+        if self.mode == Copier.OverwriteOptions.NO_OVERWRITE:
+            if os.path.exists(destination):
+                return None
+
+        return shutil.copy2(str(image), destination)

ImageCopy/image_copy.py

@@ -4,7 +4,7 @@
 from ImageCopy.image_finder import ImageFinder
 from ImageCopy.action_runner import ActionRunner
 from ImageCopy.config import Config
-from ImageCopy.image_file import copy
+from ImageCopy.copier import Copier
 from multiprocessing import Process, Queue
 
 CONFIG_FILE = "config.yml"
@@ -41,22 +41,24 @@ def progress_bar(iteration, total, prefix='', suffix='', decimals=1, length=100,
     runner.execute_transformers(images)
 
     after_copy_actions = runner.get_after_action_count() > 0
+    after_copy_queue = Queue()
+    feedback_queue = Queue()
     if after_copy_actions:
-        after_copy_queue = Queue()
-        feedback_queue = Queue()
         after_copy_process = Process(target=runner.after_action_process,
                                      args=(runner.after_actions, after_copy_queue, feedback_queue))
         after_copy_process.start()
 
+
     i = 0
     print("Copying images from", config.io.input_dir, "to", config.io.output_dir)
     progress_bar(i, len(images), prefix="Progress:", suffix="Complete", length=50, end="")
+    copier = Copier(config)
     for image in images:
         i += 1
         progress_bar(i, len(images), prefix="Progress:", suffix="Complete", length=50, end="")
         try:
-            images[image] = copy(image, images[image])
-            if after_copy_actions:
+            images[image] = copier.copy(image, images[image])
+            if after_copy_actions and images[image] is not None:
                 after_copy_queue.put({image: images[image]})
         except PermissionError as per:
             print("\n", per)  # TODO: Error handling

ImageCopy/image_file.py

@@ -3,7 +3,6 @@
 """
 import os
 import platform
-import shutil
 import time
 from pathlib import Path
 
@@ -50,20 +49,3 @@ def get_creation_time(self) -> time.struct_time:
     def __str__(self) -> str:
         return str(self.path)
 
-
-def copy(image: ImageFile, destination: str):
-    """
-    Copy the given image to its new location.
-
-    :param image: image file to copy.
-    :param destination: destination directory with or without new filename
-    """
-    split_path = destination.split("/")
-    # Workaround to allow rename
-    if "." in split_path[-1]:
-        target_dir = "/".join(split_path[:-1])
-    else:
-        target_dir = destination
-    if not os.path.exists(target_dir):
-        os.makedirs(target_dir, exist_ok=True)
-    return shutil.copy2(str(image), destination)

tests/mock/mocks.py

@@ -0,0 +1,41 @@
+from ImageCopy.Actions.after_copy_action import AfterCopyAction
+from ImageCopy.Transformers.path_transform import PathTransform
+from ImageCopy.config import Config
+
+"""
+Collection of Mock-Implementations
+Refactor in multiple files if more than 4 Mock-Classes or one Mock-Class with more than 4 Methods or code lines > 50
+"""
+
+
+class MockConfig(Config):
+    def __init__(self, grouping=None, raw_separate=None, rename=None, exif=None):
+        self.grouping = grouping
+        self.raw_separate = raw_separate
+        self.rename = rename
+        self.exif = exif
+
+
+class MockTransformer(PathTransform):
+
+    def __init__(self):
+        self.transform_called = False
+        self.transform_called_with = dict()
+
+    def transform(self, input_dict: dict):
+        self.transform_called = True
+        self.transform_called_with = input_dict
+
+    def _load_config(self, config):
+        pass
+
+
+class MockAction(AfterCopyAction):
+
+    def __init__(self):
+        self.execute_called = False
+        self.execute_called_with = dict()
+
+    def execute(self, images: dict):
+        self.execute_called = True
+        self.execute_called_with = images

tests/test_Copier.py

@@ -0,0 +1,28 @@
+from ImageCopy.config import Config
+from ImageCopy.copier import Copier
+
+
+class TestCopier:
+
+    class TestConfig(Config):
+        def __init__(self, config_path):
+            try:
+                super().__init__(config_path)
+            except Exception:
+                pass
+
+    def test_constructor_config_default(self):
+        config_dict = {"copy": {"mode": "invalid"}}
+        config = TestCopier.TestConfig(None)
+        config._cfg = config_dict
+
+        copier = Copier(config)
+        assert copier.mode == Copier.OverwriteOptions.OVERWRITE
+
+    def test_constructor_config(self):
+        config_dict = {"copy": {"mode": "warn-before-overwrite"}}
+        config = TestCopier.TestConfig(None)
+        config._cfg = config_dict
+
+        copier = Copier(config)
+        assert copier.mode == Copier.OverwriteOptions.WARN

tests/test_action_runner.py

@@ -0,0 +1,91 @@
+import random
+import pytest
+
+from ImageCopy.Actions.exif_editing import ExifEditing
+from ImageCopy.Transformers.grouping_transform import GroupingTransform
+from ImageCopy.Transformers.raw_separate_transform import RawSeparateTransform
+from ImageCopy.Transformers.rename_transform import RenameTransform
+from ImageCopy.action_runner import ActionRunner
+from tests.mock.mocks import MockConfig, MockTransformer, MockAction
+
+
+class TestActionRunner:
+
+    def test_invalid_config(self):
+        with pytest.raises(ValueError):
+            runner = ActionRunner(None)
+
+    def test_tranformers_actions_not_created(self):
+        mockConfig = MockConfig()
+        runner = ActionRunner(mockConfig)
+        assert len(runner.path_transformers) == 0
+        assert len(runner.after_actions) == 0
+
+    def test_transformers_actions_created(self):
+        mockConfig = MockConfig(dict(), dict(), {"in": "", "out": ""}, dict())
+        runner = ActionRunner(mockConfig)
+        assert len(runner.path_transformers) == 3  # grouping, raw_separate, rename
+        assert any(isinstance(transform, GroupingTransform) for transform in runner.path_transformers)
+        assert any(isinstance(transform, RawSeparateTransform) for transform in runner.path_transformers)
+        assert any(isinstance(transform, RenameTransform) for transform in runner.path_transformers)
+        assert len(runner.after_actions) == 1  # exif
+        assert len(runner.after_actions) == runner.get_after_action_count()
+        assert any(isinstance(action, ExifEditing) for action in runner.after_actions)
+
+    def test_executes_all_transformers(self):
+        mockConfig = MockConfig()
+        mockTransformers = [MockTransformer(), MockTransformer(), MockTransformer()]
+        runner = ActionRunner(mockConfig)
+        input_dict = {'test': True, 'x': 42}
+        runner.path_transformers = mockTransformers
+        runner.execute_transformers(input_dict)
+        assert all(transform.transform_called for transform in mockTransformers)
+        assert all(transform.transform_called_with == input_dict for transform in mockTransformers)
+
+    def test_executes_all_transformers_randomized_amount(self):
+        mockConfig = MockConfig()
+        mockTransformers = [MockTransformer() for x in range(random.randint(1, 20))]
+        runner = ActionRunner(mockConfig)
+        input_dict = {'test': True, 'x': 42}
+        runner.path_transformers = mockTransformers
+        runner.execute_transformers(input_dict)
+        assert all(transform.transform_called for transform in mockTransformers)
+        assert all(transform.transform_called_with == input_dict for transform in mockTransformers)
+
+    def test_executes_all_actions(self):
+        mockConfig = MockConfig()
+        mockActions = [MockAction(), MockAction()]
+        runner = ActionRunner(mockConfig)
+        input_dict = {'test': True, 'x': 42}
+        runner.after_actions = mockActions
+        runner.execute_after_actions(input_dict, lambda: None)
+        assert all(action.execute_called for action in mockActions)
+        assert all(action.execute_called_with == input_dict for action in mockActions)
+
+    def test_executes_all_actions_randomized_amaount(self):
+        mockConfig = MockConfig()
+        mockActions = [MockAction() for x in range(random.randint(1, 20))]
+        runner = ActionRunner(mockConfig)
+        input_dict = {'test': True, 'x': 42}
+        runner.after_actions = mockActions
+        runner.execute_after_actions(input_dict, lambda: None)
+        assert all(action.execute_called for action in mockActions)
+        assert all(action.execute_called_with == input_dict for action in mockActions)
+
+    def test_execute_actions_register_progress(self):
+
+        class Counter:
+            def __init__(self):
+                self.progress_counter = 0
+
+            def progress(self):
+                self.progress_counter += 1
+
+        mockConfig = MockConfig()
+        mockActions = [MockAction() for x in range(random.randint(1, 20))]
+        runner = ActionRunner(mockConfig)
+        input_dict = {'test': True, 'x': 42}
+        runner.after_actions = mockActions
+        counter = Counter()
+        runner.execute_after_actions(input_dict, counter.progress)
+        assert counter.progress_counter == len(mockActions)