Merge ~xnox/launchpad:snap-request-builds-mimick-auto into launchpad:master

Proposed by Dimitri John Ledkov
Status: Needs review
Proposed branch: ~xnox/launchpad:snap-request-builds-mimick-auto
Merge into: launchpad:master
Diff against target: 74 lines (+43/-2)
2 files modified
lib/lp/snappy/browser/snap.py (+4/-2)
lib/lp/snappy/browser/tests/test_snap.py (+39/-0)
Reviewer Review Type Date Requested Status
Colin Watson Pending
Launchpad code reviewers Pending
Review via email: mp+447665@code.launchpad.net

Commit message

snappy: Make requestbuild use auto-build archive and pocket defaults

Make request snap builds page for a snap, reuse auto-build archive & pocket, if set on the snap.

Rationale:
* Currently many snap builds are only correct, or successful if they use a predetermined set of snap recipe configuration. For example - beta channel recipe, uses beta PPA archive to pull in relevant beta .deb dependencies. Or for example use special forked dependencies from a given private archive (for pre-release or edge builds). Especially when such archives cannot be encoded in the snapcraft.yaml (as they are either private, or need to be dynamically updated to one of multiple valid options, depending on upload target).
* Auto-builds are able to correctly set desired auto-build archive, and it is correctly used
* When triggering a manual build, currently the default auto-build settings are not used. Meaning one has to carefully inspect snap recipe auto-build settings and lookup matching build archive and set it on the request snap build page to ensure the manual triggered build is done the same way as the automatically triggered one
* Fix this UI/UX issue by pre-populating manually triggered build, with the same defaults that automaticly triggered builds use. Such that for example the same build archive is used.

QAStaging test case:
* Create snap recipe
* Set it to autobuild
* Set a PPA archive for autobuilds
* Click Requests Builds
* The UI page of requests builds should have the same autobuild PPA prepulated and selected

LP: #2028687

To post a comment you must log in.
Revision history for this message
Dimitri John Ledkov (xnox) wrote :

ping

Revision history for this message
Colin Watson (cjwatson) wrote :

I'm not sure it necessarily makes sense for us to look at this before you write some tests. I think I'm OK with the approach in principle.

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

Under class TestSnapRequestBuildsView(BaseTestSnapView) I don't see any test cases that test any of the SnapRequestBuildsView.initial_values()

I think i can force some changes to snap recipe under test, and then force for test_request_builds_page() text output to match what i need.

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

Managed to add test cases, and rerun all tests under lp.snappy.browser
Reverting code change, makes the test cases fail (default to archive & updates, instead of the auto_build_archive & auto_build_pocket)

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

Call out duplicate login, in the browser test.

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

Updated rationale and qastaging test case

Unmerged commits

49f5a4b... by Dimitri John Ledkov

snappy: Make requestbuild use auto-build archive and pocket defaults

Make request snap builds page for a snap, reuse auto-build archive &
pocket, if set on the snap.

Proof of concept untested code, no tests added.

LP: #2028687

Succeeded
[SUCCEEDED] docs:0 (build)
[SUCCEEDED] lint:0 (build)
[SUCCEEDED] mypy:0 (build)
13 of 3 results

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/lib/lp/snappy/browser/snap.py b/lib/lp/snappy/browser/snap.py
index b783dcd..96b7656 100644
--- a/lib/lp/snappy/browser/snap.py
+++ b/lib/lp/snappy/browser/snap.py
@@ -474,7 +474,8 @@ class SnapRequestBuildsView(LaunchpadFormView):
474 def initial_values(self):474 def initial_values(self):
475 """See `LaunchpadFormView`."""475 """See `LaunchpadFormView`."""
476 return {476 return {
477 "archive": (477 "archive": self.context.auto_build_archive
478 or (
478 # XXX cjwatson 2019-02-04: In order to support non-Ubuntu479 # XXX cjwatson 2019-02-04: In order to support non-Ubuntu
479 # bases, we'd need to store this as None and infer it based480 # bases, we'd need to store this as None and infer it based
480 # on the inferred distro series; but this will do for now.481 # on the inferred distro series; but this will do for now.
@@ -483,7 +484,8 @@ class SnapRequestBuildsView(LaunchpadFormView):
483 else self.context.distro_series.main_archive484 else self.context.distro_series.main_archive
484 ),485 ),
485 "distro_arch_series": [],486 "distro_arch_series": [],
486 "pocket": PackagePublishingPocket.UPDATES,487 "pocket": self.context.auto_build_pocket
488 or PackagePublishingPocket.UPDATES,
487 "channels": self.context.auto_build_channels,489 "channels": self.context.auto_build_channels,
488 }490 }
489491
diff --git a/lib/lp/snappy/browser/tests/test_snap.py b/lib/lp/snappy/browser/tests/test_snap.py
index 2de0693..3b11e7c 100644
--- a/lib/lp/snappy/browser/tests/test_snap.py
+++ b/lib/lp/snappy/browser/tests/test_snap.py
@@ -2800,6 +2800,45 @@ class TestSnapRequestBuildsView(BaseTestSnapView):
2800 ),2800 ),
2801 )2801 )
28022802
2803 def test_request_builds_with_autobuild_archive(self):
2804 # Ensure that auto_build_archive is used as default for builds
2805 ppa = self.factory.makeArchive(
2806 distribution=self.ubuntu, owner=self.person, name="snap-ppa"
2807 )
2808 login_person(self.person)
2809 self.snap.auto_build_archive = ppa
2810 browser = self.getViewBrowser(
2811 self.snap, "+request-builds", user=self.person
2812 )
2813 browser.getControl("Request builds").click()
2814
2815 login_person(self.person)
2816 [request] = self.snap.pending_build_requests
2817 self.assertThat(
2818 request,
2819 MatchesStructure(
2820 archive=Equals(ppa),
2821 ),
2822 )
2823
2824 def test_request_builds_with_autobuild_pocket(self):
2825 # Ensure that auto_build_pocket is used as default for builds
2826 login_person(self.person)
2827 self.snap.auto_build_pocket = PackagePublishingPocket.SECURITY
2828 browser = self.getViewBrowser(
2829 self.snap, "+request-builds", user=self.person
2830 )
2831 browser.getControl("Request builds").click()
2832
2833 login_person(self.person)
2834 [request] = self.snap.pending_build_requests
2835 self.assertThat(
2836 request,
2837 MatchesStructure(
2838 pocket=Equals(PackagePublishingPocket.SECURITY),
2839 ),
2840 )
2841
2803 def test_request_builds_no_architectures_action(self):2842 def test_request_builds_no_architectures_action(self):
2804 # Requesting a build with no architectures selected creates a2843 # Requesting a build with no architectures selected creates a
2805 # pending build request.2844 # pending build request.

Subscribers

People subscribed via source and target branches

to status/vote changes: