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

Allow to Skip Current Generation Attempt in Provider Methods #408

Closed
jlink opened this issue Nov 10, 2022 · 12 comments
Closed

Allow to Skip Current Generation Attempt in Provider Methods #408

jlink opened this issue Nov 10, 2022 · 12 comments

Comments

@jlink
Copy link
Collaborator

jlink commented Nov 10, 2022

Testing Problem

Motivated by example in #405 (comment).

Sometimes you want to skip the current generation attempt because previously generated values
that are flat-mapped don't allow generation. Example:

@Provide
Arbitrary<MyObject> myObjects(@ForAll int size, @ForAll String code) {
    if (size == 42) {
        // skip generation here because size 42 does never work
    }
    try {
        return just(new MyObject(size, code));
    } catch (IllegalArgumentException e) {
        // skip generation because some combination of size and code is not allowed
    }
}

Suggested Solution

Introduce something like Arbitraries.skip() or Arbitraries.fail() which will proceed to next generation attempt.
This only makes sense (I assume) if there's some flat-mapping going on.

Discussion

  • Many of those cases can (better) be solved by filtering out certain parameter combinations.
  • Could be related to Support "empty" arbitrary #335
  • Would Assume.that(..) be a viable option instead of Arbitraries.skip()?
@adam-waldenberg
Copy link

adam-waldenberg commented Nov 10, 2022

In some cases (like in our case), the exception is coming from a dependency. So we really don't know all the corner cases that trigger it. So manual filtering and/or Assume doesn't really work.

@jlink
Copy link
Collaborator Author

jlink commented Nov 10, 2022

So manual filtering and/or Assume doesn't really work.

try {
   ...
} catch (Throwable t) {
   Assume.that(false);
}

not nice, but what in this world is...

@adam-waldenberg
Copy link

adam-waldenberg commented Nov 10, 2022

Just realized something.... Maybe some kind of mechanism to rerun the provider on an exception would be cleaner? Something like this.

@Provide(repeatOnException = { /* List of exceptions */ })
Arbitrary<MyObject> myObjects(@ForAll int size, @ForAll String code) {
    if (size == 42) {
        // skip generation here because size 42 does never work
    }

    /* Instead of catching it we just let it fall through */
    return new MyObject(size, code);
}

That makes the code even cleaner and also makes it quite clear what is happening. No need to return any empty values or crap like that.

@jlink
Copy link
Collaborator Author

jlink commented Nov 10, 2022

Maybe some kind of mechanism to rerun the provider on an exception would be cleaner?

Worth pondering.

@jlink
Copy link
Collaborator Author

jlink commented Nov 25, 2022

Turns out that
@Provide(ignoreException = { /* List of exceptions */ })
just for methods annotated with @Provide would be much simpler to implement than a generic Arbitraries.skip() feature.

@adam-waldenberg I assume it would serve your needs since you brought that option up?

@adam-waldenberg
Copy link

Yes. Would that end up with the provider executing again, or would it still return a null value back to the test ? That is, will I still need my @NotNull filter?

@jlink
Copy link
Collaborator Author

jlink commented Nov 26, 2022

Would just skip generation attempts with specified exception(s).
There’s already ˋArbitrary.ignoreException(..)ˋ which does what we want. It just has to be applied on the arbitrary resulting from the provider method.

@adam-waldenberg
Copy link

Sounds like a plan then.

jlink added a commit that referenced this issue Dec 2, 2022
@jlink
Copy link
Collaborator Author

jlink commented Dec 4, 2022

@Provide(ignoreExceptions = {..}) is now available in 1.7.2-SNAPSHOT.

Documentation for this is still missing, though.

@jlink
Copy link
Collaborator Author

jlink commented Dec 5, 2022

@jlink jlink closed this as completed Dec 5, 2022
@jlink jlink removed the in progress label Dec 5, 2022
@jlink
Copy link
Collaborator Author

jlink commented Jan 9, 2023

1.7.2 has been released

@adam-waldenberg
Copy link

🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants