From 07318c23e3e90e5546f116bba14255ff3a634bbc Mon Sep 17 00:00:00 2001 From: Kurt Alfred Kluever Date: Thu, 30 Jan 2025 17:44:09 -0800 Subject: [PATCH] Pretty-print integral numbers in failure messages for `isWithin` assertions. RELNOTES=Changed failure messages for `isWithin` assertions to pretty-print integral numbers. PiperOrigin-RevId: 721576493 --- .../java/com/google/common/truth/Fact.java | 62 +++++++++++++++-- .../google/common/truth/IntegerSubject.java | 14 ++-- .../com/google/common/truth/LongSubject.java | 14 ++-- .../com/google/common/truth/FactTest.java | 69 +++++++++++++++++-- .../common/truth/IntegerSubjectTest.java | 11 +-- .../google/common/truth/LongSubjectTest.java | 11 +-- 6 files changed, 148 insertions(+), 33 deletions(-) diff --git a/core/src/main/java/com/google/common/truth/Fact.java b/core/src/main/java/com/google/common/truth/Fact.java index 289177901..3fc5a311e 100644 --- a/core/src/main/java/com/google/common/truth/Fact.java +++ b/core/src/main/java/com/google/common/truth/Fact.java @@ -16,8 +16,10 @@ package com.google.common.truth; +import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Strings.padEnd; +import static com.google.common.base.Strings.padStart; import static java.lang.Math.max; import com.google.common.collect.ImmutableList; @@ -40,7 +42,7 @@ public final class Fact implements Serializable { * value." The value is converted to a string by calling {@code String.valueOf} on it. */ public static Fact fact(String key, @Nullable Object value) { - return new Fact(key, String.valueOf(value)); + return new Fact(key, String.valueOf(value), false); } /** @@ -59,15 +61,59 @@ public static Fact fact(String key, @Nullable Object value) { * */ public static Fact simpleFact(String key) { - return new Fact(key, null); + return new Fact(key, null, false); + } + + /** + * Creates a fact with the given key and value, which will be printed in a format like "key: + * value." The numeric value is converted to a string with delimiting commas. + */ + static Fact numericFact(String key, @Nullable Long value) { + return new Fact(key, formatNumericValue(value), true); + } + + /** + * Creates a fact with the given key and value, which will be printed in a format like "key: + * value." The numeric value is converted to a string with delimiting commas. + */ + static Fact numericFact(String key, @Nullable Integer value) { + return new Fact(key, formatNumericValue(value), true); + } + + static String formatNumericValue(@Nullable Object value) { + if (value == null) { + return "null"; + } + + // We only support Long and Integer for now; maybe FP numbers in the future? + checkArgument(value instanceof Long || value instanceof Integer); + + // DecimalFormat is not available on all platforms + String stringValue = String.valueOf(value); + + boolean isNegative = stringValue.startsWith("-"); + if (isNegative) { + stringValue = stringValue.substring(1); + } + + StringBuilder builder = new StringBuilder(); + for (int i = 0; i < stringValue.length(); i++) { + builder.append(stringValue.charAt(i)); + if ((stringValue.length() - i - 1) % 3 == 0 && i != stringValue.length() - 1) { + builder.append(','); + } + } + return isNegative ? "-" + builder : builder.toString(); } final String key; final @Nullable String value; + final boolean padStart; - private Fact(String key, @Nullable String value) { + private Fact(String key, @Nullable String value, boolean padStart) { this.key = checkNotNull(key); this.value = value; + this.padStart = padStart; } /** @@ -86,10 +132,14 @@ public String toString() { */ static String makeMessage(ImmutableList messages, ImmutableList facts) { int longestKeyLength = 0; + int longestValueLength = 0; boolean seenNewlineInValue = false; for (Fact fact : facts) { if (fact.value != null) { longestKeyLength = max(longestKeyLength, fact.key.length()); + if (fact.padStart) { + longestValueLength = max(longestValueLength, fact.value.length()); + } // TODO(cpovirk): Look for other kinds of newlines. seenNewlineInValue |= fact.value.contains("\n"); } @@ -121,7 +171,11 @@ static String makeMessage(ImmutableList messages, ImmutableList fa } else { builder.append(padEnd(fact.key, longestKeyLength, ' ')); builder.append(": "); - builder.append(fact.value); + if (fact.padStart) { + builder.append(padStart(fact.value, longestValueLength, ' ')); + } else { + builder.append(fact.value); + } } builder.append('\n'); } diff --git a/core/src/main/java/com/google/common/truth/IntegerSubject.java b/core/src/main/java/com/google/common/truth/IntegerSubject.java index 99fd82fd5..8a3585152 100644 --- a/core/src/main/java/com/google/common/truth/IntegerSubject.java +++ b/core/src/main/java/com/google/common/truth/IntegerSubject.java @@ -17,7 +17,7 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; -import static com.google.common.truth.Fact.fact; +import static com.google.common.truth.Fact.numericFact; import static com.google.common.truth.MathUtil.equalWithinTolerance; import org.jspecify.annotations.Nullable; @@ -101,9 +101,9 @@ public void of(int expected) { if (!equalWithinTolerance(actual, expected, tolerance)) { failWithoutActual( - fact("expected", Integer.toString(expected)), - butWas(), - fact("outside tolerance", Integer.toString(tolerance))); + numericFact("expected", expected), + numericFact("but was", actual), + numericFact("outside tolerance", tolerance)); } } }; @@ -128,9 +128,9 @@ public void of(int expected) { if (equalWithinTolerance(actual, expected, tolerance)) { failWithoutActual( - fact("expected not to be", Integer.toString(expected)), - butWas(), - fact("within tolerance", Integer.toString(tolerance))); + numericFact("expected not to be", expected), + numericFact("but was", actual), + numericFact("within tolerance", tolerance)); } } }; diff --git a/core/src/main/java/com/google/common/truth/LongSubject.java b/core/src/main/java/com/google/common/truth/LongSubject.java index 746d3cc63..97a75b613 100644 --- a/core/src/main/java/com/google/common/truth/LongSubject.java +++ b/core/src/main/java/com/google/common/truth/LongSubject.java @@ -17,7 +17,7 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; -import static com.google.common.truth.Fact.fact; +import static com.google.common.truth.Fact.numericFact; import static com.google.common.truth.MathUtil.equalWithinTolerance; import org.jspecify.annotations.Nullable; @@ -102,9 +102,9 @@ public void of(long expected) { if (!equalWithinTolerance(actual, expected, tolerance)) { failWithoutActual( - fact("expected", Long.toString(expected)), - butWas(), - fact("outside tolerance", Long.toString(tolerance))); + numericFact("expected", expected), + numericFact("but was", actual), + numericFact("outside tolerance", tolerance)); } } }; @@ -129,9 +129,9 @@ public void of(long expected) { if (equalWithinTolerance(actual, expected, tolerance)) { failWithoutActual( - fact("expected not to be", Long.toString(expected)), - butWas(), - fact("within tolerance", Long.toString(tolerance))); + numericFact("expected not to be", expected), + numericFact("but was", actual), + numericFact("within tolerance", tolerance)); } } }; diff --git a/core/src/test/java/com/google/common/truth/FactTest.java b/core/src/test/java/com/google/common/truth/FactTest.java index 13957496f..02aaaeb4a 100644 --- a/core/src/test/java/com/google/common/truth/FactTest.java +++ b/core/src/test/java/com/google/common/truth/FactTest.java @@ -17,10 +17,14 @@ package com.google.common.truth; import static com.google.common.truth.Fact.fact; +import static com.google.common.truth.Fact.formatNumericValue; import static com.google.common.truth.Fact.makeMessage; +import static com.google.common.truth.Fact.numericFact; import static com.google.common.truth.Fact.simpleFact; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; +import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; import org.junit.Test; import org.junit.runner.RunWith; @@ -29,6 +33,9 @@ /** Tests for {@link Fact}. */ @RunWith(JUnit4.class) public class FactTest { + + private static final Joiner TEXT = Joiner.on("\n"); + @Test public void string() { assertThat(fact("foo", "bar").toString()).isEqualTo("foo: bar"); @@ -51,7 +58,10 @@ public void twoFacts() { makeMessage( ImmutableList.of(), ImmutableList.of(fact("foo", "bar"), fact("longer name", "other value")))) - .isEqualTo("foo : bar\nlonger name: other value"); + .isEqualTo( + TEXT.join( + "foo : bar", // force a line break + "longer name: other value")); } @Test @@ -60,10 +70,14 @@ public void numericFacts() { makeMessage( ImmutableList.of(), ImmutableList.of( - fact("expected", 802604), - fact("but was", 773804), - fact("outside tolerance", 9599)))) - .isEqualTo("expected : 802604\nbut was : 773804\noutside tolerance: 9599"); + numericFact("expected", 802604), + numericFact("but was", 773804), + numericFact("outside tolerance", 9599)))) + .isEqualTo( + TEXT.join( + "expected : 802,604", + "but was : 773,804", + "outside tolerance: 9,599")); } @Test @@ -101,4 +115,49 @@ public void withMessage() { assertThat(makeMessage(ImmutableList.of("hello"), ImmutableList.of(fact("foo", "bar")))) .isEqualTo("hello\nfoo: bar"); } + + @Test + public void formatNumericValue_null() { + assertThat(formatNumericValue(null)).isEqualTo("null"); + } + + @Test + public void formatNumericValue_zero() { + assertThat(formatNumericValue(0)).isEqualTo("0"); + assertThat(formatNumericValue(0L)).isEqualTo("0"); + assertThat(formatNumericValue(-0)).isEqualTo("0"); + assertThat(formatNumericValue(-0L)).isEqualTo("0"); + } + + @Test + public void formatNumericValue_positive() { + assertThat(formatNumericValue(9)).isEqualTo("9"); + assertThat(formatNumericValue(999L)).isEqualTo("999"); + assertThat(formatNumericValue(9599)).isEqualTo("9,599"); + assertThat(formatNumericValue(20000L)).isEqualTo("20,000"); + assertThat(formatNumericValue(802604)).isEqualTo("802,604"); + assertThat(formatNumericValue(1234567890)).isEqualTo("1,234,567,890"); + assertThat(formatNumericValue(1234567890L)).isEqualTo("1,234,567,890"); + assertThat(formatNumericValue(Integer.MAX_VALUE)).isEqualTo("2,147,483,647"); + assertThat(formatNumericValue(Long.MAX_VALUE)).isEqualTo("9,223,372,036,854,775,807"); + } + + @Test + public void formatNumericValue_negative() { + assertThat(formatNumericValue(-9)).isEqualTo("-9"); + assertThat(formatNumericValue(-999L)).isEqualTo("-999"); + assertThat(formatNumericValue(-9599)).isEqualTo("-9,599"); + assertThat(formatNumericValue(-20000L)).isEqualTo("-20,000"); + assertThat(formatNumericValue(-802604)).isEqualTo("-802,604"); + assertThat(formatNumericValue(-1234567890)).isEqualTo("-1,234,567,890"); + assertThat(formatNumericValue(-1234567890L)).isEqualTo("-1,234,567,890"); + assertThat(formatNumericValue(Integer.MIN_VALUE)).isEqualTo("-2,147,483,648"); + assertThat(formatNumericValue(Long.MIN_VALUE)).isEqualTo("-9,223,372,036,854,775,808"); + } + + @Test + public void formatNumericValue_throwsExceptionForNonNumericValue() { + assertThrows(IllegalArgumentException.class, () -> formatNumericValue("foo")); + assertThrows(IllegalArgumentException.class, () -> formatNumericValue(2.2)); + } } diff --git a/core/src/test/java/com/google/common/truth/IntegerSubjectTest.java b/core/src/test/java/com/google/common/truth/IntegerSubjectTest.java index a9f0fef9b..f97f0b563 100644 --- a/core/src/test/java/com/google/common/truth/IntegerSubjectTest.java +++ b/core/src/test/java/com/google/common/truth/IntegerSubjectTest.java @@ -16,6 +16,7 @@ package com.google.common.truth; import static com.google.common.truth.ExpectFailure.assertThat; +import static com.google.common.truth.Fact.formatNumericValue; import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.fail; @@ -155,9 +156,9 @@ public void invokeAssertion(SimpleSubjectBuilder expect .factKeys() .containsExactly("expected", "but was", "outside tolerance") .inOrder(); - assertThat(failure).factValue("expected").isEqualTo(Integer.toString(expected)); - assertThat(failure).factValue("but was").isEqualTo(Integer.toString(actual)); - assertThat(failure).factValue("outside tolerance").isEqualTo(Integer.toString(tolerance)); + assertThat(failure).factValue("expected").isEqualTo(formatNumericValue(expected)); + assertThat(failure).factValue("but was").isEqualTo(formatNumericValue(actual)); + assertThat(failure).factValue("outside tolerance").isEqualTo(formatNumericValue(tolerance)); } @Test @@ -191,8 +192,8 @@ public void invokeAssertion(SimpleSubjectBuilder expect } }; AssertionError failure = expectFailure(callback); - assertThat(failure).factValue("expected not to be").isEqualTo(Integer.toString(expected)); - assertThat(failure).factValue("within tolerance").isEqualTo(Integer.toString(tolerance)); + assertThat(failure).factValue("expected not to be").isEqualTo(formatNumericValue(expected)); + assertThat(failure).factValue("within tolerance").isEqualTo(formatNumericValue(tolerance)); } @Test diff --git a/core/src/test/java/com/google/common/truth/LongSubjectTest.java b/core/src/test/java/com/google/common/truth/LongSubjectTest.java index c2c2e15f3..638737922 100644 --- a/core/src/test/java/com/google/common/truth/LongSubjectTest.java +++ b/core/src/test/java/com/google/common/truth/LongSubjectTest.java @@ -16,6 +16,7 @@ package com.google.common.truth; import static com.google.common.truth.ExpectFailure.assertThat; +import static com.google.common.truth.Fact.formatNumericValue; import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.fail; @@ -169,9 +170,9 @@ public void invokeAssertion(SimpleSubjectBuilder expect) { .factKeys() .containsExactly("expected", "but was", "outside tolerance") .inOrder(); - assertThat(failure).factValue("expected").isEqualTo(Long.toString(expected)); - assertThat(failure).factValue("but was").isEqualTo(Long.toString(actual)); - assertThat(failure).factValue("outside tolerance").isEqualTo(Long.toString(tolerance)); + assertThat(failure).factValue("expected").isEqualTo(formatNumericValue(expected)); + assertThat(failure).factValue("but was").isEqualTo(formatNumericValue(actual)); + assertThat(failure).factValue("outside tolerance").isEqualTo(formatNumericValue(tolerance)); } @Test @@ -205,8 +206,8 @@ public void invokeAssertion(SimpleSubjectBuilder expect) { } }; AssertionError failure = expectFailure(callback); - assertThat(failure).factValue("expected not to be").isEqualTo(Long.toString(expected)); - assertThat(failure).factValue("within tolerance").isEqualTo(Long.toString(tolerance)); + assertThat(failure).factValue("expected not to be").isEqualTo(formatNumericValue(expected)); + assertThat(failure).factValue("within tolerance").isEqualTo(formatNumericValue(tolerance)); } @Test