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#1460] Fix kogito-maven-plugin #3757

Merged
merged 17 commits into from
Dec 2, 2024

Conversation

gitgabrio
Copy link
Contributor

@gitgabrio gitgabrio commented Oct 31, 2024

Fixes apache/incubator-kie-issues#1460

Needs:
apache/incubator-kie-kogito-apps#2135
apache/incubator-kie-kogito-examples#2032

This PR:

  1. refactor generateModel mojo to internally call compile and process-classes
  2. remove need to workarounded pom
  3. remove usage of custom-lifecycle
  4. remove need of extensions=true

Changes outside of the kogito-maven-plugin are only adaptation to the new implementation

How to replicate CI configuration locally?

Build Chain tool does "simple" maven build(s), the builds are just Maven commands, but because the repositories relates and depends on each other and any change in API or class method could affect several of those repositories there is a need to use build-chain tool to handle cross repository builds and be sure that we always use latest version of the code for each repository.

build-chain tool is a build tool which can be used on command line locally or in Github Actions workflow(s), in case you need to change multiple repositories and send multiple dependent pull requests related with a change you can easily reproduce the same build by executing it on Github hosted environment or locally in your development environment. See local execution details to get more information about it.

@kie-ci3
Copy link
Contributor

kie-ci3 commented Oct 31, 2024

PR job #2 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 #3757 --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-3757/2/display/redirect

Test results:

  • PASSED: 3111
  • FAILED: 1

Those are the test failures:

org.kie.kogito.maven.plugin.it.kogito-maven-plugin-test.kogito-maven-plugin-test The build exited with code 1. See /home/jenkins/jenkins-agent/workspace/_jobs_kogito-runtimes-pr_PR-3757/kogito-pipelines/bc/apache_incubator-kie-kogito-runtimes/kogito-maven-plugin/target/it/kogito-maven-plugin-test/build.log for details.

@kie-ci3
Copy link
Contributor

kie-ci3 commented Oct 31, 2024

PR job #3 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 #3757 --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-3757/3/display/redirect

Test results:

  • PASSED: 3463
  • FAILED: 1

Those are the test failures:

org.kie.kogito.maven.plugin.it.kogito-maven-plugin-test.kogito-maven-plugin-test The build exited with code 1. See /home/jenkins/workspace/_jobs_kogito-runtimes-pr_PR-3757/kogito-pipelines/bc/apache_incubator-kie-kogito-runtimes/kogito-maven-plugin/target/it/kogito-maven-plugin-test/build.log for details.

@kie-ci3
Copy link
Contributor

kie-ci3 commented Nov 4, 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 #3757 --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-3757/4/display/redirect

Test results:

  • PASSED: 3468
  • FAILED: 1

Those are the test failures:

org.kie.kogito.maven.plugin.it.kogito-maven-plugin-test.kogito-maven-plugin-test The build exited with code 1. See /home/jenkins/jenkins-agent/workspace/_jobs_kogito-runtimes-pr_PR-3757/kogito-pipelines/bc/apache_incubator-kie-kogito-runtimes/kogito-maven-plugin/target/it/kogito-maven-plugin-test/build.log for details.

@kie-ci3
Copy link
Contributor

kie-ci3 commented Nov 4, 2024

PR job #5 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 #3757 --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-3757/5/display/redirect

Test results:

  • PASSED: 3468
  • FAILED: 1

Those are the test failures:

org.kie.kogito.maven.plugin.it.kogito-maven-plugin-test.kogito-maven-plugin-test The build exited with code 1. See /home/jenkins/workspace/_jobs_kogito-runtimes-pr_PR-3757/kogito-pipelines/bc/apache_incubator-kie-kogito-runtimes/kogito-maven-plugin/target/it/kogito-maven-plugin-test/build.log for details.

@gitgabrio gitgabrio marked this pull request as draft November 5, 2024 13:18
@gitgabrio gitgabrio marked this pull request as ready for review November 8, 2024 13:00
@gitgabrio
Copy link
Contributor Author

gitgabrio commented Nov 8, 2024

@elguardian @fjtirado @pefernan @yesamer
After different attempts, I pushed a slightly different approach.
Context:

  • due to rule engine implementation, it is not possible to avoid a first compilation of the module before rules codegen
  • consequently, it does not make sense to spend time trying to fix that issue only for processes
  • the original scope of the ticket was to remove the need of the extensions flag, as described in description, but during iteration emerged also the request to avoid multiple compilations
  • the critical bit is the ProcessClassesMojo, that is responsible to codegenerate classes related to persistence and json schemas, using reflection.

