From 1a4c67979fc1d5059d4038ff3e1f4898da8e6952 Mon Sep 17 00:00:00 2001 From: cpovirk Date: Fri, 22 Dec 2017 06:39:11 -0800 Subject: [PATCH] Have a custom implementation of Arrays.equals(double[], double[]) that is similar to JDK's version to avoid using GWT's emulated Arrays which has different behavior. Update failure messages to match the JDK's as much as possible. (based on CL 179217793) [] RELNOTES=Worked around GWT's buggy handling of `double[]` equality. Also, changed the rendering of `double[]` values under GWT to more closely match Java. The change requires [an API that isn't present in older browers](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/toLocaleString), so please report any problems you encounter. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=179922005 --- .../com/google/common/truth/Platform.java | 4 + .../truth/PrimitiveDoubleArraySubject.java | 85 +++++++++++++++---- .../com/google/common/truth/Platform.java | 60 ++++++++++++- .../PrimitiveDoubleArraySubjectTest.java | 32 +++++-- 4 files changed, 158 insertions(+), 23 deletions(-) diff --git a/core/src/main/java/com/google/common/truth/Platform.java b/core/src/main/java/com/google/common/truth/Platform.java index 691095cd0..b1bd8cc70 100644 --- a/core/src/main/java/com/google/common/truth/Platform.java +++ b/core/src/main/java/com/google/common/truth/Platform.java @@ -137,6 +137,10 @@ public String toString() { } } + static String doubleToString(double value) { + return Double.toString(value); + } + /** Returns a human readable string representation of the throwable's stack trace. */ static String getStackTraceAsString(Throwable throwable) { return Throwables.getStackTraceAsString(throwable); diff --git a/core/src/main/java/com/google/common/truth/PrimitiveDoubleArraySubject.java b/core/src/main/java/com/google/common/truth/PrimitiveDoubleArraySubject.java index d4b7dfdae..7c4dc8699 100644 --- a/core/src/main/java/com/google/common/truth/PrimitiveDoubleArraySubject.java +++ b/core/src/main/java/com/google/common/truth/PrimitiveDoubleArraySubject.java @@ -22,12 +22,12 @@ import static com.google.common.truth.DoubleSubject.checkTolerance; import static com.google.common.truth.MathUtil.equalWithinTolerance; import static com.google.common.truth.MathUtil.notEqualWithinTolerance; +import static com.google.common.truth.Platform.doubleToString; import com.google.common.collect.Iterables; import com.google.common.primitives.Doubles; import com.google.errorprone.annotations.CanIgnoreReturnValue; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import javax.annotation.Nullable; @@ -48,8 +48,8 @@ protected String underlyingType() { } @Override - protected List listRepresentation() { - return Doubles.asList(actual()); + protected List listRepresentation() { + return doubleArrayAsString(actual()); } /** @@ -84,8 +84,8 @@ public void isEqualTo(Object expected) { } try { double[] expectedArray = (double[]) expected; - if (!Arrays.equals(actual, expectedArray)) { - fail("is equal to", Doubles.asList(expectedArray)); + if (!arrayEquals(actual, expectedArray)) { + fail("is equal to", doubleArrayAsString(expectedArray)); } } catch (ClassCastException e) { failWithBadType(expected); @@ -117,7 +117,7 @@ public void isEqualTo(Object expected, double tolerance) { if (expectedArray.length != actual.length) { failWithRawMessage( "Arrays are of different lengths. expected: %s, actual %s", - Doubles.asList(expectedArray), Doubles.asList(actual)); + doubleArrayAsString(expectedArray), doubleArrayAsString(actual)); return; } List unequalIndices = new ArrayList<>(); @@ -128,7 +128,7 @@ public void isEqualTo(Object expected, double tolerance) { } if (!unequalIndices.isEmpty()) { - fail("is equal to", Doubles.asList(expectedArray)); + fail("is equal to", doubleArrayAsString(expectedArray)); return; } } catch (ClassCastException e) { @@ -155,9 +155,9 @@ public void isNotEqualTo(Object expected) { double[] actual = actual(); try { double[] expectedArray = (double[]) expected; - if (actual == expected || Arrays.equals(actual, expectedArray)) { + if (actual == expected || arrayEquals(actual, expectedArray)) { failWithRawMessage( - "%s unexpectedly equal to %s.", actualAsString(), Doubles.asList(expectedArray)); + "%s unexpectedly equal to %s.", actualAsString(), doubleArrayAsString(expectedArray)); } } catch (ClassCastException ignored) { // If it's not double[] then it's not equal and the test passes. @@ -185,7 +185,7 @@ public void isNotEqualTo(Object expectedArray, double tolerance) { double[] expected = (double[]) expectedArray; if (actual == expected) { failWithRawMessage( - "%s unexpectedly equal to %s.", actualAsString(), Doubles.asList(expected)); + "%s unexpectedly equal to %s.", actualAsString(), doubleArrayAsString(expected)); return; } if (expected.length != actual.length) { @@ -199,7 +199,7 @@ public void isNotEqualTo(Object expectedArray, double tolerance) { } if (unequalIndices.isEmpty()) { failWithRawMessage( - "%s unexpectedly equal to %s.", actualAsString(), Doubles.asList(expected)); + "%s unexpectedly equal to %s.", actualAsString(), doubleArrayAsString(expected)); return; } } catch (ClassCastException ignored) { @@ -397,7 +397,8 @@ public DoubleArrayAsIterable usingTolerance(double tolerance) { @Override public boolean compare(Double actual, Number expected) { - return actual.equals(checkedToDouble(expected)); + return Double.doubleToLongBits(actual) + == Double.doubleToLongBits(checkedToDouble(expected)); } @Override @@ -506,8 +507,62 @@ public void containsNoneOf(double[] excluded) { } private IterableSubject iterableSubject() { - return internalCustomName() != null - ? check().that(listRepresentation()).named(internalCustomName()) - : check().that(listRepresentation()); + IterableSubject result = + check().about(iterablesWithCustomDoubleToString()).that(Doubles.asList(actual())); + return internalCustomName() != null ? result.named(internalCustomName()) : result; + } + + /* + * TODO(cpovirk): Should we make Doubles.asList().toString() smarter rather than do all this? + * + * TODO(cpovirk): Or find a general solution for this and MultimapSubject.IterableEntries. But + * note that here we don't use _exactly_ PrimitiveDoubleArraySubject.this.toString(), as that + * contains "double[]." Or maybe we should stop including that in + * PrimitiveDoubleArraySubject.this.toString(), too, someday? + */ + private Factory> iterablesWithCustomDoubleToString() { + return new Factory>() { + @Override + public IterableSubject createSubject(FailureMetadata metadata, Iterable actual) { + return new IterableSubjectWithInheritedToString(metadata, actual); + } + }; + } + + private final class IterableSubjectWithInheritedToString extends IterableSubject { + IterableSubjectWithInheritedToString(FailureMetadata metadata, Iterable actual) { + super(metadata, actual); + } + + @Override + protected String actualCustomStringRepresentation() { + return doubleArrayAsString(PrimitiveDoubleArraySubject.this.actual()).toString(); + } + } + + private static boolean arrayEquals(double[] left, double[] right) { + if (left == right) { + return true; + } + if (left == null || right == null) { + return false; + } + if (left.length != right.length) { + return false; + } + for (int i = 0; i < left.length; i++) { + if (Double.doubleToLongBits(left[i]) != Double.doubleToLongBits(right[i])) { + return false; + } + } + return true; + } + + static List doubleArrayAsString(double[] items) { + List itemAsStrings = new ArrayList(items.length); + for (double item : items) { + itemAsStrings.add(doubleToString(item)); + } + return itemAsStrings; } } diff --git a/core/src/main/java/com/google/common/truth/super/com/google/common/truth/Platform.java b/core/src/main/java/com/google/common/truth/super/com/google/common/truth/Platform.java index 6cadefa0a..0fc2fa7bd 100644 --- a/core/src/main/java/com/google/common/truth/super/com/google/common/truth/Platform.java +++ b/core/src/main/java/com/google/common/truth/super/com/google/common/truth/Platform.java @@ -16,11 +16,15 @@ package com.google.common.truth; import static com.google.common.truth.StringUtil.format; +import static java.lang.Double.NEGATIVE_INFINITY; +import static java.lang.Double.POSITIVE_INFINITY; +import static java.lang.Double.parseDouble; +import static jsinterop.annotations.JsPackage.GLOBAL; import com.google.common.truth.Truth.AssertionErrorWithCause; import java.util.LinkedHashSet; import java.util.Set; -import jsinterop.annotations.JsPackage; +import jsinterop.annotations.JsProperty; import jsinterop.annotations.JsType; /** @@ -79,6 +83,22 @@ static boolean isStackTraceCleaningDisabled() { return false; } + static String doubleToString(double value) { + // This probably doesn't match Java perfectly, but we do our best. + if (value == POSITIVE_INFINITY) { + return "Infinity"; + } else if (value == NEGATIVE_INFINITY) { + return "-Infinity"; + } else if (value == 0 && 1 / value < 0) { + return "-0.0"; + } else { + // TODO(cpovirk): Would it make more sense to pass `undefined` for the locale? But how? + // Then again, we're already hardcoding "Infinity," an English word, above.... + String result = ((Number) (Object) value).toLocaleString("en-US", JavaLikeOptions.INSTANCE); + return (parseDouble(result) == value) ? result : Double.toString(value); + } + } + /** Tests if current platform is Android which is always false. */ static boolean isAndroid() { return false; @@ -101,10 +121,46 @@ private static NativeRegExp compile(String pattern) { return new NativeRegExp(pattern); } - @JsType(isNative = true, name = "RegExp", namespace = JsPackage.GLOBAL) + @JsType(isNative = true, name = "RegExp", namespace = GLOBAL) private static class NativeRegExp { public NativeRegExp(String pattern) {} public native boolean test(String input); } + + @JsType(isNative = true, name = "Number", namespace = GLOBAL) + private interface Number { + String toLocaleString(Object locales, ToLocaleStringOptions options); + } + + @JsType(isNative = true, name = "?", namespace = GLOBAL) // "structural type"; see JsType Javadoc + private interface ToLocaleStringOptions { + @JsProperty + int getMinimumFractionDigits(); + + @JsProperty + int getMaximumFractionDigits(); + + @JsProperty + boolean getUseGrouping(); + } + + private static final class JavaLikeOptions implements ToLocaleStringOptions { + private static final ToLocaleStringOptions INSTANCE = new JavaLikeOptions(); + + @Override + public int getMinimumFractionDigits() { + return 1; + } + + @Override + public int getMaximumFractionDigits() { + return 20; + } + + @Override + public boolean getUseGrouping() { + return false; + } + } } diff --git a/core/src/test/java/com/google/common/truth/PrimitiveDoubleArraySubjectTest.java b/core/src/test/java/com/google/common/truth/PrimitiveDoubleArraySubjectTest.java index e94a18433..75540b81f 100644 --- a/core/src/test/java/com/google/common/truth/PrimitiveDoubleArraySubjectTest.java +++ b/core/src/test/java/com/google/common/truth/PrimitiveDoubleArraySubjectTest.java @@ -109,7 +109,6 @@ public void isEqualTo_WithoutToleranceParameter_Fail_Shorter() { } @Test - @GwtIncompatible("gwt Arrays.equals(double[], double[])") public void isEqualTo_WithoutToleranceParameter_Fail_PlusMinusZero() { expectFailure.whenTesting().that(array(0.0d)).isEqualTo(array(-0.0d)); assertThat(expectFailure.getFailure()) @@ -265,7 +264,6 @@ public void isNotEqualTo_WithoutToleranceParameter_FailEquals() { } @Test - @GwtIncompatible("gwt Arrays.equals(double[], double[])") public void isNotEqualTo_WithoutToleranceParameter_NaN_plusZero_FailEquals() { expectFailure .whenTesting() @@ -299,7 +297,6 @@ public void isNotEqualTo_WithoutToleranceParameter_Success_Shorter() { } @Test - @GwtIncompatible("gwt Arrays.equals(double[], double[])") public void isNotEqualTo_WithoutToleranceParameter_Success_PlusMinusZero() { assertThat(array(0.0d)).isNotEqualTo(array(-0.0d)); } @@ -1461,20 +1458,26 @@ public void usingExactEquality_contains_successWithInfinity() { } @Test - @GwtIncompatible("gwt Arrays.equals(double[], double[])") public void usingExactEquality_contains_successWithNaN() { assertThat(array(1.1, NaN, 3.3)).usingExactEquality().contains(NaN); } @Test - @GwtIncompatible("gwt Arrays.equals(double[], double[])") public void usingExactEquality_contains_failureWithNegativeZero() { expectFailure.whenTesting().that(array(1.1, -0.0, 3.3)).usingExactEquality().contains(0.0); + /* + * TODO(cpovirk): Find a way to print "0.0" rather than 0 in the error, even under GWT. One + * easy(?) hack would be to make UsingCorrespondence use Platform.doubleToString() when + * applicable. Or maybe Correspondence implementations should be able to provide custom string + * conversions, similar to how we plan to let them render their own diffs. + */ assertThat(expectFailure.getFailure()) .hasMessageThat() .isEqualTo( "Not true that <[1.1, -0.0, 3.3]> contains at least one element that is " - + "exactly equal to <0.0>"); + + "exactly equal to <" + + 0.0 + + ">"); } @Test @@ -1607,6 +1610,23 @@ public void usingExactEquality_containsNoneOf_primitiveDoubleArray_failure() { + "equal to each of <[2.2]>"); } + @Test + public void smallDifferenceInLongRepresentation() { + expectFailure + .whenTesting() + .that(array(-4.4501477170144023E-308)) + .isEqualTo(array(-4.450147717014402E-308)); + } + + @Test + public void noCommas() { + // Maybe we should include commas, but we don't yet, so make sure we don't under GWT, either. + expectFailure.whenTesting().that(array(10000.0)).isEqualTo(array(20000.0)); + assertThat(expectFailure.getFailure()) + .hasMessageThat() + .isEqualTo("Not true that <(double[]) [10000.0]> is equal to <[20000.0]>"); + } + private static double[] array(double... primitives) { return primitives; }