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

remove hypothesis (at least for now) #586

Merged
merged 12 commits into from
May 3, 2023
Merged

remove hypothesis (at least for now) #586

merged 12 commits into from
May 3, 2023

Conversation

glyph
Copy link
Member

@glyph glyph commented May 2, 2023

Fixes #561

I am not entirely convinced we need Hypothesis at all, but if we do, when we reintroduce it we have to first do the work to make it deterministic in CI and to address the various deficiencies in Hyperlink's strategies if we want to use those.

(ugh I did a thing in the wrong tab, please ignore the other PR)

@glyph glyph requested a review from a team as a code owner May 2, 2023 22:02
@codecov
Copy link

codecov bot commented May 2, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01 🎉

Comparison is base (23996d1) 99.04% compared to head (87c08f6) 99.05%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #586      +/-   ##
==========================================
+ Coverage   99.04%   99.05%   +0.01%     
==========================================
  Files          44       45       +1     
  Lines        3887     3935      +48     
  Branches      520      523       +3     
==========================================
+ Hits         3850     3898      +48     
  Misses         23       23              
  Partials       14       14              
Impacted Files Coverage Δ
src/klein/test/__init__.py 100.00% <ø> (ø)
src/klein/test/not_hypothesis.py 100.00% <100.00%> (ø)
src/klein/test/test_headers.py 100.00% <100.00%> (ø)
src/klein/test/test_message.py 100.00% <100.00%> (ø)
src/klein/test/test_plating.py 100.00% <100.00%> (ø)
src/klein/test/test_request_compat.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@glyph glyph enabled auto-merge May 3, 2023 01:03
yield DecodedURL.from_text("https://example.com")
yield DecodedURL.from_text("http://example.com/")
yield DecodedURL.from_text("https://example.com/é")
yield DecodedURL.from_text("https://súbdomain.example.com/ascii/path/")
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you want a URL str that has a %-encoded path element here?

Copy link
Contributor

@dreid dreid left a comment

Choose a reason for hiding this comment

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

As a basic compatibility layer this seems like a reasonable implementation.

@glyph
Copy link
Member Author

glyph commented May 3, 2023

Thanks @dreid . Still need to get the robot to respect non-core reviews.

@glyph glyph disabled auto-merge May 3, 2023 01:32
@glyph glyph merged commit 6d8f1db into master May 3, 2023
@glyph glyph deleted the falsified branch May 3, 2023 01:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

hyperlink generates invalid URLs via hypothesis which sometimes causes our tests to fail
2 participants