[#8557] Standardize locale
Fixes #8557
OS: macOS Sonoma 14.2.1 arm64
Host: MacBook Pro (14-inch, 2021)
Locale: en_GB.UTF-8, de_DE.UTF-8
When running tests locally, some tests fail because of differences in locale, like teammates.common.util.TimeHelperTest.testGetMidnightAdjustedInstantBasedOnZone. Even though this might not be a problem when running tests as part of a workflow, it might cause confusion to some users who are running tests locally and encountering errors.
Testing on en_GB.UTF-8:
org.gradle.internal.serialize.PlaceholderAssertionError: expected: <Sun, 29 Nov 2015, 11:59 PM UTC> but was: <Sun, 29 Nov 2015, 11:59 pm UTC>
Testing on de_DE.UTF-8:
org.gradle.internal.serialize.PlaceholderAssertionError: expected: <Sun, 29 Nov 2015, 11:59 PM UTC> but was: <So., 29 Nov. 2015, 11:59 PM UTC>
For me, 5 tests were failing locally, so I ran this command to run the 5 tests which were failing.
./gradlew unitTests \
--tests "teammates.common.util.TimeHelperTest.testFormatDateTimeForDisplay" \
--tests "teammates.common.util.TimeHelperTest.testGetMidnightAdjustedInstantBasedOnZone" \
--tests "teammates.logic.api.EmailGeneratorTest.testGenerateFeedbackSessionEmails" \
--tests "teammates.logic.api.EmailGeneratorTest.testGenerateFeedbackSessionEmails_testSanitization" \
--tests "teammates.logic.api.EmailGeneratorTest.testGenerateFeedbackSessionEmails_testUsersWithDeadlineExtensions"
Outline of Solution
I updated teammates/common/util/TimeHelper::formatInstant to use the US locale when generating dates, as seen below:
DateTimeFormatter formatter = DateTimeFormatter
.ofPattern(processedPattern)
.withLocale(Locale.US);
This standardises the locale used in the app to use a US locale, so that the tests which are related to datetime formatting can pass locally.
After making this change, local tests pass on both de_DE.UTF-8 and en_GB.UTF-8.
I have yet to test this on Linux, which was what was reported in the referenced issue, but I believe that the issues that we are facing are somewhat related.
@itstrueitstrueitsrealitsreal Good solution, is there anyway you could test it on an linux enviroment before we merge?
@mingyuanc I'll get it tested by EOD.
OS: Arch Linux x86_64
Host: 20L8SA2N16 (ThinkPad T480s)
Locale: de_DE.UTF-8
Tests failed before changes:
component-tests > component-tests > teammates.common.util.TimeHelperTest > testGetMidnightAdjustedInstantBasedOnZone FAILED
component-tests > component-tests > teammates.common.util.TimeHelperTest > testFormatDateTimeForDisplay FAILED
component-tests > component-tests > teammates.common.util.TimeHelperTest > testEndOfYearDates FAILED
component-tests > component-tests > teammates.common.datatransfer.questions.FeedbackNumericalScaleQuestionDetailsTest > tesValidateResponseDetails FAILED
component-tests > component-tests > teammates.logic.api.EmailGeneratorTest > testGenerateFeedbackSessionEmails_testUsersWithDeadlineExtensions FAILED
component-tests > component-tests > teammates.logic.api.EmailGeneratorTest > testGenerateFeedbackSessionEmails_testUsersWithDeadlineExtensions FAILED
component-tests > component-tests > teammates.logic.api.EmailGeneratorTest > testGenerateFeedbackSessionEmails_testSanitization FAILED
Test failed after changes:
org.gradle.internal.serialize.PlaceholderAssertionError: expected: <5.1 is out of the range for Numerical-scale question.(min=3, max=5)> but was: <5,1 is out of the range for Numerical-scale question.(min=3, max=5)>
at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:177)
at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1145)
at teammates.test.BaseTestCase.assertEquals(BaseTestCase.java:321)
at teammates.common.datatransfer.questions.FeedbackNumericalScaleQuestionDetailsTest.tesValidateResponseDetails(FeedbackNumericalScaleQuestionDetailsTest.java:77)
...
component-tests > component-tests > teammates.common.datatransfer.questions.FeedbackNumericalScaleQuestionDetailsTest > tesValidateResponseDetails FAILED
org.opentest4j.AssertionFailedError at FeedbackNumericalScaleQuestionDetailsTest.java:77
This seems to be a result of the DecimalFormat class using , as a delimiter in the locale I set my machine to, so I added a workaround where I used java.text.DecimalFormatSymbols::setDecimalSeparator to set the delimiter to . in StringHelper::toDecimalFormatString, as follows:
/**
* Converts a double value between 0 and 1 to 3dp-string.
*/
public static String toDecimalFormatString(double doubleVal) {
DecimalFormatSymbols syms = new DecimalFormatSymbols();
syms.setDecimalSeparator('.');
DecimalFormat df = new DecimalFormat("0.###", syms);
return df.format(doubleVal);
}
After making these changes, I changed my locale to:
en_US.UTF-8
en_SG.UTF-8
and ran the tests again, and they passed on all 3 locales.
For convenience, here's the command to run the 7 tests:
./gradlew unitTests --tests "teammates.common.util.TimeHelperTest.testGetMidnightAdjustedInstantBasedOnZone" \
--tests "teammates.common.util.TimeHelperTest.testFormatDateTimeForDisplay" \
--tests "teammates.common.util.TimeHelperTest.testEndOfYearDates" \
--tests "teammates.common.datatransfer.questions.FeedbackNumericalScaleQuestionDetailsTest.tesValidateResponseDetails" \
--tests "teammates.logic.api.EmailGeneratorTest.testGenerateFeedbackSessionEmails_testUsersWithDeadlineExtensions" \
--tests "teammates.logic.api.EmailGeneratorTest.testGenerateFeedbackSessionEmails_testUsersWithDeadlineExtensions" \
--tests "teammates.logic.api.EmailGeneratorTest.testGenerateFeedbackSessionEmails_testSanitization" \
Folks, This PR seems to be stalling (no activities for the past 7 days). :snail: :cry: Hope someone can get it to move forward again soon...
Folks, This PR seems to be stalling (no activities for the past 7 days). :snail: :cry: Hope someone can get it to move forward again soon...
Folks, This PR seems to be stalling (no activities for the past 9 days). :snail: :cry: Hope someone can get it to move forward again soon...
Hey, wanted to ask about the 'Merging is blocked' warning I've been getting on my PRs (this one, #13165 and #13166.) I manually merged master into these branches, and saw this error, and I tried reverting the merge on #13165, but the error did not go away. I'm aware that test migration is currently in progress. How should I proceed?
Hi @itstrueitstrueitsrealitsreal, #13165 is merged! #13166 is still pending approval, we will get it approved soon!
The "Merging is blocked" warning is usually from either failing tests or the PR not being approved.
I see, was concerned because I saw this on my UI.
But it seems like you have the permissions to resolve that, so thanks for your help!