[webkit-reviews] review denied: [Bug 46635] test-webkitpy should have logging configured differently : [Attachment 71836] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Dirk Pranke <dpranke at chromium.org>'s
request for review:
Bug 46635: test-webkitpy should have logging configured differently
https://bugs.webkit.org/show_bug.cgi?id=46635

Attachment 71836: Patch
https://bugs.webkit.org/attachment.cgi?id=71836&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=71836&action=review

I think I understand what this does now.  It would be better to move off
modifying the root logger.

> WebKitTools/Scripts/test-webkitpy:80
> -    logger.setLevel(logging.NOTSET)
> +    logger.setLevel(logging_level)

I think this is what caused your later unit test to need to modify the root
logger.  I would leave this code for now.  You probably don't need to even use
the root logger in your tests.

>
WebKitTools/Scripts/webkitpy/layout_tests/layout_package/metered_stream_unittes
t.py:106
>	   logger = logging.getLogger()
>	   handler = logging.StreamHandler(b)
>	   logger.addHandler(handler)
> +	   level = logger.level
> +	   logger.setLevel(logging.NOTSET)
>	   m.update("bar")

Seems we should use an intermediate logger instead of modifying the root.