Increase test coverage of the random module

@tim-one

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']

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.

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.

What is wrong with adding more tests?

Hi Raymond and Serhiy,

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

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.

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!

Navigation Menu

Repository navigation

Increase test coverage of the random module #84227

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Increase test coverage of the random module #84227

Description

Activity

serhiy-storchaka commented on Mar 23, 2020

rhettinger commented on Mar 23, 2020

serhiy-storchaka commented on Mar 23, 2020

willingc commented on Mar 30, 2020

serhiy-storchaka commented on Mar 30, 2020

willingc commented on Mar 30, 2020

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions