[
https://issues.apache.org/jira/browse/TIKA-4327?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18072229#comment-18072229
]
ASF GitHub Bot commented on TIKA-4327:
--------------------------------------
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);
```
> General updates for 4.0.0
> -------------------------
>
> Key: TIKA-4327
> URL: https://issues.apache.org/jira/browse/TIKA-4327
> Project: Tika
> Issue Type: Task
> Reporter: Tilman Hausherr
> Priority: Major
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
