Skip to content
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

Use specific asserts in collections tests #64749

Closed
serhiy-storchaka opened this issue Feb 7, 2014 · 4 comments
Closed

Use specific asserts in collections tests #64749

serhiy-storchaka opened this issue Feb 7, 2014 · 4 comments
Assignees
Labels
easy tests Tests in the Lib/test dir type-feature A feature request or enhancement

Comments

@serhiy-storchaka
Copy link
Member

BPO 20550
Nosy @rhettinger, @serhiy-storchaka
PRs
  • bpo-20550: Use specific asserts in collections tests #789
  • Files
  • collections_tests_asserts.patch
  • 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 = 'https://github.com/rhettinger'
    closed_at = <Date 2014-04-03.05:33:30.525>
    created_at = <Date 2014-02-07.20:26:56.806>
    labels = ['easy', 'type-feature', 'tests']
    title = 'Use specific asserts in collections tests'
    updated_at = <Date 2017-03-23.17:07:11.930>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2017-03-23.17:07:11.930>
    actor = 'serhiy.storchaka'
    assignee = 'rhettinger'
    closed = True
    closed_date = <Date 2014-04-03.05:33:30.525>
    closer = 'rhettinger'
    components = ['Tests']
    creation = <Date 2014-02-07.20:26:56.806>
    creator = 'serhiy.storchaka'
    dependencies = []
    files = ['34076']
    hgrepos = []
    issue_num = 20550
    keywords = ['patch', 'easy']
    message_count = 4.0
    messages = ['210547', '211195', '211210', '215423']
    nosy_count = 2.0
    nosy_names = ['rhettinger', 'serhiy.storchaka']
    pr_nums = ['789']
    priority = 'normal'
    resolution = 'rejected'
    stage = 'patch review'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue20550'
    versions = ['Python 2.7', 'Python 3.3', 'Python 3.4']

    @serhiy-storchaka
    Copy link
    Member Author

    The proposed patch makes collections tests use more specific asserts. This will provide more useful failure report.

    @serhiy-storchaka serhiy-storchaka added tests Tests in the Lib/test dir easy type-feature A feature request or enhancement labels Feb 7, 2014
    @rhettinger
    Copy link
    Contributor

    The patch is missing.

    @rhettinger rhettinger self-assigned this Feb 14, 2014
    @serhiy-storchaka
    Copy link
    Member Author

    Oh, sorry. Here is a patch.

    @rhettinger
    Copy link
    Contributor

    Sorry, I don't think this makes the tests any better as all. It is code churn for no benefit.

    In our own code, the "more specific tests" risk hiding important details behind the abstraction (losing knowledge of what is actually tested). For example, I don't like that assertIn actually does a "not in" test or that assertIsNot runs "is". In those two cases, it doesn't make a difference but does hint at the loss of knowledge.

    Also, changing tests carries a higher set of risks than changing other code because there are no tests-for-the-tests. This means that it would easy to accidentally break a test from testing what it is supposed to and not know about it.

    The "has a nicer error message" is a vacuous promise. The new output:

        Traceback (most recent call last):
          File "/Users/raymond/Documents/tmp14.py", line 10, in test_one
            self.assertIs(a, b)
        AssertionError: a is not b

    Isn't any better than the current output:

        Traceback (most recent call last):
          File "/Users/raymond/Documents/tmp14.py", line 13, in test_two
            self.assertTrue(a is b)
        AssertionError: False is not true

    Both of them show a failing "is" test and the first one mysteriously outputs "is not" which is technically a different operator (albeit, unimportantly so).

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    easy tests Tests in the Lib/test dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants