Bug 197133 – Make it possible to control the renderTreeAsText output by setting options on testRunner

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Simon Fraser (smfr)]

Make it possible to control the renderTreeAsText output by setting options on testRunner

[Simon Fraser (smfr)]

Created attachment 367872 [details]
Patch

[Simon Fraser (smfr)]

Don't have the JSC C API to do this yet (bug 197134).

[Simon Fraser (smfr)]

Created attachment 367876 [details]
Patch

[EWS Watchlist]

Attachment 367876 [details] did not pass style-queue:


ERROR: Source/WebCore/rendering/RenderTreeAsText.cpp:617:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 28 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Simon Fraser (smfr)]

Created attachment 367877 [details]
Patch

[EWS Watchlist]

Attachment 367877 [details] did not pass style-queue:


ERROR: Source/WebCore/rendering/RenderTreeAsText.cpp:617:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 26 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 367877 [details]
Patch

Attachment 367877 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11940282

New failing tests:
fast/harness/render-tree-as-text-options.html

[EWS Watchlist]

Created attachment 367885 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[Simon Fraser (smfr)]

Created attachment 367893 [details]
Patch

[EWS Watchlist]

Attachment 367893 [details] did not pass style-queue:


ERROR: Source/WebCore/rendering/RenderTreeAsText.cpp:617:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 31 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Simon Fraser (smfr)]

Created attachment 367894 [details]
Patch

[EWS Watchlist]

Attachment 367894 [details] did not pass style-queue:


ERROR: Source/WebCore/rendering/RenderTreeAsText.cpp:617:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 32 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Simon Fraser (smfr)]

Created attachment 367895 [details]
Patch

[EWS Watchlist]

Attachment 367895 [details] did not pass style-queue:


ERROR: Source/WebCore/rendering/RenderTreeAsText.cpp:617:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 32 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Simon Fraser (smfr)]

Created attachment 367896 [details]
Patch

[EWS Watchlist]

Attachment 367896 [details] did not pass style-queue:


ERROR: Source/WebCore/rendering/RenderTreeAsText.cpp:617:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 32 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 367896 [details]
Patch

Attachment 367896 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/11954166

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 367931 [details]
Archive of layout-test-results from ews200 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews200  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Shawn Roberts]

EWS bots are hanging after processing this patch. 

Obsoleting the Patch so bots will stop processing.

[Simon Fraser (smfr)]

Created attachment 367988 [details]
Patch

[EWS Watchlist]

Attachment 367988 [details] did not pass style-queue:


ERROR: Source/WebCore/rendering/RenderTreeAsText.cpp:617:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 32 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 367988 [details]
Patch

Attachment 367988 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/11977752

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 368098 [details]
Archive of layout-test-results from ews101 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 367988 [details]
Patch

Attachment 367988 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/11977773

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 368102 [details]
Archive of layout-test-results from ews200 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews200  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[EWS Watchlist]

Comment on attachment 367988 [details]
Patch

Attachment 367988 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/11978036

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 368104 [details]
Archive of layout-test-results from ews114 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[Sam Weinig]

Comment on attachment 367988 [details]
Patch

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

> Tools/DumpRenderTree/TestRunner.cpp:115
> +    double options = JSValueToNumber(context, arguments[0], exception);
> +    ASSERT(!*exception);

Seems like this could throw if the value passed in is not convertible to a number. Seems like checking for the exception and returning early would be clearer to test writers than crashing. That may not be what we've done elsewhere, but seems like a better pattern to me.

> Tools/WebKitTestRunner/InjectedBundle/Bindings/CodeGeneratorTestRunner.pm:95
> +    # Underscores to camel case.
> +    $constantName =~ s{(\w+)}{
> +            ($a = lc $1) =~ s<(^[a-z]|_[a-z])><
> +                ($b = uc $1) =~ s/^_//;
> +                $b;
> +            >eg;
> +            $a;
> +        }eg;

This seems unnecessary. I'd just keep it using underscores, which are totally legal. This is generated code and doesn't have to conform to the normal rules.

