Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8282023: PropertiesStoreTest and StoreReproducibilityTest jtreg failures due to en_CA locale #7558

Closed
wants to merge 10 commits into from
+72 −29
Closed

8282023: PropertiesStoreTest and StoreReproducibilityTest jtreg failures due to en_CA locale #7558

Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ce8b237
fix PropertiesStoreTest
jaikiran Feb 21, 2022
da0e4fb
fix StoreReproducibilityTest
jaikiran Feb 21, 2022
9ef2341
copyright years
jaikiran Feb 23, 2022
c5dd7f7
implement review comments
jaikiran Feb 23, 2022
11f0800
remove @modules declaration on the jtreg test
jaikiran Feb 24, 2022
39ff7cf
review comments:
jaikiran Feb 24, 2022
9026961
no need for system.out.println since testng add the chosen params to …
jaikiran Feb 24, 2022
97c9afd
use Roger's suggestion of using Stream and Collection based APIs to a…
jaikiran Feb 24, 2022
1d14f1c
use Collections.toCollection() instead of Collectors.toSet() to allow…
jaikiran Feb 25, 2022
915f12e
HashSet::new instead of new HashSet() in test
jaikiran Feb 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 18 additions & 29 deletions test/jdk/java/util/Properties/PropertiesStoreTest.java
View file
Original file line number Diff line number Diff line change
@@ -43,22 +43,21 @@
import java.util.Properties;
import java.util.Set;
import java.util.TreeSet;
import java.util.stream.Collectors;

/*
* @test
* @summary tests the order in which the Properties.store() method writes out the properties
* @bug 8231640 8282023
* @modules jdk.localedata
* @run testng/othervm PropertiesStoreTest
*/
public class PropertiesStoreTest {

private static final String DATE_FORMAT_PATTERN = "EEE MMM dd HH:mm:ss zzz uuuu";
// use a neutral locale, since when the date comment was written by Properties.store(...),
// it internally calls the Date.toString() which always writes in a locale insensitive manner
private static final DateTimeFormatter formatter = DateTimeFormatter.ofPattern(DATE_FORMAT_PATTERN)
.withLocale(Locale.ROOT);
private static final Locale prevLocale = Locale.getDefault();
private static final DateTimeFormatter FORMATTER = DateTimeFormatter.ofPattern(DATE_FORMAT_PATTERN, Locale.ROOT);
private static final Locale PREV_LOCALE = Locale.getDefault();

@DataProvider(name = "propsProvider")
private Object[][] createProps() {
@@ -104,28 +103,18 @@ private Object[][] createProps() {
@DataProvider(name = "localeProvider")
private Object[][] provideLocales() {
// pick a non-english locale for testing
Locale nonEnglishLocale = null;
for (Locale locale : Locale.getAvailableLocales()) {
// skip ROOT locale and ENGLISH language ones
if (!locale.getLanguage().isEmpty() && !locale.getLanguage().equals("en")) {
nonEnglishLocale = locale;
System.out.println("Selected non-english locale: " + nonEnglishLocale + " for tests");
break;
}
}
if (nonEnglishLocale == null) {
return new Object[][] {
{Locale.getDefault()},
{Locale.US}, // guaranteed to be present
{Locale.ROOT}, // guaranteed to be present
};
}
return new Object[][]{
{Locale.getDefault()},
{Locale.US}, // guaranteed to be present
{Locale.ROOT}, // guaranteed to be present
{nonEnglishLocale}
};
Set<Locale> locales = Arrays.stream(Locale.getAvailableLocales())
.filter(l -> !l.getLanguage().isEmpty() && !l.getLanguage().equals("en"))
.limit(1)
.collect(Collectors.toSet());
locales.add(Locale.getDefault()); // always test the default locale
locales.add(Locale.US); // guaranteed to be present
locales.add(Locale.ROOT); // guaranteed to be present
Comment on lines +111 to +113
Copy link
Member

Choose a reason for hiding this comment

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

Can we assume the returned Set<Locale> is mutable? Collectors.toSet() javadoc reads no guarantee for mutability.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a very good point. I've updated the PR to now explicitly use a mutable Set instead of using Collectors.toSet()

Copy link
Member

Choose a reason for hiding this comment

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

This is ok, although Collectors.toCollection(HashSet::new) is a bit concise.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hello Naoto, I've updated the PR to use HashSet::new.


// return the chosen locales
return locales.stream()
.map(m -> new Locale[] {m})
.toArray(n -> new Object[n][0]);
}

/**
@@ -205,7 +194,7 @@ public void testStoreWriterDateComment(final Locale testLocale) throws Exception
testDateComment(tmpFile);
} finally {
// reset to the previous one
Locale.setDefault(prevLocale);
Locale.setDefault(PREV_LOCALE);
}
}

@@ -227,7 +216,7 @@ public void testStoreOutputStreamDateComment(final Locale testLocale) throws Exc
testDateComment(tmpFile);
} finally {
// reset to the previous one
Locale.setDefault(prevLocale);
Locale.setDefault(PREV_LOCALE);
}
}

@@ -252,7 +241,7 @@ private void testDateComment(Path file) throws Exception {
Assert.fail("No comment line found in the stored properties file " + file);
}
try {
formatter.parse(comment);
FORMATTER.parse(comment);
} catch (DateTimeParseException pe) {
Assert.fail("Unexpected date comment: " + comment, pe);
}
12 changes: 6 additions & 6 deletions test/jdk/java/util/Properties/StoreReproducibilityTest.java
View file
Original file line number Diff line number Diff line change
@@ -52,8 +52,8 @@ public class StoreReproducibilityTest {

private static final String DATE_FORMAT_PATTERN = "EEE MMM dd HH:mm:ss zzz uuuu";
private static final String SYS_PROP_JAVA_PROPERTIES_DATE = "java.properties.date";
private static final DateTimeFormatter reproducibleDateTimeFormatter = DateTimeFormatter.ofPattern(DATE_FORMAT_PATTERN)
.withLocale(Locale.ROOT).withZone(ZoneOffset.UTC);
private static final DateTimeFormatter FORMATTER = DateTimeFormatter.ofPattern(DATE_FORMAT_PATTERN, Locale.ROOT)
.withZone(ZoneOffset.UTC);

public static void main(final String[] args) throws Exception {
// no security manager enabled
@@ -88,7 +88,7 @@ public static void main(final String[] args) throws Exception {
*/
private static void testWithoutSecurityManager() throws Exception {
final List<Path> storedFiles = new ArrayList<>();
final String sysPropVal = reproducibleDateTimeFormatter.format(Instant.ofEpochSecond(243535322));
final String sysPropVal = FORMATTER.format(Instant.ofEpochSecond(243535322));
for (int i = 0; i < 5; i++) {
final Path tmpFile = Files.createTempFile("8231640", ".props");
storedFiles.add(tmpFile);
@@ -130,7 +130,7 @@ private static void testWithSecMgrExplicitPermission() throws Exception {
};
"""));
final List<Path> storedFiles = new ArrayList<>();
final String sysPropVal = reproducibleDateTimeFormatter.format(Instant.ofEpochSecond(1234342423));
final String sysPropVal = FORMATTER.format(Instant.ofEpochSecond(1234342423));
for (int i = 0; i < 5; i++) {
final Path tmpFile = Files.createTempFile("8231640", ".props");
storedFiles.add(tmpFile);
@@ -174,7 +174,7 @@ private static void testWithSecMgrNoSpecificPermission() throws Exception {
};
"""));
final List<Path> storedFiles = new ArrayList<>();
final String sysPropVal = reproducibleDateTimeFormatter.format(Instant.ofEpochSecond(1234342423));
final String sysPropVal = FORMATTER.format(Instant.ofEpochSecond(1234342423));
for (int i = 0; i < 5; i++) {
final Path tmpFile = Files.createTempFile("8231640", ".props");
storedFiles.add(tmpFile);
@@ -425,7 +425,7 @@ private static void assertCurrentDate(final Path destFile, final Date date) thro
System.out.println("Found date comment " + dateComment + " in file " + destFile);
final Date parsedDate;
try {
Instant instant = Instant.from(reproducibleDateTimeFormatter.parse(dateComment));
Instant instant = Instant.from(FORMATTER.parse(dateComment));
parsedDate = new Date(instant.toEpochMilli());
} catch (DateTimeParseException pe) {
throw new RuntimeException("Unexpected date " + dateComment + " in stored properties " + destFile, pe);