-
-
Notifications
You must be signed in to change notification settings - Fork 64
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
State-Dependent (Ad-Hoc) Action Generator #134
Comments
@DirkToewe Thanks for bringing that up. The idea to generate the next step of a sequence based on a model's state is a good one and would facilitate some type of state-based testing. However, it does not fit together with the way jqwik is currently generating and shrinking sequences and would require more or less a re-implementation of the stateful package. So the feature is on my long-term list but not yet prioritized. Not all is lost, though. The work-around is to keep an ordered list of possible IDs in your model and generate a random index to that list. If the random index exceeds the length of the list you wrap it around using modulo arithmetic. I demonstrated the approach here: I hope that helps. Feel free to get back to me if that doesn't solve your problem. |
That would have been my idea for a Workaround as well. One concern I had with that is edge cases: Wouldn't values like |
Edge cases will have a higher probability regardless. I'd also argue that trying the first and last element more often is a reasonable thing to do. That said, being able to switch edge case generation off for individual arbitraries is quite on top of jqwik's feature list. Currently planned for 1.4.0. What may matter more IMO is a uniform distribution of indices. You can enforce uniform distribution of index like that Arbitrary<Integer> index = Arbitraries
.integers()
.between(0, MAX_SIZE)
.withDistribution(RandomDistribution.uniform()); You may want to watch the statistics to see if distribution of idices is ok for you. |
I see the same I'm facing a related testing case: my test model is nested like Then actions like I'm inclined to do the following: resolve That would rely on the fact that I think that would have the following good properties:
|
BTW, this has been possible now for a while: |
The plan of abusing |
I tried yet another approach: expose It almost works, and now I am able to generate basic actions. The findings so far:
I wonder if some of the nested generators are cached.
For instance, I have the following:
Of course, it makes no sense to remove a table in case the tables are missing, however, My solution is to use
My workaround is to use
|
Would |
I do not think so. |
What effect did you observe that came through caching? The generated result should be independent of caching so there may be something else going on... |
Does shrinking still work with this approach? |
I think the observed behavior was "add column" was unable to find a workable table name (it observed "empty state" always), however, now I reverted to regular
Shrinking seems to work in a sense of "excluding actions". |
After all this discussion state-dependent generation seems a worthwhile feature for the near future. It's a major change though. My current plan is to do it for 1.7, the development of which I hope to start before Easter. |
It looks like I figured out my way through generating and applying the nested dependent actions, and I would proceed with actions that do call the database to see how all this works. I ended up using a more-or-less single class for all the actions, and I use a builder function to generate them: fun <T> action(
description: () -> String,
validate: T.() -> Boolean,
body: T.() -> Unit
)...
@Provide
fun mappingActions() = StatefulActionSequenceArbitrary(
frequencyOf(
1 to just(ClearMappingAction),
...
// Arbitrary<T>.perform is basically combine(this, that) { this, that -> this.perform(that) }
4 to arbitraryTable().perform(`update object field`()),
).ignoreException(NotFoundException::class.java)
)
fun `update object field`() =
frequency(
1 to ObjectField.OBJECT_ID,
5 to ObjectField.NAME,
).map { field ->
action<ColumnScope>(
description = { "UseObjectField($field)" },
validate = { true }
) {
rawValue = field
}
}
So far the generated sequence seems to use actions I supply, and it seems to reuse the names appropriately. |
It looks like shrinking does not work.
The following looks suspicious:
Do you think I guess the use case is if the shrinker removes "add table" action, then all the other actions that "modify the table" can't really succeed, and they can't "shrink" because the table they were about to modify just does not exist. |
In the end, it's all a hack anyway, I'm afraid. Which will hopefully no longer be necessary with the introduction of the new state-dependent generation. I've already done a POC that looks promising. I hope to find time for further exploration this weekend. |
The stacktrace clearly says that exception happens when I have my own copy of Here's what happens:
Suppose the actions were:
That set of actions works well in the initial generation because "add column" finds a usable However, when shrinker comes in place, it calls So far I replaced |
Where exactly did you replace it? |
Well, the code is I wonder if it's better to make |
Frankly speaking, that was my first idea, however now I am inclined hiding exceptions won't help. Here's another stacktrace:
The issue seems to be that it is not valid to call The way out might be rework
WDYT? |
Without having dived into that too deeply, flat-mapping is always making shrinking much worse. The upcoming approach will have to do some flat-mapping anyway in some circumstances, but probably with more knowledge about runtime dependencies. Doing ad-hoc modifications to distance metrics and shrinking might improve the situation for one use case but deteriorate it for another. I'd rather invest my time in the new foundation than mending a soon to be replaced one. |
I believe
|
|
Unfortunately, The code is easier to read and write, however, shrinking behaviour is still bad. For instance:
Then, if shrinker removes the first action, the sequence becomes: That means "action removal" might augment all the subsequent actions. If jqwik keeps I've no idea how to heal that yet. |
IMO you should either try to create your actions without reference to the state (e.g. through s.t. like the modulo approach together with more complex actions) or wait till state-dependent generation becomes available. I don't figure there's a sane way to add state-dependency ontop of the current implementation. |
Work on this issue, aka the new stateful support, started on https://github.com/jlink/jqwik/tree/new-stateful |
Have you considered merging |
That was my first approach. I re-introduced preconditions because they enable much better shrinking. |
Could you suggest some pointers? I understand that trivial actions that have no preconditions and access no state for generating The context of the question is that I struggle with supporting nested data types with the new API. Suppose the structure is as follows: class City {
Map<HouseId, House> houses;
}
class House {
Map<RoomId, Room> rooms;
}
class Room {
Map<LampId, Lamp> lamps;
}
class Lamp {
boolean isTurnedOn;
Color color;
} Here are the actions I would like to implement:
For instance: class SwitchOnLampAction<City> implements Action<City> {
boolean predicate(City city) {
return city.houses.values()
.stream().anyMatch(house ->
house.rooms.values()
.stream().anyMatch(room ->
room.lamps.values().stream().anyMatch(lamp -> !lamp.isTurnedOn)));
}
Arbitrary<Transfomer<City>> transformer(City city) {
Stream<Lamp> turnedOffLamps = city.houses.values()
.stream().flatMap(house ->
house.rooms.values()
.stream().flatMap(room ->
room.lamps.values().stream().filter(lamp -> !lamp.isTurnedOn)));
return Arbitraries.of(turnedOffLamps).map(
lamp -> Transformer.mutate(
() -> "activate lamp " + lamp + ", in room " + room + ", in house " + house + ", in city " + city
state -> { lamp.isTurnedOn = true }
)
)
}
} a) The code that "searches for an arbitrary lamp within an arbitrary room within an arbitrary house..." quickly becomes duplicated across many actions. That is why I was considering moving "predicate" into the transformer method (or even into I am trying to implement something like class LampSwitchOnTransformer implements ./*?*/..Transfomer<Lamp> {
String description() {
return "switch on lamp " + lamp;
}
boolean predicate(Lamp lamp) {
return !lamp.isTurnedOn;
}
void/*?*/ perform(Lamp lamp) {
lamp.isTurnedOn = true;
}
} And then it would be nice to spawn it via something like |
An action with a precondition but no access to the state for providing a transformer can be handled in shrinking like an action without any state-access at all. If the precondition is no longer valid the shrinking candidate will be discarded on evaluation of precondition. But you can always return a NOOP transformer if you want to. Maybe introducing something like |
I see. However, if the action accesses state for providing a transformer, then a separate precondition method does not seem helpful 🤷♂️ |
Not sure. A precondition is a different concern than the actual provisioning. Having |
I agree |
By the way, if jqwik detected "empty arbitrary", then a precondition for
static class UpdateValue implements Action<MyStore<Integer, String>> {
@Override
public Arbitrary<Transformer<MyStore<Integer, String>>> transformer(MyStore<Integer, String> state) {
Arbitrary<Integer> existingKeys = Arbitraries.of(state.keys());
return Combinators.combine(existingKeys, values())
.as((key, value) -> Transformer.mutate(
String.format("update %s=%s", key, value),
store -> {
store.store(key, value);
assertThat(store.isEmpty()).isFalse();
assertThat(store.get(key)).isEqualTo(Optional.of(value));
}
));
}
} |
I’m struggling a bit with the concept of empty arbitrary because jqwik is built on the assumption that an arbitrary will produce an endless stream of values. Introducing it for this special usage feels wrong. It’s more functional but less Java-ish. |
Frankly speaking, I have previously been bitten by Arbitraries.of(emptylist) producing an arbitrary that throws at generation time. It was hard to diagnose since it was not clear where did the empty arbitrary come from. So it might make sense to split the existing Arbitraries.of(array|collection|...) into "of(array|...) arbitrary or throw if empty" and "ofOrEmpty(array|collection..)" that would return empty arbitrary on empty input. Then, the source code would clearly highlight the call could yield empty arbitrary which is dangerous in many cases. That would be like Iterable.first (throws on empty) and Iterable.firstOrNull (null on empty). However, I have no strong preference here, and just(transformer.noop()) would be workable for me. |
I think it's worthwhile to rethink this design and moving towards a more complete Arbitrary abstraction. It's too big a change to do it in connection with the new stateful API. I'd rather do it within the re-design for jqwik 2. I'll go with |
interface Action<S> {
boolean precondition(S state);
interface Dependent<S> extends Action<S> {
Arbitrary<Transformer<S>> transformer(S state);
}
interface Independent<S> extends Action<S> {
Arbitrary<Transformer<S>> transformer();
}
} Or rather top-level types |
As for the builder, I tend to NOT have an My argument for not having an |
After trying various approaches, I was able to launch some tests with the new API. I have the following failure. Do you think ``SequentialActionChain.transformations` could be implemented without re-instantiating the shrinkables and without re-running the sequence? context(MappingState)
fun InstanceRef.arbitraryPlainTable(): Arbitrary<TableRef> {
val names = mapping.run { value }.tables.names // retrieves the list of table names from the state
if (names.isEmpty()) {
throw NOT_FOUND // <-- Exception from here
}
return Arbitraries.of(names).map { TableRef(this, it) }
}
|
I guess it would be possible to eagerly collect all transformations when they are executed, but then the lazy evaluation of description strings will no longer make sense. That’s just guesswork though without looking at the code. |
Yeah, I did not envision that when suggested lazy displayName computation :-/ |
It looks like shrink for stateful actions does not work under the current model. The issue is as follows:
I wonder if the transformers should be re-instantiated one after another even during the shrinking phase. |
It looks the solution could be: b) Allow reusing the existing transformers, however, then jqwik should verify "feasibility" of each transformer one by one as it applies steps during shrink phase. On top of that, it would likely make sense to use In other words: public interface Action<S> {
Arbitrary<Transformer<S>> transformer(Supplier<S> stateSupplier) { Then the generated I'm inclined to |
It might be that |
It seems to me that this only happens if the transformers themselves are mutable (which they should never be). |
Feedback welcome. |
Any news on that @vlsi ? Otherwise, I tend to close this issue since all my plans have now been realized in 1.7.0-SNAPSHOT |
@jlink , I've created a sample chain that reproduces the issue of "wrong state being used in transformers": #426 (comment) |
I lost overview of all the different topics discussed in this issue. Can it be closed now since #426 has been tackled? |
Testing Problem
Let's say You want to test the
List<E>::add(int, E)
method of a custom implementation of aList
. SinceList
is stateful, action sequences would be perfect for testing. So naively, a provider could be implemented as follows:This will however very likely fail or run indefinitely, as
idx
is out of bounds most of the time andprecondition
evaluates to falsealmost all of the time.
Suggested Solution
Would it be possible to support the creation of action sequences via a function of type
State => Arbitrary<Action<State>>
? Something along the lines of:Where
Arbitraries.<M>sequenceGen()
would accept aFunction<? super M, ? extends Arbitrary<Action<M>>
as argument. The choice actions would then depend the current state.sequenceGen
would be ideal for testing any kind of collection type like lists, maps, sets, ...The text was updated successfully, but these errors were encountered: