RFR(S): 8198655: test/lib/jdk/test/lib/apps/LingeredApp shouldn't inherit cout/cerr

[David Holmes]

I'm afraid I'm losing track of this change.

The key thing is that we should not have a test that launches any other 
process for which we can not see the output of that process.

David

On 17/03/2018 7:48 AM, Chris Plummer wrote:
> On 3/16/18 1:25 PM, serguei.spitsyn at oracle.com wrote:
>> Hi Chris,
>>
>> Thank you for taking care about this issue!
>>
>> On 3/16/18 11:20, Chris Plummer wrote:
>>> Hi,
>>>
>>> I've resolved the issues I had before with not seeing all the stderr 
>>> output when I tried to capture it. What I'd like to do now is have us 
>>> decide how the output should be handled from the perspective a 
>>> LingeredApp user (driver app). Currently all LingeredApp stdout is 
>>> captured and gets be returned the the driver app by calling 
>>> app.getAppOutput(). It does not appear in the .jtr file, but the test 
>>> would have the option of dumping it there it it cared to. Only one 
>>> test uses app.getAppOutput(). Currently all the LingeredApp stderr is 
>>> redirected to the console, so it does not appear in the .jtr file.
>>
>> Just a general comment to make sure I understand it and ensure we are 
>> in sync.
>> It seems much more safe to always have both stdout and stderr outputs 
>> present in the .jtr automatically file independently of of what the 
>> test does.
>>
>>
>>> So how do we want this changed? Some possibilities are:
>>>
>>> (1) capture stderr just like stdout currently is, and leave is up the 
>>> the driver app to decide if it wants to display it (after the app 
>>> terminates).
>>
>> It does not look good to me (see above) but maybe I'm missing 
>> something important here.
>>
>>> (2) capture stderr just like stdout currently is, but have 
>>> LingeredApp automatically send captured output to driver app's stdout 
>>> and stderr (after the app terminates).
>>
>> The stdout and std err will be separated in this case, right?
>> Do you have a webrev for this?
> I currently have it working like this, although I need to fix 
> LingeredApp.getAppOutput(). I had to make it return a single String 
> instead of a List of Strings, so this breaks the one test that uses this 
> API. It's easily fixed. Just haven't gotten around to it yet.
>>
>>
>>> (3) send the LingeredApp's stdout and stderr to the driver app's 
>>> stdout as it is being captured (this was the original fix Igor 
>>> suggested and the webrev supported). A minor alternative to this is 
>>> to keep the two streams separated instead of sending both to stdout.
>>>
>>> Let me know what you think. I'm inclined to go with 2, especially 
>>> since normally there is little to no output from the LingeredApp.
>>
>> The choice (2) looks good enough.
>> Not sure it is that important to have output from stdout and stderr 
>> sync'ed
>> but is is important to have the stderr present in the .jtr automatically.
>>
>> The choice (3) looks even better if it is going to work well.
> This is basically what the original webrev did. It sent LingeredApp's 
> stderr and stdout to the the driver apps stdout. It's a 1 word change to 
> make it send stderr to stderr. I think it has a bug though that did not 
> manifest itself. It seems the new copy() code that is capturing stdout 
> would be contending with the existing InputGlobbler code that is doing 
> the same. I would need to fix this to make sure 
> LingeredApp.getAppOutput() still returns all the apps stdout output.
> 
> Chris
>> Not sure, it is really necessary.
>>
>> Thanks,
>> Serguei
>>
>>
>>>
>>> BTW, here's the CR and original webrev for reference:
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8198655
>>> http://cr.openjdk.java.net/~cjplummer/8198655/webrev.00/webrev/
>>>
>>> thanks,
>>>
>>> Chris
>>>
>>
>