Minor fixes #3688
Minor fixes #3688
Conversation
| @@ -788,7 +788,7 @@ def run(data): | |||
| test.__name__, | |||
| args, | |||
| kwargs, | |||
| force_split=True, | |||
| force_split=None, | |||
There was a problem hiding this comment.
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"...
There was a problem hiding this comment.
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.
| # 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 ["@", "."]: |
There was a problem hiding this comment.
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__))
| @@ -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: | |||
There was a problem hiding this comment.
No idea, but it seemed like this was the intent? Otherwise, _nonempty_filters is not used.
There was a problem hiding this comment.
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!
| @@ -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}") | |||
There was a problem hiding this comment.
This one should be obvious.
| @@ -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: | |||
There was a problem hiding this comment.
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!
| @@ -788,7 +788,7 @@ def run(data): | |||
| test.__name__, | |||
| args, | |||
| kwargs, | |||
| force_split=True, | |||
| force_split=None, | |||
There was a problem hiding this comment.
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.
| @@ -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 | |||
There was a problem hiding this comment.
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
|
Folded into #3687 |
Minor issues found while wandering through the source. Each of them is too small for an individual PR in my opinion.
I'll describe the individual changes as code comments, and once you've had a change to nix them I'll create RELEASE and convert to proper PR.