jbpm icon indicating copy to clipboard operation
jbpm copied to clipboard

Published 20 hours ago • verified publisher icon kiegroup

[JBPM-10231] Fixed all non-idempotent tests

Open kaiyaok2 opened this issue 8 months ago • 6 comments

This PR supersedes #2421 - it fixes all non-idempotent unit tests. All tests pass and are idempotent after the fix. Below are all tests fixed:

EventHnadlingTest#testRunMultiEventProcessPerRequestRuntimeManager

Reason: With "Per Request" strategy, for every call to getRuntimeEngine(), a new instance will usually be delivered with brand new KieSession. However, the exception to this is when getRuntimeEngine() invoked within the same transaction from different places (which would be the case if testRunMultiEventProcessPerRequestRuntimeManager is re-executed). In that case, the manager caches the currently active instance. Therefore, in the second execution of the test, an error will be thrown since the retrieved (cached) KieSession is already closed by the previous test execution. To resolve this, the runtime engine manager shall be disposed to ensure that its KieSession is destroyed as well. 

Error message of the test in the repeated run:

java.lang.IllegalStateException: Session/EntityManager is closed
	at org.hibernate.internal.AbstractSharedSessionContract.checkOpen(AbstractSharedSessionContract.java:375)
	at org.hibernate.engine.spi.SharedSessionContractImplementor.checkOpen(SharedSessionContractImplementor.java:148)
	at org.hibernate.internal.SessionImpl.joinTransaction(SessionImpl.java:3633)
	at org.drools.persistence.jpa.JpaPersistenceContext.joinTransaction(JpaPersistenceContext.java:106)
	at org.drools.persistence.PersistableRunner$TransactionInterceptor.execute(PersistableRunner.java:603)
	at org.drools.persistence.PersistableRunner$TransactionInterceptor.execute(PersistableRunner.java:572)
	at org.drools.core.command.impl.AbstractInterceptor.executeNext(AbstractInterceptor.java:39)
	at org.drools.persistence.jpa.OptimisticLockRetryInterceptor.internalExecute(OptimisticLockRetryInterceptor.java:102)
	at org.drools.persistence.jpa.OptimisticLockRetryInterceptor.execute(OptimisticLockRetryInterceptor.java:83)
	at org.drools.persistence.jpa.OptimisticLockRetryInterceptor.execute(OptimisticLockRetryInterceptor.java:44)
	at org.drools.core.command.impl.AbstractInterceptor.executeNext(AbstractInterceptor.java:39)
	at org.drools.persistence.jta.TransactionLockInterceptor.execute(TransactionLockInterceptor.java:73)
	at org.drools.persistence.jta.TransactionLockInterceptor.execute(TransactionLockInterceptor.java:45)
	at org.drools.core.command.impl.AbstractInterceptor.executeNext(AbstractInterceptor.java:39)
	at org.jbpm.runtime.manager.impl.error.ExecutionErrorHandlerInterceptor.internalExecute(ExecutionErrorHandlerInterceptor.java:66)
	at org.jbpm.runtime.manager.impl.error.ExecutionErrorHandlerInterceptor.execute(ExecutionErrorHandlerInterceptor.java:52)
	at org.jbpm.runtime.manager.impl.error.ExecutionErrorHandlerInterceptor.execute(ExecutionErrorHandlerInterceptor.java:29)
	at org.drools.persistence.PersistableRunner.execute(PersistableRunner.java:400)
	at org.drools.persistence.PersistableRunner.execute(PersistableRunner.java:68)
	at org.drools.core.runtime.InternalLocalRunner.execute(InternalLocalRunner.java:37)
	at org.drools.core.runtime.InternalLocalRunner.execute(InternalLocalRunner.java:41)
	at org.drools.core.command.impl.CommandBasedStatefulKnowledgeSession.startProcess(CommandBasedStatefulKnowledgeSession.java:275)
	at org.drools.core.command.impl.CommandBasedStatefulKnowledgeSession.startProcess(CommandBasedStatefulKnowledgeSession.java:259)
	at org.jbpm.runtime.manager.concurrent.EventHnadlingTest.testRunMultiEventProcessPerRequestRuntimeManager(EventHnadlingTest.java:86)

