Re: RFR(L/M) : 8210112 : remove jdk.testlibrary.ProcessTools

[Alan Bateman]

On 06/09/2018 17:19, Igor Ignatyev wrote:

JC,
thanks for your review!

Core-libs team,
as the majority of changed tests are core-libs tests, I'd really appreciate if 
someone from core-libs (preferably a Reviewer) could review the patch.


I skimmed through a sample of the test changes and was pleased to see 
that it's just the test tags that have changed. Also pleased to see that 
the shouldXXX methods now consistently return the OutputAnalyzer (this 
seems to be one area where the version in testlibrary was different). I 
don't have time to do a detail review of the updated 
ProcessTools/OutputAnalyzer but I think you have enough reviewers already.


-Alan

Re: RFR(L/M) : 8210112 : remove jdk.testlibrary.ProcessTools

[serguei.spit...@oracle.com]

Hi Igor,

I do not see any issues with this refactoring.

Thanks,
Serguei


On 9/6/18 09:19, Igor Ignatyev wrote:

JC,
thanks for your review!

Core-libs team,
as the majority of changed tests are core-libs tests, I'd really appreciate if 
someone from core-libs (preferably a Reviewer) could review the patch.

for the record, [1] is the latest version of webrev.

[1] http://cr.openjdk.java.net/~iignatyev//8210112/webrev.02/index.html 


2433 lines changed: 334 ins; 1677 del; 422 mod;

Cheers,
-- Igor



On Sep 5, 2018, at 5:03 PM, JC Beyler  wrote:

Hi Igor,

Looks good to me then. I agree most are nits, personal preferences, and just I 
noticed them as you were cleaning them up!

Awesome clean up :-)
Jc

On Wed, Sep 5, 2018 at 3:20 PM Igor Ignatyev mailto:igor.ignat...@oracle.com>> wrote:
Hi JC,



On Sep 5, 2018, at 2:59 PM, JC Beyler mailto:jcbey...@google.com>> wrote:

Hi Igor,

I like this much better! A few more comments:

- 
http://cr.openjdk.java.net/~iignatyev//8210112/webrev.0-1/test/jdk/lib/testlibrary/OutputAnalyzerTest.java.udiff.html
 

   -> If the shouldMatch call fails, it throws an exception, why not just let that 
fail test, why are you catching and then rethrowing (like you do for 
http://cr.openjdk.java.net/~iignatyev//8210112/webrev.0-1/test/jdk/sun/tools/jcmd/TestJcmdDefaults.java.udiff.html
 
)

just to follow the style used by this tests, e.g.


   54 try {
   55 output.shouldContain(stdout);
   56 output.stdoutShouldContain(stdout);
   57 output.shouldContain(stderr);
   58 output.stderrShouldContain(stderr);
   59 } catch (RuntimeException e) {
   60 throw new Exception("shouldContain() failed", e);
   61 }
   86 try {
   87 output.shouldNotContain(nonExistingString);
   88 output.stdoutShouldNotContain(nonExistingString);
   89 output.stderrShouldNotContain(nonExistingString);
   90 } catch (RuntimeException e) {
   91 throw new Exception("shouldNotContain() failed", e);
   92 }
   93



- 
http://cr.openjdk.java.net/~iignatyev//8210112/webrev.0-1/test/jdk/sun/tools/jcmd/TestJcmdDefaults.java.udiff.html
 

There is now only a 1-liner for this method and it is called only once, 
should we inline and remove the method?

we can, but I ain't sure we should. from my point of view

   80 output.shouldHaveExitValue(0);
   81 output.shouldContain("sun.tools.jcmd.JCmd");
   82 matchListedProcesses(output);
is a bit easier to understand than
   80 output.shouldHaveExitValue(0);
   81 output.shouldContain("sun.tools.jcmd.JCmd");
   82 output.shouldMatchByLine(JCMD_LIST_REGEX);

