Bug 108262 – Tidy up Sheriffbot's Sheriffs command's unit tests

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Alan Cutter]

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?

[Alan Cutter]

Created attachment 185366 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 185366 [details]
Patch

LGTM.  I'm glad you've embraced the beauty of mocks. :)

[WebKit Review Bot]

Comment on attachment 185366 [details]
Patch

Clearing flags on attachment: 185366

Committed r141206: <http://trac.webkit.org/changeset/141206>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.