Sheriffbot command aliases need test cases

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Alan Cutter]

Currently there are no automated tests for Sheriffbot command aliases.

[Alan Cutter]

Created attachment 182506 [details]
Patch

[Steve Block]

Comment on attachment 182506 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=182506&action=review

> Tools/Scripts/webkitpy/tool/commands/sheriffbot_unittest.py:46
> +        self.assertEqual(command, Sheriffs)

Do you intend to add tests for the other sheriffbot commands too?

[Alan Cutter]

(In reply to comment #2)
> (From update of attachment 182506 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=182506&action=review
> 
> > Tools/Scripts/webkitpy/tool/commands/sheriffbot_unittest.py:46
> > +        self.assertEqual(command, Sheriffs)
> 
> Do you intend to add tests for the other sheriffbot commands too?

Yes, the other aliases had not landed at the time of writing. They have now so I'll add them.

[Eric Seidel (no email)]

Comment on attachment 182506 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=182506&action=review

> Tools/Scripts/webkitpy/tool/commands/sheriffbot_unittest.py:43
> +        tool = MockTool()
> +        options = MockOptions()
> +        options.ensure_value("confirm", False)
> +        sheriffbot = SheriffBot()
> +        sheriffbot.execute(options, [], tool, MockQueueEngine)
> +        sheriffbot.begin_work_queue()
> +        irc_bot = sheriffbot._irc_bot

Quite an epic amount of boiler plate. :)

[Alan Cutter]

Created attachment 182518 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 182518 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=182518&action=review

> Tools/Scripts/webkitpy/tool/commands/sheriffbot_unittest.py:42
> +        tool = MockTool()
> +        options = MockOptions()
> +        options.ensure_value("confirm", False)
> +        sheriffbot = SheriffBot()
> +        sheriffbot.execute(options, [], tool, MockQueueEngine)
> +        sheriffbot.begin_work_queue()

Next time we add tests, this should probably move to a setUp() method (which is automatically called for you)

> Tools/Scripts/webkitpy/tool/commands/sheriffbot_unittest.py:46
> +        revert_command, args = irc_bot._parse_command_and_args("revert")
> +        self.assertEqual(revert_command, Rollout)

I often add a _assertCommandNamed(name, command) or similar helper method when the asserts take more than one line to execute.

[Alan Cutter]

(In reply to comment #6)
> (From update of attachment 182518 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=182518&action=review
> 
> > Tools/Scripts/webkitpy/tool/commands/sheriffbot_unittest.py:42
> > +        tool = MockTool()
> > +        options = MockOptions()
> > +        options.ensure_value("confirm", False)
> > +        sheriffbot = SheriffBot()
> > +        sheriffbot.execute(options, [], tool, MockQueueEngine)
> > +        sheriffbot.begin_work_queue()
> 
> Next time we add tests, this should probably move to a setUp() method (which is automatically called for you)
> 
> > Tools/Scripts/webkitpy/tool/commands/sheriffbot_unittest.py:46
> > +        revert_command, args = irc_bot._parse_command_and_args("revert")
> > +        self.assertEqual(revert_command, Rollout)
> 
> I often add a _assertCommandNamed(name, command) or similar helper method when the asserts take more than one line to execute.

Ah yes, I did notice that pattern in the EWS tests. Next change to Sheriffbot will come with more abstraction in the tests.

[Eric Seidel (no email)]

Comment on attachment 182518 [details]
Patch

Thanks agian.

[WebKit Review Bot]

Comment on attachment 182518 [details]
Patch

Clearing flags on attachment: 182518

Committed r139592: <http://trac.webkit.org/changeset/139592>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.