Add strong typing for the following API methods

* system/root_bind.py
* system/root_init.py
* system/identifier.py

This references issue #1644

kiwi/system/identifier.py

@@ -18,6 +18,9 @@
 import random
 import struct
 
+# project
+from kiwi.storage.device_provider import DeviceProvider
+
 
 class SystemIdentifier:
     """
@@ -30,7 +33,7 @@ class SystemIdentifier:
     def __init__(self):
         self.image_id = None
 
-    def get_id(self):
+    def get_id(self) -> str:
         """
         Current hex identifier
 
@@ -40,7 +43,7 @@ class SystemIdentifier:
         """
         return self.image_id
 
-    def calculate_id(self):
+    def calculate_id(self) -> None:
         """
         Calculate random hex id
 
@@ -50,7 +53,7 @@ class SystemIdentifier:
             self._rand(), self._rand(), self._rand(), self._rand()
         )
 
-    def write(self, filename):
+    def write(self, filename: str) -> None:
         """
         Write current hex identifier to file
 
@@ -59,7 +62,7 @@ class SystemIdentifier:
         with open(filename, 'w') as identifier:
             identifier.write('%s\n' % self.image_id)
 
-    def write_to_disk(self, device_provider):
+    def write_to_disk(self, device_provider: DeviceProvider) -> None:
         """
         Write current hex identifier to MBR at offset 0x1b8 on disk
 

kiwi/system/root_bind.py

@@ -18,7 +18,8 @@
 import os
 import logging
 import shutil
-from textwrap import dedent
+import textwrap
+from typing import List
 
 # project
 from kiwi.command import Command
@@ -26,6 +27,7 @@ from kiwi.defaults import Defaults
 from kiwi.path import Path
 from kiwi.mount_manager import MountManager
 from kiwi.utils.checksum import Checksum
+from kiwi.system.root_init import RootInit
 
 from kiwi.exceptions import (
     KiwiMountKernelFileSystemsError,
@@ -50,11 +52,11 @@ class RootBind:
     :param str shared_location: shared directory between image root and
         build system root
     """
-    def __init__(self, root_init):
+    def __init__(self, root_init: RootInit):
         self.root_dir = root_init.root_dir
-        self.cleanup_files = []
-        self.mount_stack = []
-        self.dir_stack = []
+        self.cleanup_files: List[str] = []
+        self.mount_stack: List[MountManager] = []
+        self.dir_stack: List[str] = []
         # need resolv.conf/hosts for chroot name resolution
         # need /etc/sysconfig/proxy for chroot proxy usage
         self.config_files = [
@@ -72,7 +74,7 @@ class RootBind:
         # share the following directory with the host
         self.shared_location = '/' + Defaults.get_shared_cache_location()
 
-    def mount_kernel_file_systems(self):
+    def mount_kernel_file_systems(self) -> None:
         """
         Bind mount kernel filesystems
 
@@ -98,7 +100,7 @@ class RootBind:
                 '%s: %s' % (type(e).__name__, format(e))
             )
 
-    def umount_kernel_file_systems(self):
+    def umount_kernel_file_systems(self) -> None:
         """
         Umount kernel filesystems
 
@@ -111,7 +113,7 @@ class RootBind:
         ]
         self._cleanup_mounts(umounts)
 
-    def mount_shared_directory(self, host_dir=None):
+    def mount_shared_directory(self, host_dir: str = None) -> None:
         """
         Bind mount shared location
 
@@ -126,7 +128,7 @@ class RootBind:
 
         :raises KiwiMountSharedDirectoryError: if mount fails
         """
-        if not host_dir:
+        if host_dir is None:
             host_dir = self.shared_location
         try:
             Path.create(self.root_dir + host_dir)
@@ -143,7 +145,7 @@ class RootBind:
                 '%s: %s' % (type(e).__name__, format(e))
             )
 
-    def setup_intermediate_config(self):
+    def setup_intermediate_config(self) -> None:
         """
         Create intermediate config files
 
@@ -176,7 +178,7 @@ class RootBind:
                 '%s: %s' % (type(e).__name__, format(e))
             )
 
-    def cleanup(self):
+    def cleanup(self) -> None:
         """
         Cleanup mounted locations, directories and intermediate config files
         """
@@ -197,7 +199,7 @@ class RootBind:
 
             checksum = Checksum(config_file)
             if not checksum.matches(checksum.sha256(), shasum_file):
