[JENKINS-48837] Add BranchProperty support to OrganizationFolder

[nfalco79]

We have more than 50 multibranch jobs configured with branch properties. This should save time every time we change branch properties, the feature that had stop migration to organisation folder.

Each multibranch project child of the organisation folder have branch property strategy configured (the release link) only for master branch:

Any suggestion for a test case?

[bitwiseman]

@nfalco79
Could you rebase your changes on the current head of master and re-push? It would be nice to have a working incremental to use for testing.

[nfalco79]

@bitwiseman done

[bitwiseman]

@nfalco79
https://github.com/nfalco79/branch-api-plugin/tree/feature/JENKINS-48837 says "This branch is 1 commit ahead, 6 commits behind jenkinsci:master."

[nfalco79]

@bitwiseman You right, I forgot to update my origin with jenkins remote. I see that is too late for 2.5.3 release :(

[bitwiseman]

@nfalco79 Don't worry, we're not on a schedule.

[nfalco79]

Are there some comments or changes you would I do on this PR?

[bitwiseman]

@nfalco79
Sorry for the delay. To merge, I still need to load this up for manual testing on an instance. It would be good to have some automated tests too if you have time to add some.

[nfalco79]

Sure, any suggestion for a test case? I mean do you have any suggestion from which test case I can copy or take an example for these scenario?

[bitwiseman]

@nfalco79 I don't know off-hand, sorry. And I'm swamped until at least Thursday this week. I'll be able to take a look after that.

[nfalco79]

Do not worry, also because today I got an issue. If I modify branch properties (removing or adding), these changes are persisted correctly in the organisation folder config.xml but multibranch children projects are not affected by the changes also after a jenkins restart.

[nfalco79]

Ok issues resolved.

There was two issues:

1. sources variable was set to null in createBranchSources to avoid more call to createBranchSources. But this cause NPE that stop the process of update
2. The NamedExceptionsBranchPropertyStrategy only returns the branch property defined on the first matching named exception. For this reason if I do not show all defined properties for a specific branch.

[nfalco79]

@bitwiseman please any news on this? Is there something you would to change?

[bitwiseman]

@nfalco79
Hello, sorry for slow response.
I understand the feature now. Can you add some tests for this to https://github.com/jenkinsci/branch-api-plugin/blob/master/src/test/java/jenkins/branch/OrganizationFolderTest.java

[nfalco79]

I will try

[bitwiseman]

@nfalco79
If there's anything I can do to help, just say so.

[nfalco79]

@bitwiseman I'm finally back on this issue.
I add a test case to verify that multibranch project will inherites branch properties defined in the Organisation Folder.

Please let me know if you need somethingelse from my side.

[nfalco79]

@bitwiseman any news about this PR? I have add test cases as request. Please reset the "changes request" mark.

[nfalco79]

@dwnusbaum @bitwiseman this PR is stuck up please give me some feedback

[nfalco79]

rebased

[nfalco79]

failures seems not be related to the code, rerun touching a commit

[nfalco79]

[2020-05-01T00:13:50.128Z] [WARNING] Rule 5: org.apache.maven.plugins.enforcer.RequireUpperBoundDeps failed with message:
[2020-05-01T00:13:50.128Z] Failed while enforcing RequireUpperBoundDeps. The error(s) are [
[2020-05-01T00:13:50.128Z] Require upper bound dependencies error for org.slf4j:jcl-over-slf4j:1.7.25 paths to dependency are:
[2020-05-01T00:13:50.128Z] +-org.jenkins-ci.plugins:branch-api:2.5.7-rc852.82aba7c3c8bc
[2020-05-01T00:13:50.128Z] +-org.slf4j:jcl-over-slf4j:1.7.25
[2020-05-01T00:13:50.128Z] and
[2020-05-01T00:13:50.128Z] +-org.jenkins-ci.plugins:branch-api:2.5.7-rc852.82aba7c3c8bc
[2020-05-01T00:13:50.128Z] +-org.jenkins-ci.main:jenkins-core:2.222.3
[2020-05-01T00:13:50.128Z] +-org.slf4j:jcl-over-slf4j:1.7.26

pom.xml has not been modified by this PR. Build error seems to be caused because the project use jenkins version 2.138.4 but build system use recomanded configuration that use 2.222.x This make enforcer fails.

[dwnusbaum]

Thanks for the PR! I took a quick look and added some comments, can you please fix the test failures in the CI build? Thanks!

[dwnusbaum]

@nfalco79 just a heads up @car-roll is working on fixing the build errors that are present even in master over in #192, so you can probably wait for that to be merged to fix most of the issues with pom.xml and the existing tests.

[nfalco79]

i finally resolve marking slf4j suite to 1.7.26 version, so that work with jekins 2.222.3 version on build

[nfalco79]

now test pass on linux jdk8 but not on jdk11 because of mockito

Mockito cannot mock this class: class jenkins.branch.BranchCategoryFilterTest$MockSCMSource.
[2020-05-01T18:57:14.868Z] Most likely it is a private class that is not visible by Mockito

this is not related to my changes otherwise the build will not pass on JDK8 on linux.

[nfalco79]

I'm not able to replicate locally also I run Windows 10 openJDK 10.0.7 or Oracle JDK 11.0.2
using mvn clean verify -Djenkins.version=2.222.3 -Dtest=BranchCategoryFilterTest

[nfalco79]

rebased

[nfalco79]

@dwnusbaum the same test failing in my closed PR (and now in master) are failing also here:

• TestTimedOutException
• EventsTest

[nfalco79]

@bitwiseman I had rebased (again), how much longer to reach the end since this comment

This PR was just splitted as requested. Files was the same at the time of #160 (review)

[nfalco79]

ping

[bitwiseman]

@nfalco79 Thanks for sticking with this. This looks like a very useful feature, but other tasks keep taking priority. Sorry. I'll look at this as soon as I have time.

[nfalco79]

any news here

[nfalco79]

@bitwiseman @dwnusbaum
Another relase was passed any change to get this missing feature in the code base?

[dwnusbaum]

@nfalco79 The changes that went into the last release are all trivial, mostly refactoring or test changes, and the release only went out to try to fix PCT issues when testing the plugin. This is a new feature, which means it takes more time to review (need think about design, other code that might be affected/impacted, compatibility, regressions, value of feature vs cost of increased maintenance, documentation updates, etc.), and I have not had time to review it yet.

[nfalco79]

I can understand but the PR was open quite since 1 year. The feature is not different than one already exist for multibranch project. This allows users that use branch property to move to organisation project, share common settings and automate project creation/deletion. Every branch-api release I have to patch manually and load on ours jenkins production.
No other way to patches are available. Just a few of months ago our team update the plugin and forgot the patch, jenkins was offline for 4 hours fortuanlly we had a backup of configurations.

[bitwiseman]

@nfalco79
I'm cutting another release with test fixes today. I've a task to my work queue to look at this next week. This is going to get done. Okay?

[nfalco79]

Ok, let me update., and thanks.

[bitwiseman]

Bounced for CI. 😭

I've had a chance to look at it and I get what is being done. I've had a chance to manually play with it a little and it looks good so far.

[bitwiseman]

@nfalco79 Thanks for you endurance and perseverance on this PR.

[jglick]

This seems to make NoTriggerOrganizationFolderProperty almost redundant—you could use the NoTriggerBranchProperty configured at the org level—except for the fact that the NoTriggerOrganizationFolderProperty.branches regexp is more powerful than the NamedExceptionsBranchPropertyStrategy.Named.name glob. Perhaps there could be a new BranchPropertyStrategy based on regexp matches? In that case NoTriggerOrganizationFolderProperty could be automatically replaced with the newer more general configuration style in OrganizationFolder.readResolve.