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

[incubator-kie-issues-1131] test migration from V7 to code generation-36 #3682

Merged
merged 3 commits into from
Dec 20, 2024

Conversation

Abhitocode
Copy link
Contributor

@Abhitocode Abhitocode commented Sep 26, 2024

Few tests within jbpm-tests are disabled as on-exit is not supported. Check if they can be enabled and converted.

Reference issue link: https://issues.redhat.com/browse/KOGITO-2067

Below are the tests:
(https://github.com/apache/incubator-kie-kogito-runtimes/tree/main/jbpm/jbpm-tests/src/test/java/org/jbpm/bpmn2)

Below tests within IntermediateEventTest cannot be migrated as due to kie session not working in kogito
testDynamicCatchEventSignalWithVariableUpdated
testDynamicCatchEventSignalWithVariableUpdatedBroadcastSignal
image

Below tests within FlowTest fail when tried to enable and run in original v7 format,

testInclusiveSplitWithLoopInsideSubprocess - process goes to an error state, rather than maintaining the active state, when completing the work item for handler2. In LightWorkItemManager class, inside completeWorkItem method, processInstance.signalEvent("workItemCompleted", workItem) causing some error and not completing the work item

       Application app = ProcessTestHelper.newApplication();
       final Map<String, Integer> nodeInstanceExecutionCounter = new HashMap<>();
       ProcessTestHelper.registerProcessEventListener(app, new DefaultKogitoProcessEventListener() {

           @Override
           public void beforeNodeTriggered(ProcessNodeTriggeredEvent event) {
               logger.info("{} {}", event.getNodeInstance().getNodeName(), ((NodeInstanceImpl) event.getNodeInstance()).getLevel());
               Integer value = nodeInstanceExecutionCounter.get(event.getNodeInstance().getNodeName());
               if (value == null) {
                   value = 0;
               }

               value++;
               nodeInstanceExecutionCounter.put(event.getNodeInstance().getNodeName(), value);
           }

       });

       TestWorkItemHandler handler = new TestWorkItemHandler();
       TestWorkItemHandler handler2 = new TestWorkItemHandler();
       ProcessTestHelper.registerHandler(app, "testWI", handler);
       ProcessTestHelper.registerHandler(app, "testWI2", handler2);

       Process<InclusiveGatewayWithLoopInsideSubprocessModel> process = InclusiveGatewayWithLoopInsideSubprocessProcess.newProcess(app);
       InclusiveGatewayWithLoopInsideSubprocessModel model = process.createModel();
       model.setX(-1);
       ProcessInstance<InclusiveGatewayWithLoopInsideSubprocessModel> instance = process.createInstance(model);
       instance.start();

       assertThat(instance.status()).isEqualTo(ProcessInstance.STATE_ACTIVE);
       List<KogitoWorkItem> workItems = handler.getWorkItems();
       assertThat(workItems).isNotNull().hasSize(2);
       for (KogitoWorkItem wi : workItems) {
           assertThat(instance.status()).isEqualTo(ProcessInstance.STATE_ACTIVE);
           instance.completeWorkItem(wi.getStringId(), null);
       }
       assertThat(instance.status()).isEqualTo(ProcessInstance.STATE_ACTIVE);
       instance.completeWorkItem(handler2.getWorkItem().getStringId(), null);//After this line, state of the process instance is error state instead of active
       assertThat(instance.status()).isEqualTo(ProcessInstance.STATE_ACTIVE);
       instance.completeWorkItem(handler2.getWorkItem().getStringId(), null);
       assertThat(instance.status()).isEqualTo(ProcessInstance.STATE_ACTIVE);
       instance.completeWorkItem(handler.getWorkItem().getStringId(), null);
       assertThat(instance.status()).isEqualTo(ProcessInstance.STATE_COMPLETED);
       assertThat(nodeInstanceExecutionCounter).hasSize(13);
       assertThat((int) nodeInstanceExecutionCounter.get("Start")).isEqualTo(1);
       assertThat((int) nodeInstanceExecutionCounter.get("Sub Process 1")).isEqualTo(1);
       assertThat((int) nodeInstanceExecutionCounter.get("sb-start")).isEqualTo(1);
       assertThat((int) nodeInstanceExecutionCounter.get("sb-end")).isEqualTo(1);
       assertThat((int) nodeInstanceExecutionCounter.get("OR diverging")).isEqualTo(1);
       assertThat((int) nodeInstanceExecutionCounter.get("tareaWorkflow3")).isEqualTo(1);
       assertThat((int) nodeInstanceExecutionCounter.get("tareaWorkflow2")).isEqualTo(1);
       assertThat((int) nodeInstanceExecutionCounter.get("OR converging")).isEqualTo(3);
       assertThat((int) nodeInstanceExecutionCounter.get("tareaWorkflow6")).isEqualTo(1);
       assertThat((int) nodeInstanceExecutionCounter.get("Script")).isEqualTo(2);
       assertThat((int) nodeInstanceExecutionCounter.get("XOR diverging")).isEqualTo(2);
       assertThat((int) nodeInstanceExecutionCounter.get("XOR converging")).isEqualTo(2);
       assertThat((int) nodeInstanceExecutionCounter.get("End")).isEqualTo(1);

testMultiInstanceLoopCharacteristicsTaskWithOutputCompletionCondition2- Instead of changing the process instance to completed, the state is changing to error state

        Application app = ProcessTestHelper.newApplication();
        ProcessTestHelper.registerHandler(app, "Human Task",
                new SystemOutWorkItemHandler());
        List<String> myList = new ArrayList<>();
        myList.add("approved");
        myList.add("rejected");
        myList.add("approved");
        myList.add("approved");
        myList.add("rejected");
        Process<MultiInstanceLoopCharacteristicsTaskWithOutputCmpCond2Model> process = MultiInstanceLoopCharacteristicsTaskWithOutputCmpCond2Process.newProcess(app);
        MultiInstanceLoopCharacteristicsTaskWithOutputCmpCond2Model model = process.createModel();
        model.setList(myList);
        ProcessInstance<MultiInstanceLoopCharacteristicsTaskWithOutputCmpCond2Model> instance = process.createInstance(model);
        instance.start();
        assertThat(instance.status()).isEqualTo(org.kie.api.runtime.process.ProcessInstance.STATE_COMPLETED);
        assertThat(instance.variables().getListOut()).hasSize(3);

@Abhitocode Abhitocode force-pushed the incubator-kie-issues-1131-36 branch 3 times, most recently from e9244f9 to e52f0df Compare September 26, 2024 13:13
@kie-ci3
Copy link
Contributor

kie-ci3 commented Sep 26, 2024

PR job #4 was: UNSTABLE
Possible explanation: This should be test failures

Reproducer

build-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 #3682 --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-3682/4/display/redirect

Test results:

  • PASSED: 3325
  • FAILED: 3

Those are the test failures:

org.jbpm.bpmn2.IntermediateEventTest.testTimerBoundaryEventInterrupting
expected: 2
but was: 1
org.jbpm.bpmn2.IntermediateEventTest.testIntermediateCatchEventTimerDuration
expected: 2
but was: 1
org.jbpm.bpmn2.IntermediateEventTest.testEventBasedSplit2
expected: 2
but was: 1

@martinweiler
Copy link
Contributor

@Abhitocode re: How can we assert this, what changes are made to child process instances after completing parent process.

Wouldn't this not just be an issue of repeating this assert?

        List<ProcessInstance<DynamicSignalChildModel>> childInstances = childProcessDefinition.instances().stream().toList();
        assertThat(childInstances).hasSize(3);
        childInstances.forEach(instance -> assertThat(instance.status()).isEqualTo(ProcessInstance.STATE_ACTIVE));

In this test, the parent/child sub-process relation is defined as independent, so the child instances should remain active even though the parent has finished.

What about the tests in FlowTest - are you going to add them to this PR as well?

@Abhitocode
Copy link
Contributor Author

yes will be adding the flow tests

@Abhitocode Abhitocode force-pushed the incubator-kie-issues-1131-36 branch from e52f0df to ee9b7b5 Compare October 3, 2024 12:34
@Abhitocode Abhitocode force-pushed the incubator-kie-issues-1131-36 branch from ee9b7b5 to a25cc36 Compare December 6, 2024 03:52
@Abhitocode Abhitocode marked this pull request as ready for review December 6, 2024 05:28
@martinweiler
Copy link
Contributor

@Abhitocode for the two tests:

  • FlowTest.testInclusiveSplitWithLoopInsideSubprocess
  • FlowTest.testMultiInstanceLoopCharacteristicsTaskWithOutputCompletionCondition2

I have created apache/incubator-kie-issues#1719 now. Once that is done, you can raise a new PR to get those two migrated.

For the two tests in IntermediateEventTest that you mentioned above, please create another issue so that we track those as well. Thanks!

@Abhitocode
Copy link
Contributor Author

Thanks for creating the issue @martinweiler , sure will create a separate pr for tests with above issue once the scripts issue is resolved.

@martinweiler
Copy link
Contributor

@Abhitocode I have created #3815 to fix the onEntry/onExit script creation in embedded nodes. I could already include your migrated testInclusiveSplitWithLoopInsideSubprocess. For testMultiInstanceLoopCharacteristicsTaskWithOutputCompletionCondition2, some modifications are required to make the mvel scripts work inside the process, so this needs to be investigated separately.

@pefernan pefernan self-requested a review December 18, 2024 16:42
@pefernan
Copy link
Contributor

Build errors not related to this change, retriggered CI to have the greens

Copy link
Contributor

@pefernan pefernan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 when green

@pefernan pefernan merged commit e58912e into apache:main Dec 20, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants