Add a simple check to ensure all pull request tasks are completed

Author: marko-bekhta

pom.xml

@@ -140,6 +140,11 @@
       <artifactId>mockito-junit-jupiter</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>io.quarkus</groupId>
+      <artifactId>quarkus-junit5-mockito</artifactId>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
   <build>
     <plugins>

src/main/java/org/hibernate/infra/bot/CheckPullRequestContributionRules.java

@@ -166,6 +166,11 @@ private List<PullRequestCheck> createChecks(RepositoryConfig repositoryConfig, S
 			}
 		}
 
+		if ( repositoryConfig != null && repositoryConfig.pullRequestTasks != null
+				&& repositoryConfig.pullRequestTasks.getEnabled().orElse( Boolean.FALSE ) ) {
+			checks.add( new TasksCompletedCheck() );
+		}
+
 		return checks;
 	}
 
@@ -282,15 +287,34 @@ protected LicenseCheck(String agreementText) {
 		}
 
 		@Override
-		public void perform(PullRequestCheckRunContext context, PullRequestCheckRunOutput output) throws IOException {
+		public void perform(PullRequestCheckRunContext context, PullRequestCheckRunOutput output) {
+			String body = context.pullRequest.getBody();
+			PullRequestCheckRunRule rule = output.rule( "The pull request description must contain the license agreement text." );
+			if ( body != null && body.contains( agreementText ) ) {
+				rule.passed();
+			}
+			else {
+				rule.failed( """
+						The description of this pull request must contain the following license agreement text:
+						```
+						%s
+						```
+						""".formatted( agreementText ) );
+			}
+		}
+	}
+
+	static class TasksCompletedCheck extends PullRequestCheck {
+
+		protected TasksCompletedCheck() {
+			super( "Contribution — Review tasks" );
+		}
+
+		@Override
+		public void perform(PullRequestCheckRunContext context, PullRequestCheckRunOutput output) {
 			String body = context.pullRequest.getBody();
-			output.rule( """
-							The pull request description must contain the following license agreement text:
-							```
-							%s
-							```
-							""".formatted( agreementText ) )
-					.result( body != null && body.contains( agreementText ) );
+			output.rule( "All pull request tasks should be completed." )
+					.result( !EditPullRequestBodyAddTaskList.containsUnfinishedTasks( body ) );
 		}
 	}
 

src/main/java/org/hibernate/infra/bot/EditPullRequestBodyAddTaskList.java

