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

Property testing #360

Closed
Andrea opened this issue Apr 11, 2015 · 15 comments
Closed

Property testing #360

Andrea opened this issue Apr 11, 2015 · 15 comments

Comments

@Andrea
Copy link

Andrea commented Apr 11, 2015

Hi there

We were discussing on twitter the possibility of adding some property testing to this codebase during next week #fsharpex hack day (would be a great starting point and we can keep adding later). @forki rightly suggested asking for the direction we should take on this. We are all ears

@forki
Copy link
Contributor

forki commented Apr 11, 2015

I think it would be a great opportunity for newcomers to start hacking on the F# compiler project and add property based testing to the FSharp.Core.dll. Things like List.rev(List.rev xs) = xs are canonical samples for FsCheck and would still be nice to have in the compiler project.

One important point is to discuss if this would be accepted and how we can make sure that everyone involved has signed the CLA.

@Andrea
Copy link
Author

Andrea commented Apr 11, 2015

I am pretty sure we could involve SkillsMatters by asking them to email people registered to take part about this. (I can email them today or tomorrow just asking them if they can include a paragraph explaining that we are planning to this ( hacking on the compiler and adding property testing) and that to that effect people need to complete this https://cla.msopentech.com/ (is this the right one?)

@forki
Copy link
Contributor

forki commented Apr 11, 2015

Good idea.

Also it would be good to set up a fork with enabled FsCheck which people
can clone and send pull requests to.
We then can send a aggregated pull request to this main repo here.
On Apr 11, 2015 2:34 PM, "Andrea" [email protected] wrote:

I am pretty sure we could involve SkillsMatters by asking them to email
people registered to take part about this. (I can email them today or
tomorrow just asking them if they can include a paragraph explaining that
we are planning to this ( hacking on the compiler and adding property
testing) and that to that effect people need to complete this
https://cla.msopentech.com/ (is this the right one?)


Reply to this email directly or view it on GitHub
#360 (comment)
.

@Andrea
Copy link
Author

Andrea commented Apr 12, 2015

👍

@enricosada
Copy link
Contributor

👍 i want to add it too for #247 !!!
I have a tree structure RoseTree and tests but with property based is really easier (fold, map, etc)

And also the rules for #246 can be property tested

@latkin
Copy link
Contributor

latkin commented Apr 13, 2015

Cool idea! @dsyme and @KevinRansom and I talked about this, here's our thinking of how it could work.

We love the potential of getting people involved in contributing via a hackathon, and we want to help enable that in whatever manner is practical. We are happy to accept new tests that cover current test gaps, especially if they reveal bugs. We are less interested in adding new tests which just re-implement existing tests using a different framework/approach. If the focus is on tests for FSharp.Core, then we'd prefer they just be added to FSharp.Core.Unittests.

So how about this:

  • Have participants implement tests (using fscheck or whatever style they like) to FSharp.Core.Unittests
  • We'll be open to accepting any of those that clearly cover previously-missed cases and/or reveal bugs
  • To reduce overhead while in-progress, we ask that you manage the event in a fork, then send a consolidated PR

Of course any contributor who's stuff is to be brought back would need to have a CLA. That should be much easier now than in the past.

@forki
Copy link
Contributor

forki commented Apr 14, 2015

@latkin thanks. That's pretty much what I had in mind (and the new CLA helps us a lot). One additional question: what about replacing existing unittests with property test?

if you look at https://github.com/Microsoft/visualfsharp/blob/fsharp4/src/fsharp/FSharp.Core.Unittests/FSharp.Core/Microsoft.FSharp.Collections/ListModule2.fs#L441 it could be replaced with property tests in order to make the test "stronger".

@Andrea Anything left that we should discuss? Are you contacting SkillsMatters?

@Andrea
Copy link
Author

Andrea commented Apr 14, 2015

@forki hi there, yes I talked to them but I haven't seen an email sent out so I'll ping them.

@latkin
Copy link
Contributor

latkin commented Apr 14, 2015

@forki we would like to spend our limited time reviewing a small set of new tests that clearly add new value, rather than broad re-implementations of perfectly good existing tests. The true "strength" of the tests will be judged by how well they reveal new bugs.

@forki
Copy link
Contributor

forki commented Apr 14, 2015

Understood.

@kurtschelfthout
Copy link
Contributor

👍 of course - was initially a bit skeptical that it would be able to find bugs in core data structures after all these years of usage, until I read #370. Totally agree that just aiming to replace existing tests is a bit too weak - but maybe existing bugs are a good starting point to find/test more properties related to that bug. E.g. from #370 one might take a look at sorting a lot of different structures using Seq.sort and check they give the same results, or comparing the result of List|Seq|Array.sort or generally look at float handling where it may matter.

Opportunistically, just enabling the possibility to add FsCheck based tests for future tests/fixes/contributions would be useful, given the slight pains @forki had to go through to make them work. All we need is one bug ;)

Won't be at a machine today most of the day but feel free to tweet me if I can help. And have fun!

@latkin
Copy link
Contributor

latkin commented Apr 18, 2015

Something to keep in mind - the core unit tests need to build/run as portable libraries. Is the nunit fscheck stuff compatible with all portable profiles?

@forki
Copy link
Contributor

forki commented Apr 18, 2015

At the moment we are not sure what's it's actually testing at all. ;-) we had weird effects and still have no idea what I did wrong

@latkin
Copy link
Contributor

latkin commented Aug 4, 2015

ref #502

@latkin latkin added this to the F# 4.0 Update 1 milestone Aug 4, 2015
@latkin
Copy link
Contributor

latkin commented Aug 10, 2015

@forki's PR with property-based tests has been merged

@latkin latkin closed this as completed Aug 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants