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.

Sign up for GitHub

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

Jump to bottom

Increase test coverage of the random module #84227

Closed
serhiy-storchaka opened this issue Mar 23, 2020 · 6 comments
Closed

Increase test coverage of the random module #84227

serhiy-storchaka opened this issue Mar 23, 2020 · 6 comments
Labels
3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes docs Documentation in the Doc dir tests Tests in the Lib/test dir type-feature A feature request or enhancement

Comments

@serhiy-storchaka
Copy link
Member

BPO 40046
Nosy @tim-one, @rhettinger, @mdickinson, @serhiy-storchaka, @willingc
PRs
  • bpo-40046: Increase test coverage of the random module #19114
    		•

    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 = <Date 2020-03-23.15:42:53.971>
created_at = <Date 2020-03-23.09:01:11.482>
labels = ['3.7', '3.8', '3.9', 'type-feature', 'tests', 'docs']
title = 'Increase test coverage of the random module'
updated_at = <Date 2020-03-30.20:39:49.887>
user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2020-03-30.20:39:49.887>
actor = 'willingc'
assignee = 'docs@python'
closed = True
closed_date = <Date 2020-03-23.15:42:53.971>
closer = 'rhettinger'
components = ['Documentation', 'Tests']
creation = <Date 2020-03-23.09:01:11.482>
creator = 'serhiy.storchaka'
dependencies = []
files = []
hgrepos = []
issue_num = 40046
keywords = ['patch']
message_count = 6.0
messages = ['364840', '364858', '364859', '365336', '365340', '365348']
nosy_count = 6.0
nosy_names = ['tim.peters', 'rhettinger', 'mark.dickinson', 'docs@python', 'serhiy.storchaka', 'willingc']
pr_nums = ['19114']
priority = 'normal'
resolution = 'rejected'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue40046'
versions = ['Python 3.7', 'Python 3.8', 'Python 3.9']
    @serhiy-storchaka
    Copy link
    Member Author

    The propose test adds several tests for random module. Mainly tests for integer, sequence and iterable arguments.

    It also documents that randrange() accepts non-integers.

    @serhiy-storchaka serhiy-storchaka added 3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes labels Mar 23, 2020
    @serhiy-storchaka serhiy-storchaka added docs Documentation in the Doc dir tests Tests in the Lib/test dir 3.7 (EOL) end of life type-feature A feature request or enhancement 3.8 only security fixes 3.9 only security fixes labels Mar 23, 2020
    @serhiy-storchaka serhiy-storchaka added docs Documentation in the Doc dir tests Tests in the Lib/test dir type-feature A feature request or enhancement labels Mar 23, 2020
    @rhettinger
    Copy link
    Contributor

    I'm against this doc change. While factually true, it is irrelevant and distracting to most users. We do not need to test and document every implementation detail, nor should we.

    @serhiy-storchaka
    Copy link
    Member Author

    What is wrong with adding more tests?

    @willingc
    Copy link
    Contributor

    Hi Raymond and Serhiy,

    Serhiy, Was there a specific use case or regression that you felt the tests would be helpful?

    @serhiy-storchaka
    Copy link
    Member Author

    Yes, of course. The PR adds new tests for different types of arguments which currently are accepted (so it would be a regression if they will no accepted in new releases or in alternate implementations) and for types which currently are not accepted for reasons, so unexpected accepting may signal about using inefficient or incorrect algorithm.

    For example, choice() works not only with lists, but with other sequences, so the implementation which works only with lists (for example the C implementation which uses the concrete C API) will fail with new tests. If we fix this hypothetical implementation by converting the argument to list (which would be inefficient for large collections), other new tests (which check that sets and iterators are not accepted) will fail.

    @willingc
    Copy link
    Contributor

    Thanks Serhiy for the explanation.

    I agree with Raymond about the doc change.

    Raymond, These tests, after reading Serihy's response, make sense to me. Let's revisit and reconsider the PR on the tests without the doc change.

    Thanks!

    @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
    Assignees
    No one assigned
    Labels
    3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes docs Documentation in the Doc dir tests Tests in the Lib/test dir type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @rhettinger @willingc @serhiy-storchaka