Merge lp:~cjwatson/launchpad/snap-channels-branch into lp:launchpad

[Colin Watson]

Allow branches in snap store channel names.

These were formerly disallowed except in stable, so didn't make much sense here when channels support was first implemented, but they're fine now.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/lp/snappy/browser/tests/test_snap.py'
--- lib/lp/snappy/browser/tests/test_snap.py	2018-04-21 10:15:26 +0000
+++ lib/lp/snappy/browser/tests/test_snap.py	2018-05-08 16:48:28 +0000
@@ -1410,9 +1410,10 @@
 
     def test_store_channels_display(self):
         snap = self.factory.makeSnap(
-            store_channels=["track/stable", "track/edge"])
+            store_channels=["track/stable/fix-123", "track/edge/fix-123"])
         view = create_initialized_view(snap, "+index")
-        self.assertEqual("track/stable, track/edge", view.store_channels)
+        self.assertEqual(
+            "track/stable/fix-123, track/edge/fix-123", view.store_channels)
 
 
 class TestSnapRequestBuildsView(BaseTestSnapView):

=== modified file 'lib/lp/snappy/browser/widgets/storechannels.py'
--- lib/lp/snappy/browser/widgets/storechannels.py	2017-03-31 14:44:33 +0000
+++ lib/lp/snappy/browser/widgets/storechannels.py	2018-05-08 16:48:28 +0000
@@ -1,4 +1,4 @@
-# Copyright 2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2017-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -8,7 +8,10 @@
     ]
 
 from z3c.ptcompat import ViewPageTemplateFile
