launchpad-reviewers team mailing list archive

[Colin Watson <mp+428100@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/lpcraft:fix-license into lpcraft:main.

Commit message:
Fix edge case in license handling

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

In the case where a job has an `output` key but no `properties` key under that (e.g. just `output.paths`), lpcraft raised an `AssertionError` on `assert isinstance(job.output.properties, dict)`.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/lpcraft:fix-license into lpcraft:main.

diff --git a/NEWS.rst b/NEWS.rst
index 75f4484..8f188bc 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -8,6 +8,9 @@ Version history
 - Add input properties, allowing jobs to use artifacts built by previous
   pipeline stages.
 
+- Fix handling of ``license`` in the case where a job has an ``output`` key
+  but no ``properties`` key under that.
+
 0.0.24 (2022-08-05)
 ===================
 
diff --git a/lpcraft/commands/run.py b/lpcraft/commands/run.py
index 99babf1..dd10666 100644
--- a/lpcraft/commands/run.py
+++ b/lpcraft/commands/run.py
@@ -509,6 +509,7 @@ def _run_job(
         if config.license:
             if not job.output:
                 job.output = Output()
+            if job.output.properties is None:
                 job.output.properties = dict()
             values = config.license.dict()
             # workaround necessary to please mypy
diff --git a/lpcraft/commands/tests/test_run.py b/lpcraft/commands/tests/test_run.py
index 2d71247..1f8445e 100644
--- a/lpcraft/commands/tests/test_run.py
+++ b/lpcraft/commands/tests/test_run.py
@@ -2608,6 +2608,52 @@ class TestRun(RunBaseTestCase):
     @patch("lpcraft.env.get_managed_environment_project_path")
     @patch("lpcraft.commands.run.get_provider")
     @patch("lpcraft.commands.run.get_host_architecture", return_value="amd64")
+    def test_license_field_works_with_output_but_no_properties(
+        self,
+        mock_get_host_architecture,
+        mock_get_provider,
+        mock_get_project_path,
+    ):
+        target_path = Path(self.useFixture(TempDir()).path)
+        launcher = Mock(spec=launch)
+        provider = makeLXDProvider(lxd_launcher=launcher)
+        mock_get_provider.return_value = provider
+        execute_run = LocalExecuteRun(self.tmp_project_path)
+        launcher.return_value.execute_run = execute_run
+        mock_get_project_path.return_value = self.tmp_project_path
+        config = dedent(
+            """
+            pipeline:
+                - build
+
+            jobs:
+                build:
+                    series: focal
+                    architectures: amd64
+                    run: |
+                        true
+                    output:
+                        paths: [.launchpad.yaml]
+            license:
+                path: LICENSE.txt
+            """
+        )
+        Path(".launchpad.yaml").write_text(config)
+
+        result = self.run_command(
+            "run", "--output-directory", str(target_path)
+        )
+
+        self.assertEqual(0, result.exit_code)
+        job_output = target_path / "build" / "0"
+        self.assertEqual(
+            {"license": {"path": "LICENSE.txt", "spdx": None}},
+            json.loads((job_output / "properties").read_text()),
+        )
+
+    @patch("lpcraft.env.get_managed_environment_project_path")
+    @patch("lpcraft.commands.run.get_provider")
+    @patch("lpcraft.commands.run.get_host_architecture", return_value="amd64")
     def test_license_field_works_also_with_other_properties(
         self,
         mock_get_host_architecture,