Open Liberty support - Githubissues

eclipse-ee4j / cargotracker

The project demonstrates how you can develop applications with Jakarta EE using widely adopted architectural best practices like Domain-Driven Design (DDD).

https://eclipse-ee4j.github.io/cargotracker/

MIT License

343 stars 142 forks source link

Open Liberty support #283

Closed gkwan-ibm closed 1 year ago

gkwan-ibm commented 1 year ago

The changes include:

adding an openliberty profile in the pom.xml
adding 2 Open Liberty configuration files, server.xml and bootstrap.properties, under the src/main/liberty directory
updating the README.md for the instructions to run the openliberty profile

Note: no update for any Java, xml, and other files

If not merge to the master branch, suggest to create a new branch liberty-ee10.

m-reza-rahman commented 1 year ago

How serious, complete, and well-tested is this PR? What about the Arquillian tests? Are those working? Why is there not something like this: https://hantsy.github.io/jakartaee9-starter-boilerplate/arq-openliberty.html?

m-reza-rahman commented 1 year ago

If this is a PR that isn’t really ready to review yet, can you switch to draft until it is please? Otherwise it’s really confusing what I am looking at. When it is ready, you can take it out or draft state.

gkwan-ibm commented 1 year ago

How serious, complete, and well-tested is this PR? What about the Arquillian tests? Are those working? Why is there not something like this: https://hantsy.github.io/jakartaee9-starter-boilerplate/arq-openliberty.html?

The Arquillian test BookingServiceTest.java is not compatible with io.openliberty.arquillian:arquillian-liberty-managed-jakarta. Disabled it from the openliberty profile. Instead, added 3 integration tests at src/test/java/org/eclipse/cargotracker/rest directory. They can be run by Liberty dev mode, or by failsafe:integration-test goal when the app is running either started by payara or openliberty profile.

This PR was tested through payara and openlibery profiles. Let me know if need more to test.

m-reza-rahman commented 1 year ago

We cannot have separate sets of tests per runtime. What is the specific issue with getting Arquillian working with Liberty?

m-reza-rahman commented 1 year ago

With regards to testing, please manually test all application functionality to be working including batch processing - in addition to getting all existing automated tests working properly (including Arquillian). No existing automated tests should be disabled. I will do so myself before I merge, but it is expected that no bugs are introduced.

gkwan-ibm commented 1 year ago

Tested the following on Mac, Linux, and Windows

mvn clean package cargo:run
- 29 unit tests passed
- the application worked fine
mvn -P openliberty clean liberty:create liberty:install-feature verify
- 29 unit tests passed
mvn -P openliberty clean liberty:dev
- the application worked fine
- hit enter key to run tests and 29 unit tests passed

m-reza-rahman commented 1 year ago

OK. I’ll review ASAP.

m-reza-rahman commented 1 year ago

I see there is still some commit activity. Can you please confirm this PR is ready for review and merge?

gkwan-ibm commented 1 year ago

I see there is still some commit activity. Can you please confirm this PR is ready for review and merge?

yes, ready to review. That activity was caused by synchronized the branch with your yesterday commits.

m-reza-rahman commented 1 year ago

Please ensure simply running the following works. It should be possible to configure Maven so that nothing more complex is required for the build to work.

mvn clean package -Popenliberty

Let me know when this is addressed. I will review the PR again after that.

gkwan-ibm commented 1 year ago

hi @m-reza-rahman

Updated the pom.xml to make mvn clean package -Popenliberty work