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

Shrinkers should be able to provide a Stream #220

Open
sir4ur0n opened this issue Feb 26, 2019 · 5 comments
Open

Shrinkers should be able to provide a Stream #220

sir4ur0n opened this issue Feb 26, 2019 · 5 comments

Comments

@sir4ur0n
Copy link
Contributor

Most List implementation (in particular, all from the JDK) are strict, meaning we may spend time building shrinks that will never even be tested.

Java 8's Streams can be lazy, enabling library consumers to wrap their shrunk value computation inside some lazy structure (e.g. a Supplier), and Junit Quickcheck would only process values until one doesn't pass.

Ideally this should be done after #219 as it may change the way shrinking works.

@sir4ur0n
Copy link
Contributor Author

sir4ur0n commented Mar 1, 2019

@pholser What's your opinion on the public API of com.pholser.junit.quickcheck.generator.Generator#shrink?

  1. Make a breaking API change by changing the return type from List<T> to Stream<T>: for existing library consumers, it would be as easy as appending .stream() at the end of their shrinker and change the return type in the function.
  • Main benefit: API stays clean and easy to understand
  • Main drawback: consumers must apply a (minor) change to all their shrinkers.
  1. Provide both APIs (e.g. add shrinkStream()), and users can implement one or both. The shrinker could append the list results to the stream results (thus lazy results are considered before strict ones) and then shrink.
    Default implementation for shrinkStream() would return an empty stream: Stream.empty().
  • Main benefit: backward compatible.
  • Main drawback: double API, more complexity to understand for consumers.

I'm in favor of 1) since the consumer impact is minimal, and easy to explain in the documentation.

@pholser
Copy link
Owner

pholser commented Mar 4, 2019

@sir4ur0n What do you think of option 3: same as option 2, but deprecate the List<T> signature of shrink? Idea being that we don't force generator writers to break right away, but gently nudge toward lazy shrink value generation.

@pholser
Copy link
Owner

pholser commented Mar 4, 2019

I would imagine removing the List version as 1.0 becomes available.

@sir4ur0n
Copy link
Contributor Author

sir4ur0n commented Mar 4, 2019

Sounds good. I'll try to give it a shot in the coming days.

Any idea/opinion about when 1.0 lands?

@pholser
Copy link
Owner

pholser commented Mar 5, 2019

@sir4ur0n I don't have any particular time in mind for committing to a 1.0 release.

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