-                message = dedent('''\n
+                message = textwrap.dedent('''\n
                     Modifications to intermediate config file detected
 
                     The file: {0}

kiwi/system/root_init.py

@@ -41,20 +41,20 @@ class RootInit:
 
     :param str root_dir: root directory path name
     """
-    def __init__(self, root_dir, allow_existing=False):
+    def __init__(self, root_dir: str, allow_existing: bool = False):
         if not allow_existing and os.path.exists(root_dir):
             raise KiwiRootDirExists(
                 'Root directory %s already exists' % root_dir
             )
         self.root_dir = root_dir
 
-    def delete(self):
+    def delete(self) -> None:
         """
         Force delete root directory and its contents
         """
         Path.wipe(self.root_dir)
 
-    def create(self):
+    def create(self) -> None:
         """
         Create new system root directory
 

test/unit/system/identifier_test.py

@@ -10,12 +10,12 @@ class TestSystemIdentifier:
         self.identifier = SystemIdentifier()
 
     def test_get_id(self):
-        pass
+        assert self.identifier.get_id() is None
 
     @patch('random.randrange')
     def test_calculate_id(self, mock_rand):
         mock_rand.return_value = 15
-        self.identifier.calculate_id()
+        assert self.identifier.calculate_id() is None
         assert self.identifier.get_id() == '0x0f0f0f0f'
 
     def test_write(self):
@@ -23,7 +23,17 @@ class TestSystemIdentifier:
 
         m_open = mock_open()
         with patch('builtins.open', m_open, create=True):
-            self.identifier.write('mbrid-file')
+            assert self.identifier.write('mbrid-file') is None
 
         m_open.assert_called_once_with('mbrid-file', 'w')
         m_open.return_value.write.assert_called_once_with('some-id\n')
+
+    @patch('kiwi.storage.device_provider.DeviceProvider')
+    def test_write_to_disk(self, mock_device_provider):
+        self.identifier.image_id = '1'
+        mock_device_provider.get_device.return_value('device')
+
+        m_open = mock_open()
+        with patch('builtins.open', m_open, create=True):
+            assert self.identifier.write_to_disk(mock_device_provider) \
+                is None

test/unit/system/root_bind_test.py

@@ -88,7 +88,7 @@ class TestRootBind:
         mock_exists.return_value = True
         shared_mount = Mock()
         mock_mount.return_value = shared_mount
-        self.bind_root.mount_kernel_file_systems()
+        assert self.bind_root.mount_kernel_file_systems() is None
         mock_mount.assert_called_once_with(
             device='/proc', mountpoint='root-dir/proc'
         )
@@ -98,7 +98,7 @@ class TestRootBind:
     def test_umount_kernel_file_systems(self, mock_mount):
         self.mount_manager.device = '/proc'
         self.mount_manager.is_mounted = Mock(return_value=True)
-        self.bind_root.umount_kernel_file_systems()
+        assert self.bind_root.umount_kernel_file_systems() is None
         self.mount_manager.umount_lazy.assert_called_once_with()
         assert self.bind_root.mount_stack == []
 
@@ -107,7 +107,7 @@ class TestRootBind:
         self.mount_manager.device = '/proc'
         self.mount_manager.is_mounted = Mock(return_value=True)
         self.mount_manager.umount_lazy = Mock(side_effect=Exception)
-        self.bind_root.umount_kernel_file_systems()
+        assert self.bind_root.umount_kernel_file_systems() is None
         self.mount_manager.umount_lazy.assert_called_once_with()
         assert self.bind_root.mount_stack == [self.mount_manager]
 
@@ -116,7 +116,7 @@ class TestRootBind:
     def test_mount_shared_directory(self, mock_path, mock_mount):
         shared_mount = Mock()
         mock_mount.return_value = shared_mount
-        self.bind_root.mount_shared_directory()
+        assert self.bind_root.mount_shared_directory() is None
         mock_path.call_args_list = [
             call('root-dir/var/cache/kiwi'),
             call('/var/cache/kiwi')
@@ -137,7 +137,7 @@ class TestRootBind:
         mock_exists.return_value = True
 
         with patch('builtins.open') as m_open:
-            self.bind_root.setup_intermediate_config()
+            assert self.bind_root.setup_intermediate_config() is None
             m_open.assert_called_once_with(
                 'root-dir/etc/sysconfig/proxy.sha', 'w'
             )
@@ -152,6 +152,7 @@ class TestRootBind:
         ]
         checksum.sha256.assert_called_once_with()
 
+    @patch('textwrap.dedent')
     @patch('kiwi.system.root_bind.Checksum')
     @patch('kiwi.system.root_bind.MountManager.is_mounted')
     @patch('kiwi.system.root_bind.Command.run')
@@ -162,7 +163,7 @@ class TestRootBind:
     def test_cleanup(
         self, mock_move, mock_exists, mock_islink,
         mock_remove_hierarchy, mock_command, mock_is_mounted,
-        mock_Checksum
+        mock_Checksum, mock_dedent
     ):
         checksum = Mock()
         checksum.matches.return_value = False
@@ -176,7 +177,7 @@ class TestRootBind:
         mock_exists.side_effect = exists_side_effect
         mock_islink.return_value = True
         with self._caplog.at_level(logging.WARNING):
-            self.bind_root.cleanup()
+            assert self.bind_root.cleanup() is None
             self.mount_manager.umount_lazy.assert_called_once_with()
             mock_remove_hierarchy.assert_called_once_with(
                 root='root-dir', path='/mountpoint'
@@ -232,5 +233,5 @@ class TestRootBind:
         self.mount_manager.is_mounted.return_value = False
         self.mount_manager.mountpoint = '/mountpoint'
         with self._caplog.at_level(logging.WARNING):
-            self.bind_root.cleanup()
+            assert self.bind_root.cleanup() is None
             assert 'Path /mountpoint not a mountpoint' in self._caplog.text

test/unit/system/root_init_test.py

@@ -73,7 +73,7 @@ class TestRootInit:
         mock_path.return_value = False
         mock_temp.return_value = 'tmpdir'
         root = RootInit('root_dir', True)
-        root.create()
+        assert root.create() is None
         assert mock_makedirs.call_args_list == [
             call('tmpdir/var/cache/kiwi'),
             call('tmpdir/dev/pts'),
@@ -118,7 +118,7 @@ class TestRootInit:
     def test_delete(self, mock_path, mock_wipe):
         mock_path.return_value = False
         root = RootInit('root_dir')
-        root.delete()
+        assert root.delete() is None
         mock_wipe.assert_called_once_with('root_dir')
 
     def teardown(self):