Skip to content

Increase test coverage for numbers.py #77465

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
londinburgh mannequin opened this issue Apr 15, 2018 · 3 comments
Closed

Increase test coverage for numbers.py #77465

londinburgh mannequin opened this issue Apr 15, 2018 · 3 comments
Labels
tests Tests in the Lib/test dir type-feature A feature request or enhancement

Comments

@londinburgh
Copy link
Mannequin

londinburgh mannequin commented Apr 15, 2018

BPO 33284
Nosy @rhettinger, @terryjreedy, @londinburgh
PRs
  • bpo-33284 Add to a unit test to improve coverage for numbers.py #6480
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = None
    created_at = <Date 2018-04-15.18:14:55.580>
    labels = ['3.8', 'type-feature', 'tests']
    title = 'Increase test coverage for numbers.py'
    updated_at = <Date 2018-04-26.10:12:27.449>
    user = 'https://github.com/londinburgh'

    bugs.python.org fields:

    activity = <Date 2018-04-26.10:12:27.449>
    actor = 'Barry Devlin'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Tests']
    creation = <Date 2018-04-15.18:14:55.580>
    creator = 'Barry Devlin'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 33284
    keywords = ['patch']
    message_count = 3.0
    messages = ['315337', '315547', '315784']
    nosy_count = 3.0
    nosy_names = ['rhettinger', 'terry.reedy', 'Barry Devlin']
    pr_nums = ['6480']
    priority = 'normal'
    resolution = None
    stage = 'needs patch'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue33284'
    versions = ['Python 3.8']

    Linked PRs

    @londinburgh
    Copy link
    Mannequin Author

    londinburgh mannequin commented Apr 15, 2018

    The __bool__ method in the complex class in numbers is not tested.

    @londinburgh londinburgh mannequin added 3.8 (EOL) end of life tests Tests in the Lib/test dir labels Apr 15, 2018
    @terryjreedy
    Copy link
    Member

    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?

    @terryjreedy terryjreedy added the type-feature A feature request or enhancement label Apr 21, 2018
    @londinburgh
    Copy link
    Mannequin Author

    londinburgh mannequin commented Apr 26, 2018

    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, <[email protected]> wrote:

    Terry J. Reedy <[email protected]> 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 <[email protected]>
    <https://bugs.python.org/issue33284\>


    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @erlend-aasland erlend-aasland removed the 3.8 (EOL) end of life label Jan 6, 2024
    serhiy-storchaka added a commit that referenced this issue Jan 25, 2024
    miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jan 25, 2024
    …GH-111738)
    
    (cherry picked from commit e721adf)
    
    Co-authored-by: AN Long <[email protected]>
    Co-authored-by: Serhiy Storchaka <[email protected]>
    miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jan 25, 2024
    …GH-111738)
    
    (cherry picked from commit e721adf)
    
    Co-authored-by: AN Long <[email protected]>
    Co-authored-by: Serhiy Storchaka <[email protected]>
    serhiy-storchaka added a commit that referenced this issue Jan 25, 2024
    …1738) (GH-114557)
    
    (cherry picked from commit e721adf)
    
    Co-authored-by: AN Long <[email protected]>
    Co-authored-by: Serhiy Storchaka <[email protected]>
    serhiy-storchaka added a commit that referenced this issue Jan 25, 2024
    …1738) (GH-114556)
    
    (cherry picked from commit e721adf)
    
    Co-authored-by: AN Long <[email protected]>
    Co-authored-by: Serhiy Storchaka <[email protected]>
    aisk added a commit to aisk/cpython that referenced this issue Feb 11, 2024
    Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    tests Tests in the Lib/test dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants