Issue 33284: Increase test coverage for numbers.py (original) (raw)

Barry, thank you for your first submission.

You propose to test numbers.Complex.bool

def __bool__(self):
    """True if self != 0. Called for bool(self)."""
    return self != 0

by adding the following to Lib/test/test_abstract_numbers.

```
   self.assertFalse(bool(complex(0,0)))
```
```
   self.assertTrue(bool(complex(1,2)))
```

I believe that this particular addition should be rejected. It is a concrete test of the builtin complex that partially duplicates the following in test_complex.

def test_boolcontext(self):
    for i in range(100):
        self.assertTrue(complex(random() + 1e-6, random() + 1e-6))
    self.assertTrue(not complex(0.0, 0.0))

Looking the tests of collections.abc in test_collections, I believe a proper test should define a subclass of Complex (in Python), with at least init and eq methods and test instances of that.

If I were to review a patch, I would like to see a more extensive addition, one that imports test_collections.ABCTestCase (or copies and adapts the same) and uses it to test a much fuller implementation of Complex. As it is, none of the numbers abc class methods are tested.

Raymond, were you involved with the abc tests? Either way, what do you think?

Hey,

I updated my pull request based in your advice. Could you review it please?

Best,

Barry

On Sat, 21 Apr 2018, 03:20 Terry J. Reedy, <report@bugs.python.org> wrote:

Terry J. Reedy <tjreedy@udel.edu> added the comment:

Barry, thank you for your first submission.

You propose to test numbers.Complex.bool
def __bool__(self):
    """True if self != 0. Called for bool(self)."""
    return self != 0
by adding the following to Lib/test/test_abstract_numbers.
   self.assertFalse(bool(complex(0,0)))
   self.assertTrue(bool(complex(1,2)))
I believe that this particular addition should be rejected. It is a concrete test of the builtin complex that partially duplicates the following in test_complex.
def test_boolcontext(self):
    for i in range(100):
        self.assertTrue(complex(random() + 1e-6, random() + 1e-6))
    self.assertTrue(not complex(0.0, 0.0))
Looking the tests of collections.abc in test_collections, I believe a proper test should define a subclass of Complex (in Python), with at least init and eq methods and test instances of that.

If I were to review a patch, I would like to see a more extensive addition, one that imports test_collections.ABCTestCase (or copies and adapts the same) and uses it to test a much fuller implementation of Complex. As it is, none of the numbers abc class methods are tested.

Raymond, were you involved with the abc tests? Either way, what do you think?

nosy: +rhettinger, terry.reedy

Python tracker <report@bugs.python.org> <https://bugs.python.org/issue33284>