Simplify assertions with equivalent but more simple.#196
Simplify assertions with equivalent but more simple.#196aherbert merged 1 commit intoapache:masterfrom arturobernalg:feature/simplify_assert
Conversation
|
Please fix checkstyle so the Travis CI job will run with the changes. |
aherbert
left a comment
aherbert
left a comment
There was a problem hiding this comment.
All looks good except for assertions testing the equals method on an object. These must put the object where the equals method will be invoked as the first argument to the assertion. In particular this allows testing o.equals(null) on the object being tested (since null.equals(o) is nonsense.
| public void testEquals() { | ||
| Pair<Integer, Double> p1 = new Pair<>(null, null); | ||
| Assert.assertFalse(p1.equals(null)); | ||
| Assert.assertNotEquals(null, p1); |
There was a problem hiding this comment.
p1 should be the first argument to allow the assert method to invoke p1.equals(null)
| Assert.assertFalse(m.equals(null)); | ||
| Assert.assertFalse(m.equals(mt)); | ||
| Assert.assertFalse(m.equals(new Array2DRowRealMatrix(bigSingular))); | ||
| Assert.assertNotEquals(null, m); |
There was a problem hiding this comment.
m should be the first argument
| Assert.assertFalse(m.equals(null)); | ||
| Assert.assertFalse(m.equals(mt)); | ||
| Assert.assertFalse(m.equals(new BlockFieldMatrix<>(bigSingular))); | ||
| Assert.assertNotEquals(null, m); |
There was a problem hiding this comment.
m should be the first argument
| Assert.assertFalse(m.equals(null)); | ||
| Assert.assertFalse(m.equals(mt)); | ||
| Assert.assertFalse(m.equals(new BlockRealMatrix(bigSingular))); | ||
| Assert.assertNotEquals(null, m); |
There was a problem hiding this comment.
m should be the first argument
| Assert.assertFalse(m.equals(null)); | ||
| Assert.assertFalse(m.equals(mt)); | ||
| Assert.assertFalse(m.equals(new Array2DRowFieldMatrix<>(bigSingular))); | ||
| Assert.assertNotEquals(null, m); |
There was a problem hiding this comment.
m should be the first argument
| Assert.assertFalse(m.equals(null)); | ||
| Assert.assertFalse(m.equals(mt)); | ||
| Assert.assertFalse(m.equals(createSparseMatrix(bigSingular))); | ||
| Assert.assertNotEquals(null, m); |
There was a problem hiding this comment.
m should be the first argument
| final double[] p1 = new double[] { 1 }; | ||
| final PointValuePair pv1 = new PointValuePair(p1, 2); | ||
| Assert.assertFalse(pv1.equals(null)); | ||
| Assert.assertNotEquals(null, pv1); |
There was a problem hiding this comment.
pv1 should be the first argument
| Assert.assertFalse(markers.equals(null)); | ||
| Assert.assertFalse(markers.equals("")); | ||
| Assert.assertEquals(markers, markers); | ||
| Assert.assertNotEquals(null, markers); |
There was a problem hiding this comment.
markers should be the first argument
| Assert.assertEquals(ptile, ptile); | ||
| Assert.assertFalse(ptile.equals(null)); | ||
| Assert.assertFalse(ptile.equals("")); | ||
| Assert.assertNotEquals(null, ptile); |
There was a problem hiding this comment.
ptile should be the first argument
| Assert.assertFalse(ptile.equals(null)); | ||
| Assert.assertFalse(ptile.equals("")); | ||
| Assert.assertNotEquals(null, ptile); | ||
| Assert.assertNotEquals("", ptile); |
There was a problem hiding this comment.
ptile should be the first argument
|
HI @aherbert |
|
There are still a few assertions with null as the first argument. I think you updated 5 and there are 7 remaining. |
3 participants
No description provided.