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
