RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v21]

[Andrew Leonard]

On Thu, 9 Dec 2021 18:40:01 GMT, Lance Andersen <lancea at openjdk.org> wrote:

>> Andrew Leonard has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8276766: Enable jar and jmod to produce deterministic timestamped content
>>   
>>   Signed-off-by: Andrew Leonard <anleonar at redhat.com>
>
> test/jdk/tools/jar/ReproducibleJar.java line 30:
> 
>> 28:  *          reproducible
>> 29:  * @modules jdk.jartool
>> 30:  * @run testng ReproducibleJar
> 
> Probably use othervm given you are toggling the TZ for extra safety

done

> test/jdk/tools/jar/ReproducibleJar.java line 49:
> 
>> 47: import java.time.LocalDateTime;
>> 48: import java.time.ZonedDateTime;
>> 49: import java.time.ZoneId;
> 
> Duplicate import?  Perhaps get rid to the import of line 43

done

> test/jdk/tools/jar/ReproducibleJar.java line 102:
> 
>> 100:         jarFileSourceDate1.delete();
>> 101:         jarFileSourceDate2.delete();
>> 102:         TimeZone.setDefault(TZ);
> 
> I believe you could just call runAfter() from within this method

done

> test/jdk/tools/jar/ReproducibleJar.java line 120:
> 
>> 118:         // Test --date source date
>> 119:         for (String sourceDate : sourceDates) {
>> 120:             createOuterInnerDirs(dirOuter, dirInner);
> 
> If you use a DataProvider,  you could call this method from your runBeforeMethod

done

> test/jdk/tools/jar/ReproducibleJar.java line 147:
> 
>> 145:             cleanup(dirInner);
>> 146:             cleanup(dirOuter);
>> 147:             jarFileSourceDate1.delete();
> 
> Is the above needed given your  `@AfterMethod`

done

> test/jdk/tools/jar/ReproducibleJar.java line 152:
> 
>> 150: 
>> 151:     @Test
>> 152:     public void testInvalidSourceDate() throws Throwable {
> 
> Please add a basic comment of the intent of the method and consider using a DataProvider

done

> test/jdk/tools/jar/ReproducibleJar.java line 165:
> 
>> 163: 
>> 164:     @Test
>> 165:     public void testJarsReproducible() throws Throwable {
> 
> Please add a basic comment of the intent of the method and consider using a DataProvider

done

> test/jdk/tools/jar/ReproducibleJar.java line 199:
> 
>> 197:             jarFileSourceDate1.delete();
>> 198:             jarFileSourceDate2.delete();
>> 199:         }
> 
> I believe your` @AfterMethod` will address this so the above is not needed

done

> test/jdk/tools/jar/ReproducibleJar.java line 202:
> 
>> 200:     }
>> 201: 
>> 202:     static void createOuterInnerDirs(File dirOuter, File dirInner) throws Throwable {
> 
> I believe you are always passing in the same parameter values, so perhaps no need for the parameters?

done

> test/jdk/tools/jar/ReproducibleJar.java line 208:
> 
>> 206:          *         foo.txt
>> 207:          */
>> 208:         Assert.assertTrue(dirOuter.mkdir());
> 
> Please move the  comment above the method

done

> test/jdk/tools/jar/ReproducibleJar.java line 249:
> 
>> 247:     }
>> 248: 
>> 249:     private static boolean isTimeSettingChanged() {
> 
> Please add a basic comment describing the intent of the method

done

> test/jdk/tools/jar/ReproducibleJar.java line 260:
> 
>> 258:     }
>> 259: 
>> 260:     private static boolean isInTransition() {
> 
> Please add a basic comment of the intent of the method

done

> test/jdk/tools/jar/ReproducibleJar.java line 278:
> 
>> 276:         File[] x = dir.listFiles();
>> 277:         if (x != null) {
>> 278:             for (int i = 0; i < x.length; i++) {
> 
> Could use enhanced for loop here if you desire

done

-------------

PR: https://git.openjdk.java.net/jdk/pull/6481