Skip to content
Permalink
Browse files
jcheck: duplicate-issues should only check id and support "dup"
Reviewed-by: rwestberg
  • Loading branch information
edvbld committed Mar 23, 2020
1 parent 2596472 commit 2ccceab1484bf2a1916ac2bd694e00fa48c0a6f8
Showing 3 changed files with 158 additions and 7 deletions.
@@ -36,23 +36,24 @@
public class DuplicateIssuesCheck extends CommitCheck {
private final Logger log = Logger.getLogger("org.openjdk.skara.jcheck.duplicate-issues");
private final ReadOnlyRepository repo;
private Map<Issue, List<Hash>> issuesToHashes = null;
private Map<String, List<Hash>> issuesToHashes = null;

DuplicateIssuesCheck(ReadOnlyRepository repo) {
this.repo = repo;
}

private void populateIssuesToHashesMap() throws IOException {
issuesToHashes = new HashMap<Issue, List<Hash>>();
issuesToHashes = new HashMap<String, List<Hash>>();

for (var metadata : repo.commitMetadata()) {
for (var line : metadata.message()) {
var issue = Issue.fromString(line);
if (issue.isPresent()) {
if (!issuesToHashes.containsKey(issue.get())) {
issuesToHashes.put(issue.get(), new ArrayList<Hash>());
var id = issue.get().id();
if (!issuesToHashes.containsKey(id)) {
issuesToHashes.put(id, new ArrayList<Hash>());
}
issuesToHashes.get(issue.get()).add(metadata.hash());
issuesToHashes.get(id).add(metadata.hash());
}
}
}
@@ -71,10 +72,11 @@ Iterator<org.openjdk.skara.jcheck.Issue> check(Commit commit, CommitMessage mess
var metadata = CommitIssue.metadata(commit, message, conf, this);
var issues = new ArrayList<org.openjdk.skara.jcheck.Issue>();
for (var issue : message.issues()) {
var hashes = issuesToHashes.get(issue);
var hashes = issuesToHashes.get(issue.id());
if (hashes != null && hashes.size() > 1) {
log.finer("issue: the JBS issue " + issue.toString() + " has been used in multiple commits");
issues.add(new DuplicateIssuesIssue(issue, hashes, metadata));
var uniqueHashes = new ArrayList<>(new HashSet<>(hashes));
issues.add(new DuplicateIssuesIssue(issue, uniqueHashes, metadata));
}
}
return issues.iterator();
@@ -86,6 +86,10 @@ private static INI convert(INI old) {
error += ",message,hg-tag";
shouldCheckMessage = true;
}
var checkDuplicateIssues = old.get("bugids");
if (checkDuplicateIssues == null || !checkDuplicateIssues.equals("dup")) {
error += ",duplicate-issues";
}
config.add(error);

if (project.startsWith("jdk")) {
@@ -0,0 +1,145 @@
/*
* Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
package org.openjdk.skara.jcheck;

import org.openjdk.skara.vcs.*;
import org.openjdk.skara.vcs.openjdk.*;
import org.openjdk.skara.test.TemporaryDirectory;

import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.*;

import java.util.*;
import java.time.ZonedDateTime;
import java.io.IOException;
import java.nio.file.*;
import static java.nio.file.StandardOpenOption.*;

class DuplicateIssuesCheckTests {
private static JCheckConfiguration conf() {
return JCheckConfiguration.parse(List.of(
"[general]",
"project = test",
"[checks]",
"error = duplicate-issues"
));
}

private static CommitMessage message(Commit c) {
return CommitMessageParsers.v1.parse(c);
}

private static List<Issue> toList(Iterator<Issue> i) {
var list = new ArrayList<Issue>();
while (i.hasNext()) {
list.add(i.next());
}
return list;
}

@Test
void noDuplicatedIssuesShouldPass() throws IOException {
try (var dir = new TemporaryDirectory()) {
var r = Repository.init(dir.path(), VCS.GIT);

var readme = dir.path().resolve("README");
Files.write(readme, List.of("Hello, world!"));
r.add(readme);
var first = r.commit("1: Added README and .jcheck/conf", "duke", "duke@openjdk.java.net");

Files.write(readme, List.of("One more line"), WRITE, APPEND);
r.add(readme);
var second = r.commit("2: Modified README", "duke", "duke@openjdk.java.net");

Files.write(readme, List.of("One more line again"), WRITE, APPEND);
r.add(readme);
var third = r.commit("3: Modified README again", "duke", "duke@openjdk.java.net");
var check = new DuplicateIssuesCheck(r);

var commit = r.lookup(third).orElseThrow();
var issues = toList(check.check(commit, message(commit), conf()));
assertEquals(List.of(), issues);
}
}

@Test
void duplicateIssuesInMessageShouldFail() throws IOException {
try (var dir = new TemporaryDirectory()) {
var r = Repository.init(dir.path(), VCS.GIT);

var readme = dir.path().resolve("README");
Files.write(readme, List.of("Hello, world!"));
r.add(readme);
var first = r.commit("1: Added README and .jcheck/conf", "duke", "duke@openjdk.java.net");

Files.write(readme, List.of("One more line"), WRITE, APPEND);
r.add(readme);
var second = r.commit("2: Modified README", "duke", "duke@openjdk.java.net");

Files.write(readme, List.of("One more line again"), WRITE, APPEND);
r.add(readme);
var third = r.commit("3: Modified README again\n3: Modified README again", "duke", "duke@openjdk.java.net");

var check = new DuplicateIssuesCheck(r);

var commit = r.lookup(third).orElseThrow();
var issues = toList(check.check(commit, message(commit), conf()));
assertEquals(2, issues.size());
assertTrue(issues.get(0) instanceof DuplicateIssuesIssue);
var issue = (DuplicateIssuesIssue) issues.get(0);
assertEquals("3", issue.issue().id());
assertEquals(List.of(third), issue.hashes());
}
}

@Test
void duplicateIssuesInPreviousCommitsShouldFail() throws IOException {
try (var dir = new TemporaryDirectory()) {
var r = Repository.init(dir.path(), VCS.GIT);

var readme = dir.path().resolve("README");
Files.write(readme, List.of("Hello, world!"));
r.add(readme);
var first = r.commit("1: Added README and .jcheck/conf", "duke", "duke@openjdk.java.net");

Files.write(readme, List.of("One more line"), WRITE, APPEND);
r.add(readme);
var second = r.commit("2: Modified README", "duke", "duke@openjdk.java.net");

Files.write(readme, List.of("One more line again"), WRITE, APPEND);
r.add(readme);
var third = r.commit("2: Modified README again", "duke", "duke@openjdk.java.net");

var check = new DuplicateIssuesCheck(r);
var commit = r.lookup(third).orElseThrow();
var issues = toList(check.check(commit, message(commit), conf()));
assertEquals(1, issues.size());
assertTrue(issues.get(0) instanceof DuplicateIssuesIssue);
var issue = (DuplicateIssuesIssue) issues.get(0);
assertEquals("2", issue.issue().id());
assertEquals(2, issue.hashes().size());
assertTrue(issue.hashes().contains(second));
assertTrue(issue.hashes().contains(third));
}
}
}

0 comments on commit 2ccceab

Please sign in to comment.