withSamples() broken? about jqwik HOT 8 CLOSED

from jqwik.

I can replicate the problem. Thanks for catching it.

from jqwik.

After rethinking the problem I've come to the conclusion that - as strange as it might sound at first - it works as designed. Bear with me, I'll explain why and what you can do to cover your intention...

Why it works as designed

The declarative way to combine, constrain and modify Arbitrary instances is supposed to specify value generators. And, that's the crucial part, generators are created freshly for each try, i.e. for each invocation of a property/test method. That's why

Arbitraries.bigDecimals().withSamples(new BigDecimal("0.00"))

will always generate 0.00 when asked the first time within a single try. Thus it does not really make sense to use withExamples() in the way you did. It would make more sense, e.g., to generate a list of numbers with it:

Arbitraries.bigDecimals().withSamples(new BigDecimal("0.00")).list().ofSize(3)

will generate a list of three numbers, the first element of which would always be 0.00 and the others random. TLDR: withSamples() is usually used to derive or feed other generators.

What you can do

I see 4 options

1. Do nothing

By default edge cases for common generators, e.g. BigDecimal generator, are injected with a certain probability. As a response to this issue report I raised the probability for 0 to be injected somewhat. This will take effect starting in version 1.1.5.

Moreover, whenever jqwik discovers a failing property shrinking usually will try 0 as one of the first options and shrink to it.

2. Add an explicit example test

@Example
void shouldTestRandomBigDecimals_Zero() {
	shouldTestRandomBigDecimals(new BigDecimal("0.00"));
}

@Property
void shouldTestRandomBigDecimals(@ForAll BigDecimal v) {
	System.out.println(v);
}

3. Add a data-driven property

@Property
@FromData("bigDecimalExamples")
void shouldTestRandomBigDecimals_Examples(@ForAll BigDecimal bigDecimal) {
	shouldTestRandomBigDecimals(bigDecimal);
}

@Data
Iterable<Tuple.Tuple1<BigDecimal>> bigDecimalExamples() {
	return Table.of(
		new BigDecimal("0.00"),
		new BigDecimal("0.01"),
		new BigDecimal("42.01")
	);
}

@Property
void shouldTestRandomBigDecimals(@ForAll BigDecimal v) {
	System.out.println(v);
}

See https://jqwik.net/docs/current/user-guide.html#data-driven-properties for more details about that feature.

4. Open a feature request to make option 3 a bit easier

Something like @WithExamples could simplify the scenarion above:

@Data
Iterable<Tuple.Tuple1<BigDecimal>> bigDecimalExamples() {
	return Table.of(
		new BigDecimal("0.00"),
		new BigDecimal("0.01"),
		new BigDecimal("42.01")
	);
}

@Property
@WithExamples("bigDecimalExamples")
void shouldTestRandomBigDecimals(@ForAll BigDecimal v) {
	System.out.println(v);
}

I hope I could explain and give reasonable options. Feel free to reopen the issue if anything's not clear.

from jqwik.

One note from me, javadoc for withSamples states:

Create a new arbitrary of the same type but inject values in samples first before continuing with standard value generation.
(net.jqwik:jqwik-api:1.2.1)

It does evoke in me, that before generating some random things, provided sample values will be tested at first. This does not happen. All tests use just single value (first from withSamples list).

I would suggest to change javadoc to something like this:

Prepend newly created arbitrary of the same type with provided values in samples.
NOTE: Properties does usually use only first generated value from Arbitrary. Subsequent values are used during shrinking (i.e. if Property test with first sample value fails, other sample values and than former Arbitrary generator, provides values for shrinking. Another usage is, when generating list from Arbitrary. For example this will generate list of three numbers, with first item always to be zero:
<code>Arbitraries.bigDecimals().withSamples(new BigDecimal("0.00")).list().ofSize(3);</code>

Second part - enhancement proposal/request.

I am not very comfortable with provided solutions. I really want my arbitrary string generator for names to generate arbitrary random string, with some hand-crafted samples with sql injections for example. I expect that it will use samples provided by me with some probability.

My approach to this problem consist of this:

        Arbitraries.oneOf(
                Arbitraries.strings().numeric()
                        .alpha()
                        .withChars('.')
                        .ofMinLength(6)
                        .ofMaxLength(16),
                Arbitraries.samples(";-- drop all tables now();")
        );

Using frequencyOf would probably yield to even better results.

I would like to see some easiest (fluent) way of achieving this behavior. For example method withSamplesWithFrequencies().

Would it make any use for someone? Does it reasonably fit to jqwik api?

from jqwik.

"It does evoke in me, that before generating some random things, provided sample values will be tested at first. This does not happen. All tests use just single value (first from withSamples list)."

Well, it does happen if ENOUGH EXAMPLES ARE BEING CREATED - e.g. when you use it to fill a list or a set. That said and given the history of it being misunderstood I wonder if withSamples() is useful at all. Looking at my usages of it seems to suggest that it is not. I'll probably deprecate it.

As for withSamplesWithFrequencies: Is it really worth it? Your oneOf example reads nicely I think. If you consider withSamplesWithFrequencies worthwhile, please open a new issue so that we can discuss it there.

from jqwik.

Hmmm, ok, deprecating (later removing) withSamples() (method which does confuse a few people) is imho nice.

Introducing withSamplesWithFrequencies, would be imho introducing same (similar) type of shortcut as withSamples. So I will probably would like to see both methods in some helper class (some builder helper or something like that). Probably in testing code of projects which does utilize jqwik.

If you see more such "shortcut" utility methods, and you see where to put them, I can have a look at code and migrate there withSamples() and introduce withSamplesWithFrequencies there. For now I do not see big need. I have my JqwikUtils :)

import net.jqwik.api.Arbitraries;
import net.jqwik.api.Arbitrary;
import net.jqwik.api.Tuple;

public class JqwikUtils {
    /**
     * Returned {@link Arbitrary} will use first parameter in 41 cases and in case 42, will randomly choose from
     * provided samples.
     *
     * @param arbitrary {@link Arbitrary} to use for value generation in general
     * @param samples   list of samples to be randomly injected with 1:42 ratio
     * @param <T>       type of {@link Arbitrary}
     * @return newly created {@link Arbitrary} with "sometimes" injected sample value.
     */
    public static <T> Arbitrary<T> withInjectedSamples(Arbitrary<T> arbitrary, T... samples) {
        return Arbitraries.frequencyOf(
                Tuple.of(42, arbitrary),
                Tuple.of(1, Arbitraries.of(samples))
        );
    }
}

from jqwik.

In my experience people develop their individual styles and preferences of how to combine arbitraries. jqwik should enable many or most styles but not necessarily incorporate each variant into its core. Thus I'd rather defer such convenience methods until there's a stronger use case.

from jqwik.

BTW, Arbitraries.withSamples() has been deprecated in 1.2.2-SNAPSHOT

from jqwik.