31769 2009-11-21 08:26:33 -0800 AbstractQueue needs better unit testing 2010-06-10 19:42:56 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) PC OS X 10.5 NEW P2 Normal --- 1 eric webkit-unassigned abarth oldest_to_newest 165617 0 eric 2009-11-21 08:26:33 -0800 AbstractQueue.run_bugzilla_tool throws an exception It should use self.tool instead of tool. Add a unit test to make sure this doesn't happen again. 165618 1 43650 eric 2009-11-21 08:30:02 -0800 Created attachment 43650 Patch 165619 2 43650 abarth 2009-11-21 08:33:15 -0800 Comment on attachment 43650 Patch i think you should subclass the real MockBugzillaTool and override the path method we want MockBugzillaTool to grow to be able to mock all the features of the real bugzilla tool 165620 3 eric 2009-11-21 08:47:12 -0800 I don' think it's right to add this to the MockBugzillaTool. use of echo is kinda a hack. I'm not sure we always want run_bugzilla_tool() to even return a value. And if it returns a value, I'm not sure it should be the output. So I'd rather keep this hack self contained instead of adding it to some "global mock". Also this is better locality of code putting it here, making the test easier to read. 165621 4 abarth 2009-11-21 08:49:52 -0800 I don't understand your argument. I'd rather see a test that mocked out both WorkQueue and BugzillaTool and didn't change run_bugzilla_tool to return a value. To test this bug, you don't actually need to validate that we created a subprocess. You just need to validate that we don't throw a bogus exception. 165622 5 abarth 2009-11-21 08:53:05 -0800 Also, how does your test run on Windows? I think you'd rather use the real bugzilla-tool with --help 165642 6 eric 2009-11-21 13:10:09 -0800 I'm going to land the 5 char fix to unblock the commit-queue and then we can debate the proper way to unit test this all in person. 165643 7 eric 2009-11-21 13:13:18 -0800 Committed r51284: <http://trac.webkit.org/changeset/51284> 165644 8 eric 2009-11-21 13:14:03 -0800 Leaving this open and changing the title to reflect that what's left to solve here is unit testing. 43650 2009-11-21 08:30:02 -0800 2010-06-10 19:42:56 -0700 Patch bug-31769-20091121083000.patch text/plain 5626 eric ZGlmZiAtLWdpdCBhL1dlYktpdFRvb2xzL0NoYW5nZUxvZyBiL1dlYktpdFRvb2xzL0NoYW5nZUxv ZwppbmRleCBjODk3MzcxLi5iYWU5ZTUyIDEwMDY0NAotLS0gYS9XZWJLaXRUb29scy9DaGFuZ2VM b2cKKysrIGIvV2ViS2l0VG9vbHMvQ2hhbmdlTG9nCkBAIC0xLDUgKzEsMjAgQEAKIDIwMDktMTEt MjEgIEVyaWMgU2VpZGVsICA8ZXJpY0B3ZWJraXQub3JnPgogCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEFic3RyYWN0UXVldWUucnVuX2J1Z3ppbGxhX3Rv b2wgdGhyb3dzIGFuIGV4Y2VwdGlvbgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9z aG93X2J1Zy5jZ2k/aWQ9MzE3NjkKKworICAgICAgICAqIFNjcmlwdHMvbW9kdWxlcy9jb21tYW5k cy9xdWV1ZXMucHk6CisgICAgICAgICAtIEZpeCBleGNlcHRpb24gaW4gcnVuX2J1Z3ppbGxhX3Rv b2wgYW5kIG1ha2UgaXQgcmV0dXJuIGEgcmVzdWx0IHRvIGJlIG1vcmUgdGVzdGFibGUuCisgICAg ICAgICogU2NyaXB0cy9tb2R1bGVzL2NvbW1hbmRzL3F1ZXVlc191bml0dGVzdC5weTogQWRkZWQu CisgICAgICAgICogU2NyaXB0cy9tb2R1bGVzL3dlYmtpdGxhbmRpbmdzY3JpcHRzLnB5OgorICAg ICAgICAgLSBNYWtlIHJ1bl9hbmRfdGhyb3dfaWZfZmFpbCByZXR1cm4gdGhlIG91dHB1dCBmb3Ig ZWFzaWVyIHRlc3RpbmcuCisgICAgICAgICogU2NyaXB0cy9ydW4td2Via2l0LXVuaXR0ZXN0czoK KyAgICAgICAgIC0gQWRkIHF1ZXVlc191bml0dGVzdC4KKworMjAwOS0xMS0yMSAgRXJpYyBTZWlk ZWwgIDxlcmljQHdlYmtpdC5vcmc+CisKICAgICAgICAgUmV2aWV3ZWQgYnkgQWRhbSBCYXJ0aC4K IAogICAgICAgICBjb21taXQtcXVldWUgZmFpbHMgdG8gcnVuIHdpdGggInBlcm1pc3Npb25zIGVy cm9yIiBkdWUgdG8gYmFkIGJ1Z3ppbGxhLXRvb2wgcGF0aApkaWZmIC0tZ2l0IGEvV2ViS2l0VG9v bHMvU2NyaXB0cy9tb2R1bGVzL2NvbW1hbmRzL3F1ZXVlcy5weSBiL1dlYktpdFRvb2xzL1Njcmlw dHMvbW9kdWxlcy9jb21tYW5kcy9xdWV1ZXMucHkKaW5kZXggZmVlN2Y2ZC4uZTU0MGQ4MyAxMDA2 NDQKLS0tIGEvV2ViS2l0VG9vbHMvU2NyaXB0cy9tb2R1bGVzL2NvbW1hbmRzL3F1ZXVlcy5weQor KysgYi9XZWJLaXRUb29scy9TY3JpcHRzL21vZHVsZXMvY29tbWFuZHMvcXVldWVzLnB5CkBAIC05 NSw4ICs5NSw4IEBAIGNsYXNzIEFic3RyYWN0UXVldWUoQ29tbWFuZCwgV29ya1F1ZXVlRGVsZWdh dGUpOgogICAgICAgICByYWlzZSBOb3RJbXBsZW1lbnRlZEVycm9yLCAic3ViY2xhc3NlcyBtdXN0 IGltcGxlbWVudCIKIAogICAgIGRlZiBydW5fYnVnemlsbGFfdG9vbChzZWxmLCBhcmdzKToKLSAg ICAgICAgYnVnemlsbGFfdG9vbF9hcmdzID0gW3Rvb2wucGF0aCgpXSArIGFyZ3MKLSAgICAgICAg V2ViS2l0TGFuZGluZ1NjcmlwdHMucnVuX2FuZF90aHJvd19pZl9mYWlsKGJ1Z3ppbGxhX3Rvb2xf YXJncykKKyAgICAgICAgYnVnemlsbGFfdG9vbF9hcmdzID0gW3NlbGYudG9vbC5wYXRoKCldICsg YXJncworICAgICAgICByZXR1cm4gV2ViS2l0TGFuZGluZ1NjcmlwdHMucnVuX2FuZF90aHJvd19p Zl9mYWlsKGJ1Z3ppbGxhX3Rvb2xfYXJncykKIAogICAgIGRlZiBsb2dfcHJvZ3Jlc3Moc2VsZiwg cGF0Y2hfaWRzKToKICAgICAgICAgbG9nKCIlcyBpbiAlcyBbJXNdIiAlIChwbHVyYWxpemUoInBh dGNoIiwgbGVuKHBhdGNoX2lkcykpLCBzZWxmLm5hbWUsICIsICIuam9pbihwYXRjaF9pZHMpKSkK ZGlmZiAtLWdpdCBhL1dlYktpdFRvb2xzL1NjcmlwdHMvbW9kdWxlcy9jb21tYW5kcy9xdWV1ZXNf dW5pdHRlc3QucHkgYi9XZWJLaXRUb29scy9TY3JpcHRzL21vZHVsZXMvY29tbWFuZHMvcXVldWVz X3VuaXR0ZXN0LnB5Cm5ldyBmaWxlIG1vZGUgMTAwNjQ0CmluZGV4IDAwMDAwMDAuLjgwYjFhZTcK LS0tIC9kZXYvbnVsbAorKysgYi9XZWJLaXRUb29scy9TY3JpcHRzL21vZHVsZXMvY29tbWFuZHMv cXVldWVzX3VuaXR0ZXN0LnB5CkBAIC0wLDAgKzEsNDMgQEAKKyMhL3Vzci9iaW4vZW52IHB5dGhv bgorIyBDb3B5cmlnaHQgKGMpIDIwMDksIEdvb2dsZSBJbmMuIEFsbCByaWdodHMgcmVzZXJ2ZWQu CisjCisjIFJlZGlzdHJpYnV0aW9uIGFuZCB1c2UgaW4gc291cmNlIGFuZCBiaW5hcnkgZm9ybXMs IHdpdGggb3Igd2l0aG91dAorIyBtb2RpZmljYXRpb24sIGFyZSBwZXJtaXR0ZWQgcHJvdmlkZWQg dGhhdCB0aGUgZm9sbG93aW5nIGNvbmRpdGlvbnMgYXJlCisjIG1ldDoKKyMgCisjICAgICAqIFJl ZGlzdHJpYnV0aW9ucyBvZiBzb3VyY2UgY29kZSBtdXN0IHJldGFpbiB0aGUgYWJvdmUgY29weXJp Z2h0CisjIG5vdGljZSwgdGhpcyBsaXN0IG9mIGNvbmRpdGlvbnMgYW5kIHRoZSBmb2xsb3dpbmcg ZGlzY2xhaW1lci4KKyMgICAgICogUmVkaXN0cmlidXRpb25zIGluIGJpbmFyeSBmb3JtIG11c3Qg cmVwcm9kdWNlIHRoZSBhYm92ZQorIyBjb3B5cmlnaHQgbm90aWNlLCB0aGlzIGxpc3Qgb2YgY29u ZGl0aW9ucyBhbmQgdGhlIGZvbGxvd2luZyBkaXNjbGFpbWVyCisjIGluIHRoZSBkb2N1bWVudGF0 aW9uIGFuZC9vciBvdGhlciBtYXRlcmlhbHMgcHJvdmlkZWQgd2l0aCB0aGUKKyMgZGlzdHJpYnV0 aW9uLgorIyAgICAgKiBOZWl0aGVyIHRoZSBuYW1lIG9mIEdvb2dsZSBJbmMuIG5vciB0aGUgbmFt ZXMgb2YgaXRzCisjIGNvbnRyaWJ1dG9ycyBtYXkgYmUgdXNlZCB0byBlbmRvcnNlIG9yIHByb21v dGUgcHJvZHVjdHMgZGVyaXZlZCBmcm9tCisjIHRoaXMgc29mdHdhcmUgd2l0aG91dCBzcGVjaWZp YyBwcmlvciB3cml0dGVuIHBlcm1pc3Npb24uCisjIAorIyBUSElTIFNPRlRXQVJFIElTIFBST1ZJ REVEIEJZIFRIRSBDT1BZUklHSFQgSE9MREVSUyBBTkQgQ09OVFJJQlVUT1JTCisjICJBUyBJUyIg QU5EIEFOWSBFWFBSRVNTIE9SIElNUExJRUQgV0FSUkFOVElFUywgSU5DTFVESU5HLCBCVVQgTk9U CisjIExJTUlURUQgVE8sIFRIRSBJTVBMSUVEIFdBUlJBTlRJRVMgT0YgTUVSQ0hBTlRBQklMSVRZ IEFORCBGSVRORVNTIEZPUgorIyBBIFBBUlRJQ1VMQVIgUFVSUE9TRSBBUkUgRElTQ0xBSU1FRC4g SU4gTk8gRVZFTlQgU0hBTEwgVEhFIENPUFlSSUdIVAorIyBPV05FUiBPUiBDT05UUklCVVRPUlMg QkUgTElBQkxFIEZPUiBBTlkgRElSRUNULCBJTkRJUkVDVCwgSU5DSURFTlRBTCwKKyMgU1BFQ0lB TCwgRVhFTVBMQVJZLCBPUiBDT05TRVFVRU5USUFMIERBTUFHRVMgKElOQ0xVRElORywgQlVUIE5P VAorIyBMSU1JVEVEIFRPLCBQUk9DVVJFTUVOVCBPRiBTVUJTVElUVVRFIEdPT0RTIE9SIFNFUlZJ Q0VTOyBMT1NTIE9GIFVTRSwKKyMgREFUQSwgT1IgUFJPRklUUzsgT1IgQlVTSU5FU1MgSU5URVJS VVBUSU9OKSBIT1dFVkVSIENBVVNFRCBBTkQgT04gQU5ZCisjIFRIRU9SWSBPRiBMSUFCSUxJVFks IFdIRVRIRVIgSU4gQ09OVFJBQ1QsIFNUUklDVCBMSUFCSUxJVFksIE9SIFRPUlQKKyMgKElOQ0xV RElORyBORUdMSUdFTkNFIE9SIE9USEVSV0lTRSkgQVJJU0lORyBJTiBBTlkgV0FZIE9VVCBPRiBU SEUgVVNFCisjIE9GIFRISVMgU09GVFdBUkUsIEVWRU4gSUYgQURWSVNFRCBPRiBUSEUgUE9TU0lC SUxJVFkgT0YgU1VDSCBEQU1BR0UuCisKK2ltcG9ydCB1bml0dGVzdAorCitmcm9tIG1vZHVsZXMu Y29tbWFuZHMucXVldWVzIGltcG9ydCAqCisKK2NsYXNzIE1vY2tUb29sKG9iamVjdCk6CisgICAg ZGVmIHBhdGgoc2VsZik6CisgICAgICAgIHJldHVybiAiL2Jpbi9lY2hvIgorCitjbGFzcyBBYnN0 cmFjdFF1ZXVlVGVzdCh1bml0dGVzdC5UZXN0Q2FzZSk6CisgICAgZGVmIHRlc3RfcnVuX2J1Z3pp bGxhX3Rvb2woc2VsZik6CisgICAgICAgIHF1ZXVlID0gQWJzdHJhY3RRdWV1ZSgpCisgICAgICAg IHF1ZXVlLnRvb2wgPSBNb2NrVG9vbCgpICMgTm9ybWFsbHkgc2V0IGR1cmluZyBleGVjdXRlKCkK KyAgICAgICAgIyBUaGUgb3V0cHV0IGRvZXNuJ3QgbWF0dGVyIGhlcmUgc28gbXVjaCBhcyBtYWtp bmcgc3VyZSBubyBleGNlcHRpb25zIGFyZSB0aHJvd24uCisgICAgICAgIHNlbGYuYXNzZXJ0RXF1 YWxzKHF1ZXVlLnJ1bl9idWd6aWxsYV90b29sKFsiZm9vIiwgImJhciJdKSwgImZvbyBiYXJcbiIp CmRpZmYgLS1naXQgYS9XZWJLaXRUb29scy9TY3JpcHRzL21vZHVsZXMvd2Via2l0bGFuZGluZ3Nj cmlwdHMucHkgYi9XZWJLaXRUb29scy9TY3JpcHRzL21vZHVsZXMvd2Via2l0bGFuZGluZ3Njcmlw dHMucHkKaW5kZXggZTYyNDUwNS4uZTdiZWYzNSAxMDA2NDQKLS0tIGEvV2ViS2l0VG9vbHMvU2Ny aXB0cy9tb2R1bGVzL3dlYmtpdGxhbmRpbmdzY3JpcHRzLnB5CisrKyBiL1dlYktpdFRvb2xzL1Nj cmlwdHMvbW9kdWxlcy93ZWJraXRsYW5kaW5nc2NyaXB0cy5weQpAQCAtOTYsNiArOTYsNyBAQCBj bGFzcyBXZWJLaXRMYW5kaW5nU2NyaXB0czoKICAgICAgICAgICAgICAgICByZXR1cm4gY2hpbGRf cHJvY2Vzcy5wb2xsKCkKICAgICAgICAgICAgIHRlZWRfb3V0cHV0LndyaXRlKG91dHB1dF9saW5l KQogCisgICAgIyBGSVhNRTogVGhpcyBzaG91bGQgYmUgdW5pZmllZCB3aXRoIFNDTS5ydW5fY29t bWFuZCgpIGFuZCBwbGFjZWQgaW4gaXRzIG93biBtb2R1bGUuCiAgICAgQHN0YXRpY21ldGhvZAog ICAgIGRlZiBydW5fYW5kX3Rocm93X2lmX2ZhaWwoYXJncywgcXVpZXQ9RmFsc2UpOgogICAgICAg ICAjIENhY2hlIHRoZSBjaGlsZCdzIG91dHB1dCBsb2NhbGx5IHNvIGl0IGNhbiBiZSB1c2VkIGZv ciBlcnJvciByZXBvcnRzLgpAQCAtMTEyLDYgKzExMyw3IEBAIGNsYXNzIFdlYktpdExhbmRpbmdT Y3JpcHRzOgogCiAgICAgICAgIGlmIGV4aXRfY29kZToKICAgICAgICAgICAgIHJhaXNlIFNjcmlw dEVycm9yKHNjcmlwdF9hcmdzPWFyZ3MsIGV4aXRfY29kZT1leGl0X2NvZGUsIG91dHB1dD1jaGls ZF9vdXRwdXQpCisgICAgICAgIHJldHVybiBjaGlsZF9vdXRwdXQKIAogICAgIEBjbGFzc21ldGhv ZAogICAgIGRlZiBydW5fd2Via2l0X3NjcmlwdChjbHMsIHNjcmlwdF9uYW1lLCBxdWlldD1GYWxz ZSwgcG9ydD1XZWJLaXRQb3J0KToKZGlmZiAtLWdpdCBhL1dlYktpdFRvb2xzL1NjcmlwdHMvcnVu LXdlYmtpdC11bml0dGVzdHMgYi9XZWJLaXRUb29scy9TY3JpcHRzL3J1bi13ZWJraXQtdW5pdHRl c3RzCmluZGV4IDkzM2JiZGMuLjVmNzc3OTkgMTAwNzU1Ci0tLSBhL1dlYktpdFRvb2xzL1Njcmlw dHMvcnVuLXdlYmtpdC11bml0dGVzdHMKKysrIGIvV2ViS2l0VG9vbHMvU2NyaXB0cy9ydW4td2Vi a2l0LXVuaXR0ZXN0cwpAQCAtMzMsNiArMzMsNyBAQCBmcm9tIG1vZHVsZXMuYnVnemlsbGFfdW5p dHRlc3QgaW1wb3J0ICoKIGZyb20gbW9kdWxlcy5idWlsZGJvdF91bml0dGVzdCBpbXBvcnQgKgog ZnJvbSBtb2R1bGVzLmNoYW5nZWxvZ3NfdW5pdHRlc3QgaW1wb3J0ICoKIGZyb20gbW9kdWxlcy5j b21tYW5kcy5xdWVyaWVzX3VuaXR0ZXN0IGltcG9ydCAqCitmcm9tIG1vZHVsZXMuY29tbWFuZHMu cXVldWVzX3VuaXR0ZXN0IGltcG9ydCAqCiBmcm9tIG1vZHVsZXMuY29tbWl0dGVyc191bml0dGVz dCBpbXBvcnQgKgogZnJvbSBtb2R1bGVzLmNwcF9zdHlsZV91bml0dGVzdCBpbXBvcnQgKgogZnJv bSBtb2R1bGVzLmRpZmZfcGFyc2VyX3VuaXR0ZXN0IGltcG9ydCAqCg==