Remove more direct uses of os.path
authoreric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 18 Nov 2011 00:23:28 +0000 (00:23 +0000)
committereric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 18 Nov 2011 00:23:28 +0000 (00:23 +0000)
https://bugs.webkit.org/show_bug.cgi?id=72677

Reviewed by Adam Barth.

Remove more uses of os.path, including refactoring
CommitterValidator to use a Host object instead of
manual hacks to emulate what SCM and FileSystem provide.

* Scripts/webkitpy/common/checkout/changelog.py:
* Scripts/webkitpy/common/config/committervalidator.py:
* Scripts/webkitpy/common/config/committervalidator_unittest.py:
* Scripts/webkitpy/common/system/filesystem_mock.py:
 - Turns out none of our unittests actually looked at this path.
   now I've made it be a valid path.
* Scripts/webkitpy/tool/bot/feeders.py:
* Scripts/webkitpy/tool/commands/queues.py:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@100697 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog
Tools/Scripts/webkitpy/common/checkout/changelog.py
Tools/Scripts/webkitpy/common/config/committervalidator.py
Tools/Scripts/webkitpy/common/config/committervalidator_unittest.py
Tools/Scripts/webkitpy/common/system/filesystem_mock.py
Tools/Scripts/webkitpy/tool/bot/feeders.py
Tools/Scripts/webkitpy/tool/commands/queues.py

index f214715..ed8e531 100644 (file)
@@ -1,5 +1,25 @@
 2011-11-17  Eric Seidel  <eric@webkit.org>
 
+        Remove more direct uses of os.path
+        https://bugs.webkit.org/show_bug.cgi?id=72677
+
+        Reviewed by Adam Barth.
+
+        Remove more uses of os.path, including refactoring
+        CommitterValidator to use a Host object instead of
+        manual hacks to emulate what SCM and FileSystem provide.
+
+        * Scripts/webkitpy/common/checkout/changelog.py:
+        * Scripts/webkitpy/common/config/committervalidator.py:
+        * Scripts/webkitpy/common/config/committervalidator_unittest.py:
+        * Scripts/webkitpy/common/system/filesystem_mock.py:
+         - Turns out none of our unittests actually looked at this path.
+           now I've made it be a valid path.
+        * Scripts/webkitpy/tool/bot/feeders.py:
+        * Scripts/webkitpy/tool/commands/queues.py:
+
+2011-11-17  Eric Seidel  <eric@webkit.org>
+
         Teach TextFileReader about FileSystem
         https://bugs.webkit.org/show_bug.cgi?id=72673
 
index 7c6b5f2..2f6c33b 100644 (file)
@@ -30,7 +30,6 @@
 
 import codecs
 import fileinput # inplace file editing for set_reviewer_in_changelog
-import os.path
 import re
 import textwrap
 
index e543373..6cec3da 100644 (file)
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-import os
-
-from webkitpy.common.system.ospath import relpath
 from webkitpy.common.config import committers, urls
 
 
 class CommitterValidator(object):
-
-    def __init__(self, bugzilla):
-        self._bugzilla = bugzilla
-
-    def _checkout_root(self):
-        # FIXME: This is a hack, we would have this from scm.checkout_root
-        # if we had any way to get to an scm object here.
-        components = __file__.split(os.sep)
-        tools_index = components.index("Tools")
-        return os.sep.join(components[:tools_index])
+    def __init__(self, host):
+        self.host = host
 
     def _committers_py_path(self):
         # extension can sometimes be .pyc, we always want .py
-        (path, extension) = os.path.splitext(committers.__file__)
-        # FIXME: When we're allowed to use python 2.6 we can use the real
-        # os.path.relpath
-        path = relpath(path, self._checkout_root())
+        committers_path = self.host.filesystem.path_to_module(committers.__name__)
+        (path, extension) = self.host.filesystem.splitext(committers_path)
+        path = self.host.filesystem.relpath(path, self.host.scm().checkout_root)
         return ".".join([path, "py"])
 
     def _flag_permission_rejection_message(self, setter_email, flag_name):
@@ -72,21 +60,16 @@ class CommitterValidator(object):
 
     def _validate_setter_email(self, patch, result_key, rejection_function):
         committer = getattr(patch, result_key)()