however, I don't have strong preference here and if serviceability team wants, 
I can inline matchListedProcesses.


- Same for (we could inline): 
http://cr.openjdk.java.net/~iignatyev//8210112/webrev.0-1/test/jdk/sun/tools/jcmd/TestJcmdSanity.java.udiff.html
 
same
 here
- 
http://cr.openjdk.java.net/~iignatyev//8210112/webrev.0-1/test/lib/jdk/test/lib/process/OutputAnalyzer.java.udiff.html
 

 "There is no lines" -> "There are no lines"

fixed. thanks for spotting.


 - What is the advantage of having the return at all now for the 
shouldMatch methods, if it fails it throws, the test fails; otherwise it 
doesn't return anything, the test can move on, no? I saw no moment when you get 
the return to do something more with it

OutputAnalazyer is supposed to be a fluent interface, and in some cases you 
might find it used that way, so I'd prefer to have possibility to use these 
methods in a method chain, as w/ we already have for the the most of other 
should* method. I've fixed a few more should* methods to return this.



Thanks for the incremental webrev, that made looking at the changes so much 
easier!

here is the next one -- 
http://cr.openjdk.java.net/~iignatyev//8210112/webrev.1-2/index.html 


-- Igor

Jc

On Wed, Sep 5, 2018 at 2:32 PM Igor Ignatyev mailto:igor.ignat...@oracle.com>> wrote:
Hi JC,

thanks for reviewing this! I agree w/ both your comments and have updated the 
code very similarly to your suggestion.

I've also noticed that j.t.l.p.OutputAnalyzer::shouldMatchByLine met

Re: RFR(L/M) : 8210112 : remove jdk.testlibrary.ProcessTools

[Igor Ignatyev]

JC,
thanks for your review!

Core-libs team,
as the majority of changed tests are core-libs tests, I'd really appreciate if 
someone from core-libs (preferably a Reviewer) could review the patch. 

for the record, [1] is the latest version of webrev.

[1] http://cr.openjdk.java.net/~iignatyev//8210112/webrev.02/index.html 

> 2433 lines changed: 334 ins; 1677 del; 422 mod;

Cheers,
-- Igor


