Make new-run-webkit-test PrettyPatch failure reporting more awesome
Created attachment 53222 [details] Patch
Comment on attachment 53222 [details] Patch ok. missing a space
Committed r57500: <http://trac.webkit.org/changeset/57500>
Comment on attachment 53222 [details] Patch > command = ["ruby", "-I", pretty_patch_path, prettify_path, diff_path] > try: > return self._executive.run_command(command) > except OSError, e: > - # If they system is missing ruby just log the error, and stop trying. > + # If the system is missing ruby log the error and stop trying. > _pretty_patch_available = False > - return "Failed to run PrettyPatch: %s" % e > - except Executive.ScriptError, e: > - # If they system is missing ruby just log the error, and stop trying. > + _log.error("Failed to run PrettyPatch (%s): %s" % (command, e)) > + return self._pretty_patch_error_html > + except ScriptError, e: > + # If ruby failed to run for some reason, log the command output and stop trying. > _pretty_patch_available = False > - return "Failed to run PrettyPatch: %s" % e > + _log.error("Failed to run PrettyPatch (%s):\n%s" % (command, e.message_with_output())) > + return self._pretty_patch_error_html > Why does this routine have to catch either a ScriptError or an OSError? It seems like maybe the run_command() semantics aren't clear?
They mean different things. OSError means you're missing the file you tried to run. ScriptError means it returned non-zero.