On Thu, 9 Dec 2021 18:40:01 GMT, Lance Andersen <lan...@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 <anleo...@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