-from zope.formlib.interfaces import IInputWidget, WidgetInputError
+from zope.formlib.interfaces import (
+    IInputWidget,
+    WidgetInputError,
+    )
 from zope.formlib.utility import setUpWidget
 from zope.formlib.widget import (
     BrowserWidget,
@@ -55,16 +58,22 @@
         if self._widgets_set_up:
             return
         fields = [
-            TextLine(__name__="track", title=u"Track", required=False,
-                     description=_(
-                         "Track defines a series for your software. "
-                         "If not specified, the default track ('latest') is "
-                         "assumed.")
-                     ),
-            List(__name__="risks", title=u"Risk", required=False,
-                 value_type=Choice(vocabulary="SnapStoreChannel"),
-                 description=_(
-                     "Risks denote the stability of your software.")),
+            TextLine(
+                __name__="track", title=u"Track", required=False,
+                description=_(
+                    "Track defines a series for your software. "
+                    "If not specified, the default track ('latest') is "
+                    "assumed.")),
+            List(
+                __name__="risks", title=u"Risk", required=False,
+                value_type=Choice(vocabulary="SnapStoreChannel"),
+                description=_("Risks denote the stability of your software.")),
+            TextLine(
+                __name__="branch", title=u"Branch", required=False,
+                description=_(
+                    "Branches provide users with an easy way to test bug "
+                    "fixes.  They are temporary and created on demand.  If "
+                    "not specified, no branch is used.")),
             ]
 
         self.risks_widget = CustomWidgetFactory(LabeledMultiCheckBoxWidget)
@@ -79,40 +88,48 @@
         risks_widget = getattr(self, 'risks_widget', None)
         return risks_widget and bool(risks_widget.vocabulary)
 
-    def buildChannelName(self, track, risk):
-        """Return channel name composed from given track and risk."""
+    def buildChannelName(self, track, risk, branch):
+        """Return channel name composed from given track, risk, and branch."""
         channel = risk
         if track and track != self._default_track:
-            channel = track + self._separator + risk
+            channel = self._separator.join((track, channel))
+        if branch:
+            channel = self._separator.join((channel, branch))
         return channel
 
     def splitChannelName(self, channel):
-        """Return extracted track and risk from given channel name."""
+        """Return extracted track, risk, and branch from given channel name."""
         try:
-            track, risk = split_channel_name(channel)
+            track, risk, branch = split_channel_name(channel)
         except ValueError:
             raise AssertionError("Not a valid value: %r" % channel)
-        return track, risk
+        return track, risk, branch
 
     def setRenderedValue(self, value):
         """See `IWidget`."""
         self.setUpSubWidgets()
         if value:
-            # NOTE: atm target channels must belong to the same track
+            # NOTE: atm target channels must belong to the same track and
+            # branch
             tracks = set()
+            branches = set()
             risks = []
             for channel in value:
-                track, risk = self.splitChannelName(channel)
+                track, risk, branch = self.splitChannelName(channel)
                 tracks.add(track)
                 risks.append(risk)
-            if len(tracks) != 1:
+                branches.add(branch)
+            if len(tracks) != 1 or len(branches) != 1:
                 raise AssertionError("Not a valid value: %r" % value)
             track = tracks.pop()
             self.track_widget.setRenderedValue(track)
             self.risks_widget.setRenderedValue(risks)
+            branch = branches.pop()
+            self.branch_widget.setRenderedValue(branch)
         else:
             self.track_widget.setRenderedValue(None)
             self.risks_widget.setRenderedValue(None)
+            self.branch_widget.setRenderedValue(None)
 
     def hasInput(self):
         """See `IInputWidget`."""
@@ -129,13 +146,19 @@
     def getInputValue(self):
         """See `IInputWidget`."""
         self.setUpSubWidgets()
+        track = self.track_widget.getInputValue()
         risks = self.risks_widget.getInputValue()
-        track = self.track_widget.getInputValue()
+        branch = self.branch_widget.getInputValue()
         if track and self._separator in track:
             error_msg = "Track name cannot include '%s'." % self._separator
             raise WidgetInputError(
                 self.name, self.label, LaunchpadValidationError(error_msg))
-        channels = [self.buildChannelName(track, risk) for risk in risks]
+        if branch and self._separator in branch:
+            error_msg = "Branch name cannot include '%s'." % self._separator
+            raise WidgetInputError(
+                self.name, self.label, LaunchpadValidationError(error_msg))
+        channels = [
+            self.buildChannelName(track, risk, branch) for risk in risks]
         return channels
 
     def error(self):

=== modified file 'lib/lp/snappy/browser/widgets/templates/storechannels.pt'
--- lib/lp/snappy/browser/widgets/templates/storechannels.pt	2017-03-31 17:10:14 +0000
+++ lib/lp/snappy/browser/widgets/templates/storechannels.pt	2018-05-08 16:48:28 +0000
@@ -1,8 +1,19 @@
+<tal:root
+    xmlns:tal="http://xml.zope.org/namespaces/tal"
+    xmlns:metal="http://xml.zope.org/namespaces/metal"
+    omit-tag="">
+
 <table>
   <tr>
     <td>
-      <p class="formHelp">Channels to release this snap package to after uploading it to the store.
-        A channel is defined by a combination of an optional track and a risk, e.g. '2.1/stable', or 'stable'.
+      <p class="formHelp">
+        Channels to release this snap package to after uploading it to the
+        store.  A channel is defined by a combination of an optional track,
+        a risk, and an optional branch, e.g. '2.1/stable/fix-123',
+        '2.1/stable', 'stable/fix-123', or 'stable'.
+        <a href="https://docs.snapcraft.io/reference/channels"
+           target="_blank"
+           class="sprite maybe action-icon">(?)</a>
       </p>
     </td>
   </tr>
@@ -29,7 +40,17 @@
             </p>
           </td>
         </tr>
+        <tr>
+          <td>
+            <tal:widget define="widget nocall:view/branch_widget">
+              <metal:block
+                  use-macro="context/@@launchpad_widget_macros/launchpad_widget_row" />
+            </tal:widget>
+          </td>
+        </tr>
       </table>
     </td>
   </tr>
 </table>
+
+</tal:root>

=== modified file 'lib/lp/snappy/browser/widgets/tests/test_storechannelswidget.py'
--- lib/lp/snappy/browser/widgets/tests/test_storechannelswidget.py	2017-11-10 11:28:43 +0000
+++ lib/lp/snappy/browser/widgets/tests/test_storechannelswidget.py	2018-05-08 16:48:28 +0000
@@ -1,4 +1,4 @@
-# Copyright 2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2017-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 from __future__ import absolute_import, print_function, unicode_literals
@@ -71,6 +71,7 @@
         self.assertIsInstance(
             self.widget.risks_widget.vocabulary, SnapStoreChannelVocabulary)
         self.assertTrue(self.widget.has_risks_vocabulary)
+        self.assertIsNotNone(getattr(self.widget, "branch_widget", None))
 
     def test_setUpSubWidgets_second_call(self):
         # The setUpSubWidgets method exits early if a flag is set to
@@ -79,37 +80,63 @@
         self.widget.setUpSubWidgets()
         self.assertIsNone(getattr(self.widget, "track_widget", None))
         self.assertIsNone(getattr(self.widget, "risks_widget", None))
+        self.assertIsNone(getattr(self.widget, "branch_widget", None))
         self.assertIsNone(self.widget.has_risks_vocabulary)
 
-    def test_buildChannelName_no_track(self):
-        self.assertEqual("edge", self.widget.buildChannelName(None, "edge"))
+    def test_buildChannelName_no_track_or_branch(self):
+        self.assertEqual(
+            "edge", self.widget.buildChannelName(None, "edge", None))
 
     def test_buildChannelName_with_track(self):
         self.assertEqual(
-            "track/edge", self.widget.buildChannelName("track", "edge"))
-
-    def test_splitChannelName_no_track(self):
-        self.assertEqual((None, "edge"), self.widget.splitChannelName("edge"))
+            "track/edge", self.widget.buildChannelName("track", "edge", None))
+
+    def test_buildChannelName_with_branch(self):
+        self.assertEqual(
+            "edge/fix-123",
+            self.widget.buildChannelName(None, "edge", "fix-123"))
+
+    def test_buildChannelName_with_track_and_branch(self):
+        self.assertEqual(
+            "track/edge/fix-123",
+            self.widget.buildChannelName("track", "edge", "fix-123"))
+
+    def test_splitChannelName_no_track_or_branch(self):
+        self.assertEqual(
+            (None, "edge", None), self.widget.splitChannelName("edge"))
 
     def test_splitChannelName_with_track(self):
         self.assertEqual(
-            ("track", "edge"), self.widget.splitChannelName("track/edge"))
+            ("track", "edge", None),
+            self.widget.splitChannelName("track/edge"))
+
+    def test_splitChannelName_with_branch(self):
+        self.assertEqual(
+            (None, "edge", "fix-123"),
+            self.widget.splitChannelName("edge/fix-123"))
+
+    def test_splitChannelName_with_track_and_branch(self):
+        self.assertEqual(
+            ("track", "edge", "fix-123"),
+            self.widget.splitChannelName("track/edge/fix-123"))
 
     def test_splitChannelName_invalid(self):
         self.assertRaises(
-            AssertionError, self.widget.splitChannelName, "track/edge/invalid")
+            AssertionError, self.widget.splitChannelName,
+            "track/edge/invalid/too-long")
 
     def test_setRenderedValue_empty(self):
         self.widget.setRenderedValue([])
         self.assertIsNone(self.widget.track_widget._getCurrentValue())
         self.assertIsNone(self.widget.risks_widget._getCurrentValue())
 
