Changeset 52628 in webkit


Ignore:
Timestamp:
Dec 29, 2009 11:33:01 AM (14 years ago)
Author:
eric@webkit.org
Message:

2009-12-29 Eric Seidel <​eric@webkit.org>

Reviewed by Adam Barth.

Add the start of a Bug object for bugzilla.py
​https://bugs.webkit.org/show_bug.cgi?id=32995

This allowed us to get rid of some duplicated "is_obsolete" checks.

  • Scripts/modules/bugzilla.py:
    • Add a new Bug class, and move patches/unreviewed_patches filtering logic there.
    • Add _fetch_bug_page for possible future mocking. (I did not try to test fetch_*_from_bug now due to difficulties with our current validate_reviewer logic.)
    • Rename fetch_bug to fetch_bug_dictionary and add a new fetch_bug which returns a Bug object.
    • Use fetch_bug and attachments(), patches(), etc. instead of custom fetch_*_from_bug methods.
    • Reduce code in fetch_patches_from_pending_commit_list and fetch_patches_from_review_queue using list comprehensions. Use a sum(list, []) trick to flatten a list of lists into a single list.
  • Scripts/modules/bugzilla_unittest.py:
    • Remove an unneeded unicode string marker.
  • Scripts/modules/buildsteps.py:
    • define all to include just the BuildSteps
  • Scripts/modules/commands/download.py:
    • import * now that we have an all defined.
  • Scripts/modules/commands/upload.py:
    • Use fetch_bug_dictionary instead of fetch_bug.
