sageserpent-open / americium

Generation of test case data for Scala and Java, in the spirit of QuickCheck. When your test fails, it gives you a minimised failing test case and a way of reproducing the failure immediately.
MIT License
15 stars 1 forks source link

Keeping up with the Joneses. #31

Closed sageserpent-open closed 2 years ago

sageserpent-open commented 2 years ago

Reading this blog post about Jqwik: Property-based Testing in Java: The Importance of Being Shrunk introduced me to Jqwik's rather nice fluent syntax for configuring how its Arbitrary instances are built for String.

Quoted from said blog post:

@Provide
Arbitrary<String> first() {
    return Arbitraries.strings()
            .withCharRange('a', 'z')
            .ofMinLength(1).ofMaxLength(10)
            .filter(string -> string.endsWith("h"));
}

@Provide
Arbitrary<String> second() {
    return Arbitraries.strings()
            .withCharRange('0', '9')
            .ofMinLength(0).ofMaxLength(10) 
            .filter(string -> string.length() >= 1);
}

Writing an equivalent using Trials as of commit 190467f0e4ff3261d769a1dac83a0950c6581de6 (Release 1.0.0) is feasible, but results in very prolix code in comparison:

private static Trials<String> first = stringsOfSize(1, 10, charactersInRange('a', 'z'));

private static Trials<String> second = stringsOfSize(0, 10, charactersInRange('0', '9'));

private static Trials<String> stringsOfSize(int minimumSize,
                                            int maximumSize,
                                            Trials<Character> characters) {
    return api
            .choose(Range.range(minimumSize, maximumSize))
            .flatMap(size -> characters.collectionsOfSize(size,
                                                          () -> new Builder<Character, String>() {
                                                              final StringBuffer
                                                                      buffer =
                                                                      new StringBuffer();

                                                              @Override
                                                              public void add(
                                                                      Character caze) {
                                                                  buffer.append(
                                                                          caze);
                                                              }

                                                              @Override
                                                              public String build() {
                                                                  return buffer.toString();
                                                              }
                                                          }));
}

private static Trials<Character> charactersInRange(char from, char to) {
    return api.integers(from, to).map(index -> (char) (int) index);
}

One way to improve the user experience with Trials would be to have syntax like this:

final TrialsApi api = Trials.api();

final Trials<String> strings = api.characters('a', 'z').stringsOfSize(1, 10);

This would also work with the existing method characters() defined in TrialsApi - effectively giving characters the same overloads of say, integers - and thus allowing shrinking to apply, although I think that the existing implementation of characters should keep its non-shrinking behaviour; there is no obvious default to shrink to (this would also be the case for the overload that doesn't take an explicit shrinkage target too).

This also implies that the existing *OfSize methods would acquire overloads with lower and upper bounds of a range of sizes.

I'm not completely sure that the second part of this is necessary - it isn't hard to flat-map sizes, and that permits flexibility as to how the sizes are provided - could be a range or could be some other set:

final Trials<String> strings = 
    api.choose(Range(1, 10)).flatMap(size -> api.characters('a', 'z').stringsOfSize(size));

final Trials<String> strings =
    api.choose(1, 2, 10, 500).flatMap(size -> api.characters('a', 'z').stringsOfSize(size));
sageserpent-open commented 2 years ago

The flat-mapping over a range of sizes is something that I've had to do in several occasions - let's bring in some convenience overloads for that and leave other use-cases for explicit flat-mapping.

sageserpent-open commented 2 years ago

A thought - from the final thoughts on #32 regarding the interaction of value shrinking with use of Trials.*OfSize in the existing codebase, given that value shrinking can cause the complexity to go below a threshold where a failing test case can be generated, then it might be prudent to implement this with TrialsApi.choose driving the sizes, rather than say, TrialsApi.integers.

We would still get complexity shrinkage, so although it might become quite inefficient at the end of the shrinkage, we'd still get some results - this was observed when working in #32.

Then again, maybe the underlying issue of not having multivariate shrinkage should be addressed once and for all?

For now, let's bodge this by using TrialsApi.choose and come back to this once multivariate shrinkage is addressed...

sageserpent-open commented 2 years ago

This turns out to be very difficult to do in a clean manner - perhaps the design of TrialsImplementation needs a rethink...

To summarise, introducing an interface on the Java side that extends Trials<Character> with the extra fluent methods requires either some considerable faffing around with an F-bounded form of TrialsImplementation or use of ByteBuddy to wrap the usual TrialsImplementation in the extended interface via delegation. Certainly possible, but this all feels like a sledgehammer to crack a nut.

I did try defining default implementations of the fluent methods in the extended interface and hoped to use an intersection type for the trials returned by TrialsApi.characters, but Java doesn't stretch that far, which is what led to the F-bound foolishness.

This might be worth revisiting, and ByteBuddy (or for that matter Scala 3 exporting) would allow the delegation approach - or I could just write a delegating implementation by hand - but not now.

sageserpent-open commented 2 years ago

Instead of the fancy fluent syntax, what is hopefully good enough has been proffered:

private static final Trials<String> first =
        api.integers(1, 10)
           .flatMap(size -> api
                   .characters('a', 'z', 'a')
                   .collectionsOfSize(size, Builder::stringBuilder));

private static final Trials<String> second =
        api.integers(0, 10)
           .flatMap(size -> api
                   .characters('0', '9', '0')
                   .collectionsOfSize(size, Builder::stringBuilder));

@Disabled
// This now detects the 'failing' test case correctly - but it is still a
// test failure. Need to rethink what this test should look like....
@Test
void copiedFromJqwik() {
    first.and(second)
         .withLimit(50)
         .supplyTo((String first, String second) -> {
             final String concatenation = first + second;
             assertThat("Strings aren't allowed to be of length 4" +
                        " or 5 characters" + " in this test.",
                        4 > concatenation.length() ||
                        5 < concatenation.length());
         });
}

Here we use explicit flat-mapping of sizes; it isn't that painful, and re-use the existing collections support, treating a String as a kind of collection of Character. It seems OK for now.

On the Scala side we do have the fancy Trials.strings / Trials.stringsOfSize syntax via an implicit syntax extension - in fact even the Trials.several / Trials.lotsOfSize will do this by default if applied to a Trials[Char].

sageserpent-open commented 2 years ago

In passing, the Java incarnation of Trials has gone back to being an interface. Good.

sageserpent-open commented 2 years ago

This has gone out in version 1.1.0, commit b9a6185bad06695736daaea5fbd5086e1d0ce90a .