[webkit-reviews] review granted: [Bug 54699] we should skip webkitpy.common.prettypatch_unittest if ruby isn't installed : [Attachment 82901] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Feb 17 19:26:29 PST 2011


Adam Roben (aroben) <aroben at apple.com> has granted Dirk Pranke
<dpranke at chromium.org>'s request for review:
Bug 54699: we should skip webkitpy.common.prettypatch_unittest if ruby isn't
installed
https://bugs.webkit.org/show_bug.cgi?id=54699

Attachment 82901: Patch
https://bugs.webkit.org/attachment.cgi?id=82901&action=review

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
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.


More information about the webkit-reviews mailing list