launchpad-reviewers team mailing list archive

[Colin Watson <mp+430891@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:scan-generic-detect-float-version into launchpad:master.

Commit message:
Explicitly check type of report's version property

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

This is an easy mistake to make in YAML, so check for it explicitly to avoid a confusing OOPS later on.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:scan-generic-detect-float-version into launchpad:master.

diff --git a/lib/lp/soyuz/model/archivejob.py b/lib/lp/soyuz/model/archivejob.py
index f73cec1..0d9bf2c 100644
--- a/lib/lp/soyuz/model/archivejob.py
+++ b/lib/lp/soyuz/model/archivejob.py
@@ -623,6 +623,14 @@ class CIBuildUploadJob(ArchiveJobDerived):
             or "version" not in properties
         ):
             return {}
+        if not isinstance(properties["version"], str):
+            logger.warning(
+                "Failed to scan generic artifacts: expected version to be a "
+                "string, got %r (%s)",
+                properties["version"],
+                type(properties["version"]).__name__,
+            )
+            return {}
 
         all_metadata = {}
         for path in paths:
diff --git a/lib/lp/soyuz/tests/test_archivejob.py b/lib/lp/soyuz/tests/test_archivejob.py
index 623c7cb..40c3341 100644
--- a/lib/lp/soyuz/tests/test_archivejob.py
+++ b/lib/lp/soyuz/tests/test_archivejob.py
@@ -651,6 +651,43 @@ class TestCIBuildUploadJob(TestCaseWithFactory):
             ),
         )
 
+    def test__scanFiles_generic_bad_version(self):
+        # An easy mistake to make in YAML is to declare the version as a
+        # float rather than a string.  The scan job explicitly checks for
+        # this.
+        logger = self.useFixture(FakeLogger())
+        archive = self.factory.makeArchive()
+        distroseries = self.factory.makeDistroSeries(
+            distribution=archive.distribution
+        )
+        build = self.makeCIBuild(archive.distribution)
+        report = self.factory.makeRevisionStatusReport(
+            title="build-source", ci_build=build
+        )
+        report.update(
+            properties={
+                "name": "foo",
+                "version": 1.0,
+                "source": True,
+            }
+        )
+        job = CIBuildUploadJob.create(
+            build,
+            build.git_repository.owner,
+            archive,
+            distroseries,
+            PackagePublishingPocket.RELEASE,
+            target_channel="edge",
+        )
+        tmpdir = Path(self.useFixture(TempDir()).path)
+        (tmpdir / "foo-1.0").write_bytes(b"source artifact")
+        self.assertEqual({}, job._scanFiles(report, tmpdir))
+        expected_logs = [
+            "Failed to scan generic artifacts: expected version to be a "
+            "string, got 1.0 (float)",
+        ]
+        self.assertEqual(expected_logs, logger.output.splitlines())
+
     def test_run_indep(self):
         archive = self.factory.makeArchive(
             repository_format=ArchiveRepositoryFormat.PYTHON