Merge lp:~mars/tarmac/fix-plugin-import into lp:~launchpad/tarmac/lp-tarmac

[Māris Fogels]

Fix the Tarmac plugin activation code so plugins in .config are recognized.

Hi,

This branch presents a quick and dirty fix to the Tarmac plugin activation code. It adds the Tarmac plugin directories to the system path so that Tarmac can import the plugins.

There may be a better way to do this, but I am leaving that better implementation for someone else to tackle. The code passed manual testing on my local system.

Maris

[Māris Fogels (mars)]

Self-approved for the Launchpad Tarmac trial and landing on the lp-tarmac fork.

review: Approve

=== modified file 'tarmac/plugin.py'
--- tarmac/plugin.py	2010-09-02 15:18:07 +0000
+++ tarmac/plugin.py	2010-12-16 02:21:27 +0000
@@ -18,6 +18,7 @@
 
 import imp
 import os
+import sys
 
 from tarmac import plugins as _mod_plugins
 
@@ -40,6 +41,9 @@
 
     plugin_names = set()
     for path in TARMAC_PLUGIN_PATHS:
+        if path not in sys.path:
+            sys.path.insert(0, path)
+
         try:
             for _file in os.listdir(path):
                 full_path = os.path.join(path, _file)
@@ -75,6 +79,6 @@
 
     for name in plugin_names:
         try:
-            exec 'import tarmac.plugins.%s' % name in {}
+            exec 'import %s' % name in {}
         except KeyboardInterrupt:
             raise

=== modified file 'tarmac/plugins/allowedcontributors.py'
--- tarmac/plugins/allowedcontributors.py	2010-09-20 18:25:54 +0000
+++ tarmac/plugins/allowedcontributors.py	2010-12-16 02:21:27 +0000
@@ -23,6 +23,10 @@
     """Error for when a contributor does not meet validation requirements."""
 
 
+class InvalidPersonOrTeam(TarmacMergeError):
+    """Error for when a required team could not be found."""
+
+
 class AllowedContributors(TarmacPlugin):
     """Tarmac plug-in for checking whether a contributor is allowed to.
 
@@ -52,9 +56,25 @@
                 in_team = False
                 for team in self.allowed_contributors:
                     launchpad = command.launchpad
-                    lp_members = launchpad.people[team].getMembersByStatus(
-                        status=[u'Approved', u'Administrator'])
-                    members = [x.name for x in lp_members]
+                    try:
+                        lp_members = launchpad.people[team].getMembersByStatus(
+                            status=u'Approved')
+                        lp_members.extend(
+                            launchpad.people[team].getMembersByStatus(
+                                status=u'Administrator'))
+                        members = [x.name for x in lp_members]
+                    except KeyError:
+                        message = (u'Could not find person or team "%s" on '
+                                   u'Launchpad.' % team)
+                        comment = (u'Merging into %(target) requires that '
+                                   u'contributing authors be a member of an '
+                                   u'acceptable team, or a specified person. '
+                                   u'However, the person or team "%(team)s" '
+                                   u'was not found on Launchpad.' % {
+                                'target': proposal.target_branch.display_name,
+                                'team': team})
+                        raise InvalidPersonOrTeam(message, comment)
+
                     if author in members:
                         in_team = True
                         break

=== modified file 'tarmac/plugins/tests/test_allowedcontributors.py'
--- tarmac/plugins/tests/test_allowedcontributors.py	2010-09-20 18:25:54 +0000
+++ tarmac/plugins/tests/test_allowedcontributors.py	2010-12-16 02:21:27 +0000
@@ -16,7 +16,7 @@
 """Tests for the Allowed Contributors plug-in."""
 
 from tarmac.plugins.allowedcontributors import (
-    InvalidContributor, AllowedContributors)
+    InvalidContributor, InvalidPersonOrTeam, AllowedContributors)
 from tarmac.tests import TarmacTestCase
 from tarmac.tests.mock import Thing
 
@@ -47,7 +47,7 @@
                    Thing(name=u'person3', status=u'Approved'),
                    Thing(name=u'person8', status=u'Invited'),
                    Thing(name=u'personX', status=u'Expired')]
-        return [x for x in members if x.status in status]
+        return [x for x in members if x.status == status]
 
     def noMembersByStatus(self, status=None):
         """Fake method to not return any members for people."""
@@ -74,3 +74,16 @@
                           self.plugin.run,
                           command=command, target=target, source=source,
                           proposal=self.proposal)
+
+    def test_run_private_team(self):
+        """Test that the plug-in runs correctly."""
+        config = Thing(allowed_contributors=u'private_team')
+        source = Thing(authors=[u'person1', u'person2', u'person3'])
+        target = Thing(config=config)
+        launchpad = Thing(people=self.people)
+        command = Thing(launchpad=launchpad)
+        self.assertRaises(InvalidPersonOrTeam,
+                          self.plugin.run,
+                          command=command, target=target, source=source,
+                          proposal=self.proposal)
+