Skip to content

Commit

Permalink
8265828: [TestBug] Save and restore the default Locale in javafx.base…
Browse files Browse the repository at this point in the history 
… unit test LocalDateTimeStringConverterTest

Reviewed-by: kcr
  • Loading branch information
@kevinrushforth
Lukasz Kostyra authored and kevinrushforth committed Nov 29, 2022
1 parent d040c1f commit 4ad8582
Show file tree
Hide file tree
Showing 3 changed files with 230 additions and 62 deletions.
97 changes: 77 additions & 20 deletions ...es/javafx.base/src/test/java/test/javafx/util/converter/LocalDateStringConverterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@
import javafx.util.converter.LocalDateStringConverterShim;

import static org.junit.Assert.*;

import org.junit.AfterClass;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
Expand All @@ -48,42 +51,96 @@
public class LocalDateStringConverterTest {
private static final LocalDate VALID_DATE = LocalDate.of(1985, 1, 12);

private static final DateTimeFormatter aFormatter = DateTimeFormatter.ofPattern("dd MM yyyy");
private static final DateTimeFormatter aParser = DateTimeFormatter.ofPattern("yyyy MM dd");

private static Locale oldLocale = null;
private static DateTimeFormatter aFormatter = null;
private static DateTimeFormatter aParser = null;

// We can only create LocalDateStringConverter object after Locale is set.
// Unfortunately, due to unpredictability of @Parameterized.Parameters methods
// in JUnit, we have to allocate it after @BeforeClass sets up Locale and
// necessary static fields. Otherwise, the test may collide with other
// Local*StringConverter tests and cause unpredictable results.
private enum LocalDateStringConverterVariant {
NO_PARAM,
WITH_FORMATTER_PARSER,
WITH_FORMAT_STYLES,
};

@Parameterized.Parameters public static Collection implementations() {
return Arrays.asList(new Object[][] {
{ new LocalDateStringConverter(),
Locale.getDefault(Locale.Category.FORMAT), FormatStyle.SHORT,
VALID_DATE, null, null },
{ LocalDateStringConverterVariant.NO_PARAM,
FormatStyle.SHORT, VALID_DATE },

{ new LocalDateStringConverter(aFormatter, aParser),
Locale.getDefault(Locale.Category.FORMAT), null,
VALID_DATE, aFormatter, aParser },
{ LocalDateStringConverterVariant.WITH_FORMATTER_PARSER,
null, VALID_DATE },

{ new LocalDateStringConverter(FormatStyle.SHORT, Locale.UK, IsoChronology.INSTANCE),
Locale.UK, FormatStyle.SHORT,
VALID_DATE, null, null },
{ LocalDateStringConverterVariant.WITH_FORMAT_STYLES,
FormatStyle.SHORT, VALID_DATE },
});
}

private LocalDateStringConverterVariant converterVariant;
private FormatStyle dateStyle;
private LocalDate validDate;

private LocalDateStringConverter converter;
private Locale locale;
private FormatStyle dateStyle;
private DateTimeFormatter formatter, parser;
private LocalDate validDate;

public LocalDateStringConverterTest(LocalDateStringConverter converter, Locale locale, FormatStyle dateStyle, LocalDate validDate, DateTimeFormatter formatter, DateTimeFormatter parser) {
this.converter = converter;
this.locale = locale;
public LocalDateStringConverterTest(LocalDateStringConverterVariant converterVariant, FormatStyle dateStyle, LocalDate validDate) {
this.converterVariant = converterVariant;
this.dateStyle = dateStyle;
this.validDate = validDate;
this.formatter = formatter;
this.parser = parser;

// initialized after Locale is established
this.converter = null;
this.locale = null;
this.formatter = null;
this.parser = null;
}

@BeforeClass
public static void setupBeforeAll() {
// Tests require that default locale is en_US
oldLocale = Locale.getDefault();
Locale.setDefault(Locale.US);

// DateTimeFormatter uses default locale, so we can init this after updating locale
aFormatter = DateTimeFormatter.ofPattern("dd MM yyyy");
aParser = DateTimeFormatter.ofPattern("yyyy MM dd");
}

@Before public void setup() {
@AfterClass
public static void teardownAfterAll() {
// Restore VM's old locale
Locale.setDefault(oldLocale);
}

@Before
public void setup() {
// Locale is established now, so we can allocate objects depending on it
switch (this.converterVariant) {
case NO_PARAM:
this.converter = new LocalDateStringConverter();
this.locale = Locale.getDefault(Locale.Category.FORMAT);
this.formatter = null;
this.parser = null;
break;
case WITH_FORMATTER_PARSER:
this.converter = new LocalDateStringConverter(aFormatter, aParser);
this.locale = Locale.getDefault(Locale.Category.FORMAT);
this.formatter = aFormatter;
this.parser = aParser;
break;
case WITH_FORMAT_STYLES:
this.converter = new LocalDateStringConverter(FormatStyle.SHORT, Locale.UK, IsoChronology.INSTANCE);
this.locale = Locale.UK;
this.formatter = null;
this.parser = null;
break;
default:
fail("Invalid converter variant: " + this.converterVariant.toString());
}
}

/*********************************************************************
Expand Down
101 changes: 79 additions & 22 deletions ...avafx.base/src/test/java/test/javafx/util/converter/LocalDateTimeStringConverterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@
import javafx.util.converter.LocalDateTimeStringConverter;
import javafx.util.converter.LocalDateTimeStringConverterShim;

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

private static final DateTimeFormatter aFormatter = DateTimeFormatter.ofPattern("dd MM yyyy HH mm ss");
private static final DateTimeFormatter aParser = DateTimeFormatter.ofPattern("yyyy MM dd hh mm ss a");
private static DateTimeFormatter aFormatter;
private static DateTimeFormatter aParser;
private static Locale oldLocale;

// We can only create LocalDateTimeStringConverter object after Locale is set.
// Unfortunately, due to unpredictability of @Parameterized.Parameters methods
// in JUnit, we have to allocate it after @BeforeClass sets up Locale and
// necessary static fields. Otherwise, the test may collide with other
// Local*StringConverter tests and cause unpredictable results.
private enum LocalDateTimeStringConverterVariant {
NO_PARAM,
WITH_FORMATTER_PARSER,
WITH_FORMAT_STYLES,
};

@Parameterized.Parameters public static Collection implementations() {
// Tests require that default locale is en_US
Locale.setDefault(Locale.US);

return Arrays.asList(new Object[][] {
{ new LocalDateTimeStringConverter(),
Locale.getDefault(Locale.Category.FORMAT), FormatStyle.SHORT, FormatStyle.SHORT,
VALID_LDT_WITHOUT_SECONDS, null, null },
{ LocalDateTimeStringConverterVariant.NO_PARAM,
FormatStyle.SHORT, FormatStyle.SHORT, VALID_LDT_WITHOUT_SECONDS},

{ new LocalDateTimeStringConverter(aFormatter, aParser),
Locale.getDefault(Locale.Category.FORMAT), null, null,
VALID_LDT_WITH_SECONDS, aFormatter, aParser },
{ LocalDateTimeStringConverterVariant.WITH_FORMATTER_PARSER,
null, null, VALID_LDT_WITH_SECONDS},

{ new LocalDateTimeStringConverter(FormatStyle.SHORT, FormatStyle.SHORT, Locale.UK, IsoChronology.INSTANCE),
Locale.UK, FormatStyle.SHORT, FormatStyle.SHORT,
VALID_LDT_WITHOUT_SECONDS, null, null },
{ LocalDateTimeStringConverterVariant.WITH_FORMAT_STYLES,
FormatStyle.SHORT, FormatStyle.SHORT, VALID_LDT_WITHOUT_SECONDS},
});
}

private LocalDateTimeStringConverter converter;
private Locale locale;
private LocalDateTimeStringConverterVariant converterVariant;
private FormatStyle dateStyle;
private FormatStyle timeStyle;
private LocalDateTime validDateTime;

private LocalDateTimeStringConverter converter;
private Locale locale;
private DateTimeFormatter formatter, parser;

private LocalDateTime validDateTime;

public LocalDateTimeStringConverterTest(LocalDateTimeStringConverter converter, Locale locale, FormatStyle dateStyle, FormatStyle timeStyle, LocalDateTime validDateTime, DateTimeFormatter formatter, DateTimeFormatter parser) {
this.converter = converter;
this.locale = locale;
public LocalDateTimeStringConverterTest(LocalDateTimeStringConverterVariant converterVariant, FormatStyle dateStyle, FormatStyle timeStyle, LocalDateTime validDateTime) {
this.converterVariant = converterVariant;
this.dateStyle = dateStyle;
this.timeStyle = timeStyle;
this.validDateTime = validDateTime;
this.formatter = formatter;
this.parser = parser;

this.converter = null;
this.locale = null;
this.formatter = null;
this.parser = null;
}

@BeforeClass
public static void setupBeforeAll() {
// Tests require that default locale is en_US
oldLocale = Locale.getDefault();
Locale.setDefault(Locale.US);

// DateTimeFormatter uses default locale, so we can init this after updating locale
aFormatter = DateTimeFormatter.ofPattern("dd MM yyyy HH mm ss");
aParser = DateTimeFormatter.ofPattern("yyyy MM dd hh mm ss a");
}

@AfterClass
public static void teardownAfterAll() {
// Restore VM's old locale
Locale.setDefault(oldLocale);
}

@Before
public void setup() {
// Locale is established now, so we can allocate objects depending on it
switch (this.converterVariant) {
case NO_PARAM:
this.converter = new LocalDateTimeStringConverter();
this.locale = Locale.getDefault(Locale.Category.FORMAT);
this.formatter = null;
this.parser = null;
break;
case WITH_FORMATTER_PARSER:
this.converter = new LocalDateTimeStringConverter(aFormatter, aParser);
this.locale = Locale.getDefault(Locale.Category.FORMAT);
this.formatter = aFormatter;
this.parser = aParser;
break;
case WITH_FORMAT_STYLES:
this.converter = new LocalDateTimeStringConverter(FormatStyle.SHORT, FormatStyle.SHORT, Locale.UK, IsoChronology.INSTANCE);
this.locale = Locale.UK;
this.formatter = null;
this.parser = null;
break;
default:
fail("Invalid converter variant: " + this.converterVariant.toString());
}
}

/*********************************************************************
Expand Down
94 changes: 74 additions & 20 deletions ...es/javafx.base/src/test/java/test/javafx/util/converter/LocalTimeStringConverterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@
import javafx.util.converter.LocalTimeStringConverter;

import static org.junit.Assert.*;
import org.junit.AfterClass;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
Expand All @@ -53,42 +55,94 @@ public class LocalTimeStringConverterTest {
VALID_TIME_WITHOUT_SECONDS = LocalTime.of(12, 34, 0);
}

private static final DateTimeFormatter aFormatter = DateTimeFormatter.ofPattern("HH mm ss");
private static final DateTimeFormatter aParser = DateTimeFormatter.ofPattern("hh mm ss a");

private static Locale oldLocale = null;
private static DateTimeFormatter aFormatter = null;
private static DateTimeFormatter aParser = null;

// We can only create LocalTimeStringConverter object after Locale is set.
// Unfortunately, due to unpredictability of @Parameterized.Parameters methods
// in JUnit, we have to allocate it after @BeforeClass sets up Locale and
// necessary static fields. Otherwise, the test may collide with other
// Local*StringConverter tests and cause unpredictable results.
private enum LocalTimeStringConverterVariant {
NO_PARAM,
WITH_FORMATTER_PARSER,
WITH_FORMAT_STYLES,
};

@Parameterized.Parameters public static Collection implementations() {
return Arrays.asList(new Object[][] {
{ new LocalTimeStringConverter(),
Locale.getDefault(Locale.Category.FORMAT), FormatStyle.SHORT,
VALID_TIME_WITHOUT_SECONDS, null, null },
{ LocalTimeStringConverterVariant.NO_PARAM,
FormatStyle.SHORT, VALID_TIME_WITHOUT_SECONDS },

{ new LocalTimeStringConverter(aFormatter, aParser),
Locale.getDefault(Locale.Category.FORMAT), null,
VALID_TIME_WITH_SECONDS, aFormatter, aParser },
{ LocalTimeStringConverterVariant.WITH_FORMATTER_PARSER,
null, VALID_TIME_WITH_SECONDS },

{ new LocalTimeStringConverter(FormatStyle.SHORT, Locale.UK),
Locale.UK, FormatStyle.SHORT,
VALID_TIME_WITHOUT_SECONDS, null, null },
{ LocalTimeStringConverterVariant.WITH_FORMAT_STYLES,
FormatStyle.SHORT, VALID_TIME_WITHOUT_SECONDS },
});
}

private LocalTimeStringConverterVariant converterVariant;
private FormatStyle timeStyle;
private LocalTime validTime;

private LocalTimeStringConverter converter;
private Locale locale;
private FormatStyle timeStyle;
private DateTimeFormatter formatter, parser;
private LocalTime validTime;

public LocalTimeStringConverterTest(LocalTimeStringConverter converter, Locale locale, FormatStyle timeStyle, LocalTime validTime, DateTimeFormatter formatter, DateTimeFormatter parser) {
this.converter = converter;
this.locale = locale;
public LocalTimeStringConverterTest(LocalTimeStringConverterVariant converterVariant, FormatStyle timeStyle, LocalTime validTime) {
this.converterVariant = converterVariant;
this.timeStyle = timeStyle;
this.validTime = validTime;
this.formatter = formatter;
this.parser = parser;

this.locale = null;
this.formatter = null;
this.parser = null;
}

@Before public void setup() {
@BeforeClass
public static void setupBeforeAll() {
// Tests require that default locale is en_US
oldLocale = Locale.getDefault();
Locale.setDefault(Locale.US);

// DateTimeFormatter uses default locale, so we can init this after updating locale
aFormatter = DateTimeFormatter.ofPattern("HH mm ss");
aParser = DateTimeFormatter.ofPattern("hh mm ss a");
}

@AfterClass
public static void teardownAfterAll() {
// Restore VM's old locale
Locale.setDefault(oldLocale);
}

@Before
public void setup() {
// Locale is established now, so we can allocate objects depending on it
switch (this.converterVariant) {
case NO_PARAM:
this.converter = new LocalTimeStringConverter();
this.locale = Locale.getDefault(Locale.Category.FORMAT);
this.formatter = null;
this.parser = null;
break;
case WITH_FORMATTER_PARSER:
this.converter = new LocalTimeStringConverter(aFormatter, aParser);
this.locale = Locale.getDefault(Locale.Category.FORMAT);
this.formatter = aFormatter;
this.parser = aParser;
break;
case WITH_FORMAT_STYLES:
this.converter = new LocalTimeStringConverter(FormatStyle.SHORT, Locale.UK);
this.locale = Locale.UK;
this.formatter = null;
this.parser = null;
break;
default:
fail("Invalid converter variant: " + this.converterVariant.toString());
}
}

/*********************************************************************
Expand Down

1 comment on commit 4ad8582

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Issues

Please sign in to comment.