launchpad-reviewers team mailing list archive

[Colin Watson <mp+454718@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:snap-improve-infer-pro-enable into launchpad:master.

Commit message:
snap: Improve some edge cases in inferProEnable

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

Snaps with `build-base` but no `base` and snaps with `type: base` weren't being handled quite correctly, resulting in some cases returning True when they shouldn't.  Bring these cases into line with the algorithm in `Snap._findBase`.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:snap-improve-infer-pro-enable into launchpad:master.

diff --git a/lib/lp/snappy/model/snap.py b/lib/lp/snappy/model/snap.py
index c475075..92dd911 100644
--- a/lib/lp/snappy/model/snap.py
+++ b/lib/lp/snappy/model/snap.py
@@ -1675,7 +1675,18 @@ class SnapSet:
             pass
         else:
             base = snapcraft_data.get("base")
-            if base is None or base == "core":
+            build_base = snapcraft_data.get("build-base")
+            name = snapcraft_data.get("name")
+            snap_type = snapcraft_data.get("type")
+
+            if build_base is not None:
+                snap_base_name = build_base
+            elif name is not None and snap_type == "base":
+                snap_base_name = name
+            else:
+                snap_base_name = base
+
+            if snap_base_name is None or snap_base_name == "core":
                 return True
 
         return False
diff --git a/lib/lp/snappy/tests/test_snap.py b/lib/lp/snappy/tests/test_snap.py
index 5675585..0e6a31e 100644
--- a/lib/lp/snappy/tests/test_snap.py
+++ b/lib/lp/snappy/tests/test_snap.py
@@ -3670,13 +3670,16 @@ class TestSnapProcessors(TestCaseWithFactory):
         - Else, default to False
         """
 
-        refs = [self.factory.makeGitRefs()[0] for _ in range(4)]
+        refs = [self.factory.makeGitRefs()[0] for _ in range(7)]
         blobs = {
             ref.repository.getInternalPath(): blob
             for ref, blob in (
                 (refs[0], b"name: test-snap\n"),
                 (refs[1], b"name: test-snap\nbase: core\n"),
                 (refs[2], b"name: test-snap\nbase: core18\n"),
+                (refs[3], b"name: test-snap\nbuild-base: devel\n"),
+                (refs[4], b"name: core\ntype: base\n"),
+                (refs[5], b"name: core18\ntype: base\n"),
             )
         }
         self.useFixture(
@@ -3687,7 +3690,10 @@ class TestSnapProcessors(TestCaseWithFactory):
         self.assertTrue(inferProEnable(refs[0]))  # Snap with no base
         self.assertTrue(inferProEnable(refs[1]))  # Snap with 'core' base
         self.assertFalse(inferProEnable(refs[2]))  # Snap with 'core18' base
-        self.assertFalse(inferProEnable(refs[3]))  # Snap w/out snapcraft.yaml
+        self.assertFalse(inferProEnable(refs[3]))  # Snap with only build-base
+        self.assertTrue(inferProEnable(refs[4]))  # 'core' snap itself
+        self.assertFalse(inferProEnable(refs[5]))  # 'core18' snap itself
+        self.assertFalse(inferProEnable(refs[6]))  # Snap w/out snapcraft.yaml
         self.assertFalse(inferProEnable(None))  # Snap w/out ref or branch