jqwik-team / jqwik

Property-Based Testing on the JUnit Platform
http://jqwik.net
Eclipse Public License 2.0
578 stars 64 forks source link

ActionChain failure include never-executed actions since it uses wrong state object when creating transformations #426

Closed vlsi closed 1 year ago

vlsi commented 1 year ago

Testing Problem

The issue has initially been described here: https://github.com/jlink/jqwik/issues/134#issuecomment-1167338041

State-depending transformation might use the state for, well, figuring out the available transformations. However, after the transformation is applied, the set of "allowable" transformations might change, so jqwik should use only the valid state objects when deriving transformations.

Here's a test case that shows invalid result produced by chain.transformations().

I believe, the root cause here is that net.jqwik.engine.properties.state.ShrinkableChainIteration#transformer re-instantiates transformation shrinkable, and it uses the same state object for instantiating transformers and taking their names.

The exact seed does not matter much, however, the failure scenarios are different depending on the seed.

seed=42
// As you see the actual ".transformations()" produces a weird "add 10 to []" as if adding 7 was ignored.

[chain.transformations(), final state is []] 
expected: ["add 7 to []", "add 10 to [7]", "clear [7, 10]", "clear []"]
 but was: ["add 7 to []", "add 10 to []", "clear [7, 10]", "clear []"]

seed=43
// Here "adding elements after clear" is printed as "noop"
expected: ["clear []", "add 10 to []", "add 7 to [10]", "add 4 to [7, 10]"]
 but was: ["clear []", "noop", "noop", "noop"]
@Property(seed = "43")
void chainActionsAreProperlyDescribedEvenAfterChainExecution(@ForAll Random random) {
    List<String> actualOps = new ArrayList<>();
    ActionChainArbitrary<Set<Integer>> chains =
        ActionChain.<Set<Integer>>startWith(HashSet::new)
                   .addAction(
                       1,
                       (Action.Dependent<Set<Integer>>)
                           state ->
                               Arbitraries.just(
                                   Transformer.<Set<Integer>>mutate("clear " + state, set -> {
                                       actualOps.add("clear " + set);
                                       set.clear();
                                   })
                               )
                   )
                   .addAction(
                       2,
                       (Action.Dependent<Set<Integer>>)
                           state ->
                               Arbitraries.integers()
                                   .between(1, 10)
                                   .map(i -> {
                                            if (state.contains(i)) {
                                                return Transformer.noop();
                                            } else {
                                                return Transformer.mutate("add " + i + " to " + state, set -> {
                                                    actualOps.add("add " + i + " to " + set);
                                                    set.add(i);
                                                });
                                            }
                                        }
                                   )
                   )
                   .withMaxTransformations(4);

    ActionChain<Set<Integer>> chain = TestingSupport.generateFirst(chains, random);
    Set<Integer> finalState = chain.run();
    assertThat(chain.transformations())
        .describedAs("chain.transformations(), final state is %s", finalState)
        .isEqualTo(actualOps);
}
vlsi commented 1 year ago

Here's a better test case:


class SetMutatingChainState {
    final List<String> actualOps = new ArrayList<>();
    final Set<Integer> set = new HashSet<>();

    @Override
    public String toString() {
        return "set=" + set + ", actualOps=" + actualOps;
    }
}

@Property
void chainActionsAreProperlyDescribedEvenAfterChainExecution(@ForAll("setMutatingChain") ActionChain<SetMutatingChainState> chain) {
    SetMutatingChainState finalState = chain.run();

    assertThat(chain.transformations())
        .describedAs("chain.transformations(), final state is %s", finalState.set)
        .isEqualTo(finalState.actualOps);
}

@Provide
public ActionChainArbitrary<SetMutatingChainState> setMutatingChain() {
    return
        ActionChain
            .startWith(SetMutatingChainState::new)
            .addAction(
                1, (Action.Dependent<SetMutatingChainState>) state -> Arbitraries.just(
                    state.set.isEmpty()
                        ? Transformer.noop()
                        : Transformer.<SetMutatingChainState>mutate("clear " + state.set, set -> {
                        state.actualOps.add("clear " + set.set);
                        state.set.clear();
                    })
                )
            )
            .addAction(
                2,
                (Action.Dependent<SetMutatingChainState>)
                    state ->
                        Arbitraries
                            .integers()
                            .between(1, 10)
                            .map(i -> {
                                     if (state.set.contains(i)) {
                                         return Transformer.noop();
                                     } else {
                                         return Transformer.mutate("add " + i + " to " + state.set, newState -> {
                                             newState.actualOps.add("add " + i + " to " + newState.set);
                                             newState.set.add(i);
                                         });
                                     }
                                 }
                            )
            )
            .withMaxTransformations(4);
}