I tried to avoid compilation in the middle of the plugin execution, bringing around generated files and using JavaParser to read them.
Unfortunately, the last step is the generation of json schema, inside ProcessClassesMojo, that currently is done by an external library (jsonschema-generator) that accept classes as input.
Re-implement this last step using sources would be extremely time-consuming and error-prone, so I stopped there.
But, as for the initial pre-compilation, if this step requires compiled classes, there is no reason to keep implementing ProcessClassesMojo generation from sources, instead of classes.
Then, the current PR has the following flow:

  1. compilation of java classes present in the module
  2. model code-generation based on model files
  3. compilation (and dump) of only the newly code-generated classes
  4. code-generation of persistence classes and json schemas (that will also scan classes compiled in previous step)
  5. compilation (and dump) of only the newly code-generated classes

I also removed the ProcessClassesMojo and introduced some helper classes.

While this does not completely remove the need of intermediate compilation (that would be extremely time-consuming and error-prone) at least

  • resolve the original issue of extensions flag
  • avoid the usage of external library for hidden plugin invocation
  • avoid full recompilation in the middle of plugin execution
  • implement on-demand, only for needed sources, compilation

@gitgabrio
Copy link
Contributor Author

Failure inside kogito-runtimes unrelated

 - apache_incubator-kie-kogito-runtimes/jbpm/jbpm-tests/src/test/java/org/jbpm/bpmn2/StandaloneBPMNProcessTest.java | expected: 2
 ##[error]expected: 2
 but was: 1

it is present also in unmodified/testing PR

Gabriele-Cardosi added 2 commits November 12, 2024 12:47
@gitgabrio
Copy link
Contributor Author

@baldimir @yesamer
I moved the testing module outside kogito-maven-plugin one to avoid the latter depend on springboot

Copy link
Contributor

@fjtirado fjtirado left a comment

Choose a reason for hiding this comment

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

@gitgabrio This is astonishing piece of work, my sincere congratulation!
However, I have to ask you something that might be a bit painful. Its about organization of modules, It would be possible to move kogito-mavem-plugin-test under kogito-maven-plugin?
I think the structure should be: kogito-maven-plugin-parent containing kogito-maven-plugin and kogito-maven-pluging-test (I let you chose more proper name if you feel the need, my concern is the test module should not be at the root kogito level as it is now)

UPDATE:
Ok, I see why test module is outside kogito-maven-plugin. The testing depends on SpringBoot. Then I think the test module should be moved under springboot module. wdyt?

@gitgabrio
Copy link
Contributor Author

gitgabrio commented Nov 27, 2024

Hi @fjtirado
I'm not 100% sure of moving the kogito-maven-plugin-test under springboot.

On one side, clearly, it requires springboot-related dependencies, but on the other side it would make more sense (to me) to keep the two (plugin and test) somehow grouped.
So, maybe a "kogito-maven-plugin" parent, that contains both (the actual plugin and the springboot test) where we may add, in future, other tests (e.g. plain-java one): wdyt ? does this make sense ?

Copy link
Contributor

@yesamer yesamer left a comment

Choose a reason for hiding this comment

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

@gitgabrio remarkable job, well done

@gitgabrio gitgabrio merged commit 1b3674b into apache:main Dec 2, 2024
6 checks passed
@gitgabrio gitgabrio deleted the incubator-kie-issues#1460 branch December 2, 2024 13:22
rgdoliveira pushed a commit to rgdoliveira/kogito-runtimes that referenced this pull request Dec 2, 2024
* [incubator-kie-issues#1460] Working unit tests. Working external example

* [incubator-kie-issues#1460] Implemented it tests

* [incubator-kie-issues#1460] Adapted tests

* [incubator-kie-issues#1460] Disable flaky test

* [incubator-kie-issues#1460] Fix maven reactor

* [incubator-kie-issues#1460] Fix maven reactor

* [incubator-kie-issues#1460] Fix maven reactor

* [incubator-kie-issues#1460] Optimizing compilation invocation. Removing ProcessClassesMojo.java

* [incubator-kie-issues#1460] Removing left-overs

* [incubator-kie-issues#1460] Moving kogito-maven-plugin-test as standalone module to avoid springboot dependency inside kogito-maven-plugin

* [incubator-kie-issues#1460] Removing leftover properties

---------

Co-authored-by: Gabriele-Cardosi <[email protected]>
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.

Investigate and fix kogito-maven-plugin usage inside kogito-runtimes project itself
5 participants