Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1029: Skara bot should prevent integration of PR with incorrect issue type #1162

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -195,6 +195,19 @@ private List<String> botSpecificIntegrationBlockers() {
ret.add("Title mismatch between PR and JBS for issue " + issueString);
setExpiration(Duration.ofMinutes(10));
}

var properties = iss.get().properties();
if (!properties.containsKey("issuetype")) {
var issueString = "[" + iss.get().id() + "](" + iss.get().webUrl() + ")";
ret.add("Issue " + issueString + " does not contain property `issuetype`");
setExpiration(Duration.ofMinutes(10));
} else {
var issueType = properties.get("issuetype").asString();
if (!primaryTypes.contains(issueType)) {
ret.add("Issue of type `" + issueType + "` is not allowed for integrations");
setExpiration(Duration.ofMinutes(10));
}
}
} else {
log.warning("Failed to retrieve information on issue " + currentIssue.id());
setExpiration(Duration.ofMinutes(10));
@@ -478,10 +491,6 @@ private String getStatusMessage(List<Comment> comments, List<Review> reviews, Pu
progressBody.append(" ⚠️ Issue is not open.");
}
}
if (properties.containsKey("issuetype") && !primaryTypes.contains(properties.get("issuetype").asString())) {
progressBody.append(" ⚠️ Unexpected issue type `").append(properties.get("issuetype").asString()).append("`.");
setExpiration(Duration.ofMinutes(10));
}
progressBody.append("\n");
} else {
progressBody.append("⚠️ Failed to retrieve information on issue `");
@@ -970,7 +970,7 @@ void issueInSummary(TestInfo testInfo) throws IOException {
var masterHash = localRepo.resolve("master").orElseThrow();
localRepo.push(masterHash, author.url(), "master", true);

var issue1 = issues.createIssue("My first issue", List.of("Hello"), Map.of());
var issue1 = issues.createIssue("My first issue", List.of("Hello"), Map.of("issuetype", JSON.of("Bug")));

// Make a change with a corresponding PR
var editHash = CheckableRepository.appendAndCommit(localRepo);
@@ -990,7 +990,7 @@ void issueInSummary(TestInfo testInfo) throws IOException {
assertTrue(pr.body().contains("My first issue"));

// Change the issue
var issue2 = issues.createIssue("My second issue", List.of("Body"), Map.of());
var issue2 = issues.createIssue("My second issue", List.of("Body"), Map.of("issuetype", JSON.of("Bug")));
pr.setTitle(issue2.id() + ": This is a pull request");

// Check the status again
@@ -1067,7 +1067,7 @@ void issueInSummaryExternalUpdate(TestInfo testInfo) throws IOException {
var masterHash = localRepo.resolve("master").orElseThrow();
localRepo.push(masterHash, author.url(), "master", true);

var issue1 = issues.createIssue("My first issue", List.of("Hello"), Map.of());
var issue1 = issues.createIssue("My first issue", List.of("Hello"), Map.of("issuetype", JSON.of("Bug")));

// Make a change with a corresponding PR
var editHash = CheckableRepository.appendAndCommit(localRepo);
@@ -1087,7 +1087,7 @@ void issueInSummaryExternalUpdate(TestInfo testInfo) throws IOException {
assertTrue(pr.body().contains("My first issue"));

// Change the issue
var issue2 = issues.createIssue("My second issue", List.of("Body"), Map.of());
var issue2 = issues.createIssue("My second issue", List.of("Body"), Map.of("issuetype", JSON.of("Bug")));
pr.setTitle(issue2.id() + ": This is a pull request");

// Check the status again
@@ -1507,8 +1507,8 @@ void allowedIssueTypes(TestInfo testInfo) throws IOException {

var bug = issues.createIssue("My first bug", List.of("A bug"),
Map.of("issuetype", JSON.of("Bug")));
var feature = issues.createIssue("My first feature", List.of("A feature"),
Map.of("issuetype", JSON.of("Backport")));
var backport = issues.createIssue("My first feature", List.of("A feature"),
Map.of("issuetype", JSON.of("Backport")));

// Populate the projects repository
var localRepo = CheckableRepository.init(tempFolder.path(), author.repositoryType());
@@ -1531,17 +1531,17 @@ void allowedIssueTypes(TestInfo testInfo) throws IOException {
assertEquals(CheckStatus.SUCCESS, bugCheck.status());

// Make a change with a corresponding PR
var featureHash = CheckableRepository.appendAndCommit(localRepo);
localRepo.push(featureHash, author.url(), "feature", true);
var featurePR = credentials.createPullRequest(author, "master", "feature",
feature.id() + ": My first feature", true);
var backportHash = CheckableRepository.appendAndCommit(localRepo);
localRepo.push(backportHash, author.url(), "backport", true);
var backportPR = credentials.createPullRequest(author, "master", "backport",
backport.id() + ": My first backport", true);

// Check the status
TestBotRunner.runPeriodicItems(checkBot);
assertTrue(featurePR.body().contains(feature.id()));
assertTrue(featurePR.body().contains("My first feature"));
assertTrue(featurePR.body().contains("## Issue\n"));
assertTrue(featurePR.body().contains("Unexpected issue type"));
assertTrue(backportPR.body().contains(backport.id()));
assertTrue(backportPR.body().contains("My first feature"));
assertTrue(backportPR.body().contains("### Integration blocker"));
assertTrue(backportPR.body().contains("Issue of type `Backport` is not allowed for integrations"));
}
}

@@ -378,7 +378,7 @@ public PullRequest createPullRequest(HostedRepository hostedRepository, String t
}

public Issue createIssue(IssueProject issueProject, String title) {
var issue = issueProject.createIssue(title, List.of(), Map.of());
var issue = issueProject.createIssue(title, List.of(), Map.of("issuetype", JSON.of("Bug")));
issuesToBeClosed.add(issue);
return issue;
}