Nathan Beyer 写道:
-----Original Message----- From: Spark Shen [mailto:[EMAIL PROTECTED]+ assertEquals("Should return -0.0", -0.0, Math.cbrt(-0.0), 0D);This assertion is not correct here. The spec of java.lang.Math.cbrt(double) says: 'If the argument is zero, then the result is a zero with the same sign as the argument.' But, assert this way, the sign of the result will be omitted. That is to say, assertEquals("Should return -0.0", -0.0, Math.expm1(+0.0), 0D); will also pass. This occurs in many other places in the commit, I suggest to eliminate all the inappropriate delta.The problem isn't the delta, the problem is that these tests were ALL using boxing, which meant that assertion was based on Double.equals(Object). As such, this makes the tests confusing and unclear, especially with all of the assertions working this way. In this specific case, where you want to check the sign, you want to use an assertion that's similar to how Double.equals works - convert each double into bit representations and assert that all bit values are the same. But in cases where you want to assert a specific double value, say 3.0, you want to test the double, not the bit values. My personal opinion is that in test cases around primitives, there should be NO boxing/unboxing utilized. Make the tests explicit and clear. For example, to completely test this case where -0.0 is returned, I would suggest the following assertions: double actual = Math.cbrt(-0.0); // call assertEquals(long, long) assertEquals(Double.doubleToLongBits(-0.0),Double.doubleToLongBits(actual));
Hi Nathan: Agree. And I will refactor the test case and provide a patch soon.
Compare that to this (which only compiles with Java 5). // call assertEquals(Object, Object) assertEquals(-0.0, Math.cbrt(-0.0)); Isn't the former more obvious and straightforward?After looking into the source code JUnit: static public void assertEquals(String message, double expected, double actual, double delta) { // handle infinity specially since subtracting to infinite values gives NaN and the // the following test fails if (Double.isInfinite(expected)) { if (!(expected == actual)) failNotEquals(message, new Double(expected), new Double(actual)); } else if (!(Math.abs(expected-actual) <= delta)) // Because comparison with NaN always returns false (Spark comment: if condition will make sign of zero meaningless) failNotEquals(message, new Double(expected), new Double(actual)); } This may be a bug of JUnit. Since it does not differentiate +/-0.If this is a bug, it's too late now. JUnit has been this way for years. -Nathan --------------------------------------------------------------------- Terms of use : http://incubator.apache.org/harmony/mailing.html To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
-- Spark Shen China Software Development Lab, IBM --------------------------------------------------------------------- Terms of use : http://incubator.apache.org/harmony/mailing.html To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
