launchpad-reviewers team mailing list archive

[Jürgen Gmach <mp+412255@xxxxxxxxxxxxxxxxxx>]

Jürgen Gmach has proposed merging ~jugmac00/lpcraft:increase-test-coverage into lpcraft:main.

Commit message:
WIP: increase test coverage for lpcraft

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jugmac00/lpcraft/+git/lpcraft/+merge/412255
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~jugmac00/lpcraft:increase-test-coverage into lpcraft:main.

diff --git a/lpcraft/providers/_buildd.py b/lpcraft/providers/_buildd.py
index 83ad5d6..9acbc3a 100644
--- a/lpcraft/providers/_buildd.py
+++ b/lpcraft/providers/_buildd.py
@@ -81,7 +81,7 @@ class LPCraftBuilddBaseConfiguration(bases.BuilddBase):
 
     def __eq__(self, other: Any) -> bool:
         if not isinstance(other, LPCraftBuilddBaseConfiguration):
-            return NotImplemented
+            raise TypeError
         return (
             self.alias == other.alias
             and self.environment == other.environment
diff --git a/lpcraft/providers/tests/test_buildd.py b/lpcraft/providers/tests/test_buildd.py
index 95410b0..03898c8 100644
--- a/lpcraft/providers/tests/test_buildd.py
+++ b/lpcraft/providers/tests/test_buildd.py
@@ -3,8 +3,10 @@
 
 from unittest.mock import Mock, patch
 
+import pytest
 from craft_providers import Executor, bases
 from craft_providers.actions import snap_installer
+from craft_providers.bases.buildd import BuilddBaseAlias
 
 from lpcraft.providers._buildd import (
     SERIES_TO_BUILDD_IMAGE_ALIAS,
@@ -45,3 +47,10 @@ class TestLPCraftBuilddBaseConfiguration(ProviderBaseTestCase):
             config.setup(executor=mock_instance)
 
         self.assertIsNotNone(raised.exception.__cause__)
+
+    def test_compare_configuration_with_other_type(self):
+        """The configuration should only be comparable to its own type"""
+        with pytest.raises(TypeError):
+            "foo" == LPCraftBuilddBaseConfiguration(
+                alias=BuilddBaseAlias.FOCAL,
+            )
diff --git a/lpcraft/providers/tests/test_lxd.py b/lpcraft/providers/tests/test_lxd.py
index b1fd347..6c7f147 100644
--- a/lpcraft/providers/tests/test_lxd.py
+++ b/lpcraft/providers/tests/test_lxd.py
@@ -343,7 +343,7 @@ class TestLXDProvider(ProviderBaseTestCase):
                 series="focal",
                 architecture="amd64",
             ):
-                pass
+                pass  # pragma: no cover
 
         self.assertIs(error, raised.exception.__cause__)
 
@@ -353,7 +353,6 @@ class TestLXDProvider(ProviderBaseTestCase):
             "craft_providers.lxd.launch", autospec=True, side_effect=error
         )
         provider = self.makeLXDProvider(lxd_launcher=mock_launcher)
-
         with self.assertRaisesRegex(CommandError, r"Boom") as raised:
             with provider.launched_environment(
                 project_name="my-project",
@@ -361,14 +360,13 @@ class TestLXDProvider(ProviderBaseTestCase):
                 series="focal",
                 architecture="amd64",
             ):
-                pass
+                pass  # pragma: no cover
 
         self.assertIs(error, raised.exception.__cause__)
 
     def test_launched_environment_unmounts_and_stops_after_error(self):
         mock_launcher = Mock(spec=launch)
         provider = self.makeLXDProvider(lxd_launcher=mock_launcher)
