feat/1573 arquillian cargo replacement#2471
Conversation
|
@shiftyseaotter Great that you managed to refactor the tests to Arquillian. This will make the maintenance easier. There are a few resolvable minor Sonar findings. Please address them if possible. The tests are executing, so I assume the review is just about formal stuff. I will review that soon! |
There was a problem hiding this comment.
Pull request overview
This pull request migrates the webapp integration tests from the Cargo Maven plugin to Arquillian, harmonizing the test infrastructure with the existing engine integration tests. This change addresses issue #1573 by consolidating the testing approach across the project.
Changes:
- Replaced Cargo-based webapp deployment with Arquillian managed containers for both Tomcat and WildFly
- Restructured the integration tests module by consolidating
integration-testsandshared-enginesubmodules into a single module - Created new
arquillian-extensionsmodule to share Arquillian lifecycle observers between engine and webapp tests - Updated package structure from
org.operaton.bpmtoorg.operaton.bpm.integrationtestfor test classes
Reviewed changes
Copilot reviewed 41 out of 44 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| qa/pom.xml | Added arquillian-extensions module and version properties |
| qa/arquillian-extensions/** | New module containing shared Arquillian extensions for testcontainers integration |
| qa/integration-tests-webapps/pom.xml | Restructured from parent POM to single module with Tomcat/WildFly profiles |
| qa/integration-tests-webapps/src/test/resources/arquillian.xml | New Arquillian configuration for Tomcat and WildFly containers |
| qa/integration-tests-webapps/src/test/java/** | Updated package names and refactored base test classes to use Arquillian APIs |
| qa/integration-tests-webapps/shared-engine/** | Removed - functionality consolidated into parent module |
| qa/integration-tests-webapps/integration-tests/** | Removed - consolidated into parent module |
| qa/wildfly-runtime/src/main/wildfly/standalone/configuration/standalone.xml | Added SameSite cookie filter for session security tests |
| qa/tomcat-runtime/src/main/conf/context.xml | Added JarScanner configuration workaround |
| parent/pom.xml | Downgraded Tomcat from 11.0.18 to 10.1.42 for Arquillian compatibility |
| qa/integration-tests-engine/pom.xml | Moved Arquillian extension to shared module |
| distro/run/qa/** | Updated to consume webapps test-jar and align with new package structure |
Comments suppressed due to low confidence (2)
qa/integration-tests-webapps/README.md:6
- Grammatical error: "You a build" should be "You need a build" or "A build".
qa/integration-tests-webapps/README.md:5 - Grammatical error: "it qa/pom.xml" should be "in qa/pom.xml".
javahippie
left a comment
There was a problem hiding this comment.
Hi, that's a huge effort and will help us a lot, thanks for doing this! I have two review comments, only the one about Tomcat is a concern. I guess that there is no arquillian-tomcat-managed-11, yet?
Sure, will adress all but the "ip" one :) |
|
@shiftyseaotter What is the current state? Please resolve the trivial conversations yourself when they are addressed. When we are back on Tomcat 11 and everything is working as expected, we should be pretty close to finish this one. |
Can you please review it again? Everything should be in order now. Except for random ports, which will be handled in separate issue |
renamed test EmbeddedEngineRestWildflyTest to IT to indicate that it is an integration test removes version of dependency plugin for wildfly profile fixes for PR notes and Sonar issues revert tomcat version back to 11.x since servlet-api 6.1.0 is used now add correct operaton-qa-integration-tests-webapps scope and classifier Refactoring. Removed unused imports and methods Refactoring. Removed duplicate variables, commented out parameters. Moved TestProperties to run due to it being the only place it is used now consolidate runtime configuration parameters added configuration of arquillian container from pom. Streamlined integration test modules, due to shared engine not needed anymore tomcat support. downgraded tomcat runtime version to Jakarta 6.0.0 due to conflicts #1573 draft solution, hardcoded wildfly.
cd2be5b to
78bd942
Compare
|
Looks good. I have squashed and rebased your changes. Planning to merge them. |
|
kthoms
left a comment
There was a problem hiding this comment.
Thank you @shiftyseaotter for resolving this work
feat(integration-tests):
webapps: Integration Tests are using Arquillian instead of Cargo Plugin
closes #1573