[JBPM-10231] Fixed all non-idempotent tests #2422
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kaiyaok2
changed the title
|
@gmunozfe Please kindly lmk if I shall merge this to the previous PR |
|
jenkins test this |
kaiyaok2
changed the title
|
@gmunozfe Thanks for your message. Now all relevant code changes are in this PR. |
|
jenkins test this |
|
gmunozfe
added
backport-7.67.x
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 newKieSession. However, the exception to this is whengetRuntimeEngine()invoked within the same transaction from different places (which would be the case iftestRunMultiEventProcessPerRequestRuntimeManageris 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)KieSessionis already closed by the previous test execution. To resolve this, the runtime engine manager shall be disposed to ensure that itsKieSessionis destroyed as well.Error message of the test in the repeated run:
Fix: Add the line
manager.disposeRuntimeEngine(runtime);before closing the managerPerCaseRuntimeManagerTest#testMultipleProcessesInSingleCaseCompletedInSequence
Reason: The "Per Case" strategy specifies that every process instance that belongs to same case will be bound to a single
ksessionfor its entire life time. Once started, whenever such operations are invoked, the manager will retrieve the sameksession. Similarly,manager.disposeRuntimeEngine(runtime)shall be called beforemanager.close()to ensure that the second test execution does not directly retrieve the already closedksession.Error message in the repeated run:
Fix: Same as the previous test: add
manager.disposeRuntimeEngine(runtime);before closing the managerPerRequestRuntimeManagerTest#testMultiplePerRequestManagerFromSingleThread
Reason: This is actually a typo - the process instance
runtime2is managed bymanager2, notmanager.The original code is
It shall be changed to
Error message of one of the tests in the repeated run:
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 viaunits.add(deploymentUnit);, and every deploy unit in the listunitsis undeployed in thecleanup()method. However, intestSerializationClassesLimitedInDeploymentDependencies(), multiple deploy units are constructed (childDeploymentUnitandparentDeploymentUnit) and are not directly added tounits. Therefore, these units are not undeployed after test execution, so the second test execution will throw an error.Error message in the second run:
Fix: Add
childDeploymentUnitandparentDeploymentUnitto the listunitsat the end of the test.FilteredKModuleDeploymentServiceTest#testSerializationClassesLimitedInDeploymentItself
Reason: Similar with the test above,
testSerializationClassesLimitedInDeploymentItself()constructs multiple deploy units (limitDeploymentUnitandallDeploymentUnit) without adding them tounits, so they're not undeployed after test execution.Error message in the second run:
Fix: Add
limitDeploymentUnitandallDeploymentUnitto the listunitsat 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 inIncrementServicerecords the total number of service processes, so repeated test runs will still increment the counter.Error message in the second run:
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, thedoCacheClientflag is enabled, meaning that repeated calls toexecuteWorkItem()with the same parameters will use the cached client and not invokegetNewPooledHttpClient()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:
Fix: Ensure there's no existing
cachedClientwhen startingtestPooledClientCreationWithDefaultTimeouts().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, themanageris not closed at the end of the test, so repeated runs will throw "already exists" error.Error message in the second run:
Fix: Add a
tearDown()method to clear the createdpersonas well as its application in the database after test execution. Also, close themanagerat the end of the test.AsyncWIHOnOracleTest#testAsyncWIHExecutedMoreThanOnceOnOracle
Reason: The helper class
CounterCommandrecords the global number of command executions. The test asserts thatCounterCommand.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:
Fix: Add a
resetCounter()function in the helper class and call it in thetearDown()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 (vialatch.await()) and assertsprocessInstanceis cleared (null) after that. However, the static latch is not reset after the first test execution, solatch.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:
Fix: Construct a new latch for each test execution.
MBeansMonitoringWithJBpmTest#testRulesAndProcesses
Reason: In each test execution, a KieContainer
kcis built by callingnewKieContainer(). The test does not disposekcafter 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:
Fix: Clear
kcat the end of the test.