-        # If the flag is set, and we don't recognize the setter, reject the
-        # flag!
+        # If the flag is set, and we don't recognize the setter, reject the flag!
         setter_email = patch._attachment_dictionary.get("%s_email" % result_key)
         if setter_email and not committer:
-            rejection_function(patch.id(),
-                self._flag_permission_rejection_message(setter_email,
-                                                        result_key))
+            rejection_function(patch.id(), self._flag_permission_rejection_message(setter_email, result_key))
             return False
         return True
 
     def _reject_patch_if_flags_are_invalid(self, patch):
-        return (self._validate_setter_email(
-                patch, "reviewer", self.reject_patch_from_review_queue)
-            and self._validate_setter_email(
-                patch, "committer", self.reject_patch_from_commit_queue))
+        return (self._validate_setter_email(patch, "reviewer", self.reject_patch_from_review_queue)
+            and self._validate_setter_email(patch, "committer", self.reject_patch_from_commit_queue))
 
     def patches_after_rejecting_invalid_commiters_and_reviewers(self, patches):
         return [patch for patch in patches if self._reject_patch_if_flags_are_invalid(patch)]
@@ -95,7 +78,7 @@ class CommitterValidator(object):
                                        attachment_id,
                                        additional_comment_text=None):
         comment_text = "Rejecting attachment %s from commit-queue." % attachment_id
-        self._bugzilla.set_flag_on_attachment(attachment_id,
+        self.host.bugs.set_flag_on_attachment(attachment_id,
                                               "commit-queue",
                                               "-",
                                               comment_text,
@@ -105,7 +88,7 @@ class CommitterValidator(object):
                                        attachment_id,
                                        additional_comment_text=None):
         comment_text = "Rejecting attachment %s from review queue." % attachment_id
-        self._bugzilla.set_flag_on_attachment(attachment_id,
+        self.host.bugs.set_flag_on_attachment(attachment_id,
                                               'review',
                                               '-',
                                               comment_text,
index 58fd3a5..232f077 100644 (file)
 
 import unittest
 
+from webkitpy.common.host_mock import MockHost
 from .committervalidator import CommitterValidator
 
 
 class CommitterValidatorTest(unittest.TestCase):
     def test_flag_permission_rejection_message(self):
-        validator = CommitterValidator(bugzilla=None)
+        validator = CommitterValidator(MockHost())
         self.assertEqual(validator._committers_py_path(), "Tools/Scripts/webkitpy/common/config/committers.py")
         expected_messsage = """foo@foo.com does not have review permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.
 
index f4ea4ab..57dce3d 100644 (file)
@@ -87,7 +87,7 @@ class MockFileSystem(object):
         return home_directory + self.sep + parts[1]
 
     def path_to_module(self, module_name):
-        return "/mock-checkout/Tools/Scripts/webkitpy/%s" % module_name
+        return "/mock-checkout/Tools/Scripts/" + module_name.replace('.', '/') + ".py"
 
     def chdir(self, path):
         path = self.normpath(path)
index 0b7f23d..4ba2f04 100644 (file)
@@ -44,7 +44,7 @@ class CommitQueueFeeder(AbstractFeeder):
 
     def __init__(self, tool):
         AbstractFeeder.__init__(self, tool)
-        self.committer_validator = CommitterValidator(self._tool.bugs)
+        self.committer_validator = CommitterValidator(self._tool)
 
     def _update_work_items(self, item_ids):
         # FIXME: This is the last use of update_work_items, the commit-queue
index ce02d92..f61a639 100644 (file)
@@ -267,7 +267,7 @@ class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler, CommitQueueTaskD
 
     def begin_work_queue(self):
         AbstractPatchQueue.begin_work_queue(self)
-        self.committer_validator = CommitterValidator(self._tool.bugs)
+        self.committer_validator = CommitterValidator(self._tool)
         self._expected_failures = ExpectedFailures()
         self._layout_test_results_reader = LayoutTestResultsReader(self._tool, self._log_directory())
 
@@ -288,7 +288,7 @@ class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler, CommitQueueTaskD
                 return True
             self._did_retry(patch)
         except ScriptError, e:
-            validator = CommitterValidator(self._tool.bugs)
+            validator = CommitterValidator(self._tool)
             validator.reject_patch_from_commit_queue(patch.id(), self._error_message_for_bug(task, patch, e))
             results_archive = task.results_archive_from_patch_test_run(patch)
             if results_archive: