Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
chunkBySize revealed bug - references dotnet#501
Browse files Browse the repository at this point in the history
@forki
forki committed Jun 17, 2015
1 parent b7c523f commit 6f05f77
Showing 1 changed file with 36 additions and 25 deletions.
61 changes: 36 additions & 25 deletions src/fsharp/FSharp.Core.PropertyTests/CollectionModulesConsistency.fs
View file
Original file line number Diff line number Diff line change
@@ -30,6 +30,29 @@ let ``append is consistent`` () =
Check.QuickThrowOnFailure append<string>
Check.QuickThrowOnFailure append<NormalFloat>

let averageFloat (xs : NormalFloat []) =
let xs = xs |> Array.map float
let s = run (fun () -> xs |> Seq.average)
let l = run (fun () -> xs |> List.ofArray |> List.average)
let a = run (fun () -> xs |> Array.average)
s = a && l = a

[<Test>]
let ``average is consistent`` () =
Check.QuickThrowOnFailure averageFloat

let averageBy (xs : float []) f =

This comment has been minimized.

Sign in to view

Copy link
@vasily-kirichenko

vasily-kirichenko Jun 17, 2015

Collaborator

I think it's better to use (F (_, f)) instead of just f, it will produce much more readable messages.
let xs = xs |> Array.map float
let f x = (f x : NormalFloat) |> float
let s = run (fun () -> xs |> Seq.averageBy f)
let l = run (fun () -> xs |> List.ofArray |> List.averageBy f)
let a = run (fun () -> xs |> Array.averageBy f)
s = a && l = a

[<Test>]
let ``averageBy is consistent`` () =
Check.QuickThrowOnFailure averageBy

let contains<'a when 'a : equality> (xs : 'a []) x =
let s = xs |> Seq.contains x
let l = xs |> List.ofArray |> List.contains x
@@ -54,6 +77,18 @@ let ``choose is consistent`` () =
Check.QuickThrowOnFailure contains<string>
Check.QuickThrowOnFailure contains<float>

let chunkBySize<'a when 'a : equality> (xs : 'a []) size =

This comment has been minimized.

Sign in to view

Copy link
@vasily-kirichenko

vasily-kirichenko Jun 17, 2015

Collaborator

How about adding the following property:

[<Test>]
let ``Seq.chunkBySize >> concat = original seq``() =
    let prop (a: _ list) (NonNegativeInt chunkSize) = 
        Seq.chunkBySize chunkSize a |> Seq.concat |> Seq.toList = a
    Check.One ({ Config.VerboseThrowOnFailure with MaxTest = 1000 }, prop)

This comment has been minimized.

Sign in to view

Copy link
@forki

forki Jun 17, 2015

Author Owner

I already have that in "ListProperties"

This comment has been minimized.

Sign in to view

Copy link
@vasily-kirichenko

vasily-kirichenko Jun 17, 2015

Collaborator

OK :) What about this one:

[<Test>]
let ``Seq::chunkBySize produces chunks exactly of size `chunkSize`, except the last one, which can be smaller, but not empty``() =
    let prop (a: _ list) (NonNegativeInt chunkSize) =
        match a |> Seq.chunkBySize chunkSize |> Seq.toList with
        | [] -> a = []
        | h :: [] -> h.Length <= chunkSize
        | chunks ->
            let lastChunk = chunks |> List.last
            let headChunks = chunks |> Seq.take (chunks.Length - 1) |> Seq.toList
            headChunks |> List.forall (Array.length >> (=) chunkSize)
            &&
            lastChunk <> [||]
            &&
            lastChunk.Length <= chunkSize
    Check.One (largeConfig, prop)

This comment has been minimized.

Sign in to view

Copy link
@forki

forki via email Jun 17, 2015

Author Owner

This comment has been minimized.

Sign in to view

Copy link
@forki

forki Jun 17, 2015

Author Owner

cb1a91e
let s = run (fun () -> xs |> Seq.chunkBySize size |> Seq.map Seq.toArray |> Seq.toArray)
let l = run (fun () -> xs |> List.ofArray |> List.chunkBySize size |> Seq.map Seq.toArray |> Seq.toArray)
let a = run (fun () -> xs |> Array.chunkBySize size |> Seq.map Seq.toArray |> Seq.toArray)
s = a && l = a

[<Test>]
let ``chunkBySize is consistent`` () =
Check.QuickThrowOnFailure chunkBySize<int>
Check.QuickThrowOnFailure chunkBySize<string>
Check.QuickThrowOnFailure chunkBySize<float>

This comment has been minimized.

Sign in to view

Copy link
@vasily-kirichenko

vasily-kirichenko Jun 17, 2015

Collaborator

I don't think that testing with various generic types has any meaning.

This comment has been minimized.

Sign in to view

Copy link
@forki

forki Jun 17, 2015

Author Owner

I think it's important to test the functions with reference and value types.
Ideally I'd like to let FsCheck use random types that fit the requirements. But there are some issues with that. As an example: most library functions don't work with random floats since nan is not really comparable (but float claims to be comparable). So we have to explicitly use NormalFloat

@kurtschelfthout

let collect<'a> (xs : 'a []) f =
let s = xs |> Seq.collect f
let l = xs |> List.ofArray |> List.collect (fun x -> f x |> List.ofArray)
@@ -112,28 +147,4 @@ let sort<'a when 'a : comparison> (xs : 'a []) =
let ``sort is consistent`` () =
Check.QuickThrowOnFailure sort<int>
Check.QuickThrowOnFailure sort<string>
Check.QuickThrowOnFailure sort<NormalFloat>


let averageFloat (xs : NormalFloat []) =
let xs = xs |> Array.map float
let s = run (fun () -> xs |> Seq.average)
let l = run (fun () -> xs |> List.ofArray |> List.average)
let a = run (fun () -> xs |> Array.average)
s = a && l = a

[<Test>]
let ``average is consistent`` () =
Check.QuickThrowOnFailure averageFloat

let averageBy (xs : float []) f =
let xs = xs |> Array.map float
let f x = (f x : NormalFloat) |> float
let s = run (fun () -> xs |> Seq.averageBy f)
let l = run (fun () -> xs |> List.ofArray |> List.averageBy f)
let a = run (fun () -> xs |> Array.averageBy f)
s = a && l = a

[<Test>]
let ``averageBy is consistent`` () =
Check.QuickThrowOnFailure averageBy
Check.QuickThrowOnFailure sort<NormalFloat>

1 comment on commit 6f05f77

@kurtschelfthout
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, generic types are tricky. Since the whole thing is pretty messy in .NET I don't think there is a clean solution. E.g. even though some type says "I'm generic on type 'a" it can look at the actual 'a say using reflection and the signature does not expose this.

F#, in some sense, makes this worse because the ad hoc constraints like equality and comparison afaik are not visible using reflection, so I don't think there is an automatic way for FsCheck to pick up on that. I believe FsCheck will currently do "something" when it finds a generic type, which is to use the obj generator, which generates strings and ints and booleans (off the top of my head). I explicitly excluded things like floats for the reason @forki mentioned.

All in all FsCheck could use some heuristics with sensible defaults. But formally speaking, if a method is generic on its arguments it should not matter what type you pass in there - if it works for one, it works for all of them (theorems for free and all that.). Not the case in .NET.

Please sign in to comment.