@@ -34,6 +34,7 @@ public class EditPullRequestBodyAddTaskList {
 	private static final String START_MARKER = "<!-- Hibernate GitHub Bot task list start -->";
 
 	private static final String END_MARKER = "<!-- Hibernate GitHub Bot task list end -->";
+	private static final Set<Character> REGEX_ESCAPE_CHARS = Set.of( '(', ')', '[', ']', '{', '}', '\\', '.', '?', '*', '+' );
 
 	@Inject
 	DeploymentConfig deploymentConfig;
@@ -113,7 +114,20 @@ else if ( currentTasks != null ) {
 	}
 
 	private boolean tasksAreTheSame(String currentTasks, String tasks) {
-		return Patterns.compile( tasks.replace( "- [ ]", "- \\[.\\]" ).replace( "\n", "\\n" ) )
+		StringBuilder sb = new StringBuilder();
+		for ( char c : tasks.toCharArray() ) {
+			if ( REGEX_ESCAPE_CHARS.contains( c ) ) {
+				sb.append( '\\' );
+			}
+			if ( c == '\n' ) {
+				sb.append( '\\' ).append( 'n' );
+			}
+			else {
+				sb.append( c );
+			}
+		}
+
+		return Patterns.compile( sb.toString().replace( "- \\[ \\]", "- \\[.\\]" ) )
 				.matcher( currentTasks )
 				.matches();
 	}
@@ -151,6 +165,17 @@ private void addTasks(StringBuilder sb, List<String> tasks) {
 		sb.append( "\n" );
 	}
 
+	public static boolean containsUnfinishedTasks(String body) {
+		if ( body == null || body.isEmpty() ) {
+			return false;
+		}
+		String taskBody = currentTaskBody( body );
+		if ( taskBody == null ) {
+			return false;
+		}
+		return taskBody.contains( "- [ ]" );
+	}
+
 	private static String currentTaskBody(String originalBody) {
 		// Check if the body already contains the tasks
 		final int startIndex = originalBody.indexOf( START_MARKER );

src/test/java/org/hibernate/infra/bot/tests/AbstractPullRequestTest.java

@@ -16,8 +16,6 @@ abstract class AbstractPullRequestTest {
 	final GHCheckRunBuilder titleCheckRunUpdateBuilderMock = mockCheckRunBuilder();
 	final GHCheckRunBuilder jiraCheckRunCreateBuilderMock = mockCheckRunBuilder();
 	final GHCheckRunBuilder jiraCheckRunUpdateBuilderMock = mockCheckRunBuilder();
-	final GHCheckRunBuilder licenseCheckRunCreateBuilderMock = mockCheckRunBuilder();
-	final GHCheckRunBuilder licenseCheckRunUpdateBuilderMock = mockCheckRunBuilder();
 
 	GHCheckRunBuilder mockCheckRunBuilder() {
 		return mock( GHCheckRunBuilder.class, withSettings().defaultAnswer( Answers.RETURNS_SELF ) );
@@ -36,11 +34,6 @@ void mockCheckRuns(GHRepository repoMock, String headSHA) throws IOException {
 		);
 		mockUpdateCheckRun( repoMock, 43L, jiraCheckRunUpdateBuilderMock, jiraCheckRunMock );
 
-		GHCheckRun licenseCheckRunMock = mock( GHCheckRun.class );
-		mockCreateCheckRun( repoMock, "Contribution — License agreement", headSHA,
-				licenseCheckRunCreateBuilderMock, licenseCheckRunMock, 44L
-		);
-		mockUpdateCheckRun( repoMock, 44L, licenseCheckRunUpdateBuilderMock, licenseCheckRunMock );
 	}
 
 	void mockCreateCheckRun(GHRepository repoMock, String name, String headSHA,

src/test/java/org/hibernate/infra/bot/tests/CheckPullRequestContributionRulesLicenseTest.java

@@ -2,6 +2,7 @@
 
 import static io.quarkiverse.githubapp.testing.GitHubAppTesting.given;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;
@@ -13,6 +14,8 @@
 
 import io.quarkiverse.githubapp.testing.GitHubAppTest;
 import io.quarkus.test.junit.QuarkusTest;
+import org.kohsuke.github.GHCheckRun;
+import org.kohsuke.github.GHCheckRunBuilder;
 import org.kohsuke.github.GHEvent;
 import org.kohsuke.github.GHPullRequest;
 import org.kohsuke.github.GHRepository;
@@ -23,6 +26,20 @@
 @GitHubAppTest
 @ExtendWith(MockitoExtension.class)
 public class CheckPullRequestContributionRulesLicenseTest extends AbstractPullRequestTest {
+
+	final GHCheckRunBuilder licenseCheckRunCreateBuilderMock = mockCheckRunBuilder();
+	final GHCheckRunBuilder licenseCheckRunUpdateBuilderMock = mockCheckRunBuilder();
+
+	@Override
+	void mockCheckRuns(GHRepository repoMock, String headSHA) throws IOException {
+		super.mockCheckRuns( repoMock, headSHA );
+		GHCheckRun licenseCheckRunMock = mock( GHCheckRun.class );
+		mockCreateCheckRun( repoMock, "Contribution — License agreement", headSHA,
+				licenseCheckRunCreateBuilderMock, licenseCheckRunMock, 44L
+		);
+		mockUpdateCheckRun( repoMock, 44L, licenseCheckRunUpdateBuilderMock, licenseCheckRunMock );
+	}
+
 	@Test
 	void bodyMissingLicenseAgreement() throws IOException {
 		long repoId = 344815557L;
@@ -75,7 +92,8 @@ void bodyMissingLicenseAgreement() throws IOException {
 									
 									This pull request does not follow the contribution rules. Could you have a look?
 									
-									❌ The pull request description must contain the following license agreement text:
+									❌ The pull request description must contain the license agreement text.
+									    ↳ The description of this pull request must contain the following license agreement text:
 									```
 									----------------------
 									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)