Skip to content

Makes AuthorizationContainer extend Describable#169

Merged
daniel-beck merged 1 commit intojenkinsci:masterfrom
Vlatombe:authorizationccontainer-describable-compat
Mar 31, 2025
Merged

Makes AuthorizationContainer extend Describable#169
daniel-beck merged 1 commit intojenkinsci:masterfrom
Vlatombe:authorizationccontainer-describable-compat

Conversation

@Vlatombe
Copy link
Member

@Vlatombe Vlatombe commented Mar 19, 2025

This is needed for compatibility with core tests updated by jenkinsci/jenkins#10426

Testing done

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

This is for compatibility work with a core change i'm preparing...
@Vlatombe Vlatombe changed the title Makes AuthorizationContainer extend Describable Makes AuthorizationContainer extend Describable Mar 20, 2025
@Vlatombe Vlatombe marked this pull request as ready for review March 20, 2025 09:24
@Vlatombe Vlatombe requested a review from a team as a code owner March 20, 2025 09:24
@Vlatombe
Copy link
Member Author

ping @daniel-beck (btw, @jenkinsci/matrix-auth-plugin-developers is empty)

@daniel-beck
Copy link
Member

daniel-beck commented Mar 24, 2025

Is this a prerequisite for the core change? What makes matrix-auth special that it breaks things, while the core PR is generally backward-compatible?

@Vlatombe
Copy link
Member Author

This is required to apply the changes to test/ module, otherwise compilation fails with

[ERROR] /Users/vlatombe/projects/github.com/jenkinsci/jenkins/test/src/test/java/hudson/model/QueueTest.java:[805,20] types hudson.model.Describable<T> and org.jenkinsci.plugins.matrixauth.AuthorizationContainer are incompatible;
[ERROR]   class hudson.model.QueueTest.AliceCannotBuild inherits abstract and default for getDescriptor() from types hudson.model.Describable and org.jenkinsci.plugins.matrixauth.AuthorizationContainer
[ERROR] /Users/vlatombe/projects/github.com/jenkinsci/jenkins/test/src/test/java/hudson/model/FingerprintTest.java:[659,20] types hudson.model.Describable<T> and org.jenkinsci.plugins.matrixauth.AuthorizationContainer are incompatible;
[ERROR]   class hudson.model.FingerprintTest.NoInheritanceProjectMatrixAuthorizationStrategy inherits abstract and default for getDescriptor() from types hudson.model.Describable and org.jenkinsci.plugins.matrixauth.AuthorizationContainer

@daniel-beck
Copy link
Member

I see, that makes sense and would indicate it's exclusive to my class hierarchy here.

Unsure whether I will have time to address this PR this week, but should be in a release next week.

@jglick
Copy link
Member

jglick commented Mar 25, 2025

Why not just adjust those two tests to use MockAuthorizationStrategy?

Copy link
Member

@daniel-beck daniel-beck left a comment

Choose a reason for hiding this comment

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

This seems so much cleaner than before, thanks!

@daniel-beck daniel-beck merged commit d3e182c into jenkinsci:master Mar 31, 2025
17 checks passed
@Vlatombe Vlatombe deleted the authorizationccontainer-describable-compat branch April 1, 2025 15:15
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.

3 participants