Bug 122303 - Add a verbose flag to binding tests
Summary: Add a verbose flag to binding tests
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P4 Enhancement
Assignee: Nobody
Keywords: InRadar
Depends on:
Reported: 2013-10-03 15:11 PDT by Matthew Hanson
Modified: 2014-06-02 23:52 PDT (History)
7 users (show)

See Also:

v1 Patch for 122303 (2.58 KB, patch)
2013-10-03 15:36 PDT, Matthew Hanson
kling: review-
kling: commit-queue-
Details | Formatted Diff | Diff
Patch v2 (deleted)
2014-05-29 07:08 PDT, Tibor Mészáros
ossy: review-
ossy: commit-queue-
Details | Formatted Diff | Diff
Patch v3 (2.94 KB, patch)
2014-05-30 05:13 PDT, Tibor Mészáros
ossy: review-
ossy: commit-queue-
Details | Formatted Diff | Diff
Patch v4 (2.74 KB, patch)
2014-06-02 04:14 PDT, Tibor Mészáros
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Matthew Hanson 2013-10-03 15:11:41 PDT
While investigating Bug 122017, I found that positive BindingTests results are unconditionally printed to stdout.  It is often desirable to suppress the output of these test cases since they are run often and expected to fail rarely.

The BindingTests class is defined in Tools/Scripts/webkitpy/bindings/main.py.
Comment 1 Radar WebKit Bug Importer 2013-10-03 15:12:04 PDT
Comment 2 Matthew Hanson 2013-10-03 15:36:33 PDT
Created attachment 213305 [details]
v1 Patch for 122303

Added a verbose keyword argument to the BindingsTests constructor.
Verbose mode, which is on by default, prints positive results to stdout as before.
When the verbose flag is set to false, positive results are not printed at all.
Comment 3 Peter Gal 2013-10-18 05:13:41 PDT
Comment on attachment 213305 [details]
v1 Patch for 122303

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

The 'run-bindings-test' script should be adapted for this change. Without that there is no way to disable it from command line.

> Tools/Scripts/webkitpy/bindings/main.py:117
> +                if self.verbose is True:

A simple 'if self.verbose' is enough. Also 'is' is for identity checking see http://docs.python.org/2/library/operator.html#operator.is_
Comment 4 Radar WebKit Bug Importer 2013-12-06 11:25:59 PST
Comment 5 Radar WebKit Bug Importer 2013-12-17 11:24:20 PST
Comment 6 Andreas Kling 2014-02-05 17:38:02 PST
Comment on attachment 213305 [details]
v1 Patch for 122303

r-, please fix the problems pointed out by Peter Gal.
Comment 7 Tibor Mészáros 2014-05-29 07:08:46 PDT
Created attachment 232246 [details]
Patch v2

I had finished Matthew's patch. The following changes where made:
- Added --no-verbose option to the run-bindigs-test.
- Changed "if self.verbose is True" to "self.verbose" as Peter Gal wanted.
Comment 8 Peter Gal 2014-05-29 09:02:15 PDT
Comment on attachment 232246 [details]
Patch v2

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

> Tools/Scripts/run-bindings-tests:47
> +    if "--no-verbose" in argv:
> +        verbose = False
> +    else:
> +        verbose = True

We should make this a bit simpler a one-liner, like in the line 42. Eg.: verbose = not "--no-verbose" in argv

> Tools/Scripts/run-bindings-tests:52
> +        'GObject',

I don't see any point of this change, why is this needed?

> Tools/Scripts/webkitpy/bindings/main.py:117
>              else:
> -                print 'PASS: (%s) %s' % (generator, output_file)
> +                if self.verbose:

Looking at this know I realize we can merge the 'else' and 'if self.verbose' into an 'elsif self.verbose' :)
Comment 9 Csaba Osztrogonác 2014-05-29 09:03:55 PDT
Comment on attachment 232246 [details]
Patch v2

r-, please fix the problems pointed out by Péter Gál.
Comment 10 Tibor Mészáros 2014-05-30 05:13:50 PDT
Created attachment 232292 [details]
Patch v3

All has been fixed, that Peter mentioned :)
Comment 11 Csaba Osztrogonác 2014-05-30 05:47:09 PDT
Comment on attachment 232292 [details]
Patch v3

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

> Tools/Scripts/run-bindings-tests:43
> +    

This extra newline is unnecessary.

> Tools/Scripts/webkitpy/bindings/main.py:117
> +            elif self.verbose:
> +                    print 'PASS: (%s) %s' % (generator, output_file)

Please remove this extra indentation.
Comment 12 Tibor Mészáros 2014-06-02 04:14:50 PDT
Created attachment 232375 [details]
Patch v4

The newline and the extra indentation has been removed.
Comment 13 WebKit Commit Bot 2014-06-02 23:52:00 PDT
Comment on attachment 232375 [details]
Patch v4

Clearing flags on attachment: 232375

Committed r169558: <http://trac.webkit.org/changeset/169558>
Comment 14 WebKit Commit Bot 2014-06-02 23:52:04 PDT
All reviewed patches have been landed.  Closing bug.