Resend RFR 8231766: Files.copy/move do not honor requested compression method when copying/moving within the same zip file
Lance Andersen
lance.andersen at oracle.com
Thu Oct 24 12:21:52 UTC 2019
Hi Brian,
Thank you for the review.
> On Oct 23, 2019, at 8:03 PM, Brian Burkhalter <brian.burkhalter at oracle.com> wrote:
>
> Hi Lance,
>
> Please see comments on the test below listed by line numbers.
>
> Brian
>
>> On Oct 23, 2019, at 1:49 PM, Alan Bateman <Alan.Bateman at oracle.com <mailto:Alan.Bateman at oracle.com>> wrote:
>>
>> On 23/10/2019 18:48, Lance Andersen wrote:
>>> Thanks for the suggestion Alan.
>>>
>>> Webrev updated: http://cr.openjdk.java.net/~lancea/8231766/webrev.01/index.html <http://cr.openjdk.java.net/~lancea/8231766/webrev.01/index.html>
>>>
>>>
>> Thanks, looks much simpler. The test is big and I don't have time to go through that in detail right now. Maybe Christoph or someone else might have cycles to help.
>
> 114
> How can verify() return true if compression != expectedCompression? It looks like at 111 the contents of e0 are copied with e00’s name but retain e0’s compression method. Probably I am missing something.
The initial zip is created just adding e0 and e1 and the compression used for adding these files is determined via the setting of the no compression property (or default value) when the Zip FS is created/accessed.
The expected compression for e00 will match what the targetCompression is set to when the ZipFS is being created to update the file.
>
> 148
> Similar question as for line 114.
>
> 251
> Similar question as for line 114
>
> 286
> Similar question as for line 114
>
> 336
> Could TestNG “expectedExceptions” parameter of the @Test annotation be used instead of catching the FAE (and put line 352 in a finally block)?
>
Past reviews have suggested to move away from expectedException annotation element and use assertThrows() which the reason I went this way.
> 363-371
> Could Files.createTempFile() be used instead of having this method?
In this case, I am not creating the file, just generating a path name
>
> 447
> Line commented out
>
Thank you. I will remove(thought I had)
> 110, 144, 181, 247, 280, 347, 399
> Indentation
I ran the intellij reformat code command, but can do it again.
Best
Lance
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
Lance.Andersen at oracle.com <mailto:Lance.Andersen at oracle.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/nio-dev/attachments/20191024/77400124/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: oracle_sig_logo.gif
Type: image/gif
Size: 658 bytes
Desc: not available
URL: <https://mail.openjdk.java.net/pipermail/nio-dev/attachments/20191024/77400124/oracle_sig_logo.gif>
More information about the nio-dev
mailing list