refactor some more to make the entire Multipass class context aware.

also add unit test

snapcraft/internal/build_providers/_multipass/_multipass.py

@@ -15,8 +15,6 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import shlex
-from contextlib import contextmanager
-from typing import Generator
 
 from .. import errors
 from .._base_provider import BaseProvider
@@ -32,38 +30,42 @@ def __init__(self, *, project, echoer) -> None:
         super().__init__(project=project, echoer=echoer,
                          executor=self._multipass_cmd.execute)
 
-    @contextmanager
-    def new_instance(self, keep=True) -> Generator:
-        """Create the multipass instance."""
-        try:
-            self.launch_instance(self._multipass_cmd.launch, image='16.04')
-            self.setup_snapcraft()
-            yield self
-        finally:
-            try:
-                self._try_stop_and_delete(keep=keep)
-            except errors.ProviderStopError as stop_error:
-                self.echoer.warning('Could not stop {!r}: {}.'.format(
-                    self.instance_name, stop_error))
-            except errors.ProviderDeleteError as delete_error:
-                self.echoer.warning('Could not stop {!r}: {}.'.format(
-                    self.instance_name, delete_error))
-
-    def _try_stop_and_delete(self, *, keep: bool) -> None:
+    def __enter__(self):
+        self.create()
+        return self
+
+    def __exit__(self, exc_type, exc_val, exc_tb):
+        self.destroy()
+
+    def create(self):
+        """Create the multipass instance and setup the build environment."""
+        self.launch_instance(self._multipass_cmd.launch, image='16.04')
+        self.setup_snapcraft()
+
+    def destroy(self):
+        """Destroy the instance, trying to stop it first."""
         try:
             instance_info = self._get_instance_info()
         except errors.ProviderInfoError as info_error:
-            # An error here means this instance may not exist
+            self.echoer.warning(
+                'Failed to obtain the status of {!r} when trying to '
+                'delete: {}'.format(self.instance_name, info_error))
             return
 
-        if instance_info.is_stopped():
-            return
-        self._multipass_cmd.stop(instance_name=self.instance_name)
-        if not keep:
+        try:
+            if not instance_info.is_stopped():
+                self._multipass_cmd.stop(instance_name=self.instance_name)
             self._multipass_cmd.delete(instance_name=self.instance_name)
+        except errors.ProviderStopError as stop_error:
+            self.echoer.warning('Could not stop {!r}: {}.'.format(
+                self.instance_name, stop_error))
+        except errors.ProviderDeleteError as stop_error:
+            self.echoer.warning('Could not stop {!r}: {}.'.format(
+                self.instance_name, stop_error))
 
     def provision_project(self, tarball: str) -> None:
         """Provision the multipass instance with the project to work with."""
+        # TODO add instance check.
         # Step 0, sanitize the input
         tarball = shlex.quote(tarball)
 
@@ -81,13 +83,15 @@ def provision_project(self, tarball: str) -> None:
                                     instance_name=self.instance_name)
 
     def build_project(self) -> None:
+        # TODO add instance check.
         # Use the full path as /snap/bin is not in PATH.
         snapcraft_cmd = 'cd {}; /snap/bin/snapcraft snap --output {}'.format(
             self.project_dir, self.snap_filename)
         self._multipass_cmd.execute(command=['sh', '-c', snapcraft_cmd],
                                     instance_name=self.instance_name)
 
     def retrieve_snap(self) -> str:
+        # TODO add instance check.
         source = '{}:{}/{}'.format(self.instance_name,
                                    self.project_dir,
                                    self.snap_filename)

snapcraft/internal/lifecycle/_containers.py

@@ -54,8 +54,7 @@ def cleanbuild(*, project, echoer, build_environment, remote='') -> None:
         return
 
     build_provider_class = build_providers.get_provider_for('multipass')
-    build_provider = build_provider_class(project=project, echoer=echoer)
-    with build_provider.new_instance(keep=False) as instance:
+    with build_provider_class(project=project, echoer=echoer) as instance:
         instance.provision_project(tar_filename)
         instance.build_project()
         instance.retrieve_snap()

tests/unit/build_providers/__init__.py

