Skip to content

Commit

Permalink
Have a custom implementation of Arrays.equals(double[], double[]) tha…
Browse files Browse the repository at this point in the history
…t 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
  • Loading branch information
cpovirk committed Dec 22, 2017
1 parent 5176771 commit 1a4c679
Show file tree
Hide file tree
Showing 4 changed files with 158 additions and 23 deletions.
4 changes: 4 additions & 0 deletions core/src/main/java/com/google/common/truth/Platform.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -48,8 +48,8 @@ protected String underlyingType() {
}

@Override
protected List<Double> listRepresentation() {
return Doubles.asList(actual());
protected List<String> listRepresentation() {
return doubleArrayAsString(actual());
}

/**
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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<Integer> unequalIndices = new ArrayList<>();
Expand All @@ -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) {
Expand All @@ -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.
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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<IterableSubject, Iterable<?>> iterablesWithCustomDoubleToString() {
return new Factory<IterableSubject, Iterable<?>>() {
@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<String> doubleArrayAsString(double[] items) {
List<String> itemAsStrings = new ArrayList<String>(items.length);
for (double item : items) {
itemAsStrings.add(doubleToString(item));
}
return itemAsStrings;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit 1a4c679

Please sign in to comment.