> On Sep 5, 2018, at 5:03 PM, JC Beyler  wrote:
> 
> Hi Igor,
> 
> Looks good to me then. I agree most are nits, personal preferences, and just 
> I noticed them as you were cleaning them up!
> 
> Awesome clean up :-)
> Jc
> 
> On Wed, Sep 5, 2018 at 3:20 PM Igor Ignatyev  > wrote:
> Hi JC,
> 
> 
>> On Sep 5, 2018, at 2:59 PM, JC Beyler > > wrote:
>> 
>> Hi Igor,
>> 
>> I like this much better! A few more comments:
>> 
>> - 
>> http://cr.openjdk.java.net/~iignatyev//8210112/webrev.0-1/test/jdk/lib/testlibrary/OutputAnalyzerTest.java.udiff.html
>>  
>> 
>>   -> If the shouldMatch call fails, it throws an exception, why not just let 
>> that fail test, why are you catching and then rethrowing (like you do for 
>> http://cr.openjdk.java.net/~iignatyev//8210112/webrev.0-1/test/jdk/sun/tools/jcmd/TestJcmdDefaults.java.udiff.html
>>  
>> )
> just to follow the style used by this tests, e.g.
> 
>>   54 try {
>>   55 output.shouldContain(stdout);
>>   56 output.stdoutShouldContain(stdout);
>>   57 output.shouldContain(stderr);
>>   58 output.stderrShouldContain(stderr);
>>   59 } catch (RuntimeException e) {
>>   60 throw new Exception("shouldContain() failed", e);
>>   61 }
> 
>>   86 try {
>>   87 output.shouldNotContain(nonExistingString);
>>   88 output.stdoutShouldNotContain(nonExistingString);
>>   89 output.stderrShouldNotContain(nonExistingString);
>>   90 } catch (RuntimeException e) {
>>   91 throw new Exception("shouldNotContain() failed", e);
>>   92 }
>>   93 
> 
> 
>> - 
>> http://cr.openjdk.java.net/~iignatyev//8210112/webrev.0-1/test/jdk/sun/tools/jcmd/TestJcmdDefaults.java.udiff.html
>>  
>> 
>>There is now only a 1-liner for this method and it is called only once, 
>> should we inline and remove the method?
> 
> we can, but I ain't sure we should. from my point of view
> 
>   80 output.shouldHaveExitValue(0);
>   81 output.shouldContain("sun.tools.jcmd.JCmd");
>   82 matchListedProcesses(output);
> is a bit easier to understand than
>   80 output.shouldHaveExitValue(0);
>   81 output.shouldContain("sun.tools.jcmd.JCmd");
>   82 output.shouldMatchByLine(JCMD_LIST_REGEX);
> 
> however, I don't have strong preference here and if serviceability team 
> wants, I can inline matchListedProcesses.
> 
>> - Same for (we could inline): 
>> http://cr.openjdk.java.net/~iignatyev//8210112/webrev.0-1/test/jdk/sun/tools/jcmd/TestJcmdSanity.java.udiff.html
>>  
>> same
>>  here
> 
>> 
>> - 
>> http://cr.openjdk.java.net/~iignatyev//8210112/webrev.0-1/test/lib/jdk/test/lib/process/OutputAnalyzer.java.udiff.html
>>  
>> 
>> "There is no lines" -> "There are no lines"
> fixed. thanks for spotting.
> 
>> - What is the advantage of having the return at all now for the 
>> shouldMatch methods, if it fails it throws, the test fails; otherwise it 
>> doesn't return anything, the test can move on, no? I saw no moment when you 
>> get the return to do something more with it
> OutputAnalazyer is supposed to be a fluent interface, and in some cases you 
> might find it used that way, so I'd prefer to have possibility to use these 
> methods in a method chain, as w/ we already have for the the most of other 
> should* method. I've fixed a few more should* methods to return this.
> 
> 
>> Thanks for the incremental webrev, that made looking at the changes so much 
>> easier!
> 
> here is the next one -- 
> http://cr.openjdk.java.net/~iignatyev//8210112/webrev.1-2/index.html 
> 
> 
> -- Igor
>> Jc
>> 
>> On Wed, Sep 5, 2018 at 2:32 PM Igor Ignatyev > > wrote:
>> Hi JC,
>> 
>> thanks for reviewing this! I agree w/ both your com

Re: RFR(L/M) : 8210112 : remove jdk.testlibrary.ProcessTools

[Igor Ignatyev]

Hi JC,


> On Sep 5, 2018, at 2:59 PM, JC Beyler  wrote:
> 
> Hi Igor,
> 
> I like this much better! A few more comments:
> 
> - 
> http://cr.openjdk.java.net/~iignatyev//8210112/webrev.0-1/test/jdk/lib/testlibrary/OutputAnalyzerTest.java.udiff.html
>  
> 
>   -> If the shouldMatch call fails, it throws an exception, why not just let 
> that fail test, why are you catching and then rethrowing (like you do for 
> http://cr.openjdk.java.net/~iignatyev//8210112/webrev.0-1/test/jdk/sun/tools/jcmd/TestJcmdDefaults.java.udiff.html
>  
> )
just to follow the style used by this tests, e.g.

>   54 try {
>   55 output.shouldContain(stdout);
>   56 output.stdoutShouldContain(stdout);
>   57 output.shouldContain(stderr);
>   58 output.stderrShouldContain(stderr);
>   59 } catch (RuntimeException e) {
>   60 throw new Exception("shouldContain() failed", e);
>   61 }

