RFR: JDK-8242522: Minor LingeredApp improvements

Alex Menkov alexey.menkov at oracle.com
Tue Apr 28 00:36:53 UTC 2020


updated webrev:
http://cr.openjdk.java.net/~amenkov/jdk15/LingeredApp_improve/webrev.2/

Moved getStdoutAsList to OutputBuffer.

--alex

On 04/24/2020 20:38, Chris Plummer wrote:
> On 4/24/20 6:52 PM, Alex Menkov wrote:
>> Hi Chris,
>>
>> On 04/24/2020 15:42, Chris Plummer wrote:
>>> Hi Alex,
>>>
>>> Overall it looks good, although I'm not so sure I agree with removing 
>>> getAppOutput(). It's a generally useful utility API. Seems it is 
>>> better off in LingeredApp.java rather than in the one test that 
>>> currently uses it.
>>
>> To me the method just adds noise to the class.
>> If you think it can be useful for some other tests, I'd move it to to 
>> OutputBuffer (make it default method which uses 
>> OutputBuffer.getStdout()):
>>
>> default public List<String> getStdOutAsList()
> I think that's a good idea. It looks like what tests are commonly doing 
> now is using String.split():
> 
>              String[] appLines   = 
> app.getOutput().getStdout().split("\\R");
> 
> I'm not sure of the pros and cons of producing a String[] this way vs a 
> List<String> using getStdOutAsList(). Maybe both have their uses. But 
> one thing for sure is the split() code is a lot more readable. One 
> reason I prefer getStdOutAsList() being placed in a library or utility 
> class is because TBH I find Streams code very hard to read, and 
> combining with chained Reader code doesn't help any, so I just assume it 
> be in a library that I'm less apt to look at rather than in the test 
> where I'm bound to find my eyes glazing over as I'm drawn in to figure 
> it out.
> 
> Chris
>>
>> --alex
>>
>>>
>>> thanks,
>>>
>>> Chris
>>>
>>> On 4/24/20 3:17 PM, Alex Menkov wrote:
>>>> Hi all,
>>>>
>>>> Please review the fix for
>>>> https://bugs.openjdk.java.net/browse/JDK-8242522
>>>> webrev:
>>>> http://cr.openjdk.java.net/~amenkov/jdk15/LingeredApp_improve/webrev/
>>>>
>>>> The fix contains minor fixes/improvements for LingeredApp and tests 
>>>> which use it. See jira for details.
>>>>
>>>> Tested all tests which use LingeredApp.
>>>>
>>>> --alex
>>>
> 
> 


More information about the serviceability-dev mailing list