Location:
trunk/WebKitTools
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/WebKitTools/ChangeLog

    r52626 r52628  
     12009-12-29  Eric Seidel  <eric@webkit.org>
     2
     3        Reviewed by Adam Barth.
     4
     5        Add the start of a Bug object for bugzilla.py
     6        https://bugs.webkit.org/show_bug.cgi?id=32995
     7
     8        This allowed us to get rid of some duplicated "is_obsolete" checks.
     9
     10        * Scripts/modules/bugzilla.py:
     11         - Add a new Bug class, and move patches/unreviewed_patches filtering logic there.
     12         - Add _fetch_bug_page for possible future mocking.
     13           (I did not try to test fetch_*_from_bug now due to difficulties with our current validate_reviewer logic.)
     14         - Rename fetch_bug to fetch_bug_dictionary and add a new fetch_bug which returns a Bug object.
     15         - Use fetch_bug and attachments(), patches(), etc. instead of custom fetch_*_from_bug methods.
     16         - Reduce code in fetch_patches_from_pending_commit_list and fetch_patches_from_review_queue
     17           using list comprehensions. Use a sum(list, []) trick to flatten a list of lists into a single list.
     18        * Scripts/modules/bugzilla_unittest.py:
     19         - Remove an unneeded unicode string marker.
     20        * Scripts/modules/buildsteps.py:
     21         - define __all__ to include just the BuildSteps
     22        * Scripts/modules/commands/download.py:
     23         - import * now that we have an __all__ defined.
     24        * Scripts/modules/commands/upload.py:
     25         - Use fetch_bug_dictionary instead of fetch_bug.
     26
    1272009-12-29  Daniel Bates  <dbates@webkit.org>
    228

  • trunk/WebKitTools/Scripts/modules/bugzilla.py

    r52595 r52628  
    6464
    6565
     66# FIXME: This class is kinda a hack for now.  It exists so we have one place
     67# to hold bug logic, even if much of the code deals with dictionaries still.
     68class Bug(object):
     69    def __init__(self, bug_dictionary):
     70        self.bug_dictionary = bug_dictionary
     71
     72    # Rarely do we actually want obsolete attachments
     73    def attachments(self, include_obsolete=False):
     74        if include_obsolete:
     75            return self.bug_dictionary["attachments"][:] # Return a copy in either case.
     76        return [attachment for attachment in self.bug_dictionary["attachments"] if not attachment["is_obsolete"]]
     77
     78    def patches(self, include_obsolete=False):
     79        return [patch for patch in self.attachments(include_obsolete) if patch["is_patch"]]
     80
     81    def unreviewed_patches(self):
     82        return [patch for patch in self.patches() if patch.get("review") == "?"]
     83
     84
    6685class Bugzilla(object):
    6786    def __init__(self, dryrun=False, committers=CommitterList()):
    … …  
    6988        self.authenticated = False
    7089
     90        # FIXME: We should use some sort of Browser mock object when in dryrun mode (to prevent any mistakes).
    7191        self.browser = Browser()
    7292        # Ignore bugs.webkit.org/robots.txt until we fix it to allow this script
    … …  
    124144        return bug
    125145
    126     def fetch_bug(self, bug_id):
     146    # Makes testing fetch_*_from_bug() possible until we have a better BugzillaNetwork abstration.
     147    def _fetch_bug_page(self, bug_id):
    127148        bug_url = self.bug_url_for_bug_id(bug_id, xml=True)
    128149        log("Fetching: %s" % bug_url)
    129         page = self.browser.open(bug_url)
    130         return self._parse_bug_page(page)
    131 
    132     # This should be an attachments() method on a Bug object.
    133     def fetch_attachments_from_bug(self, bug_id):
    134         bug = self.fetch_bug(bug_id)
    135         if not bug:
    136             return None
    137         return bug["attachments"]
     150        return self.browser.open(bug_url)
     151
     152    def fetch_bug_dictionary(self, bug_id):
     153        return self._parse_bug_page(self._fetch_bug_page(bug_id))
     154
     155    # FIXME: A BugzillaCache object should provide all these fetch_ methods.
     156    def fetch_bug(self, bug_id):
     157        return Bug(self.fetch_bug_dictionary(bug_id))
    138158
    139159    def _parse_bug_id_from_attachment_page(self, page):
    … …  
    158178        if not bug_id:
    159179            return None
    160         attachments = self.fetch_attachments_from_bug(bug_id)
    161         for attachment in attachments:
     180        for attachment in self.fetch_bug(bug_id).attachments(include_obsolete=True):
    162181            # FIXME: Once we have a real Attachment class we shouldn't paper over this possible comparison failure
    163182            # and we should remove the int() == int() hacks and leave it just ==.
    … …  
    167186        return None # This should never be hit.
    168187
     188    # fetch_patches_from_bug exists until we expose a Bug class outside of bugzilla.py
    169189    def fetch_patches_from_bug(self, bug_id):
    170         return [patch for patch in self.fetch_attachments_from_bug(bug_id) if patch['is_patch'] and not patch['is_obsolete']]
     190        return self.fetch_bug(bug_id).patches()
    171191
    172192    # _view_source_link belongs in some sort of webkit_config.py module.
    … …  
    216236        self._validate_committer(patch, reject_invalid_patches=False)
    217237
    218     def fetch_unreviewed_patches_from_bug(self, bug_id):
    219         return [patch for patch in self.fetch_attachments_from_bug(bug_id) if patch.get('review') == '?' and not patch['is_obsolete']]
    220 
    221238    # FIXME: fetch_reviewed_patches_from_bug and fetch_commit_queue_patches_from_bug
    222239    # should share more code and use list comprehensions.
    223240    def fetch_reviewed_patches_from_bug(self, bug_id, reject_invalid_patches=False):
    224241        reviewed_patches = []
    225         for attachment in self.fetch_attachments_from_bug(bug_id):
    226             if self._validate_reviewer(attachment, reject_invalid_patches) and not attachment['is_obsolete']:
     242        for attachment in self.fetch_bug(bug_id).attachments():
     243            if self._validate_reviewer(attachment, reject_invalid_patches):
    227244                reviewed_patches.append(attachment)
    228245        return reviewed_patches
    … …  
    231248        commit_queue_patches = []
    232249        for attachment in self.fetch_reviewed_patches_from_bug(bug_id, reject_invalid_patches):
    233             if self._validate_committer(attachment, reject_invalid_patches) and not attachment['is_obsolete']:
     250            if self._validate_committer(attachment, reject_invalid_patches):
    234251                commit_queue_patches.append(attachment)
    235252        return commit_queue_patches
    … …  
    275292
    276293    def fetch_patches_from_pending_commit_list(self):
    277         patches_needing_commit = []
    278         for bug_id in self.fetch_bug_ids_from_needs_commit_list():
    279             patches = self.fetch_reviewed_patches_from_bug(bug_id)
    280             patches_needing_commit += patches
    281         return patches_needing_commit
     294        return sum([self.fetch_reviewed_patches_from_bug(bug_id) for bug_id in self.fetch_bug_ids_from_needs_commit_list()], [])
    282295
    283296    def fetch_patches_from_review_queue(self, limit=None):
    284         patches_to_review = []
    285         for bug_id in self.fetch_bug_ids_from_review_queue():
    286             if limit and len(patches_to_review) >= limit:
    287                 break
    288             patches = self.fetch_unreviewed_patches_from_bug(bug_id)
    289             patches_to_review += patches
    290         return patches_to_review
     297        return sum([self.fetch_bug(bug_id).unreviewed_patches() for bug_id in self.fetch_bug_ids_from_review_queue()[:limit]], []) # [:None] returns the whole array.
    291298
    292299    def authenticate(self):
    … …  
    445452        self._set_flag_on_attachment(attachment_id, 'review', '-', comment_text, additional_comment_text)
    446453
     454    # FIXME: All of these bug editing methods have a ridiculous amount of copy/paste code.
    447455    def obsolete_attachment(self, attachment_id, comment_text = None):
    448456        self.authenticate()

  • trunk/WebKitTools/Scripts/modules/bugzilla_unittest.py

    r52595 r52628  
    180180            'url': 'https://bugs.webkit.org/attachment.cgi?id=45548',
    181181            'is_obsolete': False,
    182             'review': u'?',
     182            'review': '?',
    183183            'is_patch': True,
    184184            'attacher_email': 'mjs@apple.com',

  • trunk/WebKitTools/Scripts/modules/buildsteps.py

    r52599 r52628  
    3838from modules.changelogs import ChangeLog
    3939
     40# FIXME: Why do some of these have "Step" in their name but not all?
     41__all__ = [
     42    "ApplyPatchStep",
     43    "ApplyPatchWithLocalCommitStep",
     44    "BuildStep",
     45    "CheckStyleStep",
     46    "CleanWorkingDirectoryStep",
     47    "CleanWorkingDirectoryWithLocalCommitsStep",
     48    "CloseBugForLandDiffStep",
     49    "CloseBugStep",
     50    "ClosePatchStep",
     51    "CommitStep",
     52    "CompleteRollout",
     53    "CreateBugStep",
     54    "EnsureBuildersAreGreenStep",
     55    "EnsureLocalCommitIfNeeded",
     56    "ObsoletePatchesOnBugStep",
     57    "PostDiffToBugStep",
     58    "PrepareChangeLogForRevertStep",
     59    "PrepareChangeLogStep",
     60    "PromptForBugOrTitleStep",
     61    "RevertRevisionStep",
     62    "RunTestsStep",
     63    "UpdateChangeLogsWithReviewerStep",
     64    "UpdateStep",
     65]
    4066
    4167class CommandOptions(object):

  • trunk/WebKitTools/Scripts/modules/commands/download.py

    r52528 r52628  
    3434
    3535from modules.bugzilla import parse_bug_id
    36 # FIXME: This list is rediculous.  We need to learn the ways of __all__.
    37 from modules.buildsteps import CommandOptions, EnsureBuildersAreGreenStep, EnsureLocalCommitIfNeeded, UpdateChangeLogsWithReviewerStep, CleanWorkingDirectoryStep, CleanWorkingDirectoryWithLocalCommitsStep, UpdateStep, ApplyPatchStep, ApplyPatchWithLocalCommitStep, BuildStep, CheckStyleStep, RunTestsStep, CommitStep, ClosePatchStep, CloseBugStep, CloseBugForLandDiffStep, PrepareChangeLogForRevertStep, RevertRevisionStep, CompleteRollout
     36# We could instead use from modules import buildsteps and then prefix every buildstep with "buildsteps."
     37from modules.buildsteps import *
    3838from modules.changelogs import ChangeLog
    3939from modules.comments import bug_comment_from_commit_text

  • trunk/WebKitTools/Scripts/modules/commands/upload.py

    r52600 r52628  
    251251            (bug_id, svn_revision) = self._determine_bug_id_and_svn_revision(tool, bug_id, svn_revision)
    252252
    253         log("Bug: <%s> %s" % (tool.bugs.short_bug_url_for_bug_id(bug_id), tool.bugs.fetch_bug(bug_id)["title"]))
     253        log("Bug: <%s> %s" % (tool.bugs.short_bug_url_for_bug_id(bug_id), tool.bugs.fetch_bug_dictionary(bug_id)["title"]))
    254254        log("Revision: %s" % svn_revision)
    255255
Note: See TracChangeset for help on using the changeset viewer.