Configure the Generator object, beyond using annotations
Closed this issue · 16 comments
Thank you for developing quick check for Java, it's an interesting project.
I have an object outside of quick check, similar to a state machine, that I can query for the events that are accepted at any given moment (i.e. that cause a transition).
I want to use this object in a Generator to produce valid events randomly, but I can't figure out how to pass anything but a string (via an annotation) to instantiate the Generator.
Can you help me?
Ok, it seems I was able to find a solution myself: the generator initializes itself fully, and I use a List as parameter:
public void testMethod(List<@From(EventGenerator.class) Object> events){...}
However, shrinking does not seem to work. Any idea what the reason could be?
@bertilmuth Thanks for your interest in junit-quickcheck! Sorry for the late reply, I was on vacation.
Is it the list elements that you wish to shrink, or the lists themselves? Lists should be shrunk without you having to do anything...would you be willing to share some more of your code? Perhaps we'll be able to pinpoint your issue. Thanks!
Thank you for your answer.
I want to shrink the lists themselves.
Here's the shortened code of the EventGenerator class:
public class EventGenerator extends Generator<Object> {
private ModelRunner modelRunner;
public EventGenerator() {
super(Object.class);
...
modelRunner = new ModelRunner().run(model);
}
@Override
public Object generate(SourceOfRandomness random, GenerationStatus status) {
Object eventObject = generateEventObject(random);
modelRunner.reactTo(eventObject);
return eventObject;
}
private Object generateEventObject(SourceOfRandomness random) {
ArrayList<Class<?>> eventTypes = new ArrayList<>(modelRunner.getReactToTypes());
int eventIndex = random.nextInt(eventTypes.size());
Class<?> eventType = eventTypes.get(eventIndex);
Object event;
try {
event = eventType.newInstance();
} catch (Exception e) {
event = null;
}
return event;
}
}The modelRunner is similar to a state machine.
The call modelRunner.reactTo(eventObject); triggers a state transition.
The call modelRunner.getReactToTypes() returns the event classes of the event objects that are accepted in the current state.
The point of this generator is to generate a sequence of valid events.
@bertilmuth Thanks for the info! I believe I see what's going on.
The shrinking mechanism is set up to filter out nodes in the shrink tree that have greater or equal "magnitude" than the magnitude of the node at the root (the "original" failure). Each generator is responsible for telling the "magnitude" of a particular value that it produced. The magnitude of a generated list is the sum of the magnitudes of its elements. The magnitude of EventObjects is zero, because the generators for event objects likely do not override magnitude(), so they get a default magnitude of zero. Thus, any list of event objects will have magnitude zero regardless of its size or contents.
Basically, the shrinking mechanism produced a bunch of shrink candidates, but then immediately filtered them out on ground of equal magnitude to the original failure.
I wonder if zero is a poor choice for default magnitude(). I'll experiment with 1 as a default magnitude value, and see if 1) it helps your specific case, and 2) doesn't disturb the expectations of other code.
Thank you very much! I am looking forward to your answer. I will try it out myself as well (i.e. overriding magnitude).
Just out of curiosity: is there a reason that the magnitude method returns a default value, instead of being abstract and forcing sub classes to implement it?
@bertilmuth My initial thought was not to break existing generator implementations by introducing an abstract method. I wonder if, in hindsight, doing so ended up causing confusion greater than the heartbreak having broken generators for a short time would have caused.
I see. Well, maybe it's a candidate for the v1.0 then. I guess people expect things to break at major releases.
@bertilmuth I changed Generator.magnitude() to respond with 1 instead of 0, re-ran all my tests, and they all passed. I feel better about possibly introducing this change in an 0.8.x release.
How does your test behave if you override magnitude() in EventGenerator to respond with 1?
Hmm. Tried it, but the solutions found do not look minimal. I used
@Property(shrink = true)
on the test method.
Is that enough? Anything else to take care of?
@bertilmuth I'd like to understand more about what your test is doing. I assume that, given the list of events, it decides whether the sequence is valid?
From a "counterexample" list, the list generator will produce a list of shrinks by: removing some elements of the counterexample, and also producing lists of the same size as the counterexample, with one element shrunken (I guess in our particular case, events cannot be shrunk). I believe it may be the case that the shrinks that are nearest in magnitude to the original counterexample are getting tried first, and perhaps the default maxShrinks value of 100 is exceeded -- so your "smallest" counterexample isn't that much smaller, if at all, from the original.
Can you try increasing the maxShrinks value on your property and see what happens? Thanks!
Thank you so much for taking the time to think about my case and answer so thoroughly.
Your proposal seems to help, there are still a few cases where the result does not seem to be minimal.
My test is NOT testing for valid events. The method modelRunner.getReactToTypes will only return events that are valid in the current state. So that is taken for granted.
Instead, the test checks if applying the events leads to an unexpected state of the system.
One issue may be: the order of events is important. So the events need to occur starting with index 0 of the list, ending with the first index that leads the test to fail. That's what I would call the minimum solution.
@bertilmuth Ah ok, that makes sense. Perhaps a shrinking strategy where the counterexample is repeatedly reduced starting from the greater index would make more sense in your case. It may also lead to fewer and more consistently smaller counterexamples. The default shrinking strategy for lists may not help too much in your situation.
The next thing you might try, if interested, is to subclass a list generator and override shrink() to take the counterexample it's given and return a singleton list of that counterexample with its rightmost element removed.
This brings up the next question: should junit-quickcheck offer a way to override a generator's shrinking strategy without the programmer having to subclass the generator and override shrink()?
In short: yes. Being able to set the shrinking strategy would be helpful. If I am not mistaken, generation and shrinking are two separate concerns.
In any case, whether this is implemented or not, it would be helpful to have more transparency on how shrinking works (e.g. in the docs). For me, it felt a bit magical:)
@bertilmuth Thanks for the feedback. For purposes of this issue, I will only change Generator.magnitude() to answer 1 rather than zero. If you'd like to be able to override how values are shrunk in a manner other than method override (like, perhaps, through an annotation), would you file a separate issue to that effect?
Closing for now. If we're looking to provide overriding shrink strategies via annotation, let's make a separate issue for that.