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

Support "empty" arbitrary #335

Open
vlsi opened this issue May 19, 2022 · 2 comments
Open

Support "empty" arbitrary #335

vlsi opened this issue May 19, 2022 · 2 comments

Comments

@vlsi
Copy link
Contributor

vlsi commented May 19, 2022

Testing Problem

See #134 (comment)

Similar case:

frequency(empty array) fails with

    net.jqwik.api.JqwikException: [] does not contain any positive frequencies.
        at app//net.jqwik.engine.support.ChooseRandomlyByFrequency.<init>(ChooseRandomlyByFrequency.java:17)
        at app//net.jqwik.engine.properties.arbitraries.randomized.FrequencyGenerator.<init>(FrequencyGenerator.java:12)
        at app//net.jqwik.engine.properties.arbitraries.randomized.RandomGenerators.frequency(RandomGenerators.java:151)
        at app//net.jqwik.engine.facades.ArbitrariesFacadeImpl.randomFrequency(ArbitrariesFacadeImpl.java:106)
        at app//net.jqwik.api.Arbitraries.frequency(Arbitraries.java:318)
        at app//net.jqwik.kotlin.api.JqwikGlobalsKt.frequency(JqwikGlobals.kt:113)

Discussion

It might be helpful to explicitly support "empty arbitrary", so:

  1. combine(emptyArbitrary(), other arbitraries) yields emptyArbitrary()
  2. emptyArbitrary().map, emptyArbitrary().flatMap yields emptyArbitrary()

Pros:

  • Making the result values consistent would be nice. For instance, Arbitraries.of(Collection) accepts emptyList() just fine
  • Allowing for empty arbitrary might be a reasonable return value for State-Dependent (Ad-Hoc) Action Generator #134 instead of precondition to signal that the given action is not really applicable, and it yields zero possible transformations (== Arbitrary<Transformation<...>> is empty)

