Format with google-java-format 1.7 Change-Id: I2c0caa26bd4a6b28ae1a60037863fff2742984d1
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/Action.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/Action.java index 6da211c..85b3f2a 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/findowners/Action.java +++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/Action.java
@@ -124,10 +124,7 @@ static List<String> getReviewers(ChangeData changeData, AccountCache accountCache) { try { // Reviewers may have no preferred email, skip them if the preferred email is not set. - return changeData - .reviewers() - .all() - .stream() + return changeData.reviewers().all().stream() .map(accountCache::get) .flatMap(Streams::stream) .map(a -> a.getAccount().getPreferredEmail()) @@ -162,8 +159,17 @@ int patchset = getValidPatchsetNum(changeData, params.patchset); ProjectState projectState = projectCache.get(changeData.project()); Boolean useCache = params.nocache == null || !params.nocache; - OwnersDb db = Cache.getInstance(configFactory, repoManager).get( - useCache, projectState, accountCache, emails, repoManager, configFactory, changeData, patchset); + OwnersDb db = + Cache.getInstance(configFactory, repoManager) + .get( + useCache, + projectState, + accountCache, + emails, + repoManager, + configFactory, + changeData, + patchset); Collection<String> changedFiles = changeData.currentFilePaths(); Map<String, Set<String>> file2Owners = db.findOwners(changedFiles);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/Checker.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/Checker.java index 7b7540b..21af450 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/findowners/Checker.java +++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/Checker.java
@@ -47,8 +47,12 @@ private final ChangeData changeData; private int minVoteLevel; - Checker(GitRepositoryManager repoManager, PluginConfigFactory configFactory, - ProjectState projectState, ChangeData changeData, int v) { + Checker( + GitRepositoryManager repoManager, + PluginConfigFactory configFactory, + ProjectState projectState, + ChangeData changeData, + int v) { this.repoManager = repoManager; this.configFactory = configFactory; this.projectState = projectState; @@ -122,14 +126,15 @@ ChangeData changeData = null; try { changeData = StoredValues.CHANGE_DATA.get(engine); - Checker checker = new Checker( - StoredValues.REPO_MANAGER.get(engine), - StoredValues.PLUGIN_CONFIG_FACTORY.get(engine), - StoredValues.PROJECT_STATE.get(engine), - changeData, minVoteLevel); + Checker checker = + new Checker( + StoredValues.REPO_MANAGER.get(engine), + StoredValues.PLUGIN_CONFIG_FACTORY.get(engine), + StoredValues.PROJECT_STATE.get(engine), + changeData, + minVoteLevel); return checker.findApproval( - StoredValues.ACCOUNT_CACHE.get(engine), - StoredValues.EMAILS.get(engine)); + StoredValues.ACCOUNT_CACHE.get(engine), StoredValues.EMAILS.get(engine)); } catch (OrmException | IOException e) { logger.atSevere().withCause(e).log("Exception for %s ", Config.getChangeId(changeData)); return 0; // owner approval may or may not be required. @@ -137,8 +142,7 @@ } /** Returns 1 if owner approval is found, -1 if missing, 0 if unneeded. */ - int findApproval(AccountCache accountCache, Emails emails) - throws OrmException, IOException { + int findApproval(AccountCache accountCache, Emails emails) throws OrmException, IOException { if (isExemptFromOwnerApproval(changeData)) { return 0; }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/Config.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/Config.java index 0653f3f..9d5ec98 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/findowners/Config.java +++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/Config.java
@@ -107,7 +107,8 @@ } String name = getPluginConfig(projectState).getString(OWNERS_FILE_NAME, defaultName); if (name.trim().isEmpty()) { - logger.atSevere().log("Project %s has wrong %s: \"%s\" for %s", + logger.atSevere().log( + "Project %s has wrong %s: \"%s\" for %s", projectState.getProject(), OWNERS_FILE_NAME, name, getChangeId(c)); return defaultName; }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/OwnersDb.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/OwnersDb.java index 2ae18de..d706f2d 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/findowners/OwnersDb.java +++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/OwnersDb.java
@@ -16,8 +16,8 @@ import static java.nio.charset.StandardCharsets.UTF_8; -import com.google.common.collect.Ordering; import com.google.common.collect.Multimap; +import com.google.common.collect.Ordering; import com.google.common.flogger.FluentLogger; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Project; @@ -115,8 +115,8 @@ String found = "Found"; if (content.isEmpty()) { String changeId = Config.getChangeId(changeData); - logger.atSevere().log("Missing root %s for %s of %s", - ownersFileName, changeId, projectName); + logger.atSevere().log( + "Missing root %s for %s of %s", ownersFileName, changeId, projectName); found = "Missing"; } logs.add(found + " root " + ownersFileName); @@ -140,8 +140,13 @@ String filePath = dir + "/" + ownersFileName; String content = getFile(readFiles, repo, projectName, id, filePath, logs); if (content != null && !content.isEmpty()) { - addFile(readFiles, projectName, branch, dir + "/", dir + "/" + ownersFileName, - content.split("\\R")); + addFile( + readFiles, + projectName, + branch, + dir + "/", + dir + "/" + ownersFileName, + content.split("\\R")); } if (stopLooking.contains(dir + "/") || !dir.contains("/")) { break; // stop looking through parent directory @@ -232,8 +237,13 @@ } } - void addFile(Map<String, String> readFiles, String project, String branch, - String dirPath, String filePath, String[] lines) { + void addFile( + Map<String, String> readFiles, + String project, + String branch, + String dirPath, + String filePath, + String[] lines) { Parser parser = new Parser(readFiles, repoManager, project, branch, filePath, logs); Parser.Result result = parser.parseFile(dirPath, lines); if (result.stopLooking) { @@ -338,7 +348,7 @@ foundStar |= findStarOwner(dirPath + "/", distance, paths, distances); } if (stopLooking.contains(dirPath + "/") // stop looking parent - || foundNoParentGlob // per-file "set noparent" + || foundNoParentGlob // per-file "set noparent" || !dirPath.contains("/") /* root */) { break; } @@ -407,8 +417,13 @@ } /** Returns file content or empty string; uses project+branch+file names. */ - public static String getRepoFile(Map<String, String> readFiles, GitRepositoryManager repoManager, - String project, String branch, String file, List<String> logs) { + public static String getRepoFile( + Map<String, String> readFiles, + GitRepositoryManager repoManager, + String project, + String branch, + String file, + List<String> logs) { // 'file' must be an absolute path from the root of 'project'. logs.add("getRepoFile:" + Parser.getFileKey(project, branch, file)); file = Util.gitRepoFilePath(file); @@ -432,8 +447,13 @@ } /** Returns file content or empty string; uses Repository. */ - private static String getFile(Map<String, String> readFiles, - Repository repo, String project, ObjectId id, String file, List<String> logs) { + private static String getFile( + Map<String, String> readFiles, + Repository repo, + String project, + ObjectId id, + String file, + List<String> logs) { file = Util.gitRepoFilePath(file); String content = findReadFile(readFiles, project, file); if (content == null) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/OwnersValidator.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/OwnersValidator.java index 1b890a8..a91f3e4 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/findowners/OwnersValidator.java +++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/OwnersValidator.java
@@ -141,9 +141,10 @@ throw new CommitValidationException("failed to check owners files", e); } if (checker.hasError()) { - checker.addError("See OWNERS file syntax document at " - + "https://gerrit.googlesource.com/plugins/find-owners/+/" - + "master/src/main/resources/Documentation/syntax.md"); + checker.addError( + "See OWNERS file syntax document at " + + "https://gerrit.googlesource.com/plugins/find-owners/+/" + + "master/src/main/resources/Documentation/syntax.md"); throw new CommitValidationException("found invalid owners file", checker.messages); } return checker.messages; @@ -179,8 +180,8 @@ void check(String ownersFileName) throws IOException { Map<String, ObjectId> ownerFiles = allFiles.entrySet().stream() - .filter(e -> ownersFileName.equals(new File(e.getKey()).getName())) - .collect(Collectors.toMap(e -> e.getKey(), e -> e.getValue())); + .filter(e -> ownersFileName.equals(new File(e.getKey()).getName())) + .collect(Collectors.toMap(e -> e.getKey(), e -> e.getValue())); String projectName = event.project.getName(); for (String path : ownerFiles.keySet()) { String key = projectName + ":" + path; @@ -239,7 +240,8 @@ int num = 0; for (String line : lines) { checkLine(project, path, ++num, line); - }; + } + ; } void checkFile(String project, String path, String content) { @@ -248,8 +250,8 @@ void checkFile(String project, String path, ObjectLoader ol) { try { - BufferedReader reader = new BufferedReader( - new InputStreamReader(ol.openStream(), StandardCharsets.UTF_8)); + BufferedReader reader = + new BufferedReader(new InputStreamReader(ol.openStream(), StandardCharsets.UTF_8)); checkFile(project, path, reader.lines().toArray(String[]::new)); } catch (Exception e) { addError("cannot open file: " + path); @@ -308,9 +310,8 @@ } /** - * Check if an included file exists and with valid syntax. - * An included file could be (1) in the current CL, (2) in the same repository, - * (3) in a different repository, (4) in another CL. + * Check if an included file exists and with valid syntax. An included file could be (1) in the + * current CL, (2) in the same repository, (3) in a different repository, (4) in another CL. * Case (4) is not checked yet. */ void checkIncludeOrFile(String project, String path, int num, String line) { @@ -351,8 +352,9 @@ } // Included file is in repository or other CL. addVerboseMsg("check repo file " + key); - String content = OwnersDb.getRepoFile(readFiles, repoManager, KPF[1], - event.refName, repoFile, new ArrayList<>()); + String content = + OwnersDb.getRepoFile( + readFiles, repoManager, KPF[1], event.refName, repoFile, new ArrayList<>()); if (isNullOrEmpty(content)) { addVerboseMsg("cannot find file: " + key); // unchecked: including-file-path : line number : source line @@ -370,7 +372,7 @@ } else if ((email = Parser.parseEmail(line)) != null) { collectEmail(email, project, path, lineNumber); } else if ((owners = Parser.parsePerFileOwners(line)) != null) { - for (String owner: owners) { + for (String owner : owners) { if (owner.startsWith("file:")) { // Pass the whole line, not just owner, to report any syntax error., checkIncludeOrFile(project, path, lineNumber, line); @@ -386,11 +388,9 @@ } } // end of inner class Checker - /** - * Return a map from "Path to changed file" to "ObjectId of the file". - */ - private static Map<String, ObjectId> getChangedFiles( - RevCommit c, RevWalk revWalk) throws IOException { + /** Return a map from "Path to changed file" to "ObjectId of the file". */ + private static Map<String, ObjectId> getChangedFiles(RevCommit c, RevWalk revWalk) + throws IOException { final Map<String, ObjectId> content = new HashMap<>(); visitChangedEntries( c,
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/Parser.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/Parser.java index b26d1a9..faf6141 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/findowners/Parser.java +++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/Parser.java
@@ -30,18 +30,15 @@ import java.util.regex.Pattern; /** - * Parse lines in an OWNERS file and put them into an OwnersDb. - * One Parser object should be created to parse only one OWNERS file. - * It keeps repoManager, project, branch, and filePath of the OWNERS + * Parse lines in an OWNERS file and put them into an OwnersDb. One Parser object should be created + * to parse only one OWNERS file. It keeps repoManager, project, branch, and filePath of the OWNERS * file so it can find files that are included by OWNERS. * - * The usage pattern is: - * Parser parser = new Parser(repoManager, project, branch, repoFilePath); - * String content = OwnersDb.getRepoFile(readFiles, repoManager, project, - * branch, repoFilePath, logs); - * Parser.Result result = parser.parseFile(dirPath, content); + * <p>The usage pattern is: Parser parser = new Parser(repoManager, project, branch, repoFilePath); + * String content = OwnersDb.getRepoFile(readFiles, repoManager, project, branch, repoFilePath, + * logs); Parser.Result result = parser.parseFile(dirPath, content); * - * OWNERS file syntax, semantics, and examples are included in syntax.md. + * <p>OWNERS file syntax, semantics, and examples are included in syntax.md. */ class Parser { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); @@ -53,29 +50,29 @@ protected static final String COMMA = "[\\s]*,[\\s]*"; // used in unit tests // Separator for project and file paths in an include line. - private static final String COLUMN = "[\\s]*:[\\s]*"; // project:file + private static final String COLUMN = "[\\s]*:[\\s]*"; // project:file - private static final String BOL = "^[\\s]*"; // begin-of-line - private static final String EOL = "[\\s]*(#.*)?$"; // end-of-line - private static final String GLOB = "[^\\s,=]+"; // a file glob + private static final String BOL = "^[\\s]*"; // begin-of-line + private static final String EOL = "[\\s]*(#.*)?$"; // end-of-line + private static final String GLOB = "[^\\s,=]+"; // a file glob // TODO: have a more precise email address pattern. - private static final String EMAIL_OR_STAR = "([^\\s<>@,]+@[^\\s<>@#,]+|\\*)"; - private static final String EMAIL_LIST = + private static final String EMAIL_OR_STAR = "([^\\s<>@,]+@[^\\s<>@#,]+|\\*)"; + private static final String EMAIL_LIST = "(" + EMAIL_OR_STAR + "(" + COMMA + EMAIL_OR_STAR + ")*)"; // A Gerrit project name followed by a column and optional spaces. - private static final String PROJECT_NAME = "([^\\s:]+" + COLUMN + ")?"; + private static final String PROJECT_NAME = "([^\\s:]+" + COLUMN + ")?"; // A relative or absolute file path name without any column or space character. - private static final String FILE_PATH = "([^\\s:#]+)"; + private static final String FILE_PATH = "([^\\s:#]+)"; - private static final String PROJECT_AND_FILE = PROJECT_NAME + FILE_PATH; + private static final String PROJECT_AND_FILE = PROJECT_NAME + FILE_PATH; - private static final String SET_NOPARENT = "set[\\s]+noparent"; + private static final String SET_NOPARENT = "set[\\s]+noparent"; - private static final String FILE_DIRECTIVE = "file:[\\s]*" + PROJECT_AND_FILE; - private static final String INCLUDE_OR_FILE = "(file:[\\s]*|include[\\s]+)"; + private static final String FILE_DIRECTIVE = "file:[\\s]*" + PROJECT_AND_FILE; + private static final String INCLUDE_OR_FILE = "(file:[\\s]*|include[\\s]+)"; // Simple input lines with 0 or 1 sub-pattern. private static final Pattern PAT_COMMENT = Pattern.compile(BOL + EOL); @@ -144,18 +141,32 @@ } } - Parser(Map<String, String> readFiles, GitRepositoryManager repoManager, - String project, String branch, String file) { + Parser( + Map<String, String> readFiles, + GitRepositoryManager repoManager, + String project, + String branch, + String file) { init(readFiles, repoManager, project, branch, file, new ArrayList<>()); } - Parser(Map<String, String> readFiles, GitRepositoryManager repoManager, - String project, String branch, String file, List<String> logs) { + Parser( + Map<String, String> readFiles, + GitRepositoryManager repoManager, + String project, + String branch, + String file, + List<String> logs) { init(readFiles, repoManager, project, branch, file, logs); } - private void init(Map<String, String> readFiles, GitRepositoryManager repoManager, - String project, String branch, String file, List<String> logs) { + private void init( + Map<String, String> readFiles, + GitRepositoryManager repoManager, + String project, + String branch, + String file, + List<String> logs) { this.readFiles = readFiles; this.repoManager = repoManager; this.branch = branch; @@ -201,7 +212,7 @@ } else { projectName = project; // default project name } - return new String[]{keyword, projectName, m.group(3).trim()}; + return new String[] {keyword, projectName, m.group(3).trim()}; } static String removeExtraSpaces(String s) { @@ -210,11 +221,12 @@ static String[] parsePerFile(String line) { Matcher m = PAT_PER_FILE.matcher(line); - if (!m.matches() || !isGlobs(m.group(1).trim()) + if (!m.matches() + || !isGlobs(m.group(1).trim()) || !PAT_PER_FILE_OWNERS.matcher(m.group(2).trim()).matches()) { return null; } - return new String[]{removeExtraSpaces(m.group(1)), removeExtraSpaces(m.group(2))}; + return new String[] {removeExtraSpaces(m.group(1)), removeExtraSpaces(m.group(2))}; } static String[] parsePerFileOwners(String line) { @@ -266,14 +278,13 @@ } /** - * Parse given lines of an OWNERS files; return parsed Result. - * It can recursively call itself to parse included files. + * Parse given lines of an OWNERS files; return parsed Result. It can recursively call itself to + * parse included files. * - * @param dir is the directory that contains "changed files" of a CL, - * not necessarily the OWNERS or included file directory. - * "owners" found in lines control changed files in 'dir'. - * 'dir' ends with '/' or is empty when parsing an included file. - * @param lines are the source lines of the file to be parsed. + * @param dir is the directory that contains "changed files" of a CL, not necessarily the OWNERS + * or included file directory. "owners" found in lines control changed files in 'dir'. 'dir' + * ends with '/' or is empty when parsing an included file. + * @param lines are the source lines of the file to be parsed. * @return the parsed data */ Result parseFile(String dir, String[] lines) { @@ -301,8 +312,8 @@ } /** - * Parse a line in OWNERS file and add parsed info into result. - * This function should be called only by parseFile and Parser unit tests. + * Parse a line in OWNERS file and add parsed info into result. This function should be called + * only by parseFile and Parser unit tests. * * @param result a Result object to keep parsed info. * @param dir the path to OWNERS file directory. @@ -347,7 +358,7 @@ } } for (String glob : dirGlobs) { - for (String owner: ownerEmails) { + for (String owner : ownerEmails) { Util.addToMap(result.owner2paths, owner, dir + glob); } } @@ -360,11 +371,11 @@ } /** - * Find and parse an included file and append data to the 'result'. - * For an 'include' statement, parsed data is all appended to the given result parameter. - * For a 'file:' statement or directive, only owner emails are appended. - * If the project+file name is found in the stored result set, the stored result is reused. - * The inclusion is skipped if the to be included file is already on the including file stack. + * Find and parse an included file and append data to the 'result'. For an 'include' statement, + * parsed data is all appended to the given result parameter. For a 'file:' statement or + * directive, only owner emails are appended. If the project+file name is found in the stored + * result set, the stored result is reused. The inclusion is skipped if the to be included file is + * already on the including file stack. * * @param result to where the included file data should be added. * @param dir the including file's directory or glob.
diff --git a/src/test/java/com/googlesource/gerrit/plugins/findowners/ApiIT.java b/src/test/java/com/googlesource/gerrit/plugins/findowners/ApiIT.java index d8c79b8..5ca6de3 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/findowners/ApiIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/findowners/ApiIT.java
@@ -17,8 +17,8 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth8.assertThat; -import com.google.common.flogger.FluentLogger; import com.google.common.collect.Multimap; +import com.google.common.flogger.FluentLogger; import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.acceptance.RestResponse; import com.google.gerrit.acceptance.TestPlugin; @@ -108,13 +108,7 @@ Action.Parameters param = new Action.Parameters(); Action action = new Action( - pluginConfig, - null, - changeDataFactory, - accountCache, - emails, - repoManager, - projectCache); + pluginConfig, null, changeDataFactory, accountCache, emails, repoManager, projectCache); Response<RestResult> response = action.apply(cr, param); RestResult result = response.value(); verifyRestResult(result, 1, 1, changeInfo._number, false);
diff --git a/src/test/java/com/googlesource/gerrit/plugins/findowners/FindOwners.java b/src/test/java/com/googlesource/gerrit/plugins/findowners/FindOwners.java index 10e327b..acef31f 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/findowners/FindOwners.java +++ b/src/test/java/com/googlesource/gerrit/plugins/findowners/FindOwners.java
@@ -100,8 +100,8 @@ gApi.changes().id(change.getChangeId()).current().submit(new SubmitInput()); } - protected PushOneCommit.Result addFile( - String subject, String file, String content) throws Exception { + protected PushOneCommit.Result addFile(String subject, String file, String content) + throws Exception { PushOneCommit.Result c = createChange(subject, file, content); approveSubmit(c); return c; @@ -155,15 +155,25 @@ protected int checkApproval(PushOneCommit.Result r) throws Exception { Project.NameKey project = r.getChange().project(); Cache cache = getCache().init(0, 0); - OwnersDb db = cache.get(true, projectCache.get(project), accountCache, emails, - repoManager, pluginConfig, r.getChange(), 1); + OwnersDb db = + cache.get( + true, + projectCache.get(project), + accountCache, + emails, + repoManager, + pluginConfig, + r.getChange(), + 1); Checker c = new Checker(repoManager, pluginConfig, null, r.getChange(), 1); return c.findApproval(accountCache, db); } // Remove '"' and space; replace '\n' with ' '; ignore "owner_revision" and "HostName:*". protected static String filteredJson(String json) { - return json.replaceAll("[\" ]*", "").replace('\n', ' ').replaceAll("owner_revision:[^ ]* ", "") + return json.replaceAll("[\" ]*", "") + .replace('\n', ' ') + .replaceAll("owner_revision:[^ ]* ", "") .replaceAll("HostName:[^ ]*, ", ""); }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/findowners/IncludeIT.java b/src/test/java/com/googlesource/gerrit/plugins/findowners/IncludeIT.java index 4c376e1..f5829a2 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/findowners/IncludeIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/findowners/IncludeIT.java
@@ -35,14 +35,17 @@ PushOneCommit.Result c2 = addFile("1", "t.c", "##"); // Submitted c2 still finds no owners before c1 is submitted. assertThat(getOwnersResponse(c2)).contains("owners:[], files:[ t.c ]"); - PushOneCommit.Result c1 = addFile("2", "OWNERS", - "x@x\na@a\ninclude P1/P2 : f1\ninclude ./d1/d2/../../f2\n"); + PushOneCommit.Result c1 = + addFile("2", "OWNERS", "x@x\na@a\ninclude P1/P2 : f1\ninclude ./d1/d2/../../f2\n"); // Now c2 should find owners, but include directives find no repository or file. String ownersAX = "owners:[ " + ownerJson("a@a") + ", " + ownerJson("x@x") + " ]"; String path2owners = "path2owners:{ ./:[ a@a, x@x ] }"; String owner2paths = "owner2paths:{ a@a:[ ./ ], x@x:[ ./ ] }"; String projectName = project.get(); - String expectedInLog = "project:" + projectName + ", " + String expectedInLog = + "project:" + + projectName + + ", " + "ownersFileName:OWNERS, " + "getBranchId:refs/heads/master(FOUND), " + "findOwnersFileFor:./t.c, " @@ -52,8 +55,12 @@ + "getRepoFile:P1/P2:refs/heads/master:f1, " + "getRepoFileException:repositorynotfound:P1/P2, " // repository not found + "parseLine:include:(), " // missing file is treated as empty - + "parseLine:include:" + projectName + ":./d1/d2/../../f2, " - + "getRepoFile:" + projectName + ":refs/heads/master:f2, " + + "parseLine:include:" + + projectName + + ":./d1/d2/../../f2, " + + "getRepoFile:" + + projectName + + ":refs/heads/master:f2, " + "getFile:f2(NOTFOUND), " // same repository but f2 is missing + "parseLine:include:(), " // missing file is treated as empty + "countNumOwners, " @@ -82,8 +89,8 @@ addFile("c0", "f2", "g1@g\ng2@g\n"); // c2 and c1 are both submitted before existence of OWNERS. PushOneCommit.Result c2 = addFile("c2", "t.c", "##"); - PushOneCommit.Result c1 = addFile("c1", "OWNERS", - "x@x\na@a\ninclude P1/P2 : f1\ninclude ./d1/d2/../../f2\n"); + PushOneCommit.Result c1 = + addFile("c1", "OWNERS", "x@x\na@a\ninclude P1/P2 : f1\ninclude ./d1/d2/../../f2\n"); String ownerA = ownerJson("a@a"); String ownerX = ownerJson("x@x"); String ownerG1 = ownerJson("g1@g"); @@ -93,7 +100,10 @@ String path2owners = "path2owners:{ ./:[ a@a, g1@g, g2@g, x@x ] }"; String owner2paths = "owner2paths:{ a@a:[ ./ ], g1@g:[ ./ ], g2@g:[ ./ ], x@x:[ ./ ] }"; String projectName = project.get(); - String expectedInLog = "project:" + projectName + ", " + String expectedInLog = + "project:" + + projectName + + ", " + "ownersFileName:OWNERS, " + "getBranchId:refs/heads/master(FOUND), " + "findOwnersFileFor:./t.c, " @@ -103,8 +113,12 @@ + "getRepoFile:P1/P2:refs/heads/master:f1, " + "getRepoFileException:repositorynotfound:P1/P2, " + "parseLine:include:(), " // P1/P2 is still not found - + "parseLine:include:" + projectName + ":./d1/d2/../../f2, " - + "getRepoFile:" + projectName + ":refs/heads/master:f2, " + + "parseLine:include:" + + projectName + + ":./d1/d2/../../f2, " + + "getRepoFile:" + + projectName + + ":refs/heads/master:f2, " + "getFile:f2:(...), " // f2 is included + "countNumOwners, " + "findOwners, " @@ -145,8 +159,17 @@ String ownerD2 = ownerJson("d2d2@g"); String ownerF2 = ownerJson("d2f2@g"); String ownerX = ownerJson("x@g", 0, 1, 0); - assertThat(getOwnersResponse(c1)).contains("owners:[ " + ownerD2 + ", " - + ownerF2 + ", " + ownerD3 + ", " + ownerX + " ], files:[ d3/t.c ]"); + assertThat(getOwnersResponse(c1)) + .contains( + "owners:[ " + + ownerD2 + + ", " + + ownerF2 + + ", " + + ownerD3 + + ", " + + ownerX + + " ], files:[ d3/t.c ]"); } @Test @@ -154,24 +177,16 @@ // Test difference between include and file statements. // The file statement skips "set noparent" and "per-file" statements. addFile("d1", "d1/OWNERS", "d1@g\n"); - addFile("d1/d1", "d1/d1/OWNERS", - "per-file *.c=d1d1p@g\nd1d1@g\nfile: d1/OWNERS\n"); - addFile("d1/d1/d1", "d1/d1/d1/OWNERS", - "set noparent\nper-file *.c=d1d1d1p@g\nd1d1d1@g\n"); - addFile("d1/d2", "d1/d2/OWNERS", - "per-file *.c=d1d2p@g\nd1d2@g\ninclude d1/OWNERS\n"); - addFile("d1/d2/d1", "d1/d2/d1/OWNERS", - "set noparent\nper-file *.c=d1d2d1p@g\nd1d2d1@g\n"); + addFile("d1/d1", "d1/d1/OWNERS", "per-file *.c=d1d1p@g\nd1d1@g\nfile: d1/OWNERS\n"); + addFile("d1/d1/d1", "d1/d1/d1/OWNERS", "set noparent\nper-file *.c=d1d1d1p@g\nd1d1d1@g\n"); + addFile("d1/d2", "d1/d2/OWNERS", "per-file *.c=d1d2p@g\nd1d2@g\ninclude d1/OWNERS\n"); + addFile("d1/d2/d1", "d1/d2/d1/OWNERS", "set noparent\nper-file *.c=d1d2d1p@g\nd1d2d1@g\n"); addFile("d2", "d2/OWNERS", "d2@g\n"); - addFile("d2/d1", "d2/d1/OWNERS", - "per-file *.c=d2d1p@g\nd2d1@g\nfile: ./d1/OWNERS\n"); - addFile("d2/d1/d1", "d2/d1/d1/OWNERS", - "set noparent\nper-file *.c=d2d1d1p@g\nd2d1d1@g\n"); - addFile("d2/d2", "d2/d2/OWNERS", - "per-file *.c=d2d2p@g\nd2d2@g\ninclude ./d1/OWNERS\n"); - addFile("d2/d2/d1", "d2/d2/d1/OWNERS", - "set noparent\nper-file *.c=d2d2d1p@g\nd2d2d1@g\n"); + addFile("d2/d1", "d2/d1/OWNERS", "per-file *.c=d2d1p@g\nd2d1@g\nfile: ./d1/OWNERS\n"); + addFile("d2/d1/d1", "d2/d1/d1/OWNERS", "set noparent\nper-file *.c=d2d1d1p@g\nd2d1d1@g\n"); + addFile("d2/d2", "d2/d2/OWNERS", "per-file *.c=d2d2p@g\nd2d2@g\ninclude ./d1/OWNERS\n"); + addFile("d2/d2/d1", "d2/d2/d1/OWNERS", "set noparent\nper-file *.c=d2d2d1p@g\nd2d2d1@g\n"); addFile("d3", "d3/OWNERS", "d3@g\n"); addFile("d3/d1/d1", "d3/d1/d1/OWNERS", "d3d1d1@g\nfile: ../../../d1/d1/OWNERS\n"); @@ -184,17 +199,15 @@ PushOneCommit.Result c22 = createChange("c22", "d3/d2/d2/t.c", "test"); // file and file - String owners11 = "file2owners:{ ./d3/d1/d1/t.c:" - + "[ d1d1@g, d1d1d1@g, d3@g, d3d1d1@g ] }"; + String owners11 = "file2owners:{ ./d3/d1/d1/t.c:" + "[ d1d1@g, d1d1d1@g, d3@g, d3d1d1@g ] }"; // file and include - String owners12 = "file2owners:{ ./d3/d1/d2/t.c:" - + "[ d1d2@g, d1d2d1@g, d3@g, d3d1d2@g ] }"; + String owners12 = "file2owners:{ ./d3/d1/d2/t.c:" + "[ d1d2@g, d1d2d1@g, d3@g, d3d1d2@g ] }"; // include and file - String owners21 = "file2owners:{ ./d3/d2/d1/t.c:" - + "[ d2d1@g, d2d1d1@g, d2d1p@g, d3@g, d3d2d1@g ] }"; + String owners21 = + "file2owners:{ ./d3/d2/d1/t.c:" + "[ d2d1@g, d2d1d1@g, d2d1p@g, d3@g, d3d2d1@g ] }"; // include and include - String owners22 = "file2owners:{ ./d3/d2/d2/t.c:" - + "[ d2d2@g, d2d2d1@g, d2d2d1p@g, d2d2p@g, d3d2d2@g ] }"; + String owners22 = + "file2owners:{ ./d3/d2/d2/t.c:" + "[ d2d2@g, d2d2d1@g, d2d2d1p@g, d2d2p@g, d3d2d2@g ] }"; assertThat(getOwnersDebugResponse(c11)).contains(owners11); assertThat(getOwnersDebugResponse(c12)).contains(owners12); @@ -233,25 +246,46 @@ PushOneCommit.Result c = createChange("6", "t.c", "#\n"); String response = getOwnersDebugResponse(c); String projectName = project.get(); - String expectedInLog = "project:" + projectName + ", " + String expectedInLog = + "project:" + + projectName + + ", " + "ownersFileName:OWNERS, " + "getBranchId:refs/heads/master(FOUND), " + "findOwnersFileFor:./t.c, " + "findOwnersFileIn:., " + "getFile:OWNERS:(...), " - + "parseLine:include:" + projectName + ":./d1/../f1, " - + "getRepoFile:" + projectName + ":refs/heads/master:f1, " + + "parseLine:include:" + + projectName + + ":./d1/../f1, " + + "getRepoFile:" + + projectName + + ":refs/heads/master:f1, " + "getFile:f1:(...), " - + "parseLine:include:" + projectName + ":./f2, " - + "getRepoFile:" + projectName + ":refs/heads/master:f2, " + + "parseLine:include:" + + projectName + + ":./f2, " + + "getRepoFile:" + + projectName + + ":refs/heads/master:f2, " + "getFile:f2:(...), " - + "parseLine:include:" + projectName + ":d1/../f3, " - + "getRepoFile:" + projectName + ":refs/heads/master:f3, " + + "parseLine:include:" + + projectName + + ":d1/../f3, " + + "getRepoFile:" + + projectName + + ":refs/heads/master:f3, " + "getFile:f3:(...), " - + "parseLine:include:" + projectName + ":/f4, " - + "getRepoFile:" + projectName + ":refs/heads/master:f4, " + + "parseLine:include:" + + projectName + + ":/f4, " + + "getRepoFile:" + + projectName + + ":refs/heads/master:f4, " + "getFile:f4:(...), " - + "parseLine:errorRecursion:include:" + projectName + ":d2/../f2, " + + "parseLine:errorRecursion:include:" + + projectName + + ":d2/../f2, " + "countNumOwners, " + "findOwners, " + "checkFile:./t.c, " @@ -259,8 +293,8 @@ + "addOwnerWeightsIn:./ " + "] "; assertThat(response).contains("path2owners:{ ./:[ f1@g, f2@g, f3@g, f4@g, x@g ] }"); - assertThat(response).contains( - "owner2paths:{ f1@g:[ ./ ], f2@g:[ ./ ], f3@g:[ ./ ], f4@g:[ ./ ], x@g:[ ./ ] }"); + assertThat(response) + .contains("owner2paths:{ f1@g:[ ./ ], f2@g:[ ./ ], f3@g:[ ./ ], f4@g:[ ./ ], x@g:[ ./ ] }"); assertThat(response).contains(expectedInLog); } @@ -275,55 +309,97 @@ addFile("3", "d2/d3/f3", "f3@g\ninclude /d0/f0\n"); addFile("4", "d4/f4", "f4@g\ninclude ../d2/f2\n"); addFile("5", "d4/d5/f5", "f5@g\ninclude /d2/f2\ninclude ../f4\n"); - PushOneCommit.Result c = addFile("6", "d6/OWNERS", - "f6@g\ninclude /d0/f0\ninclude ../d1/d2/f1\n" - + "include ../d2/f2\ninclude /d2/d3/f3\ninclude /d2/../d4/d5/f5\ninclude /d4/f4\n"); + PushOneCommit.Result c = + addFile( + "6", + "d6/OWNERS", + "f6@g\ninclude /d0/f0\ninclude ../d1/d2/f1\n" + + "include ../d2/f2\ninclude /d2/d3/f3\ninclude /d2/../d4/d5/f5\ninclude /d4/f4\n"); String result = getOwnersDebugResponse(c); assertThat(result).contains("{ ./d6/OWNERS:[ f0@g, f1@g, f2@g, f3@g, f4@g, f5@g, f6@g ] }"); String projectName = project.get(); String skipLog = "parseLine:useSaved:include:" + projectName + ":"; - for (String path : new String[]{"../../d0/f0", "../d0/f0", "../d2/f2", "/d2/f2", "/d4/f4"}) { + for (String path : new String[] {"../../d0/f0", "../d0/f0", "../d2/f2", "/d2/f2", "/d4/f4"}) { assertThat(result).contains(skipLog + path); } - String expectedInLog = "project:" + projectName + ", " - + "ownersFileName:OWNERS, " - + "getBranchId:refs/heads/master(FOUND), " - + "findOwnersFileFor:./d6/OWNERS, " - + "findOwnersFileIn:./d6, " - + "getFile:d6/OWNERS:(...), " - + "parseLine:include:" + projectName + ":/d0/f0, " - + "getRepoFile:" + projectName + ":refs/heads/master:d0/f0, " - + "getFile:d0/f0:(...), " - + "parseLine:include:" + projectName + ":../d1/d2/f1, " - + "getRepoFile:" + projectName + ":refs/heads/master:d1/d2/f1, " - + "getFile:d1/d2/f1:(...), " - + "parseLine:useSaved:include:" + projectName + ":../../d0/f0, " - + "parseLine:include:" + projectName + ":../d2/f2, " - + "getRepoFile:" + projectName + ":refs/heads/master:d2/f2, " - + "getFile:d2/f2:(...), " - + "parseLine:useSaved:include:" + projectName + ":../d0/f0, " - + "parseLine:include:" + projectName + ":/d2/d3/f3, " - + "getRepoFile:" + projectName + ":refs/heads/master:d2/d3/f3, " - + "getFile:d2/d3/f3:(...), " - + "parseLine:useSaved:include:" + projectName + ":/d0/f0, " - + "parseLine:include:" + projectName + ":/d2/../d4/d5/f5, " - + "getRepoFile:" + projectName + ":refs/heads/master:d4/d5/f5, " - + "getFile:d4/d5/f5:(...), " - + "parseLine:useSaved:include:" + projectName + ":/d2/f2, " - + "parseLine:include:" + projectName + ":../f4, " - + "getRepoFile:" + projectName + ":refs/heads/master:d4/f4, " - + "getFile:d4/f4:(...), " - + "parseLine:useSaved:include:" + projectName + ":../d2/f2, " - + "parseLine:useSaved:include:" + projectName + ":/d4/f4, " - + "findOwnersFileIn:., " - + "getFile:OWNERS(NOTFOUND), " - + "countNumOwners, " - + "findOwners, " - + "checkFile:./d6/OWNERS, " - + "checkDir:./d6, " - + "checkDir:., " - + "addOwnerWeightsIn:./d6/ " - + "] "; + String expectedInLog = + "project:" + + projectName + + ", " + + "ownersFileName:OWNERS, " + + "getBranchId:refs/heads/master(FOUND), " + + "findOwnersFileFor:./d6/OWNERS, " + + "findOwnersFileIn:./d6, " + + "getFile:d6/OWNERS:(...), " + + "parseLine:include:" + + projectName + + ":/d0/f0, " + + "getRepoFile:" + + projectName + + ":refs/heads/master:d0/f0, " + + "getFile:d0/f0:(...), " + + "parseLine:include:" + + projectName + + ":../d1/d2/f1, " + + "getRepoFile:" + + projectName + + ":refs/heads/master:d1/d2/f1, " + + "getFile:d1/d2/f1:(...), " + + "parseLine:useSaved:include:" + + projectName + + ":../../d0/f0, " + + "parseLine:include:" + + projectName + + ":../d2/f2, " + + "getRepoFile:" + + projectName + + ":refs/heads/master:d2/f2, " + + "getFile:d2/f2:(...), " + + "parseLine:useSaved:include:" + + projectName + + ":../d0/f0, " + + "parseLine:include:" + + projectName + + ":/d2/d3/f3, " + + "getRepoFile:" + + projectName + + ":refs/heads/master:d2/d3/f3, " + + "getFile:d2/d3/f3:(...), " + + "parseLine:useSaved:include:" + + projectName + + ":/d0/f0, " + + "parseLine:include:" + + projectName + + ":/d2/../d4/d5/f5, " + + "getRepoFile:" + + projectName + + ":refs/heads/master:d4/d5/f5, " + + "getFile:d4/d5/f5:(...), " + + "parseLine:useSaved:include:" + + projectName + + ":/d2/f2, " + + "parseLine:include:" + + projectName + + ":../f4, " + + "getRepoFile:" + + projectName + + ":refs/heads/master:d4/f4, " + + "getFile:d4/f4:(...), " + + "parseLine:useSaved:include:" + + projectName + + ":../d2/f2, " + + "parseLine:useSaved:include:" + + projectName + + ":/d4/f4, " + + "findOwnersFileIn:., " + + "getFile:OWNERS(NOTFOUND), " + + "countNumOwners, " + + "findOwners, " + + "checkFile:./d6/OWNERS, " + + "checkDir:./d6, " + + "checkDir:., " + + "addOwnerWeightsIn:./d6/ " + + "] "; assertThat(result).contains(expectedInLog); } @@ -348,9 +424,22 @@ PushOneCommit.Result c1 = createChange("c1", "d2/t.c", "Hello!"); // included: pA:d2/OWNERS, pA:d2/../f1, pA:d1/f1, pB:d2/f2, pB:d2/../f1, pB:./d1/f1 // inherited: pA:OWNERS - String owners = "owners:[ " + ownerJson("pAd1f1@g") + ", " + ownerJson("pAd2@g") + ", " - + ownerJson("pAf1@g") + ", " + ownerJson("pBd1f1@g") + ", " + ownerJson("pBd2f2@g") - + ", " + ownerJson("pBf1@g") + ", " + ownerJson("pA@g", 0, 1, 0) + " ]"; + String owners = + "owners:[ " + + ownerJson("pAd1f1@g") + + ", " + + ownerJson("pAd2@g") + + ", " + + ownerJson("pAf1@g") + + ", " + + ownerJson("pBd1f1@g") + + ", " + + ownerJson("pBd2f2@g") + + ", " + + ownerJson("pBf1@g") + + ", " + + ownerJson("pA@g", 0, 1, 0) + + " ]"; assertThat(getOwnersResponse(c1)).contains(owners); } }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/findowners/InheritanceIT.java b/src/test/java/com/googlesource/gerrit/plugins/findowners/InheritanceIT.java index e3bc749..9adb4f9 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/findowners/InheritanceIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/findowners/InheritanceIT.java
@@ -31,14 +31,15 @@ @Test public void includePerFileNoParentTest() throws Exception { // Test included file with per-file and set noparent, which affects the including file. - PushOneCommit.Result c1 = addFile("1", "d1/d1/OWNERS", - "d1d1@g\nper-file OW* = set noparent\nper-file OWNERS=d1d1o@g\n"); - PushOneCommit.Result c2 = addFile("2", "d1/OWNERS", - "d1@g\nper-file OWNERS=d1o@g\nper-file * = set noparent\n"); - PushOneCommit.Result c3 = addFile( "3", "d2/d1/OWNERS", - "per-file O*S=d2d1o@g\nd2d1@g\ninclude ../../d1/d1/OWNERS\n"); - PushOneCommit.Result c4 = addFile("4", - "d2/OWNERS", "d2@g\nper-file OWNERS=d2o@g\nper-file *S=set noparent \n"); + PushOneCommit.Result c1 = + addFile( + "1", "d1/d1/OWNERS", "d1d1@g\nper-file OW* = set noparent\nper-file OWNERS=d1d1o@g\n"); + PushOneCommit.Result c2 = + addFile("2", "d1/OWNERS", "d1@g\nper-file OWNERS=d1o@g\nper-file * = set noparent\n"); + PushOneCommit.Result c3 = + addFile("3", "d2/d1/OWNERS", "per-file O*S=d2d1o@g\nd2d1@g\ninclude ../../d1/d1/OWNERS\n"); + PushOneCommit.Result c4 = + addFile("4", "d2/OWNERS", "d2@g\nper-file OWNERS=d2o@g\nper-file *S=set noparent \n"); // Files that match per-file globs with set noparent do not inherit global default owners. // But include directive can include more per-file owners as in c3. assertThat(getOwnersResponse(c1)).contains("{ ./d1/d1/OWNERS:[ d1d1o@g ] }"); @@ -93,20 +94,19 @@ .contains("owners:[ " + ownerY + ", " + ownerX010 + " ], files:[ d2/t.c ]"); // Add "d2/d1/t.c" file, which is owned by ./d2 and root owners. PushOneCommit.Result c5 = createChange("c5", "d2/d1/t.c", "Hello!"); - assertThat(getOwnersResponse(c5)).contains( - "owners:[ " + ownerY + ", " + ownerX010 + " ], files:[ d2/d1/t.c ]"); + assertThat(getOwnersResponse(c5)) + .contains("owners:[ " + ownerY + ", " + ownerX010 + " ], files:[ d2/d1/t.c ]"); // Add "d3/t.c" file, which is owned only by ./d3 owners due to "set noparent". PushOneCommit.Result c6 = createChange("c6", "d3/t.c", "Hello!"); String ownerB = ownerJson("b@b"); assertThat(getOwnersResponse(c6)).contains("owners:[ " + ownerB + " ], files:[ d3/t.c ]"); // Add "d3/d1/t.c" file, which is owned only by ./d3 owners due to "set noparent". PushOneCommit.Result c7 = createChange("c7", "d3/d1/t.c", "Hello!"); - assertThat(getOwnersResponse(c7)).contains( - "owners:[ " + ownerB + " ], files:[ d3/d1/t.c ]"); + assertThat(getOwnersResponse(c7)).contains("owners:[ " + ownerB + " ], files:[ d3/d1/t.c ]"); // Add "d4/t.c" file, which is owned by ./d4 and ./d2 owners, but not root owners. PushOneCommit.Result c8 = createChange("c8", "d4/t.c", "Hello!"); String ownerZ = ownerJson("z@z"); - assertThat(getOwnersResponse(c8)).contains( - "owners:[ " + ownerY + ", " + ownerZ + ", " + ownerX010 + " ], files:[ d4/t.c ]"); + assertThat(getOwnersResponse(c8)) + .contains("owners:[ " + ownerY + ", " + ownerZ + ", " + ownerX010 + " ], files:[ d4/t.c ]"); } }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/findowners/OwnersValidatorIT.java b/src/test/java/com/googlesource/gerrit/plugins/findowners/OwnersValidatorIT.java index 8587b35..c88283f 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/findowners/OwnersValidatorIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/findowners/OwnersValidatorIT.java
@@ -59,8 +59,6 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; -import org.junit.rules.TestWatcher; -import org.junit.runner.Description; /** Test OwnersValidator, which checks syntax of changed OWNERS files. */ @TestPlugin(name = "find-owners", sysModule = "com.googlesource.gerrit.plugins.findowners.Module") @@ -71,8 +69,11 @@ @Inject private ProjectOperations projectOperations; protected Project.NameKey newProject(String name) { - return projectOperations.newProject().parent(new Project.NameKey("All-Projects")). - name(name).create(); + return projectOperations + .newProject() + .parent(new Project.NameKey("All-Projects")) + .name(name) + .create(); } private void switchProject(Project.NameKey p) throws Exception { @@ -80,8 +81,7 @@ testRepo = cloneProject(project); } - private void addProjectFile( - String project, String file, String content) throws Exception { + private void addProjectFile(String project, String file, String content) throws Exception { switchProject(newProject(project)); PushOneCommit.Result c = createChange("c", file, content); approve(c.getChangeId()); @@ -168,24 +168,26 @@ assertThat(validate(event, true, ENABLED_CONFIG)).isEmpty(); } - private static final String[] INCLUDE_STMTS = new String[]{ - " include p1/p2 : /d1/owners", - "include p2/p1://d1/owners #comment", - "include ../d2/owners", - " per-file *.java = file: //d2/OWNERS.java #", - "per-file *.cpp,*cc=file:p1/p2:/c++owners", - " file: p1/p2 : /d1/owners ", // repeated - "file: p3/p2://d1/owners #comment", - "file: ../d2/owners", // repeated - "file: //OWNERS", // recursive inclusion - "file:///OWNERS.android"}; + private static final String[] INCLUDE_STMTS = + new String[] { + " include p1/p2 : /d1/owners", + "include p2/p1://d1/owners #comment", + "include ../d2/owners", + " per-file *.java = file: //d2/OWNERS.java #", + "per-file *.cpp,*cc=file:p1/p2:/c++owners", + " file: p1/p2 : /d1/owners ", // repeated + "file: p3/p2://d1/owners #comment", + "file: ../d2/owners", // repeated + "file: //OWNERS", // recursive inclusion + "file:///OWNERS.android" + }; private static final ImmutableSet<String> SKIP_INCLUDE_STMTS = ImmutableSet.of(" file: p1/p2 : /d1/owners ", "file: ../d2/owners", "file: //OWNERS"); private static String allIncludeStatements() { String statement = ""; - for (String s: INCLUDE_STMTS) { + for (String s : INCLUDE_STMTS) { statement += s + "\n"; } return statement; @@ -195,8 +197,8 @@ Set<String> msgs = new HashSet<>(); for (int i = 0; i < INCLUDE_STMTS.length; i++) { if (!SKIP_INCLUDE_STMTS.contains(INCLUDE_STMTS[i])) { - msgs.add("MSG: unchecked: OWNERS:" + (i + 1) - + ": " + Parser.getIncludeOrFile(INCLUDE_STMTS[i])); + msgs.add( + "MSG: unchecked: OWNERS:" + (i + 1) + ": " + Parser.getIncludeOrFile(INCLUDE_STMTS[i])); } } return msgs; @@ -223,24 +225,24 @@ msgs.add("MSG: owner: u1@g.com"); msgs.add("MSG: owner: u2.m@g.com"); msgs.add("MSG: owner: user1@google.com"); - String[] missing = new String[]{ - "p1/p2:OWNERS.android", - "p1/p2:c++owners", - "p1/p2:d1/owners", - "p1/p2:d2/OWNERS.java", - "p1/p2:d2/owners", - "p2/p1:d1/owners", - "p3/p2:d1/owners", - }; + String[] missing = + new String[] { + "p1/p2:OWNERS.android", + "p1/p2:c++owners", + "p1/p2:d1/owners", + "p1/p2:d2/OWNERS.java", + "p1/p2:d2/owners", + "p2/p1:d1/owners", + "p3/p2:d1/owners", + }; for (String s : missing) { msgs.add("MSG: cannot find file: " + s); msgs.add("MSG: check repo file " + s); } - String[] skips = new String[]{ - "p1/p2:OWNERS", - "p1/p2:d1/owners", - "p1/p2:d2/owners", - }; + String[] skips = + new String[] { + "p1/p2:OWNERS", "p1/p2:d1/owners", "p1/p2:d2/owners", + }; for (String s : skips) { msgs.add("MSG: skip repeated include of " + s); } @@ -323,13 +325,15 @@ addProjectFile("p2", "d2/owners", "x@g.com\nerr\ninclude ../d2/owners\n"); ImmutableMap<String, String> files = ImmutableMap.of( - "d1/" + OWNERS, "include ../d2/owners\ninclude /d2/owners\n" - + "include p1:/d2/owners\ninclude p2:/d2/owners\n"); - ImmutableSet<String> expected = ImmutableSet.of( - "ERROR: unknown: x@g.com at p2:d2/owners:1", - "ERROR: syntax: p2:d2/owners:2: err", - "ERROR: syntax: d2/owners:1: wrong", - "ERROR: syntax: d2/owners:2: xyz"); + "d1/" + OWNERS, + "include ../d2/owners\ninclude /d2/owners\n" + + "include p1:/d2/owners\ninclude p2:/d2/owners\n"); + ImmutableSet<String> expected = + ImmutableSet.of( + "ERROR: unknown: x@g.com at p2:d2/owners:1", + "ERROR: syntax: p2:d2/owners:2: err", + "ERROR: syntax: d2/owners:1: wrong", + "ERROR: syntax: d2/owners:2: xyz"); CommitReceivedEvent event = makeCommitEvent("p1", "T", files); assertThat(validate(event, false, ENABLED_CONFIG)).containsExactlyElementsIn(expected); }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/findowners/ParserTest.java b/src/test/java/com/googlesource/gerrit/plugins/findowners/ParserTest.java index 6998277..fee0569 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/findowners/ParserTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/findowners/ParserTest.java
@@ -174,10 +174,16 @@ @Test public void perFileGoodDirectiveTest() { String[] directives = { - "abc@google.com#comment", " *# comment", " xyz@gmail.com # comment", - "a@g.com , xyz@gmail.com , * # comment", "*,*#comment", " a@b,c@d ", - " set noparent ", "\tset\t\tnoparent\t", - "file://java.owners", " file: p1/p2 : /OWNERS " + "abc@google.com#comment", + " *# comment", + " xyz@gmail.com # comment", + "a@g.com , xyz@gmail.com , * # comment", + "*,*#comment", + " a@b,c@d ", + " set noparent ", + "\tset\t\tnoparent\t", + "file://java.owners", + " file: p1/p2 : /OWNERS " }; String[] globsList = {"*", "*,*.c", " *test*.java , *.cc, *.cpp ", "*.bp,*.mk ,A* "}; for (String directive : directives) { @@ -204,7 +210,7 @@ assertThat(owners).hasLength(1); } else { String[] paths = result.owner2paths.get(e).toArray(new String[1]); - assertThat(paths).hasLength(globList.length); // should not work for "set noparent" + assertThat(paths).hasLength(globList.length); // should not work for "set noparent" Arrays.sort(paths); for (int g = 0; g < globList.length; g++) { assertThat(paths[g]).isEqualTo(mockedTestDir() + globList[g]); @@ -218,9 +224,15 @@ @Test public void perFileBadDirectiveTest() { String[] directives = { - " ** ", "a b@c .co", "a@b@c #com", "a.<b>@zc#", - " , a@b ", "a@b, , c@d #", "a@b, set noparent", - "a@b, file://java.owners", "*,file:OWNERS" + " ** ", + "a b@c .co", + "a@b@c #com", + "a.<b>@zc#", + " , a@b ", + "a@b, , c@d #", + "a@b, set noparent", + "a@b, file://java.owners", + "*,file:OWNERS" }; for (String directive : directives) { String line = "per-file *test*.c=" + directive; @@ -244,7 +256,8 @@ testOneIncludeOrFileLine(project, line, "file", projectName, filePath); } - private void testOneIncludeLine(String project, String line, String projectName, String filePath) { + private void testOneIncludeLine( + String project, String line, String projectName, String filePath) { testOneIncludeOrFileLine(project, line, "include", projectName, filePath); } @@ -260,24 +273,30 @@ @Test public void getIncludeOrFileTest() { - String[] line = new String[] { - "", "wrong input", "INCLUDE X", - "include //f2.txt # ", - " include P1/P2: ../f1 # ", - " file://f3 # ", - "file: P1:f3", - " per-file *.c,file.c = file: /OWNERS # ", - "per-file *=file:P1/P2: /O# ", - }; - String[] expected = new String[] { - "", "", "", - "include //f2.txt", - "include P1/P2:../f1", - "file://f3", - "file:P1:f3", - "file:/OWNERS", - "file:P1/P2:/O", - }; + String[] line = + new String[] { + "", + "wrong input", + "INCLUDE X", + "include //f2.txt # ", + " include P1/P2: ../f1 # ", + " file://f3 # ", + "file: P1:f3", + " per-file *.c,file.c = file: /OWNERS # ", + "per-file *=file:P1/P2: /O# ", + }; + String[] expected = + new String[] { + "", + "", + "", + "include //f2.txt", + "include P1/P2:../f1", + "file://f3", + "file:P1:f3", + "file:/OWNERS", + "file:P1/P2:/O", + }; for (int i = 0; i < line.length; i++) { assertThat(Parser.getIncludeOrFile(line[i])).isEqualTo(expected[i]); }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/findowners/PerFileIT.java b/src/test/java/com/googlesource/gerrit/plugins/findowners/PerFileIT.java index 3b5e076..26fc914 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/findowners/PerFileIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/findowners/PerFileIT.java
@@ -36,7 +36,7 @@ String ownerA = ownerJson("a@a"); String ownerB = ownerJson("b@b"); String ownerC = ownerJson("c@c"); - String ownerABC = "owners:[ " +ownerA + ", " + ownerB + ", " + ownerC; + String ownerABC = "owners:[ " + ownerA + ", " + ownerB + ", " + ownerC; String ownerX = ownerJson("x@x"); assertThat(getOwnersResponse(c2)).contains(ownerABC + ", " + ownerX + " ], files:[ t.c ]"); // Add "t.txt" file, which has only global default owners. @@ -66,11 +66,11 @@ PushOneCommit.Result c3 = addFile("3", "d2/d1/OWNERS", "d2d1@g\ninclude ../../d1/d1/OWNERS\n"); PushOneCommit.Result c4 = addFile("4", "d2/OWNERS", "d2@g\nper-file OWNERS=d2o@g"); // Files that match per-file globs now inherit global default owners. - assertThat(getOwnersResponse(c1)).contains( - "{ ./d1/d1/OWNERS:[ d1@g, d1d1@g, d1d1o@g, d1o@g ] }"); + assertThat(getOwnersResponse(c1)) + .contains("{ ./d1/d1/OWNERS:[ d1@g, d1d1@g, d1d1o@g, d1o@g ] }"); assertThat(getOwnersResponse(c2)).contains("{ ./d1/OWNERS:[ d1@g, d1o@g ] }"); - assertThat(getOwnersResponse(c3)).contains( - "{ ./d2/d1/OWNERS:[ d1d1@g, d1d1o@g, d2@g, d2d1@g, d2o@g ] }"); + assertThat(getOwnersResponse(c3)) + .contains("{ ./d2/d1/OWNERS:[ d1d1@g, d1d1o@g, d2@g, d2d1@g, d2o@g ] }"); assertThat(getOwnersResponse(c4)).contains("{ ./d2/OWNERS:[ d2@g, d2o@g ] }"); } }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/findowners/UtilTest.java b/src/test/java/com/googlesource/gerrit/plugins/findowners/UtilTest.java index 1f846fe..0540885 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/findowners/UtilTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/findowners/UtilTest.java
@@ -155,12 +155,14 @@ @Test public void normalizedDirFilePathTest() throws IOException { - String[] dirs = {"", "", ".", "./d1/..", "/d1/d2/../..", - "d1", "d2/../d1", "/d1", "/d1", "/d1"}; - String[] files = {"f0.c", "./f1.c", "./f2", "/f3", "f4", - "f5", "f6", "f7", "f8", "../d5/../d1/f9"}; - String[] paths = {"./f0.c", "./f1.c", "./f2", "./f3", "./f4", - "./d1/f5", "./d1/f6", "./d1/f7", "./d1/f8", "./d1/f9"}; + String[] dirs = {"", "", ".", "./d1/..", "/d1/d2/../..", "d1", "d2/../d1", "/d1", "/d1", "/d1"}; + String[] files = { + "f0.c", "./f1.c", "./f2", "/f3", "f4", "f5", "f6", "f7", "f8", "../d5/../d1/f9" + }; + String[] paths = { + "./f0.c", "./f1.c", "./f2", "./f3", "./f4", "./d1/f5", "./d1/f6", "./d1/f7", "./d1/f8", + "./d1/f9" + }; for (int i = 0; i < files.length; i++) { assertThat(Util.normalizedDirFilePath(dirs[i], files[i])).isEqualTo(paths[i]); } @@ -180,54 +182,54 @@ @Test public void addKeyToMapTest() { - Map<String, Set<String>> m = new HashMap<>(); - String key1 = "key1"; - String key2 = "key2"; - assertThat(m).isEmpty(); - Util.addKeyToMap(m, key1); - assertThat(m).hasSize(1); - assertThat(m.get(key1)).isEmpty(); - Util.addKeyToMap(m, key1); - assertThat(m).hasSize(1); - assertThat(m.get(key1)).isEmpty(); - Util.addKeyToMap(m, key2); - assertThat(m).hasSize(2); - assertThat(m.get(key1)).isEmpty(); - assertThat(m.get(key2)).isEmpty(); + Map<String, Set<String>> m = new HashMap<>(); + String key1 = "key1"; + String key2 = "key2"; + assertThat(m).isEmpty(); + Util.addKeyToMap(m, key1); + assertThat(m).hasSize(1); + assertThat(m.get(key1)).isEmpty(); + Util.addKeyToMap(m, key1); + assertThat(m).hasSize(1); + assertThat(m.get(key1)).isEmpty(); + Util.addKeyToMap(m, key2); + assertThat(m).hasSize(2); + assertThat(m.get(key1)).isEmpty(); + assertThat(m.get(key2)).isEmpty(); } @Test public void addToMapTest() { - Map<String, Set<String>> m = new HashMap<>(); - String key1 = "key1"; - String key2 = "key2"; - Util.addToMap(m, key1, "v1"); - Util.addToMap(m, key2, "v2"); - Util.addToMap(m, key2, "v3"); - assertThat(m.get(key1)).containsExactly("v1"); - assertThat(m.get(key2)).containsExactly("v2", "v3"); + Map<String, Set<String>> m = new HashMap<>(); + String key1 = "key1"; + String key2 = "key2"; + Util.addToMap(m, key1, "v1"); + Util.addToMap(m, key2, "v2"); + Util.addToMap(m, key2, "v3"); + assertThat(m.get(key1)).containsExactly("v1"); + assertThat(m.get(key2)).containsExactly("v2", "v3"); } @Test public void addAllToMapTest() { - Map<String, Set<String>> m = new HashMap<>(); - Set<String> s = new HashSet<>(); - String key = "key"; - Util.addAllToMap(m, key, s); - assertThat(m.get(key)).isEmpty(); - s.add("v1"); - Util.addAllToMap(m, key, s); - assertThat(m.get(key)).containsExactly("v1"); - Util.addAllToMap(m, key, s); - assertThat(m.get(key)).containsExactly("v1"); - s.add("v2"); - Util.addAllToMap(m, key, s); - assertThat(m.get(key)).containsExactly("v2", "v1"); - Util.addAllToMap(m, key, s); - assertThat(m.get(key)).containsExactly("v2", "v1"); - s.add("v3"); - Util.addAllToMap(m, key, s); - assertThat(m.get(key)).containsExactly("v2", "v1", "v3"); + Map<String, Set<String>> m = new HashMap<>(); + Set<String> s = new HashSet<>(); + String key = "key"; + Util.addAllToMap(m, key, s); + assertThat(m.get(key)).isEmpty(); + s.add("v1"); + Util.addAllToMap(m, key, s); + assertThat(m.get(key)).containsExactly("v1"); + Util.addAllToMap(m, key, s); + assertThat(m.get(key)).containsExactly("v1"); + s.add("v2"); + Util.addAllToMap(m, key, s); + assertThat(m.get(key)).containsExactly("v2", "v1"); + Util.addAllToMap(m, key, s); + assertThat(m.get(key)).containsExactly("v2", "v1"); + s.add("v3"); + Util.addAllToMap(m, key, s); + assertThat(m.get(key)).containsExactly("v2", "v1", "v3"); } @Test
diff --git a/src/test/java/com/googlesource/gerrit/plugins/findowners/Watcher.java b/src/test/java/com/googlesource/gerrit/plugins/findowners/Watcher.java index ae743a7..34b70cc 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/findowners/Watcher.java +++ b/src/test/java/com/googlesource/gerrit/plugins/findowners/Watcher.java
@@ -22,7 +22,9 @@ class Watcher extends TestWatcher { private final FluentLogger logger; - Watcher(FluentLogger logger) { this.logger = logger; } + Watcher(FluentLogger logger) { + this.logger = logger; + } @Override public void starting(final Description method) {