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

JSPWIKI-1194 - Fix CI build by ensuring test-jar is built and deploying with JDK 17 #351

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

arturobernalg
Copy link
Member

@arturobernalg arturobernalg commented Jun 26, 2024

Update Jenkinsfile to Include Java 21 and Resolve CI Issues

This PR addresses the CI build failure by updating the Jenkinsfile. The changes include:

  1. Adding support for Java 21 in the build stages.
  2. Ensuring the SonarQube analysis and deployment steps are correctly executed.
  3. Building and deploying test-jar dependencies to resolve missing artifact issues.

These updates aim to maintain the stability of the CI pipeline and ensure all necessary artifacts are properly built and deployed.

@juanpablo-santos
Copy link
Contributor

Hi,

couple of notes,

  • the test-jar issue could be solved just by placing <skip>false</skip>just below/inside the pom generating the test-jar. That config defaults to ${maven.test.skip} and that's why on a clean environment the build fails (the artifact is not built)

    • -DskipTests skips test execution, whereas -Dmaven.test.skip skips tests execution and compilation, so it's a bit faster. At the deploy stage test had already passed so that flag was added to speed the deploy a bit. Unsure how applying both flags solves the issue on a clean env (no 2.12.3-SNAPSHOT on local repository and -o passed to the build)
  • non sonar builds are skipping tests; they shouldn't they help us ensure there are no problems with other JDKs

  • as for switching to jdk-17

    • this would force everyone consuming the snapshot artifacts to forcibly use JDK-17. I'd rather do that at the same time of moving from javax to jakarta, preferably on another PR
    • when we move to jdk-17, as the sonar scanner requires also at least jdk-17, the stage of the jenkinsfile could be simplified by undoing thie commit

cheers,
juan pablo

@arturobernalg
Copy link
Member Author

Hi,

couple of notes,

* the test-jar issue could be solved just by placing `<skip>false</skip>`just below/inside the [pom generating the test-jar](https://github.com/apache/jspwiki/blob/master/jspwiki-210-test-adaptees/pom.xml#L93). That config defaults to `${maven.test.skip}` and that's why on a clean environment the build fails (the artifact is not built)
  
  * `-DskipTests` skips test execution, whereas `-Dmaven.test.skip` skips tests execution _and_ compilation, so it's a bit faster. At the deploy stage test had already passed so that flag was added to speed the deploy a bit. Unsure how applying both flags solves the issue on a clean env (no 2.12.3-SNAPSHOT on local repository and `-o` passed to the build)

* non sonar builds are skipping tests; they shouldn't they help us ensure there are no problems with other JDKs

* as for switching to jdk-17
  
  * this would force everyone consuming the snapshot artifacts to forcibly use JDK-17. I'd rather do that at the same time of moving from javax to jakarta, preferably on another PR
  * when we move to jdk-17, as the sonar scanner requires also at least jdk-17, the stage of the jenkinsfile could be simplified by undoing thie [commit](https://github.com/apache/jspwiki/commit/8c0d55e344d3f48ca7747e05fa2306a32b72800c)

cheers, juan pablo

Hi @juanpablo-santos ,

Thanks for your feedback.

Test-Jar Issue:
    I added <skip>false</skip> to the maven-jar-plugin configuration in the jspwiki-210-test-adaptee module to ensure the test-jar is always built.

Test Execution:
    I will ensure that non-Sonar builds do not skip tests to maintain coverage and catch issues across different JDKs.

JDK-17 Integration:
    I agree that switching to JDK-17 and moving from javax to jakarta should be handled in a separate PR to ensure a smooth transition. Once we switch to JDK-17, we can simplify the Jenkinsfile accordingly.

Cheers,

…re test-jar is built by adding <skip>false</skip> in the maven-jar-plugin configuration.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants