launchpad-reviewers team mailing list archive

[Colin Watson <mp+413708@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/lpcraft:show-job-output into lpcraft:main.

Commit message:
Show job output by default

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/lpcraft/+git/lpcraft/+merge/413708

While it's fine to hide things like container setup output in normal operation, the actual job output is important enough that we should really show it by default.  `lpcraft --quiet` still hides it.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/lpcraft:show-job-output into lpcraft:main.

diff --git a/lpcraft/commands/run.py b/lpcraft/commands/run.py
index 1805b05..2bbf75f 100644
--- a/lpcraft/commands/run.py
+++ b/lpcraft/commands/run.py
@@ -10,7 +10,7 @@ from argparse import Namespace
 from pathlib import Path, PurePath
 from typing import List, Optional, Set
 
-from craft_cli import emit
+from craft_cli import EmitterMode, emit
 from craft_providers import Executor
 from craft_providers.actions.snap_installer import install_from_store
 from dotenv import dotenv_values
@@ -233,6 +233,9 @@ def _run_job(
                 )
         full_run_cmd = ["bash", "--noprofile", "--norc", "-ec", run_command]
         emit.progress("Running the job")
+        original_mode = emit.get_mode()
+        if original_mode == EmitterMode.NORMAL:
+            emit.set_mode(EmitterMode.VERBOSE)
         with emit.open_stream(f"Running {full_run_cmd}") as stream:
             proc = instance.execute_run(
                 full_run_cmd,
@@ -241,6 +244,8 @@ def _run_job(
                 stdout=stream,
                 stderr=stream,
             )
+        if original_mode == EmitterMode.NORMAL:
+            emit.set_mode(original_mode)
         if proc.returncode != 0:
             raise CommandError(
                 f"Job {job_name!r} for "
diff --git a/lpcraft/commands/tests/test_run.py b/lpcraft/commands/tests/test_run.py
index 3872f20..07cc226 100644
--- a/lpcraft/commands/tests/test_run.py
+++ b/lpcraft/commands/tests/test_run.py
@@ -1,8 +1,10 @@
 # Copyright 2021 Canonical Ltd.  This software is licensed under the
 # GNU General Public License version 3 (see the file LICENSE).
 
+import io
 import json
 import os
+import re
 import subprocess
 from pathlib import Path
 from textwrap import dedent
@@ -1020,6 +1022,89 @@ class TestRun(RunBaseTestCase):
             execute_run.call_args_list,
         )
 
+    @patch("lpcraft.commands.run.get_provider")
+    @patch("lpcraft.commands.run.get_host_architecture", return_value="amd64")
+    @patch("sys.stderr", new_callable=io.StringIO)
+    def test_quiet(
+        self, mock_stderr, mock_get_host_architecture, mock_get_provider
+    ):
+        def execute_run(
+            command: List[str], **kwargs: Any
+        ) -> "subprocess.CompletedProcess[AnyStr]":
+            os.write(kwargs["stdout"], b"test\n")
+            return subprocess.CompletedProcess([], 0)
+
+        launcher = Mock(spec=launch)
+        provider = self.makeLXDProvider(lxd_launcher=launcher)
+        mock_get_provider.return_value = provider
+        launcher.return_value.execute_run.side_effect = execute_run
+        config = dedent(
+            """
+            pipeline:
+                - test
+
+            jobs:
+                test:
+                    series: focal
+                    architectures: amd64
+                    run: echo test
+            """
+        )
+        Path(".launchpad.yaml").write_text(config)
+
+        result = self.run_command("-q", "run")
+        self.assertEqual(0, result.exit_code)
+        self.assertEqual("", mock_stderr.getvalue())
+
+    @patch("lpcraft.commands.run.get_provider")
+    @patch("lpcraft.commands.run.get_host_architecture", return_value="amd64")
+    @patch("sys.stderr", new_callable=io.StringIO)
+    def test_normal(
+        self, mock_stderr, mock_get_host_architecture, mock_get_provider
+    ):
+        def execute_run(
+            command: List[str], **kwargs: Any
+        ) -> "subprocess.CompletedProcess[AnyStr]":
+            os.write(kwargs["stdout"], b"test\n")
+            return subprocess.CompletedProcess([], 0)
+
+        launcher = Mock(spec=launch)
+        provider = self.makeLXDProvider(lxd_launcher=launcher)
+        mock_get_provider.return_value = provider
+        launcher.return_value.execute_run.side_effect = execute_run
+        config = dedent(
+            """
+            pipeline:
+                - test
+
+            jobs:
+                test:
+                    series: focal
+                    architectures: amd64
+                    run: echo test
+            """
+        )
+        Path(".launchpad.yaml").write_text(config)
+
+        result = self.run_command("run")
+        self.assertEqual(0, result.exit_code)
+        stderr_lines = [
+            re.sub(
+                r"^(?P<date>.+?) (?P<time>.+?) (?P<text>.*?) *$",
+                r"\g<text>",
+                line,
+            )
+            for line in mock_stderr.getvalue().splitlines()
+        ]
+        self.assertEqual(
+            [
+                "Running ['bash', '--noprofile', '--norc', '-ec', "
+                "'echo test']",
+                ":: test",
+            ],
+            stderr_lines[-2:],
+        )
+
 
 class TestRunOne(RunBaseTestCase):
     def test_missing_config_file(self):