[OpenJDK 2D-Dev] RFR: 8145776: [TEST] add a test checking multipage tiff creation

[Alexander Stepanov]

the unused import was removed (please see 
http://cr.openjdk.java.net/~avstepan/8145776/webrev.02/test/javax/imageio/plugins/tiff/MultiPageTest/MultiPageTest.java.html)

 > canWriteRasters should be checked also.
it returns false in the test. canInsertImage(index) should be checked 
before writeInsert(), and it returns false for jpg, gif, png, bmp 
(moreover, we can't write in sequence for bmp and png, and for jpeg 
checking of the colors equality requires some tolerance), thus the 
checking "black" images could be inserted for tiff only.

So it seems the test logic could be used "as is" for the tiff format 
only (as intended), and I'd like to keep it without sizable changes.

Regards,
Alexander

On 1/12/2016 6:18 PM, Alexander Stepanov wrote:
> In such a case (canWriteSequence() && canWriteRasters()), I guess, we 
> return to the sole TIFF :)
>
> On 1/12/2016 6:14 PM, Sergey Bylokhov wrote:
>> On 12/01/16 18:08, Alexander Stepanov wrote:
>>>  > applicable to all image types which canWriteSequence=true?
>>> I'm not sure. E.g., for "gif" canWriteSequence() returns "true", but
>>> UnsupportedOperationException is thrown when calling writeInsert() (I
>>> have to double-check).
>>
>> canWriteRasters should be checked also.
>>
>>>
>>> (moreover, not sure if the gif created in such a way could be called
>>> "multipage" in the same sense as tiff).
>>>
>>>  > "import javax.imageio.plugins.tiff.*" is unnecessary
>>> yes, it should be removed, thanks.
>>>
>>> Regards,
>>> Alexander
>>>
>>> On 1/12/2016 5:36 PM, Sergey Bylokhov wrote:
>>>> Since "import javax.imageio.plugins.tiff.*" is unnecessary in the
>>>> test, i guess it is applicable to all image types which
>>>> canWriteSequence=true?
>>>>
>>>> On 11/01/16 14:49, Alexander Stepanov wrote:
>>>>> Hello Semyon,
>>>>>
>>>>> Thank you, that eliminates the question with the internal package.
>>>>> Probably Brian meant the same (but I just didn't understand this at
>>>>> first).
>>>>>
>>>>> Please review the updated webrev:
>>>>> http://cr.openjdk.java.net/~avstepan/8145776/webrev.02/
>>>>>
>>>>> Thanks,
>>>>> Alexander
>>>>>
>>>>> On 1/11/2016 11:59 AM, Semyon Sadetsky wrote:
>>>>>> Hello Alexander,
>>>>>>
>>>>>> Why don't use the reader method analogue for writer? Then the test
>>>>>> could use the public API which is preferable.
>>>>>>
>>>>>> Just a cosmetic note. Lines should be wrapped at 80 position.
>>>>>>
>>>>>> --Semyon
>>>>>>
>>>>>> On 12/29/2015 3:44 PM, Alexander Stepanov wrote:
>>>>>>> Hello Brian,
>>>>>>>
>>>>>>> Thank you for the notes, please see the updated webrev:
>>>>>>> http://cr.openjdk.java.net/~yan/8145776/webrev.01/
>>>>>>>
>>>>>>> 1., 3. - fixed
>>>>>>> WRT 2.: this import is necessary to use TIFFImageWriter,
>>>>>>> TIFFImageWriterSpi. As it follows from
>>>>>>> http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/07ae3247e988, the
>>>>>>> majority of new classes (excepting tag sets) were added to
>>>>>>> com.sun.imageio.plugins.tiff. Do you mean these classes would be
>>>>>>> moved to javax/imageio soon? (please note also a @module tag added
>>>>>>> for compatibility with modular java).
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Alexander
>>>>>>>
>>>>>>> On 12/29/2015 2:12 AM, Brian Burkhalter wrote:
>>>>>>>> Hello Alexander,
>>>>>>>>
>>>>>>>> A few comments.
>>>>>>>>
>>>>>>>> 1) Lines 53,59: By convention it is better to put constants in 
>>>>>>>> upper
>>>>>>>> case, e.g., NUM_IMAGES and BLACK_SIZE.
>>>>>>>>
>>>>>>>> 2) Line 45, 102: The test references the internal package
>>>>>>>> com.sun.imageio.plugins.tiff. In general it is better to use the
>>>>>>>> public packages.
>>>>>>>>
>>>>>>>> 3) Line 69: If the test fails, it would be good to know the random
>>>>>>>> seed. It would also be good to be able to set the seed when 
>>>>>>>> running
>>>>>>>> the test. The approach taken in core libraries may be seen in
>>>>>>>> test/java/math/BigInteger/BigIntegerTest.java. The
>>>>>>>> jdk.testlibrary.RandomFactory class is used to create the Random
>>>>>>>> instance and to obtain any seed provided via Java properties. This
>>>>>>>> requires the test tags “@library /lib/testlibrary” and “@build
>>>>>>>> jdk.testlibrary.*” and it is also good to add “@key randomness.”
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>>
>>>>>>>> Brian
>>>>>>>>
>>>>>>>> On Dec 25, 2015, at 5:10 AM, Alexander Stepanov
>>>>>>>> <alexander.v.stepanov at oracle.com> wrote:
>>>>>>>>
>>>>>>>>> Could you please review the following fix
>>>>>>>>> http://cr.openjdk.java.net/~yan/8145776/webrev.00/
>>>>>>>>> <http://cr.openjdk.java.net/%7Eyan/8145776/webrev.00/>
>>>>>>>>> for
>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8145776
>>>>>>>>>
>>>>>>>>> Just a single test added checking the correctness of multi-page
>>>>>>>>> TIFF image creation.
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>>
>