106754 2013-01-13 21:28:22 -0800 Sheriffbot command aliases need test cases 2013-01-14 01:31:32 -0800 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 106628 1 alancutter alancutter abarth dpranke eric noel.gordon webkit.review.bot oldest_to_newest 806068 0 alancutter 2013-01-13 21:28:22 -0800 Currently there are no automated tests for Sheriffbot command aliases. 806077 1 182506 alancutter 2013-01-13 22:02:28 -0800 Created attachment 182506 Patch 806082 2 182506 steveblock 2013-01-13 22:09:25 -0800 Comment on attachment 182506 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? 806083 3 alancutter 2013-01-13 22:14:21 -0800 (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. 806086 4 182506 eric 2013-01-13 22:30:07 -0800 Comment on attachment 182506 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. :) 806114 5 182518 alancutter 2013-01-13 23:35:15 -0800 Created attachment 182518 Patch 806120 6 182518 eric 2013-01-13 23:56:08 -0800 Comment on attachment 182518 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. 806145 7 alancutter 2013-01-14 01:14:58 -0800 (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. 806153 8 182518 eric 2013-01-14 01:24:54 -0800 Comment on attachment 182518 Patch Thanks agian. 806159 9 182518 webkit.review.bot 2013-01-14 01:31:28 -0800 Comment on attachment 182518 Patch Clearing flags on attachment: 182518 Committed r139592: <http://trac.webkit.org/changeset/139592> 806160 10 webkit.review.bot 2013-01-14 01:31:32 -0800 All reviewed patches have been landed. Closing bug. 182506 2013-01-13 22:02:28 -0800 2013-01-13 23:35:11 -0800 Patch bug-106754-20130114165929.patch text/plain 2104 alancutter U3VidmVyc2lvbiBSZXZpc2lvbjogMTM5NTgwCmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggMDU0MzYxN2JkNzY2ZjRiNGRhNTk0MjhlMmY4ZjgxOWE3 MzI4NzNiMy4uOWJmYjgxMmNmNGMyZjhhOTY1NjQ2YjMzZWZmMDJjMmZiMmIyNmRmYyAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDE1 IEBACiAyMDEzLTAxLTEzICBBbGFuIEN1dHRlciAgPGFsYW5jdXR0ZXJAY2hyb21pdW0ub3JnPgog CisgICAgICAgIFNoZXJpZmZib3QgY29tbWFuZCBhbGlhc2VzIG5lZWQgdGVzdCBjYXNlcworICAg ICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTA2NzU0CisKKyAg ICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBTY3JpcHRzL3dl YmtpdHB5L3Rvb2wvY29tbWFuZHMvc2hlcmlmZmJvdF91bml0dGVzdC5weToKKyAgICAgICAgKFNo ZXJpZmZCb3RUZXN0LnRlc3RfY29tbWFuZF9hbGlhc2VzKToKKworMjAxMy0wMS0xMyAgQWxhbiBD dXR0ZXIgIDxhbGFuY3V0dGVyQGNocm9taXVtLm9yZz4KKwogICAgICAgICBNYWtlICJnYXJkZW5l cnMiIGFuIGFsaWFzIGZvciAic2hlcmlmZnMiIGluIHNoZXJyaWZib3QKICAgICAgICAgaHR0cHM6 Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEwNjYyNwogCmRpZmYgLS1naXQgYS9U b29scy9TY3JpcHRzL3dlYmtpdHB5L3Rvb2wvY29tbWFuZHMvc2hlcmlmZmJvdF91bml0dGVzdC5w eSBiL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvdG9vbC9jb21tYW5kcy9zaGVyaWZmYm90X3VuaXR0 ZXN0LnB5CmluZGV4IDlhYTU3YjEyMzJjMDY3OTY0MDVhN2U1NDgwNGRiZjlmZWExMzg2NWMuLmE3 MDc5YzZlZGZjNDM1ZDhlNWFkNDQ0MzNmMWMyZmFmMzI3MWYzMGUgMTAwNjQ0Ci0tLSBhL1Rvb2xz L1NjcmlwdHMvd2Via2l0cHkvdG9vbC9jb21tYW5kcy9zaGVyaWZmYm90X3VuaXR0ZXN0LnB5Cisr KyBiL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvdG9vbC9jb21tYW5kcy9zaGVyaWZmYm90X3VuaXR0 ZXN0LnB5CkBAIC0yNiw4ICsyNiwyMSBAQAogIyAoSU5DTFVESU5HIE5FR0xJR0VOQ0UgT1IgT1RI RVJXSVNFKSBBUklTSU5HIElOIEFOWSBXQVkgT1VUIE9GIFRIRSBVU0UKICMgT0YgVEhJUyBTT0ZU V0FSRSwgRVZFTiBJRiBBRFZJU0VEIE9GIFRIRSBQT1NTSUJJTElUWSBPRiBTVUNIIERBTUFHRS4K IAotZnJvbSB3ZWJraXRweS50b29sLmNvbW1hbmRzLnF1ZXVlc3Rlc3QgaW1wb3J0IFF1ZXVlc1Rl c3QKK2Zyb20gd2Via2l0cHkudG9vbC5jb21tYW5kcy5xdWV1ZXN0ZXN0IGltcG9ydCBRdWV1ZXNU ZXN0LCBNb2NrUXVldWVFbmdpbmUKK2Zyb20gd2Via2l0cHkudG9vbC5jb21tYW5kcyBpbXBvcnQg U2hlcmlmZkJvdAorZnJvbSB3ZWJraXRweS50b29sLm1vY2t0b29sIGltcG9ydCBNb2NrVG9vbCwg TW9ja09wdGlvbnMKK2Zyb20gd2Via2l0cHkudG9vbC5ib3QuaXJjX2NvbW1hbmQgaW1wb3J0IFNo ZXJpZmZzCiAKIAogY2xhc3MgU2hlcmlmZkJvdFRlc3QoUXVldWVzVGVzdCk6Ci0gICAgcGFzcyAg IyBObyB1bml0dGVzdHMgYXMgdGhlIG1vbWVudC4KKyAgICBkZWYgdGVzdF9jb21tYW5kX2FsaWFz ZXMoc2VsZik6CisgICAgICAgIHRvb2wgPSBNb2NrVG9vbCgpCisgICAgICAgIG9wdGlvbnMgPSBN b2NrT3B0aW9ucygpCisgICAgICAgIG9wdGlvbnMuZW5zdXJlX3ZhbHVlKCJjb25maXJtIiwgRmFs c2UpCisgICAgICAgIHNoZXJpZmZib3QgPSBTaGVyaWZmQm90KCkKKyAgICAgICAgc2hlcmlmZmJv dC5leGVjdXRlKG9wdGlvbnMsIFtdLCB0b29sLCBNb2NrUXVldWVFbmdpbmUpCisgICAgICAgIHNo ZXJpZmZib3QuYmVnaW5fd29ya19xdWV1ZSgpCisgICAgICAgIGlyY19ib3QgPSBzaGVyaWZmYm90 Ll9pcmNfYm90CisgICAgICAgICMgVGVzdCBTaGVyaWZmcyBjb21tYW5kIGFsaWFzZXMKKyAgICAg ICAgY29tbWFuZCwgYXJncyA9IGlyY19ib3QuX3BhcnNlX2NvbW1hbmRfYW5kX2FyZ3MoImdhcmRl bmVycyIpCisgICAgICAgIHNlbGYuYXNzZXJ0RXF1YWwoY29tbWFuZCwgU2hlcmlmZnMpCg== 182518 2013-01-13 23:35:15 -0800 2013-01-14 01:31:28 -0800 Patch bug-106754-20130114183215.patch text/plain 2700 alancutter U3VidmVyc2lvbiBSZXZpc2lvbjogMTM5NTg1CmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggMGNjODhiZGY2N2RiNDNkMjhmZWZhYmQ0MDE0MDE1NWM0 NTEyOTJmYy4uNGFmMTZmMmU5ZjQyMTAyMWViYTQ3MmVhZTYxNGIyZjZjZjM4OTk1MCAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDE1 IEBACiAyMDEzLTAxLTEzICBBbGFuIEN1dHRlciAgPGFsYW5jdXR0ZXJAY2hyb21pdW0ub3JnPgog CisgICAgICAgIFNoZXJpZmZib3QgY29tbWFuZCBhbGlhc2VzIG5lZWQgdGVzdCBjYXNlcworICAg ICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTA2NzU0CisKKyAg ICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBTY3JpcHRzL3dl YmtpdHB5L3Rvb2wvY29tbWFuZHMvc2hlcmlmZmJvdF91bml0dGVzdC5weToKKyAgICAgICAgKFNo ZXJpZmZCb3RUZXN0LnRlc3RfY29tbWFuZF9hbGlhc2VzKToKKworMjAxMy0wMS0xMyAgQWxhbiBD dXR0ZXIgIDxhbGFuY3V0dGVyQGNocm9taXVtLm9yZz4KKwogICAgICAgICBBbGxvdyB1c2VycyB0 byBtaXNzcGVsbCB0aGUgInNoZXJpZmZzIiBjb21tYW5kIGluIHNoZXJpZmZib3QKICAgICAgICAg aHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEwNjYyOAogCmRpZmYgLS1n aXQgYS9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L3Rvb2wvY29tbWFuZHMvc2hlcmlmZmJvdF91bml0 dGVzdC5weSBiL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvdG9vbC9jb21tYW5kcy9zaGVyaWZmYm90 X3VuaXR0ZXN0LnB5CmluZGV4IDlhYTU3YjEyMzJjMDY3OTY0MDVhN2U1NDgwNGRiZjlmZWExMzg2 NWMuLjYzMDU3ZWZmYmYwMTdlMzU5NGM1YmU2MjJmM2E1ZmI0ZmY3OTFmYzcgMTAwNjQ0Ci0tLSBh L1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvdG9vbC9jb21tYW5kcy9zaGVyaWZmYm90X3VuaXR0ZXN0 LnB5CisrKyBiL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvdG9vbC9jb21tYW5kcy9zaGVyaWZmYm90 X3VuaXR0ZXN0LnB5CkBAIC0yNiw4ICsyNiwzMCBAQAogIyAoSU5DTFVESU5HIE5FR0xJR0VOQ0Ug T1IgT1RIRVJXSVNFKSBBUklTSU5HIElOIEFOWSBXQVkgT1VUIE9GIFRIRSBVU0UKICMgT0YgVEhJ UyBTT0ZUV0FSRSwgRVZFTiBJRiBBRFZJU0VEIE9GIFRIRSBQT1NTSUJJTElUWSBPRiBTVUNIIERB TUFHRS4KIAotZnJvbSB3ZWJraXRweS50b29sLmNvbW1hbmRzLnF1ZXVlc3Rlc3QgaW1wb3J0IFF1 ZXVlc1Rlc3QKK2Zyb20gd2Via2l0cHkudG9vbC5jb21tYW5kcy5xdWV1ZXN0ZXN0IGltcG9ydCBR dWV1ZXNUZXN0LCBNb2NrUXVldWVFbmdpbmUKK2Zyb20gd2Via2l0cHkudG9vbC5jb21tYW5kcyBp bXBvcnQgU2hlcmlmZkJvdAorZnJvbSB3ZWJraXRweS50b29sLm1vY2t0b29sIGltcG9ydCBNb2Nr VG9vbCwgTW9ja09wdGlvbnMKK2Zyb20gd2Via2l0cHkudG9vbC5ib3QuaXJjX2NvbW1hbmQgaW1w b3J0IFJvbGxvdXQsIFNoZXJpZmZzCiAKIAogY2xhc3MgU2hlcmlmZkJvdFRlc3QoUXVldWVzVGVz dCk6Ci0gICAgcGFzcyAgIyBObyB1bml0dGVzdHMgYXMgdGhlIG1vbWVudC4KKyAgICBkZWYgdGVz dF9jb21tYW5kX2FsaWFzZXMoc2VsZik6CisgICAgICAgIHRvb2wgPSBNb2NrVG9vbCgpCisgICAg ICAgIG9wdGlvbnMgPSBNb2NrT3B0aW9ucygpCisgICAgICAgIG9wdGlvbnMuZW5zdXJlX3ZhbHVl KCJjb25maXJtIiwgRmFsc2UpCisgICAgICAgIHNoZXJpZmZib3QgPSBTaGVyaWZmQm90KCkKKyAg ICAgICAgc2hlcmlmZmJvdC5leGVjdXRlKG9wdGlvbnMsIFtdLCB0b29sLCBNb2NrUXVldWVFbmdp bmUpCisgICAgICAgIHNoZXJpZmZib3QuYmVnaW5fd29ya19xdWV1ZSgpCisgICAgICAgIGlyY19i b3QgPSBzaGVyaWZmYm90Ll9pcmNfYm90CisgICAgICAgICMgVGVzdCBSb2xsb3V0IGNvbW1hbmQg YWxpYXNlcworICAgICAgICByZXZlcnRfY29tbWFuZCwgYXJncyA9IGlyY19ib3QuX3BhcnNlX2Nv bW1hbmRfYW5kX2FyZ3MoInJldmVydCIpCisgICAgICAgIHNlbGYuYXNzZXJ0RXF1YWwocmV2ZXJ0 X2NvbW1hbmQsIFJvbGxvdXQpCisgICAgICAgICMgVGVzdCBTaGVyaWZmcyBjb21tYW5kIGFsaWFz ZXMKKyAgICAgICAgZ2FyZGVuZXJzX2NvbW1hbmQsIGFyZ3MgPSBpcmNfYm90Ll9wYXJzZV9jb21t YW5kX2FuZF9hcmdzKCJnYXJkZW5lcnMiKQorICAgICAgICBzZWxmLmFzc2VydEVxdWFsKGdhcmRl bmVyc19jb21tYW5kLCBTaGVyaWZmcykKKyAgICAgICAgc2hlcmlmc19jb21tYW5kLCBhcmdzID0g aXJjX2JvdC5fcGFyc2VfY29tbWFuZF9hbmRfYXJncygic2hlcmlmcyIpCisgICAgICAgIHNlbGYu YXNzZXJ0RXF1YWwoc2hlcmlmc19jb21tYW5kLCBTaGVyaWZmcykKKyAgICAgICAgc2hlcnJpZnNf Y29tbWFuZCwgYXJncyA9IGlyY19ib3QuX3BhcnNlX2NvbW1hbmRfYW5kX2FyZ3MoInNoZXJyaWZz IikKKyAgICAgICAgc2VsZi5hc3NlcnRFcXVhbChzaGVycmlmc19jb21tYW5kLCBTaGVyaWZmcykK KyAgICAgICAgc2hlcnJpZmZzX2NvbW1hbmQsIGFyZ3MgPSBpcmNfYm90Ll9wYXJzZV9jb21tYW5k X2FuZF9hcmdzKCJzaGVycmlmZnMiKQorICAgICAgICBzZWxmLmFzc2VydEVxdWFsKHNoZXJyaWZm c19jb21tYW5kLCBTaGVyaWZmcykK