Bug 234525 – Make it possible to enable log channels in WebKitTestRunner and DumpRenderTree

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

[Simon Fraser (smfr)]

Make it possible to enable log channels in WebKitTestRunner and DumpRenderTree

[Simon Fraser (smfr)]

Created attachment 447629 [details]
Patch

[Wenson Hsieh]

Comment on attachment 447629 [details]
Patch

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

> Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:124
> +        [[NSUserDefaults standardUserDefaults] setValue:[NSString stringWithUTF8String:options.webCoreLogChannels.c_str()] forKey:@"WebKitLogging"];

This should be `options.webKitLogChannels.c_str()`.

[Simon Fraser (smfr)]

Created attachment 447634 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 447639 [details]
Patch

[Sam Weinig]

Comment on attachment 447629 [details]
Patch

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

> Tools/DumpRenderTree/mac/DumpRenderTree.mm:1008
> +        {"WebCoreLogging", required_argument, nullptr, 'W'},

The other options use dashes between words. For consistency, how about -webcore-logging?

> Tools/WebKitTestRunner/Options.cpp:155
> +static bool handleOptionLogChannels(Options& options, const char* option, const char* channels)

Can you move this above the `handleOptionUnmatched` function? I think It makes sense to keep that one last.

> Tools/WebKitTestRunner/Options.cpp:194
> +    optionList.append(Option("--WebCoreLogging", "Enable WebCore log channels", handleOptionLogChannels, true));
> +    optionList.append(Option("--WebKitLogging", "Enable WebKit log channels", handleOptionLogChannels, true));

Same comment as about. Can we keep these consistent as well? Also, I would recommend just using two different functions rather than reparsing.

[Simon Fraser (smfr)]

https://trac.webkit.org/changeset/287292/webkit

[Radar WebKit Bug Importer]

<rdar://problem/86747488>