← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] ~cjwatson/launchpad:merge-snap-build-channels-widgets into launchpad:master

 

Colin Watson has proposed merging ~cjwatson/launchpad:merge-snap-build-channels-widgets into launchpad:master with ~cjwatson/launchpad:snap-build-channels-field as a prerequisite.

Commit message:
Use a common SnapBuildChannelsWidget for snap and charm recipes

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

These were previously very similar, so consolidate them into common code.  They now pick up the list of snap names to offer from the context field's vocabulary, so there is now exactly one place (`SnapBuildChannelsField._core_snap_names`) that knows about the list of valid core snap names.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:merge-snap-build-channels-widgets into launchpad:master.
diff --git a/lib/lp/charms/browser/widgets/charmrecipebuildchannels.py b/lib/lp/app/widgets/snapbuildchannels.py
similarity index 83%
rename from lib/lp/charms/browser/widgets/charmrecipebuildchannels.py
rename to lib/lp/app/widgets/snapbuildchannels.py
index e1b59cc..a229272 100644
--- a/lib/lp/charms/browser/widgets/charmrecipebuildchannels.py
+++ b/lib/lp/app/widgets/snapbuildchannels.py
@@ -1,10 +1,10 @@
-# Copyright 2021 Canonical Ltd.  This software is licensed under the
+# Copyright 2018-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-"""A widget for selecting source snap channels for charm recipe builds."""
+"""A widget for selecting source snap channels for builds."""
 
 __all__ = [
-    "CharmRecipeBuildChannelsWidget",
+    'SnapBuildChannelsWidget',
     ]
 
 from zope.browserpage import ViewPageTemplateFile