>   86 try {
>   87 output.shouldNotContain(nonExistingString);
>   88 output.stdoutShouldNotContain(nonExistingString);
>   89 output.stderrShouldNotContain(nonExistingString);
>   90 } catch (RuntimeException e) {
>   91 throw new Exception("shouldNotContain() failed", e);
>   92 }
>   93 


> - 
> http://cr.openjdk.java.net/~iignatyev//8210112/webrev.0-1/test/jdk/sun/tools/jcmd/TestJcmdDefaults.java.udiff.html
>  
> 
>There is now only a 1-liner for this method and it is called only once, 
> should we inline and remove the method?

we can, but I ain't sure we should. from my point of view

  80 output.shouldHaveExitValue(0);
  81 output.shouldContain("sun.tools.jcmd.JCmd");
  82 matchListedProcesses(output);
is a bit easier to understand than
  80 output.shouldHaveExitValue(0);
  81 output.shouldContain("sun.tools.jcmd.JCmd");
  82 output.shouldMatchByLine(JCMD_LIST_REGEX);

however, I don't have strong preference here and if serviceability team wants, 
I can inline matchListedProcesses.

> - Same for (we could inline): 
> http://cr.openjdk.java.net/~iignatyev//8210112/webrev.0-1/test/jdk/sun/tools/jcmd/TestJcmdSanity.java.udiff.html
>  
> same
>  here

> 
> - 
> http://cr.openjdk.java.net/~iignatyev//8210112/webrev.0-1/test/lib/jdk/test/lib/process/OutputAnalyzer.java.udiff.html
>  
> 
> "There is no lines" -> "There are no lines"
fixed. thanks for spotting.

> - What is the advantage of having the return at all now for the 
> shouldMatch methods, if it fails it throws, the test fails; otherwise it 
> doesn't return anything, the test can move on, no? I saw no moment when you 
> get the return to do something more with it
OutputAnalazyer is supposed to be a fluent interface, and in some cases you 
might find it used that way, so I'd prefer to have possibility to use these 
methods in a method chain, as w/ we already have for the the most of other 
should* method. I've fixed a few more should* methods to return this.


> Thanks for the incremental webrev, that made looking at the changes so much 
> easier!

here is the next one -- 
http://cr.openjdk.java.net/~iignatyev//8210112/webrev.1-2/index.html 


-- Igor
> Jc
> 
> On Wed, Sep 5, 2018 at 2:32 PM Igor Ignatyev  > wrote:
> Hi JC,
> 
> thanks for reviewing this! I agree w/ both your comments and have updated the 
> code very similarly to your suggestion.
> 
> I've also noticed that j.t.l.p.OutputAnalyzer::shouldMatchByLine method 
> family is a bit different from other should* (and strange), besides checking 
> that the lines match the pattern, shouldMatchByLine methods do not check that 
> it's greater than zero and return number of matched lines instead. however 
> all users of these methods do check that the return results is non zero. I 
> have updated these methods to check that there are lines to match and updated 
> all their users correspondingly. Doing that, I also made some harmless 
> refactoring, like moving Pattern::compile from loops, using "\R" as 
> end-of-line pattern.
> 
> incremental webrev: 
> http://cr.openjdk.java.net/~iignatyev//8210112/webrev.0-1/index.html 
> 
> 
> Thanks,
> -- Igor
> 
>> On Sep 4, 2018, at 8:01 PM, JC B

Re: RFR(L/M) : 8210112 : remove jdk.testlibrary.ProcessTools

[JC Beyler]

Hi Igor,

I like this much better! A few more comments:

-
http://cr.openjdk.java.net/~iignatyev//8210112/webrev.0-1/test/jdk/lib/testlibrary/OutputAnalyzerTest.java.udiff.html
  -> If the shouldMatch call fails, it throws an exception, why not just
let that fail test, why are you catching and then rethrowing (like you do
for
http://cr.openjdk.java.net/~iignatyev//8210112/webrev.0-1/test/jdk/sun/tools/jcmd/TestJcmdDefaults.java.udiff.html
)

