Allow a way to tell @Provide methods that we are unable to create an Arbitrary with the given arguments and we need a new set

[adam-waldenberg]

Testing Problem

I have the following test class:

public class PublishSporkIntegrityTest extends BaseCodecTest<PublishSpork> {
	@Provide
	public Arbitrary<?> providePublishSpork(@ForAll Type type, @ForAll short flags,
		@ForAll @NotEmpty byte[] signature, @ForAll Instant time) {

		try {
			/* Code that creates a PublishSpork instance */
			return Arbitraries.of(instance);
		} catch (IllegalArgumentException ex) {
			assertThat(type, is(Type.UNDEFINED));
		}

		return Arbitraries.nothing();
	}

	@Property
	@SneakyThrows
	public void shouldMatch(@ForAll("providePublishSpork") PublishSpork publishSpork,
		@Mocked ChannelHandlerContext context) {

		/* Some code conducting the test, using the provided publishSpork instance */
		assertThat(...);
	}
}

It works well most of the time. But whenever Arbitraries.nothing() is passed back by the provider it seems like publishSpork is coming in as null, causing a NullPointerException. Right now, the solution to this problem is to do the following in the test method and skip the assertion (and logic) whenever null comes in;

public class PublishSporkIntegrityTest extends BaseCodecTest<PublishSpork> {
	@Property
	@SneakyThrows
	public void shouldMatch(@ForAll("providePublishSpork") PublishSpork publishSpork,
		@Mocked ChannelHandlerContext context) {

		if (Objects.nonNull(publishSpork)) {
			/* Some code conducting the test, using the provided publishSpork instance */
			assertThat(...);
		}
	}
}

Is this is how it is supposed to work at the moment?

Suggested Solution

Arbitraries.nothing() presents a perfect opportunity to filter calls to the property test whenever a provider returns nothing(). Rather than passing null (which seems wrong) it would be a lot better if Jqwik just moved on to the next try and made a new attempt to gather values from the providers. That, or just call that specific provider again until a value is returned.

[jlink]

nothing() is not intended to be used in that way. Its sole purpose is to return nothing, i.e. void in the context of generated functions. I assume you’d like something along the lines of #335, but that hasn’t been implemented yet.

In your case I‘d probably use a filter function to only return an arbitrary if some condition is fulfilled. Your code is missing some elements for me to see what this condition is. (gridSporkType seems to come from nowhere)

BTW, if you use a non wildcard return type for the provider function (Arbitrary<PublishSpork>) the type system tells you that you’re using nothing() in an unintended way.

[jlink]

One more thing. If you ever need something like

if (Objects.nonNull(publishSpork)) {
	/* Some code conducting the test, using the provided publishSpork instance */
	assertThat(...);
}

the idiomatic way is

Assume.that(Objects.nonNull(publishSpork));
assertThat(...);

This will give jqwik a better chance to do the right thing during shrinking. And it looks cleaner IMO.

[adam-waldenberg]

gridSporkType

gridSporkType should be type - it's just a typo when I cleaned up the code for the example. Not quite sure if #335 would be the same thing.

I probably didn't explain it that well - but essentially what I'm looking for is a way to tell JQwik that;

I am unable to create an object with the values (or value) you gave me - please give me new ones so I can try again

Using in Arbitraries.nothing(); and a check in the actual test essentially gives me that (at the expense of some of the invocations just passing), but it's not pretty.

[jlink]

Using in Arbitraries.nothing(); and a check in the actual test essentially gives me that (at the expense of some of the invocations just passing), but it's not pretty.

It's also just coincidental behaviour. You could as well use

Arbitraries.just((PublishSpork) null)

which would at least be type-safe.

I suggest you go with something like:

@Provide
public Arbitrary<?> providePublishSpork(@ForAll Type type, @ForAll short flags,
        @ForAll @NotEmpty byte[] signature, @ForAll Instant time) {

    return type
            .filter(t -> !(t instanceof Type.UNDEFINED))
            .map(t -> {
                     /* Code that creates a PublishSpork instance */
                     return Arbitraries.just(instance);
            });
}

[adam-waldenberg]

Using in Arbitraries.nothing(); and a check in the actual test essentially gives me that (at the expense of some of the invocations just passing), but it's not pretty.

It's also just coincidental behaviour. You could as well use

Arbitraries.just((PublishSpork) null)

which would at least be type-safe.

I suggest you go with something like:

@Provide
public Arbitrary<?> providePublishSpork(@ForAll Type type, @ForAll short flags,
        @ForAll @NotEmpty byte[] signature, @ForAll Instant time) {

    return type
            .filter(t -> !(t instanceof Type.UNDEFINED))
            .map(t -> {
                     /* Code that creates a PublishSpork instance */
                     return Arbitraries.just(instance);
            });
}

Nah, that wont work. You are assuming that Type.UNDEFINED should always fail - the failure does not stem from the type, but from the incoming arguments. What if we had something like this?:

	@Provide
	public Arbitrary<?> provideSomething(@ForAll Arg1 arg1, @ForAll Arg2 arg2) {
		try {
			/* Code creating instance */
			return Arbitraries.of(instance);
		} catch (Exception1 | Exception2 | Exception3 ex) {
			/* NOP - The passed args makes instantiation fail with an exception */
			/* Likely a failure we are expecting, so we still want to continue... */
		}

		return Arbitraries.nothing(); /* What else can we do here? */
	}

We are stuck here unless we rely on that coincidental behavior. From what I can tell, one way is obviously to not rely on @ForAll at all and instead create the arbiraries right in the provider and do as you suggested. However, that's not as elegant as having a simple way of telling JQwik that we want it to rerun the provider and just give us new values.

