Fixing status flaky test #3777
Merged
Fixing status flaky test #3777
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
fjtirado
commented
Nov 14, 2024
| @@ -400,8 +400,7 @@ public void testEventBasedSplit2() throws Exception { | |||
| org.kie.kogito.process.ProcessInstance<EventBasedSplit2Model> instanceTimer = processDefinition.createInstance(modelTimer); | |||
| instanceTimer.start(); | |||
| assertThat(instanceTimer.status()).isEqualTo(org.kie.kogito.process.ProcessInstance.STATE_ACTIVE); | |||
| assertThat(countDownListener.waitTillCompleted(15000)).isTrue(); | |||
| assertThat(instanceYes.status()).isEqualTo(org.kie.kogito.process.ProcessInstance.STATE_COMPLETED); | |||
There was a problem hiding this comment.
This is a leffover, instanceYes has already being checked (btw, this probably makes the issue less frequent adding a small delay ;))
I also restored the timer to the default value (since that was not the issue)
fjtirado
commented
Nov 14, 2024
jbpm/jbpm-flow/src/main/java/org/kie/kogito/process/impl/AbstractProcessInstance.java
Outdated
Show resolved
Hide resolved
fjtirado
force-pushed
the
fixing_flaky_test_2
branch
from
4177278 to
f2390de
Compare
There is a chance that status is invoked over a process instance that has been finished, but the container kogito process instance does not know yet, even if processcompleted listener has been executed (and thats the issue) Although this can be fixed by altering the order in which the different listener are invoked (which might probably have other undesired effect), I do not see a reason why status should not be read from underlying process instance, if present. Probably processInstance field should be embedded into an atomic reference, but volatile might have the same effect with few code changes.
fjtirado
force-pushed
the
fixing_flaky_test_2
branch
from
f2390de to
dfd99df
Compare
|
PR job Reproducerbuild-chain build full_downstream -f 'https://raw.githubusercontent.com/${AUTHOR:apache}/incubator-kie-kogito-pipelines/${BRANCH:main}/.ci/buildchain-config-pr-cdb.yaml' -o 'bc' -p apache/incubator-kie-kogito-runtimes -u #3777 --skipParallelCheckout NOTE: To install the build-chain tool, please refer to https://github.com/kiegroup/github-action-build-chain#local-execution Please look here: https://ci-builds.apache.org/job/KIE/job/kogito/job/main/job/pullrequest_jobs/job/kogito-runtimes-pr/job/PR-3777/3/display/redirect Test results:
Those are the test failures: org.kie.persistence.filesystem.FileSystemProcessInstancesTest.testBasicFlowExpecting value to be true but was false org.kie.persistence.filesystem.FileSystemProcessInstancesTest.testBasicFlowWithStartFromCannot invoke "org.jbpm.workflow.instance.WorkflowProcessInstance.getVariables()" because "this.processInstance" is nullorg.kie.persistence.jdbc.PostgreSqlProcessInstancesIT.testBasicFlowNo value presentorg.kie.persistence.jdbc.PostgreSqlProcessInstancesIT.testMultipleProcessesExpected size: 1 but was: 0 in: [] org.kie.persistence.jdbc.PostgreSqlProcessInstancesIT.testProcessWithDifferentVersionExpected size: 1 but was: 0 in: [] org.kie.persistence.jdbc.PostgreSqlProcessInstancesIT.testBasicTaskFlowExpecting value to be true but was false org.kie.persistence.jdbc.PostgreSqlProcessInstancesIT.testUpdateNo value presentorg.kie.persistence.jdbc.PostgreSqlProcessInstancesLockIT.testMigrateSingleProcess instance with id '93678156-9b87-4d63-9840-78bd53cf3c56' updated or deleted by other requestorg.kie.persistence.jdbc.PostgreSqlProcessInstancesLockIT.testMigrateAllProcess instance with id '471ba762-b82c-4748-9d6c-2912f3155ace' updated or deleted by other requestorg.kie.persistence.jdbc.PostgreSqlProcessInstancesLockIT.testBasicFlowProcess instance with id '4ae463b8-1971-4492-89e0-46a7b698cb46' updated or deleted by other requestorg.kie.persistence.jdbc.PostgreSqlProcessInstancesLockIT.testMultipleProcessesProcess instance with id '648b8a1d-76ac-4222-a6cb-0013ca5c4436' updated or deleted by other requestorg.kie.persistence.jdbc.PostgreSqlProcessInstancesLockIT.testProcessWithDifferentVersionProcess instance with id '0e4d0dc1-6414-4ee3-ae2d-0daa69c283f1' updated or deleted by other requestorg.kie.persistence.jdbc.PostgreSqlProcessInstancesLockIT.testBasicTaskFlowProcess instance with id 'ca019b0b-7bb2-4dc4-b969-4ba52fc5a316' updated or deleted by other requestorg.kie.persistence.jdbc.PostgreSqlProcessInstancesLockIT.testRemoveProcess instance with id 'a835b0c3-d594-4452-aee5-50a55738f6d0' updated or deleted by other requestorg.kie.persistence.jdbc.PostgreSqlProcessInstancesLockIT.testUpdateProcess instance with id '05beb70d-669c-4275-a42a-e928d8adf2f0' updated or deleted by other requestorg.kie.persistence.postgresql.PostgresqlProcessInstancesIT.testMigrateSingleCannot invoke "org.jbpm.process.instance.ProcessInstance.configureTimers()" because "jbpmProcessInstance" is nullorg.kie.persistence.postgresql.PostgresqlProcessInstancesIT.testMigrateAllCannot invoke "org.jbpm.process.instance.ProcessInstance.configureTimers()" because "jbpmProcessInstance" is nullorg.kie.persistence.postgresql.PostgresqlProcessInstancesIT.testBasicFlowCannot invoke "org.jbpm.process.instance.ProcessInstance.configureTimers()" because "jbpmProcessInstance" is nullorg.kie.persistence.postgresql.PostgresqlProcessInstancesIT.testMultipleProcessesCannot invoke "org.jbpm.process.instance.ProcessInstance.configureTimers()" because "jbpmProcessInstance" is nullorg.kie.persistence.postgresql.PostgresqlProcessInstancesIT.testProcessWithDifferentVersionCannot invoke "org.jbpm.process.instance.ProcessInstance.configureTimers()" because "jbpmProcessInstance" is nullorg.kie.persistence.postgresql.PostgresqlProcessInstancesIT.testRemoveCannot invoke "org.jbpm.process.instance.ProcessInstance.configureTimers()" because "jbpmProcessInstance" is nullorg.kie.persistence.postgresql.PostgresqlProcessInstancesIT.testUpdateCannot invoke "org.jbpm.process.instance.ProcessInstance.configureTimers()" because "jbpmProcessInstance" is nullorg.kie.persistence.postgresql.PostgresqlProcessInstancesWithLockIT.testMigrateSingleCannot invoke "org.jbpm.process.instance.ProcessInstance.configureTimers()" because "jbpmProcessInstance" is nullorg.kie.persistence.postgresql.PostgresqlProcessInstancesWithLockIT.testMigrateAllCannot invoke "org.jbpm.process.instance.ProcessInstance.configureTimers()" because "jbpmProcessInstance" is nullorg.kie.persistence.postgresql.PostgresqlProcessInstancesWithLockIT.testBasicFlowCannot invoke "org.jbpm.process.instance.ProcessInstance.configureTimers()" because "jbpmProcessInstance" is nullorg.kie.persistence.postgresql.PostgresqlProcessInstancesWithLockIT.testMultipleProcessesCannot invoke "org.jbpm.process.instance.ProcessInstance.configureTimers()" because "jbpmProcessInstance" is nullorg.kie.persistence.postgresql.PostgresqlProcessInstancesWithLockIT.testProcessWithDifferentVersionCannot invoke "org.jbpm.process.instance.ProcessInstance.configureTimers()" because "jbpmProcessInstance" is nullorg.kie.persistence.postgresql.PostgresqlProcessInstancesWithLockIT.testRemoveCannot invoke "org.jbpm.process.instance.ProcessInstance.configureTimers()" because "jbpmProcessInstance" is nullorg.kie.persistence.postgresql.PostgresqlProcessInstancesWithLockIT.testUpdateCannot invoke "org.jbpm.process.instance.ProcessInstance.configureTimers()" because "jbpmProcessInstance" is null |
|
Tries a first commit that read state from process insance if process instance is not null. Not working, because disconnected process instance are not null, and if set to null, it causes side effect on persistece module, Try altering the order of listeners. |
rgdoliveira
pushed a commit
to rgdoliveira/kogito-runtimes
that referenced
this pull request
Nov 18, 2024
* Fixing status flaky test There is a chance that status is invoked over a process instance that has been finished, but the container kogito process instance does not know yet, even if processcompleted listener has been executed (and thats the issue) Although this can be fixed by altering the order in which the different listener are invoked (which might probably have other undesired effect), I do not see a reason why status should not be read from underlying process instance, if present. Probably processInstance field should be embedded into an atomic reference, but volatile might have the same effect with few code changes. * Try changing order of listeners
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.
AbstracPRocessInstance.getStatus does not inmediately match WorfklowProcessIntance.getState after KogitoProcessEventListener.afterProcessCompleted is invoked (eventually they are sync). There is a chance that tests that relies on afterProcessCompleted to handle CountdownLatch receive a wrong status is they check the status just after the countdownlatch reach zero (thats why those test are flaky)