Skip to content

Commit

Permalink
Compare for equality, not approximate equality within a tolerance of …
Browse files Browse the repository at this point in the history
…`0.0`.

IIRC, the behavior of the two differs only for:
- comparing NaN to itself
- comparing either infinity to itself
- comparing positive zero to negative zero

And as best I can tell, the only one of those that has any chance of coming up in these tests is the zero case. And, as best I can tell, in those cases, we actively want to test that we're returning positive zero, not negative zero. So let's test for that, and let's simplify the code in doing so.

(followup to cl/649135877; "followup" to cl/108355695, which ported these assertions from JUnit, known for steering users away from exact-equality comparisons)

RELNOTES=n/a
PiperOrigin-RevId: 649195688
  • Loading branch information
cpovirk authored and Google Java Core Libraries committed Jul 3, 2024
1 parent c8829bf commit 8dac907
Show file tree
Hide file tree
Showing 12 changed files with 30 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -179,10 +179,8 @@ public void testPopulationCovariance() {
assertThrows(
IllegalStateException.class,
() -> emptyAccumulatorByAddAllEmptyPairedStats.populationCovariance());
assertThat(oneValueAccumulator.populationCovariance()).isWithin(0.0).of(0.0);
assertThat(oneValueAccumulatorByAddAllEmptyPairedStats.populationCovariance())
.isWithin(0.0)
.of(0.0);
assertThat(oneValueAccumulator.populationCovariance()).isEqualTo(0.0);
assertThat(oneValueAccumulatorByAddAllEmptyPairedStats.populationCovariance()).isEqualTo(0.0);
assertThat(twoValuesAccumulator.populationCovariance())
.isWithin(ALLOWED_ERROR)
.of(TWO_VALUES_SUM_OF_PRODUCTS_OF_DELTAS / 2);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public void testYStats() {

public void testPopulationCovariance() {
assertThrows(IllegalStateException.class, () -> EMPTY_PAIRED_STATS.populationCovariance());
assertThat(ONE_VALUE_PAIRED_STATS.populationCovariance()).isWithin(0.0).of(0.0);
assertThat(ONE_VALUE_PAIRED_STATS.populationCovariance()).isEqualTo(0.0);
assertThat(createSingleStats(Double.POSITIVE_INFINITY, 1.23).populationCovariance()).isNaN();
assertThat(createSingleStats(Double.NEGATIVE_INFINITY, 1.23).populationCovariance()).isNaN();
assertThat(createSingleStats(Double.NaN, 1.23).populationCovariance()).isNaN();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,9 +269,9 @@ public void testMean() {
}

public void testSum() {
assertThat(emptyAccumulator.sum()).isWithin(0.0).of(0.0);
assertThat(emptyAccumulatorByAddAllEmptyIterable.sum()).isWithin(0.0).of(0.0);
assertThat(emptyAccumulatorByAddAllEmptyStats.sum()).isWithin(0.0).of(0.0);
assertThat(emptyAccumulator.sum()).isEqualTo(0.0);
assertThat(emptyAccumulatorByAddAllEmptyIterable.sum()).isEqualTo(0.0);
assertThat(emptyAccumulatorByAddAllEmptyStats.sum()).isEqualTo(0.0);
assertThat(oneValueAccumulator.sum()).isWithin(ALLOWED_ERROR).of(ONE_VALUE);
assertThat(oneValueAccumulatorByAddAllEmptyStats.sum()).isWithin(ALLOWED_ERROR).of(ONE_VALUE);
assertThat(twoValuesAccumulator.sum()).isWithin(ALLOWED_ERROR).of(TWO_VALUES_MEAN * 2);
Expand Down Expand Up @@ -317,8 +317,8 @@ public void testPopulationVariance() {
() -> emptyAccumulatorByAddAllEmptyIterable.populationVariance());
assertThrows(
IllegalStateException.class, () -> emptyAccumulatorByAddAllEmptyStats.populationVariance());
assertThat(oneValueAccumulator.populationVariance()).isWithin(0.0).of(0.0);
assertThat(oneValueAccumulatorByAddAllEmptyStats.populationVariance()).isWithin(0.0).of(0.0);
assertThat(oneValueAccumulator.populationVariance()).isEqualTo(0.0);
assertThat(oneValueAccumulatorByAddAllEmptyStats.populationVariance()).isEqualTo(0.0);
assertThat(twoValuesAccumulator.populationVariance())
.isWithin(ALLOWED_ERROR)
.of(TWO_VALUES_SUM_OF_SQUARES_OF_DELTAS / 2);
Expand Down Expand Up @@ -392,10 +392,8 @@ public void testPopulationStandardDeviation() {
assertThrows(
IllegalStateException.class,
() -> emptyAccumulatorByAddAllEmptyStats.populationStandardDeviation());
assertThat(oneValueAccumulator.populationStandardDeviation()).isWithin(0.0).of(0.0);
assertThat(oneValueAccumulatorByAddAllEmptyStats.populationStandardDeviation())
.isWithin(0.0)
.of(0.0);
assertThat(oneValueAccumulator.populationStandardDeviation()).isEqualTo(0.0);
assertThat(oneValueAccumulatorByAddAllEmptyStats.populationStandardDeviation()).isEqualTo(0.0);
assertThat(twoValuesAccumulator.populationStandardDeviation())
.isWithin(ALLOWED_ERROR)
.of(sqrt(TWO_VALUES_SUM_OF_SQUARES_OF_DELTAS / 2));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ public void testSum() {
public void testPopulationVariance() {
assertThrows(IllegalStateException.class, () -> EMPTY_STATS_VARARGS.populationVariance());
assertThrows(IllegalStateException.class, () -> EMPTY_STATS_ITERABLE.populationVariance());
assertThat(ONE_VALUE_STATS.populationVariance()).isWithin(0.0).of(0.0);
assertThat(ONE_VALUE_STATS.populationVariance()).isEqualTo(0.0);
assertThat(Stats.of(POSITIVE_INFINITY).populationVariance()).isNaN();
assertThat(Stats.of(NEGATIVE_INFINITY).populationVariance()).isNaN();
assertThat(Stats.of(NaN).populationVariance()).isNaN();
Expand Down Expand Up @@ -245,7 +245,7 @@ public void testPopulationStandardDeviation() {
IllegalStateException.class, () -> EMPTY_STATS_VARARGS.populationStandardDeviation());
assertThrows(
IllegalStateException.class, () -> EMPTY_STATS_ITERABLE.populationStandardDeviation());
assertThat(ONE_VALUE_STATS.populationStandardDeviation()).isWithin(0.0).of(0.0);
assertThat(ONE_VALUE_STATS.populationStandardDeviation()).isEqualTo(0.0);
assertThat(TWO_VALUES_STATS.populationStandardDeviation())
.isWithin(ALLOWED_ERROR)
.of(sqrt(TWO_VALUES_SUM_OF_SQUARES_OF_DELTAS / 2));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ static void assertStatsApproxEqual(Stats expectedStats, Stats actualStats) {
}
} else if (expectedStats.count() == 1) {
assertThat(actualStats.mean()).isWithin(ALLOWED_ERROR).of(expectedStats.mean());
assertThat(actualStats.populationVariance()).isWithin(0.0).of(0.0);
assertThat(actualStats.populationVariance()).isEqualTo(0.0);
assertThat(actualStats.min()).isWithin(ALLOWED_ERROR).of(expectedStats.min());
assertThat(actualStats.max()).isWithin(ALLOWED_ERROR).of(expectedStats.max());
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,7 @@ public void testFloatValue() {
UnsignedLong unsignedValue = UnsignedLong.fromLongBits(value);
assertWithMessage("Float value of " + unsignedValue)
.that(unsignedValue.floatValue())
.isWithin(0.0f)
.of(unsignedValue.bigIntegerValue().floatValue());
.isEqualTo(unsignedValue.bigIntegerValue().floatValue());
}
}

Expand All @@ -161,8 +160,7 @@ public void testDoubleValue() {
UnsignedLong unsignedValue = UnsignedLong.fromLongBits(value);
assertWithMessage("Double value of " + unsignedValue)
.that(unsignedValue.doubleValue())
.isWithin(0.0)
.of(unsignedValue.bigIntegerValue().doubleValue());
.isEqualTo(unsignedValue.bigIntegerValue().doubleValue());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,10 +179,8 @@ public void testPopulationCovariance() {
assertThrows(
IllegalStateException.class,
() -> emptyAccumulatorByAddAllEmptyPairedStats.populationCovariance());
assertThat(oneValueAccumulator.populationCovariance()).isWithin(0.0).of(0.0);
assertThat(oneValueAccumulatorByAddAllEmptyPairedStats.populationCovariance())
.isWithin(0.0)
.of(0.0);
assertThat(oneValueAccumulator.populationCovariance()).isEqualTo(0.0);
assertThat(oneValueAccumulatorByAddAllEmptyPairedStats.populationCovariance()).isEqualTo(0.0);
assertThat(twoValuesAccumulator.populationCovariance())
.isWithin(ALLOWED_ERROR)
.of(TWO_VALUES_SUM_OF_PRODUCTS_OF_DELTAS / 2);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public void testYStats() {

public void testPopulationCovariance() {
assertThrows(IllegalStateException.class, () -> EMPTY_PAIRED_STATS.populationCovariance());
assertThat(ONE_VALUE_PAIRED_STATS.populationCovariance()).isWithin(0.0).of(0.0);
assertThat(ONE_VALUE_PAIRED_STATS.populationCovariance()).isEqualTo(0.0);
assertThat(createSingleStats(Double.POSITIVE_INFINITY, 1.23).populationCovariance()).isNaN();
assertThat(createSingleStats(Double.NEGATIVE_INFINITY, 1.23).populationCovariance()).isNaN();
assertThat(createSingleStats(Double.NaN, 1.23).populationCovariance()).isNaN();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,9 +276,9 @@ public void testMean() {
}

public void testSum() {
assertThat(emptyAccumulator.sum()).isWithin(0.0).of(0.0);
assertThat(emptyAccumulatorByAddAllEmptyIterable.sum()).isWithin(0.0).of(0.0);
assertThat(emptyAccumulatorByAddAllEmptyStats.sum()).isWithin(0.0).of(0.0);
assertThat(emptyAccumulator.sum()).isEqualTo(0.0);
assertThat(emptyAccumulatorByAddAllEmptyIterable.sum()).isEqualTo(0.0);
assertThat(emptyAccumulatorByAddAllEmptyStats.sum()).isEqualTo(0.0);
assertThat(oneValueAccumulator.sum()).isWithin(ALLOWED_ERROR).of(ONE_VALUE);
assertThat(oneValueAccumulatorByAddAllEmptyStats.sum()).isWithin(ALLOWED_ERROR).of(ONE_VALUE);
assertThat(twoValuesAccumulator.sum()).isWithin(ALLOWED_ERROR).of(TWO_VALUES_MEAN * 2);
Expand Down Expand Up @@ -324,8 +324,8 @@ public void testPopulationVariance() {
() -> emptyAccumulatorByAddAllEmptyIterable.populationVariance());
assertThrows(
IllegalStateException.class, () -> emptyAccumulatorByAddAllEmptyStats.populationVariance());
assertThat(oneValueAccumulator.populationVariance()).isWithin(0.0).of(0.0);
assertThat(oneValueAccumulatorByAddAllEmptyStats.populationVariance()).isWithin(0.0).of(0.0);
assertThat(oneValueAccumulator.populationVariance()).isEqualTo(0.0);
assertThat(oneValueAccumulatorByAddAllEmptyStats.populationVariance()).isEqualTo(0.0);
assertThat(twoValuesAccumulator.populationVariance())
.isWithin(ALLOWED_ERROR)
.of(TWO_VALUES_SUM_OF_SQUARES_OF_DELTAS / 2);
Expand Down Expand Up @@ -399,10 +399,8 @@ public void testPopulationStandardDeviation() {
assertThrows(
IllegalStateException.class,
() -> emptyAccumulatorByAddAllEmptyStats.populationStandardDeviation());
assertThat(oneValueAccumulator.populationStandardDeviation()).isWithin(0.0).of(0.0);
assertThat(oneValueAccumulatorByAddAllEmptyStats.populationStandardDeviation())
.isWithin(0.0)
.of(0.0);
assertThat(oneValueAccumulator.populationStandardDeviation()).isEqualTo(0.0);
assertThat(oneValueAccumulatorByAddAllEmptyStats.populationStandardDeviation()).isEqualTo(0.0);
assertThat(twoValuesAccumulator.populationStandardDeviation())
.isWithin(ALLOWED_ERROR)
.of(sqrt(TWO_VALUES_SUM_OF_SQUARES_OF_DELTAS / 2));
Expand Down
4 changes: 2 additions & 2 deletions guava-tests/test/com/google/common/math/StatsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ public void testSum() {
public void testPopulationVariance() {
assertThrows(IllegalStateException.class, () -> EMPTY_STATS_VARARGS.populationVariance());
assertThrows(IllegalStateException.class, () -> EMPTY_STATS_ITERABLE.populationVariance());
assertThat(ONE_VALUE_STATS.populationVariance()).isWithin(0.0).of(0.0);
assertThat(ONE_VALUE_STATS.populationVariance()).isEqualTo(0.0);
assertThat(Stats.of(POSITIVE_INFINITY).populationVariance()).isNaN();
assertThat(Stats.of(NEGATIVE_INFINITY).populationVariance()).isNaN();
assertThat(Stats.of(NaN).populationVariance()).isNaN();
Expand Down Expand Up @@ -253,7 +253,7 @@ public void testPopulationStandardDeviation() {
IllegalStateException.class, () -> EMPTY_STATS_VARARGS.populationStandardDeviation());
assertThrows(
IllegalStateException.class, () -> EMPTY_STATS_ITERABLE.populationStandardDeviation());
assertThat(ONE_VALUE_STATS.populationStandardDeviation()).isWithin(0.0).of(0.0);
assertThat(ONE_VALUE_STATS.populationStandardDeviation()).isEqualTo(0.0);
assertThat(TWO_VALUES_STATS.populationStandardDeviation())
.isWithin(ALLOWED_ERROR)
.of(sqrt(TWO_VALUES_SUM_OF_SQUARES_OF_DELTAS / 2));
Expand Down
2 changes: 1 addition & 1 deletion guava-tests/test/com/google/common/math/StatsTesting.java
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ static void assertStatsApproxEqual(Stats expectedStats, Stats actualStats) {
}
} else if (expectedStats.count() == 1) {
assertThat(actualStats.mean()).isWithin(ALLOWED_ERROR).of(expectedStats.mean());
assertThat(actualStats.populationVariance()).isWithin(0.0).of(0.0);
assertThat(actualStats.populationVariance()).isEqualTo(0.0);
assertThat(actualStats.min()).isWithin(ALLOWED_ERROR).of(expectedStats.min());
assertThat(actualStats.max()).isWithin(ALLOWED_ERROR).of(expectedStats.max());
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,7 @@ public void testFloatValue() {
UnsignedLong unsignedValue = UnsignedLong.fromLongBits(value);
assertWithMessage("Float value of " + unsignedValue)
.that(unsignedValue.floatValue())
.isWithin(0.0f)
.of(unsignedValue.bigIntegerValue().floatValue());
.isEqualTo(unsignedValue.bigIntegerValue().floatValue());
}
}

Expand All @@ -161,8 +160,7 @@ public void testDoubleValue() {
UnsignedLong unsignedValue = UnsignedLong.fromLongBits(value);
assertWithMessage("Double value of " + unsignedValue)
.that(unsignedValue.doubleValue())
.isWithin(0.0)
.of(unsignedValue.bigIntegerValue().doubleValue());
.isEqualTo(unsignedValue.bigIntegerValue().doubleValue());
}
}

Expand Down

0 comments on commit 8dac907

Please sign in to comment.