Bug 37563 - new-run-webkit-tests prints out nothing when build-dumprendertree fails
Summary: new-run-webkit-tests prints out nothing when build-dumprendertree fails
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-14 00:17 PDT by Eric Seidel (no email)
Modified: 2010-09-24 15:04 PDT (History)
5 users (show)

See Also:


Attachments
Patch (1.55 KB, patch)
2010-09-13 18:54 PDT, Kenichi Ishibashi
no flags Details | Formatted Diff | Diff
Patch V1 (2.29 KB, patch)
2010-09-24 02:34 PDT, Kenichi Ishibashi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2010-04-14 00:17:41 PDT
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.
Comment 1 Kenichi Ishibashi 2010-09-13 18:54:20 PDT
Created attachment 67507 [details]
Patch
Comment 2 Dirk Pranke 2010-09-13 18:57:39 PDT
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 @@
> +2010-09-13  Kenichi Ishibashi  <bashi@google.com>
> +
> +        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  <dpranke@chromium.org>
>  
>          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
Comment 3 chris fleizach 2010-09-21 14:51:39 PDT
Comment on attachment 67507 [details]
Patch

ditto Dirk's suggestion
Comment 4 Kenichi Ishibashi 2010-09-24 02:31:52 PDT
(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.
Comment 5 Kenichi Ishibashi 2010-09-24 02:34:57 PDT
Created attachment 68662 [details]
Patch V1
Comment 6 Dirk Pranke 2010-09-24 13:58:50 PDT
Comment on attachment 68662 [details]
Patch V1

LGTM, but I'm not a reviewer.
Comment 7 Eric Seidel (no email) 2010-09-24 14:16:52 PDT
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. :)
Comment 8 WebKit Commit Bot 2010-09-24 15:04:27 PDT
Comment on attachment 68662 [details]
Patch V1

Clearing flags on attachment: 68662

Committed r68294: <http://trac.webkit.org/changeset/68294>
Comment 9 WebKit Commit Bot 2010-09-24 15:04:33 PDT
All reviewed patches have been landed.  Closing bug.