Skip to content

Commit 5b9c3b2

Browse files
author
Vladimir Sitnikov
committed
Fix ActionChain failure include never-executed actions since it uses wrong state object when creating transformations
This change caches the transformation if it accesses state. fixes #426
1 parent 3de9ca9 commit 5b9c3b2

File tree

2 files changed

+79
-3
lines changed

2 files changed

+79
-3
lines changed

engine/src/main/java/net/jqwik/engine/properties/state/ShrinkableChainIteration.java

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,12 @@
66
import net.jqwik.api.*;
77
import net.jqwik.api.state.*;
88

9+
import org.jetbrains.annotations.*;
10+
911
class ShrinkableChainIteration<T> {
1012
final Shrinkable<Transformer<T>> shrinkable;
1113
private final Predicate<T> precondition;
14+
private final @Nullable Transformer<T> transformer;
1215
final boolean accessState;
1316
final boolean changeState;
1417

@@ -31,18 +34,21 @@ private ShrinkableChainIteration(
3134
this.accessState = accessState;
3235
this.changeState = changeState;
3336
this.shrinkable = shrinkable;
37+
// Transformer method might access state, so we need to cache the value here
38+
// otherwise it might be evaluated with wrong state (e.g. after chain executes)
39+
this.transformer = accessState ? shrinkable.value() : null;
3440
}
3541

3642
@Override
3743
public String toString() {
3844
return String.format(
3945
"Iteration[accessState=%s, changeState=%s, transformation=%s]",
40-
accessState, changeState, shrinkable.value().transformation()
46+
accessState, changeState, transformer().transformation()
4147
);
4248
}
4349

4450
boolean isEndOfChain() {
45-
return shrinkable.value().equals(Transformer.END_OF_CHAIN);
51+
return transformer().equals(Transformer.END_OF_CHAIN);
4652
}
4753

4854
Optional<Predicate<T>> precondition() {
@@ -65,6 +71,6 @@ String transformation() {
6571
}
6672

6773
Transformer<T> transformer() {
68-
return shrinkable.value();
74+
return transformer == null ? shrinkable.value() : transformer;
6975
}
7076
}

engine/src/test/java/net/jqwik/engine/properties/state/ActionChainArbitraryTests.java

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,76 @@ ActionChainArbitrary<String> xOrFailing() {
119119
.withMaxTransformations(30);
120120
}
121121

122+
static class SetMutatingChainState {
123+
final List<String> actualOps = new ArrayList<>();
124+
final Set<Integer> set = new HashSet<>();
125+
126+
@Override
127+
public String toString() {
128+
return "set=" + set + ", actualOps=" + actualOps;
129+
}
130+
}
131+
132+
@Property
133+
void chainActionsAreProperlyDescribedEvenAfterChainExecution(@ForAll("setMutatingChain") ActionChain<SetMutatingChainState> chain) {
134+
SetMutatingChainState finalState = chain.run();
135+
136+
assertThat(chain.transformations())
137+
.describedAs("chain.transformations() should be the same as the list of operations in finalState.actualOps, final state is %s", finalState.set)
138+
.isEqualTo(finalState.actualOps);
139+
}
140+
141+
@Provide
142+
public ActionChainArbitrary<SetMutatingChainState> setMutatingChain() {
143+
return
144+
ActionChain
145+
.startWith(SetMutatingChainState::new)
146+
// This is an action that does not depend on the state to produce the transformation
147+
.addAction(
148+
1,
149+
Action.just("clear anyway", state -> {
150+
state.actualOps.add("clear anyway");
151+
state.set.clear();
152+
return state;
153+
})
154+
)
155+
// Below actions depend on the state to derive the transformations
156+
.addAction(
157+
1,
158+
(Action.Dependent<SetMutatingChainState>)
159+
state ->
160+
Arbitraries
161+
.just(
162+
state.set.isEmpty()
163+
? Transformer.noop()
164+
: Transformer.<SetMutatingChainState>mutate("clear " + state.set, set -> {
165+
state.actualOps.add("clear " + set.set);
166+
state.set.clear();
167+
})
168+
)
169+
)
170+
.addAction(
171+
2,
172+
(Action.Dependent<SetMutatingChainState>)
173+
state ->
174+
Arbitraries
175+
.integers()
176+
.between(1, 10)
177+
.map(i -> {
178+
if (state.set.contains(i)) {
179+
return Transformer.noop();
180+
} else {
181+
return Transformer.mutate("add " + i + " to " + state.set, newState -> {
182+
newState.actualOps.add("add " + i + " to " + newState.set);
183+
newState.set.add(i);
184+
});
185+
}
186+
}
187+
)
188+
)
189+
.withMaxTransformations(5);
190+
}
191+
122192
@Property
123193
void chainChoosesBetweenTwoActions(@ForAll("xOrY") ActionChain<String> chain) {
124194
String result = chain.run();

0 commit comments

Comments
 (0)