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

[JENKINS-73316] Require Java 17 and 2.475; adapt test suite to EE 9 #421

Merged
merged 1 commit into from
Sep 4, 2024

Conversation

basil
Copy link
Member

@basil basil commented Jun 3, 2024

See JENKINS-73316. Allow this plugin's test suite to be exercised regularly in Plugin Compatibility Tester (PCT) in https://github.com/jenkinsci/bom by requiring Java 17 or newer and Jenkins 2.475 (released today) or newer.

This PR goes against the usual advice from https://www.jenkins.io/doc/developer/plugin-development/choosing-jenkins-baseline/ by requiring a weekly release. The advantage is that this will allow plugin compatibility testing (PCT). The disadvantage is that new features and bug fixes can't be released to LTS users for another few months.

The choice is up to the maintainer as to whether or not to accept this PR. If this PR is rejected, we will stop testing this plugin in PCT.

@basil basil force-pushed the jakarta branch 2 times, most recently from 391b775 to 01a5e93 Compare June 18, 2024 23:31
@basil basil changed the title Incremental build for testing (Jakarta) [JENKINS-73316] Prepare LDAP Jun 21, 2024
@basil basil changed the title [JENKINS-73316] Prepare LDAP [JENKINS-73316] Adapt SAML plugin for Jetty 12 (EE 9) Jun 21, 2024
Copy link
Member Author

@basil basil left a comment

Choose a reason for hiding this comment

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

The update to Jetty 12 (EE 9) in Jenkins core (hopefully for the next LTS) will break org.jenkinsci.plugins.saml.OpenSamlWrapperTest, which we test in BOM.

As described in https://issues.jenkins.io/browse/JENKINS-73316 one solution to this problem is what is demonstrated in this PR. This fixes the test failure at the cost of requiring a weekly release with Jetty 12 (EE 9), currently scheduled for later in September or October. That goes against the advice in https://www.jenkins.io/doc/developer/plugin-development/choosing-jenkins-baseline/ to "Prefer an LTS version over weekly versions", but it is not unheard of in Jenkins plugins to depend on a weekly release, especially where there are complex breaking changes in core involved (of which this is surely an example).

@kuisathaverat Are you OK with the approach taken in this PR (depending on a weekly release of core)? If not, are you willing to try the alternative approach described in https://issues.jenkins.io/browse/JENKINS-73316 to remove Mockito usages from OpenSamlWrapperTest? If neither option is feasible, then we might have to disable this test in BOM/PCT (which is also undesirable because it reduces test coverage against recent weekly releases).

@kuisathaverat
Copy link

It is fine, I will need the Java 17 requirement to bump the pac4j dependency

@basil basil changed the title [JENKINS-73316] Adapt SAML plugin for Jetty 12 (EE 9) [JENKINS-73316] Require Java 17 and 2.475; adapt test suite to EE 9 Sep 4, 2024
@basil basil marked this pull request as ready for review September 4, 2024 14:48
.github/workflows/codeql.yml Outdated Show resolved Hide resolved
@kuisathaverat kuisathaverat merged commit 99810fb into jenkinsci:main Sep 4, 2024
15 checks passed
@basil basil removed the developer label Sep 4, 2024
@basil basil deleted the jakarta branch September 4, 2024 18:44
@kuisathaverat
Copy link

@basil
Copy link
Member Author

basil commented Sep 4, 2024

Thank you very much!

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.

4 participants