[adam-waldenberg]

I tried an alternative approach with a configurator,

public class NotNullConfigurator extends ArbitraryConfiguratorBase {
	@Override
	public <T> Arbitrary<T> configure(Arbitrary<T> arbitrary, TypeUsage targetType) {
		return arbitrary.filter(obj -> Objects.nonNull(obj));
	}
}

@Retention(RetentionPolicy.RUNTIME)
@Target({ ElementType.ANNOTATION_TYPE, ElementType.PARAMETER, ElementType.TYPE_USE })
public @interface NotNull {
	/* Empty on purpose */
}

I changed my provider and added the annotation to my @Property:

@Provide
public Arbitrary<PublishSpork> providePublishSpork(...) {
	try {
		....
		return Arbitraries.of(instance);

	} catch (IllegalArgumentException ex) {
		assertThat(gridSporkType, is(Type.UNDEFINED));
	}

	return Arbitraries.just(null);
}

@Property
public void shouldMatch(@ForAll("providePublishSpork") @NotNull PublishSpork publishSpork,
	@Mocked ChannelHandlerContext context) {
		...
	}
}

I know its parsing the service file and loading the class. Still, the filtering does not seem to work. Am I misinterpreting the
documentation? Because if this worked it would be a pretty nice way of solving it.

[jlink]

We are stuck here unless we rely on that coincidental behavior. From what I can tell, one way is obviously to not rely on @ForAll at all and instead create the arbiraries right in the provider and do as you suggested. However, that's not as elegant as having a simple way of telling JQwik that we want it to rerun the provider and just give us new values.

Thinking a bit more about your example, there probably should be a simpler way to shortcut the generation of values.
Maybe something like Arbitraries.skip() or Arbitraries.fail().

Implementing the above-mentioned Arbitraries.empty() feature would be more general, but also solve your problem.

[jlink]

Still, the filtering does not seem to work. Am I misinterpreting the
documentation? Because if this worked it would be a pretty nice way of solving it.

ArbitraryConfiguratora must be either registered or provided by a domain. Did you do that?

[adam-waldenberg]

We are stuck here unless we rely on that coincidental behavior. From what I can tell, one way is obviously to not rely on @ForAll at all and instead create the arbiraries right in the provider and do as you suggested. However, that's not as elegant as having a simple way of telling JQwik that we want it to rerun the provider and just give us new values.

Thinking a bit more about your example, there probably should be a simpler way to shortcut the generation of values. Maybe something like Arbitraries.skip() or Arbitraries.fail().

Implementing the above-mentioned Arbitraries.empty() feature would be more general, but also solve your problem.

Yes. If JQwik also filtered it, we would also not get "unnecessary/empty" calls to the property like now. Plus looks a lot better and makes more sense than Arbitraries.just(null);

[adam-waldenberg]

Still, the filtering does not seem to work. Am I misinterpreting the
documentation? Because if this worked it would be a pretty nice way of solving it.

ArbitraryConfiguratora must be either registered or provided by a domain. Did you do that?

I did. I registered it in /META-INF/services/net.jqwik.api.configurators.ArbitraryConfigurator and also verified that the file is being loaded (by adding non-exisiting classes as a test - which gives me an error). Even though it's loading it, it's not being used though.

I also tried the @Domain approach with a wrapped class implementing ArbitraryConfiguratorBase, but that breaks the property test and causes it not to find the arbitraries for any general types (int, long, Instant, enums, etc etc - they all stop working) and it throws a CannotFindArbitraryException So that doesn't work at all at the moment. I created a new issue (#407) for this one.

[adam-waldenberg]

Still, the filtering does not seem to work. Am I misinterpreting the
documentation? Because if this worked it would be a pretty nice way of solving it.

ArbitraryConfiguratora must be either registered or provided by a domain. Did you do that?

I did. I registered it in /META-INF/services/net.jqwik.api.configurators.ArbitraryConfigurator and also verified that the file is being loaded (by adding non-exisiting classes as a test). Even though it's loading it, it's not being used though.

For now my hacky solution with the manual check will have to do. I will see if I can whip up a unit test replicating the failing configurator. If so - I will make a pull request later to replicate it.

[jlink]

I did. I registered it in /META-INF/services/net.jqwik.api.configurators.ArbitraryConfigurator and also verified that the file is being loaded (by adding non-exisiting classes as a test - which gives me an error). Even though it's loading it, it's not being used though.

Just noticed that your configurator should look different to make it react to @NotNull:

public class NotNullConfigurator extends ArbitraryConfiguratorBase {
	public <T> Arbitrary<T> configure(Arbitrary<T> arbitrary, NotNull ignore) {
		return arbitrary.filter(obj -> Objects.nonNull(obj));
	}
}

This should invoke the configurator - at least it did in my experiments - but it won't solve your problem, because wrapping the arbitrary instance created by Arbitraries.just(null) with this filter will result in a TooManyFilterMissesException.

Your configurator example came with a surprising result, though. When debugging it, I stumbled upon a bug in the annotations processing code. :-)

[adam-waldenberg]

Nice. 👍 Every bug found is a win.

I got the filtering to work by registering it under a @Domain rather than via the service file. Not sure why it's not working then, but I will investigate it later. In my case filtering with @NotNull works great and does not throw an exception - I'm guessing the max number of allowed misses is not reached.

Going to stick with this solution for now and will tweak it once there is something like skip() or fail(). Thanks for the support.

[jlink]

Added #408 as follow up.