> Tools/WebKitTestRunner/InjectedBundle/Bindings/CodeGeneratorTestRunner.pm:621
> +    my $attributes = [];

Can you call this $attributesAndConstants to make me happier :).

> LayoutTests/fast/harness/render-tree-as-text-options.html:36
> +                    testRunner.RENDER_TREE_SHOW_ALL_LAYERS + 
> +                    testRunner.RENDER_TREE_SHOW_LAYER_NESTING + 

Why '+' rather than the more traditional '|'? Is this the syntax you want, or would rather it be TestRunner.RENDER_TREE_FOO, e.g. on the constructor?

[Simon Fraser (smfr)]

Created attachment 368132 [details]
Patch

[EWS Watchlist]

Attachment 368132 [details] did not pass style-queue:


ERROR: Tools/DumpRenderTree/TestRunner.cpp:2087:  getRENDER_TREE_SHOW_ALL_LAYERS is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Tools/DumpRenderTree/TestRunner.cpp:2092:  getRENDER_TREE_SHOW_LAYER_NESTING is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Tools/DumpRenderTree/TestRunner.cpp:2097:  getRENDER_TREE_SHOW_COMPOSITED_LAYERS is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Tools/DumpRenderTree/TestRunner.cpp:2102:  getRENDER_TREE_SHOW_OVERFLOW is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Tools/DumpRenderTree/TestRunner.cpp:2107:  getRENDER_TREE_SHOW_SVG_GEOMETRY is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Tools/DumpRenderTree/TestRunner.cpp:2112:  getRENDER_TREE_SHOW_LAYER_FRAGMENTS is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/rendering/RenderTreeAsText.cpp:617:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 7 in 32 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Simon Fraser (smfr)]

Created attachment 368135 [details]
Patch

[EWS Watchlist]

Attachment 368135 [details] did not pass style-queue:


ERROR: Tools/DumpRenderTree/TestRunner.cpp:2087:  getRENDER_TREE_SHOW_ALL_LAYERS is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Tools/DumpRenderTree/TestRunner.cpp:2092:  getRENDER_TREE_SHOW_LAYER_NESTING is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Tools/DumpRenderTree/TestRunner.cpp:2097:  getRENDER_TREE_SHOW_COMPOSITED_LAYERS is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Tools/DumpRenderTree/TestRunner.cpp:2102:  getRENDER_TREE_SHOW_OVERFLOW is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Tools/DumpRenderTree/TestRunner.cpp:2107:  getRENDER_TREE_SHOW_SVG_GEOMETRY is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Tools/DumpRenderTree/TestRunner.cpp:2112:  getRENDER_TREE_SHOW_LAYER_FRAGMENTS is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/rendering/RenderTreeAsText.cpp:617:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 7 in 32 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Commit Bot]

Comment on attachment 368135 [details]
Patch

Clearing flags on attachment: 368135

Committed r244599: <https://trac.webkit.org/changeset/244599>

[EWS Watchlist]

Comment on attachment 368135 [details]
Patch

Attachment 368135 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/11985047

New failing tests:
fast/harness/render-tree-as-text-options.html

[EWS Watchlist]

Created attachment 368151 [details]
Archive of layout-test-results from ews204 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews204  Port: win-future  Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit

[Darin Adler]

Comment on attachment 368135 [details]
Patch

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

> Source/WebCore/rendering/RenderTreeAsText.h:41
> +enum class RenderAsTextFlag {

Was nice to modernize this a little. Maybe turn it into an OptionSet later?

[Darin Adler]

Comment on attachment 368135 [details]
Patch

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

>> Source/WebCore/rendering/RenderTreeAsText.h:41
>> +enum class RenderAsTextFlag {
> 
> Was nice to modernize this a little. Maybe turn it into an OptionSet later?

Oh, I see, that’s what you did!

[Ryan Haddad]

This landed in https://trac.webkit.org/changeset/244599/webkit

[Radar WebKit Bug Importer]

<rdar://problem/50215914>