@@ -27,18 +27,15 @@ from lp.services.webapp.interfaces import (
 
 
 @implementer(ISingleLineWidgetLayout, IAlwaysSubmittedWidget, IInputWidget)
-class CharmRecipeBuildChannelsWidget(BrowserWidget, InputWidget):
+class SnapBuildChannelsWidget(BrowserWidget, InputWidget):
 
-    template = ViewPageTemplateFile("templates/charmrecipebuildchannels.pt")
+    template = ViewPageTemplateFile("templates/snapbuildchannels.pt")
     hint = False
-    snap_names = ["charmcraft", "core", "core18", "core20", "core22"]
     _widgets_set_up = False
 
-    def __init__(self, context, request):
-        super(CharmRecipeBuildChannelsWidget, self).__init__(context, request)
-        self.hint = (
-            "The channels to use for build tools when building the charm "
-            "recipe.")
+    @property
+    def snap_names(self):
+        return sorted(term.value for term in self.context.key_type.vocabulary)
 
     def setUpSubWidgets(self):
         if self._widgets_set_up:
@@ -52,6 +49,9 @@ class CharmRecipeBuildChannelsWidget(BrowserWidget, InputWidget):
         for field in fields:
             setUpWidget(
                 self, field.__name__, field, IInputWidget, prefix=self.name)
+        self.widgets = {
+            snap_name: getattr(self, "%s_widget" % snap_name)
+            for snap_name in self.snap_names}
         self._widgets_set_up = True
 
     def setRenderedValue(self, value):
@@ -60,8 +60,7 @@ class CharmRecipeBuildChannelsWidget(BrowserWidget, InputWidget):
         if not zope_isinstance(value, dict):
             value = {}
         for snap_name in self.snap_names:
-            getattr(self, "%s_widget" % snap_name).setRenderedValue(
-                value.get(snap_name))
+            self.widgets[snap_name].setRenderedValue(value.get(snap_name))
 
     def hasInput(self):
         """See `IInputWidget`."""
@@ -84,8 +83,7 @@ class CharmRecipeBuildChannelsWidget(BrowserWidget, InputWidget):
         self.setUpSubWidgets()
         channels = {}
         for snap_name in self.snap_names:
-            widget = getattr(self, snap_name + "_widget")
-            channel = widget.getInputValue()
+            channel = self.widgets[snap_name].getInputValue()
             if channel:
                 channels[snap_name] = channel
         return channels
@@ -97,7 +95,7 @@ class CharmRecipeBuildChannelsWidget(BrowserWidget, InputWidget):
                 self.getInputValue()
         except InputErrors as error:
             self._error = error
-        return super(CharmRecipeBuildChannelsWidget, self).error()
+        return super(SnapBuildChannelsWidget, self).error()
 
     def __call__(self):
         """See `IBrowserWidget`."""
diff --git a/lib/lp/app/widgets/templates/snapbuildchannels.pt b/lib/lp/app/widgets/templates/snapbuildchannels.pt
new file mode 100644
index 0000000..18de222
--- /dev/null
+++ b/lib/lp/app/widgets/templates/snapbuildchannels.pt
@@ -0,0 +1,12 @@
+<tal:root
+    xmlns:tal="http://xml.zope.org/namespaces/tal";
+    omit-tag="">
+
+<table class="subordinate">
+  <tr tal:repeat="snap_name view/snap_names">
+    <td tal:content="snap_name" />
+    <td><div tal:content="structure python: view.widgets[snap_name]()" /></td>
+  </tr>
+</table>
+
+</tal:root>
diff --git a/lib/lp/charms/browser/widgets/tests/test_charmrecipebuildchannelswidget.py b/lib/lp/app/widgets/tests/test_snapbuildchannels.py
similarity index 72%
rename from lib/lp/charms/browser/widgets/tests/test_charmrecipebuildchannelswidget.py
rename to lib/lp/app/widgets/tests/test_snapbuildchannels.py
index 834c801..cbc35d2 100644
--- a/lib/lp/charms/browser/widgets/tests/test_charmrecipebuildchannelswidget.py
+++ b/lib/lp/app/widgets/tests/test_snapbuildchannels.py
@@ -1,4 +1,4 @@
-# Copyright 2021 Canonical Ltd.  This software is licensed under the
+# Copyright 2018-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 import re
@@ -7,14 +7,10 @@ from zope.formlib.interfaces import (
     IBrowserWidget,
     IInputWidget,
     )
-from zope.schema import Dict
 
-from lp.charms.browser.widgets.charmrecipebuildchannels import (
-    CharmRecipeBuildChannelsWidget,
-    )
-from lp.charms.interfaces.charmrecipe import CHARM_RECIPE_ALLOW_CREATE
+from lp.app.widgets.snapbuildchannels import SnapBuildChannelsWidget
 from lp.services.beautifulsoup import BeautifulSoup
-from lp.services.features.testing import FeatureFixture
+from lp.services.fields import SnapBuildChannelsField
 from lp.services.webapp.servers import LaunchpadTestRequest
 from lp.testing import (
     TestCaseWithFactory,
@@ -23,20 +19,20 @@ from lp.testing import (
 from lp.testing.layers import DatabaseFunctionalLayer
 
 
-class TestCharmRecipeBuildChannelsWidget(TestCaseWithFactory):
+class TestSnapBuildChannelsWidget(TestCaseWithFactory):
 
     layer = DatabaseFunctionalLayer
 
     def setUp(self):
-        super(TestCharmRecipeBuildChannelsWidget, self).setUp()
-        self.useFixture(FeatureFixture({CHARM_RECIPE_ALLOW_CREATE: "on"}))
-        field = Dict(
+        super().setUp()
+        field = SnapBuildChannelsField(
             __name__="auto_build_channels",
-            title="Source snap channels for automatic builds")
-        self.context = self.factory.makeCharmRecipe()
+            title="Source snap channels for automatic builds",
+            extra_snap_names=["snapcraft"])
+        self.context = self.factory.makeSnap()
         self.field = field.bind(self.context)
         self.request = LaunchpadTestRequest()
-        self.widget = CharmRecipeBuildChannelsWidget(self.field, self.request)
+        self.widget = SnapBuildChannelsWidget(self.field, self.request)
 
     def test_implements(self):
         self.assertTrue(verifyObject(IBrowserWidget, self.widget))
@@ -44,74 +40,68 @@ class TestCharmRecipeBuildChannelsWidget(TestCaseWithFactory):
 
     def test_template(self):
         self.assertTrue(
-            self.widget.template.filename.endswith(
-                "charmrecipebuildchannels.pt"),
+            self.widget.template.filename.endswith("snapbuildchannels.pt"),
             "Template was not set up.")
 
-    def test_hint(self):
-        self.assertEqual(
-            "The channels to use for build tools when building the charm "
-            "recipe.",
-            self.widget.hint)
-
     def test_setUpSubWidgets_first_call(self):
         # The subwidgets are set up and a flag is set.
         self.widget.setUpSubWidgets()
         self.assertTrue(self.widget._widgets_set_up)
-        self.assertIsNotNone(getattr(self.widget, "charmcraft_widget", None))
         self.assertIsNotNone(getattr(self.widget, "core_widget", None))
         self.assertIsNotNone(getattr(self.widget, "core18_widget", None))
         self.assertIsNotNone(getattr(self.widget, "core20_widget", None))
         self.assertIsNotNone(getattr(self.widget, "core22_widget", None))
+        self.assertIsNotNone(getattr(self.widget, "snapcraft_widget", None))
 
     def test_setUpSubWidgets_second_call(self):
         # The setUpSubWidgets method exits early if a flag is set to
         # indicate that the widgets were set up.
         self.widget._widgets_set_up = True
         self.widget.setUpSubWidgets()
-        self.assertIsNone(getattr(self.widget, "charmcraft_widget", None))
         self.assertIsNone(getattr(self.widget, "core_widget", None))
         self.assertIsNone(getattr(self.widget, "core18_widget", None))
         self.assertIsNone(getattr(self.widget, "core20_widget", None))
         self.assertIsNone(getattr(self.widget, "core22_widget", None))
+        self.assertIsNone(getattr(self.widget, "snapcraft_widget", None))
 
     def test_setRenderedValue_None(self):
         self.widget.setRenderedValue(None)
-        self.assertIsNone(self.widget.charmcraft_widget._getCurrentValue())
         self.assertIsNone(self.widget.core_widget._getCurrentValue())
         self.assertIsNone(self.widget.core18_widget._getCurrentValue())
         self.assertIsNone(self.widget.core20_widget._getCurrentValue())
         self.assertIsNone(self.widget.core22_widget._getCurrentValue())
+        self.assertIsNone(self.widget.snapcraft_widget._getCurrentValue())
 
     def test_setRenderedValue_empty(self):
         self.widget.setRenderedValue({})
-        self.assertIsNone(self.widget.charmcraft_widget._getCurrentValue())
         self.assertIsNone(self.widget.core_widget._getCurrentValue())
         self.assertIsNone(self.widget.core18_widget._getCurrentValue())
         self.assertIsNone(self.widget.core20_widget._getCurrentValue())
         self.assertIsNone(self.widget.core22_widget._getCurrentValue())
+        self.assertIsNone(self.widget.snapcraft_widget._getCurrentValue())
 
     def test_setRenderedValue_one_channel(self):
-        self.widget.setRenderedValue({"charmcraft": "stable"})
-        self.assertEqual(
-            "stable", self.widget.charmcraft_widget._getCurrentValue())
+        self.widget.setRenderedValue({"snapcraft": "stable"})
         self.assertIsNone(self.widget.core_widget._getCurrentValue())
         self.assertIsNone(self.widget.core18_widget._getCurrentValue())
         self.assertIsNone(self.widget.core20_widget._getCurrentValue())
+        self.assertIsNone(self.widget.core20_widget._getCurrentValue())
         self.assertIsNone(self.widget.core22_widget._getCurrentValue())
+        self.assertEqual(
+            "stable", self.widget.snapcraft_widget._getCurrentValue())
 
     def test_setRenderedValue_all_channels(self):
         self.widget.setRenderedValue(
-            {"charmcraft": "stable", "core": "candidate", "core18": "beta",
-             "core20": "edge", "core22": "edge/feature"})
-        self.assertEqual(
-            "stable", self.widget.charmcraft_widget._getCurrentValue())
+            {"core": "candidate", "core18": "beta", "core20": "edge",
+             "core22": "edge/feature", "snapcraft": "stable"})
         self.assertEqual(
             "candidate", self.widget.core_widget._getCurrentValue())
         self.assertEqual("beta", self.widget.core18_widget._getCurrentValue())
         self.assertEqual("edge", self.widget.core20_widget._getCurrentValue())
         self.assertEqual(
             "edge/feature", self.widget.core22_widget._getCurrentValue())
+        self.assertEqual(
+            "stable", self.widget.snapcraft_widget._getCurrentValue())
 
     def test_hasInput_false(self):
         # hasInput is false when there are no channels in the form data.
@@ -121,7 +111,7 @@ class TestCharmRecipeBuildChannelsWidget(TestCaseWithFactory):
     def test_hasInput_true(self):
         # hasInput is true when there are channels in the form data.
         self.widget.request = LaunchpadTestRequest(
-            form={"field.auto_build_channels.charmcraft": "stable"})
+            form={"field.auto_build_channels.snapcraft": "stable"})
         self.assertTrue(self.widget.hasInput())
 
     def test_hasValidInput_true(self):
@@ -129,45 +119,45 @@ class TestCharmRecipeBuildChannelsWidget(TestCaseWithFactory):
         # (At the moment, individual channel names are not validated, so
         # there is no "false" counterpart to this test.)
         form = {
-            "field.auto_build_channels.charmcraft": "stable",
             "field.auto_build_channels.core": "",
             "field.auto_build_channels.core18": "beta",
             "field.auto_build_channels.core20": "edge",
             "field.auto_build_channels.core22": "edge/feature",
+            "field.auto_build_channels.snapcraft": "stable",
             }
         self.widget.request = LaunchpadTestRequest(form=form)
         self.assertTrue(self.widget.hasValidInput())
 
     def test_getInputValue(self):
         form = {
-            "field.auto_build_channels.charmcraft": "stable",
             "field.auto_build_channels.core": "",
             "field.auto_build_channels.core18": "beta",
             "field.auto_build_channels.core20": "edge",
             "field.auto_build_channels.core22": "edge/feature",
+            "field.auto_build_channels.snapcraft": "stable",
             }
         self.widget.request = LaunchpadTestRequest(form=form)
         self.assertEqual(
-            {"charmcraft": "stable", "core18": "beta", "core20": "edge",
-             "core22": "edge/feature"},
+            {"core18": "beta", "core20": "edge", "core22": "edge/feature",
+             "snapcraft": "stable"},
             self.widget.getInputValue())
 
     def test_call(self):
         # The __call__ method sets up the widgets.
         markup = self.widget()
-        self.assertIsNotNone(self.widget.charmcraft_widget)
         self.assertIsNotNone(self.widget.core_widget)
         self.assertIsNotNone(self.widget.core18_widget)
         self.assertIsNotNone(self.widget.core20_widget)
         self.assertIsNotNone(self.widget.core22_widget)
+        self.assertIsNotNone(self.widget.snapcraft_widget)
         soup = BeautifulSoup(markup)
         fields = soup.find_all(["input"], {"id": re.compile(".*")})
         expected_ids = [
-            "field.auto_build_channels.charmcraft",
             "field.auto_build_channels.core",
             "field.auto_build_channels.core18",
             "field.auto_build_channels.core20",
             "field.auto_build_channels.core22",
+            "field.auto_build_channels.snapcraft",
             ]
         ids = [field["id"] for field in fields]
         self.assertContentEqual(expected_ids, ids)
diff --git a/lib/lp/charms/browser/charmrecipe.py b/lib/lp/charms/browser/charmrecipe.py
index da8f66c..ed30edf 100644
--- a/lib/lp/charms/browser/charmrecipe.py
+++ b/lib/lp/charms/browser/charmrecipe.py
@@ -23,6 +23,7 @@ from lazr.restful.interface import (
     )
 from zope.component import getUtility
 from zope.error.interfaces import IErrorReportingUtility
+from zope.formlib.widget import CustomWidgetFactory
 from zope.interface import (
     implementer,
     Interface,
@@ -38,9 +39,7 @@ from lp.app.browser.launchpadform import (
     )
 from lp.app.browser.lazrjs import InlinePersonEditPickerWidget
 from lp.app.browser.tales import format_link
-from lp.charms.browser.widgets.charmrecipebuildchannels import (
-    CharmRecipeBuildChannelsWidget,
-    )
+from lp.app.widgets.snapbuildchannels import SnapBuildChannelsWidget
 from lp.charms.interfaces.charmhubclient import (
     BadRequestPackageUploadResponse,
     )
@@ -314,7 +313,11 @@ class CharmRecipeAddView(CharmRecipeAuthorizeMixin, LaunchpadFormView):
     schema = ICharmRecipeEditSchema
 
     custom_widget_git_ref = GitRefWidget
-    custom_widget_auto_build_channels = CharmRecipeBuildChannelsWidget
+    custom_widget_auto_build_channels = CustomWidgetFactory(
+        SnapBuildChannelsWidget,
+        hint=(
+            "The channels to use for build tools when building the charm "
+            "recipe."))
     custom_widget_store_channels = StoreChannelsWidget
 
     @property
@@ -504,7 +507,11 @@ class CharmRecipeEditView(BaseCharmRecipeEditView):
         "store_channels",
         ]
     custom_widget_git_ref = GitRefWidget
-    custom_widget_auto_build_channels = CharmRecipeBuildChannelsWidget
+    custom_widget_auto_build_channels = CustomWidgetFactory(
+        SnapBuildChannelsWidget,
+        hint=(
+            "The channels to use for build tools when building the charm "
+            "recipe."))
     custom_widget_store_channels = StoreChannelsWidget
 
     def validate(self, data):
@@ -623,7 +630,11 @@ class CharmRecipeRequestBuildsView(LaunchpadFormView):
             ICharmRecipe["auto_build_channels"], __name__="channels",
             title="Source snap channels", required=True)
 
