Copilot commented on code in PR #2753:
URL: https://github.com/apache/tika/pull/2753#discussion_r3057060620
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-image-module/src/test/java/org/apache/tika/parser/image/JpegParserTest.java:
##########
@@ -38,10 +41,24 @@
import org.apache.tika.parser.ParseContext;
import org.apache.tika.parser.Parser;
+@Isolated
public class JpegParserTest extends TikaTest {
static TimeZone CURR_TIME_ZONE = TimeZone.getDefault();
private final Parser parser = new JpegParser();
+ static private TimeZone timezone;
+
+ @BeforeAll
+ static void init() {
+ // metadata-extractor 2.20.0 started making these parsed dates depend
on the JVM default time zone;
+ // force GMT so the assertions remain deterministic across environments
+ timezone = TimeZone.getDefault();
+ TimeZone.setDefault(TimeZone.getTimeZone("GMT"));
+ }
+ @AfterAll
+ static void tearDown() {
+ TimeZone.setDefault(timezone);
Review Comment:
The field name `timezone` is ambiguous (it’s specifically the
original/default timezone being saved for restoration). Renaming it to
something like `originalTimeZone` (and using standard modifier order `private
static`) would make the intent clearer and reduce the chance of incorrect
future use.
```suggestion
private static TimeZone originalTimeZone;
@BeforeAll
static void init() {
// metadata-extractor 2.20.0 started making these parsed dates
depend on the JVM default time zone;
// force GMT so the assertions remain deterministic across
environments
originalTimeZone = TimeZone.getDefault();
TimeZone.setDefault(TimeZone.getTimeZone("GMT"));
}
@AfterAll
static void tearDown() {
TimeZone.setDefault(originalTimeZone);
```
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-image-module/src/test/java/org/apache/tika/parser/image/JpegParserTest.java:
##########
@@ -38,10 +41,24 @@
import org.apache.tika.parser.ParseContext;
import org.apache.tika.parser.Parser;
+@Isolated
public class JpegParserTest extends TikaTest {
static TimeZone CURR_TIME_ZONE = TimeZone.getDefault();
private final Parser parser = new JpegParser();
+ static private TimeZone timezone;
+
+ @BeforeAll
+ static void init() {
+ // metadata-extractor 2.20.0 started making these parsed dates depend
on the JVM default time zone;
+ // force GMT so the assertions remain deterministic across environments
+ timezone = TimeZone.getDefault();
+ TimeZone.setDefault(TimeZone.getTimeZone("GMT"));
+ }
+ @AfterAll
+ static void tearDown() {
+ TimeZone.setDefault(timezone);
Review Comment:
This test now has two separate static fields holding the default timezone
(`CURR_TIME_ZONE` and `timezone`). That duplication makes it easy for the
restore logic to diverge over time. Consider consolidating to a single field
(e.g., keep one 'original default TZ' variable) and, if it never changes after
capture, declare it `private static final` where possible.
```suggestion
private static final TimeZone ORIGINAL_DEFAULT_TIME_ZONE =
TimeZone.getDefault();
private final Parser parser = new JpegParser();
@BeforeAll
static void init() {
// metadata-extractor 2.20.0 started making these parsed dates
depend on the JVM default time zone;
// force GMT so the assertions remain deterministic across
environments
TimeZone.setDefault(TimeZone.getTimeZone("GMT"));
}
@AfterAll
static void tearDown() {
TimeZone.setDefault(ORIGINAL_DEFAULT_TIME_ZONE);
```
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-image-module/src/test/java/org/apache/tika/parser/image/JpegParserTest.java:
##########
@@ -38,10 +41,24 @@
import org.apache.tika.parser.ParseContext;
import org.apache.tika.parser.Parser;
+@Isolated
public class JpegParserTest extends TikaTest {
static TimeZone CURR_TIME_ZONE = TimeZone.getDefault();
private final Parser parser = new JpegParser();
+ static private TimeZone timezone;
+
+ @BeforeAll
+ static void init() {
+ // metadata-extractor 2.20.0 started making these parsed dates depend
on the JVM default time zone;
+ // force GMT so the assertions remain deterministic across environments
+ timezone = TimeZone.getDefault();
+ TimeZone.setDefault(TimeZone.getTimeZone("GMT"));
+ }
+ @AfterAll
+ static void tearDown() {
+ TimeZone.setDefault(timezone);
Review Comment:
The field name `timezone` is ambiguous (it’s specifically the
original/default timezone being saved for restoration). Renaming it to
something like `originalTimeZone` (and using standard modifier order `private
static`) would make the intent clearer and reduce the chance of incorrect
future use.
```suggestion
private static TimeZone originalTimeZone;
@BeforeAll
static void init() {
// metadata-extractor 2.20.0 started making these parsed dates
depend on the JVM default time zone;
// force GMT so the assertions remain deterministic across
environments
originalTimeZone = TimeZone.getDefault();
TimeZone.setDefault(TimeZone.getTimeZone("GMT"));
}
@AfterAll
static void tearDown() {
TimeZone.setDefault(originalTimeZone);
```
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-image-module/src/test/java/org/apache/tika/parser/image/JpegParserTest.java:
##########
@@ -38,10 +41,24 @@
import org.apache.tika.parser.ParseContext;
import org.apache.tika.parser.Parser;
+@Isolated
public class JpegParserTest extends TikaTest {
static TimeZone CURR_TIME_ZONE = TimeZone.getDefault();
private final Parser parser = new JpegParser();
+ static private TimeZone timezone;
+
+ @BeforeAll
+ static void init() {
+ // metadata-extractor 2.20.0 started making these parsed dates depend
on the JVM default time zone;
+ // force GMT so the assertions remain deterministic across environments
+ timezone = TimeZone.getDefault();
+ TimeZone.setDefault(TimeZone.getTimeZone("GMT"));
+ }
+ @AfterAll
+ static void tearDown() {
+ TimeZone.setDefault(timezone);
Review Comment:
The field name `timezone` is ambiguous (it’s specifically the
original/default timezone being saved for restoration). Renaming it to
something like `originalTimeZone` (and using standard modifier order `private
static`) would make the intent clearer and reduce the chance of incorrect
future use.
```suggestion
private static TimeZone originalTimeZone;
@BeforeAll
static void init() {
// metadata-extractor 2.20.0 started making these parsed dates
depend on the JVM default time zone;
// force GMT so the assertions remain deterministic across
environments
originalTimeZone = TimeZone.getDefault();
TimeZone.setDefault(TimeZone.getTimeZone("GMT"));
}
@AfterAll
static void tearDown() {
TimeZone.setDefault(originalTimeZone);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