-    def test_setRenderedValue_no_track(self):
-        # Channels do not include a track
+    def test_setRenderedValue_no_track_or_branch(self):
+        # Channels do not include a track or branch
         risks = ['candidate', 'edge']
         self.widget.setRenderedValue(risks)
         self.assertIsNone(self.widget.track_widget._getCurrentValue())
         self.assertEqual(risks, self.widget.risks_widget._getCurrentValue())
+        self.assertIsNone(self.widget.branch_widget._getCurrentValue())
 
     def test_setRenderedValue_with_track(self):
         # Channels including a track
@@ -118,12 +145,39 @@
         self.assertEqual('2.2', self.widget.track_widget._getCurrentValue())
         self.assertEqual(
             ['candidate', 'edge'], self.widget.risks_widget._getCurrentValue())
+        self.assertIsNone(self.widget.branch_widget._getCurrentValue())
+
+    def test_setRenderedValue_with_branch(self):
+        # Channels including a branch
+        channels = ['candidate/fix-123', 'edge/fix-123']
+        self.widget.setRenderedValue(channels)
+        self.assertIsNone(self.widget.track_widget._getCurrentValue())
+        self.assertEqual(
+            ['candidate', 'edge'], self.widget.risks_widget._getCurrentValue())
+        self.assertEqual(
+            'fix-123', self.widget.branch_widget._getCurrentValue())
+
+    def test_setRenderedValue_with_track_and_branch(self):
+        # Channels including a track and branch
+        channels = ['2.2/candidate/fix-123', '2.2/edge/fix-123']
+        self.widget.setRenderedValue(channels)
+        self.assertEqual('2.2', self.widget.track_widget._getCurrentValue())
+        self.assertEqual(
+            ['candidate', 'edge'], self.widget.risks_widget._getCurrentValue())
+        self.assertEqual(
+            'fix-123', self.widget.branch_widget._getCurrentValue())
 
     def test_setRenderedValue_invalid_value(self):