-
         with self.assertRaisesRegex(RuntimeError, r"Boom"):
             with provider.launched_environment(
                 project_name="my-project",
diff --git a/lpcraft/providers/tests/test_replay_logs.py b/lpcraft/providers/tests/test_replay_logs.py
new file mode 100644
index 0000000..2ddfd63
--- /dev/null
+++ b/lpcraft/providers/tests/test_replay_logs.py
@@ -0,0 +1,47 @@
+from pathlib import Path
+from shutil import copyfile
+from unittest.mock import Mock, call, patch
+
+from craft_providers import Executor
+from fixtures import TempDir
+
+from lpcraft.commands.tests import CommandBaseTestCase
+from lpcraft.providers import replay_logs
+
+
+class TestReplayLogs(CommandBaseTestCase):
+    @patch("lpcraft.providers.emit")
+    @patch("lpcraft.providers.get_managed_environment_log_path")
+    def test_cannot_pull_file(self, mock_get_path, mock_emit):
+        mock_instance = Mock(spec=Executor)
+        mock_instance.pull_file.side_effect = FileNotFoundError()
+        mock_emit._initiated = True
+
+        replay_logs(mock_instance)
+
+        mock_emit.trace.assert_called_with("No logs found in instance.")
+
+    @patch("lpcraft.providers.emit")
+    @patch("lpcraft.providers.get_managed_environment_log_path")
+    def test_replay_logs(self, mock_get_remote_path, mock_emit):
+        def fake_pull_file(source, destination):
+            copyfile(source, destination)
+
+        mock_instance = Mock(spec=Executor)
+        mock_instance.pull_file = fake_pull_file
+
+        self.tempdir = Path(self.useFixture(TempDir()).path)
+        path = self.tempdir / "stub_remote_log_file"
+        path.write_text("line1\nline2\nline3")
+        mock_get_remote_path.return_value = str(path)
+
+        mock_emit._initiated = True
+
+        replay_logs(mock_instance)
+
+        mock_emit.trace.call_args_list == [
+            call("Logs captured from managed instance:"),
+            call(":: line1"),
+            call(":: line2"),
+            call(":: line3"),
+        ]
diff --git a/lpcraft/tests/test_error.py b/lpcraft/tests/test_error.py
new file mode 100644
index 0000000..4c7f303
--- /dev/null
+++ b/lpcraft/tests/test_error.py
@@ -0,0 +1,14 @@
+# Copyright 2021 Canonical Ltd.  This software is licensed under the
+# GNU General Public License version 3 (see the file LICENSE).
+
+from unittest import TestCase
+
+from lpcraft.errors import CommandError
+
+
+class TestError(TestCase):
+    def test_compare_command_error_with_other_type(self):
+        """If the other type is not a CommandError, defer eq to other type."""
+        self.assertEqual(
+            NotImplemented, CommandError("message").__eq__("message")
+        )
diff --git a/lpcraft/tests/test_main.py b/lpcraft/tests/test_main.py
index dfcb817..41e2a6a 100644
--- a/lpcraft/tests/test_main.py
+++ b/lpcraft/tests/test_main.py
@@ -2,8 +2,9 @@
 # GNU General Public License version 3 (see the file LICENSE).
 
 import sys
+from unittest.mock import call, patch
 
-from craft_cli import EmitterMode
+from craft_cli import CraftError, EmitterMode
 from fixtures import MockPatch
 from testtools import TestCase
 
@@ -42,3 +43,35 @@ class TestMain(TestCase):
             sys.stderr,
         )
         mock_emit.ended_ok.assert_called_once_with()
+
+    @patch("lpcraft.main.emit")
+    @patch("lpcraft.main.run")
+    def test_keyboard_interrupt(self, mock_run, mock_emit):
+        self.useFixture(MockPatch("sys.argv", ["lpcraft"]))
+        mock_run.side_effect = KeyboardInterrupt()
+
+        ret = main()
+
+        self.assertEqual(1, ret)
+        # cannot compare the results of `call` directly,
+        # but the str representation
+        self.assertEqual(
+            str(call(CraftError("Interrupted."))),
+            str(mock_emit.error.call_args_list[0]),
+        )
+
+    @patch("lpcraft.main.emit")
+    @patch("lpcraft.main.run")
+    def test_handling_unexpected_exception(self, mock_run, mock_emit):
+        self.useFixture(MockPatch("sys.argv", ["lpcraft"]))
+        mock_run.side_effect = RuntimeError()
+
+        ret = main()
+
+        self.assertEqual(1, ret)
+        # cannot compare the results of `call` directly,
+        # but the str representation
+        self.assertEqual(
+            str(call(CraftError("lpcraft internal error: RuntimeError()"))),
+            str(mock_emit.error.call_args_list[0]),
+        )
diff --git a/tox.ini b/tox.ini
index 4bba8cc..24731ca 100644
--- a/tox.ini
+++ b/tox.ini
@@ -32,6 +32,7 @@ basepython =
     python3.8
 deps =
     -r requirements.txt
+    .[test]
     mypy
     types-PyYAML
 commands =