Bug 108262 - Tidy up Sheriffbot's Sheriffs command's unit tests
Summary: Tidy up Sheriffbot's Sheriffs command's unit tests
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alan Cutter
Depends on:
Reported: 2013-01-29 17:08 PST by Alan Cutter
Modified: 2013-01-29 19:02 PST (History)
4 users (show)

See Also:

Patch (6.75 KB, patch)
2013-01-29 18:12 PST, Alan Cutter
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alan Cutter 2013-01-29 17:08:14 PST
From bug 105698.

(In reply to comment #19)
> (From update of attachment 181006 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=181006&action=review
> > Tools/Scripts/webkitpy/tool/bot/irc_command.py:147
> > +            sheriff_js = Web().get_binary(url, True)
> rather than creating a Web() object, you should've used the web member of the tool object passed to you in execute(). Then you could've used tests that had a mocked out web object, rather than fetching test files. Generally speaking, we prefer tests that are self-contained with inline data; using the test files means hitting the filesystem which is slow and a bit more cumbersome.
> > Tools/Scripts/webkitpy/tool/bot/testdata/webkit_sheriff_zero.js:1
> > +document.write('');
> I don't this file is actually being used, is it?
Comment 1 Alan Cutter 2013-01-29 18:12:58 PST
Created attachment 185366 [details]
Comment 2 Eric Seidel (no email) 2013-01-29 18:18:45 PST
Comment on attachment 185366 [details]

LGTM.  I'm glad you've embraced the beauty of mocks. :)
Comment 3 WebKit Review Bot 2013-01-29 19:02:22 PST
Comment on attachment 185366 [details]

Clearing flags on attachment: 185366

Committed r141206: <http://trac.webkit.org/changeset/141206>
Comment 4 WebKit Review Bot 2013-01-29 19:02:26 PST
All reviewed patches have been landed.  Closing bug.