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]
*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

    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


Reply via email to