The failure is like

timestamp = 2022-12-12T14:08:41.120091, ActionChainArbitraryTests:chainActionsAreProperlyDescribedEvenAfterChainExecution = 
  org.opentest4j.AssertionFailedError:
    [chain.transformations(), final state is [1]] 
    expected: ["add 1 to []"]
     but was: ["add 1 to [2, 8, 9, 10]"]

                              |-----------------------jqwik-----------------------
tries = 1                     | # of calls to property
checks = 1                    | # of not rejected calls
generation = RANDOMIZED       | parameters are randomly generated
after-failure = SAMPLE_FIRST  | try previously failed sample, then previous seed
when-fixed-seed = ALLOW       | fixing the random seed is allowed
edge-cases#mode = MIXIN       | edge cases are mixed in
edge-cases#total = 0          | # of all combined edge cases
edge-cases#tried = 0          | # of edge cases tried in current run
seed = 8966156234646364899    | random seed to reproduce generated values

Shrunk Sample (4 steps)
-----------------------
  chain: ActionChain[NOT_RUN]: 1 max actions

  After Execution
  ---------------
    chain: ActionChain[SUCCEEDED]: ["add 1 to [2, 8, 9, 10]"]

Original Sample
---------------
  chain: ActionChain[NOT_RUN]: 4 max actions

  After Execution
  ---------------
    chain: ActionChain[SUCCEEDED]: ["noop", "noop", "noop", "noop"]

  Original Error
  --------------
  org.opentest4j.AssertionFailedError:
    [chain.transformations(), final state is [2, 8, 9, 10]] 
    expected: ["add 2 to []", "add 9 to [2]", "add 10 to [2, 9]", "add 8 to [2, 9, 10]"]
     but was: ["noop", "noop", "noop", "noop"]

[chain.transformations(), final state is [1]] 
expected: ["add 1 to []"]
 but was: ["add 1 to [2, 8, 9, 10]"]
