You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2021/06/20 06:14:49 UTC
[GitHub] [commons-numbers] arturobernalg opened a new pull request #98: Replace assert by simpler but equivalent calls.
arturobernalg opened a new pull request #98:
URL: https://github.com/apache/commons-numbers/pull/98
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-numbers] aherbert commented on a change in pull request #98: Replace assert by simpler but equivalent calls.
Posted by GitBox <gi...@apache.org>.
aherbert commented on a change in pull request #98:
URL: https://github.com/apache/commons-numbers/pull/98#discussion_r654888225
##########
File path: commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/FractionTest.java
##########
@@ -463,10 +463,10 @@ void testSubtract() {
@Test
void testEqualsAndHashCode() {
final Fraction zero = Fraction.of(0, 1);
- Assertions.assertTrue(zero.equals(zero));
- Assertions.assertFalse(zero.equals(null));
- Assertions.assertFalse(zero.equals(new Object()));
- Assertions.assertFalse(zero.equals(Double.valueOf(0)));
+ Assertions.assertEquals(zero, zero);
+ Assertions.assertNotEquals(null, zero);
+ Assertions.assertNotEquals(zero, new Object());
+ Assertions.assertNotEquals(zero, Double.valueOf(0), 0.0);
Review comment:
This is creating a compile failure in the CI build. Remove the `0.0` tolerance argument. This is a test for Object.equals and not floating-point equivalence of doubles.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-numbers] aherbert commented on a change in pull request #98: Replace assert by simpler but equivalent calls.
Posted by GitBox <gi...@apache.org>.
aherbert commented on a change in pull request #98:
URL: https://github.com/apache/commons-numbers/pull/98#discussion_r654886432
##########
File path: commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/FractionTest.java
##########
@@ -463,10 +463,10 @@ void testSubtract() {
@Test
void testEqualsAndHashCode() {
final Fraction zero = Fraction.of(0, 1);
- Assertions.assertTrue(zero.equals(zero));
- Assertions.assertFalse(zero.equals(null));
- Assertions.assertFalse(zero.equals(new Object()));
- Assertions.assertFalse(zero.equals(Double.valueOf(0)));
+ Assertions.assertEquals(zero, zero);
+ Assertions.assertNotEquals(null, zero);
Review comment:
Reverse argument order, null must be second.
##########
File path: commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/BigFractionTest.java
##########
@@ -625,10 +625,10 @@ void testSubtract() {
@Test
void testEqualsAndHashCode() {
final BigFraction zero = BigFraction.of(0, 1);
- Assertions.assertTrue(zero.equals(zero));
- Assertions.assertFalse(zero.equals(null));
- Assertions.assertFalse(zero.equals(new Object()));
- Assertions.assertFalse(zero.equals(Double.valueOf(0)));
+ Assertions.assertEquals(zero, zero);
+ Assertions.assertNotEquals(null, zero);
Review comment:
Reverse argument order, null must be second.
##########
File path: commons-numbers-quaternion/src/test/java/org/apache/commons/numbers/quaternion/QuaternionTest.java
##########
@@ -378,7 +378,7 @@ final void testObjectEquals() {
final Quaternion q3 = Quaternion.of(one, Math.nextUp(one), one, one);
Assertions.assertNotEquals(q3, q1);
- Assertions.assertFalse(q3.equals("bar"));
+ Assertions.assertNotEquals("bar", q3);
Review comment:
Reverse argument order
##########
File path: commons-numbers-angle/src/test/java/org/apache/commons/numbers/angle/AngleTest.java
##########
@@ -60,19 +60,19 @@ void testEquals() {
Assertions.assertEquals(Angle.Turn.of(value), asTurn);
Assertions.assertEquals(asTurn, asTurn);
Assertions.assertNotEquals(asTurn, Angle.Turn.of(nextValue));
- Assertions.assertFalse(asTurn.equals(null));
+ Assertions.assertNotEquals(null, asTurn);
final Angle.Rad asRad = Angle.Rad.of(value);
Assertions.assertEquals(Angle.Rad.of(value), asRad);
Assertions.assertEquals(asRad, asRad);
Assertions.assertNotEquals(asRad, Angle.Rad.of(nextValue));
- Assertions.assertFalse(asRad.equals(null));
+ Assertions.assertNotEquals(null, asRad);
Review comment:
Reverse argument order, null must be second.
##########
File path: commons-numbers-angle/src/test/java/org/apache/commons/numbers/angle/AngleTest.java
##########
@@ -60,19 +60,19 @@ void testEquals() {
Assertions.assertEquals(Angle.Turn.of(value), asTurn);
Review comment:
Reverse these arguments for consistency
##########
File path: commons-numbers-angle/src/test/java/org/apache/commons/numbers/angle/AngleTest.java
##########
@@ -60,19 +60,19 @@ void testEquals() {
Assertions.assertEquals(Angle.Turn.of(value), asTurn);
Assertions.assertEquals(asTurn, asTurn);
Assertions.assertNotEquals(asTurn, Angle.Turn.of(nextValue));
- Assertions.assertFalse(asTurn.equals(null));
+ Assertions.assertNotEquals(null, asTurn);
final Angle.Rad asRad = Angle.Rad.of(value);
Assertions.assertEquals(Angle.Rad.of(value), asRad);
Assertions.assertEquals(asRad, asRad);
Assertions.assertNotEquals(asRad, Angle.Rad.of(nextValue));
- Assertions.assertFalse(asRad.equals(null));
+ Assertions.assertNotEquals(null, asRad);
final Angle.Deg asDeg = Angle.Deg.of(value);
Assertions.assertEquals(Angle.Deg.of(value), asDeg);
Review comment:
Reverse these arguments for consistency
##########
File path: commons-numbers-angle/src/test/java/org/apache/commons/numbers/angle/AngleTest.java
##########
@@ -60,19 +60,19 @@ void testEquals() {
Assertions.assertEquals(Angle.Turn.of(value), asTurn);
Assertions.assertEquals(asTurn, asTurn);
Assertions.assertNotEquals(asTurn, Angle.Turn.of(nextValue));
- Assertions.assertFalse(asTurn.equals(null));
+ Assertions.assertNotEquals(null, asTurn);
final Angle.Rad asRad = Angle.Rad.of(value);
Assertions.assertEquals(Angle.Rad.of(value), asRad);
Review comment:
Reverse these arguments for consistency
##########
File path: commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/ComplexTest.java
##########
@@ -1722,13 +1722,13 @@ void testNthRootInf() {
@Test
void testEqualsWithNull() {
final Complex x = Complex.ofCartesian(3.0, 4.0);
- Assertions.assertFalse(x.equals(null));
+ Assertions.assertNotEquals(null, x);
Review comment:
Reverse argument order, null must be second.
##########
File path: commons-numbers-field/src/test/java/org/apache/commons/numbers/field/FP64Test.java
##########
@@ -43,14 +43,14 @@ void testEquals() {
final FP64 b = FP64.of(4.56);
// Same instance
- Assertions.assertTrue(a.equals(a));
+ Assertions.assertEquals(a, a);
// Same value
- Assertions.assertTrue(a.equals(FP64.of(a.doubleValue())));
+ Assertions.assertEquals(a, FP64.of(a.doubleValue()));
// Different value
- Assertions.assertFalse(a.equals(b));
+ Assertions.assertNotEquals(a, b);
// Different object
- Assertions.assertFalse(a.equals(new Object()));
- Assertions.assertFalse(a.equals(null));
+ Assertions.assertNotEquals(a, new Object());
+ Assertions.assertNotEquals(null, a);
Review comment:
Reverse argument order, null must be second.
##########
File path: commons-numbers-angle/src/test/java/org/apache/commons/numbers/angle/AngleTest.java
##########
@@ -60,19 +60,19 @@ void testEquals() {
Assertions.assertEquals(Angle.Turn.of(value), asTurn);
Assertions.assertEquals(asTurn, asTurn);
Assertions.assertNotEquals(asTurn, Angle.Turn.of(nextValue));
- Assertions.assertFalse(asTurn.equals(null));
+ Assertions.assertNotEquals(null, asTurn);
Review comment:
Reverse argument order, null must be second.
##########
File path: commons-numbers-angle/src/test/java/org/apache/commons/numbers/angle/AngleTest.java
##########
@@ -60,19 +60,19 @@ void testEquals() {
Assertions.assertEquals(Angle.Turn.of(value), asTurn);
Assertions.assertEquals(asTurn, asTurn);
Assertions.assertNotEquals(asTurn, Angle.Turn.of(nextValue));
- Assertions.assertFalse(asTurn.equals(null));
+ Assertions.assertNotEquals(null, asTurn);
final Angle.Rad asRad = Angle.Rad.of(value);
Assertions.assertEquals(Angle.Rad.of(value), asRad);
Assertions.assertEquals(asRad, asRad);
Assertions.assertNotEquals(asRad, Angle.Rad.of(nextValue));
- Assertions.assertFalse(asRad.equals(null));
+ Assertions.assertNotEquals(null, asRad);
final Angle.Deg asDeg = Angle.Deg.of(value);
Assertions.assertEquals(Angle.Deg.of(value), asDeg);
Assertions.assertEquals(asDeg, asDeg);
Assertions.assertNotEquals(asDeg, Angle.Deg.of(nextValue));
- Assertions.assertFalse(asDeg.equals(null));
+ Assertions.assertNotEquals(null, asDeg);
Review comment:
Reverse argument order, null must be second.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-numbers] arturobernalg commented on pull request #98: Replace assert by simpler but equivalent calls.
Posted by GitBox <gi...@apache.org>.
arturobernalg commented on pull request #98:
URL: https://github.com/apache/commons-numbers/pull/98#issuecomment-864512185
> When using `assertNotEquals` the `null` argument must be second so that the assertion calls the equals method on the class being tested with the `null` as the argument.
>
> In general the class where the equals method is being tested should always be first.
hi @aherbert
Changed
TY
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-numbers] coveralls commented on pull request #98: Replace assert by simpler but equivalent calls.
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #98:
URL: https://github.com/apache/commons-numbers/pull/98#issuecomment-864560200
[](https://coveralls.io/builds/40724853)
Coverage remained the same at 99.657% when pulling **c53efde09f2d18bf7a8f13e3246994aab6311001 on arturobernalg:feature/simplify_assert** into **93c45e9dcde7f283c620508daacaf36383925596 on apache:master**.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-numbers] asfgit merged pull request #98: Replace assert by simpler but equivalent calls.
Posted by GitBox <gi...@apache.org>.
asfgit merged pull request #98:
URL: https://github.com/apache/commons-numbers/pull/98
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-numbers] coveralls edited a comment on pull request #98: Replace assert by simpler but equivalent calls.
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #98:
URL: https://github.com/apache/commons-numbers/pull/98#issuecomment-864560200
[](https://coveralls.io/builds/40726504)
Coverage remained the same at 99.657% when pulling **c53efde09f2d18bf7a8f13e3246994aab6311001 on arturobernalg:feature/simplify_assert** into **93c45e9dcde7f283c620508daacaf36383925596 on apache:master**.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-numbers] aherbert commented on a change in pull request #98: Replace assert by simpler but equivalent calls.
Posted by GitBox <gi...@apache.org>.
aherbert commented on a change in pull request #98:
URL: https://github.com/apache/commons-numbers/pull/98#discussion_r664467158
##########
File path: commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/BigFractionTest.java
##########
@@ -625,10 +625,10 @@ void testSubtract() {
@Test
void testEqualsAndHashCode() {
final BigFraction zero = BigFraction.of(0, 1);
- Assertions.assertTrue(zero.equals(zero));
- Assertions.assertFalse(zero.equals(null));
- Assertions.assertFalse(zero.equals(new Object()));
- Assertions.assertFalse(zero.equals(Double.valueOf(0)));
+ Assertions.assertEquals(zero, zero);
+ Assertions.assertNotEquals(null, zero);
Review comment:
Ping. This once still needs to be reversed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-numbers] aherbert commented on a change in pull request #98: Replace assert by simpler but equivalent calls.
Posted by GitBox <gi...@apache.org>.
aherbert commented on a change in pull request #98:
URL: https://github.com/apache/commons-numbers/pull/98#discussion_r654896139
##########
File path: commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/FractionTest.java
##########
@@ -463,10 +463,10 @@ void testSubtract() {
@Test
void testEqualsAndHashCode() {
final Fraction zero = Fraction.of(0, 1);
- Assertions.assertTrue(zero.equals(zero));
- Assertions.assertFalse(zero.equals(null));
- Assertions.assertFalse(zero.equals(new Object()));
- Assertions.assertFalse(zero.equals(Double.valueOf(0)));
+ Assertions.assertEquals(zero, zero);
+ Assertions.assertNotEquals(zero, null);
+ Assertions.assertNotEquals(zero, new Object());
+ Assertions.assertNotEquals(zero, (double) 0);
Review comment:
Do not cast 0 to `double`. This requires an object. Use `Double.valueOf(0)`.
##########
File path: commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/BigFractionTest.java
##########
@@ -625,10 +625,10 @@ void testSubtract() {
@Test
void testEqualsAndHashCode() {
final BigFraction zero = BigFraction.of(0, 1);
- Assertions.assertTrue(zero.equals(zero));
- Assertions.assertFalse(zero.equals(null));
- Assertions.assertFalse(zero.equals(new Object()));
- Assertions.assertFalse(zero.equals(Double.valueOf(0)));
+ Assertions.assertEquals(zero, zero);
+ Assertions.assertNotEquals(null, zero);
+ Assertions.assertNotEquals(zero, new Object());
+ Assertions.assertNotEquals(zero, Double.valueOf(0), 0.0);
Review comment:
Remove the delta tolerance here. And two lines above please reverse the order of null and zero.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org