On 04/05/2010 05:39 PM, James Westby wrote: >> Was there any lint? > > I don't know, running "make lint" runs it on lots of unexpected files. When there are no uncommitted changes, "make lint" compares your tree to the parent branch to determine which files it runs against. This means it will also lint files changed by your prerequisite branch. You parent branch may also be out of date. You can also run bin/lint.sh with the list of files you modified. If you use pipelines and have the lp-review-body plugin installed, you can use lp-propose (formerly lp-submit) and it will use the previous pipe to determine which files to lint. Anyhow, there was lots of lint: $ make lint = Launchpad lint = Checking for conflicts. and issues in doctests and templates. Running jslint, xmllint, pyflakes, and pylint. Using normal rules. Linting changed files: lib/canonical/launchpad/interfaces/_schema_circular_imports.py lib/lp/code/errors.py lib/lp/code/interfaces/branch.py lib/lp/code/interfaces/branchtarget.py lib/lp/code/interfaces/codeimport.py lib/lp/code/interfaces/hasbranches.py lib/lp/code/interfaces/webservice.py lib/lp/code/model/branchtarget.py lib/lp/code/model/codeimport.py lib/lp/code/model/hasbranches.py lib/lp/code/model/tests/test_codeimport.py lib/lp/code/stories/webservice/xx-branchmergeproposal.txt lib/lp/code/stories/webservice/xx-code-import.txt lib/lp/registry/interfaces/product.py lib/lp/registry/interfaces/sourcepackage.py lib/lp/registry/model/product.py lib/lp/registry/model/sourcepackage.py == Pyflakes notices == lib/canonical/launchpad/interfaces/_schema_circular_imports.py 43: 'IBranchTarget' imported but unused lib/lp/code/interfaces/branch.py 11: undefined name 'UnknownBranchTypeError' in __all__ lib/lp/code/interfaces/webservice.py 8: 'BranchMergeProposalExists' imported but unused 9: 'BranchCreatorNotOwner' imported but unused 9: 'IBranchSet' imported but unused 9: 'IBranch' imported but unused 9: 'BranchExists' imported but unused 9: 'BranchCreatorNotMemberOfOwnerTeam' imported but unused 12: 'IBranchMergeProposal' imported but unused 13: 'IBranchSubscription' imported but unused 14: 'ICodeImport' imported but unused 15: 'ICodeReviewComment' imported but unused 16: 'ICodeReviewVoteReference' imported but unused 17: 'IStaticDiff' imported but unused 17: 'IDiff' imported but unused 17: 'IPreviewDiff' imported but unused lib/lp/registry/model/sourcepackage.py 684: local variable 'displayname' is assigned to but never used == Pylint notices == lib/canonical/launchpad/interfaces/_schema_circular_imports.py 18: [F0401] Unable to import 'lazr.restful.declarations' 43: [W0611] Unused import IBranchTarget lib/lp/code/errors.py 20: [F0401] Unable to import 'lazr.restful.declarations' lib/lp/code/interfaces/branch.py 442: [C0322, IBranch.setOwner] Operator not preceded by a space title=_("The new owner of the branch."), ^ schema=IPerson)) @export_write_operation() def setOwner(new_owner, user): 451: [C0322, IBranch.setTarget] Operator not preceded by a space title=_("The project the branch belongs to."), ^ schema=Interface, required=False), # Really IProduct source_package=Reference( title=_("The source package the branch belongs to."), schema=Interface, required=False)) # Really ISourcePackage @export_write_operation() def setTarget(user, project=None, source_package=None): 511: [C0322, IBranch.isPersonTrustedReviewer] Operator not preceded by a space title=_("A person for which the reviewer status is in question."), ^ schema=IPerson)) @export_read_operation() def isPersonTrustedReviewer(reviewer): 760: [C0322, IBranch._createMergeProposal] Operator not preceded by a space needs_review=Bool(title=_('Needs review'), ^ description=_('If True the proposal needs review.' 'Otherwise, it will be work in progress.')), initial_comment=Text( title=_('Initial comment'), description=_("Registrant's initial description of proposal.")), commit_message=Text( title=_('Commit message'), description=_('Message to use when committing this merge.')), reviewers=List(value_type=Reference(schema=IPerson)), review_types=List(value_type=TextLine()) ) @call_with(registrant=REQUEST_USER) @export_factory_operation(Interface, []) def _createMergeProposal( registrant, target_branch, prerequisite_branch=None, needs_review=True, initial_comment=None, commit_message=None, reviewers=None, review_types=None): 937: [C0322, IBranch.subscribe] Operator not preceded by a space schema=IPerson), ^ notification_level=Choice( title=_("The level of notification to subscribe to."), vocabulary=BranchSubscriptionNotificationLevel), max_diff_lines=Choice( title=_("The max number of lines for diff email."), vocabulary=BranchSubscriptionDiffSize), code_review_level=Choice( title=_("The level of code review notification emails."), vocabulary=CodeReviewNotificationLevel)) @operation_returns_entry(Interface) # Really IBranchSubscription @export_write_operation() def subscribe(person, notification_level, max_diff_lines, code_review_level): 965: [C0322, IBranch.getSubscription] Operator not preceded by a space schema=IPerson)) ^ @operation_returns_entry(Interface) # Really IBranchSubscription @export_read_operation() def getSubscription(person): 976: [C0322, IBranch.unsubscribe] Operator not preceded by a space title=_("The person to unsubscribe"), ^ schema=IPerson)) @export_write_operation() def unsubscribe(person): 1226: [C0322, IBranchSet.getByUrls] Operator not preceded by a space title=u'A list of URLs of branches', ^ description=( u'These can be URLs external to ' u'Launchpad, lp: URLs, or http://bazaar.launchpad.net/ URLs, ' u'or any mix of all these different kinds.'), value_type=TextLine(), required=True)) @export_read_operation() def getByUrls(urls): lib/lp/code/interfaces/branchtarget.py 21: [F0401] Unable to import 'zope.interface' 22: [F0401] Unable to import 'zope.security.interfaces' 27: [F0401] Unable to import 'lazr.restful.fields' lib/lp/code/interfaces/codeimport.py 17: [F0401] Unable to import 'zope.interface' 18: [F0401] Unable to import 'zope.schema' 27: [F0401] Unable to import 'lazr.restful.declarations' 29: [F0401] Unable to import 'lazr.restful.fields' lib/lp/code/interfaces/hasbranches.py 104: [C0301] Line too long (79/78) 15: [F0401] Unable to import 'zope.interface' 16: [F0401] Unable to import 'zope.schema' 22: [F0401] Unable to import 'lazr.restful.declarations' 25: [F0401] Unable to import 'lazr.restful.fields' 42: [E0213, IHasBranches.getBranches] Method should have "self" as first argument 42: [C0322, IHasBranches.getBranches] Operator not preceded by a space modified_since=Datetime( ^ title=_('Limit the branches to those modified since this date.'), required=False)) @call_with(visible_by_user=REQUEST_USER) @operation_returns_collection_of(Interface) # Really IBranch. @export_read_operation() def getBranches(status=None, visible_by_user=None, modified_since=None): 74: [E0213, IHasMergeProposals.getMergeProposals] Method should have "self" as first argument 100: [E0213, IHasRequestedReviews.getRequestedReviews] Method should have "self" as first argument 128: [E0213, IHasCodeImports.newCodeImport] Method should have "self" as first argument 128: [C0322, IHasCodeImports.newCodeImport] Operator not preceded by a space rcs_type=Choice(vocabulary=RevisionControlSystems, required=True), ^ url=TextLine(title=_('Foreign VCS URL')), cvs_root=TextLine(title=_('CVS root URL')), cvs_module=TextLine(title=_('CVS module to import')), owner=Reference(title=_('Owner of the resulting branch'), schema=Interface) ) @call_with(registrant=REQUEST_USER) @export_factory_operation(Interface, []) # Really ICodeImport. def newCodeImport(registrant=None, branch_name=None, rcs_type=None, url=None, cvs_root=None, cvs_module=None, owner=None): lib/lp/code/interfaces/webservice.py 13: [W0611] Unused import IBranchSubscription 9: [W0611] Unused import IBranchSet 9: [W0611] Unused import BranchCreatorNotOwner 17: [W0611] Unused import IDiff 16: [W0611] Unused import ICodeReviewVoteReference 17: [W0611] Unused import IStaticDiff 12: [W0611] Unused import IBranchMergeProposal 9: [W0611] Unused import IBranch 17: [W0611] Unused import IPreviewDiff 8: [W0611] Unused import BranchMergeProposalExists 9: [W0611] Unused import BranchExists 9: [W0611] Unused import BranchCreatorNotMemberOfOwnerTeam 14: [W0611] Unused import ICodeImport 15: [W0611] Unused import ICodeReviewComment lib/lp/code/model/branchtarget.py 15: [F0401] Unable to import 'zope.component' 16: [F0401] Unable to import 'zope.interface' 17: [F0401] Unable to import 'zope.security.proxy' 328: [W0231, ProductSeriesBranchTarget.__init__] __init__ method from base class 'ProductBranchTarget' is not called lib/lp/code/model/codeimport.py 23: [F0401] Unable to import 'zope.component' 24: [F0401] Unable to import 'zope.event' 25: [F0401] Unable to import 'zope.interface' 27: [F0401] Unable to import 'lazr.lifecycle.event' lib/lp/code/model/hasbranches.py 64: [C0301] Line too long (79/78) 14: [F0401] Unable to import 'zope.component' lib/lp/code/model/tests/test_codeimport.py 12: [F0401] Unable to import 'zope.component' 13: [F0401] Unable to import 'zope.security.proxy' lib/lp/registry/interfaces/product.py 30: [F0401] Unable to import 'zope.interface' 31: [F0401] Unable to import 'zope.schema' 33: [F0401] Unable to import 'zope.schema.vocabulary' 34: [F0401] Unable to import 'lazr.enum' 73: [F0401] Unable to import 'lazr.restful.fields' 74: [F0401] Unable to import 'lazr.restful.declarations' 774: [C0322, IProductSet.createProduct] Operator not preceded by a space freshmeatproject='freshmeat_project', wikiurl='wiki_url', ^ downloadurl='download_url', sourceforgeproject='sourceforge_project', programminglang='programming_lang') @export_factory_operation( IProduct, ['name', 'displayname', 'title', 'summary', 'description', 'project', 'homepageurl', 'screenshotsurl', 'downloadurl', 'freshmeatproject', 'wikiurl', 'sourceforgeproject', 'programminglang', 'license_reviewed', 'licenses', 'license_info', 'registrant']) @export_operation_as('new_project') def createProduct(owner, name, displayname, title, summary, description=None, project=None, homepageurl=None, screenshotsurl=None, wikiurl=None, downloadurl=None, freshmeatproject=None, sourceforgeproject=None, programminglang=None, license_reviewed=False, mugshot=None, logo=None, icon=None, licenses=None, license_info=None, registrant=None): lib/lp/registry/interfaces/sourcepackage.py 19: [F0401] Unable to import 'zope.interface' 20: [F0401] Unable to import 'zope.schema' 21: [F0401] Unable to import 'lazr.enum' 28: [F0401] Unable to import 'lazr.restful.fields' 29: [F0401] Unable to import 'lazr.restful.declarations' 189: [C0322, ISourcePackage.getBranch] Operator not preceded by a space vocabulary=DBEnumeratedType)) ^ @operation_returns_entry(Interface) @export_read_operation() def getBranch(pocket): 207: [C0322, ISourcePackage.setBranch] Operator not preceded by a space vocabulary=DBEnumeratedType), ^ branch=Reference(Interface, title=_("Branch"), required=False)) @call_with(registrant=REQUEST_USER) @export_write_operation() def setBranch(pocket, branch, registrant): lib/lp/registry/model/product.py 22: [F0401] Unable to import 'zope.interface' 23: [F0401] Unable to import 'zope.component' 24: [F0401] Unable to import 'zope.security.proxy' 27: [F0401] Unable to import 'lazr.delegates' lib/lp/registry/model/sourcepackage.py 15: [F0401] Unable to import 'zope.interface' 16: [F0401] Unable to import 'zope.component' Some of these may be bogus, but many of them should be fixed. You should suppress the pylint warnings in lib/lp/code/interfaces/hasbranches.py >> What would you think about expressing xx-code-import.txt's "Exceptions" >> heading as a unit test? Failing that, it needs two blank lines separating it >> from the previous section. > > I don't know how to do webservice tests as unit tests, if you can point me to > examples I will convert them. What works in doctests will usually work in unit tests: === added file 'lib/lp/code/tests/test_imports.py' --- lib/lp/code/tests/test_imports.py 1970-01-01 00:00:00 +0000 +++ lib/lp/code/tests/test_imports.py 2010-04-06 15:24:50 +0000 @@ -0,0 +1,39 @@ +# Copyright 2010 Canonical Ltd. This software is licensed under the +# GNU Affero General Public License version 3 (see the file LICENSE). + +"""Tests for code import API.""" + +__metaclass__ = type + + +from unittest import TestLoader + +from canonical.testing.layers import LaunchpadFunctionalLayer +from canonical.launchpad.testing.pages import webservice_for_person +from canonical.launchpad.webapp.interfaces import OAuthPermission +from lp.testing import ANONYMOUS, login, logout, TestCaseWithFactory + + +class TestCodeImportAPI(TestCaseWithFactory): + + layer = LaunchpadFunctionalLayer + + def test_cannot_create_other_team_import(self): + login(ANONYMOUS) + person = self.factory.makePerson(name='import-owner') + other_team = self.factory.makeTeam(name='other-team') + product = self.factory.makeProduct(name='scruff') + product_url = '/' + product.name + new_remote_url = self.factory.getUniqueURL() + logout() + import_webservice = webservice_for_person( + person, permission=OAuthPermission.WRITE_PUBLIC) + other_team_url = import_webservice.getAbsoluteUrl('/~other-team') + result = import_webservice.named_post( + product_url, 'newCodeImport', branch_name='other-team-import', + rcs_type='Git', url=new_remote_url, owner=other_team_url) + self.assertEqual(400, result.status) + + +def test_suite(): + return TestLoader().loadTestsFromName(__name__) Aaron