-        # Multiple channels, different tracks, unsupported
-        channels = ['2.2/candidate', '2.1/edge']
-        self.assertRaises(
-            AssertionError, self.widget.setRenderedValue, channels)
+        # Multiple channels, different tracks or branches, unsupported
+        self.assertRaises(
+            AssertionError, self.widget.setRenderedValue,
+            ['2.2/candidate', '2.1/edge'])
+        self.assertRaises(
+            AssertionError, self.widget.setRenderedValue,
+            ['candidate/fix-123', 'edge/fix-124'])
+        self.assertRaises(
+            AssertionError, self.widget.setRenderedValue,
+            ['2.2/candidate', 'edge/fix-123'])
 
     def test_hasInput_false(self):
         # hasInput is false when there is no risk set in the form data.
@@ -143,6 +197,7 @@
         form = {
             "field.channels.track": "",
             "field.channels.risks": ["invalid"],
+            "field.channels.branch": "",
             }
         self.widget.request = LaunchpadTestRequest(form=form)
         self.assertFalse(self.widget.hasValidInput())
@@ -152,6 +207,7 @@
         form = {
             "field.channels.track": "track",
             "field.channels.risks": ["stable", "beta"],
+            "field.channels.branch": "branch",
             }
         self.widget.request = LaunchpadTestRequest(form=form)
         self.assertTrue(self.widget.hasValidInput())
@@ -164,24 +220,62 @@
 
     def test_getInputValue_invalid_track(self):
         # An error is raised when the track includes a '/'.
-        form = {"field.channels.track": "tra/ck",
-                "field.channels.risks": ["beta"]}
+        form = {
+            "field.channels.track": "tra/ck",
+            "field.channels.risks": ["beta"],
+            "field.channels.branch": "",
+            }
         self.assertGetInputValueError(form, "Track name cannot include '/'.")
 
-    def test_getInputValue_no_track(self):
+    def test_getInputValue_invalid_branch(self):
+        # An error is raised when the branch includes a '/'.
+        form = {
+            "field.channels.track": "",
+            "field.channels.risks": ["beta"],
+            "field.channels.branch": "bra/nch",
+            }
+        self.assertGetInputValueError(form, "Branch name cannot include '/'.")
+
+    def test_getInputValue_no_track_or_branch(self):
         self.widget.request = LaunchpadTestRequest(
-            form={"field.channels.track": "",
-                  "field.channels.risks": ["beta", "edge"]})
+            form={
+                "field.channels.track": "",
+                "field.channels.risks": ["beta", "edge"],
+                "field.channels.branch": "",
+                })
         expected = ["beta", "edge"]
         self.assertEqual(expected, self.widget.getInputValue())
 
     def test_getInputValue_with_track(self):
         self.widget.request = LaunchpadTestRequest(
-            form={"field.channels.track": "track",
-                  "field.channels.risks": ["beta", "edge"]})
+            form={
+                "field.channels.track": "track",
+                "field.channels.risks": ["beta", "edge"],
+                "field.channels.branch": "",
+                })
         expected = ["track/beta", "track/edge"]
         self.assertEqual(expected, self.widget.getInputValue())
 
+    def test_getInputValue_with_branch(self):
+        self.widget.request = LaunchpadTestRequest(
+            form={
+                "field.channels.track": "",
+                "field.channels.risks": ["beta", "edge"],
+                "field.channels.branch": "fix-123",
+                })
+        expected = ["beta/fix-123", "edge/fix-123"]
+        self.assertEqual(expected, self.widget.getInputValue())
+
+    def test_getInputValue_with_track_and_branch(self):
+        self.widget.request = LaunchpadTestRequest(
+            form={
+                "field.channels.track": "track",
+                "field.channels.risks": ["beta", "edge"],
+                "field.channels.branch": "fix-123",
+                })
+        expected = ["track/beta/fix-123", "track/edge/fix-123"]
+        self.assertEqual(expected, self.widget.getInputValue())
+
     def test_call(self):
         # The __call__ method sets up the widgets.
         markup = self.widget()
