Looks good to me Regards, Manajit
> On 14-May-2018, at 10:38 PM, Phil Race <[email protected]> wrote: > > This works on my system. > > +1 > > -phil. > > On 05/10/2018 01:30 PM, Pankaj Bansal wrote: >> Hi Phil, >> >> Thanks for the quick review. >> >> <<Execing xrandr twice ? Can't we either just re-write the code to not use >> takeWhile() which >> <<is part of the problem, or less desirably store the results into an >> internal buffer >> <<and read it from there using StringReader/StringWriter - wrapped in >> BufferedReader/BufferedWriter ? >> Yes that makes much more sense. I have removed takeWhile as I think filter >> should do the work here. There will not be need to ignore any line. <> >> I have also used the try-with-resource as suggested. >> Webrev: >> http://cr.openjdk.java.net/~pbansal/8196616/webrev.01/ >> <http://cr.openjdk.java.net/%7Epbansal/8196616/webrev.01/> >> >> >> Regards, >> Pankaj Bansal >> From: Phil Race >> Sent: Friday, May 11, 2018 12:59 AM >> To: Pankaj Bansal; [email protected] <mailto:[email protected]> >> Subject: Re: [11] Review Request: JDK-8196616 >> java/awt/GraphicsDevice/DisplayModes/CompareToXrandrTest.java fails >> >> Execing xrandr twice ? Can't we either just re-write the code to not use >> takeWhile() which >> is part of the problem, or less desirably store the results into an internal >> buffer >> and read it from there using StringReader/StringWriter - wrapped in >> BufferedReader/BufferedWriter ? >> >> Also I think you should use try-with-resources on the reading from the >> "real" stream. >> >> And a style point : >> while((line = reader.readLine()) != null) { >> -> >> while ((line = reader.readLine()) != null) { >> >> -phil. >> >> >> >> On 05/10/2018 12:21 PM, Pankaj Bansal wrote: >> Hi All, Please review test fix for the below bug: >> >> Bug: https://bugs.openjdk.java.net/browse/JDK-8196616 >> <https://bugs.openjdk.java.net/browse/JDK-8196616> >> >> Webrev: http://cr.openjdk.java.net/~pbansal/8196616/webrev.00/ >> <http://cr.openjdk.java.net/%7Epbansal/8196616/webrev.00/> >> >> >> The test case create a BufferedReader and this BufferedReader is used to >> find XRanderModes and JavaModes and compare them. >> It is assumed in this test that the first two lines of the BufferedReader >> don’t have the useful mode data and contain some other information. So it >> ignores first two lines. But this is not always true and test may have to >> ignore more or less lines depending upon the data. >> >> Made changes to the test to check how many lines should be ignored, instead >> of hard coding the number of lines to ignore. >> >> Regards, >> Pankaj Bansal >> >> > >
