Skip to content

Commit 4ad8582

Browse files
Lukasz Kostyrakevinrushforth
Lukasz Kostyra
authored and
kevinrushforth
committedNov 29, 2022
8265828: [TestBug] Save and restore the default Locale in javafx.base unit test LocalDateTimeStringConverterTest
Reviewed-by: kcr
·
25+820-ga
1 parent d040c1f commit 4ad8582

File tree

3 files changed

+230
-62
lines changed

3 files changed

+230
-62
lines changed
 

‎modules/javafx.base/src/test/java/test/javafx/util/converter/LocalDateStringConverterTest.java

+77-20
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,10 @@
3737
import javafx.util.converter.LocalDateStringConverterShim;
3838

3939
import static org.junit.Assert.*;
40+
41+
import org.junit.AfterClass;
4042
import org.junit.Before;
43+
import org.junit.BeforeClass;
4144
import org.junit.Test;
4245
import org.junit.runner.RunWith;
4346
import org.junit.runners.Parameterized;
@@ -48,42 +51,96 @@
4851
public class LocalDateStringConverterTest {
4952
private static final LocalDate VALID_DATE = LocalDate.of(1985, 1, 12);
5053

51-
private static final DateTimeFormatter aFormatter = DateTimeFormatter.ofPattern("dd MM yyyy");
52-
private static final DateTimeFormatter aParser = DateTimeFormatter.ofPattern("yyyy MM dd");
53-
54+
private static Locale oldLocale = null;
55+
private static DateTimeFormatter aFormatter = null;
56+
private static DateTimeFormatter aParser = null;
57+
58+
// We can only create LocalDateStringConverter object after Locale is set.
59+
// Unfortunately, due to unpredictability of @Parameterized.Parameters methods
60+
// in JUnit, we have to allocate it after @BeforeClass sets up Locale and
61+
// necessary static fields. Otherwise, the test may collide with other
62+
// Local*StringConverter tests and cause unpredictable results.
63+
private enum LocalDateStringConverterVariant {
64+
NO_PARAM,
65+
WITH_FORMATTER_PARSER,
66+
WITH_FORMAT_STYLES,
67+
};
5468

5569
@Parameterized.Parameters public static Collection implementations() {
5670
return Arrays.asList(new Object[][] {
57-
{ new LocalDateStringConverter(),
58-
Locale.getDefault(Locale.Category.FORMAT), FormatStyle.SHORT,
59-
VALID_DATE, null, null },
71+
{ LocalDateStringConverterVariant.NO_PARAM,
72+
FormatStyle.SHORT, VALID_DATE },
6073

61-
{ new LocalDateStringConverter(aFormatter, aParser),
62-
Locale.getDefault(Locale.Category.FORMAT), null,
63-
VALID_DATE, aFormatter, aParser },
74+
{ LocalDateStringConverterVariant.WITH_FORMATTER_PARSER,
75+
null, VALID_DATE },
6476

65-
{ new LocalDateStringConverter(FormatStyle.SHORT, Locale.UK, IsoChronology.INSTANCE),
66-
Locale.UK, FormatStyle.SHORT,
67-
VALID_DATE, null, null },
77+
{ LocalDateStringConverterVariant.WITH_FORMAT_STYLES,
78+
FormatStyle.SHORT, VALID_DATE },
6879
});
6980
}
7081

82+
private LocalDateStringConverterVariant converterVariant;
83+
private FormatStyle dateStyle;
84+
private LocalDate validDate;
85+
7186
private LocalDateStringConverter converter;
7287
private Locale locale;
73-
private FormatStyle dateStyle;
7488
private DateTimeFormatter formatter, parser;
75-
private LocalDate validDate;
7689

77-
public LocalDateStringConverterTest(LocalDateStringConverter converter, Locale locale, FormatStyle dateStyle, LocalDate validDate, DateTimeFormatter formatter, DateTimeFormatter parser) {
78-
this.converter = converter;
79-
this.locale = locale;
90+
public LocalDateStringConverterTest(LocalDateStringConverterVariant converterVariant, FormatStyle dateStyle, LocalDate validDate) {
91+
this.converterVariant = converterVariant;
8092
this.dateStyle = dateStyle;
8193
this.validDate = validDate;
82-
this.formatter = formatter;
83-
this.parser = parser;
94+
95+
// initialized after Locale is established
96+
this.converter = null;
97+
this.locale = null;
98+
this.formatter = null;
99+
this.parser = null;
100+
}
101+
102+
@BeforeClass
103+
public static void setupBeforeAll() {
104+
// Tests require that default locale is en_US
105+
oldLocale = Locale.getDefault();
106+
Locale.setDefault(Locale.US);
107+
108+
// DateTimeFormatter uses default locale, so we can init this after updating locale
109+
aFormatter = DateTimeFormatter.ofPattern("dd MM yyyy");
110+
aParser = DateTimeFormatter.ofPattern("yyyy MM dd");
84111
}
85112

86-
@Before public void setup() {
113+
@AfterClass
114+
public static void teardownAfterAll() {
115+
// Restore VM's old locale
116+
Locale.setDefault(oldLocale);
117+
}
118+
119+
@Before
120+
public void setup() {
121+
// Locale is established now, so we can allocate objects depending on it
122+
switch (this.converterVariant) {
123+
case NO_PARAM:
124+
this.converter = new LocalDateStringConverter();
125+
this.locale = Locale.getDefault(Locale.Category.FORMAT);
126+
this.formatter = null;
127+
this.parser = null;
128+
break;
129+
case WITH_FORMATTER_PARSER:
130+
this.converter = new LocalDateStringConverter(aFormatter, aParser);
131+
this.locale = Locale.getDefault(Locale.Category.FORMAT);
132+
this.formatter = aFormatter;
133+
this.parser = aParser;
134+
break;
135+
case WITH_FORMAT_STYLES:
136+
this.converter = new LocalDateStringConverter(FormatStyle.SHORT, Locale.UK, IsoChronology.INSTANCE);
137+
this.locale = Locale.UK;
138+
this.formatter = null;
139+
this.parser = null;
140+
break;
141+
default:
142+
fail("Invalid converter variant: " + this.converterVariant.toString());
143+
}
87144
}
88145

89146
/*********************************************************************

‎modules/javafx.base/src/test/java/test/javafx/util/converter/LocalDateTimeStringConverterTest.java

+79-22
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@
4040
import javafx.util.converter.LocalDateTimeStringConverter;
4141
import javafx.util.converter.LocalDateTimeStringConverterShim;
4242

43+
import org.junit.AfterClass;
44+
import org.junit.Before;
45+
import org.junit.BeforeClass;
4346
import org.junit.Test;
4447
import org.junit.runner.RunWith;
4548
import org.junit.runners.Parameterized;
@@ -53,44 +56,98 @@ public class LocalDateTimeStringConverterTest {
5356
private static final LocalDateTime VALID_LDT_WITH_SECONDS = LocalDateTime.of(1985, 1, 12, 12, 34, 56);
5457
private static final LocalDateTime VALID_LDT_WITHOUT_SECONDS = LocalDateTime.of(1985, 1, 12, 12, 34, 0);
5558

56-
private static final DateTimeFormatter aFormatter = DateTimeFormatter.ofPattern("dd MM yyyy HH mm ss");
57-
private static final DateTimeFormatter aParser = DateTimeFormatter.ofPattern("yyyy MM dd hh mm ss a");
59+
private static DateTimeFormatter aFormatter;
60+
private static DateTimeFormatter aParser;
61+
private static Locale oldLocale;
62+
63+
// We can only create LocalDateTimeStringConverter object after Locale is set.
64+
// Unfortunately, due to unpredictability of @Parameterized.Parameters methods
65+
// in JUnit, we have to allocate it after @BeforeClass sets up Locale and
66+
// necessary static fields. Otherwise, the test may collide with other
67+
// Local*StringConverter tests and cause unpredictable results.
68+
private enum LocalDateTimeStringConverterVariant {
69+
NO_PARAM,
70+
WITH_FORMATTER_PARSER,
71+
WITH_FORMAT_STYLES,
72+
};
5873

5974
@Parameterized.Parameters public static Collection implementations() {
60-
// Tests require that default locale is en_US
61-
Locale.setDefault(Locale.US);
62-
6375
return Arrays.asList(new Object[][] {
64-
{ new LocalDateTimeStringConverter(),
65-
Locale.getDefault(Locale.Category.FORMAT), FormatStyle.SHORT, FormatStyle.SHORT,
66-
VALID_LDT_WITHOUT_SECONDS, null, null },
76+
{ LocalDateTimeStringConverterVariant.NO_PARAM,
77+
FormatStyle.SHORT, FormatStyle.SHORT, VALID_LDT_WITHOUT_SECONDS},
6778

68-
{ new LocalDateTimeStringConverter(aFormatter, aParser),
69-
Locale.getDefault(Locale.Category.FORMAT), null, null,
70-
VALID_LDT_WITH_SECONDS, aFormatter, aParser },
79+
{ LocalDateTimeStringConverterVariant.WITH_FORMATTER_PARSER,
80+
null, null, VALID_LDT_WITH_SECONDS},
7181

72-
{ new LocalDateTimeStringConverter(FormatStyle.SHORT, FormatStyle.SHORT, Locale.UK, IsoChronology.INSTANCE),
73-
Locale.UK, FormatStyle.SHORT, FormatStyle.SHORT,
74-
VALID_LDT_WITHOUT_SECONDS, null, null },
82+
{ LocalDateTimeStringConverterVariant.WITH_FORMAT_STYLES,
83+
FormatStyle.SHORT, FormatStyle.SHORT, VALID_LDT_WITHOUT_SECONDS},
7584
});
7685
}
7786

78-
private LocalDateTimeStringConverter converter;
79-
private Locale locale;
87+
private LocalDateTimeStringConverterVariant converterVariant;
8088
private FormatStyle dateStyle;
8189
private FormatStyle timeStyle;
90+
private LocalDateTime validDateTime;
91+
92+
private LocalDateTimeStringConverter converter;
93+
private Locale locale;
8294
private DateTimeFormatter formatter, parser;
8395

84-
private LocalDateTime validDateTime;
8596

86-
public LocalDateTimeStringConverterTest(LocalDateTimeStringConverter converter, Locale locale, FormatStyle dateStyle, FormatStyle timeStyle, LocalDateTime validDateTime, DateTimeFormatter formatter, DateTimeFormatter parser) {
87-
this.converter = converter;
88-
this.locale = locale;
97+
public LocalDateTimeStringConverterTest(LocalDateTimeStringConverterVariant converterVariant, FormatStyle dateStyle, FormatStyle timeStyle, LocalDateTime validDateTime) {
98+
this.converterVariant = converterVariant;
8999
this.dateStyle = dateStyle;
90100
this.timeStyle = timeStyle;
91101
this.validDateTime = validDateTime;
92-
this.formatter = formatter;
93-
this.parser = parser;
102+
103+
this.converter = null;
104+
this.locale = null;
105+
this.formatter = null;
106+
this.parser = null;
107+
}
108+
109+
@BeforeClass
110+
public static void setupBeforeAll() {
111+
// Tests require that default locale is en_US
112+
oldLocale = Locale.getDefault();
113+
Locale.setDefault(Locale.US);
114+
115+
// DateTimeFormatter uses default locale, so we can init this after updating locale
116+
aFormatter = DateTimeFormatter.ofPattern("dd MM yyyy HH mm ss");
117+
aParser = DateTimeFormatter.ofPattern("yyyy MM dd hh mm ss a");
118+
}
119+
120+
@AfterClass
121+
public static void teardownAfterAll() {
122+
// Restore VM's old locale
123+
Locale.setDefault(oldLocale);
124+
}
125+
126+
@Before
127+
public void setup() {
128+
// Locale is established now, so we can allocate objects depending on it
129+
switch (this.converterVariant) {
130+
case NO_PARAM:
131+
this.converter = new LocalDateTimeStringConverter();
132+
this.locale = Locale.getDefault(Locale.Category.FORMAT);
133+
this.formatter = null;
134+
this.parser = null;
135+
break;
136+
case WITH_FORMATTER_PARSER:
137+
this.converter = new LocalDateTimeStringConverter(aFormatter, aParser);
138+
this.locale = Locale.getDefault(Locale.Category.FORMAT);
139+
this.formatter = aFormatter;
140+
this.parser = aParser;
141+
break;
142+
case WITH_FORMAT_STYLES:
143+
this.converter = new LocalDateTimeStringConverter(FormatStyle.SHORT, FormatStyle.SHORT, Locale.UK, IsoChronology.INSTANCE);
144+
this.locale = Locale.UK;
145+
this.formatter = null;
146+
this.parser = null;
147+
break;
148+
default:
149+
fail("Invalid converter variant: " + this.converterVariant.toString());
150+
}
94151
}
95152

96153
/*********************************************************************

‎modules/javafx.base/src/test/java/test/javafx/util/converter/LocalTimeStringConverterTest.java

+74-20
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,9 @@
3636
import javafx.util.converter.LocalTimeStringConverter;
3737

3838
import static org.junit.Assert.*;
39+
import org.junit.AfterClass;
3940
import org.junit.Before;
41+
import org.junit.BeforeClass;
4042
import org.junit.Test;
4143
import org.junit.runner.RunWith;
4244
import org.junit.runners.Parameterized;
@@ -53,42 +55,94 @@ public class LocalTimeStringConverterTest {
5355
VALID_TIME_WITHOUT_SECONDS = LocalTime.of(12, 34, 0);
5456
}
5557

56-
private static final DateTimeFormatter aFormatter = DateTimeFormatter.ofPattern("HH mm ss");
57-
private static final DateTimeFormatter aParser = DateTimeFormatter.ofPattern("hh mm ss a");
58-
58+
private static Locale oldLocale = null;
59+
private static DateTimeFormatter aFormatter = null;
60+
private static DateTimeFormatter aParser = null;
61+
62+
// We can only create LocalTimeStringConverter object after Locale is set.
63+
// Unfortunately, due to unpredictability of @Parameterized.Parameters methods
64+
// in JUnit, we have to allocate it after @BeforeClass sets up Locale and
65+
// necessary static fields. Otherwise, the test may collide with other
66+
// Local*StringConverter tests and cause unpredictable results.
67+
private enum LocalTimeStringConverterVariant {
68+
NO_PARAM,
69+
WITH_FORMATTER_PARSER,
70+
WITH_FORMAT_STYLES,
71+
};
5972

6073
@Parameterized.Parameters public static Collection implementations() {
6174
return Arrays.asList(new Object[][] {
62-
{ new LocalTimeStringConverter(),
63-
Locale.getDefault(Locale.Category.FORMAT), FormatStyle.SHORT,
64-
VALID_TIME_WITHOUT_SECONDS, null, null },
75+
{ LocalTimeStringConverterVariant.NO_PARAM,
76+
FormatStyle.SHORT, VALID_TIME_WITHOUT_SECONDS },
6577

66-
{ new LocalTimeStringConverter(aFormatter, aParser),
67-
Locale.getDefault(Locale.Category.FORMAT), null,
68-
VALID_TIME_WITH_SECONDS, aFormatter, aParser },
78+
{ LocalTimeStringConverterVariant.WITH_FORMATTER_PARSER,
79+
null, VALID_TIME_WITH_SECONDS },
6980

70-
{ new LocalTimeStringConverter(FormatStyle.SHORT, Locale.UK),
71-
Locale.UK, FormatStyle.SHORT,
72-
VALID_TIME_WITHOUT_SECONDS, null, null },
81+
{ LocalTimeStringConverterVariant.WITH_FORMAT_STYLES,
82+
FormatStyle.SHORT, VALID_TIME_WITHOUT_SECONDS },
7383
});
7484
}
7585

86+
private LocalTimeStringConverterVariant converterVariant;
87+
private FormatStyle timeStyle;
88+
private LocalTime validTime;
89+
7690
private LocalTimeStringConverter converter;
7791
private Locale locale;
78-
private FormatStyle timeStyle;
7992
private DateTimeFormatter formatter, parser;
80-
private LocalTime validTime;
8193

82-
public LocalTimeStringConverterTest(LocalTimeStringConverter converter, Locale locale, FormatStyle timeStyle, LocalTime validTime, DateTimeFormatter formatter, DateTimeFormatter parser) {
83-
this.converter = converter;
84-
this.locale = locale;
94+
public LocalTimeStringConverterTest(LocalTimeStringConverterVariant converterVariant, FormatStyle timeStyle, LocalTime validTime) {
95+
this.converterVariant = converterVariant;
8596
this.timeStyle = timeStyle;
8697
this.validTime = validTime;
87-
this.formatter = formatter;
88-
this.parser = parser;
98+
99+
this.locale = null;
100+
this.formatter = null;
101+
this.parser = null;
89102
}
90103

91-
@Before public void setup() {
104+
@BeforeClass
105+
public static void setupBeforeAll() {
106+
// Tests require that default locale is en_US
107+
oldLocale = Locale.getDefault();
108+
Locale.setDefault(Locale.US);
109+
110+
// DateTimeFormatter uses default locale, so we can init this after updating locale
111+
aFormatter = DateTimeFormatter.ofPattern("HH mm ss");
112+
aParser = DateTimeFormatter.ofPattern("hh mm ss a");
113+
}
114+
115+
@AfterClass
116+
public static void teardownAfterAll() {
117+
// Restore VM's old locale
118+
Locale.setDefault(oldLocale);
119+
}
120+
121+
@Before
122+
public void setup() {
123+
// Locale is established now, so we can allocate objects depending on it
124+
switch (this.converterVariant) {
125+
case NO_PARAM:
126+
this.converter = new LocalTimeStringConverter();
127+
this.locale = Locale.getDefault(Locale.Category.FORMAT);
128+
this.formatter = null;
129+
this.parser = null;
130+
break;
131+
case WITH_FORMATTER_PARSER:
132+
this.converter = new LocalTimeStringConverter(aFormatter, aParser);
133+
this.locale = Locale.getDefault(Locale.Category.FORMAT);
134+
this.formatter = aFormatter;
135+
this.parser = aParser;
136+
break;
137+
case WITH_FORMAT_STYLES:
138+
this.converter = new LocalTimeStringConverter(FormatStyle.SHORT, Locale.UK);
139+
this.locale = Locale.UK;
140+
this.formatter = null;
141+
this.parser = null;
142+
break;
143+
default:
144+
fail("Invalid converter variant: " + this.converterVariant.toString());
145+
}
92146
}
93147

94148
/*********************************************************************

1 commit comments

Comments
 (1)

openjdk-notifier[bot] commented on Nov 29, 2022

@openjdk-notifier[bot]
openjdk-notifier[bot]
on Nov 29, 2022

Review

Issues

Please sign in to comment.