-
http://cr.openjdk.java.net/~iignatyev//8210112/webrev.0-1/test/jdk/sun/tools/jcmd/TestJcmdDefaults.java.udiff.html
   There is now only a 1-liner for this method and it is called only once,
should we inline and remove the method?

- Same for (we could inline):
http://cr.openjdk.java.net/~iignatyev//8210112/webrev.0-1/test/jdk/sun/tools/jcmd/TestJcmdSanity.java.udiff.html

-
http://cr.openjdk.java.net/~iignatyev//8210112/webrev.0-1/test/lib/jdk/test/lib/process/OutputAnalyzer.java.udiff.html
"There is no lines" -> "There are no lines"
- What is the advantage of having the return at all now for the
shouldMatch methods, if it fails it throws, the test fails; otherwise it
doesn't return anything, the test can move on, no? I saw no moment when you
get the return to do something more with it

Thanks for the incremental webrev, that made looking at the changes so much
easier!
Jc

On Wed, Sep 5, 2018 at 2:32 PM Igor Ignatyev 
wrote:

> Hi JC,
>
> thanks for reviewing this! I agree w/ both your comments and have updated
> the code very similarly to your suggestion.
>
> I've also noticed that j.t.l.p.OutputAnalyzer::shouldMatchByLine method
> family is a bit different from other should* (and strange), besides
> checking that the lines match the pattern, shouldMatchByLine methods do not
> check that it's greater than zero and return number of matched lines
> instead. however all users of these methods do check that the return
> results is non zero. I have updated these methods to check that there are
> lines to match and updated all their users correspondingly. Doing that, I
> also made some harmless refactoring, like moving Pattern::compile from
> loops, using "\R" as end-of-line pattern.
>
> incremental webrev:
> http://cr.openjdk.java.net/~iignatyev//8210112/webrev.0-1/index.html
>
> Thanks,
> -- Igor
>
> On Sep 4, 2018, at 8:01 PM, JC Beyler  wrote:
>
> Hi Igor,
>
> I reviewed the webrev but I noticed two things:
>
> - Small nit:
>   - In
> http://cr.openjdk.java.net/~iignatyev//8210112/webrev.00/test/lib/jdk/test/lib/process/ProcessTools.java.udiff.html
>  - I thought we don't have to flush as the stream gets closed and by
> closing flushes the stream, isn't that redundant then?
>
> -
> http://cr.openjdk.java.net/~iignatyev//8210112/webrev.00/test/lib/jdk/test/lib/process/OutputBuffer.java.udiff.html
>   - Seems we could refactor a bit this no?
>  - If we put the Future and ByteArrayOutputStream in a separate class
> (ex TaskStream), then the constructor and the getters could be factorized:
>
> class TaskStream {
>private final ByteArrayOutputStream buffer;
>private Future task;
>
>public TaskStream(InputStream stream) {
>   buffer = new ByteArrayOutputStream();
>   task = new StreamPumper(stream, buffer).process();
>}
>
> public String getBuffer() {
>   try {
> task.get();
> return buffer.toString();
>   } catch (InterruptedException e) {
> Thread.currentThread().interrupt();
> throw new OutputBufferException(e);
>   } catch (ExecutionException | CancellationException e) {
> throw new OutputBufferException(e);
>   }
> }
> }
>
> +  class LazyOutputBuffer implements OutputBuffer {
>
> +private final TaskStream stderr;
>
> +private final TaskStream stdout;
>
> +private final Process p;++private LazyOutputBuffer(Process p) {+ 
>  this.p = p;
>
> +  stderr = new TaskStream(p.getInputStream());
>
> +  stdout = new TaskStream(p.getErrorStream());+}++@Override+
> public String getStdout() {
>
> +  return stdout.getBuffer();
>
> +}+@Override+public String getStderr() {
>
> +   return stderr.getBuffer()+}
>
>
> I think it is more clear, what do you think?
>
>
> Apart from those two elements, it looks good to me :), nice refactor!
>
> Jc
>
>
>
> On Tue, Sep 4, 2018 at 6:33 PM Igor Ignatyev 
> wrote:
>
>> http://cr.openjdk.java.net/~iignatyev//8210112/webrev.00/index.html
>> > 2375 lines changed: 322 ins; 1662 del; 391 mod
>>
>> Hi all,
>>
>> could you please review the patch which removes
>> jdk.testlibrary.ProcessTools and its friends and replaces all theirs usages
>> w/ corresponding classes from jdk.test.lib.process?
>>
>> there were a few differences b/w implementations which are addressed by
>> the patch:
>>  - j.t.l.p.ProcessTools missed executeProcess(ProcessBuilder, String)
>> method
>>  - j.t.l.p.OutputAnalyzer didn't have shouldMatchByLine methods family
>>  - j.t.l.p.OutputBuffer was a very rudimentary and 

Re: RFR(L/M) : 8210112 : remove jdk.testlibrary.ProcessTools

[Igor Ignatyev]

Hi JC,

thanks for reviewing this! I agree w/ both your comments and have updated the 
code very similarly to your suggestion.

I've also noticed that j.t.l.p.OutputAnalyzer::shouldMatchByLine method family 
is a bit different from other should* (and strange), besides checking that the 
lines match the pattern, shouldMatchByLine methods do not check that it's 
greater than zero and return number of matched lines instead. however all users 
of these methods do check that the return results is non zero. I have updated 
these methods to check that there are lines to match and updated all their 
users correspondingly. Doing that, I also made some harmless refactoring, like 
moving Pattern::compile from loops, using "\R" as end-of-line pattern.

incremental webrev: 
http://cr.openjdk.java.net/~iignatyev//8210112/webrev.0-1/index.html

Thanks,
-- Igor

> On Sep 4, 2018, at 8:01 PM, JC Beyler  wrote:
> 
> Hi Igor,
> 
> I reviewed the webrev but I noticed two things:
> 
> - Small nit:
>   - In 
> http://cr.openjdk.java.net/~iignatyev//8210112/webrev.00/test/lib/jdk/test/lib/process/ProcessTools.java.udiff.html
>  
> 
>  - I thought we don't have to flush as the stream gets closed and by 
> closing flushes the stream, isn't that redundant then?
> 
> - 
> http://cr.openjdk.java.net/~iignatyev//8210112/webrev.00/test/lib/jdk/test/lib/process/OutputBuffer.java.udiff.html
>  
> 
>   - Seems we could refactor a bit this no?
>  - If we put the Future and ByteArrayOutputStream in a separate class (ex 
> TaskStream), then the constructor and the getters could be factorized:
> 
> class TaskStream {
>private final ByteArrayOutputStream buffer;
>private Future task;
> 
>public TaskStream(InputStream stream) {
>   buffer = new ByteArrayOutputStream();
>   task = new StreamPumper(stream, buffer).process();
>}
> 
> public String getBuffer() {
>   try {
> task.get();
> return buffer.toString();
>   } catch (InterruptedException e) {
> Thread.currentThread().interrupt();
> throw new OutputBufferException(e);
>   } catch (ExecutionException | CancellationException e) {
> throw new OutputBufferException(e);
>   }
> }
> }
> +  class LazyOutputBuffer implements OutputBuffer {
> +private final TaskStream stderr;
> +private final TaskStream stdout;
> +private final Process p;
> +
> +private LazyOutputBuffer(Process p) {
> +  this.p = p;
> +  stderr = new TaskStream(p.getInputStream());
> +  stdout = new TaskStream(p.getErrorStream());
> +}
> +
> +@Override
> +public String getStdout() {
> +  return stdout.getBuffer();
> +}
> 
> +@Override
> +public String getStderr() {
> +   return stderr.getBuffer()
> +}
> 
> I think it is more clear, what do you think?
> 
> Apart from those two elements, it looks good to me :), nice refactor!
> Jc
> 
> 
> On Tue, Sep 4, 2018 at 6:33 PM Igor Ignatyev  > wrote:
> http://cr.openjdk.java.net/~iignatyev//8210112/webrev.00/index.html 
> 
> > 2375 lines changed: 322 ins; 1662 del; 391 mod
> 
> Hi all,
> 
> could you please review the patch which removes jdk.testlibrary.ProcessTools 
> and its friends and replaces all theirs usages w/ corresponding classes from 
> jdk.test.lib.process?
> 
> there were a few differences b/w implementations which are addressed by the 
> patch:
>  - j.t.l.p.ProcessTools missed executeProcess(ProcessBuilder, String) method
>  - j.t.l.p.OutputAnalyzer didn't have shouldMatchByLine methods family
>  - j.t.l.p.OutputBuffer was a very rudimentary and didn't serve any purposes, 
> while j.t.OutputBuffer provided lazy access to a process's cout, cerr and 
> exitcode. I have changed j.t.l.p.OutputBuffer to be an interface w/ two 
> implementations LazyOutputBuffer and EagerOutputBuffer, and updated 
> j.t.l.p.OutputAnalyzer to get values from an OutputBuffer instead of storing 
> them.
>  - j.t.l.p.ProcessTools::createJavaProcessBuilder always adds '-cp', but 
> j.t.ProcessTools::createJavaProcessBuilder did not. I have identified tests 
> which really depend on absence of '-cp' and updated them to create 
> ProcessBuilder directly, namely JavaClassPathTest and 
> AppendToClassPathModuleTest.
> 
> the rest of the patch is straightforward change of used classes w/ adding 
> @library /test/lib if necessary and removing @library /lib/testlibrary if 
> possible.  
> 
> webrev: http://cr.openjdk.java.net/~iignatyev//8210112/webrev.00/index.html 
> 
> testing: tier1-tier3 + :jdk_svc
> JBS: https://bugs.openjdk.java.net/browse/JDK-8210112 

