Alan, This is a newer version: http://cr.openjdk.java.net/~shurailine/8159523/webrev.03/
Shura > On Oct 17, 2016, at 12:16 AM, Alan Bateman <alan.bate...@oracle.com> wrote: > > On 14/10/2016 23:26, Alexandre (Shura) Iline wrote: > >> Could you please take another look? >> >> I have added more options, and fixed other things you have pointed out. I >> have also picked up a couple more tests to cover the newly added methods. >> >> http://cr.openjdk.java.net/~shurailine/8159523/webrev.02/ >> >> > The usages are easy to read so it looks to me that this effort is coming > along well. > > On repeating options again then this version is an improvement but it still > doesn't add to the list for usages like .addExports(...).addExports(...). So > rather than taking a String[] then I think it would be simpler to take a > String for one value and just add to the builder's list. > > I see shouldContain requires specifying the OutputKind when sometimes the > test doesn't care if the output is sent to stdout or stderr, maybe there can > be a shouldContain overload that searches both streams (we have this in > ProcessTools already). > > For the updated tests then I assume `throws Exception` can be removed as > there are no unchecked exceptions now. Related is whether TaskError should be > TaskException extends RuntimeException instead. > > -Alan