@@ -192,5 +286,6 @@
         expected_ids = [
             "field.channels.risks.%d" % i for i in range(len(self.risks))]
         expected_ids.append("field.channels.track")
+        expected_ids.append("field.channels.branch")
         ids = [field["id"] for field in fields]
         self.assertContentEqual(expected_ids, ids)

=== modified file 'lib/lp/snappy/interfaces/snap.py'
--- lib/lp/snappy/interfaces/snap.py	2018-04-21 10:01:22 +0000
+++ lib/lp/snappy/interfaces/snap.py	2018-05-08 16:48:28 +0000
@@ -583,7 +583,9 @@
         description=_(
             "Channels to release this snap package to after uploading it to "
             "the store. A channel is defined by a combination of an optional "
-            " track and a risk, e.g. '2.1/stable', or 'stable'.")))
+            " track, a risk, and an optional branch, e.g. "
+            "'2.1/stable/fix-123', '2.1/stable', 'stable/fix-123', or "
+            "'stable'.")))
 
 
 class ISnapAdminAttributes(Interface):

=== modified file 'lib/lp/snappy/validators/channels.py'
--- lib/lp/snappy/validators/channels.py	2017-03-27 19:28:36 +0000
+++ lib/lp/snappy/validators/channels.py	2018-05-08 16:48:28 +0000
@@ -1,10 +1,12 @@
-# Copyright 2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2017-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Validators for the .store_channels attribute."""
 
 __metaclass__ = type
 