Fix: Add the line manager.disposeRuntimeEngine(runtime); before closing the manager

PerCaseRuntimeManagerTest#testMultipleProcessesInSingleCaseCompletedInSequence

Reason: The "Per Case" strategy specifies that every process instance that belongs to same case will be bound to a single ksession for its entire life time.  Once started, whenever such operations are invoked, the manager will retrieve the same ksession. Similarly, manager.disposeRuntimeEngine(runtime) shall be called before manager.close() to ensure that the second test execution does not directly retrieve the already closed ksession.

Error message in the repeated run:

java.lang.IllegalStateException: Session/EntityManager is closed
	at org.hibernate.internal.AbstractSharedSessionContract.checkOpen(AbstractSharedSessionContract.java:375)
	at org.hibernate.engine.spi.SharedSessionContractImplementor.checkOpen(SharedSessionContractImplementor.java:148)
	at org.hibernate.internal.SessionImpl.joinTransaction(SessionImpl.java:3633)
	at org.drools.persistence.jpa.JpaPersistenceContext.joinTransaction(JpaPersistenceContext.java:106)
	at org.drools.persistence.PersistableRunner$TransactionInterceptor.execute(PersistableRunner.java:603)
	at org.drools.persistence.PersistableRunner$TransactionInterceptor.execute(PersistableRunner.java:572)
	at org.drools.core.command.impl.AbstractInterceptor.executeNext(AbstractInterceptor.java:39)
	at org.drools.persistence.jpa.OptimisticLockRetryInterceptor.internalExecute(OptimisticLockRetryInterceptor.java:102)
	at org.drools.persistence.jpa.OptimisticLockRetryInterceptor.execute(OptimisticLockRetryInterceptor.java:83)
	at org.drools.persistence.jpa.OptimisticLockRetryInterceptor.execute(OptimisticLockRetryInterceptor.java:44)
	at org.drools.core.command.impl.AbstractInterceptor.executeNext(AbstractInterceptor.java:39)
	at org.drools.persistence.jta.TransactionLockInterceptor.execute(TransactionLockInterceptor.java:73)
	at org.drools.persistence.jta.TransactionLockInterceptor.execute(TransactionLockInterceptor.java:45)
	at org.drools.core.command.impl.AbstractInterceptor.executeNext(AbstractInterceptor.java:39)
	at org.jbpm.runtime.manager.impl.error.ExecutionErrorHandlerInterceptor.internalExecute(ExecutionErrorHandlerInterceptor.java:66)
	at org.jbpm.runtime.manager.impl.error.ExecutionErrorHandlerInterceptor.execute(ExecutionErrorHandlerInterceptor.java:52)
	at org.jbpm.runtime.manager.impl.error.ExecutionErrorHandlerInterceptor.execute(ExecutionErrorHandlerInterceptor.java:29)
	at org.drools.persistence.PersistableRunner.execute(PersistableRunner.java:400)
	at org.drools.persistence.PersistableRunner.execute(PersistableRunner.java:68)
	at org.drools.core.runtime.InternalLocalRunner.execute(InternalLocalRunner.java:37)
	at org.drools.core.runtime.InternalLocalRunner.execute(InternalLocalRunner.java:41)
	at org.drools.core.command.impl.CommandBasedStatefulKnowledgeSession.getIdentifier(CommandBasedStatefulKnowledgeSession.java:134)
	at org.jbpm.runtime.manager.impl.PerCaseRuntimeManagerTest.testMultipleProcessesInSingleCaseCompletedInSequence(PerCaseRuntimeManagerTest.java:349)

Fix: Same as the previous test: add manager.disposeRuntimeEngine(runtime); before closing the manager

PerRequestRuntimeManagerTest#testMultiplePerRequestManagerFromSingleThread

Reason: This is actually a typo - the process instance runtime2 is managed by manager2, not manager. The original code is

manager.disposeRuntimeEngine(runtime1);
manager.disposeRuntimeEngine(runtime2);

It shall be changed to 

manager.disposeRuntimeEngine(runtime1);
manager2.disposeRuntimeEngine(runtime2);

Error message of one of the tests in the repeated run:

java.lang.IllegalStateException: RuntimeManager with id second is already active
	at org.jbpm.runtime.manager.impl.AbstractRuntimeManager.<init>(AbstractRuntimeManager.java:141)
	at org.jbpm.runtime.manager.impl.PerRequestRuntimeManager.<init>(PerRequestRuntimeManager.java:74)
	at org.jbpm.runtime.manager.impl.RuntimeManagerFactoryImpl.newPerRequestRuntimeManager(RuntimeManagerFactoryImpl.java:73)
	at org.jbpm.runtime.manager.impl.PerRequestRuntimeManagerTest.testMultiplePerRequestManagerFromSingleThread(PerRequestRuntimeManagerTest.java:543)

Fix: As described above

FilteredKModuleDeploymentServiceTest#testSerializationClassesLimitedInDeploymentDependencies

Reason: The test class keeps track of units, a list of that stores the Kmodule deploy units constructed by each unit test. Unit tests append to this list via units.add(deploymentUnit);, and every deploy unit in the list units is undeployed in the cleanup() method. However, in testSerializationClassesLimitedInDeploymentDependencies(), multiple deploy units are constructed (childDeploymentUnit and parentDeploymentUnit) and are not directly added to units. Therefore, these units are not undeployed after test execution, so the second test execution will throw an error.

Error message in the second run:

java.lang.RuntimeException: java.lang.RuntimeException: java.lang.IllegalStateException: RuntimeManager with id org.test:jbpm-kie-services-filter-test-dep:1.0.0 is already active
	at org.jbpm.kie.services.impl.KModuleDeploymentService.deploy(KModuleDeploymentService.java:221)
	at org.jbpm.kie.services.test.FilteredKModuleDeploymentServiceTest.testSerializationClassesLimitedInDeploymentDependencies(FilteredKModuleDeploymentServiceTest.java:429)

Fix: Add childDeploymentUnit and parentDeploymentUnit to the list units at the end of the test.

FilteredKModuleDeploymentServiceTest#testSerializationClassesLimitedInDeploymentItself

Reason: Similar with the test above, testSerializationClassesLimitedInDeploymentItself() constructs multiple deploy units (limitDeploymentUnit and allDeploymentUnit) without adding them to units, so they're not undeployed after test execution.

Error message in the second run:

java.lang.RuntimeException: java.lang.RuntimeException: java.lang.IllegalStateException: RuntimeManager with id org.test:jbpm-kie-services-filter-test:1.0.0 is already active
	at org.jbpm.kie.services.impl.KModuleDeploymentService.deploy(KModuleDeploymentService.java:221)
	at org.jbpm.kie.services.test.FilteredKModuleDeploymentServiceTest.testSerializationClassesLimitedInDeploymentItself(FilteredKModuleDeploymentServiceTest.java:359)

Fix: Add limitDeploymentUnit and allDeploymentUnit to the list units at the end of the test.

AsyncWorkItemHandlerTest#testRunProcessWithAsyncHandlerCallbackErrorRetry

Reason: The test asserts IncrementService.get() returns 0 at first and then 1 after executing the script task process. However, the static counter in IncrementService records the total number of service processes, so repeated test runs will still increment the counter.

Error message in the second run:

java.lang.AssertionError: expected:<0> but was:<1>
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.failNotEquals(Assert.java:835)
	at org.junit.Assert.assertEquals(Assert.java:647)
	at org.junit.Assert.assertEquals(Assert.java:633)
	at org.jbpm.executor.impl.wih.AsyncWorkItemHandlerTest.testRunProcessWithAsyncHandlerCallbackErrorRetry(AsyncWorkItemHandlerTest.java:530)

Fix: Record the value returned by IncrementService.get() in a local variable when starting the test. Check if it's unchanged at the time of the first original assertion, and if it's incremented by 1 at the time of the second original assertion. This preserves test logic while removes non-idempotency.

RestWorkitemHandlerClientCreationTest#testPooledClientCreationWithDefaultTimeouts

Reason: When mocking the class RESTWorkItemHandler, the doCacheClient flag is enabled, meaning that repeated calls to executeWorkItem() with the same parameters will use the cached client and not invoke getNewPooledHttpClient() repeatedly. In the repeated execution of this test, since the parameters are unchanged, getNewPooledHttpClient() will not be called at all, thus the exception.