RFR(L/M) : 8210112 : remove jdk.testlibrary.ProcessTools

[Igor Ignatyev]

http://cr.openjdk.java.net/~iignatyev//8210112/webrev.00/index.html
> 2375 lines changed: 322 ins; 1662 del; 391 mod

Hi all,

could you please review the patch which removes jdk.testlibrary.ProcessTools 
and its friends and replaces all theirs usages w/ corresponding classes from 
jdk.test.lib.process?

there were a few differences b/w implementations which are addressed by the 
patch:
 - j.t.l.p.ProcessTools missed executeProcess(ProcessBuilder, String) method
 - j.t.l.p.OutputAnalyzer didn't have shouldMatchByLine methods family
 - j.t.l.p.OutputBuffer was a very rudimentary and didn't serve any purposes, 
while j.t.OutputBuffer provided lazy access to a process's cout, cerr and 
exitcode. I have changed j.t.l.p.OutputBuffer to be an interface w/ two 
implementations LazyOutputBuffer and EagerOutputBuffer, and updated 
j.t.l.p.OutputAnalyzer to get values from an OutputBuffer instead of storing 
them.
 - j.t.l.p.ProcessTools::createJavaProcessBuilder always adds '-cp', but 
j.t.ProcessTools::createJavaProcessBuilder did not. I have identified tests 
which really depend on absence of '-cp' and updated them to create 
ProcessBuilder directly, namely JavaClassPathTest and 
AppendToClassPathModuleTest.

the rest of the patch is straightforward change of used classes w/ adding 
@library /test/lib if necessary and removing @library /lib/testlibrary if 
possible.  

webrev: http://cr.openjdk.java.net/~iignatyev//8210112/webrev.00/index.html
testing: tier1-tier3 + :jdk_svc
JBS: https://bugs.openjdk.java.net/browse/JDK-8210112

Thanks,
-- Igor