Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-61290

Hard to see if branch was skipped because of discovery strategy

    XMLWordPrintable

    Details

    • Similar Issues:

      Description

      I was recently trying to figure out why one of our branches got disabled and no longer built, not even looking at `Jenkinsfile` presence, and I followed a wrong trail for very long (assumed we have a github cache issue again, taking false-negative responses from old GH site bugs cached by our master... as it happened last year...)

      But in the end by chance found that the setup is to not process branches that also have PRs opened from them, and indeed someone made the PR

      So got two questions:

      1) I want a message telling why a branch was skipped! I sort of found where in github-branch-source-plugin code it begins "Checking branch" https://github.com/jenkinsci/github-branch-source-plugin/blob/master/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource.java#L987
      ... and where it decides it is no good https://github.com/jenkinsci/github-branch-source-plugin/blob/master/src/main/java/org/jenkinsci/plugins/github_branch_source/BranchDiscoveryTrait.java#L230
      ...but a lot of java voodoo happens in-between, so I have no idea where to plug in such logging that it would end up on the branch indexing console

      So if someone knowledgeable could pick it up to do properly, would be nice
      There are only a few hits of "isExcluded" and mostly in events processing (so not sure if it applies to polling/indexing discoveries) but I think they also do not have a "listener" from which I could "getLogger()". Maybe throwing an exception could do it, but would likely disrupt the existing processing loops :\

      2) is there/can someone add some complex filter to always process branches from a certain name pattern (e.g. `release/*`), even if there are PRs from them?

        Attachments

          Activity

          Hide
          jimklimov Jim Klimov added a comment - - edited

          As a stop-gap solution that I hopefully know how to make, I would add jenkins console logging right into the `isExcluded()` methods, where they `return true`, so at least part of investigation like the one I did, would become easier.

          This is sub-par because the message would not appear where it is helpful to people with access to the indexing job log, but at least is something...

          https://github.com/jenkinsci/github-branch-source-plugin/pull/284

          Show
          jimklimov Jim Klimov added a comment - - edited As a stop-gap solution that I hopefully know how to make, I would add jenkins console logging right into the `isExcluded()` methods, where they `return true`, so at least part of investigation like the one I did, would become easier. This is sub-par because the message would not appear where it is helpful to people with access to the indexing job log, but at least is something... https://github.com/jenkinsci/github-branch-source-plugin/pull/284
          Hide
          bitwiseman Liam Newman added a comment -

          I believe that PR now does provide information in the indexing log. But it still needs tests.

          Show
          bitwiseman Liam Newman added a comment - I believe that PR now does provide information in the indexing log. But it still needs tests.
          Hide
          jimklimov Jim Klimov added a comment -

          FWIW, nearly identical solution proposed for bitbucket-branch-source-plugin also, at https://github.com/jenkinsci/bitbucket-branch-source-plugin/pull/318

          Show
          jimklimov Jim Klimov added a comment - FWIW, nearly identical solution proposed for bitbucket-branch-source-plugin also, at https://github.com/jenkinsci/bitbucket-branch-source-plugin/pull/318
          Hide
          jimklimov Jim Klimov added a comment -

          Adding tests proved to be troublesome, mostly because I don't understand how that test framework is set up to mock some situation and check that logs did appear in the expected manner, and got lost in adding hordes of files to emulate the github REST API dialog leading to this or that setup

          And as with many of my submissions to different projects, getting a feature working is maybe 5% of the effort, the rest is fighting against the numerous always-different ways to pass the tests (and separately, PR discussions of the actual solution that do make it better). And when the dayjob agrees at all to let me fix what bugs us, they agree to that 5% which is the actual fix

          I certainly do see the benefit of having the solution maintained in the upstream rather than having to rebuild a privately patched one for every upgrade, and handing off the maintenance burden needs the testability (private builds do too but we can look away for simple fixes like this), but in this case it seems the effort of doing so is prohibitive... as in, if I couldn't even understand how to do it over a couple of evenings, I can't guarantee allocating any time to it in the coming months and would focus on more achievable targets - I am sort of overbooked.

          Long story short, if anyone can pick up writing the testing rituals properly, and get the PRs merged, they really are more than welcome!

          Show
          jimklimov Jim Klimov added a comment - Adding tests proved to be troublesome, mostly because I don't understand how that test framework is set up to mock some situation and check that logs did appear in the expected manner, and got lost in adding hordes of files to emulate the github REST API dialog leading to this or that setup And as with many of my submissions to different projects, getting a feature working is maybe 5% of the effort, the rest is fighting against the numerous always-different ways to pass the tests (and separately, PR discussions of the actual solution that do make it better). And when the dayjob agrees at all to let me fix what bugs us, they agree to that 5% which is the actual fix I certainly do see the benefit of having the solution maintained in the upstream rather than having to rebuild a privately patched one for every upgrade, and handing off the maintenance burden needs the testability (private builds do too but we can look away for simple fixes like this), but in this case it seems the effort of doing so is prohibitive... as in, if I couldn't even understand how to do it over a couple of evenings, I can't guarantee allocating any time to it in the coming months and would focus on more achievable targets - I am sort of overbooked. Long story short, if anyone can pick up writing the testing rituals properly, and get the PRs merged, they really are more than welcome!
          Hide
          bitwiseman Liam Newman added a comment -

          Jim Klimov
          I understand, you just described everyone's eternal problem.

          This is definitely a useful addition, but I have other things that are higher priority that I have to do as well. This is why I asked you to write as much of test as you could and push those changes. If you provide a test that you know exercises this behavior, someone with knowledge can add the checks to show it working. I might be able find the time to do that much, but not to write the tests from scratch.

          Show
          bitwiseman Liam Newman added a comment - Jim Klimov I understand, you just described everyone's eternal problem. This is definitely a useful addition, but I have other things that are higher priority that I have to do as well. This is why I asked you to write as much of test as you could and push those changes. If you provide a test that you know exercises this behavior, someone with knowledge can add the checks to show it working. I might be able find the time to do that much, but not to write the tests from scratch.

            People

            Assignee:
            Unassigned Unassigned
            Reporter:
            jimklimov Jim Klimov
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Dates

              Created:
              Updated: