Skip to content

bpo-40286: Use random.randbytes() in tests #19575

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
Merged
merged 2 commits into from
Apr 17, 2020
Merged

bpo-40286: Use random.randbytes() in tests #19575

merged 2 commits into from
Apr 17, 2020

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Apr 17, 2020 •
edited by bedevere-bot
Loading

@vstinner vstinner requested a review from ethanfurman as a code owner April 17, 2020 20:11
@bedevere-bot bedevere-bot added tests Tests in the Lib/test dir awaiting core review labels Apr 17, 2020
@vstinner
Copy link
Member Author

cc @serhiy-storchaka

serhiy-storchaka
serhiy-storchaka reviewed Apr 17, 2020
View reviewed changes
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh! I just have wrote almost identical patch!

There is yet one place in test_zlib:

@@ -829,13 +828,9 @@ def genblock(seed, length, step=1024, generator=random):
     """length-byte stream of random data from a seed (in step-byte blocks)."""
     if seed is not None:
         generator.seed(seed)
-    randint = generator.randint
-    if length < step or step < 2:
-        step = length
-    blocks = bytes()
-    for i in range(0, length, step):
-        blocks += bytes(randint(0, 255) for x in range(step))
-    return blocks
+    if length > step > 1:
+        length += -length % step
+    return generator.randbytes(length)

Lib/test/test_tarfile.py
@@ -386,7 +386,7 @@ def test_null_tarfile(self):
def test_ignore_zeros(self):
# Test TarFile's ignore_zeros option.
# generate 512 pseudorandom bytes
data = Random(0).getrandbits(512*8).to_bytes(512, 'big')
data = Random(0).randbytes(512)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is better to use the global randbytes().

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It uses a seed of 0. I would prefer to use a temporary instance, rather than modifying random._inst.

@vstinner
Copy link
Member Author

Huh! I just have wrote almost identical patch!

He he, great minds think alike ;-)

@vstinner
Copy link
Member Author

There is yet one place in test_zlib: (...)

Oh right, I also updated this code.

@vstinner vstinner merged commit 87502dd into python:master Apr 17, 2020
@vstinner vstinner deleted the test_randbytes branch April 17, 2020 20:54
CuriousLearner added a commit to CuriousLearner/cpython that referenced this pull request May 29, 2020
@CuriousLearner
Merge branch 'master' into bpo-26543
e4d193d 
* master: (1985 commits)
  bpo-40179: Fix translation of #elif in Argument Clinic (pythonGH-19364)
  bpo-35967: Skip test with `uname -p` on Android (pythonGH-19577)
  bpo-40257: Improve help for the typing module (pythonGH-19546)
  Fix two typos in multiprocessing (pythonGH-19571)
  bpo-40286: Use random.randbytes() in tests (pythonGH-19575)
  bpo-40286: Makes simpler the relation between randbytes() and getrandbits() (pythonGH-19574)
  bpo-39894: Route calls from pathlib.Path.samefile() to os.stat() via the path accessor (pythonGH-18836)
  bpo-39897: Remove needless `Path(self.parent)` call, which makes `is_mount()` misbehave in `Path` subclasses. (pythonGH-18839)
  bpo-40282: Allow random.getrandbits(0) (pythonGH-19539)
  bpo-40302: UTF-32 encoder SWAB4() macro use a|b rather than a+b (pythonGH-19572)
  bpo-40302: Replace PY_INT64_T with int64_t (pythonGH-19573)
  bpo-40286: Add randbytes() method to random.Random (pythonGH-19527)
  bpo-39901: Move `pathlib.Path.owner()` and `group()` implementations into the path accessor. (pythonGH-18844)
  bpo-40300: Allow empty logging.Formatter.default_msec_format. (pythonGH-19551)
  bpo-40302: Add pycore_byteswap.h header file (pythonGH-19552)
  bpo-40287: Fix SpooledTemporaryFile.seek() return value (pythonGH-19540)
  Minor modernization and readability improvement to the tokenizer example (pythonGH-19558)
  bpo-40294: Fix _asyncio when module is loaded/unloaded multiple times (pythonGH-19542)
  Fix parameter names in assertIn() docs (pythonGH-18829)
  bpo-39793: use the same domain on make_msgid tests (python#18698)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes

@ethanfurman ethanfurman Awaiting requested review from ethanfurman

Assignees
No one assigned
Labels
skip news tests Tests in the Lib/test dir
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@vstinner @serhiy-storchaka @the-knights-who-say-ni @bedevere-bot