Bug 37563 – new-run-webkit-tests prints out nothing when build-dumprendertree fails

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Eric Seidel (no email)]

new-run-webkit-tests prints out nothing when build-dumprendertree fails

% new-run-webkit-tests
Checking build ...%
%  echo $?
255

It's also failing to print a newline when ending, thus causing the % to print in zsh. :)

We must have our python logging configured wrong somehow.

[Kenichi Ishibashi]

Created attachment 67507 [details]
Patch

[Dirk Pranke]

Comment on attachment 67507 [details]
Patch

> diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
> index 2b72256b90d79bd73b6ff47ee2268c2632cb1084..f08aae58e77c64060b673b6459da2b00a45b21d5 100644
> --- a/WebKitTools/ChangeLog
> +++ b/WebKitTools/ChangeLog
> @@ -1,3 +1,14 @@
> [phone]  Kenichi Ishibashi  <[e-mail]>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        new-run-webkit-tests prints out nothing when build-dumprendertree fails
> +        https://bugs.webkit.org/show_bug.cgi?id=37563
> +
> +        Print error message when build-dumprendertree fails.
> +
> +        * Scripts/webkitpy/layout_tests/port/webkit.py:
> +
>  2010-09-12  Dirk Pranke  <[e-mail]>
>  
>          Unreviewed, build fix.
> diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/webkit.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/webkit.py
> index b085ceb7e68fc6bdfc25bea2aafae8ab8bc92db8..a81b0fd21696b1b311cac81b0cdf650dff2adabb 100644
> --- a/WebKitTools/Scripts/webkitpy/layout_tests/port/webkit.py
> +++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/webkit.py
> @@ -84,10 +84,14 @@ class WebKitPort(base.Port):
>          return ''
>  
>      def _build_driver(self):
> -        return not self._executive.run_command([
> +        exit_code = self._executive.run_command([
>              self.script_path("build-dumprendertree"),
>              self.flag_from_configuration(self._options.configuration),
>          ], return_exit_code=True)
> +        if exit_code != 0:
> +            _log.error("Failed to build DumpRenderTree")
> +            return False
> +        return True

It's probably bad that the generic code in run_webkit_tests.py isn't printing anything in this case. Can you add a _log.error('Build check failed') message at run_webkit_test.py:1420 or thereabouts as well?

Otherwise the change looks good to me (although I'm not a reviewer).

-- Dirk

[chris fleizach]

Comment on attachment 67507 [details]
Patch

ditto Dirk's suggestion

[Kenichi Ishibashi]

(In reply to comment #2)

Hi Dirk,

Thank you for your comment!

> It's probably bad that the generic code in run_webkit_tests.py isn't printing anything in this case. Can you add a _log.error('Build check failed') message at run_webkit_test.py:1420 or thereabouts as well?

I've added the message in order to follow your suggestion. Patch will come soon.

[Kenichi Ishibashi]

Created attachment 68662 [details]
Patch V1

[Dirk Pranke]

Comment on attachment 68662 [details]
Patch V1

LGTM, but I'm not a reviewer.

[Eric Seidel (no email)]

Comment on attachment 68662 [details]
Patch V1

View in context: https://bugs.webkit.org/attachment.cgi?id=68662&action=review

> WebKitTools/Scripts/webkitpy/layout_tests/port/webkit.py:87
> -        return not self._executive.run_command([
> +        exit_code = self._executive.run_command([

We don't really need to stash the exit_code in a local, but this is fine. :)

[WebKit Commit Bot]

Comment on attachment 68662 [details]
Patch V1

Clearing flags on attachment: 68662

Committed r68294: <http://trac.webkit.org/changeset/68294>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.