54699 2011-02-17 17:52:15 -0800 we should skip webkitpy.common.prettypatch_unittest if ruby isn't installed 2011-02-18 15:48:00 -0800 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) PC OS X 10.5 RESOLVED FIXED P2 Normal --- 48728 1 dpranke dpranke abarth aroben eric mihaip webkit.review.bot oldest_to_newest 353281 0 dpranke 2011-02-17 17:52:15 -0800 since there's no point in running them. 353333 1 82901 dpranke 2011-02-17 19:18:56 -0800 Created attachment 82901 Patch 353334 2 webkit.review.bot 2011-02-17 19:20:39 -0800 Attachment 82901 did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/Scripts/webkitpy..." exit_code: 1 Tools/Scripts/webkitpy/common/prettypatch_unittest.py:44: trailing whitespace [pep8/W291] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style. 353337 3 82901 aroben 2011-02-17 19:26:29 -0800 Comment on attachment 82901 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=82901&action=review > Tools/Scripts/webkitpy/common/prettypatch_unittest.py:40 > + result = executive.run_command(['ruby', '--version']) It would be nice if we didn't have to actually try to execute ruby. But it's probably the most foolproof check. > Tools/Scripts/webkitpy/common/prettypatch_unittest.py:70 > def test_pretty_diff_encodings(self): > + if not self.check_ruby(): > + return Should we print a warning? > Tools/Scripts/webkitpy/common/prettypatch_unittest.py:72 > pretty_patch = PrettyPatch(Executive(), self._webkit_root()) Should the prettypatch module do something sensible when ruby isn't installed, too? > Tools/Scripts/webkitpy/common/prettypatch_unittest.py:79 > def test_pretty_print_empty_string(self): > + if not self.check_ruby(): > + return Same question. 353340 4 dpranke 2011-02-17 19:30:39 -0800 (In reply to comment #3) > (From update of attachment 82901 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=82901&action=review > > > Tools/Scripts/webkitpy/common/prettypatch_unittest.py:40 > > + result = executive.run_command(['ruby', '--version']) > > It would be nice if we didn't have to actually try to execute ruby. But it's probably the most foolproof check. > Yeah, I don't know of a better way to do this. > > Tools/Scripts/webkitpy/common/prettypatch_unittest.py:70 > > def test_pretty_diff_encodings(self): > > + if not self.check_ruby(): > > + return > > Should we print a warning? > I don't think we generally want to print warnings when we skip over tests that we know we can't execute, just like we don't in run-webkit-tests. At some point when I can revamp test-webkitpy it'll have more useful logging for this sort of thing (bug 49299) > > Tools/Scripts/webkitpy/common/prettypatch_unittest.py:72 > > pretty_patch = PrettyPatch(Executive(), self._webkit_root()) > > Should the prettypatch module do something sensible when ruby isn't installed, too? > Maybe. I'll file a separate bug to look into that. Right now this is trapped in new-run-webkit-tests, but I think webkitpy.tool will probably get upset. > > Tools/Scripts/webkitpy/common/prettypatch_unittest.py:79 > > def test_pretty_print_empty_string(self): > > + if not self.check_ruby(): > > + return > > Same question. 353897 5 dpranke 2011-02-18 15:47:22 -0800 Committed r79046: <http://trac.webkit.org/changeset/79046> 353898 6 dpranke 2011-02-18 15:48:00 -0800 bug 54799 filed to consider making the prettypatch module the place to sensibly handle ruby not being installed. 82901 2011-02-17 19:18:56 -0800 2011-02-17 19:26:29 -0800 Patch bug-54699-20110217191855.patch text/plain 2247 dpranke ZGlmZiAtLWdpdCBhL1Rvb2xzL0NoYW5nZUxvZyBiL1Rvb2xzL0NoYW5nZUxvZwppbmRleCBmNmY5 NDNjMDIzZTk3MmU2Y2E0ZWZlNTNiNDdiZTRlNjFlMDE0MjJmLi5iZjlhZGU3ZTY1NzVlZDkzOGZl MzIyZjk4MGM2MTM3ZWY0Y2YwZTI0IDEwMDY0NAotLS0gYS9Ub29scy9DaGFuZ2VMb2cKKysrIGIv VG9vbHMvQ2hhbmdlTG9nCkBAIC0yLDYgKzIsMTYgQEAKIAogICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KIAorICAgICAgICBTa2lwIHByZXR0eSBwYXRjaCB1bml0IHRlc3RzIGlm IHJ1YnkgaXNuJ3QgaW5zdGFsbGVkLgorCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD01NDY5OQorCisgICAgICAgICogU2NyaXB0cy93ZWJraXRweS9jb21t b24vcHJldHR5cGF0Y2hfdW5pdHRlc3QucHk6CisKKzIwMTEtMDItMTcgIERpcmsgUHJhbmtlICA8 ZHByYW5rZUBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BT ISkuCisKICAgICAgICAgU2tpcAogICAgICAgICB3ZWJraXRweS5jb21tb24ubmV0LnRlc3RvdXRw dXRzZXRfdW5pdHRlc3QuVGVzdE91dHB1dFNldFRlc3QuXAogICAgICAgICAgICAgdGVzdF9jYW5f aW5mZXJfcGxhdGZvcm1fZnJvbV9wYXRoX2lmX25vbmVfcHJvdmlkZWQgb24gd2luMzIKZGlmZiAt LWdpdCBhL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvY29tbW9uL3ByZXR0eXBhdGNoX3VuaXR0ZXN0 LnB5IGIvVG9vbHMvU2NyaXB0cy93ZWJraXRweS9jb21tb24vcHJldHR5cGF0Y2hfdW5pdHRlc3Qu cHkKaW5kZXggMTMwNzg1NmMxZjk5MDljMmM4YjU0ZTg4MzIxYzk0MzQyODYzYmU2Zi4uZDhjYjJh NWIxODlkNGY3ZDQ5Zjc4YzQyNmZmMmI3OWYxYWFiY2M4NiAxMDA2NDQKLS0tIGEvVG9vbHMvU2Ny aXB0cy93ZWJraXRweS9jb21tb24vcHJldHR5cGF0Y2hfdW5pdHRlc3QucHkKKysrIGIvVG9vbHMv U2NyaXB0cy93ZWJraXRweS9jb21tb24vcHJldHR5cGF0Y2hfdW5pdHRlc3QucHkKQEAgLTM0LDcg KzM0LDE0IEBAIGZyb20gd2Via2l0cHkuY29tbW9uLnByZXR0eXBhdGNoIGltcG9ydCBQcmV0dHlQ YXRjaAogCiAKIGNsYXNzIFByZXR0eVBhdGNoVGVzdCh1bml0dGVzdC5UZXN0Q2FzZSk6Ci0KKyAg ICBkZWYgY2hlY2tfcnVieShzZWxmKToKKyAgICAgICAgZXhlY3V0aXZlID0gRXhlY3V0aXZlKCkK KyAgICAgICAgdHJ5OgorICAgICAgICAgICAgcmVzdWx0ID0gZXhlY3V0aXZlLnJ1bl9jb21tYW5k KFsncnVieScsICctLXZlcnNpb24nXSkKKyAgICAgICAgZXhjZXB0IE9TRXJyb3IsIGU6CisgICAg ICAgICAgICByZXR1cm4gRmFsc2UKKyAgICAgICAgcmV0dXJuIFRydWUKKyAgICAKICAgICBfZGlm Zl93aXRoX211bHRpcGxlX2VuY29kaW5ncyA9ICIiIgogSW5kZXg6IHV0ZjhfdGVzdAogPT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PQpAQCAtNTksMTIgKzY2LDE4IEBAIEluZGV4OiBsYXRpbjFfdGVzdAogICAgICAgICByZXR1 cm4gd2Via2l0X3Jvb3QKIAogICAgIGRlZiB0ZXN0X3ByZXR0eV9kaWZmX2VuY29kaW5ncyhzZWxm KToKKyAgICAgICAgaWYgbm90IHNlbGYuY2hlY2tfcnVieSgpOgorICAgICAgICAgICAgcmV0dXJu CisKICAgICAgICAgcHJldHR5X3BhdGNoID0gUHJldHR5UGF0Y2goRXhlY3V0aXZlKCksIHNlbGYu X3dlYmtpdF9yb290KCkpCiAgICAgICAgIHByZXR0eSA9IHByZXR0eV9wYXRjaC5wcmV0dHlfZGlm ZihzZWxmLl9kaWZmX3dpdGhfbXVsdGlwbGVfZW5jb2RpbmdzKQogICAgICAgICBzZWxmLmFzc2Vy dFRydWUocHJldHR5KSAgIyBXZSBnb3Qgc29tZSBvdXRwdXQKICAgICAgICAgc2VsZi5hc3NlcnRU cnVlKGlzaW5zdGFuY2UocHJldHR5LCBzdHIpKSAgIyBJdCdzIGEgYnl0ZSBhcnJheSwgbm90IHVu aWNvZGUKIAogICAgIGRlZiB0ZXN0X3ByZXR0eV9wcmludF9lbXB0eV9zdHJpbmcoc2VsZik6Cisg ICAgICAgIGlmIG5vdCBzZWxmLmNoZWNrX3J1YnkoKToKKyAgICAgICAgICAgIHJldHVybgorCiAg ICAgICAgICMgTWFrZSBzdXJlIHRoYXQgYW4gZW1wdHkgZGlmZiBkb2VzIG5vdCBoYW5nIHRoZSBw cm9jZXNzLgogICAgICAgICBwcmV0dHlfcGF0Y2ggPSBQcmV0dHlQYXRjaChFeGVjdXRpdmUoKSwg c2VsZi5fd2Via2l0X3Jvb3QoKSkKICAgICAgICAgc2VsZi5hc3NlcnRFcXVhbChwcmV0dHlfcGF0 Y2gucHJldHR5X2RpZmYoIiIpLCAiIikK