Launchpad itself

Merge lp:~james-w/launchpad/code-imports-for-source-packages into lp:launchpad/db-devel

  1. code-imports-for-source-packages
  2. Merge into db-devel
Proposed by James Westby
Status: Merged
Approved by: Michael Hudson-Doyle
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~james-w/launchpad/code-imports-for-source-packages
Merge into: lp:launchpad/db-devel
Prerequisite: lp:~james-w/launchpad/new-code-import-method-on-branchtarget
Diff against target: 269 lines (+145/-12)
7 files modified
lib/lp/code/mail/tests/test_codeimport.py (+28/-0)
lib/lp/code/model/branchtarget.py (+1/-1)
lib/lp/code/model/tests/test_branchtarget.py (+15/-8)
lib/lp/code/model/tests/test_codeimport.py (+21/-0)
lib/lp/code/stories/codeimport/xx-admin-codeimport.txt (+22/-1)
lib/lp/code/stories/webservice/xx-code-import.txt (+51/-2)
lib/lp/testing/factory.py (+7/-0)
To merge this branch: bzr merge lp:~james-w/launchpad/code-imports-for-source-packages
Reviewer Review Type Date Requested Status
Michael Hudson-Doyle Approve
Review via email: mp+22191@code.launchpad.net

Commit message

Package branches are now a vaild target for code imports

Description of the change

Hi,

This flips the switch so that code imports can be registered
for package branches, though there's no way to actually create
them yet.

It's mainly adding the tests, as it's a single line change to
allow them to be created for source packages now.

Thanks,

James

To post a comment you must log in.
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :
Download full text (4.9 KiB)

Two minor whitespace tweaks please, or rather the same one in two
places. Other than that, awesome!

> === modified file 'lib/lp/code/interfaces/branchtarget.py'
> --- lib/lp/code/interfaces/branchtarget.py 2010-01-05 08:18:23 +0000
> +++ lib/lp/code/interfaces/branchtarget.py 2010-03-26 01:45:34 +0000
> @@ -88,6 +88,9 @@
> supports_merge_proposals = Attribute(
> "Does this target support merge proposals at all?")
>
> + supports_code_imports = Attribute(
> + "Does this target support code imports at all?")
> +
> def areBranchesMergeable(other_target):
> """Are branches from other_target mergeable into this target."""
>
> @@ -113,3 +116,18 @@
>
> def getBugTask(bug):
> """Get the BugTask for a given bug related to the branch target."""
> +
> + def newCodeImport(registrant, branch_name, rcs_type,
> + url=None, cvs_root=None, cvs_module=None):

This should be formatted like so:

    def newCodeImport(registrant, branch_name, rcs_type, url=None,
                      cvs_root=None, cvs_module=None):

