From 2f8f8ccf447db771807365ebdf41fdaf24dc3986 Mon Sep 17 00:00:00 2001 From: Ben Hamilton Date: Wed, 9 Dec 2015 09:13:55 -0800 Subject: [PATCH] Fix TimeFormatTest failing in non-US English locale Summary: This is the last test which fails in non-US English locales. The only places it's used are from `TestResultSummary` and `TestCaseSummary`. Refactoring `TestResultSummary` to support multiple locales is pretty painful, so I hard-coded that class to US English. Refactoring `TestCaseSummary` isn't as bad, so I'll separately send out a diff to fix that. Temporarily, this diff hard-codes it to US English as well. Partial fix for https://github.com/facebook/buck/issues/557 Test Plan: `ant java-test -Dtest.class=TimeFormatTest` Reviewed By: k21 fb-gh-sync-id: 78752f4 --- src/com/facebook/buck/test/TestCaseSummary.java | 4 +++- .../facebook/buck/test/TestResultSummary.java | 5 ++++- src/com/facebook/buck/util/TimeFormat.java | 8 +++++--- test/com/facebook/buck/util/TimeFormatTest.java | 16 ++++++++++------ 4 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/com/facebook/buck/test/TestCaseSummary.java b/src/com/facebook/buck/test/TestCaseSummary.java index 30786f00396..860a0bd3feb 100644 --- a/src/com/facebook/buck/test/TestCaseSummary.java +++ b/src/com/facebook/buck/test/TestCaseSummary.java @@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableList; import java.util.List; +import java.util.Locale; import javax.annotation.concurrent.Immutable; @@ -165,7 +166,8 @@ public String getOneLineSummary(boolean hasPassingDependencies, Ansi ansi) { return String.format("%s%s %s %2d Passed %2d Skipped %2d Failed %s", status, padding, - !isCached ? TimeFormat.formatForConsole(totalTime, ansi) + // TODO(bhamiltoncx): Refactor this class to pass in the Locale. + !isCached ? TimeFormat.formatForConsole(Locale.US, totalTime, ansi) : ansi.asHighlightedStatusText(severityLevel, "CACHED"), getPassedCount(), skippedCount, diff --git a/src/com/facebook/buck/test/TestResultSummary.java b/src/com/facebook/buck/test/TestResultSummary.java index 993b93375a2..81f2ae2a39e 100644 --- a/src/com/facebook/buck/test/TestResultSummary.java +++ b/src/com/facebook/buck/test/TestResultSummary.java @@ -23,6 +23,7 @@ import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; +import java.util.Locale; import java.util.Objects; import javax.annotation.Nullable; @@ -139,7 +140,9 @@ public long getTime() { public String toString() { return String.format("%s %s %s#%s()", isSuccess() ? "PASS" : "FAIL", - TimeFormat.formatForConsole(getTime(), Ansi.withoutTty()), + // Hard-coding US English is not great, but refactoring this class to take in a Locale + // is a ton of work (we can't change this API, since toString() is called everywhere). + TimeFormat.formatForConsole(Locale.US, getTime(), Ansi.withoutTty()), testCaseName, getTestName()); } diff --git a/src/com/facebook/buck/util/TimeFormat.java b/src/com/facebook/buck/util/TimeFormat.java index b1ffb8ea349..207142da4f2 100644 --- a/src/com/facebook/buck/util/TimeFormat.java +++ b/src/com/facebook/buck/util/TimeFormat.java @@ -16,6 +16,8 @@ package com.facebook.buck.util; +import java.util.Locale; + public class TimeFormat { private TimeFormat() {} @@ -23,14 +25,14 @@ private TimeFormat() {} /** * @return a six-character string, so it is a fixed width */ - public static String formatForConsole(long durationInMillis, Ansi ansi) { + public static String formatForConsole(Locale locale, long durationInMillis, Ansi ansi) { if (durationInMillis < 100) { return ansi.asSuccessText("<100ms"); } else if (durationInMillis < 1000) { - return ansi.asWarningText(String.format(" %dms", durationInMillis)); + return ansi.asWarningText(String.format(locale, " %dms", durationInMillis)); } else { double seconds = durationInMillis / 1000.0; - return ansi.asErrorText(String.format("%5.1fs", seconds)); + return ansi.asErrorText(String.format(locale, "%5.1fs", seconds)); } } } diff --git a/test/com/facebook/buck/util/TimeFormatTest.java b/test/com/facebook/buck/util/TimeFormatTest.java index c627cf80f21..45f9bbad4a3 100644 --- a/test/com/facebook/buck/util/TimeFormatTest.java +++ b/test/com/facebook/buck/util/TimeFormatTest.java @@ -18,6 +18,8 @@ import static org.junit.Assert.assertEquals; +import java.util.Locale; + import org.junit.Test; public class TimeFormatTest { @@ -26,13 +28,15 @@ public class TimeFormatTest { public void testFormatForConsole() { Ansi ansi = Ansi.withoutTty(); - assertEquals("<100ms", TimeFormat.formatForConsole(0, ansi)); - assertEquals("<100ms", TimeFormat.formatForConsole(99, ansi)); + assertEquals("<100ms", TimeFormat.formatForConsole(Locale.US, 0, ansi)); + assertEquals("<100ms", TimeFormat.formatForConsole(Locale.US, 99, ansi)); + + assertEquals(" 100ms", TimeFormat.formatForConsole(Locale.US, 100, ansi)); + assertEquals(" 999ms", TimeFormat.formatForConsole(Locale.US, 999, ansi)); - assertEquals(" 100ms", TimeFormat.formatForConsole(100, ansi)); - assertEquals(" 999ms", TimeFormat.formatForConsole(999, ansi)); + assertEquals(" 1.0s", TimeFormat.formatForConsole(Locale.US, 1000, ansi)); + assertEquals(" 1.2s", TimeFormat.formatForConsole(Locale.US, 1200, ansi)); - assertEquals(" 1.0s", TimeFormat.formatForConsole(1000, ansi)); - assertEquals(" 1.2s", TimeFormat.formatForConsole(1200, ansi)); + assertEquals(" 3,4s", TimeFormat.formatForConsole(Locale.GERMAN, 3400, ansi)); } }