diff --git a/src/main/java/org/hibernate/infra/bot/CheckPullRequestContributionRules.java b/src/main/java/org/hibernate/infra/bot/CheckPullRequestContributionRules.java index d6c737b..cf73ae5 100644 --- a/src/main/java/org/hibernate/infra/bot/CheckPullRequestContributionRules.java +++ b/src/main/java/org/hibernate/infra/bot/CheckPullRequestContributionRules.java @@ -158,9 +158,8 @@ private List createChecks(RepositoryConfig repositoryConfig, S && repositoryConfig.licenseAgreement != null && repositoryConfig.licenseAgreement.getEnabled().orElse( Boolean.FALSE ) ) { Matcher matcher = repositoryConfig.licenseAgreement.getPullRequestTemplatePattern().matcher( pullRequestTemplate ); - if ( matcher.matches() - && matcher.groupCount() == 1 ) { - checks.add( new LicenseCheck( matcher.group( 1 ).trim() ) ); + if ( matcher.matches() && matcher.groupCount() == 1 ) { + checks.add( new LicenseCheck( matcher.group( 1 ).trim(), repositoryConfig.licenseAgreement.getIgnore() ) ); } else { throw new IllegalArgumentException( "Misconfigured license agreement check. Pattern should contain exactly 1 match group. Pattern: %s. Fetched Pull Request template: %s".formatted( repositoryConfig.licenseAgreement.getPullRequestTemplatePattern(), pullRequestTemplate ) ); @@ -169,7 +168,7 @@ private List createChecks(RepositoryConfig repositoryConfig, S if ( repositoryConfig != null && repositoryConfig.pullRequestTasks != null && repositoryConfig.pullRequestTasks.getEnabled().orElse( Boolean.FALSE ) ) { - checks.add( new TasksCompletedCheck() ); + checks.add( new TasksCompletedCheck(repositoryConfig.pullRequestTasks.getIgnore() ) ); } return checks; @@ -192,33 +191,25 @@ public void perform(PullRequestCheckRunContext context, PullRequestCheckRunOutpu } } - static class JiraIssuesCheck extends PullRequestCheck { + static class JiraIssuesCheck extends IgnorablePullRequestCheck { private final Pattern issueKeyPattern; private final Integer issueLinksLimit; - private final List ignoredPRConfigurations; private final GlobMatcher ignoredFilesMatcher; JiraIssuesCheck(Pattern issueKeyPattern, Integer issueLinksLimit, List ignoredPRConfigurations, List ignoreFilePatterns) { - super( "Contribution — JIRA issues" ); + super( "Contribution — JIRA issues", ignoredPRConfigurations ); this.issueKeyPattern = issueKeyPattern; this.issueLinksLimit = issueLinksLimit; - this.ignoredPRConfigurations = ignoredPRConfigurations; this.ignoredFilesMatcher = new GlobMatcher( ignoreFilePatterns ); } @Override - public void perform(PullRequestCheckRunContext context, PullRequestCheckRunOutput output) throws IOException { - if ( !shouldCheckPullRequest( context ) ) { - // Means we have an ignore rule configured that matches our pull request. - // No need to check anything else. - return; - } - + public void doPerform(PullRequestCheckRunContext context, PullRequestCheckRunOutput output) throws IOException { String title = context.pullRequest.getTitle(); String body = context.pullRequest.getBody(); @@ -263,32 +254,19 @@ public void perform(PullRequestCheckRunContext context, PullRequestCheckRunOutpu } } } - - private boolean shouldCheckPullRequest(PullRequestCheckRunContext context) throws IOException { - GHUser author = context.pullRequest.getUser(); - String title = context.pullRequest.getTitle(); - for ( RepositoryConfig.IgnoreConfiguration ignore : ignoredPRConfigurations ) { - if ( ignore.getUser().equals( author.getLogin() ) - && ignore.getTitlePattern().matcher( title ).matches() ) { - return false; - } - } - - return true; - } } - static class LicenseCheck extends PullRequestCheck { + static class LicenseCheck extends IgnorablePullRequestCheck { private final String agreementText; - protected LicenseCheck(String agreementText) { - super( "Contribution — License agreement" ); + protected LicenseCheck(String agreementText, List ignoredPRConfigurations) { + super( "Contribution — License agreement", ignoredPRConfigurations ); this.agreementText = Patterns.sanitizeNewLines( agreementText ); } @Override - public void perform(PullRequestCheckRunContext context, PullRequestCheckRunOutput output) { + public void doPerform(PullRequestCheckRunContext context, PullRequestCheckRunOutput output) { String body = Patterns.sanitizeNewLines( context.pullRequest.getBody() ); PullRequestCheckRunRule rule = output.rule( "The pull request description must contain the license agreement text." ); if ( body != null && body.contains( agreementText ) ) { @@ -305,18 +283,54 @@ public void perform(PullRequestCheckRunContext context, PullRequestCheckRunOutpu } } - static class TasksCompletedCheck extends PullRequestCheck { + static class TasksCompletedCheck extends IgnorablePullRequestCheck { - protected TasksCompletedCheck() { - super( "Contribution — Review tasks" ); + protected TasksCompletedCheck(List ignoredPRConfigurations) { + super( "Contribution — Review tasks", ignoredPRConfigurations ); } @Override - public void perform(PullRequestCheckRunContext context, PullRequestCheckRunOutput output) { + public void doPerform(PullRequestCheckRunContext context, PullRequestCheckRunOutput output) { String body = context.pullRequest.getBody(); output.rule( "All pull request tasks should be completed." ) .result( !EditPullRequestBodyAddTaskList.containsUnfinishedTasks( body ) ); } } + static abstract class IgnorablePullRequestCheck extends PullRequestCheck { + + private final List ignoredPRConfigurations; + + protected IgnorablePullRequestCheck(String title, List ignoredPRConfigurations) { + super( title ); + this.ignoredPRConfigurations = ignoredPRConfigurations; + } + + @Override + public final void perform(PullRequestCheckRunContext context, PullRequestCheckRunOutput output) throws IOException { + if ( !shouldCheckPullRequest( context ) ) { + // Means we have an ignore rule configured that matches our pull request. + // No need to check anything else. + return; + } + + doPerform( context, output ); + } + + abstract void doPerform(PullRequestCheckRunContext context, PullRequestCheckRunOutput output) throws IOException; + + protected boolean shouldCheckPullRequest(PullRequestCheckRunContext context) throws IOException { + GHUser author = context.pullRequest.getUser(); + String title = context.pullRequest.getTitle(); + for ( RepositoryConfig.IgnoreConfiguration ignore : ignoredPRConfigurations ) { + if ( ignore.getUser().equals( author.getLogin() ) + && ignore.getTitlePattern().matcher( title ).matches() ) { + return false; + } + } + + return true; + } + } + } diff --git a/src/main/java/org/hibernate/infra/bot/EditPullRequestBodyAddTaskList.java b/src/main/java/org/hibernate/infra/bot/EditPullRequestBodyAddTaskList.java index deefebf..1704077 100644 --- a/src/main/java/org/hibernate/infra/bot/EditPullRequestBodyAddTaskList.java +++ b/src/main/java/org/hibernate/infra/bot/EditPullRequestBodyAddTaskList.java @@ -27,6 +27,7 @@ import org.kohsuke.github.GHPullRequest; import org.kohsuke.github.GHPullRequestCommitDetail; import org.kohsuke.github.GHRepository; +import org.kohsuke.github.GHUser; public class EditPullRequestBodyAddTaskList { private static final Logger LOG = Logger.getLogger( EditPullRequestBodyAddTaskList.class ); @@ -58,7 +59,7 @@ private void addUpdateTaskList( return; } - if ( !shouldCheck( repository, pullRequest ) ) { + if ( !shouldCheck( repository, pullRequest, repositoryConfig.pullRequestTasks.getIgnore() ) ) { return; } @@ -175,7 +176,15 @@ private static String currentTaskBody(String originalBody) { } } - private boolean shouldCheck(GHRepository repository, GHPullRequest pullRequest) { + private boolean shouldCheck(GHRepository repository, GHPullRequest pullRequest, List ignoredPRConfigurations) throws IOException { + GHUser author = pullRequest.getUser(); + String title = pullRequest.getTitle(); + for ( RepositoryConfig.IgnoreConfiguration ignore : ignoredPRConfigurations ) { + if ( ignore.getUser().equals( author.getLogin() ) + && ignore.getTitlePattern().matcher( title ).matches() ) { + return false; + } + } return !GHIssueState.CLOSED.equals( pullRequest.getState() ) && repository.getId() == pullRequest.getBase().getRepository().getId(); } diff --git a/src/main/java/org/hibernate/infra/bot/config/RepositoryConfig.java b/src/main/java/org/hibernate/infra/bot/config/RepositoryConfig.java index f59c125..e277199 100644 --- a/src/main/java/org/hibernate/infra/bot/config/RepositoryConfig.java +++ b/src/main/java/org/hibernate/infra/bot/config/RepositoryConfig.java @@ -138,6 +138,7 @@ public void setPattern(String pattern) { public static class LicenseAgreement { private Optional enabled = Optional.empty(); private Pattern pullRequestTemplatePattern = Patterns.compile( ".+([-]{22}.+[-]{22}).++" ); + private List ignore = Collections.emptyList(); public Optional getEnabled() { return enabled; @@ -154,12 +155,21 @@ public Pattern getPullRequestTemplatePattern() { public void setPullRequestTemplatePattern(String pullRequestTemplatePattern) { this.pullRequestTemplatePattern = Patterns.compile( pullRequestTemplatePattern ); } + + public List getIgnore() { + return ignore; + } + + public void setIgnore(List ignore) { + this.ignore = ignore; + } } public static class TaskList { private static final String DEFAULT_TASKS_CATEGORY = "default"; private Optional enabled = Optional.empty(); private Map> tasks = new HashMap<>(); + private List ignore = Collections.emptyList(); public Optional getEnabled() { return enabled; @@ -180,5 +190,13 @@ public void setTasks(Map> tasks) { public List defaultTasks() { return tasks.getOrDefault( DEFAULT_TASKS_CATEGORY, List.of() ); } + + public List getIgnore() { + return ignore; + } + + public void setIgnore(List ignore) { + this.ignore = ignore; + } } } diff --git a/src/test/java/org/hibernate/infra/bot/tests/CheckPullRequestContributionRulesLicenseTest.java b/src/test/java/org/hibernate/infra/bot/tests/CheckPullRequestContributionRulesLicenseTest.java index 2f097f0..41666eb 100644 --- a/src/test/java/org/hibernate/infra/bot/tests/CheckPullRequestContributionRulesLicenseTest.java +++ b/src/test/java/org/hibernate/infra/bot/tests/CheckPullRequestContributionRulesLicenseTest.java @@ -156,4 +156,56 @@ void bodyContainsLicenseAgreement() throws IOException { } ); } + @Test + void licenseCheckIgnored() throws IOException { + long repoId = 344815557L; + long prId = 585627026L; + given() + .github( mocks -> { + mocks.configFile("hibernate-github-bot.yml") + .fromString( """ + jira: + projectKey: "HSEARCH" + # We also ignore jira keys check as dependabot PRs won't have them anyways: + ignore: + - user: dependabot[bot] + titlePattern: ".*\\\\bmaven\\\\b.*\\\\bplugin\\\\b.*" + licenseAgreement: + enabled: true + ignore: + - user: dependabot[bot] + titlePattern: ".*\\\\bmaven\\\\b.*\\\\bplugin\\\\b.*" + """ ); + mocks.configFile("PULL_REQUEST_TEMPLATE.md") + .fromString( """ + [Please describe here what your change is about] + + + ---------------------- + By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license](https://www.apache.org/licenses/LICENSE-2.0.txt) + and can be relicensed under the terms of the [LGPL v2.1 license](https://www.gnu.org/licenses/old-licenses/lgpl-2.1.txt) in the future at the maintainers' discretion. + For more information on licensing, please check [here](https://github.com/hibernate/hibernate-search/blob/main/CONTRIBUTING.md#legal). + + ---------------------- + """ ); + + GHRepository repoMock = mocks.repository( "yrodiere/hibernate-github-bot-playground" ); + when( repoMock.getId() ).thenReturn( repoId ); + + PullRequestMockHelper.start( mocks, prId, repoMock ) + .noComments(); + + mockCheckRuns( repoMock, "6e9f11a1e2946b207c6eb245ec942f2b5a3ea156" ); + } ) + .when() + .payloadFromClasspath( "/pullrequest-opened-hsearch-1111-dependabot-upgrades-build-dependencies.json" ) + .event( GHEvent.PULL_REQUEST ) + .then() + .github( mocks -> { + verifyNoMoreInteractions( mocks.ghObjects() ); + } ); + } + } diff --git a/src/test/java/org/hibernate/infra/bot/tests/CheckPullRequestContributionRulesTasksTest.java b/src/test/java/org/hibernate/infra/bot/tests/CheckPullRequestContributionRulesTasksTest.java index e15dc0d..1afaadb 100644 --- a/src/test/java/org/hibernate/infra/bot/tests/CheckPullRequestContributionRulesTasksTest.java +++ b/src/test/java/org/hibernate/infra/bot/tests/CheckPullRequestContributionRulesTasksTest.java @@ -213,4 +213,58 @@ void bodyContainsTasks() throws IOException { } ); } + @Test + void tasksCheckIgnored() throws IOException { + long repoId = 344815557L; + long prId = 585627026L; + given() + .github( mocks -> { + mocks.configFile("hibernate-github-bot.yml") + .fromString( """ + jira: + projectKey: "HSEARCH" + # We also ignore jira keys check as dependabot PRs won't have them anyways: + ignore: + - user: dependabot[bot] + titlePattern: ".*\\\\bmaven\\\\b.*\\\\bplugin\\\\b.*" + pullRequestTasks: + enabled: true + tasks: + default: + - task1 + - task2 + - task3 + - task4 + bug: + - bug task1 + - bug task2 + - bug task3 + - bug task4 + improvement: + - improvement task1 + - improvement task2 + - improvement task3 + - improvement task4 + ignore: + - user: dependabot[bot] + titlePattern: ".*\\\\bmaven\\\\b.*\\\\bplugin\\\\b.*" + """ ); + + GHRepository repoMock = mocks.repository( "yrodiere/hibernate-github-bot-playground" ); + when( repoMock.getId() ).thenReturn( repoId ); + + PullRequestMockHelper.start( mocks, prId, repoMock ) + .noComments(); + + mockCheckRuns( repoMock, "6e9f11a1e2946b207c6eb245ec942f2b5a3ea156" ); + } ) + .when() + .payloadFromClasspath( "/pullrequest-opened-hsearch-1111-dependabot-upgrades-build-dependencies.json" ) + .event( GHEvent.PULL_REQUEST ) + .then() + .github( mocks -> { + verifyNoMoreInteractions( mocks.ghObjects() ); + } ); + } + }