@@ -0,0 +1,39 @@
+# -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*-
+#
+# Copyright (C) 2018 Canonical Ltd
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 3 as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+from unittest import mock
+
+from snapcraft.project import Project, ProjectInfo
+
+from tests import unit
+
+
+class BaseProviderBaseTest(unit.TestCase):
+
+    def setUp(self):
+        super().setUp()
+
+        self.instance_name = 'ridicoulus-hours'
+        patcher = mock.patch('petname.Generate',
+                             return_value=self.instance_name)
+        patcher.start()
+        self.addCleanup(patcher.stop)
+
+        self.project = Project()
+        self.project.info = ProjectInfo(dict(name='project-name'))
+
+        self.echoer_mock = mock.Mock()
+        self.executor_mock = mock.Mock()

tests/unit/build_providers/multipass/test_multipass.py

@@ -0,0 +1,191 @@
+# -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*-
+#
+# Copyright (C) 2018 Canonical Ltd
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 3 as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+from textwrap import dedent
+from unittest import mock
+
+from tests.unit.build_providers import BaseProviderBaseTest
+from snapcraft.internal.build_providers import errors
+from snapcraft.internal.build_providers._multipass import (
+    Multipass, MultipassCommand)
+
+
+_DEFAULT_INSTANCE_INFO = dedent("""\
+    {
+        "errors": [
+        ],
+        "info": {
+            "ridicoulus-hours": {
+                "disks": {
+                    "sda1": {
+                        "total": "5136297984",
+                        "used": "1076072448"
+                    }
+                },
+                "image_hash": "bc990872f070249450fc187d668bf65e2f87ffa3d7cf6e2934cb84b62af368e1",
+                "image_release": "16.04 LTS",
+                "ipv4": [
+                    "10.74.70.141"
+                ],
+                "load": [
+                    0,
+                    0,
+                    0
+                ],
+                "memory": {
+                    "total": 1040318464,
+                    "used": 42754048
+                },
+                "mounts": {
+                },
+                "release": "Ubuntu 16.04.4 LTS",
+                "state": "RUNNING"
+            }
+        }
+    }
+    """)  # noqa: E501
+
+
+class MultipassTest(BaseProviderBaseTest):
+
+    def setUp(self):
+        super().setUp()
+
+        patcher = mock.patch(
+            'snapcraft.internal.build_providers._multipass.'
+            '_multipass.MultipassCommand',
+            spec=MultipassCommand)
+        self.multipass_cmd_mock = patcher.start()
+        self.addCleanup(patcher.stop)
+
+        # default data returned for info
+        self.multipass_cmd_mock().info.return_value = \
+            _DEFAULT_INSTANCE_INFO.encode()
+
+    def test_instance_with_contextmanager(self):
+        with Multipass(project=self.project,
+                       echoer=self.echoer_mock) as instance:
+            instance.provision_project('source.tar')
+            instance.build_project()
+            instance.retrieve_snap()
+
+        self.multipass_cmd_mock().launch.assert_called_once_with(
+            image='16.04', instance_name=self.instance_name)
+        self.multipass_cmd_mock().execute.assert_has_calls([
+            mock.call(
+                instance_name=self.instance_name,
+                command=['sudo', 'snap', 'install', 'snapcraft',
+                         '--classic']),
+            mock.call(
+                instance_name=self.instance_name,
+                command=['mkdir', 'project-name']),
+            mock.call(
+                instance_name=self.instance_name,
+                command=['tar', '-xvf', 'source.tar', '-C', 'project-name']),
+            mock.call(
+                instance_name=self.instance_name,
+                command=['sh', '-c', 'cd project-name; /snap/bin/snapcraft '
+                         'snap --output project-name_{}.snap'.format(
+                             self.project.deb_arch)]),
+        ])
+        self.multipass_cmd_mock().info.assert_called_once_with(
+            instance_name=self.instance_name, output_format='json')
+
+        self.multipass_cmd_mock().copy_files.assert_has_calls([
+            mock.call(destination='{}:source.tar'.format(self.instance_name),
+                      source='source.tar'),
+            mock.call(destination='project-name_{}.snap'.format(
+                        self.project.deb_arch),
+                      source='{}:project-name/project-name_{}.snap'.format(
+                        self.instance_name, self.project.deb_arch)),
+        ])
+        self.multipass_cmd_mock().stop.assert_called_once_with(
+            instance_name=self.instance_name)
+        self.multipass_cmd_mock().delete.assert_called_once_with(
+            instance_name=self.instance_name)
+
+    def test_provision_project(self):
+        multipass = Multipass(project=self.project, echoer=self.echoer_mock)
+
+        # In the real world, MultipassCommand would return an error when
+        # calling this on an instance that does not exist.
+        multipass.provision_project('source.tar')
+
+        self.multipass_cmd_mock().execute.assert_has_calls([
+            mock.call(
+                instance_name=self.instance_name,
+                command=['mkdir', 'project-name']),
+            mock.call(
+                instance_name=self.instance_name,
+                command=['tar', '-xvf', 'source.tar', '-C', 'project-name']),
+            ])
+        self.multipass_cmd_mock().copy_files.assert_called_once_with(
+            destination='{}:source.tar'.format(self.instance_name),
+            source='source.tar')
+
+        self.multipass_cmd_mock().launch.assert_not_called()
+        self.multipass_cmd_mock().info.assert_not_called()
+        self.multipass_cmd_mock().stop.assert_not_called()
+        self.multipass_cmd_mock().delete.assert_not_called()
+
+    def test_build_project(self):
+        multipass = Multipass(project=self.project, echoer=self.echoer_mock)
+
+        # In the real world, MultipassCommand would return an error when
+        # calling this on an instance that does not exist.
+        multipass.build_project()
+
+        self.multipass_cmd_mock().execute.assert_called_once_with(
+            instance_name=self.instance_name,
+            command=['sh', '-c', 'cd project-name; /snap/bin/snapcraft '
+                     'snap --output project-name_{}.snap'.format(
+                        self.project.deb_arch)])
+
+        self.multipass_cmd_mock().copy_files.assert_not_called()
+        self.multipass_cmd_mock().launch.assert_not_called()
+        self.multipass_cmd_mock().info.assert_not_called()
+        self.multipass_cmd_mock().stop.assert_not_called()
+        self.multipass_cmd_mock().delete.assert_not_called()
+
+    def test_retrieve_snap(self):
+        multipass = Multipass(project=self.project, echoer=self.echoer_mock)
+
+        # In the real world, MultipassCommand would return an error when
+        # calling this on an instance that does not exist.
+        multipass.retrieve_snap()
+
+        self.multipass_cmd_mock().copy_files.assert_called_once_with(
+            destination='project-name_{}.snap'.format(self.project.deb_arch),
+            source='{}:project-name/project-name_{}.snap'.format(
+                self.instance_name, self.project.deb_arch))
+
+        self.multipass_cmd_mock().execute.assert_not_called()
+        self.multipass_cmd_mock().launch.assert_not_called()
+        self.multipass_cmd_mock().info.assert_not_called()
+        self.multipass_cmd_mock().stop.assert_not_called()
+        self.multipass_cmd_mock().delete.assert_not_called()
+
+    def test_instance_does_not_exist_on_destroy(self):
+        # An error is raised if the queried image does not exist
+        self.multipass_cmd_mock().info.side_effect = errors.ProviderInfoError(
+            provider_name=self.instance_name)
+
+        multipass = Multipass(project=self.project, echoer=self.echoer_mock)
+
+        multipass.destroy()
+
+        self.multipass_cmd_mock().stop.assert_not_called()
+        self.multipass_cmd_mock().delete.assert_not_called()

tests/unit/build_providers/test_base_provider.py

@@ -18,27 +18,11 @@
 
 from testtools.matchers import Equals
 
-from snapcraft.project import Project, ProjectInfo
+from . import BaseProviderBaseTest
 from snapcraft.internal.build_providers._base_provider import BaseProvider
-from tests import unit
 
 
-class BaseProviderTest(unit.TestCase):
-
-    def setUp(self):
-        super().setUp()
-
-        self.instance_name = 'ridicoulus-hours'
-        patcher = mock.patch('petname.Generate',
-                             return_value=self.instance_name)
-        patcher.start()
-        self.addCleanup(patcher.stop)
-
-        self.project = Project()
-        self.project.info = ProjectInfo(dict(name='project-name'))
-
-        self.echoer_mock = mock.Mock()
-        self.executor_mock = mock.Mock()
+class BaseProviderTest(BaseProviderBaseTest):
 
     def test_initialize(self):
         base_provider = BaseProvider(project=self.project,