GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-107862: Add roundtrip `hypothesis` tests to `test_binascii` by sobolevn · Pull Request #107863 · python/cpython

Conversation

@sobolevn

@sobolevn sobolevn commented Aug 11, 2023 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member
  • Issue: [Meta] Research: what can we test with Hypothesis? #107862

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@sobolevn
pythongh-107862: Add roundtrip hypothesis tests to test_binascii
4662195
@sobolevn
sobolevn requested review from pganssle and vstinner August 11, 2023 10:53
@bedevere-bot bedevere-bot added awaiting review tests Tests in the Lib/test dir labels Aug 11, 2023
@bedevere-bot bedevere-bot mentioned this pull request Aug 11, 2023
[Meta] Research: what can we test with Hypothesis? #107862
Open
@sobolevn sobolevn added the skip news label Aug 11, 2023
@ambv

ambv commented Aug 11, 2023

Copy link
Copy Markdown
Contributor

Closing and re-opening to retrigger CLA checks. Sorry for the noise.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ambv ambv closed this Aug 11, 2023
@ambv ambv reopened this Aug 11, 2023
hauntsaninja
hauntsaninja approved these changes Sep 1, 2023
View reviewed changes

@hauntsaninja hauntsaninja left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thanks for working on this!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/test_binascii.py
@hypothesis.given(binary=hypothesis.strategies.binary())
def test_hex_roundtrip(self, binary):
converted = binascii.hexlify(self.type2test(binary))
restored = binascii.unhexlify(self.type2test(converted))

@hauntsaninja hauntsaninja Sep 1, 2023

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Wonder if unhexlify should also take a sep and bytes_per_sep argument

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting review labels Sep 1, 2023
@hauntsaninja
hauntsaninja enabled auto-merge (squash) September 1, 2023 21:04
@hauntsaninja
hauntsaninja merged commit 3b73f9f into python:main Sep 1, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label Sep 1, 2023
@sobolevn

sobolevn commented Sep 1, 2023

Copy link
Copy Markdown
Member Author

Btw, @rhettinger was against doing this.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@hauntsaninja

hauntsaninja commented Sep 1, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

I agree that we shouldn't rely on Hypothesis' strategies as a substitute for fixed tests.

But I think the implementation chosen for adding property based tests in CPython is quite sensible! None of the Hypothesis strategies run by default in our test suite. Hypothesis is stubbed out and simply used used as a DSL to write property based tests where @hypothesis.example is used to provide the fixed test cases. However, if we wish, we can opt-in to get the real non-stub Hypothesis to run its strategies against the already written test because it's in a DSL it understands.

For this specific PR, I guess maybe I should have asked Nikita to add some specific @hypothesis.example here. Raymond, I'm happy to open a PR adding examples, or to revert, or to let it be. Just let me know!

(I also obviously agree we shouldn't use Hypothesis to write tests to enforce properties that are not actually properties, like the colorsys example)
Zac-HD reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@hugovk hugovk mentioned this pull request Sep 14, 2023
test_b2a_roundtrip failure in test_binascii.py #109418
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hauntsaninja hauntsaninja hauntsaninja approved these changes
@vstinner vstinner Awaiting requested review from vstinner
@pganssle pganssle Awaiting requested review from pganssle

Uh oh!

There was an error while loading. Please reload this page.

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.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@sobolevn @ambv @hauntsaninja @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.