Error message in the second run:

java.lang.AssertionError: expected:<0> but was:<1>
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.failNotEquals(Assert.java:835)
	at org.junit.Assert.assertEquals(Assert.java:647)
	at org.junit.Assert.assertEquals(Assert.java:633)
	at org.jbpm.executor.impl.wih.AsyncWorkItemHandlerTest.testRunProcessWithAsyncHandlerCallbackErrorRetry(AsyncWorkItemHandlerTest.java:530)

Fix: Ensure there's no existing cachedClient when starting testPooledClientCreationWithDefaultTimeouts().

ParentChildMarshallingJpaTest#testProcess

Reason: The test asserts that em.createQuery("select i from Person i").getResultList().size() is always 1 at the end of the test execution. However, each test execution will add a person to the database, so the size of the result list is accumulative, causing the assertion to fail in repeated runs. Also, the manager is not closed at the end of the test, so repeated runs will throw "already exists" error.

Error message in the second run:

java.lang.AssertionError: expected:<1> but was:<2>
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.failNotEquals(Assert.java:835)
	at org.junit.Assert.assertEquals(Assert.java:647)
	at org.junit.Assert.assertEquals(Assert.java:633)
	at org.jbpm.test.functional.jpa.ParentChildMarshallingJpaTest.testProcess(ParentChildMarshallingJpaTest.java:121)

Fix: Add a tearDown() method to clear the created person as well as its application in the database after test execution. Also, close the manager at the end of the test.

AsyncWIHOnOracleTest#testAsyncWIHExecutedMoreThanOnceOnOracle

Reason: The helper class CounterCommand records the global number of command executions. The test asserts that CounterCommand.getCounter() is always 1 after executing a process. However, repeated test runs will accumulate the counter, so the assertion would fail.

Error message in the second run:

org.opentest4j.AssertionFailedError: [The job has been executed multiple times] 
Expecting:
 <2>
to be equal to:
 <1>
but was not.
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
	at org.jbpm.test.regression.async.AsyncWIHOnOracleTest.testAsyncWIHExecutedMoreThanOnceOnOracle(AsyncWIHOnOracleTest.java:73)

Fix: Add a resetCounter() function in the helper class and call it in the tearDown() method.

AsyncMultisubprocessTest#testCorrectProcessStateAfterExceptionEndSignal

Reason: The test defines a static CountDownLatch, which counts down when the signal of process completion is received. Then the test waits for the countdown (via latch.await()) and asserts processInstance is cleared (null) after that. However, the static latch is not reset after the first test execution, so latch.await() will not wait in the repeated executions, so the assertion is made before the process completes, causing the error.

Error message in the second run:

org.opentest4j.AssertionFailedError: 
Expecting:
 <WorkflowProcessInstance1 [processId=TestSubMult,state=1]>
to be equal to:
 <null>
but was not.
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
	at org.jbpm.test.functional.async.AsyncMultisubprocessTest.testCorrectProcessStateAfterExceptionEndSignal(AsyncMultisubprocessTest.java:108)

Fix: Construct a new latch for each test execution.

MBeansMonitoringWithJBpmTest#testRulesAndProcesses

Reason: In each test execution, a KieContainer kc is built by calling newKieContainer(). The test does not dispose kc after test execution, so in the repeated test execution, newKieContainer() will throw an error since a KieContainer of the same ID is already existing.

Error message in the second run:

java.lang.IllegalStateException: There's already another KieContainer created with the id myContainerId
	at org.drools.compiler.kie.builder.impl.KieServicesImpl.newKieContainer(KieServicesImpl.java:215)
	at org.drools.compiler.kie.builder.impl.KieServicesImpl.newKieContainer(KieServicesImpl.java:180)
	at org.jbpm.test.functional.monitoring.MBeansMonitoringWithJBpmTest.testRulesAndProcesses(MBeansMonitoringWithJBpmTest.java:181)

Fix: Clear kc at the end of the test.

kaiyaok2 avatar May 26 '24 03:05 kaiyaok2