-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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.
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
MAPREDUCE-7270. TestHistoryViewerPrinter could be failed when the locale isn't English. #1942
Conversation
sungpeo
commented
Apr 7, 2020
- set locale for the test
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
...ient-core/src/test/java/org/apache/hadoop/mapreduce/jobhistory/TestHistoryViewerPrinter.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 with a good QA. Thanks!
🎊 +1 overall
This message was automatically generated. |
@@ -43,6 +46,19 @@ | |||
|
|||
private final String LINE_SEPARATOR = System.lineSeparator(); | |||
|
|||
private static Locale DEFAULT_LOCALE; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I missed in previous review, but do you mind making this a static final variable e.g.
private static final Locale DEFAULT_LOCALE = Locale.getDefault();
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@liuml07
I think the static DEFAULT_LOCALE which could be stale during other test suites or settings,
because, the DEFAULT_LOCALE will be set before all of the tests.
What do you think about?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is only initialized during the static @BeforeClass setUp()
method, and it will not change during every test cases in this class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry to confuse you.
I worried that the result of Locale.getDefault()
might be changed.
But I get it I don't have to.
I'll push the change.
I think the Jenkins failed for unrelated issues. @aajisaka could you tell me how I can trigger a Jenkins run for this again manually? |
Have triggered another run manually: https://ci-hadoop.apache.org/job/hadoop-multibranch/view/change-requests/job/PR-1942/8/ FWIW, the way I trigger it is by logging into the previous build URL (may need committer access) and click the "Replay" button on the left panel. |
@liuml07 Actually the findbugs support was replaced with spotbugs in HADOOP-16870, I guess that is the reason for the issue. https://ci-hadoop.apache.org/job/hadoop-multibranch/view/change-requests/job/PR-1942/10/console If this too causes any issue, may be we might need to rebase this PR. |
Thank you very much @ayushtkn The report was not posted here, but the new run was fine. Since there is a checkstyle warning, could you rebase this PR from
|
…ale isn't English. - set Locale.ENGLISH for the test
@liuml07 Okay, I rebased trunk and fixed the warning. |
💔 -1 overall
This message was automatically generated. |
…ale isn't English. (apache#1942) Contributed by Sungpeo Kook. Signed-off-by: Mingliang Liu <liuml07@apache.org>