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

Minor fixes #3688

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion hypothesis-python/src/hypothesis/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -788,7 +788,7 @@ def run(data):
test.__name__,
args,
kwargs,
force_split=True,
force_split=None,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This makes verbose formatting similar to what is shown here: https://github.com/HypothesisWorks/hypothesis/blob/master/HypothesisWorks.github.io/_posts/2016-04-16-anatomy-of-a-test.md

I think that is much nicer, but I assume there's a reason it was True so feel free to say "no"...

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's nice when the whole call fits on a single line, but much worse when all the arguments are on a single line after the test name, and that turns out to be common in practice. One-line-per-argument is a consistent way to get out of that, and also means that explain mode (#3555) looks similar to standard reports.

arg_slices=argslices,
leading_comment=(
"# " + context.data.slice_comments[(0, 0)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,7 @@ def biased_coin(
# (see https://bugs.python.org/issue2506), coverage incorrectly
# thinks that this condition is always true. You can trivially
# check by adding `else: assert False` and running the tests.
# Note: Fixed in both CPython and PyPy 3.10, with PEP 626
Copy link
Member

Choose a reason for hiding this comment

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

Oh, nice! Since we now only support Python >= 3.8, and run coverage jobs on 3.10, let's just delete the pragma and this whole comment ☺️

data.draw_bits(bits, forced=int(result))
break
data.stop_example()
Expand Down
8 changes: 4 additions & 4 deletions hypothesis-python/src/hypothesis/internal/reflection.py
Original file line number Diff line number Diff line change
Expand Up @@ -335,10 +335,10 @@ def extract_lambda_source(f):
break
except SyntaxError:
continue
if tree is None and source.startswith("@"):
# This will always eventually find a valid expression because
# the decorator must be a valid Python function call, so will
# eventually be syntactically valid and break out of the loop.
if tree is None and source and source[0] in ["@", "."]:
Copy link
Contributor Author

@jobh jobh Jun 28, 2023

Choose a reason for hiding this comment

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

If you show stats for the sample code in issue 3618 (intentionally not linked to avoid irrelevant backref), it will show (see <unknown> at end of line)

      * 42.20%, Aborted test because unable to satisfy just(Rule(targets=(), function=rule1, arguments={'data': lists(text(), min_size=4, unique=True)}, preconditions=(), bundles=())).filter(RuleStrategy(machine=MyStateMachine({...})).is_valid).filter(lambda r: <unknown>)

With this change (strip leading .), it shows

      * 41.50%, Aborted test because unable to satisfy just(Rule(targets=(), function=rule1, arguments={'data': lists(text(), min_size=4, unique=True)}, preconditions=(), bundles=())).filter(RuleStrategy(machine=MyStateMachine({...})).is_valid).filter(lambda r: feature_flags.is_enabled(r.function.__name__))

# This will always eventually find a valid expression because the
# decorator or chained operator must be a valid Python function call,
# so will eventually be syntactically valid and break out of the loop.
# Thus, this loop can never terminate normally.
for i in range(len(source) + 1):
p = source[1:i]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ def not_yet_in_unique_list(val):
while elements.more():
value = filtered.do_filtered_draw(data)
if value is filter_not_satisfied:
elements.reject("Aborted test because unable to satisfy {filtered!r}")
elements.reject(f"Aborted test because unable to satisfy {filtered!r}")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one should be obvious.

else:
for key, seen in zip(self.keys, seen_sets):
seen.add(key(value))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,3 +102,6 @@ def __repr__(self):
class FeatureStrategy(SearchStrategy):
def do_draw(self, data):
return FeatureFlags(data)

def __repr__(self):
return f"FeatureStrategy()"
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ def __repr__(self):
)

def filter(self, condition):
if condition in (str.lower, str.title, str.upper):
if condition in self._nonempty_filters:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No idea, but it seemed like this was the intent? Otherwise, _nonempty_filters is not used.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, this is tricky: here we're emitting a warning because it seems likely that you intended str.islower instead of str.lower (etc.); the _nonempty_filters are invoked via the call to ListStrategy.filter or super().filter below.

Adding a comment to explain that would be useful though!

warnings.warn(
f"You applied str.{condition.__name__} as a filter, but this allows "
f"all nonempty strings! Did you mean str.is{condition.__name__}?",
Expand Down