(https://dev.launchpad.net/PythonStyleGuide#Wrapping%20long%20arguments%20in%20function%20definitions)

> + """Create a new code import for this target.
> +
> + :param registrant: the `IPerson` who should be recorded as creating
> + the import and will own the resulting branch.
> + :param branch_name: the name the resulting branch should have.
> + :param rcs_type: the type of the foreign VCS.
> + :param url: the url to import from if the import isn't CVS.
> + :param cvs_root: if the import is from CVS the CVSROOT to import from.
> + :param cvs_module: if the import is from CVS the module to import.
> + :returns: an `ICodeImport`.
> + :raises AssertionError: if supports_code_imports is False.
> + """

> === modified file 'lib/lp/code/model/branchtarget.py'
> --- lib/lp/code/model/branchtarget.py 2010-01-05 08:18:23 +0000
> +++ lib/lp/code/model/branchtarget.py 2010-03-26 01:45:34 +0000
> @@ -19,6 +19,7 @@
> from lp.code.interfaces.branchcollection import IAllBranches
> from lp.code.interfaces.branchtarget import (
> check_default_stacked_on, IBranchTarget)
> +from lp.code.interfaces.codeimport import ICodeImportSet
> from lp.registry.interfaces.pocket import PackagePublishingPocket
> from canonical.launchpad.webapp.interfaces import ICanonicalUrlData
>
> @@ -36,6 +37,12 @@
> def __ne__(self, other):
> return self.context != other.context
>
> + def newCodeImport(self, registrant, branch_name, rcs_type, url=None,
> + cvs_root=None, cvs_module=None):

Similarly.

> + return getUtility(ICodeImportSet).new(
> + registrant, self, branch_name, rcs_type, url=url,
> + cvs_root=cvs_root, cvs_module=cvs_module)
> +
>
> class PackageBranchTarget(_BaseBranchTarget):
> implements(IBranchTarget)
> @@ -95,6 +102,11 @@
> """See `IBranchTarget`."""
> return True
>
> + @property
> + def supports_code_imports(self):
> + """See `IBranchTarget`."""
> + return False
> +
> def areBranchesMergeable(self, other_target)...

Read more...

review: Approve
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

That was a review of the other proposal, of course :/

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

The only problem I have with this is that

Header
++++++

lines should be preceded by two blank lines. There are two in the diff ('Approving an import' and 'Package Branches') that are not (one of these not being your fault, but please fix anyway).

Otherwise, great.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/code/mail/tests/test_codeimport.py'
2--- lib/lp/code/mail/tests/test_codeimport.py 2010-03-18 15:39:58 +0000
3+++ lib/lp/code/mail/tests/test_codeimport.py 2010-04-02 02:46:34 +0000
4@@ -111,6 +111,34 @@
5 '-- \nYou are getting this email because you are a member of the '
6 'vcs-imports team.\n', msg.get_payload(decode=True))
7
8+ def test_new_source_package_import(self):
9+ # Test the email for a new sourcepackage import.
10+ eric = self.factory.makePerson(name='eric')
11+ distro = self.factory.makeDistribution(name='foobuntu')
12+ series = self.factory.makeDistroSeries(
13+ name='manic', distribution=distro)
14+ fooix = self.factory.makeSourcePackage(
15+ sourcename='fooix', distroseries=series)
16+ # Eric needs to be logged in for the mail to be sent.
17+ login_person(eric)
18+ code_import = self.factory.makePackageCodeImport(
19+ hg_repo_url='http://hg.example.com/fooix.hg',
20+ branch_name='master', sourcepackage=fooix, registrant=eric)
21+ transaction.commit()
22+ msg = message_from_string(stub.test_emails[0][2])
23+ self.assertEqual('code-import', msg['X-Launchpad-Notification-Type'])
24+ self.assertEqual(
25+ '~eric/foobuntu/manic/fooix/master', msg['X-Launchpad-Branch'])
26+ self.assertEqual(
27+ 'A new mercurial code import has been requested '
28+ 'by Eric:\n'
29+ ' http://code.launchpad.dev/~eric/foobuntu/manic/fooix/master\n'
30+ 'from\n'
31+ ' http://hg.example.com/fooix.hg\n'
32+ '\n'
33+ '-- \nYou are getting this email because you are a member of the '
34+ 'vcs-imports team.\n', msg.get_payload(decode=True))
35+
36
37 def test_suite():
38 return TestLoader().loadTestsFromName(__name__)
39
40=== modified file 'lib/lp/code/model/branchtarget.py'
41--- lib/lp/code/model/branchtarget.py 2010-04-02 02:46:31 +0000
42+++ lib/lp/code/model/branchtarget.py 2010-04-02 02:46:34 +0000
43@@ -105,7 +105,7 @@
44 @property
45 def supports_code_imports(self):
46 """See `IBranchTarget`."""
47- return False
48+ return True
49
50 def areBranchesMergeable(self, other_target):
51 """See `IBranchTarget`."""
52
53=== modified file 'lib/lp/code/model/tests/test_branchtarget.py'
54--- lib/lp/code/model/tests/test_branchtarget.py 2010-04-02 02:46:31 +0000
55+++ lib/lp/code/model/tests/test_branchtarget.py 2010-04-02 02:46:34 +0000
56@@ -177,15 +177,22 @@
57 ubuntu_branches.teamowner)
58 self.assertEqual(branch, self.target.default_merge_target)
59
60- def test_doesnt_support_code_imports(self):
61- self.assertFalse(self.target.supports_code_imports)
62+ def test_supports_code_imports(self):
63+ self.assertTrue(self.target.supports_code_imports)
64
65- def test_creating_code_import_fails(self):
66- self.assertRaises(
67- AssertionError, self.target.newCodeImport,
68- self.factory.makePerson(),
69- self.factory.getUniqueString("name-"),
70- RevisionControlSystems.GIT, url=self.factory.getUniqueURL())
71+ def test_creating_code_import_succeeds(self):
72+ target_url = self.factory.getUniqueURL()
73+ branch_name = self.factory.getUniqueString("name-")
74+ owner = self.factory.makePerson()
75+ code_import = self.target.newCodeImport(
76+ owner, branch_name, RevisionControlSystems.GIT, url=target_url)
77+ code_import = removeSecurityProxy(code_import)
78+ self.assertProvides(code_import, ICodeImport)
79+ self.assertEqual(target_url, code_import.url)
80+ self.assertEqual(branch_name, code_import.branch.name)
81+ self.assertEqual(owner, code_import.registrant)
82+ self.assertEqual(owner, code_import.branch.owner)
83+ self.assertEqual(self.target, code_import.branch.target)
84
85
86 class TestPersonBranchTarget(TestCaseWithFactory, BaseBranchTargetTests):
87
88=== modified file 'lib/lp/code/model/tests/test_codeimport.py'
89--- lib/lp/code/model/tests/test_codeimport.py 2010-04-02 02:46:31 +0000
90+++ lib/lp/code/model/tests/test_codeimport.py 2010-04-02 02:46:34 +0000
91@@ -10,6 +10,7 @@
92 from sqlobject import SQLObjectNotFound
93 from storm.store import Store
94 from zope.component import getUtility
95+from zope.security.proxy import removeSecurityProxy
96
97 from lp.code.model.codeimport import CodeImportSet
98 from lp.code.model.codeimportevent import CodeImportEvent
99@@ -134,6 +135,26 @@
100 url=self.factory.getUniqueURL(),
101 review_status=None)
102
103+ def test_create_source_package_import(self):
104+ """Test that we can create an import targetting a source package."""
105+ registrant = self.factory.makePerson()
106+ source_package = self.factory.makeSourcePackage()
107+ target = IBranchTarget(source_package)
108+ code_import = CodeImportSet().new(
109+ registrant=registrant,
110+ target=target,
111+ branch_name='imported',
112+ rcs_type=RevisionControlSystems.HG,
113+ url=self.factory.getUniqueURL(),
114+ review_status=None)
115+ code_import = removeSecurityProxy(code_import)
116+ self.assertEqual(registrant, code_import.registrant)
117+ self.assertEqual(registrant, code_import.branch.owner)
118+ self.assertEqual(target, code_import.branch.target)
119+ self.assertEqual(source_package, code_import.branch.sourcepackage)
120+ # And a job is still created
121+ self.assertIsNot(None, code_import.import_job)
122+
123
124 class TestCodeImportDeletion(TestCaseWithFactory):
125 """Test the deletion of CodeImports."""
126
127=== modified file 'lib/lp/code/stories/codeimport/xx-admin-codeimport.txt'
128--- lib/lp/code/stories/codeimport/xx-admin-codeimport.txt 2010-03-18 15:39:58 +0000
129+++ lib/lp/code/stories/codeimport/xx-admin-codeimport.txt 2010-04-02 02:46:34 +0000
130@@ -38,6 +38,11 @@
131 >>> hg_import_location = str(canonical_url(hg_import.branch))
132 >>> hg_import_branch_unique_name = hg_import.branch.unique_name
133
134+ >>> package_import = factory.makePackageCodeImport(
135+ ... hg_repo_url="http://hg.example.org/zap")
136+ >>> package_import_location = str(canonical_url(package_import.branch))
137+ >>> package_import_branch_unique_name = package_import.branch.unique_name
138+
139 >>> logout()
140
141 >>> import_browser = setupBrowser(
142@@ -109,6 +114,11 @@
143 >>> print_form_fields(import_browser)
144 field.url: http://hg.example.org/bar
145
146+ >>> import_browser.open(package_import_location)
147+ >>> import_browser.getLink('Edit import source or review import').click()
148+ >>> print_form_fields(import_browser)
149+ field.url: http://hg.example.org/zap
150+
151
152 Editing the import location
153 +++++++++++++++++++++++++++
154@@ -158,7 +168,7 @@
155 ... print extract_text(message)
156 The code import has been updated.
157
158-and Mercurial imports.
159+Mercurial imports,
160
161 >>> import_browser.open(hg_import_location + '/+edit-import')
162 >>> import_browser.getControl('URL').value = \
163@@ -168,6 +178,17 @@
164 ... print extract_text(message)
165 The code import has been updated.
166
167+and imports targetting source packages.
168+
169+ >>> import_browser.open(package_import_location + '/+edit-import')
170+ >>> import_browser.getControl('URL').value = \
171+ ... 'http://metal.example.org/zap'
172+ >>> import_browser.getControl('Update').click()
173+ >>> for message in get_feedback_messages(import_browser.contents):
174+ ... print extract_text(message)
175+ The code import has been updated.
176+
177+
178 Approving an import
179 +++++++++++++++++++
180
181
182=== modified file 'lib/lp/code/stories/webservice/xx-code-import.txt'
183--- lib/lp/code/stories/webservice/xx-code-import.txt 2010-03-18 15:39:58 +0000
184+++ lib/lp/code/stories/webservice/xx-code-import.txt 2010-04-02 02:46:34 +0000
185@@ -12,7 +12,8 @@
186 >>> person = factory.makePerson(name='import-owner')
187 >>> product = factory.makeProduct(name='scruff')
188 >>> code_import = removeSecurityProxy(factory.makeProductCodeImport(
189- ... registrant=person, product=product, branch_name='import'))
190+ ... registrant=person, product=product, branch_name='import',
191+ ... svn_branch_url="http://svn.domain.com/source"))
192 >>> no_import_branch = removeSecurityProxy(factory.makeProductBranch(
193 ... owner=person, product=product, name='no-import'))
194 >>> logout()
195@@ -49,7 +50,55 @@
196 >>> print representation['rcs_type']
197 Subversion via CSCVS
198 >>> print representation['url']
199- http://domain9.domain.com/path10
200+ http://svn.domain.com/source
201+ >>> print representation['cvs_root']
202+ None
203+ >>> print representation['cvs_module']
204+ None
205+ >>> print representation['date_last_successful']
206+ None
207+
208+
209+Package Branches
210+----------------
211+
212+The same is true for package branches.
213+
214+ >>> login(ANONYMOUS)
215+ >>> distribution = factory.makeDistribution(name='scruffbuntu')
216+ >>> distroseries = factory.makeDistroSeries(
217+ ... name='manic', distribution=distribution)
218+ >>> source_package = factory.makeSourcePackage(
219+ ... sourcename='scruff', distroseries=distroseries)
220+ >>> code_import = removeSecurityProxy(factory.makePackageCodeImport(
221+ ... registrant=person, sourcepackage=source_package,
222+ ... branch_name='import',
223+ ... svn_branch_url="http://svn.domain.com/package_source"))
224+ >>> logout()
225+
226+There is a link on the branch object
227+
228+ >>> branch_url = '/' + code_import.branch.unique_name
229+ >>> response = webservice.get(branch_url)
230+ >>> representation = response.jsonBody()
231+ >>> print representation['code_import_link']
232+ http://.../~import-owner/scruffbuntu/manic/scruff/import/+code-import
233+
234+and there is information available about the import itsef.
235+
236+ >>> import_url = representation['code_import_link']
237+ >>> response = webservice.get(import_url)
238+ >>> representation = response.jsonBody()
239+ >>> print representation['self_link'] == import_url
240+ True
241+ >>> print representation['branch_link']
242+ http://.../~import-owner/scruffbuntu/manic/scruff/import
243+ >>> print representation['review_status']
244+ Pending Review
245+ >>> print representation['rcs_type']
246+ Subversion via CSCVS
247+ >>> print representation['url']
248+ http://svn.domain.com/package_source
249 >>> print representation['cvs_root']
250 None
251 >>> print representation['cvs_module']
252
253=== modified file 'lib/lp/testing/factory.py'
254--- lib/lp/testing/factory.py 2010-03-26 21:03:30 +0000
255+++ lib/lp/testing/factory.py 2010-04-02 02:46:34 +0000
256@@ -1406,6 +1406,13 @@
257 return target.newFAQ(
258 owner=target.owner, title=title, content='content')
259
260+ def makePackageCodeImport(self, sourcepackage=None, **kwargs):
261+ """Make a code import targetting a sourcepackage."""
262+ if sourcepackage is None:
263+ sourcepackage = self.makeSourcePackage()
264+ target = IBranchTarget(sourcepackage)
265+ return self.makeCodeImport(target=target, **kwargs)
266+
267 def makeProductCodeImport(self, product=None, **kwargs):
268 """Make a code import targetting a product."""
269 if product is None:

Subscribers

People subscribed via source and target branches

to status/vote changes: