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

2018-09-07 Thread 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

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

2018-09-07 Thread 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

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

2018-09-06 Thread 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]

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

2018-09-05 Thread 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 > >

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

2018-09-05 Thread 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

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

2018-09-05 Thread 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

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

2018-09-04 Thread 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