search

KostyaSha / github-integration-plugin

Jenkins GitHub Integration Plugin
https://wiki.jenkins-ci.org/display/JENKINS/GitHub+Integration+Plugin
MIT License
98 stars 86 forks source link

[bugfix] Fix Github Branch Commit Checks Bug #391

Open smilejj91 opened 2 days ago

smilejj91 commented 2 days ago

This is a PR fixing an issue related to Commit Check in the Github Branch section of the build trigger.

  1. Fixed an issue where the job configuration was broken when saving after activating the Commit Message pattern option.

    • The issue is related to this link

  2. Fixed an issue where the job was not triggered even if it matched the Commit Message Pattern.

We would appreciate your positive review.

Thank you.

This change is Reviewable

smilejj91 commented 2 days ago

@KostyaSha Hello. I mentioned you because I know you are the maintainer of the repository. I would appreciate it if you could check the PR. Thank you. 😀

KostyaSha commented 2 days ago

would be good to make some test...

smilejj91 commented 2 days ago

would be good to make some test...

@KostyaSha you're right. I have already checked the relevant functional tests and provided a PR. Is it possible to attach a test image for confirmation?

smilejj91 commented 2 days ago

[how to reproduce issue] Create Freestyle Job -> Job Configuration -> Build triggers -> Github Branches -> Trigger Events -> Commit Checks -> Commit Message Pattern -> Job Save -> Reopen Job Configuration

image

[before apply this commit] : job configuration is broken

image

[after apply this commit] : job configuration is not broken

image
smilejj91 commented 2 days ago

I already apply this commit and use github branches commit check feature. I am using it well without any issues so far. If this commit is reflected in the official plugin, it would be nice for me to not have to separately manage the custom plugin.

KostyaSha commented 1 day ago

I mean for GitHubBranchCommitMessageCheck.java not, UI.

smilejj91 commented 1 day ago

I mean for GitHubBranchCommitMessageCheck.java not, UI.

@KostyaSha Oh, I see. It would be a good idea to test it in your own environment before and after applying the commit. It seems difficult to show the functional test results before and after modification in this PR.

What is certain is that if the current commit is not reflected, the commit message is compared well, but when it passes after comparison, the job is not triggered.

KostyaSha commented 1 day ago

Usually it's done with mocks, you initialise only MessageCheck and pass mocked objects that contains commit pattern and check that it return expected cause

smilejj91 commented 1 day ago

@KostyaSha thank you. Test execution may have been skipped in the master branch, so I thought the test had passed. Related test code was modified and reflected in additional commits. test-success.txt