Skip to content

Commit

Permalink
Regression in Arb.list?
Browse files Browse the repository at this point in the history
  • Loading branch information
@nomisRev
nomisRev committed Apr 14, 2024
1 parent 680ecf0 commit d5b188c
Showing 1 changed file with 4 additions and 1 deletion.
5 changes: 4 additions & 1 deletion arrow-libs/core/arrow-core/src/commonTest/kotlin/arrow/core/test/Generators.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import io.kotest.property.arbitrary.bind
import io.kotest.property.arbitrary.boolean
import io.kotest.property.arbitrary.choice
import io.kotest.property.arbitrary.constant
import io.kotest.property.arbitrary.filter
import io.kotest.property.arbitrary.int
import io.kotest.property.arbitrary.list
import io.kotest.property.arbitrary.set
Expand All @@ -29,7 +30,9 @@ import kotlin.coroutines.startCoroutine
// copied from kotest-extensions-arrow

fun <A> Arb.Companion.nonEmptyList(arb: Arb<A>, range: IntRange = 0 .. 100): Arb<NonEmptyList<A>> =
Arb.list(arb, max(range.first, 1) .. range.last).map { NonEmptyList(it) }
Arb.list(arb, max(range.first, 1) .. range.last)
.filter { it.isNotEmpty() }

This comment has been minimized.

Sign in to view

Copy link
@kyay10

kyay10 Apr 14, 2024 •
edited
Loading

Collaborator

Is that not covered by the start of the range being at least 1?
It might make the code intent clearer to use coerceAtLeast btw

This comment has been minimized.

Sign in to view

Copy link
@nomisRev

nomisRev Apr 22, 2024

Author Member

Hey @kyay10,
That is what I assumed as well, that's why I mentioned regression int he commit message.

Locally, adding filter { it.isNotEmpty() } fixed the issue, and I tried coerceAtLeast first but it seemed to me there was some regression on Range because I was still getting empty lists in tests.

This comment has been minimized.

Sign in to view

Copy link
@kyay10

kyay10 Apr 22, 2024

Collaborator

Oh it might maybe be because range.last might be 0. Perhaps doing max(range.last, 1) or just a very simple range.takeIf { it.isNotEmpty } ?: (1..1)

This comment has been minimized.

Sign in to view

Copy link
@kyay10

kyay10 Apr 22, 2024

Collaborator

I'll test locally and see what happens
.map { NonEmptyList(it) }

fun <A> Arb.Companion.nonEmptySet(arb: Arb<A>, range: IntRange = 0 .. 100): Arb<NonEmptySet<A>> =
Arb.set(arb, max(range.first, 1) .. range.last).map { it.toNonEmptySetOrNull()!! }
Expand Down

0 comments on commit d5b188c

Please sign in to comment.