Skip to content

Commit

Permalink
Fix TimeFormatTest failing in non-US English locale
Browse files Browse the repository at this point in the history
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 facebook#557

Test Plan: `ant java-test -Dtest.class=TimeFormatTest`

Reviewed By: k21

fb-gh-sync-id: 78752f4
  • Loading branch information
bhamiltoncx authored and shs96c committed Dec 25, 2015
1 parent dc2fa02 commit 2f8f8cc
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 11 deletions.
4 changes: 3 additions & 1 deletion src/com/facebook/buck/test/TestCaseSummary.java
Expand Up @@ -23,6 +23,7 @@
import com.google.common.collect.ImmutableList;

import java.util.List;
import java.util.Locale;

import javax.annotation.concurrent.Immutable;

Expand Down Expand Up @@ -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,
Expand Down
5 changes: 4 additions & 1 deletion src/com/facebook/buck/test/TestResultSummary.java
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
Expand Down
8 changes: 5 additions & 3 deletions src/com/facebook/buck/util/TimeFormat.java
Expand Up @@ -16,21 +16,23 @@

package com.facebook.buck.util;

import java.util.Locale;

public class TimeFormat {

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));
}
}
}
16 changes: 10 additions & 6 deletions test/com/facebook/buck/util/TimeFormatTest.java
Expand Up @@ -18,6 +18,8 @@

import static org.junit.Assert.assertEquals;

import java.util.Locale;

import org.junit.Test;

public class TimeFormatTest {
Expand All @@ -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));
}
}

0 comments on commit 2f8f8cc

Please sign in to comment.