Cons:

  • In many cases users want non-empty arbitrary, so it might be helpful to just fail if the user attempts to instantiate an "empty arbitrary", so they can fix the logic. For instance, the current Arbitraries.of(emptyList()) is kind of hard to debug since the resulting arbitrary thows at the generation time only (see net.jqwik.engine.properties.arbitraries.randomized.RandomGenerators#choose(java.util.List<U>)) that happens at a completely different stacktrace

Frankly speaking, I am inclined that explicit support for empty arbitraries would be preferable.


As "non-empty" arbitrary might often be needed, I believe, it might be nice to split "empty-related" methods in two flavours.
First of all, some methods will never return empty arbitraries: Arbitraries.strings() would be non-empty.
On the other hand, Arbitraries.of(collection), Arbitraries.of(array) might be empty in case the input collection is empty.
So it might be worth providing two methods in the API:

  1. Arbitraries.of(collection). Return an arbitrary, or throw in case the collection is empty. That is basically the current method, and it matches the current behaviour.
  2. Arbitraries.ofOrEmpty(collection). Retrun an arbitrary if collection is non-empty, return "empty arbitrary" in case the collection is empty.

This idea was inspired by orNull naming pattern in Kotlin: https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.collections/first-or-null.html.
See https://twitter.com/VladimirSitnikv/status/1521899018112151553 , https://www.youtube.com/watch?v=NJjEFXeiuKY&t=58s&ab_channel=JetBrainsTV

However, if we follow orEmpty route, we might to add combineOrEmpty, Arbitrary.flatMapOrEmpty, etc which might sound too invasive.


Here's the case when I faced frequency does not contain... error.
The meaning is as follows:

  1. I need to create a table and add columns there. add table is a transformer that would add a table to the state
  2. Table definition requires some columns (1..1000 of them)

Here's the trick: all the columns must be compatible with each other. All columns should be either "multiple" or "single". However, the gotcha is that single column needs at least one single attribute in the state, and multiple column requires at least one multiple attribute in the model.

So in order to generate add table transformer I query the list of existing types of attributes (e.g. SINGLE or MULTIPLE or both), and then I select which columns to create based on that.

It turns out that frequency(empty) fails. However, if there are no attributes to support the table, I would prefer to skip add table altogether.

    context(MappingState)
    fun availableAttrMultiplicity(): EnumSet<MultiplicityFilter> {
        val attrTypes = EnumSet.noneOf(MultiplicityFilter::class.java)
        mapping.metadata.attributes
            .takeWhile { attrTypes.size < 2 }
            .forEach { attrTypes += if (it.multiple) MultiplicityFilter.MULTIPLE else MultiplicityFilter.SINGLE }
        return attrTypes
    }

    context(MappingState)
    fun `add table`() =
        combine(
            arbitraryTableName(),
            frequency(
                *availableAttrMultiplicity()
                    .map {
                        when(it) {
                            MultiplicityFilter.SINGLE -> 7 to it
                            else -> 2 to it
                        }
                    }.toTypedArray()
            ).flatMap { multiplicity ->
                arbitraryColumnName().set().ofMinSize(1).ofMaxSize(1000).flatMapEach { _, u ->
                    `add column`(just(u), multiplicity)
                }
            },
            `add object type to table`().filter({
                // Exclude should not be the first operation in the table
                it.operation != TableTypeOperation.EXCLUDE
            }, 853),
            `change is_lazy`(),
            `change is_background`(),
        ) { tableName, columnAction, addObjectType, isLazy, isBackground ->
            action<InstanceMappingScope>(
                description = { "AddTable($tableName, $columnAction, $addObjectType, $isLazy, $isBackground)" },
                validate = { tableName !in tables },
            ) {
                tables {
                    table(tableName) {
                        columnAction.forEach {
                            if (it.precondition(this)) {
                                it.invokeInline(this)
                            }
                        }
                        addObjectType.invokeInline(this)
                        isLazy.invokeInline(this)
                        isBackground.invokeInline(this)
                    }
                }
            }
        }
@vlsi
Copy link
Contributor Author

vlsi commented May 19, 2022

Here's another case. If I split single and multiple transformers, I might want to put it as follows:

    fun `add table`() =
        frequencyOf(
            1 to `add single table`(),
            1 to `add multiple table`()
        )

    fun `add single table`() =
        ...

    fun `add multiple table`() =
        ... // I wish this method could return emptyArbitrary that would be just ignored by frequencyOf above

However, add multiple table is not always possible, so I would prefer that add multiple table could return emptyArbitrary which would be ignored by frequencyOf.

Of course, I could work around that by something like the below, however propagating empty through the other methods (e.g. combine(definitelyEmpty, otherArbitrary) should yield definitelyEmpty)

    @Suppress("UNCHECKED_CAST")
    fun<T> emptyArbitrary(): Arbitrary<T> = EMPTY_ARBITRARY as Arbitrary<T>

    fun <T> Arbitrary<T>.isEmpty() = this == EMPTY_ARBITRARY
    fun <T> Arbitrary<T>.isNotEmpty() = !isEmpty()

    fun <T> frequencyOfNonEmpty(vararg frequencies: Pair<Int, Arbitrary<out T>>): Arbitrary<T> =
        frequencyOf(
            *frequencies
                .filter { it.second.isNotEmpty() }
                .ifEmpty { return emptyArbitrary() }
                .toTypedArray()
        )

// plus the similar combineNonEmpty overloads

@jlink
Copy link
Collaborator

jlink commented Jan 26, 2024

I'm thinking of introducing an empty Arbitrary in jqwik2.

The question I'm facing is: What guarantees would an arbitrary give to the property runner? A few options:

  • None. A call to the next value can succeed or not. The next call can - depending on the (random) source - produce a value or not.
  • I either produce an infinite stream of values or none at all.
  • I produce values until I don't, then I won't produce values any more.

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

2 participants