[Piglit] [PATCH 1/3] framework: Never redirect sys.stderr to /dev/null.

[Jose Fonseca]

On 08/12/15 15:56, Dylan Baker wrote:
> On Tue, Dec 08, 2015 at 03:51:40PM +0000, Jose Fonseca wrote:
>> On 08/12/15 15:38, Dylan Baker wrote:
>>> On Tue, Dec 08, 2015 at 01:23:28PM +0000, Jose Fonseca wrote:
>>>> Awful idea, as it prevents easy debugging/diagnosis of framework issues.
>>>>
>>>> If the test is verbose, then the child process' stderr should be
>>>> redirected, and not Python's process.
>>>> ---
>>>>   framework/tests/profile_tests.py    |  3 ---
>>>>   framework/tests/run_parser_tests.py | 10 ----------
>>>>   2 files changed, 13 deletions(-)
>>>>
>>>> diff --git a/framework/tests/profile_tests.py b/framework/tests/profile_tests.py
>>>> index 3a7d68b..010dbc3 100644
>>>> --- a/framework/tests/profile_tests.py
>>>> +++ b/framework/tests/profile_tests.py
>>>> @@ -31,9 +31,6 @@ from framework.tests import utils
>>>>   from framework import grouptools, dmesg, profile, exceptions, options
>>>>   from framework.test import GleanTest
>>>>
>>>> -# Don't print sys.stderr to the console
>>>> -sys.stderr = sys.stdout
>>>> -
>>>>
>>>>   @utils.no_error
>>>>   def test_initialize_testprofile():
>>>> diff --git a/framework/tests/run_parser_tests.py b/framework/tests/run_parser_tests.py
>>>> index 5f0b21b..677adfc 100644
>>>> --- a/framework/tests/run_parser_tests.py
>>>> +++ b/framework/tests/run_parser_tests.py
>>>> @@ -156,11 +156,6 @@ class TestBackend(_Helpers):
>>>>           self._unset_config()
>>>>           self._move_piglit_conf()
>>>>
>>>> -        # This has sideffects, it shouldn't effect anything in this module, but
>>>> -        # it may cause later problems. But without this we get ugly error spew
>>>> -        # from this test.
>>>> -        sys.stderr = open(os.devnull, 'w')
>>>> -
>>>>           with utils.tempdir() as tdir:
>>>>               with open(os.path.join(tdir, 'piglit.conf'), 'w') as f:
>>>>                   f.write('[core]\nbackend=foobar')
>>>> @@ -249,11 +244,6 @@ class TestPlatform(_Helpers):
>>>>           self._unset_config()
>>>>           self._move_piglit_conf()
>>>>
>>>> -        # This has sideffects, it shouldn't effect anything in this module, but
>>>> -        # it may cause later problems. But without this we get ugly error spew
>>>> -        # from this test.
>>>> -        sys.stderr = open(os.devnull, 'w')
>>>> -
>>>>           with utils.tempdir() as tdir:
>>>>               with open(os.path.join(tdir, 'piglit.conf'), 'w') as f:
>>>>                   f.write('[core]\nplatform=foobar')
>>>> --
>>>> 2.5.0
>>>>
>>>
>>> Nose will print stdout when the test fails or errors, but not when it
>>> passes. Printing stderr when it's expected is ugly and misleading.
>>
>> I don't understand how Nose fits here.  Isn't that something you use to test
>> the piglit framework itself?
>>
>> Or am I to understand that you're making Piglit harder to use for OpenGL
>> developers, so that it is easier to use for Piglit developers? Isn't that
>> backwards?
>>
>>
>> The fact is that, without at least some of the above hunks, the shader
>> runner tests output gets silently redirected.
>>
>>
>> Jose
>
> These are framework tests that you're modifying. They're run by nose.
>

You're right.  I could swear that without this change stderr wasn't 
working, but it does now.

I'll drop this one.

Jose