Accept user-defined strategies

[pradeep90]

tl;dr: This PR adds support for user-defined strategies in the settings of check_all_laws. Below is a root cause analysis for why we can get flaky tests if we directly register custom strategies with hypothesis.

Why custom strategies registered with hypothesis can lead to spurious test failures

Problem: If a user registers a HKT that is not a ContainerN, that will still be considered by hypothesis as a subclass of KindN[ContainerN, ...].

Example: Say we have _Wrapper that is a KindN but not a ContainerN and we register a custom strategy for it.

st.register_type_strategy(
    _Wrapper,
    st.builds(_Wrapper, st.integers()),
)

If we run tests/test_laws.py::test_io_containern_associative_law, hypothesis will need KindN['ContainerN', _NewType1, _SecondType, _ThirdType] and will try all the types in the global type lookup that are subclasses of the type. Unfortunately, _Wrapper will fit the bill:

  # Inside `hypothesis/strategies/_internal/core.py`
(Pdb) try_issubclass(_Wrapper, KindN[ContainerN, int, int, int])
True

So, hypothesis will create a _Wrapper as an instance of KindN[ContainerN, ...], which will crash the test:

E       AttributeError: '_Wrapper' object has no attribute 'bind'
E       while generating 'Draw 1' from builds(associative_law)
E       Falsifying example: factory(
E           source=data(...),
E       )
E       Called function: (lambda *args, **kwargs: <unknown>)(None) -> <Wrapper: 0>

This means we can't risk having any descendant of KindN in the _global_type_lookup. We should discourage users from directly registering their own strategies for their containers.

Current Behavior: We drop any user-defined strategies

The good news is that we currently don't allow it right now. We used to allow users to register custom strategies with hypothesis, but we stopped in #1893, probably because of the difficulties above. This means any strategy registered using st.register_type_strategy gets ignored.

Unfortunately, there is no way to override the strategies created by the plugin.

This PR: Register strategies only when running laws

How to register a custom strategy for a container without unintentionally using it for KindN?

Proposed solution: Register the user-defined strategy only when running laws for that container.

We accept the strategy in check_all_laws. This gives us the flexibility to try different strategies without modifying the source code.

Alternative designs:

• Have a method in the container class: This is too restrictive. We want to favour composition over inheritance.
• Have a global variable: This is a pretty big headache. Debugging the current hypothesis behavior was pretty frustrating because a change in one test broke unrelated tests. The whole point of returns is to avoid global mutation :)

Implementation:

• Extract a context manager that temporarily sets strategies for multiple types
  • Eventually, we could move all the hypothesis mutations into this new module, so that the plugin code is largely decoupled from those gory details. I want to keep this PR simple(-ish).
• Accept a container strategy in the settings

Checklist

• I have double checked that there are no unrelated changes in this pull request (old patches, accidental config files, etc)
• I have created at least one test case for the changes I have made
• I have updated the documentation for the changes I have made
• I have added my changes to the CHANGELOG.md

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (82ef3ef) to head (b616e82).
Report is 344 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##            master     #2061    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           80        81     +1     
  Lines         2485      2576    +91     
  Branches       437        44   -393     
==========================================
+ Hits          2485      2576    +91     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[sobolevn]

Thank you! Great work! A couple of nits.

[sobolevn]

Maybe we should assert that users cannot pass both use_init and container_stratregy 🤔 ?

[pradeep90]

I've added a __post_init__ check for Settings. There are already overloads for check_all_laws, so that function should be safe. Ideally, we would make the inputs mutually exclusive in the types, but I don't know how to do that in a backward-compatible way. Leaving it at this.

[sobolevn]

Let's also document Settings in hypothesis_plugins.rst since it is now public.

[pradeep90]

I'd made it public just so that I could use it in tests :P. I'm making it private again, because check_all_laws accepts direct arguments, not a settings object. So, I don't think it makes much sense to document the class in public and unnecessarily make the class harder to change in the future. Lmk if you feel strongly.

[sobolevn]

Thanks!