PYTHON-5392 Better test assertions for comparisons #2350
Conversation
test/test_bson.py
Outdated
| self.assertLessEqual(MinKey(), 1) | ||
| self.assertLessEqual(MinKey(), MinKey()) | ||
| self.assertLessEqual(MinKey(), None) | ||
| self.assertLessEqual(MinKey(), 1) |
There was a problem hiding this comment.
We want to preserve the operation that each test is verifying. This test should be an assertGreater, not an assertLessEqual. The same holds for any other tests that have had their operations changed to preserve the original test case. The value should change so the test accurately reflects what operation is being verified.
There was a problem hiding this comment.
to clarify, instead of being assertFalse(a > b) it should beassertGreater(b, a), correct?
There was a problem hiding this comment.
Hmm, in this case MinKey and MaxKey define their ge and such methods to be different. @ShaneHarvey can you offer the reasoning behind these tests?
test/test_bson.py
Outdated
| self.assertLess(MinKey(), 1) | ||
| self.assertGreaterEqual(MinKey(), MinKey()) | ||
| self.assertNotEqual(MinKey(), 1) | ||
| self.assertNotEqual(MinKey(), 1) |
There was a problem hiding this comment.
We don't need duplicate assertions: the case above checks !=, and the case below checks ==.
test/test_bson.py
Outdated
| @@ -1045,6 +1045,7 @@ def test_exception_wrapping(self): | |||
|
|
|||
| def test_minkey_maxkey_comparison(self): | |||
| # MinKey's <, <=, >, >=, !=, and ==. | |||
| # These tests should be kept as assertTrue as opposed to using unittest's build in comparison assertions. | |||
There was a problem hiding this comment.
Can you add the reason why?
Co-authored-by: Noah Stapp <[email protected]>
Another subtask PR for PYTHON-5262, this one is for comparisons:
self.assertTrue(... > ...)->self.assertGreater(..., ...)self.assertTrue(... >= ...)->self.assertGreaterEqual(..., ...)self.assertTrue(... < ...)->self.assertLess(..., ...)self.assertTrue(... <= ...)->self.assertLessEqual(..., ...)self.assertFalse(... >= ...)->self.assertLess(..., ...)self.assertFalse(... > ...)->self.assertLessEqual(..., ...)self.assertFalse(... <= ...)->self.assertGreater(..., ...)self.assertFalse(... < ...)->self.assertGreaterEqual(..., ...)self.assertTrue(... == ...)->self.assertEqual(..., ...)self.assertTrue(... != ...)->self.assertNotEqual(..., ...)self.assertFalse(... == ...)->self.assertNotEqual(..., ...)