-    custom_widget_channels = CharmRecipeBuildChannelsWidget
+    custom_widget_channels = CustomWidgetFactory(
+        SnapBuildChannelsWidget,
+        hint=(
+            "The channels to use for build tools when building the charm "
+            "recipe."))
 
     @property
     def cancel_url(self):
diff --git a/lib/lp/charms/browser/widgets/__init__.py b/lib/lp/charms/browser/widgets/__init__.py
deleted file mode 100644
index e69de29..0000000
--- a/lib/lp/charms/browser/widgets/__init__.py
+++ /dev/null
diff --git a/lib/lp/charms/browser/widgets/templates/charmrecipebuildchannels.pt b/lib/lp/charms/browser/widgets/templates/charmrecipebuildchannels.pt
deleted file mode 100644
index 3b68f80..0000000
--- a/lib/lp/charms/browser/widgets/templates/charmrecipebuildchannels.pt
+++ /dev/null
@@ -1,28 +0,0 @@
-<tal:root
-    xmlns:tal="http://xml.zope.org/namespaces/tal";
-    omit-tag="">
-
-<table class="subordinate">
-  <tr>
-    <td>charmcraft</td>
-    <td><div tal:content="structure view/charmcraft_widget" /></td>
-  </tr>
-  <tr>
-    <td>core</td>
-    <td><div tal:content="structure view/core_widget" /></td>
-  </tr>
-  <tr>
-    <td>core18</td>
-    <td><div tal:content="structure view/core18_widget" /></td>
-  </tr>
-  <tr>
-    <td>core20</td>
-    <td><div tal:content="structure view/core20_widget" /></td>
-  </tr>
-  <tr>
-    <td>core22</td>
-    <td><div tal:content="structure view/core22_widget" /></td>
-  </tr>
-</table>
-
-</tal:root>
diff --git a/lib/lp/charms/browser/widgets/tests/__init__.py b/lib/lp/charms/browser/widgets/tests/__init__.py
deleted file mode 100644
index e69de29..0000000
--- a/lib/lp/charms/browser/widgets/tests/__init__.py
+++ /dev/null
diff --git a/lib/lp/snappy/browser/snap.py b/lib/lp/snappy/browser/snap.py
index fa8396d..6922b70 100644
--- a/lib/lp/snappy/browser/snap.py
+++ b/lib/lp/snappy/browser/snap.py
@@ -55,6 +55,7 @@ from lp.app.widgets.itemswidgets import (
     LaunchpadRadioWidget,
     LaunchpadRadioWidgetWithDescription,
     )
