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

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

Closed
vlsi opened this issue Dec 12, 2022 · 1 comment · Fixed by #427

Comments

@vlsi
Copy link
Contributor

vlsi commented Dec 12, 2022

Testing Problem

The issue has initially been described here: #134 (comment)

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
Copy link
Contributor Author

vlsi commented Dec 12, 2022

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)

@vlsi vlsi changed the title Action chain steps should use only the most recent state object when creating transformations ActionChain failure include never-executed actions since it uses wrong state object when creating transformations Dec 12, 2022
vlsi pushed a commit to vlsi/jqwik that referenced this issue Dec 12, 2022
…wrong state object when creating transformations

This change caches the transformation if it accesses state.

fixes jqwik-team#426
jlink pushed a commit that referenced this issue Dec 18, 2022
…wrong state object when creating transformations

This change caches the transformation if it accesses state.

fixes #426
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant