-
Notifications
You must be signed in to change notification settings - Fork 213
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-1103] Move tests and create infrastructure to move old legacy v7 tests to kogito. #3478
Conversation
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 #3478 --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-3478/1/display/redirect Test results:
Those are the test failures: org.kie.kogito.quarkus.jbpm.OrdersProcessIT.testOrdersRest1 expectation failed.Expected status code <201> but was <500>. |
b3196b0
to
7799408
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great move @elguardian - just minor comment.
...jbpm-flow-builder/src/main/java/org/jbpm/compiler/canonical/LambdaSubProcessNodeVisitor.java
Outdated
Show resolved
Hide resolved
jbpm/jbpm-tests/src/test/bpmn/org/jbpm/bpmn2/flow/BPMN2-MultipleProcessInOneFile.bpmn2
Show resolved
Hide resolved
import org.kie.kogito.process.bpmn2.BpmnVariables; | ||
import org.kie.kogito.process.impl.AbstractProcess; | ||
|
||
public class XXX extends AbstractProcess<Model> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
XXXX?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is just a template. so it does not really matter, it could be whatever
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about: ThisIsJustTemplate or something like?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replaced
@@ -0,0 +1,29 @@ | |||
<?xml version="1.0" encoding="UTF-8"?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this file needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
usually bak file are auto generated on save by some editors... maybe clarify that this is not the case somewhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing few updates, other than that.. looks good
|
||
|
||
public static org.kie.kogito.process.Process<Model> newProcess(Application application) { | ||
return new XXX(application.config().get(ProcessConfig.class), application); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ThisIsJustTemplate update missing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replaced
return new XXX(application.config().get(ProcessConfig.class), application); | ||
} | ||
|
||
public XXX(ProcessConfig processConfig, Application application) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ThisIsJustTemplate update missing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replaced
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thx! this is very good, thank you again @elguardian
…ove old legacy v7 tests to kogito.
8d03c15
to
e53fdf2
Compare
…ove old legacy v7 tests to kogito. (apache#3478) * [incubator-kie-issues-1103] Move tests and create infrastructure to move old legacy v7 tests to kogito.
issue: apache/incubator-kie-issues#1103
move tests and bpmn2 compilation of v7 legacy runtime. the goal is not affect the current status of tests while allow working on migrating tests and features.