Isolate mutations from calls to Gen.bind #51
Conversation
|
The string/keyword |
|
Quoting @cmeeren from hedgehogqa/fsharp-hedgehog#192 (comment)
I think correctness is more important than efficiency, so I think we should merge this PR because it fixes bugs and worry about performance another day. |
|
Thanks a lot! I don't have the capacity to do a thorough review of this. If you think it is ready to go, I'll merge and release a new version. Could you also update the changelog and fsproj version? |
|
Alternatively, if you'd rather wait to release this until other PRs are merged, don't increment the fsproj version (that triggers a release). |
I agree. I wanted to hold out for a couple of days in case it was fixed upstream, and then I forgot all about it. |
|
And if this closes #52, could you add that to the OP so it's done automatically? |
| return fun dt -> shape.Set dt f } } | ||
| member _.Visit(shape: ShapeMember<'DeclaringType, 'MemberType>) = | ||
| autoInner<'MemberType> config recursionDepths | ||
| |> Gen.map (fun mt -> fun dt -> shape.Set dt mt) |
There was a problem hiding this comment.
Nice. (Pity that we can't |> Gen.map shape.Set though.)
|
Oh, I think the issue with efficiency was that your immutable code was quadratic (due to repeatedly concatenating a single character to a string accumulator). Pretty sure the runtime of my code is linear. |
Let's wait until I also fix the shuffle functions. |
Yes, it's ready. In particular, I first added tests that fail send now they pass. |
|
Excellent! Could you also update the changelog (repo root) and fsproj version? It will then be released automatically once I merge. |
|
I updated the changelog. Can we wait to release a new version until after I fix the same bugs in |
The exact same mutations still happen, but now they happen all at once instead of alternating with calls to Gen.bind (via let! in gen CE)
TysonMN
force-pushed
the
bug_fix_333_Hedgehog
branch
from
a4a8f72 to
fada582
Compare
|
(Force pushed to improve the names of the tests) |
|
Excellent, thank you very much! |
Revolves #52
...by working around hedgehogqa/fsharp-hedgehog#192.
@dharmaturtle, after this completes and a new version is released, you should update
Hedgehog.Xunitto require this new version and make a release so that users get this bug fix.