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