+from zope.schema.vocabulary import getVocabularyRegistry
+
 from lp import _
 from lp.app.validators import LaunchpadValidationError
 from lp.services.webapp.escaping import (
@@ -17,17 +19,42 @@
 channel_components_delimiter = '/'
 
 
+def _is_risk(component):
+    """Does this channel component identify a risk?"""
+    vocabulary = getVocabularyRegistry().get(None, "SnapStoreChannel")
+    try:
+        vocabulary.getTermByToken(component)
+    except LookupError:
+        return False
+    else:
+        return True
+
+
 def split_channel_name(channel):
-    """Return extracted track and risk from given channel name."""
+    """Return extracted track, risk, and branch from given channel name."""
     components = channel.split(channel_components_delimiter)
-    if len(components) == 2:
-        track, risk = components
+    if len(components) == 3:
+        track, risk, branch = components
+    elif len(components) == 2:
+        # Identify risk to determine if this is track/risk or risk/branch.
+        if _is_risk(components[0]):
+            if _is_risk(components[1]):
+                raise ValueError(
+                    "Branch name cannot match a risk name: %r" % channel)
+            track = None
+            risk, branch = components
+        elif _is_risk(components[1]):
+            track, risk = components
+            branch = None
+        else:
+            raise ValueError("No valid risk provided: %r" % channel)
     elif len(components) == 1:
         track = None
         risk = components[0]
+        branch = None
     else:
         raise ValueError("Invalid channel name: %r" % channel)
-    return track, risk
+    return track, risk, branch
 
 
 def channels_validator(channels):
@@ -35,19 +62,29 @@
     LaunchpadValidationError.
     """
     tracks = set()
+    branches = set()
     for name in channels:
         try:
-            track, risk = split_channel_name(name)
+            track, risk, branch = split_channel_name(name)
         except ValueError:
             message = _(
                 "Invalid channel name '${name}'. Channel names must be of the "
-                "form 'track/risk' or 'risk'.",
+                "form 'track/risk/branch', 'track/risk', 'risk/branch', or "
+                "'risk'.",
                 mapping={'name': html_escape(name)})
             raise LaunchpadValidationError(structured(message))
         tracks.add(track)
+        branches.add(branch)
+
+    # XXX cjwatson 2018-05-08: These are slightly arbitrary restrictions,
+    # but they make the UI much simpler.
 
     if len(tracks) != 1:
         message = _("Channels must belong to the same track.")
         raise LaunchpadValidationError(structured(message))
 
+    if len(branches) != 1:
+        message = _("Channels must belong to the same branch.")
+        raise LaunchpadValidationError(structured(message))
+
     return True

=== modified file 'lib/lp/snappy/validators/tests/test_channels.py'
--- lib/lp/snappy/validators/tests/test_channels.py	2017-10-20 13:35:42 +0000
+++ lib/lp/snappy/validators/tests/test_channels.py	2018-05-08 16:48:28 +0000
@@ -1,4 +1,4 @@
-# Copyright 2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2017-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 from __future__ import absolute_import, print_function, unicode_literals
@@ -6,11 +6,14 @@
 __metaclass__ = type
 
 from lp.app.validators import LaunchpadValidationError
+from lp.snappy.interfaces.snapstoreclient import ISnapStoreClient
 from lp.snappy.validators.channels import (
     channels_validator,
     split_channel_name,
     )
 from lp.testing import TestCaseWithFactory
+from lp.testing.fakemethod import FakeMethod
+from lp.testing.fixture import ZopeUtilityFixture
 from lp.testing.layers import LaunchpadFunctionalLayer
 
 
@@ -18,17 +21,50 @@
 
     layer = LaunchpadFunctionalLayer
 
-    def test_split_channel_name_no_track(self):
-        self.assertEqual((None, "edge"), split_channel_name("edge"))
+    def setUp(self):
+        super(TestChannelsValidator, self).setUp()
+        self.risks = [
+            {"name": "stable", "display_name": "Stable"},
+            {"name": "candidate", "display_name": "Candidate"},
+            {"name": "beta", "display_name": "Beta"},
+            {"name": "edge", "display_name": "Edge"},
+            ]
+        snap_store_client = FakeMethod()
+        snap_store_client.listChannels = FakeMethod(result=self.risks)
+        self.useFixture(
+            ZopeUtilityFixture(snap_store_client, ISnapStoreClient))
+
+    def test_split_channel_name_no_track_or_branch(self):
+        self.assertEqual((None, "edge", None), split_channel_name("edge"))
 
     def test_split_channel_name_with_track(self):
-        self.assertEqual(("track", "edge"), split_channel_name("track/edge"))
-
-    def test_split_channel_name_invalid(self):
-        self.assertRaises(ValueError, split_channel_name, "track/edge/invalid")
+        self.assertEqual(
+            ("track", "edge", None), split_channel_name("track/edge"))
+
+    def test_split_channel_name_with_branch(self):
+        self.assertEqual(
+            (None, "edge", "fix-123"), split_channel_name("edge/fix-123"))
+
+    def test_split_channel_name_with_track_and_branch(self):
+        self.assertEqual(
+            ("track", "edge", "fix-123"),
+            split_channel_name("track/edge/fix-123"))
+
+    def test_split_channel_name_no_risk(self):
+        self.assertRaises(ValueError, split_channel_name, "track/fix-123")
+
+    def test_split_channel_name_ambiguous_risk(self):
+        self.assertRaises(ValueError, split_channel_name, "edge/stable")
+
+    def test_split_channel_name_too_many_components(self):
+        self.assertRaises(
+            ValueError, split_channel_name, "track/edge/invalid/too-long")
 
     def test_channels_validator_valid(self):
+        self.assertTrue(
+            channels_validator(['1.1/beta/fix-123', '1.1/edge/fix-123']))
         self.assertTrue(channels_validator(['1.1/beta', '1.1/edge']))
+        self.assertTrue(channels_validator(['beta/fix-123', 'edge/fix-123']))
         self.assertTrue(channels_validator(['beta', 'edge']))
 
     def test_channels_validator_multiple_tracks(self):
@@ -36,7 +72,12 @@
             LaunchpadValidationError, channels_validator,
             ['1.1/stable', '2.1/edge'])
 
+    def test_channels_validator_multiple_branches(self):
+        self.assertRaises(
+            LaunchpadValidationError, channels_validator,
+            ['stable/fix-123', 'edge/fix-124'])
+
     def test_channels_validator_invalid_channel(self):
         self.assertRaises(
             LaunchpadValidationError, channels_validator,
-            ['1.1/stable/invalid'])
+            ['1.1/stable/invalid/too-long'])