+from lp.app.widgets.snapbuildchannels import SnapBuildChannelsWidget
 from lp.buildmaster.interfaces.processor import IProcessorSet
 from lp.code.browser.widgets.gitref import GitRefWidget
 from lp.code.interfaces.branch import IBranch
@@ -87,9 +88,6 @@ from lp.services.webapp.interfaces import ICanonicalUrlData
 from lp.services.webapp.url import urlappend
 from lp.services.webhooks.browser import WebhookTargetNavigationMixin
 from lp.snappy.browser.widgets.snaparchive import SnapArchiveWidget
-from lp.snappy.browser.widgets.snapbuildchannels import (
-    SnapBuildChannelsWidget,
-    )
 from lp.snappy.browser.widgets.storechannels import StoreChannelsWidget
 from lp.snappy.interfaces.snap import (
     CannotAuthorizeStoreUploads,
@@ -100,6 +98,7 @@ from lp.snappy.interfaces.snap import (
     MissingSnapcraftYaml,
     NoSuchSnap,
     SNAP_PRIVATE_FEATURE_FLAG,
+    SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG,
     SnapPrivateFeatureDisabled,
     )
 from lp.snappy.interfaces.snapbuild import (
@@ -395,6 +394,29 @@ def builds_and_requests_for_snap(snap):
     return items
 
 
+class HintedSnapBuildChannelsWidget(SnapBuildChannelsWidget):
+    """A variant of SnapBuildChannelsWidget with appropriate hints."""
+
+    def __init__(self, context, request):
+        super().__init__(context, request)
+        self.hint = (
+            'The channels to use for build tools when building the snap '
+            'package.\n')
+        default_snapcraft_channel = (
+            getFeatureFlag(SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG) or "apt")
+        if default_snapcraft_channel == "apt":
+            self.hint += (
+                'If unset, or if the channel for snapcraft is set to "apt", '
+                'the default is to install snapcraft from the source archive '
+                'using apt.')
+        else:
+            self.hint += (
+                'If unset, the default is to install snapcraft from the "%s" '
+                'channel.  Setting the channel for snapcraft to "apt" causes '
+                'snapcraft to be installed from the source archive using '
+                'apt.' % default_snapcraft_channel)
+
+
 class SnapRequestBuildsView(LaunchpadFormView):
     """A view for requesting builds of a snap package."""
 
@@ -424,7 +446,7 @@ class SnapRequestBuildsView(LaunchpadFormView):
     custom_widget_archive = SnapArchiveWidget
     custom_widget_distro_arch_series = LabeledMultiCheckBoxWidget
     custom_widget_pocket = LaunchpadDropdownWidget
-    custom_widget_channels = SnapBuildChannelsWidget
+    custom_widget_channels = HintedSnapBuildChannelsWidget
 
     help_links = {
         "pocket": "/+help-snappy/snap-build-pocket.html",
@@ -535,7 +557,7 @@ class SnapAddView(SnapAuthorizeMixin, EnableProcessorsMixin,
     custom_widget_store_distro_series = LaunchpadRadioWidget
     custom_widget_auto_build_archive = SnapArchiveWidget
     custom_widget_auto_build_pocket = LaunchpadDropdownWidget
-    custom_widget_auto_build_channels = SnapBuildChannelsWidget
+    custom_widget_auto_build_channels = HintedSnapBuildChannelsWidget
     custom_widget_store_channels = StoreChannelsWidget
 
     help_links = {
@@ -912,7 +934,7 @@ class SnapEditView(BaseSnapEditView, EnableProcessorsMixin):
         GitRefWidget, allow_external=True)
     custom_widget_auto_build_archive = SnapArchiveWidget
     custom_widget_auto_build_pocket = LaunchpadDropdownWidget
-    custom_widget_auto_build_channels = SnapBuildChannelsWidget
+    custom_widget_auto_build_channels = HintedSnapBuildChannelsWidget
     custom_widget_store_channels = StoreChannelsWidget
     # See `setUpWidgets` method.
     custom_widget_information_type = CustomWidgetFactory(
diff --git a/lib/lp/snappy/browser/tests/test_snap.py b/lib/lp/snappy/browser/tests/test_snap.py
index b1a55d4..9ac4e2c 100644
--- a/lib/lp/snappy/browser/tests/test_snap.py
+++ b/lib/lp/snappy/browser/tests/test_snap.py
@@ -42,6 +42,9 @@ from zope.testbrowser.browser import LinkNotFoundError
 
 from lp.app.enums import InformationType
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
+from lp.app.widgets.tests.test_snapbuildchannels import (
+    TestSnapBuildChannelsWidget,
+    )
 from lp.buildmaster.enums import BuildStatus
 from lp.buildmaster.interfaces.processor import IProcessorSet
 from lp.code.errors import (
@@ -68,6 +71,7 @@ from lp.services.propertycache import get_property_cache
 from lp.services.webapp import canonical_url
 from lp.services.webapp.servers import LaunchpadTestRequest
 from lp.snappy.browser.snap import (
+    HintedSnapBuildChannelsWidget,
     SnapAdminView,
     SnapEditView,
     SnapView,
@@ -76,6 +80,7 @@ from lp.snappy.interfaces.snap import (
     CannotModifySnapProcessor,
     ISnapSet,
     SNAP_PRIVATE_FEATURE_FLAG,
+    SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG,
     SNAP_TESTING_FLAGS,
     SnapBuildRequestStatus,
     SnapPrivateFeatureDisabled,
@@ -2081,6 +2086,47 @@ class TestSnapView(BaseTestSnapView):
         self.assertEqual(authorize_url, authorize_link.url)
 
 
+class TestHintedSnapBuildChannelsWidget(TestSnapBuildChannelsWidget):
+
+    def setUp(self):
+        super().setUp()
+        self.widget = HintedSnapBuildChannelsWidget(self.field, self.request)
+
+    def test_hint_no_feature_flag(self):
+        self.assertEqual(
+            'The channels to use for build tools when building the snap '
+            'package.\n'
+            'If unset, or if the channel for snapcraft is set to "apt", '
+            'the default is to install snapcraft from the source archive '
+            'using apt.',
+            self.widget.hint)
+
+    def test_hint_feature_flag_apt(self):
+        self.useFixture(
+            FeatureFixture({SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG: "apt"}))
+        widget = HintedSnapBuildChannelsWidget(self.field, self.request)
+        self.assertEqual(
+            'The channels to use for build tools when building the snap '
+            'package.\n'
+            'If unset, or if the channel for snapcraft is set to "apt", '
+            'the default is to install snapcraft from the source archive '
+            'using apt.',
+            widget.hint)
+
+    def test_hint_feature_flag_real_channel(self):
+        self.useFixture(
+            FeatureFixture({SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG: "stable"}))
+        widget = HintedSnapBuildChannelsWidget(self.field, self.request)
+        self.assertEqual(
+            'The channels to use for build tools when building the snap '
+            'package.\n'
+            'If unset, the default is to install snapcraft from the "stable" '
+            'channel.  Setting the channel for snapcraft to "apt" causes '
+            'snapcraft to be installed from the source archive using '
+            'apt.',
+            widget.hint)
+
+
 class TestSnapRequestBuildsView(BaseTestSnapView):
 
     def setUp(self):
diff --git a/lib/lp/snappy/browser/widgets/snapbuildchannels.py b/lib/lp/snappy/browser/widgets/snapbuildchannels.py
deleted file mode 100644
index 1be81fc..0000000
--- a/lib/lp/snappy/browser/widgets/snapbuildchannels.py
+++ /dev/null
@@ -1,120 +0,0 @@
-# Copyright 2018-2019 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""A widget for selecting source snap channels for builds."""
-
-__all__ = [
-    'SnapBuildChannelsWidget',
-    ]
-
-from zope.browserpage import ViewPageTemplateFile
-from zope.formlib.interfaces import IInputWidget
-from zope.formlib.utility import setUpWidget
-from zope.formlib.widget import (
-    BrowserWidget,
-    InputErrors,
-    InputWidget,
-    )
-from zope.interface import implementer
-from zope.schema import TextLine
-from zope.security.proxy import isinstance as zope_isinstance
-
-from lp.app.errors import UnexpectedFormData
-from lp.services.features import getFeatureFlag
-from lp.services.webapp.interfaces import (
-    IAlwaysSubmittedWidget,
-    ISingleLineWidgetLayout,
-    )
-from lp.snappy.interfaces.snap import SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG
-
-
-@implementer(ISingleLineWidgetLayout, IAlwaysSubmittedWidget, IInputWidget)
-class SnapBuildChannelsWidget(BrowserWidget, InputWidget):
-
-    template = ViewPageTemplateFile("templates/snapbuildchannels.pt")
-    hint = False
-    snap_names = ["core", "core18", "core20", "core22", "snapcraft"]
-    _widgets_set_up = False
-
-    def __init__(self, context, request):
-        super(SnapBuildChannelsWidget, self).__init__(context, request)
-        self.hint = (
-            'The channels to use for build tools when building the snap '
-            'package.\n')
-        default_snapcraft_channel = (
-            getFeatureFlag(SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG) or "apt")
-        if default_snapcraft_channel == "apt":
-            self.hint += (
-                'If unset, or if the channel for snapcraft is set to "apt", '
-                'the default is to install snapcraft from the source archive '
-                'using apt.')
-        else:
-            self.hint += (
-                'If unset, the default is to install snapcraft from the "%s" '
-                'channel.  Setting the channel for snapcraft to "apt" causes '
-                'snapcraft to be installed from the source archive using '
-                'apt.' % default_snapcraft_channel)
-
-    def setUpSubWidgets(self):
-        if self._widgets_set_up:
-            return
-        fields = [
-            TextLine(
-                __name__=snap_name, title="%s channel" % snap_name,
-                required=False)
-            for snap_name in self.snap_names
-            ]
-        for field in fields:
-            setUpWidget(
-                self, field.__name__, field, IInputWidget, prefix=self.name)
-        self._widgets_set_up = True
-
-    def setRenderedValue(self, value):
-        """See `IWidget`."""
-        self.setUpSubWidgets()
-        if not zope_isinstance(value, dict):
-            value = {}
-        for snap_name in self.snap_names:
-            getattr(self, "%s_widget" % snap_name).setRenderedValue(
-                value.get(snap_name))
-
-    def hasInput(self):
-        """See `IInputWidget`."""
-        return any(
-            "%s.%s" % (self.name, snap_name) in self.request.form
-            for snap_name in self.snap_names)
-
-    def hasValidInput(self):
-        """See `IInputWidget`."""
-        try:
-            self.getInputValue()
-            return True
-        except InputErrors:
-            return False
-        except UnexpectedFormData:
-            return False
-
-    def getInputValue(self):
-        """See `IInputWidget`."""
-        self.setUpSubWidgets()
-        channels = {}
-        for snap_name in self.snap_names:
-            widget = getattr(self, snap_name + "_widget")
-            channel = widget.getInputValue()
-            if channel:
-                channels[snap_name] = channel
-        return channels
-
-    def error(self):
-        """See `IBrowserWidget`."""
-        try:
-            if self.hasInput():
-                self.getInputValue()
-        except InputErrors as error:
-            self._error = error
-        return super(SnapBuildChannelsWidget, self).error()
-
-    def __call__(self):
-        """See `IBrowserWidget`."""
-        self.setUpSubWidgets()
-        return self.template()
diff --git a/lib/lp/snappy/browser/widgets/templates/snapbuildchannels.pt b/lib/lp/snappy/browser/widgets/templates/snapbuildchannels.pt
deleted file mode 100644
index 5a92fdf..0000000
--- a/lib/lp/snappy/browser/widgets/templates/snapbuildchannels.pt
+++ /dev/null
@@ -1,28 +0,0 @@
-<tal:root
-    xmlns:tal="http://xml.zope.org/namespaces/tal";
-    omit-tag="">
-
-<table class="subordinate">
-  <tr>
-    <td>core</td>
-    <td><div tal:content="structure view/core_widget" /></td>
-  </tr>
-  <tr>
-    <td>core18</td>
-    <td><div tal:content="structure view/core18_widget" /></td>
-  </tr>
-  <tr>
-    <td>core20</td>
-    <td><div tal:content="structure view/core20_widget" /></td>
-  </tr>
-  <tr>
-    <td>core22</td>
-    <td><div tal:content="structure view/core22_widget" /></td>
-  </tr>
-  <tr>
-    <td>snapcraft</td>
-    <td><div tal:content="structure view/snapcraft_widget" /></td>
-  </tr>
-</table>
-
-</tal:root>
diff --git a/lib/lp/snappy/browser/widgets/tests/test_snapbuildchannelswidget.py b/lib/lp/snappy/browser/widgets/tests/test_snapbuildchannelswidget.py
deleted file mode 100644
index 2d4dc25..0000000
--- a/lib/lp/snappy/browser/widgets/tests/test_snapbuildchannelswidget.py
+++ /dev/null
@@ -1,200 +0,0 @@
-# Copyright 2018-2019 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-import re
-
-from zope.formlib.interfaces import (
-    IBrowserWidget,
-    IInputWidget,
-    )
-from zope.schema import Dict
-
-from lp.services.beautifulsoup import BeautifulSoup
-from lp.services.features.testing import FeatureFixture
-from lp.services.webapp.servers import LaunchpadTestRequest
-from lp.snappy.browser.widgets.snapbuildchannels import (
-    SnapBuildChannelsWidget,
-    )
-from lp.snappy.interfaces.snap import SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG
-from lp.testing import (
-    TestCaseWithFactory,
-    verifyObject,
-    )
-from lp.testing.layers import DatabaseFunctionalLayer
-
-
-class TestSnapBuildChannelsWidget(TestCaseWithFactory):
-
-    layer = DatabaseFunctionalLayer
-
-    def setUp(self):
-        super(TestSnapBuildChannelsWidget, self).setUp()
-        field = Dict(
-            __name__="auto_build_channels",
-            title="Source snap channels for automatic builds")
-        self.context = self.factory.makeSnap()
-        self.field = field.bind(self.context)
-        self.request = LaunchpadTestRequest()
-        self.widget = SnapBuildChannelsWidget(self.field, self.request)
-
-    def test_implements(self):
-        self.assertTrue(verifyObject(IBrowserWidget, self.widget))
-        self.assertTrue(verifyObject(IInputWidget, self.widget))
-
-    def test_template(self):
-        self.assertTrue(
-            self.widget.template.filename.endswith("snapbuildchannels.pt"),
-            "Template was not set up.")
-
-    def test_hint_no_feature_flag(self):
-        self.assertEqual(
-            'The channels to use for build tools when building the snap '
-            'package.\n'
-            'If unset, or if the channel for snapcraft is set to "apt", '
-            'the default is to install snapcraft from the source archive '
-            'using apt.',
-            self.widget.hint)
-
-    def test_hint_feature_flag_apt(self):
-        self.useFixture(
-            FeatureFixture({SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG: "apt"}))
-        widget = SnapBuildChannelsWidget(self.field, self.request)
-        self.assertEqual(
-            'The channels to use for build tools when building the snap '
-            'package.\n'
-            'If unset, or if the channel for snapcraft is set to "apt", '
-            'the default is to install snapcraft from the source archive '
-            'using apt.',
-            widget.hint)
-
-    def test_hint_feature_flag_real_channel(self):
-        self.useFixture(
-            FeatureFixture({SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG: "stable"}))
-        widget = SnapBuildChannelsWidget(self.field, self.request)
-        self.assertEqual(
-            'The channels to use for build tools when building the snap '
-            'package.\n'
-            'If unset, the default is to install snapcraft from the "stable" '
-            'channel.  Setting the channel for snapcraft to "apt" causes '
-            'snapcraft to be installed from the source archive using '
-            'apt.',
-            widget.hint)
-
-    def test_setUpSubWidgets_first_call(self):
-        # The subwidgets are set up and a flag is set.
-        self.widget.setUpSubWidgets()
-        self.assertTrue(self.widget._widgets_set_up)
-        self.assertIsNotNone(getattr(self.widget, "core_widget", None))
-        self.assertIsNotNone(getattr(self.widget, "core18_widget", None))
-        self.assertIsNotNone(getattr(self.widget, "core20_widget", None))
-        self.assertIsNotNone(getattr(self.widget, "core22_widget", None))
-        self.assertIsNotNone(getattr(self.widget, "snapcraft_widget", None))
-
-    def test_setUpSubWidgets_second_call(self):
-        # The setUpSubWidgets method exits early if a flag is set to
-        # indicate that the widgets were set up.
-        self.widget._widgets_set_up = True
-        self.widget.setUpSubWidgets()
-        self.assertIsNone(getattr(self.widget, "core_widget", None))
-        self.assertIsNone(getattr(self.widget, "core18_widget", None))
-        self.assertIsNone(getattr(self.widget, "core20_widget", None))
-        self.assertIsNone(getattr(self.widget, "core22_widget", None))
-        self.assertIsNone(getattr(self.widget, "snapcraft_widget", None))
-
-    def test_setRenderedValue_None(self):
-        self.widget.setRenderedValue(None)
-        self.assertIsNone(self.widget.core_widget._getCurrentValue())
-        self.assertIsNone(self.widget.core18_widget._getCurrentValue())
-        self.assertIsNone(self.widget.core20_widget._getCurrentValue())
-        self.assertIsNone(self.widget.core22_widget._getCurrentValue())
-        self.assertIsNone(self.widget.snapcraft_widget._getCurrentValue())
-
-    def test_setRenderedValue_empty(self):
-        self.widget.setRenderedValue({})
-        self.assertIsNone(self.widget.core_widget._getCurrentValue())
-        self.assertIsNone(self.widget.core18_widget._getCurrentValue())
-        self.assertIsNone(self.widget.core20_widget._getCurrentValue())
-        self.assertIsNone(self.widget.core22_widget._getCurrentValue())
-        self.assertIsNone(self.widget.snapcraft_widget._getCurrentValue())
-
-    def test_setRenderedValue_one_channel(self):
-        self.widget.setRenderedValue({"snapcraft": "stable"})
-        self.assertIsNone(self.widget.core_widget._getCurrentValue())
-        self.assertIsNone(self.widget.core18_widget._getCurrentValue())
-        self.assertIsNone(self.widget.core20_widget._getCurrentValue())
-        self.assertIsNone(self.widget.core20_widget._getCurrentValue())
-        self.assertIsNone(self.widget.core22_widget._getCurrentValue())
-        self.assertEqual(
-            "stable", self.widget.snapcraft_widget._getCurrentValue())
-
-    def test_setRenderedValue_all_channels(self):
-        self.widget.setRenderedValue(
-            {"core": "candidate", "core18": "beta", "core20": "edge",
-             "core22": "edge/feature", "snapcraft": "stable"})
-        self.assertEqual(
-            "candidate", self.widget.core_widget._getCurrentValue())
-        self.assertEqual("beta", self.widget.core18_widget._getCurrentValue())
-        self.assertEqual("edge", self.widget.core20_widget._getCurrentValue())
-        self.assertEqual(
-            "edge/feature", self.widget.core22_widget._getCurrentValue())
-        self.assertEqual(
-            "stable", self.widget.snapcraft_widget._getCurrentValue())
-
-    def test_hasInput_false(self):
-        # hasInput is false when there are no channels in the form data.
-        self.widget.request = LaunchpadTestRequest(form={})
-        self.assertFalse(self.widget.hasInput())
-
-    def test_hasInput_true(self):
-        # hasInput is true when there are channels in the form data.
-        self.widget.request = LaunchpadTestRequest(
-            form={"field.auto_build_channels.snapcraft": "stable"})
-        self.assertTrue(self.widget.hasInput())
-
-    def test_hasValidInput_true(self):
-        # The field input is valid when all submitted channels are valid.
-        # (At the moment, individual channel names are not validated, so
-        # there is no "false" counterpart to this test.)
-        form = {
-            "field.auto_build_channels.core": "",
-            "field.auto_build_channels.core18": "beta",
-            "field.auto_build_channels.core20": "edge",
-            "field.auto_build_channels.core22": "edge/feature",
-            "field.auto_build_channels.snapcraft": "stable",
-            }
-        self.widget.request = LaunchpadTestRequest(form=form)
-        self.assertTrue(self.widget.hasValidInput())
-
-    def test_getInputValue(self):
-        form = {
-            "field.auto_build_channels.core": "",
-            "field.auto_build_channels.core18": "beta",
-            "field.auto_build_channels.core20": "edge",
-            "field.auto_build_channels.core22": "edge/feature",
-            "field.auto_build_channels.snapcraft": "stable",
-            }
-        self.widget.request = LaunchpadTestRequest(form=form)
-        self.assertEqual(
-            {"core18": "beta", "core20": "edge", "core22": "edge/feature",
-             "snapcraft": "stable"},
-            self.widget.getInputValue())
-
-    def test_call(self):
-        # The __call__ method sets up the widgets.
-        markup = self.widget()
-        self.assertIsNotNone(self.widget.core_widget)
-        self.assertIsNotNone(self.widget.core18_widget)
-        self.assertIsNotNone(self.widget.core20_widget)
-        self.assertIsNotNone(self.widget.core22_widget)
-        self.assertIsNotNone(self.widget.snapcraft_widget)
-        soup = BeautifulSoup(markup)
-        fields = soup.find_all(["input"], {"id": re.compile(".*")})
-        expected_ids = [
-            "field.auto_build_channels.core",
-            "field.auto_build_channels.core18",
-            "field.auto_build_channels.core20",
-            "field.auto_build_channels.core22",
-            "field.auto_build_channels.snapcraft",
-            ]
-        ids = [field["id"] for field in fields]
-        self.assertContentEqual(expected_ids, ids)