org.opentest4j.AssertionFailedError: [chain.transformations(), final state is [1]] 
expected: ["add 1 to []"]
 but was: ["add 1 to [2, 8, 9, 10]"]
    at java.base@11.0.13/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
    at java.base@11.0.13/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
    at java.base@11.0.13/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
    at app//net.jqwik.engine.properties.state.ActionChainArbitraryTests.chainActionsAreProperlyDescribedEvenAfterChainExecution(ActionChainArbitraryTests.java:130)
    at java.base@11.0.13/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base@11.0.13/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base@11.0.13/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base@11.0.13/java.lang.reflect.Method.invoke(Method.java:566)
    at app//org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:727)
    at app//org.junit.platform.commons.support.ReflectionSupport.invokeMethod(ReflectionSupport.java:198)
    at app//net.jqwik.engine.execution.CheckedPropertyFactory.lambda$createRawFunction$1(CheckedPropertyFactory.java:84)
    at app//net.jqwik.engine.execution.CheckedPropertyFactory.lambda$createRawFunction$2(CheckedPropertyFactory.java:91)
    at app//net.jqwik.engine.properties.CheckedFunction.execute(CheckedFunction.java:17)
    at app//net.jqwik.api.lifecycle.AroundTryHook.lambda$static$0(AroundTryHook.java:57)
    at app//net.jqwik.engine.execution.lifecycle.HookSupport.lambda$wrap$2(HookSupport.java:48)
    at app//net.jqwik.engine.hooks.lifecycle.TryLifecycleMethodsHook.aroundTry(TryLifecycleMethodsHook.java:57)
    at app//net.jqwik.engine.execution.lifecycle.HookSupport.lambda$wrap$3(HookSupport.java:53)
    at app//net.jqwik.engine.execution.CheckedPropertyFactory.lambda$createTryExecutor$0(CheckedPropertyFactory.java:60)
    at app//net.jqwik.engine.execution.lifecycle.AroundTryLifecycle.execute(AroundTryLifecycle.java:23)
    at app//net.jqwik.engine.properties.GenericProperty.lambda$createFalsifier$1(GenericProperty.java:229)
    at app//net.jqwik.engine.properties.shrinking.PropertyShrinker.lambda$shrink$0(PropertyShrinker.java:55)
    at app//net.jqwik.engine.properties.shrinking.PropertyShrinker.lambda$shrinkAsLongAsSampleImproves$6(PropertyShrinker.java:122)
    at app//net.jqwik.engine.properties.shrinking.AbstractSampleShrinker.lambda$falsify$5(AbstractSampleShrinker.java:91)
    at java.base@11.0.13/java.util.HashMap.computeIfAbsent(HashMap.java:1134)
    at app//net.jqwik.engine.properties.shrinking.AbstractSampleShrinker.falsify(AbstractSampleShrinker.java:91)
    at app//net.jqwik.engine.properties.shrinking.AbstractSampleShrinker.lambda$shrink$2(AbstractSampleShrinker.java:53)
    at java.base@11.0.13/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:195)
    at java.base@11.0.13/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:177)
    at java.base@11.0.13/java.util.stream.ReferencePipeline$11$1.accept(ReferencePipeline.java:442)
    at java.base@11.0.13/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:195)
    at java.base@11.0.13/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:195)
    at java.base@11.0.13/java.util.stream.StreamSpliterators$WrappingSpliterator.tryAdvance(StreamSpliterators.java:302)
    at java.base@11.0.13/java.util.stream.Streams$ConcatSpliterator.tryAdvance(Streams.java:720)
    at java.base@11.0.13/java.util.stream.Streams$ConcatSpliterator.tryAdvance(Streams.java:727)
    at java.base@11.0.13/java.util.stream.Streams$ConcatSpliterator.tryAdvance(Streams.java:727)
    at java.base@11.0.13/java.util.stream.Streams$ConcatSpliterator.tryAdvance(Streams.java:727)
    at java.base@11.0.13/java.util.stream.ReferencePipeline.forEachWithCancel(ReferencePipeline.java:127)
    at java.base@11.0.13/java.util.stream.AbstractPipeline.copyIntoWithCancel(AbstractPipeline.java:502)
    at java.base@11.0.13/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:488)
    at java.base@11.0.13/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
    at java.base@11.0.13/java.util.stream.FindOps$FindOp.evaluateSequential(FindOps.java:150)
    at java.base@11.0.13/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    at java.base@11.0.13/java.util.stream.ReferencePipeline.findAny(ReferencePipeline.java:548)
    at app//net.jqwik.engine.properties.shrinking.AbstractSampleShrinker.shrink(AbstractSampleShrinker.java:63)
    at app//net.jqwik.engine.properties.shrinking.OneAfterTheOtherParameterShrinker.shrinkSingleParameter(OneAfterTheOtherParameterShrinker.java:43)
    at app//net.jqwik.engine.properties.shrinking.OneAfterTheOtherParameterShrinker.shrink(OneAfterTheOtherParameterShrinker.java:25)
    at app//net.jqwik.engine.properties.shrinking.ShrinkingAlgorithm.shrinkOneParameterAfterTheOther(ShrinkingAlgorithm.java:52)
    at app//net.jqwik.engine.properties.shrinking.ShrinkingAlgorithm.shrink(ShrinkingAlgorithm.java:32)
    at app//net.jqwik.engine.properties.shrinking.PropertyShrinker.shrinkAsLongAsSampleImproves(PropertyShrinker.java:135)
    at app//net.jqwik.engine.properties.shrinking.PropertyShrinker.lambda$shrink$3(PropertyShrinker.java:87)
    at app//net.jqwik.engine.execution.lifecycle.CurrentTestDescriptor.runWithDescriptor(CurrentTestDescriptor.java:17)
    at app//net.jqwik.engine.properties.shrinking.PropertyShrinker.lambda$withTimeout$4(PropertyShrinker.java:103)
    at app//net.jqwik.engine.execution.lifecycle.CurrentDomainContext.runWithContext(CurrentDomainContext.java:28)