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

[Sergey Bylokhov]

On 12/01/16 19:53, Alexander Stepanov wrote:
> 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.

yes but in past we had a situations when such assumption was broken in 
some new writers/readers: JDK-8146887, JDK-8144245, JDK-8145240. And the 
bugs were uncovered because the tests verify all installed providers.

>
> 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.
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>


-- 
Best regards, Sergey.