classification
Title: Increase test coverage for numbers.py
Type: enhancement Stage: needs patch
Components: Tests Versions: Python 3.8
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Barry Devlin, rhettinger, terry.reedy
Priority: normal Keywords: patch

Created on 2018-04-15 18:14 by Barry Devlin, last changed 2018-04-26 10:12 by Barry Devlin.

Pull Requests
URL Status Linked Edit
PR 6480 open python-dev, 2018-04-15 18:19
Messages (3)
msg315337 - (view) Author: Barry Devlin (Barry Devlin) * Date: 2018-04-15 18:14
The __bool__ method in the complex class in numbers is not tested.
msg315547 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2018-04-21 02:20
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?
msg315784 - (view) Author: Barry Devlin (Barry Devlin) * Date: 2018-04-26 10:12
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>
> _______________________________________
>
History
Date User Action Args
2018-04-26 10:12:27Barry Devlinsetmessages: + msg315784
2018-04-21 02:21:11terry.reedysettype: enhancement
stage: patch review -> needs patch
2018-04-21 02:20:45terry.reedysetnosy: + rhettinger, terry.reedy
messages: + msg315547
2018-04-15 18:19:57python-devsetkeywords: + patch
stage: patch review
pull_requests: + pull_request6180
2018-04-15 18:14:55Barry Devlincreate