launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-only-available-processors into lp:launchpad.

Commit message:
Restrict the default for processors in SnapSet.new to those listed by Snap.available_processors.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-only-available-processors/+merge/362724

This doesn't matter on production because the set of build_by_default processors is very conservative, but it's a little less strange and makes some tests less conceptually confusing.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-only-available-processors into lp:launchpad.

=== modified file 'lib/lp/snappy/model/snap.py'
--- lib/lp/snappy/model/snap.py	2018-11-07 14:45:31 +0000
+++ lib/lp/snappy/model/snap.py	2019-02-05 11:29:12 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -54,7 +54,6 @@
 from lp.app.interfaces.security import IAuthorization
 from lp.buildmaster.enums import BuildStatus
 from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet
-from lp.buildmaster.interfaces.processor import IProcessorSet
 from lp.buildmaster.model.buildfarmjob import BuildFarmJob
 from lp.buildmaster.model.buildqueue import BuildQueue
 from lp.buildmaster.model.processor import Processor
@@ -964,8 +963,7 @@
 
         if processors is None:
             processors = [
-                p for p in getUtility(IProcessorSet).getAll()
-                if p.build_by_default]
+                p for p in snap.available_processors if p.build_by_default]
         snap.setProcessors(processors)
 
         return snap

=== modified file 'lib/lp/snappy/tests/test_snap.py'
--- lib/lp/snappy/tests/test_snap.py	2019-01-25 11:47:20 +0000
+++ lib/lp/snappy/tests/test_snap.py	2019-02-05 11:29:12 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test snap packages."""
@@ -1912,15 +1912,22 @@
             name="arm", restricted=True, build_by_default=False)
 
     def test_new_default_processors(self):
-        # SnapSet.new creates a SnapArch for each Processor with
+        # SnapSet.new creates a SnapArch for each available Processor with
         # build_by_default set.
-        self.factory.makeProcessor(name="default", build_by_default=True)
-        self.factory.makeProcessor(name="nondefault", build_by_default=False)
+        new_procs = [
+            self.factory.makeProcessor(name="default", build_by_default=True),
+            self.factory.makeProcessor(
+                name="nondefault", build_by_default=False),
+            ]
         owner = self.factory.makePerson()
+        distroseries = self.factory.makeDistroSeries()
+        for processor in self.unrestricted_procs + [self.arm] + new_procs:
+            self.factory.makeDistroArchSeries(
+                distroseries=distroseries, architecturetag=processor.name,
+                processor=processor)
         snap = getUtility(ISnapSet).new(
-            registrant=owner, owner=owner,
-            distro_series=self.factory.makeDistroSeries(), name="snap",
-            branch=self.factory.makeAnyBranch())
+            registrant=owner, owner=owner, distro_series=distroseries,
+            name="snap", branch=self.factory.makeAnyBranch())
         self.assertContentEqual(
             ["386", "amd64", "hppa", "default"],
             [processor.name for processor in snap.processors])
@@ -2545,7 +2552,12 @@
         ppa_admin = self.factory.makePerson(member_of=[ppa_admin_team])
         self.factory.makeProcessor(
             "arm", "ARM", "ARM", restricted=True, build_by_default=False)
-        snap = self.makeSnap()
+        distroseries = self.factory.makeDistroSeries()
+        for processor in getUtility(IProcessorSet).getAll():
+            self.factory.makeDistroArchSeries(
+                distroseries=distroseries, architecturetag=processor.name,
+                processor=processor)
+        snap = self.makeSnap(distroseries=distroseries)
         self.assertProcessors(ppa_admin, snap, ["386", "hppa", "amd64"])
 
         response = self.setProcessors(ppa_admin, snap, ["386", "arm"])
@@ -2565,7 +2577,12 @@
 
     def test_setProcessors_owner(self):
         """The snap owner can enable/disable unrestricted processors."""
-        snap = self.makeSnap()
+        distroseries = self.factory.makeDistroSeries()
+        for processor in getUtility(IProcessorSet).getAll():
+            self.factory.makeDistroArchSeries(
+                distroseries=distroseries, architecturetag=processor.name,
+                processor=processor)
+        snap = self.makeSnap(distroseries=distroseries)
         self.assertProcessors(self.person, snap, ["386", "hppa", "amd64"])
 
         response = self.setProcessors(self.person, snap, ["386"])