From 0b25ccde00462898bd1e477672bb671798a88b19 Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Thu, 21 Dec 2023 02:06:08 +0100 Subject: [PATCH 01/13] Implement `TaskSeq.skipWhile`, `skipWhileAsync`, `skipWhileInclusive` and `skipWhileInclusiveAsync` --- src/FSharp.Control.TaskSeq/TaskSeq.fs | 4 + src/FSharp.Control.TaskSeq/TaskSeq.fsi | 66 ++++++++++++-- src/FSharp.Control.TaskSeq/TaskSeqInternal.fs | 85 +++++++++++++++++-- 3 files changed, 142 insertions(+), 13 deletions(-) diff --git a/src/FSharp.Control.TaskSeq/TaskSeq.fs b/src/FSharp.Control.TaskSeq/TaskSeq.fs index be9961f7..d3aedcb9 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeq.fs +++ b/src/FSharp.Control.TaskSeq/TaskSeq.fs @@ -297,6 +297,10 @@ type TaskSeq private () = static member takeWhileAsync predicate source = Internal.takeWhile Exclusive (PredicateAsync predicate) source static member takeWhileInclusive predicate source = Internal.takeWhile Inclusive (Predicate predicate) source static member takeWhileInclusiveAsync predicate source = Internal.takeWhile Inclusive (PredicateAsync predicate) source + static member skipWhile predicate source = Internal.skipWhile Exclusive (Predicate predicate) source + static member skipWhileAsync predicate source = Internal.skipWhile Exclusive (PredicateAsync predicate) source + static member skipWhileInclusive predicate source = Internal.skipWhile Inclusive (Predicate predicate) source + static member skipWhileInclusiveAsync predicate source = Internal.skipWhile Inclusive (PredicateAsync predicate) source static member tryPick chooser source = Internal.tryPick (TryPick chooser) source static member tryPickAsync chooser source = Internal.tryPick (TryPickAsync chooser) source diff --git a/src/FSharp.Control.TaskSeq/TaskSeq.fsi b/src/FSharp.Control.TaskSeq/TaskSeq.fsi index 68b8d4e2..4705d370 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeq.fsi +++ b/src/FSharp.Control.TaskSeq/TaskSeq.fsi @@ -830,10 +830,10 @@ type TaskSeq = static member takeWhile: predicate: ('T -> bool) -> source: TaskSeq<'T> -> TaskSeq<'T> /// - /// Returns a sequence that, when iterated, yields elements of the underlying sequence while the + /// Returns a task sequence that, when iterated, yields elements of the underlying sequence while the /// given asynchronous function returns , and then returns no further elements. /// The first element where the predicate returns is not included in the resulting sequence - /// (see also ). + /// (see also ). /// If is synchronous, consider using . /// /// @@ -844,7 +844,7 @@ type TaskSeq = static member takeWhileAsync: predicate: ('T -> #Task) -> source: TaskSeq<'T> -> TaskSeq<'T> /// - /// Returns a sequence that, when iterated, yields elements of the underlying sequence until the given + /// Returns a task sequence that, when iterated, yields elements of the underlying sequence until the given /// function returns , returns that element /// and then returns no further elements (see also ). This function returns /// at least one element of a non-empty sequence, or the empty task sequence if the input is empty. @@ -858,9 +858,9 @@ type TaskSeq = static member takeWhileInclusive: predicate: ('T -> bool) -> source: TaskSeq<'T> -> TaskSeq<'T> /// - /// Returns a sequence that, when iterated, yields elements of the underlying sequence until the given + /// Returns a task sequence that, when iterated, yields elements of the underlying sequence until the given /// asynchronous function returns , returns that element - /// and then returns no further elements (see also ). This function returns + /// and then returns no further elements (see also ). This function returns /// at least one element of a non-empty sequence, or the empty task sequence if the input is empty. /// If is synchronous, consider using . /// @@ -871,6 +871,62 @@ type TaskSeq = /// Thrown when the input task sequence is null. static member takeWhileInclusiveAsync: predicate: ('T -> #Task) -> source: TaskSeq<'T> -> TaskSeq<'T> + /// + /// Returns a task sequence that, when iterated, skips elements of the underlying sequence while the + /// given function returns , and then yields the remaining + /// elements. The first element where the predicate returns is returned, which means that this + /// function will skip 0 or more elements (see also ). + /// If is asynchronous, consider using . + /// + /// + /// A function that evaluates to false when no more items should be skipped. + /// The input task sequence. + /// The resulting task sequence. + /// Thrown when the input task sequence is null. + static member skipWhile: predicate: ('T -> bool) -> source: TaskSeq<'T> -> TaskSeq<'T> + + /// + /// Returns a task sequence that, when iterated, skips elements of the underlying sequence while the + /// given asynchronous function returns , and then yields the + /// remaining elements. The first element where the predicate returns is returned, which + /// means that this function will skip 0 or more elements (see also ). + /// If is synchronous, consider using . + /// + /// + /// An asynchronous function that evaluates to false when no more items should be skipped. + /// The input task sequence. + /// The resulting task sequence. + /// Thrown when the input task sequence is null. + static member skipWhileAsync: predicate: ('T -> #Task) -> source: TaskSeq<'T> -> TaskSeq<'T> + + /// + /// Returns a task sequence that, when iterated, skips elements of the underlying sequence until the given + /// function returns , also skips that element + /// and then yields the remaining elements (see also ). This function skips + /// at least one element of a non-empty sequence, or returns the empty task sequence if the input is empty. + /// If is asynchronous, consider using . + /// ` + /// + /// A function that evaluates to false when no more items should be skipped. + /// The input task sequence. + /// The resulting task sequence. + /// Thrown when the input task sequence is null. + static member skipWhileInclusive: predicate: ('T -> bool) -> source: TaskSeq<'T> -> TaskSeq<'T> + + /// + /// Returns a task sequence that, when iterated, skips elements of the underlying sequence until the given + /// function returns , also skips that element + /// and then yields the remaining elements (see also ). This function skips + /// at least one element of a non-empty sequence, or returns the empty task sequence if the input is empty. + /// If is synchronous, consider using . + /// + /// + /// An asynchronous function that evaluates to false when no more items should be skipped. + /// The input task sequence. + /// The resulting task sequence. + /// Thrown when the input task sequence is null. + static member skipWhileInclusiveAsync: predicate: ('T -> #Task) -> source: TaskSeq<'T> -> TaskSeq<'T> + /// /// Applies the given function to successive elements, returning the first result where /// the function returns . diff --git a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs index d6a33421..8dba6ffe 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs +++ b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs @@ -13,9 +13,9 @@ type internal AsyncEnumStatus = [] type internal WhileKind = - /// The item under test is included even if false + /// The item under test is included (or skipped) even if false | Inclusive - /// The item under test is always excluded + /// The item under test is always excluded (or not skipped) | Exclusive [] @@ -731,48 +731,52 @@ module internal TaskSeqInternal = taskSeq { use e = source.GetAsyncEnumerator CancellationToken.None - let! step = e.MoveNextAsync() - let mutable more = step + let! moveFirst = e.MoveNextAsync() + let mutable more = moveFirst match whileKind, predicate with - | Exclusive, Predicate predicate -> + | Exclusive, Predicate predicate -> // takeWhile while more do let value = e.Current more <- predicate value if more then + // yield ONLY if predicate is true yield value let! ok = e.MoveNextAsync() more <- ok - | Inclusive, Predicate predicate -> + | Inclusive, Predicate predicate -> // takeWhileInclusive while more do let value = e.Current more <- predicate value + // yield, regardless of result of predicate yield value if more then let! ok = e.MoveNextAsync() more <- ok - | Exclusive, PredicateAsync predicate -> + | Exclusive, PredicateAsync predicate -> // takeWhileAsync while more do let value = e.Current let! passed = predicate value more <- passed if more then + // yield ONLY if predicate is true yield value let! ok = e.MoveNextAsync() more <- ok - | Inclusive, PredicateAsync predicate -> + | Inclusive, PredicateAsync predicate -> // takeWhileInclusiveAsync while more do let value = e.Current let! passed = predicate value more <- passed + // yield regardless of predicate yield value if more then @@ -780,6 +784,71 @@ module internal TaskSeqInternal = more <- ok } + let skipWhile whileKind predicate (source: TaskSeq<_>) = + checkNonNull (nameof source) source + + taskSeq { + use e = source.GetAsyncEnumerator CancellationToken.None + let! moveFirst = e.MoveNextAsync() + let mutable more = moveFirst + + match whileKind, predicate with + | Exclusive, Predicate predicate -> // skipWhile + while more && predicate e.Current do + let! ok = e.MoveNextAsync() + more <- ok + + if more then + // yield the last one where the predicate was false + // (this ensures we skip 0 or more) + yield e.Current + + while! e.MoveNextAsync() do // get the rest + yield e.Current + + | Inclusive, Predicate predicate -> // skipWhileInclusive + while more && predicate e.Current do + let! ok = e.MoveNextAsync() + more <- ok + + if more then + // yield the rest (this ensures we skip 1 or more) + while! e.MoveNextAsync() do + yield e.Current + + | Exclusive, PredicateAsync predicate -> // skipWhileAsync + while more do + let! passed = predicate e.Current + more <- passed + + if more then + let! ok = e.MoveNextAsync() + more <- ok + + // yield the rest + if more then + // yield the last one where the predicate was false + // (this ensures we skip 0 or more) + yield e.Current + + while! e.MoveNextAsync() do + yield e.Current + + | Inclusive, PredicateAsync predicate -> // skipWhileInclusiveAsync + while more do + let! passed = predicate e.Current + more <- passed + + if more then + let! ok = e.MoveNextAsync() + more <- ok + + if more then + // yield the rest (this ensures we skip 1 or more) + while! e.MoveNextAsync() do + yield e.Current + } + // Consider turning using an F# version of this instead? // https://github.com/i3arnon/ConcurrentHashSet type ConcurrentHashSet<'T when 'T: equality>(ct) = From 9a48e58d8eacd295d8116a967ab80ed2d7a33ef7 Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Thu, 21 Dec 2023 03:23:43 +0100 Subject: [PATCH 02/13] A slightly modified implementation based on test experiences --- src/FSharp.Control.TaskSeq/TaskSeqInternal.fs | 39 ++++++++++++------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs index 8dba6ffe..99b66add 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs +++ b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs @@ -817,34 +817,43 @@ module internal TaskSeqInternal = yield e.Current | Exclusive, PredicateAsync predicate -> // skipWhileAsync - while more do - let! passed = predicate e.Current - more <- passed + let mutable cont = true + if more then + let! ok = predicate e.Current + cont <- ok - if more then - let! ok = e.MoveNextAsync() - more <- ok + while more && cont do + let! moveNext = e.MoveNextAsync() + if moveNext then + let! ok = predicate e.Current + cont <- ok + + more <- moveNext - // yield the rest if more then // yield the last one where the predicate was false // (this ensures we skip 0 or more) yield e.Current - while! e.MoveNextAsync() do + while! e.MoveNextAsync() do // get the rest yield e.Current | Inclusive, PredicateAsync predicate -> // skipWhileInclusiveAsync - while more do - let! passed = predicate e.Current - more <- passed + let mutable cont = true + if more then + let! ok = predicate e.Current + cont <- ok - if more then - let! ok = e.MoveNextAsync() - more <- ok + while more && cont do + let! moveNext = e.MoveNextAsync() + if moveNext then + let! ok = predicate e.Current + cont <- ok + + more <- moveNext if more then - // yield the rest (this ensures we skip 1 or more) + // get the rest, this gives 1 or more semantics while! e.MoveNextAsync() do yield e.Current } From 27be66e95b100e219fd755a081cdcd60534fd420 Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Thu, 21 Dec 2023 03:24:12 +0100 Subject: [PATCH 03/13] Add tests for `skipWhile` and `skipWhileInclusive` and async variants --- .../FSharp.Control.TaskSeq.Test.fsproj | 1 + .../TaskSeq.SkipWhile.Tests.fs | 276 ++++++++++++++++++ .../TaskSeq.TakeWhile.Tests.fs | 4 + 3 files changed, 281 insertions(+) create mode 100644 src/FSharp.Control.TaskSeq.Test/TaskSeq.SkipWhile.Tests.fs diff --git a/src/FSharp.Control.TaskSeq.Test/FSharp.Control.TaskSeq.Test.fsproj b/src/FSharp.Control.TaskSeq.Test/FSharp.Control.TaskSeq.Test.fsproj index 16ba1b70..3536363d 100644 --- a/src/FSharp.Control.TaskSeq.Test/FSharp.Control.TaskSeq.Test.fsproj +++ b/src/FSharp.Control.TaskSeq.Test/FSharp.Control.TaskSeq.Test.fsproj @@ -36,6 +36,7 @@ + diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.SkipWhile.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.SkipWhile.Tests.fs new file mode 100644 index 00000000..899043f4 --- /dev/null +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.SkipWhile.Tests.fs @@ -0,0 +1,276 @@ +module TaskSeq.Tests.skipWhile + +open System + +open Xunit +open FsUnit.Xunit + +open FSharp.Control + +// +// TaskSeq.skipWhile +// TaskSeq.skipWhileAsync +// TaskSeq.skipWhileInclusive +// TaskSeq.skipWhileInclusiveAsync +// + +[] +module With = + /// The only real difference in semantics between the base and the *Inclusive variant lies in whether the final item is returned. + /// NOTE the semantics are very clear on only propagating a single failing item in the inclusive case. + let getFunction inclusive isAsync = + match inclusive, isAsync with + | false, false -> TaskSeq.skipWhile + | false, true -> fun pred -> TaskSeq.skipWhileAsync (pred >> Task.fromResult) + | true, false -> TaskSeq.skipWhileInclusive + | true, true -> fun pred -> TaskSeq.skipWhileInclusiveAsync (pred >> Task.fromResult) + + /// This is the base condition as one would expect in actual code + let inline cond x = x <> 6 + + /// For each of the tests below, we add a guard that will trigger if the predicate is passed items known to be beyond the + /// first failing item in the known sequence (which is 1..10) + let inline condWithGuard x = + let res = cond x + + if x > 6 then + failwith "Test sequence should not be enumerated beyond the first item failing the predicate" + + res + +module EmptySeq = + + // TaskSeq-skipWhile+A stands for: + // skipWhile + skipWhileAsync etc. + + [)>] + let ``TaskSeq-skipWhile+A has no effect`` variant = task { + do! + Gen.getEmptyVariant variant + |> TaskSeq.skipWhile ((=) 12) + |> verifyEmpty + + do! + Gen.getEmptyVariant variant + |> TaskSeq.skipWhileAsync ((=) 12 >> Task.fromResult) + |> verifyEmpty + } + + [)>] + let ``TaskSeq-skipWhileInclusive+A has no effect`` variant = task { + do! + Gen.getEmptyVariant variant + |> TaskSeq.skipWhileInclusive ((=) 12) + |> verifyEmpty + + do! + Gen.getEmptyVariant variant + |> TaskSeq.skipWhileInclusiveAsync ((=) 12 >> Task.fromResult) + |> verifyEmpty + } + +module Immutable = + + // TaskSeq-skipWhile+A stands for: + // skipWhile + skipWhileAsync etc. + + [)>] + let ``TaskSeq-skipWhile+A filters correctly`` variant = task { + do! + Gen.getSeqImmutable variant + |> TaskSeq.skipWhile ((>) 5) // skip while less than 5 + |> verifyDigitsAsString "EFGHIJ" + + do! + Gen.getSeqImmutable variant + |> TaskSeq.skipWhileAsync (fun x -> task { return x < 5 }) + |> verifyDigitsAsString "EFGHIJ" + } + + [)>] + let ``TaskSeq-skipWhile+A does not skip first item when false`` variant = task { + do! + Gen.getSeqImmutable variant + |> TaskSeq.skipWhile ((=) 0) + |> verifyDigitsAsString "ABCDEFGHIJ" // all 10 remain! + + do! + Gen.getSeqImmutable variant + |> TaskSeq.skipWhileAsync ((=) 0 >> Task.fromResult) + |> verifyDigitsAsString "ABCDEFGHIJ" // all 10 remain! + } + + [)>] + let ``TaskSeq-skipWhileInclusive+A filters correctly`` variant = task { + do! + Gen.getSeqImmutable variant + |> TaskSeq.skipWhileInclusive ((>) 5) + |> verifyDigitsAsString "GHIJ" // last 4 + + do! + Gen.getSeqImmutable variant + |> TaskSeq.skipWhileInclusiveAsync (fun x -> task { return x < 5 }) + |> verifyDigitsAsString "GHIJ" + } + + + [)>] + let ``TaskSeq-skipWhileInclusive+A returns the empty sequence if always true`` variant = task { + do! + Gen.getSeqImmutable variant + |> TaskSeq.skipWhileInclusive ((<) -1) + |> verifyEmpty + + do! + Gen.getSeqImmutable variant + |> TaskSeq.skipWhileInclusiveAsync (fun x -> task { return true }) + |> verifyEmpty + } + + [)>] + let ``TaskSeq-skipWhileInclusive+A always skips at least the first item`` variant = task { + do! + Gen.getSeqImmutable variant + |> TaskSeq.skipWhileInclusive ((=) 0) + |> verifyDigitsAsString "BCDEFGHIJ" + + do! + Gen.getSeqImmutable variant + |> TaskSeq.skipWhileInclusiveAsync ((=) 0 >> Task.fromResult) + |> verifyDigitsAsString "BCDEFGHIJ" + } + +module SideEffects = + [)>] + let ``TaskSeq-skipWhile filters correctly`` variant = + Gen.getSeqWithSideEffect variant + |> TaskSeq.skipWhile condWithGuard + |> verifyDigitsAsString "ABCDE" + + [)>] + let ``TaskSeq-skipWhileAsync filters correctly`` variant = + Gen.getSeqWithSideEffect variant + |> TaskSeq.skipWhileAsync (fun x -> task { return condWithGuard x }) + |> verifyDigitsAsString "ABCDE" + + [] + [] + [] + [] + [] + let ``TaskSeq-skipWhileXXX prove it does not read beyond the failing yield`` (inclusive, isAsync) = task { + let mutable x = 42 // for this test, the potential mutation should not actually occur + let functionToTest = getFunction inclusive isAsync ((=) 42) + + let items = taskSeq { + yield x // Always passes the test; always returned + yield x * 2 // the failing item (which will also be yielded in the result when using *Inclusive) + x <- x + 1 // we are proving we never get here + } + + let expected = if inclusive then [| 42; 84 |] else [| 42 |] + + let! first = items |> functionToTest |> TaskSeq.toArrayAsync + let! repeat = items |> functionToTest |> TaskSeq.toArrayAsync + + first |> should equal expected + repeat |> should equal expected + x |> should equal 42 + } + + [] + [] + [] + [] + [] + let ``TaskSeq-skipWhileXXX prove side effects are executed`` (inclusive, isAsync) = task { + let mutable x = 41 + let functionToTest = getFunction inclusive isAsync ((>) 50) + + let items = taskSeq { + x <- x + 1 + yield x + x <- x + 2 + yield x * 2 + x <- x + 200 // as previously proven, we should not trigger this + } + + let expectedFirst = if inclusive then [| 42; 44 * 2 |] else [| 42 |] + let expectedRepeat = if inclusive then [| 45; 47 * 2 |] else [| 45 |] + + let! first = items |> functionToTest |> TaskSeq.toArrayAsync + x |> should equal 44 + let! repeat = items |> functionToTest |> TaskSeq.toArrayAsync + x |> should equal 47 + + first |> should equal expectedFirst + repeat |> should equal expectedRepeat + } + + [)>] + let ``TaskSeq-skipWhile consumes the prefix of a longer sequence, with mutation`` variant = task { + let ts = Gen.getSeqWithSideEffect variant + + let! first = + TaskSeq.skipWhile (fun x -> x < 5) ts + |> TaskSeq.toArrayAsync + + let expected = [| 1..4 |] + first |> should equal expected + + // side effect, reiterating causes it to resume from where we left it (minus the failing item) + let! repeat = + TaskSeq.skipWhile (fun x -> x < 5) ts + |> TaskSeq.toArrayAsync + + repeat |> should not' (equal expected) + } + + [)>] + let ``TaskSeq-skipWhileInclusiveAsync consumes the prefix for a longer sequence, with mutation`` variant = task { + let ts = Gen.getSeqWithSideEffect variant + + let! first = + TaskSeq.skipWhileInclusiveAsync (fun x -> task { return x < 5 }) ts + |> TaskSeq.toArrayAsync + + let expected = [| 1..5 |] + first |> should equal expected + + // side effect, reiterating causes it to resume from where we left it (minus the failing item) + let! repeat = + TaskSeq.skipWhileInclusiveAsync (fun x -> task { return x < 5 }) ts + |> TaskSeq.toArrayAsync + + repeat |> should not' (equal expected) + } + +module Other = + [] + [] + [] + [] + [] + let ``TaskSeq-skipWhileXXX exclude all items after predicate fails`` (inclusive, isAsync) = + let functionToTest = With.getFunction inclusive isAsync + + [ 1; 2; 2; 3; 3; 2; 1 ] + |> TaskSeq.ofSeq + |> functionToTest (fun x -> x <= 2) + |> verifyDigitsAsString (if inclusive then "ABBC" else "ABB") + + [] + [] + [] + [] + [] + let ``TaskSeq-skipWhileXXX stops consuming after predicate fails`` (inclusive, isAsync) = + let functionToTest = With.getFunction inclusive isAsync + + seq { + yield! [ 1; 2; 2; 3; 3 ] + yield failwith "Too far" + } + |> TaskSeq.ofSeq + |> functionToTest (fun x -> x <= 2) + |> verifyDigitsAsString (if inclusive then "ABBC" else "ABB") diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.TakeWhile.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.TakeWhile.Tests.fs index a68e29ea..fca8d08f 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.TakeWhile.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.TakeWhile.Tests.fs @@ -39,6 +39,10 @@ module With = res module EmptySeq = + + // TaskSeq-takeWhile+A stands for: + // takeWhile + takeWhileAsync etc. + [)>] let ``TaskSeq-takeWhile+A has no effect`` variant = task { do! From 8eb270c52e9efb8ed417c7e1ccefbc56cf7b550b Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Thu, 21 Dec 2023 14:24:17 +0100 Subject: [PATCH 04/13] Formatting --- src/FSharp.Control.TaskSeq/TaskSeqInternal.fs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs index 99b66add..f52104b2 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs +++ b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs @@ -818,12 +818,14 @@ module internal TaskSeqInternal = | Exclusive, PredicateAsync predicate -> // skipWhileAsync let mutable cont = true + if more then let! ok = predicate e.Current cont <- ok while more && cont do let! moveNext = e.MoveNextAsync() + if moveNext then let! ok = predicate e.Current cont <- ok @@ -840,12 +842,14 @@ module internal TaskSeqInternal = | Inclusive, PredicateAsync predicate -> // skipWhileInclusiveAsync let mutable cont = true + if more then let! ok = predicate e.Current cont <- ok while more && cont do let! moveNext = e.MoveNextAsync() + if moveNext then let! ok = predicate e.Current cont <- ok @@ -853,7 +857,7 @@ module internal TaskSeqInternal = more <- moveNext if more then - // get the rest, this gives 1 or more semantics + // get the rest, this gives 1 or more semantics while! e.MoveNextAsync() do yield e.Current } From 033731d6cccf7fa90244189d69eed54e6054468c Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Thu, 21 Dec 2023 15:10:40 +0100 Subject: [PATCH 05/13] Improve existing `takeWhile` tests a bit --- .../TaskSeq.TakeWhile.Tests.fs | 35 ++++++++++++++----- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.TakeWhile.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.TakeWhile.Tests.fs index fca8d08f..19cf1eaa 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.TakeWhile.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.TakeWhile.Tests.fs @@ -128,16 +128,30 @@ module Immutable = module SideEffects = [)>] - let ``TaskSeq-takeWhile filters correctly`` variant = - Gen.getSeqWithSideEffect variant - |> TaskSeq.takeWhile condWithGuard - |> verifyDigitsAsString "ABCDE" + let ``TaskSeq-takeWhile+A filters correctly`` variant = task { + do! + Gen.getSeqWithSideEffect variant + |> TaskSeq.takeWhile condWithGuard + |> verifyDigitsAsString "ABCDE" + + do! + Gen.getSeqWithSideEffect variant + |> TaskSeq.takeWhileAsync (fun x -> task { return condWithGuard x }) + |> verifyDigitsAsString "ABCDE" + } [)>] - let ``TaskSeq-takeWhileAsync filters correctly`` variant = - Gen.getSeqWithSideEffect variant - |> TaskSeq.takeWhileAsync (fun x -> task { return condWithGuard x }) - |> verifyDigitsAsString "ABCDE" + let ``TaskSeq-takeWhileInclusive+A filters correctly`` variant = task { + do! + Gen.getSeqWithSideEffect variant + |> TaskSeq.takeWhileInclusive condWithGuard + |> verifyDigitsAsString "ABCDEF" + + do! + Gen.getSeqWithSideEffect variant + |> TaskSeq.takeWhileInclusiveAsync (fun x -> task { return condWithGuard x }) + |> verifyDigitsAsString "ABCDEF" + } [] [] @@ -184,6 +198,7 @@ module SideEffects = let expectedFirst = if inclusive then [| 42; 44 * 2 |] else [| 42 |] let expectedRepeat = if inclusive then [| 45; 47 * 2 |] else [| 45 |] + x |> should equal 41 let! first = items |> functionToTest |> TaskSeq.toArrayAsync x |> should equal 44 let! repeat = items |> functionToTest |> TaskSeq.toArrayAsync @@ -205,6 +220,7 @@ module SideEffects = first |> should equal expected // side effect, reiterating causes it to resume from where we left it (minus the failing item) + // which means the original sequence has now changed due to the side effect let! repeat = TaskSeq.takeWhile (fun x -> x < 5) ts |> TaskSeq.toArrayAsync @@ -224,6 +240,7 @@ module SideEffects = first |> should equal expected // side effect, reiterating causes it to resume from where we left it (minus the failing item) + // which means the original sequence has now changed due to the side effect let! repeat = TaskSeq.takeWhileInclusiveAsync (fun x -> task { return x < 5 }) ts |> TaskSeq.toArrayAsync @@ -237,7 +254,7 @@ module Other = [] [] [] - let ``TaskSeq-takeWhileXXX exclude all items after predicate fails`` (inclusive, isAsync) = + let ``TaskSeq-takeWhileXXX should exclude all items after predicate fails`` (inclusive, isAsync) = let functionToTest = With.getFunction inclusive isAsync [ 1; 2; 2; 3; 3; 2; 1 ] From 8567f3480599b2e45f0f81fbe1b0ae85c7a77854 Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Thu, 21 Dec 2023 15:11:11 +0100 Subject: [PATCH 06/13] Add/fix more tests, some assume we do not read to end of stream, this should be fixed --- .../TaskSeq.SkipWhile.Tests.fs | 172 +++++++++++++----- 1 file changed, 124 insertions(+), 48 deletions(-) diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.SkipWhile.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.SkipWhile.Tests.fs index 899043f4..f1833c9b 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.SkipWhile.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.SkipWhile.Tests.fs @@ -16,8 +16,7 @@ open FSharp.Control [] module With = - /// The only real difference in semantics between the base and the *Inclusive variant lies in whether the final item is returned. - /// NOTE the semantics are very clear on only propagating a single failing item in the inclusive case. + /// The only real difference in semantics between the base and the *Inclusive variant lies in whether the final item is skipped. let getFunction inclusive isAsync = match inclusive, isAsync with | false, false -> TaskSeq.skipWhile @@ -25,19 +24,6 @@ module With = | true, false -> TaskSeq.skipWhileInclusive | true, true -> fun pred -> TaskSeq.skipWhileInclusiveAsync (pred >> Task.fromResult) - /// This is the base condition as one would expect in actual code - let inline cond x = x <> 6 - - /// For each of the tests below, we add a guard that will trigger if the predicate is passed items known to be beyond the - /// first failing item in the known sequence (which is 1..10) - let inline condWithGuard x = - let res = cond x - - if x > 6 then - failwith "Test sequence should not be enumerated beyond the first item failing the predicate" - - res - module EmptySeq = // TaskSeq-skipWhile+A stands for: @@ -76,6 +62,12 @@ module Immutable = [)>] let ``TaskSeq-skipWhile+A filters correctly`` variant = task { + // truth table for f(x) = x < 5 + // 1 2 3 4 5 6 7 8 9 10 + // T T T T F F F F F F (stops at first F) + // x x x x _ _ _ _ _ _ (skips exclusive) + // A B C D E F G H I J + do! Gen.getSeqImmutable variant |> TaskSeq.skipWhile ((>) 5) // skip while less than 5 @@ -102,15 +94,21 @@ module Immutable = [)>] let ``TaskSeq-skipWhileInclusive+A filters correctly`` variant = task { + // truth table for f(x) = x < 5 + // 1 2 3 4 5 6 7 8 9 10 + // T T T T F F F F F F (stops at first F) + // x x x x x _ _ _ _ _ (skips inclusively) + // A B C D E F G H I J + do! Gen.getSeqImmutable variant |> TaskSeq.skipWhileInclusive ((>) 5) - |> verifyDigitsAsString "GHIJ" // last 4 + |> verifyDigitsAsString "FGHIJ" // last 4 do! Gen.getSeqImmutable variant |> TaskSeq.skipWhileInclusiveAsync (fun x -> task { return x < 5 }) - |> verifyDigitsAsString "GHIJ" + |> verifyDigitsAsString "FGHIJ" } @@ -142,16 +140,42 @@ module Immutable = module SideEffects = [)>] - let ``TaskSeq-skipWhile filters correctly`` variant = - Gen.getSeqWithSideEffect variant - |> TaskSeq.skipWhile condWithGuard - |> verifyDigitsAsString "ABCDE" + let ``TaskSeq-skipWhile+A filters correctly`` variant = task { + // truth table for f(x) = x < 6 + // 1 2 3 4 5 6 7 8 9 10 + // T T T T T F F F F F (stops at first F) + // x x x x x _ _ _ _ _ (skips exclusively) + // A B C D E F G H I J + + do! + Gen.getSeqWithSideEffect variant + |> TaskSeq.skipWhile ((>) 6) + |> verifyDigitsAsString "FGHIJ" + + do! + Gen.getSeqWithSideEffect variant + |> TaskSeq.skipWhileAsync (fun x -> task { return x < 6 }) + |> verifyDigitsAsString "FGHIJ" + } [)>] - let ``TaskSeq-skipWhileAsync filters correctly`` variant = - Gen.getSeqWithSideEffect variant - |> TaskSeq.skipWhileAsync (fun x -> task { return condWithGuard x }) - |> verifyDigitsAsString "ABCDE" + let ``TaskSeq-skipWhileInclusive+A filters correctly`` variant = task { + // truth table for f(x) = x < 6 + // 1 2 3 4 5 6 7 8 9 10 + // T T T T T F F F F F (stops at first F) + // x x x x x x _ _ _ _ (skips inclusively) + // A B C D E F G H I J + + do! + Gen.getSeqWithSideEffect variant + |> TaskSeq.skipWhileInclusive ((>) 6) + |> verifyDigitsAsString "GHIJ" + + do! + Gen.getSeqWithSideEffect variant + |> TaskSeq.skipWhileInclusiveAsync (fun x -> task { return x < 6 }) + |> verifyDigitsAsString "GHIJ" + } [] [] @@ -163,19 +187,23 @@ module SideEffects = let functionToTest = getFunction inclusive isAsync ((=) 42) let items = taskSeq { - yield x // Always passes the test; always returned - yield x * 2 // the failing item (which will also be yielded in the result when using *Inclusive) + yield x // Always passes the test; always skipped + yield x * 2 // Fails the test, skipped depending on "inclusive" x <- x + 1 // we are proving we never get here } - let expected = if inclusive then [| 42; 84 |] else [| 42 |] + // we skip one more if "inclusive" + let expected = if inclusive then [||] else [| 84 |] + x |> should equal 42 let! first = items |> functionToTest |> TaskSeq.toArrayAsync + x |> should equal 42 let! repeat = items |> functionToTest |> TaskSeq.toArrayAsync + x |> should equal 42 first |> should equal expected repeat |> should equal expected - x |> should equal 42 + x |> should equal 42 // if the var changed, we got too far } [] @@ -195,9 +223,10 @@ module SideEffects = x <- x + 200 // as previously proven, we should not trigger this } - let expectedFirst = if inclusive then [| 42; 44 * 2 |] else [| 42 |] - let expectedRepeat = if inclusive then [| 45; 47 * 2 |] else [| 45 |] + let expectedFirst = if inclusive then [||] else [| 44 * 2 |] + let expectedRepeat = if inclusive then [||] else [| 47 * 2 |] + x |> should equal 41 let! first = items |> functionToTest |> TaskSeq.toArrayAsync x |> should equal 44 let! repeat = items |> functionToTest |> TaskSeq.toArrayAsync @@ -215,10 +244,11 @@ module SideEffects = TaskSeq.skipWhile (fun x -> x < 5) ts |> TaskSeq.toArrayAsync - let expected = [| 1..4 |] + let expected = [| 5..10 |] first |> should equal expected // side effect, reiterating causes it to resume from where we left it (minus the failing item) + // which means the original sequence has now changed due to the side effect let! repeat = TaskSeq.skipWhile (fun x -> x < 5) ts |> TaskSeq.toArrayAsync @@ -234,10 +264,11 @@ module SideEffects = TaskSeq.skipWhileInclusiveAsync (fun x -> task { return x < 5 }) ts |> TaskSeq.toArrayAsync - let expected = [| 1..5 |] + let expected = [| 6..10 |] first |> should equal expected // side effect, reiterating causes it to resume from where we left it (minus the failing item) + // which means the original sequence has now changed due to the side effect let! repeat = TaskSeq.skipWhileInclusiveAsync (fun x -> task { return x < 5 }) ts |> TaskSeq.toArrayAsync @@ -251,26 +282,71 @@ module Other = [] [] [] - let ``TaskSeq-skipWhileXXX exclude all items after predicate fails`` (inclusive, isAsync) = - let functionToTest = With.getFunction inclusive isAsync + let ``TaskSeq-skipWhileXXX should include all items after predicate fails`` (inclusive, isAsync) = task { + do! + [ 1; 2; 2; 3; 3; 2; 1 ] + |> TaskSeq.ofSeq + |> TaskSeq.skipWhile (fun x -> x <= 2) + |> verifyDigitsAsString "CCBA" + + do! + [ 1; 2; 2; 3; 3; 2; 1 ] + |> TaskSeq.ofSeq + |> TaskSeq.skipWhileInclusive (fun x -> x <= 2) + |> verifyDigitsAsString "CBA" + + do! + [ 1; 2; 2; 3; 3; 2; 1 ] + |> TaskSeq.ofSeq + |> TaskSeq.skipWhileAsync (fun x -> Task.fromResult (x <= 2)) + |> verifyDigitsAsString "CCBA" - [ 1; 2; 2; 3; 3; 2; 1 ] - |> TaskSeq.ofSeq - |> functionToTest (fun x -> x <= 2) - |> verifyDigitsAsString (if inclusive then "ABBC" else "ABB") + do! + [ 1; 2; 2; 3; 3; 2; 1 ] + |> TaskSeq.ofSeq + |> TaskSeq.skipWhileInclusiveAsync (fun x -> Task.fromResult (x <= 2)) + |> verifyDigitsAsString "CBA" + } [] [] [] [] [] - let ``TaskSeq-skipWhileXXX stops consuming after predicate fails`` (inclusive, isAsync) = - let functionToTest = With.getFunction inclusive isAsync + let ``TaskSeq-skipWhileXXX stops consuming after predicate fails`` (inclusive, isAsync) = task { + do! + seq { + yield! [ 1; 2; 2; 3; 3 ] + yield failwith "Too far" + } + |> TaskSeq.ofSeq + |> TaskSeq.skipWhile (fun x -> x <= 2) + |> verifyDigitsAsString "CC" - seq { - yield! [ 1; 2; 2; 3; 3 ] - yield failwith "Too far" - } - |> TaskSeq.ofSeq - |> functionToTest (fun x -> x <= 2) - |> verifyDigitsAsString (if inclusive then "ABBC" else "ABB") + do! + seq { + yield! [ 1; 2; 2; 3; 3 ] + yield failwith "Too far" + } + |> TaskSeq.ofSeq + |> TaskSeq.skipWhileInclusive (fun x -> x <= 2) + |> verifyDigitsAsString "C" + + do! + seq { + yield! [ 1; 2; 2; 3; 3 ] + yield failwith "Too far" + } + |> TaskSeq.ofSeq + |> TaskSeq.skipWhileAsync (fun x -> Task.fromResult (x <= 2)) + |> verifyDigitsAsString "CC" + + do! + seq { + yield! [ 1; 2; 2; 3; 3 ] + yield failwith "Too far" + } + |> TaskSeq.ofSeq + |> TaskSeq.skipWhileInclusiveAsync (fun x -> Task.fromResult (x <= 2)) + |> verifyDigitsAsString "C" + } From 17aa445c5082534a30a0f128b2b191acbfc9f6c2 Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Thu, 21 Dec 2023 15:26:50 +0100 Subject: [PATCH 07/13] Fix tests that assumed we did not "read till end", which we always do --- .../TaskSeq.SkipWhile.Tests.fs | 86 +++++++------------ 1 file changed, 33 insertions(+), 53 deletions(-) diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.SkipWhile.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.SkipWhile.Tests.fs index f1833c9b..95568ca3 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.SkipWhile.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.SkipWhile.Tests.fs @@ -14,6 +14,8 @@ open FSharp.Control // TaskSeq.skipWhileInclusiveAsync // +exception SideEffectPastEnd of string + [] module With = /// The only real difference in semantics between the base and the *Inclusive variant lies in whether the final item is skipped. @@ -182,28 +184,26 @@ module SideEffects = [] [] [] - let ``TaskSeq-skipWhileXXX prove it does not read beyond the failing yield`` (inclusive, isAsync) = task { + let ``TaskSeq-skipWhileXXX prove it reads the entire input stream`` (inclusive, isAsync) = task { let mutable x = 42 // for this test, the potential mutation should not actually occur let functionToTest = getFunction inclusive isAsync ((=) 42) let items = taskSeq { yield x // Always passes the test; always skipped yield x * 2 // Fails the test, skipped depending on "inclusive" - x <- x + 1 // we are proving we never get here + x <- x + 1 // we are proving we ALWAYS get here } - // we skip one more if "inclusive" - let expected = if inclusive then [||] else [| 84 |] - x |> should equal 42 let! first = items |> functionToTest |> TaskSeq.toArrayAsync - x |> should equal 42 + x |> should equal 43 + first |> should equal (if inclusive then [||] else [| 84 |]) + let! repeat = items |> functionToTest |> TaskSeq.toArrayAsync - x |> should equal 42 + x |> should equal 44 - first |> should equal expected - repeat |> should equal expected - x |> should equal 42 // if the var changed, we got too far + repeat + |> should equal (if inclusive then [| 86 |] else [| 43; 86 |]) } [] @@ -211,7 +211,7 @@ module SideEffects = [] [] [] - let ``TaskSeq-skipWhileXXX prove side effects are executed`` (inclusive, isAsync) = task { + let ``TaskSeq-skipWhileXXX prove side effects are properly executed`` (inclusive, isAsync) = task { let mutable x = 41 let functionToTest = getFunction inclusive isAsync ((>) 50) @@ -220,17 +220,18 @@ module SideEffects = yield x x <- x + 2 yield x * 2 - x <- x + 200 // as previously proven, we should not trigger this + x <- x + 200 // as previously proven, we should ALWAYS trigger this } - let expectedFirst = if inclusive then [||] else [| 44 * 2 |] - let expectedRepeat = if inclusive then [||] else [| 47 * 2 |] + let expectedFirst = if inclusive then [||] else [| 88 |] + let expectedRepeat = if inclusive then [| 494 |] else [| 245; 494 |] x |> should equal 41 let! first = items |> functionToTest |> TaskSeq.toArrayAsync - x |> should equal 44 + x |> should equal 244 + let! repeat = items |> functionToTest |> TaskSeq.toArrayAsync - x |> should equal 47 + x |> should equal 447 first |> should equal expectedFirst repeat |> should equal expectedRepeat @@ -313,40 +314,19 @@ module Other = [] [] [] - let ``TaskSeq-skipWhileXXX stops consuming after predicate fails`` (inclusive, isAsync) = task { - do! - seq { - yield! [ 1; 2; 2; 3; 3 ] - yield failwith "Too far" - } - |> TaskSeq.ofSeq - |> TaskSeq.skipWhile (fun x -> x <= 2) - |> verifyDigitsAsString "CC" - - do! - seq { - yield! [ 1; 2; 2; 3; 3 ] - yield failwith "Too far" - } - |> TaskSeq.ofSeq - |> TaskSeq.skipWhileInclusive (fun x -> x <= 2) - |> verifyDigitsAsString "C" - - do! - seq { - yield! [ 1; 2; 2; 3; 3 ] - yield failwith "Too far" - } - |> TaskSeq.ofSeq - |> TaskSeq.skipWhileAsync (fun x -> Task.fromResult (x <= 2)) - |> verifyDigitsAsString "CC" - - do! - seq { - yield! [ 1; 2; 2; 3; 3 ] - yield failwith "Too far" - } - |> TaskSeq.ofSeq - |> TaskSeq.skipWhileInclusiveAsync (fun x -> Task.fromResult (x <= 2)) - |> verifyDigitsAsString "C" - } + let ``TaskSeq-skipWhileXXX stops consuming after predicate fails`` (inclusive, isAsync) = + let testSkipper skipper = + fun () -> + seq { + yield! [ 1; 2; 2; 3; 3 ] + yield SideEffectPastEnd "Too far" |> raise + } + |> TaskSeq.ofSeq + |> skipper + |> consumeTaskSeq + |> should throwAsyncExact typeof + + do testSkipper (TaskSeq.skipWhile (fun x -> x <= 2)) + do testSkipper (TaskSeq.skipWhileInclusive (fun x -> x <= 2)) + do testSkipper (TaskSeq.skipWhileAsync (fun x -> Task.fromResult (x <= 2))) + do testSkipper (TaskSeq.skipWhileInclusiveAsync (fun x -> Task.fromResult (x <= 2))) From 0aa074223041949e3282e95ee6517ebe6d64f6d8 Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Thu, 21 Dec 2023 15:48:35 +0100 Subject: [PATCH 08/13] Clarify test use-cases by using clearer syntax --- .../TaskSeq.SkipWhile.Tests.fs | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.SkipWhile.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.SkipWhile.Tests.fs index 95568ca3..59cc871e 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.SkipWhile.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.SkipWhile.Tests.fs @@ -72,7 +72,7 @@ module Immutable = do! Gen.getSeqImmutable variant - |> TaskSeq.skipWhile ((>) 5) // skip while less than 5 + |> TaskSeq.skipWhile (fun x -> x < 5) |> verifyDigitsAsString "EFGHIJ" do! @@ -104,7 +104,7 @@ module Immutable = do! Gen.getSeqImmutable variant - |> TaskSeq.skipWhileInclusive ((>) 5) + |> TaskSeq.skipWhileInclusive (fun x -> x < 5) |> verifyDigitsAsString "FGHIJ" // last 4 do! @@ -118,12 +118,12 @@ module Immutable = let ``TaskSeq-skipWhileInclusive+A returns the empty sequence if always true`` variant = task { do! Gen.getSeqImmutable variant - |> TaskSeq.skipWhileInclusive ((<) -1) + |> TaskSeq.skipWhileInclusive (fun x -> x > -1) // always true |> verifyEmpty do! Gen.getSeqImmutable variant - |> TaskSeq.skipWhileInclusiveAsync (fun x -> task { return true }) + |> TaskSeq.skipWhileInclusiveAsync (fun x -> Task.fromResult (x > -1)) |> verifyEmpty } @@ -151,7 +151,7 @@ module SideEffects = do! Gen.getSeqWithSideEffect variant - |> TaskSeq.skipWhile ((>) 6) + |> TaskSeq.skipWhile (fun x -> x < 6) |> verifyDigitsAsString "FGHIJ" do! @@ -170,7 +170,7 @@ module SideEffects = do! Gen.getSeqWithSideEffect variant - |> TaskSeq.skipWhileInclusive ((>) 6) + |> TaskSeq.skipWhileInclusive (fun x -> x < 6) |> verifyDigitsAsString "GHIJ" do! @@ -213,7 +213,7 @@ module SideEffects = [] let ``TaskSeq-skipWhileXXX prove side effects are properly executed`` (inclusive, isAsync) = task { let mutable x = 41 - let functionToTest = getFunction inclusive isAsync ((>) 50) + let functionToTest = getFunction inclusive isAsync (fun x -> x < 50) let items = taskSeq { x <- x + 1 @@ -326,7 +326,7 @@ module Other = |> consumeTaskSeq |> should throwAsyncExact typeof - do testSkipper (TaskSeq.skipWhile (fun x -> x <= 2)) - do testSkipper (TaskSeq.skipWhileInclusive (fun x -> x <= 2)) - do testSkipper (TaskSeq.skipWhileAsync (fun x -> Task.fromResult (x <= 2))) - do testSkipper (TaskSeq.skipWhileInclusiveAsync (fun x -> Task.fromResult (x <= 2))) + testSkipper (TaskSeq.skipWhile (fun x -> x <= 2)) + testSkipper (TaskSeq.skipWhileInclusive (fun x -> x <= 2)) + testSkipper (TaskSeq.skipWhileAsync (fun x -> Task.fromResult (x <= 2))) + testSkipper (TaskSeq.skipWhileInclusiveAsync (fun x -> Task.fromResult (x <= 2))) From 565f3f557b2c4e1d079ea5078f293bb5f0d5def1 Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Thu, 21 Dec 2023 18:27:20 +0100 Subject: [PATCH 09/13] Apply review comments: redesign two tests to better test edge cases and for clarity (tx @bartelink) --- .../TaskSeq.SkipWhile.Tests.fs | 85 ++++++++----------- 1 file changed, 34 insertions(+), 51 deletions(-) diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.SkipWhile.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.SkipWhile.Tests.fs index 59cc871e..03e05243 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.SkipWhile.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.SkipWhile.Tests.fs @@ -16,15 +16,6 @@ open FSharp.Control exception SideEffectPastEnd of string -[] -module With = - /// The only real difference in semantics between the base and the *Inclusive variant lies in whether the final item is skipped. - let getFunction inclusive isAsync = - match inclusive, isAsync with - | false, false -> TaskSeq.skipWhile - | false, true -> fun pred -> TaskSeq.skipWhileAsync (pred >> Task.fromResult) - | true, false -> TaskSeq.skipWhileInclusive - | true, true -> fun pred -> TaskSeq.skipWhileInclusiveAsync (pred >> Task.fromResult) module EmptySeq = @@ -179,41 +170,37 @@ module SideEffects = |> verifyDigitsAsString "GHIJ" } - [] - [] - [] - [] - [] - let ``TaskSeq-skipWhileXXX prove it reads the entire input stream`` (inclusive, isAsync) = task { - let mutable x = 42 // for this test, the potential mutation should not actually occur - let functionToTest = getFunction inclusive isAsync ((=) 42) + [] + let ``TaskSeq-skipWhile and variants prove it reads the entire input stream`` () = task { + let mutable x = 42 let items = taskSeq { - yield x // Always passes the test; always skipped - yield x * 2 // Fails the test, skipped depending on "inclusive" + yield x + yield x * 2 x <- x + 1 // we are proving we ALWAYS get here } - x |> should equal 42 - let! first = items |> functionToTest |> TaskSeq.toArrayAsync + let testSkipper skipper expected = task { + let! first = items |> skipper |> TaskSeq.toArrayAsync + return first |> should equal expected + } + + do! testSkipper (TaskSeq.skipWhile ((=) 42)) [| 84 |] x |> should equal 43 - first |> should equal (if inclusive then [||] else [| 84 |]) - let! repeat = items |> functionToTest |> TaskSeq.toArrayAsync + do! testSkipper (TaskSeq.skipWhileInclusive ((=) 43)) [||] x |> should equal 44 - repeat - |> should equal (if inclusive then [| 86 |] else [| 43; 86 |]) + do! testSkipper (TaskSeq.skipWhileAsync (fun x -> Task.fromResult (x = 44))) [| 88 |] + x |> should equal 45 + + do! testSkipper (TaskSeq.skipWhileInclusiveAsync (fun x -> Task.fromResult (x = 45))) [||] + x |> should equal 46 } - [] - [] - [] - [] - [] - let ``TaskSeq-skipWhileXXX prove side effects are properly executed`` (inclusive, isAsync) = task { + [] + let ``TaskSeq-skipWhile and variants prove side effects are properly executed`` () = task { let mutable x = 41 - let functionToTest = getFunction inclusive isAsync (fun x -> x < 50) let items = taskSeq { x <- x + 1 @@ -223,18 +210,22 @@ module SideEffects = x <- x + 200 // as previously proven, we should ALWAYS trigger this } - let expectedFirst = if inclusive then [||] else [| 88 |] - let expectedRepeat = if inclusive then [| 494 |] else [| 245; 494 |] + let testSkipper skipper expected = task { + let! first = items |> skipper |> TaskSeq.toArrayAsync + return first |> should equal expected + } - x |> should equal 41 - let! first = items |> functionToTest |> TaskSeq.toArrayAsync + do! testSkipper (TaskSeq.skipWhile ((=) 42)) [| 88 |] x |> should equal 244 - let! repeat = items |> functionToTest |> TaskSeq.toArrayAsync + do! testSkipper (TaskSeq.skipWhileInclusive ((=) 245)) [||] x |> should equal 447 - first |> should equal expectedFirst - repeat |> should equal expectedRepeat + do! testSkipper (TaskSeq.skipWhileAsync (fun x -> Task.fromResult (x = 448))) [| 900 |] + x |> should equal 650 + + do! testSkipper (TaskSeq.skipWhileInclusiveAsync (fun x -> Task.fromResult (x = 651))) [||] + x |> should equal 853 } [)>] @@ -278,12 +269,8 @@ module SideEffects = } module Other = - [] - [] - [] - [] - [] - let ``TaskSeq-skipWhileXXX should include all items after predicate fails`` (inclusive, isAsync) = task { + [] + let ``TaskSeq-skipWhileXXX should include all items after predicate fails`` () = task { do! [ 1; 2; 2; 3; 3; 2; 1 ] |> TaskSeq.ofSeq @@ -309,12 +296,8 @@ module Other = |> verifyDigitsAsString "CBA" } - [] - [] - [] - [] - [] - let ``TaskSeq-skipWhileXXX stops consuming after predicate fails`` (inclusive, isAsync) = + [] + let ``TaskSeq-skipWhileXXX stops consuming after predicate fails`` () = let testSkipper skipper = fun () -> seq { From ef920c1e45df6a910a12727f9c79150cffce6048 Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Thu, 21 Dec 2023 18:30:58 +0100 Subject: [PATCH 10/13] Rename function names --- .../TaskSeq.TakeWhile.Tests.fs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.TakeWhile.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.TakeWhile.Tests.fs index 19cf1eaa..81fc3bf4 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.TakeWhile.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.TakeWhile.Tests.fs @@ -158,7 +158,7 @@ module SideEffects = [] [] [] - let ``TaskSeq-takeWhileXXX prove it does not read beyond the failing yield`` (inclusive, isAsync) = task { + let ``TaskSeq-takeWhile and variants prove it does not read beyond the failing yield`` (inclusive, isAsync) = task { let mutable x = 42 // for this test, the potential mutation should not actually occur let functionToTest = getFunction inclusive isAsync ((=) 42) @@ -183,7 +183,7 @@ module SideEffects = [] [] [] - let ``TaskSeq-takeWhileXXX prove side effects are executed`` (inclusive, isAsync) = task { + let ``TaskSeq-takeWhile and variants prove side effects are executed`` (inclusive, isAsync) = task { let mutable x = 41 let functionToTest = getFunction inclusive isAsync ((>) 50) @@ -254,7 +254,7 @@ module Other = [] [] [] - let ``TaskSeq-takeWhileXXX should exclude all items after predicate fails`` (inclusive, isAsync) = + let ``TaskSeq-takeWhile and variants excludes all items after predicate fails`` (inclusive, isAsync) = let functionToTest = With.getFunction inclusive isAsync [ 1; 2; 2; 3; 3; 2; 1 ] @@ -267,7 +267,7 @@ module Other = [] [] [] - let ``TaskSeq-takeWhileXXX stops consuming after predicate fails`` (inclusive, isAsync) = + let ``TaskSeq-takeWhile and variants stops consuming after predicate fails`` (inclusive, isAsync) = let functionToTest = With.getFunction inclusive isAsync seq { From 708a4b65decbfa00d444a5136cbb9d638e8ea59e Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Thu, 21 Dec 2023 18:55:44 +0100 Subject: [PATCH 11/13] Apply review comments, small clarifications/code improvements, tx @bartelink --- src/FSharp.Control.TaskSeq/TaskSeq.fsi | 2 +- src/FSharp.Control.TaskSeq/TaskSeqInternal.fs | 52 +++++++++---------- 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/src/FSharp.Control.TaskSeq/TaskSeq.fsi b/src/FSharp.Control.TaskSeq/TaskSeq.fsi index 4705d370..c34895d2 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeq.fsi +++ b/src/FSharp.Control.TaskSeq/TaskSeq.fsi @@ -889,7 +889,7 @@ type TaskSeq = /// Returns a task sequence that, when iterated, skips elements of the underlying sequence while the /// given asynchronous function returns , and then yields the /// remaining elements. The first element where the predicate returns is returned, which - /// means that this function will skip 0 or more elements (see also ). + /// means that this function will skip 0 or more elements (see also ). /// If is synchronous, consider using . /// /// diff --git a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs index f52104b2..e59d50c3 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs +++ b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs @@ -13,7 +13,7 @@ type internal AsyncEnumStatus = [] type internal WhileKind = - /// The item under test is included (or skipped) even if false + /// The item under test is included (or skipped) even when the predicate returns false | Inclusive /// The item under test is always excluded (or not skipped) | Exclusive @@ -648,9 +648,9 @@ module internal TaskSeqInternal = use e = source.GetAsyncEnumerator CancellationToken.None for _ in 1..count do - let! ok = e.MoveNextAsync() + let! hasMore = e.MoveNextAsync() - if not ok then + if not hasMore then raiseInsufficient () while! e.MoveNextAsync() do @@ -731,8 +731,8 @@ module internal TaskSeqInternal = taskSeq { use e = source.GetAsyncEnumerator CancellationToken.None - let! moveFirst = e.MoveNextAsync() - let mutable more = moveFirst + let! notEmpty = e.MoveNextAsync() + let mutable more = notEmpty match whileKind, predicate with | Exclusive, Predicate predicate -> // takeWhile @@ -743,20 +743,20 @@ module internal TaskSeqInternal = if more then // yield ONLY if predicate is true yield value - let! ok = e.MoveNextAsync() - more <- ok + let! hasMore = e.MoveNextAsync() + more <- hasMore | Inclusive, Predicate predicate -> // takeWhileInclusive while more do let value = e.Current more <- predicate value - // yield, regardless of result of predicate + // yield regardless of result of predicate yield value if more then - let! ok = e.MoveNextAsync() - more <- ok + let! hasMore = e.MoveNextAsync() + more <- hasMore | Exclusive, PredicateAsync predicate -> // takeWhileAsync while more do @@ -767,8 +767,8 @@ module internal TaskSeqInternal = if more then // yield ONLY if predicate is true yield value - let! ok = e.MoveNextAsync() - more <- ok + let! hasMore = e.MoveNextAsync() + more <- hasMore | Inclusive, PredicateAsync predicate -> // takeWhileInclusiveAsync while more do @@ -780,8 +780,8 @@ module internal TaskSeqInternal = yield value if more then - let! ok = e.MoveNextAsync() - more <- ok + let! hasMore = e.MoveNextAsync() + more <- hasMore } let skipWhile whileKind predicate (source: TaskSeq<_>) = @@ -795,8 +795,8 @@ module internal TaskSeqInternal = match whileKind, predicate with | Exclusive, Predicate predicate -> // skipWhile while more && predicate e.Current do - let! ok = e.MoveNextAsync() - more <- ok + let! hasMore = e.MoveNextAsync() + more <- hasMore if more then // yield the last one where the predicate was false @@ -808,8 +808,8 @@ module internal TaskSeqInternal = | Inclusive, Predicate predicate -> // skipWhileInclusive while more && predicate e.Current do - let! ok = e.MoveNextAsync() - more <- ok + let! hasMore = e.MoveNextAsync() + more <- hasMore if more then // yield the rest (this ensures we skip 1 or more) @@ -820,15 +820,15 @@ module internal TaskSeqInternal = let mutable cont = true if more then - let! ok = predicate e.Current - cont <- ok + let! hasMore = predicate e.Current + cont <- hasMore while more && cont do let! moveNext = e.MoveNextAsync() if moveNext then - let! ok = predicate e.Current - cont <- ok + let! hasMore = predicate e.Current + cont <- hasMore more <- moveNext @@ -844,15 +844,15 @@ module internal TaskSeqInternal = let mutable cont = true if more then - let! ok = predicate e.Current - cont <- ok + let! hasMore = predicate e.Current + cont <- hasMore while more && cont do let! moveNext = e.MoveNextAsync() if moveNext then - let! ok = predicate e.Current - cont <- ok + let! hasMore = predicate e.Current + cont <- hasMore more <- moveNext From faf18414128eeb43d4c7b2a5798828186fa752a2 Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Thu, 21 Dec 2023 20:14:01 +0100 Subject: [PATCH 12/13] Update `readme.md`, package info and release notes --- README.md | 5 +++-- assets/nuget-package-readme.md | 5 +++-- release-notes.txt | 7 ++++--- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index d8b23491..a3c499e6 100644 --- a/README.md +++ b/README.md @@ -319,8 +319,8 @@ This is what has been implemented so far, is planned or skipped: | ✅ [#90][] | `singleton` | `singleton` | | | | ✅ [#209][]| `skip` | `skip` | | | | ✅ [#209][]| | `drop` | | | -| | `skipWhile` | `skipWhile` | `skipWhileAsync` | | -| | | `skipWhileInclusive` | `skipWhileInclusiveAsync` | | +| ✅ [#219][]| `skipWhile` | `skipWhile` | `skipWhileAsync` | | +| ✅ [#219][]| | `skipWhileInclusive` | `skipWhileInclusiveAsync` | | | ❓ | `sort` | | | [note #1](#note1 "These functions require a form of pre-materializing through 'TaskSeq.cache', similar to the approach taken in the corresponding 'Seq' functions. It doesn't make much sense to have a cached async sequence. However, 'AsyncSeq' does implement these, so we'll probably do so eventually as well.") | | ❓ | `sortBy` | | | [note #1](#note1 "These functions require a form of pre-materializing through 'TaskSeq.cache', similar to the approach taken in the corresponding 'Seq' functions. It doesn't make much sense to have a cached async sequence. However, 'AsyncSeq' does implement these, so we'll probably do so eventually as well.") | | ❓ | `sortByAscending` | | | [note #1](#note1 "These functions require a form of pre-materializing through 'TaskSeq.cache', similar to the approach taken in the corresponding 'Seq' functions. It doesn't make much sense to have a cached async sequence. However, 'AsyncSeq' does implement these, so we'll probably do so eventually as well.") | @@ -603,6 +603,7 @@ module TaskSeq = [#133]: https://github.com/fsprojects/FSharp.Control.TaskSeq/issues/133 [#209]: https://github.com/fsprojects/FSharp.Control.TaskSeq/issues/209 [#217]: https://github.com/fsprojects/FSharp.Control.TaskSeq/issues/217 +[#219]: https://github.com/fsprojects/FSharp.Control.TaskSeq/issues/219 [issues]: https://github.com/fsprojects/FSharp.Control.TaskSeq/issues [nuget]: https://www.nuget.org/packages/FSharp.Control.TaskSeq/ diff --git a/assets/nuget-package-readme.md b/assets/nuget-package-readme.md index 1a7651e6..1e1a7f84 100644 --- a/assets/nuget-package-readme.md +++ b/assets/nuget-package-readme.md @@ -199,8 +199,8 @@ This is what has been implemented so far, is planned or skipped: | ✅ [#90][] | `singleton` | `singleton` | | | | ✅ [#209][]| `skip` | `skip` | | | | ✅ [#209][]| | `drop` | | | -| | `skipWhile` | `skipWhile` | `skipWhileAsync` | | -| | | `skipWhileInclusive` | `skipWhileInclusiveAsync` | | +| ✅ [#219][]| `skipWhile` | `skipWhile` | `skipWhileAsync` | | +| ✅ [#219][]| | `skipWhileInclusive` | `skipWhileInclusiveAsync` | | | ❓ | `sort` | | | [note #1](#note1 "These functions require a form of pre-materializing through 'TaskSeq.cache', similar to the approach taken in the corresponding 'Seq' functions. It doesn't make much sense to have a cached async sequence. However, 'AsyncSeq' does implement these, so we'll probably do so eventually as well.") | | ❓ | `sortBy` | | | [note #1](#note1 "These functions require a form of pre-materializing through 'TaskSeq.cache', similar to the approach taken in the corresponding 'Seq' functions. It doesn't make much sense to have a cached async sequence. However, 'AsyncSeq' does implement these, so we'll probably do so eventually as well.") | | ❓ | `sortByAscending` | | | [note #1](#note1 "These functions require a form of pre-materializing through 'TaskSeq.cache', similar to the approach taken in the corresponding 'Seq' functions. It doesn't make much sense to have a cached async sequence. However, 'AsyncSeq' does implement these, so we'll probably do so eventually as well.") | @@ -308,3 +308,4 @@ _The motivation for `readOnly` in `Seq` is that a cast from a mutable array or l [#126]: https://github.com/fsprojects/FSharp.Control.TaskSeq/pull/126 [#209]: https://github.com/fsprojects/FSharp.Control.TaskSeq/issues/209 [#217]: https://github.com/fsprojects/FSharp.Control.TaskSeq/issues/217 +[#219]: https://github.com/fsprojects/FSharp.Control.TaskSeq/issues/219 diff --git a/release-notes.txt b/release-notes.txt index 06930db4..74096d35 100644 --- a/release-notes.txt +++ b/release-notes.txt @@ -3,9 +3,10 @@ Release notes: 0.4.x (unreleased) - overhaul all doc comments, add exceptions, improve IDE quick-info experience, #136 - new surface area functions, fixes #208: - * TaskSeq.take, TaskSeq.skip, #209 - * TaskSeq.truncate, TaskSeq.drop, #209 - * TaskSeq.where, TaskSeq.whereAsync, #217 + * TaskSeq.take, skip, #209 + * TaskSeq.truncate, drop, #209 + * TaskSeq.where, whereAsync, #217 + * TaskSeq.skipWhile, skipWhileInclusive, skipWhileAsync, skipWhileInclusiveAsync, #219 - Performance: less thread hops with 'StartImmediateAsTask' instead of 'StartAsTask', fixes #135 - BINARY INCOMPATIBILITY: 'TaskSeq' module is now static members on 'TaskSeq<_>', fixes #184 From a213a1f32344303c472a09990af8a459bec67111 Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Thu, 21 Dec 2023 22:44:05 +0100 Subject: [PATCH 13/13] Fix FS3511 "This state machine is not statically compilable" in tests --- .../TaskSeq.SkipWhile.Tests.fs | 47 +++++++++++-------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.SkipWhile.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.SkipWhile.Tests.fs index 03e05243..b0bf214e 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.SkipWhile.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.SkipWhile.Tests.fs @@ -171,7 +171,8 @@ module SideEffects = } [] - let ``TaskSeq-skipWhile and variants prove it reads the entire input stream`` () = task { + let ``TaskSeq-skipWhile and variants prove it reads the entire input stream`` () = + let mutable x = 42 let items = taskSeq { @@ -180,26 +181,29 @@ module SideEffects = x <- x + 1 // we are proving we ALWAYS get here } + // this needs to be lifted from the task or it raises the infamous + // warning FS3511 on CI: This state machine is not statically compilable let testSkipper skipper expected = task { let! first = items |> skipper |> TaskSeq.toArrayAsync return first |> should equal expected } - do! testSkipper (TaskSeq.skipWhile ((=) 42)) [| 84 |] - x |> should equal 43 + task { + do! testSkipper (TaskSeq.skipWhile ((=) 42)) [| 84 |] + x |> should equal 43 - do! testSkipper (TaskSeq.skipWhileInclusive ((=) 43)) [||] - x |> should equal 44 + do! testSkipper (TaskSeq.skipWhileInclusive ((=) 43)) [||] + x |> should equal 44 - do! testSkipper (TaskSeq.skipWhileAsync (fun x -> Task.fromResult (x = 44))) [| 88 |] - x |> should equal 45 + do! testSkipper (TaskSeq.skipWhileAsync (fun x -> Task.fromResult (x = 44))) [| 88 |] + x |> should equal 45 - do! testSkipper (TaskSeq.skipWhileInclusiveAsync (fun x -> Task.fromResult (x = 45))) [||] - x |> should equal 46 - } + do! testSkipper (TaskSeq.skipWhileInclusiveAsync (fun x -> Task.fromResult (x = 45))) [||] + x |> should equal 46 + } [] - let ``TaskSeq-skipWhile and variants prove side effects are properly executed`` () = task { + let ``TaskSeq-skipWhile and variants prove side effects are properly executed`` () = let mutable x = 41 let items = taskSeq { @@ -210,23 +214,26 @@ module SideEffects = x <- x + 200 // as previously proven, we should ALWAYS trigger this } + // this needs to be lifted from the task or it raises the infamous + // warning FS3511 on CI: This state machine is not statically compilable let testSkipper skipper expected = task { let! first = items |> skipper |> TaskSeq.toArrayAsync return first |> should equal expected } - do! testSkipper (TaskSeq.skipWhile ((=) 42)) [| 88 |] - x |> should equal 244 + task { + do! testSkipper (TaskSeq.skipWhile ((=) 42)) [| 88 |] + x |> should equal 244 - do! testSkipper (TaskSeq.skipWhileInclusive ((=) 245)) [||] - x |> should equal 447 + do! testSkipper (TaskSeq.skipWhileInclusive ((=) 245)) [||] + x |> should equal 447 - do! testSkipper (TaskSeq.skipWhileAsync (fun x -> Task.fromResult (x = 448))) [| 900 |] - x |> should equal 650 + do! testSkipper (TaskSeq.skipWhileAsync (fun x -> Task.fromResult (x = 448))) [| 900 |] + x |> should equal 650 - do! testSkipper (TaskSeq.skipWhileInclusiveAsync (fun x -> Task.fromResult (x = 651))) [||] - x |> should equal 853 - } + do! testSkipper (TaskSeq.skipWhileInclusiveAsync (fun x -> Task.fromResult (x = 651))) [||] + x |> should equal 853 + } [)>] let ``TaskSeq-skipWhile consumes the prefix of a longer sequence, with mutation`` variant = task {