Hypothesis testing #136
Hypothesis testing #136
Conversation
Codecov Report
@@ Coverage Diff @@
## main #136 +/- ##
==========================================
- Coverage 97.45% 97.44% -0.02%
==========================================
Files 41 41
Lines 4007 4068 +61
Branches 28 28
==========================================
+ Hits 3905 3964 +59
- Misses 102 104 +2
Continue to review full report at Codecov.
|
tests/test_hypothesis.py
Outdated
| class BranchModel(TypedDict): | ||
| name: str | ||
| sub_branch: Optional['BranchModel'] | ||
|
|
||
|
|
||
| @settings(**settings_dict) | ||
| @given(strategies.from_type(BranchModel)) | ||
| def test_recursive_hyp(recursive_schema, data): |
There was a problem hiding this comment.
The reason that this test passes is that Hypothesis doesn't generate cyclic references by default; unless you write a strategy which can explicitly choose between "generate a new BranchModel" and "reuse an existing BranchModel" you'll always get the former. That might look something like:
@st.composite
def branch_models_with_cycles(draw, existing: list[BranchModel] | None = None) -> BranchModel:
if existing is None:
existing = []
name = draw(st.text())
sub_branch = draw(st.builds(BranchModel) | st.sampled_from(existing)) # might need to handle empty-list case?
existing.append(sub_branch) # ok this recursion scheme is broken but you get the idea
return BranchModel(name=name, sub_branch=sub_branch)There was a problem hiding this comment.
Looks good, I'll add this.
There was a problem hiding this comment.
I've tried tweaking this and got a recursion error in hypothesis (I think)
Adjusted function:
@strategies.composite
def branch_models_with_cycles(draw, existing: list[BranchModel] = strategies.from_type(list[BranchModel])):
# debug(existing)
name = draw(strategies.text())
sub_branch = draw(strategies.from_type(BranchModel) | strategies.sampled_from(existing))
branch = BranchModel(name=name, sub_branch=sub_branch)
existing.append(branch)
return branchThe traceback is massive, gist of the output here.
There was a problem hiding this comment.
This one works:
@st.composite
def branch_models_with_cycles(draw, existing: list[BranchModel] | None = None):
if existing is None:
existing = []
name = draw(st.text())
sub_branch = draw(
st.none()
| (st.sampled_from(existing) if existing else st.nothing())
| branch_models_with_cycles(existing)
)
branch = BranchModel(name=name, sub_branch=sub_branch)
existing.append(branch)
return branchI think your underlying problem was that existing: list[BranchModel] = strategies.from_type(list[BranchModel]) is ill-typed; the default value is a strategy which generates list[BranchModel], rather than an instance of that type.
There was a problem hiding this comment.
I've tried that unfortunately it's not causing a failure, since the base of this PR pre-dates #134, I think that means this code still isn't generate cyclic references.
(Ignore the failures on CI, that's just rust nightly and will fix itself soon).
There was a problem hiding this comment.
🤦 yeah, by the clear light of day we have to append to the existing list before we draw from it, not afterwards, and then mutate the sub_branch in place afterwards:
@st.composite
def branch_models_with_cycles(draw, existing=None):
if existing is None:
existing = []
model = BranchModel(name=draw(st.text()), sub_branch=None)
existing.append(model)
model["sub_branch"] = draw(
st.none()
| st.builds(BranchModel, name=st.text(), sub_branch=branch_models_with_cycles(existing))
| st.sampled_from(existing)
)
return modelI've checked that this generates cycles as well as just generating values at all. Sorry for the back-and-forth!
There was a problem hiding this comment.
Not at all! I should be the one apologising.
Thanks so much, that's working now.
|
Don't actually have a working demo, but here's the jotted-off-in-five-minutes version; I can help with a real fix on the weekend if you need it 🙂 |
samuelcolvin
force-pushed
the
hypothesis-testing
branch
from
302516e to
b69f93e
Compare
|
@Zac-HD thanks so much for your help on this. I'll extend the hypothesis tests more in future. |
|
Very happy to help 😁 If there's ever a way I can help give Pydantic first-class support for Hypothesis, just let me know! |
|
Thanks so much for the offer, I'll let you know when we get to that point. |
@Zac-HD hope you don't mind me tagging you in this and asking for advice, let me know if you'd prefer me to create an issue on hypothesis.
There's a particular failure I'd like hypothesis to find so I can be confident my fix (#134) for it works generally.
The example is demonstrated in
test_recursive_broken- basically recursive data with a cyclic reference. Currently it seems hypothesis is not generating this case. Is there a way to get it to do so?