Use recommended configurations to test plugin #40
Conversation
This makes sure that they can be constructed properly. That's also a requirement for newer Jenkins versions. Otherwise subtile issues may arise in tests. The following popped up by just fixing a descriptor's display name (see next commit). > org.kohsuke.stapler.NoStaplerConstructorException: There's no @DataBoundConstructor on any constructor of class org.jenkinsci.plugins.authorizeproject.ProjectQueueItemAuthenticatorTest$AuthorizeProjectStrategyWithOldSignature
darxriggs
changed the title
|
@ikedam on build #1 you can see that using One way to get this onto master would be that I put commit e61a835 that changes the Jenkinsfile on top and change the target branch to master. This would a) keep the history clean and b) make tests for every commit in this pull request run successfully (useful in case What's your thought on this? |
|
I don't think we have to keeping the history clean (it always can be dirty), but I want clarify what changes are actually for this pull request. Actually, we need #36 to have tests pass.
Let me have some more time to review this request as I'm not sure what happen after resetting feature/35_RecommendedConfigurations (I'm afraid it results this broken and I cannot review changes). |
There was a problem hiding this comment.
Thanks for the fixes.
I want you have following changes before merging this request:
- Revert the change of target Jenkins version.
- Rebase to jenkinsci:feature/35_RecommendedConfigurations which will be reset to [WIP] Use recommended build configurations on ci.jenkins.io - DON'T MERGE #35
src/test/java/org/jenkinsci/plugins/authorizeproject/ProjectQueueItemAuthenticatorTest.java
Show resolved
Hide resolved
src/test/java/org/jenkinsci/plugins/authorizeproject/testutil/AuthorizationCheckBuilder.java
Show resolved
Hide resolved
.../org/jenkinsci/plugins/authorizeproject/strategy/SpecificUsersAuthorizationStrategyTest.java
Show resolved
Hide resolved
.../org/jenkinsci/plugins/authorizeproject/strategy/SpecificUsersAuthorizationStrategyTest.java
Show resolved
Hide resolved
ikedam
force-pushed
the
feature/35_RecommendedConfigurations
branch
from
d9e5942 to
e61a835
Compare
The synthetic test case with userId="" has to be removed because the underlying library does not support this anymore.
Due to dependency changes in the parent pom via the previous commit, it's not any longer required to specify this dependency.
Without this change a RuntimeException is thrown for Jenkins >= 2.102 because hudson.model.Project#builders cannot be serialized. That's because the AuthorizationCheckBuilder is included in there and the Authentication class it uses is not in the whitelist of serializable classes. For more details see these blog posts - https://jenkins.io/blog/2018/03/15/jep-200-lts/ - https://jenkins.io/blog/2018/01/13/jep-200/
Without this change some tests would fail for Jenkins >= 2.129. They assumed that an API token is always available. Starting with this Jenkins version no legacy API token is created by default. Therefore creating one in the test setup. For more details see https://jenkins.io/blog/2018/07/02/new-api-token-system/
…user sessions Without this change this test would fail for Jenkins >= 2.150.2, reporting that access it denied for the "authorization" endpoint for user "anonymous". For more details see https://jenkins.io/changelog-stable/#v2.150.2 This change is just fixing the tests (due to inter-test dependencies). Fixing the actual issue is still to be done.
darxriggs
force-pushed
the
full-recommended-configurations
branch
from
1788cd8 to
daaa764
Compare
|
This pull request now contains all changes as I would want them to be merged. All but one of your review comments have been incorporated. If you insist on not having Java & Jenkins upgraded here I would do it but reluctantly - see #40 (comment). |
This pull request contains all changes required for using
buildPlugin(configurations: buildPlugin.recommendedConfigurations()).The first relevant commit is "Use @DataBoundConstructor for every AuthorizeProjectStrategy in tests". As of 2019-08-27 there are 5 other commits contained because the target branch is not yet rebased on the master branch.