[Webkit-unassigned] [Bug 54374] Add method to make BuildBot return test outputs

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=54374





--- Comment #14 from James Kozianski <koz at chromium.org>  2011-03-06 19:50:58 PST ---
(In reply to comment #12)
> (From update of attachment 83604 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=83604&action=review
> 
> > Tools/Scripts/webkitpy/common/net/buildbot/buildbot.py:98
> > +        revision_build_pairs = self.revision_build_pairs_with_results()
> 
> I don't remember.  Isn't this very expensive to compute?

_fetch_revision_to_build_map() is expensive, but its result is cached, and that's what revision_build_pairs_with_results() uses.

> 
> > Tools/Scripts/webkitpy/common/net/buildbot/buildbot.py:232
> > +    def results_zip_url(self):
> 
> Please test this.

Done.

> 
> > Tools/Scripts/webkitpy/common/net/buildbot/buildbot_unittest.py:418
> > +        class MockBuilder(Builder):
> 
> No need to make this local to the test.  These are very often shared between more than one test.

I've removed this class.

> 
> > Tools/Scripts/webkitpy/common/net/buildbot/buildbot_unittest.py:420
> > +                Builder.__init__(self, 'mock builder', BuildBot())
> 
> Mocks shouldn't inherit from real objets.  If you want to test hte real object, just mock out the methods.

Done.

> 
> > Tools/Scripts/webkitpy/common/net/buildbot/buildbot_unittest.py:422
> > +            def _fetch_build(self, build_number):
> 
> I would have probably just replaced the methods on a real object.

Done.

> 
> > Tools/Scripts/webkitpy/common/net/buildbot/buildbot_unittest.py:425
> > +                else:
> 
> No else after return in WEbKIt.

Done.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.