Merge "Add a link to syntax.md after error messages"
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 0729f72..8048558 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/findowners/OwnersDb.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/OwnersDb.java
@@ -64,6 +64,7 @@
Map<String, Set<String>> path2Owners = new HashMap<>(); // dir or file glob to owner emails
Set<String> readDirs = new HashSet<>(); // directories in which we have checked OWNERS
Set<String> stopLooking = new HashSet<>(); // directories where OWNERS has "set noparent"
+ Set<String> noParentGlobs = new HashSet<>(); // per-file globs with "set noparent"
Map<String, String> preferredEmails = new HashMap<>(); // owner email to preferred email
List<String> errors = new ArrayList<>(); // error messages
List<String> logs = new ArrayList<>(); // trace/debug messages
@@ -172,10 +173,14 @@
Util.addToMap(owner2Paths, owner, path);
Util.addToMap(path2Owners, path, owner);
if (path.length() > 0 && path.charAt(path.length() - 1) != '/') {
- Util.addToMap(dir2Globs, Util.getDirName(path) + "/", path); // A file glob.
+ add2dir2Globs(Util.getDirName(path) + "/", path); // A file glob.
}
}
+ void add2dir2Globs(String dir, String glob) {
+ Util.addToMap(dir2Globs, dir, glob);
+ }
+
void addPreferredEmails(Set<String> ownerEmails) {
List<String> owners = new ArrayList<>(ownerEmails);
owners.removeIf(o -> preferredEmails.get(o) != null);
@@ -222,6 +227,7 @@
if (result.stopLooking) {
stopLooking.add(dirPath);
}
+ noParentGlobs.addAll(result.noParentGlobs);
addPreferredEmails(result.owner2paths.keySet());
for (String owner : result.owner2paths.keySet()) {
String email = preferredEmails.get(owner);
@@ -229,6 +235,9 @@
addOwnerPathPair(email, path);
}
}
+ for (String glob : result.noParentGlobs) {
+ add2dir2Globs(Util.getDirName(glob) + "/", glob);
+ }
if (config.getReportSyntaxError()) {
result.warnings.forEach(w -> logger.atWarning().log(w));
result.errors.forEach(w -> logger.atSevere().log(w));
@@ -286,7 +295,7 @@
for (String fileName : files) {
fileName = Util.addDotPrefix(fileName);
logs.add("checkFile:" + fileName);
- String dirPath = Util.getParentDir(fileName);
+ String dirPath = Util.getParentDir(fileName); // ".", "./d1", "./d1/d2", etc.
String baseName = fileName.substring(dirPath.length() + 1);
int distance = 1;
FileSystem fileSystem = FileSystems.getDefault();
@@ -298,26 +307,26 @@
while (true) {
int savedSizeOfPaths = paths.size();
logs.add("checkDir:" + dirPath);
+ boolean foundNoParentGlob = false;
if (dir2Globs.containsKey(dirPath + "/")) {
Set<String> patterns = dir2Globs.get(dirPath + "/");
for (String pat : patterns) {
PathMatcher matcher = fileSystem.getPathMatcher("glob:" + pat);
if (matcher.matches(Paths.get(dirPath + "/" + baseName))) {
foundStar |= findStarOwner(pat, distance, paths, distances);
+ foundNoParentGlob |= noParentGlobs.contains(pat);
// Do not break here, a file could match multiple globs
// with different owners.
// OwnerWeights.add won't add duplicated files.
}
}
- // NOTE: A per-file directive can only specify owner emails,
- // not "set noparent".
}
- // If baseName does not match per-file glob, paths is not changed.
- // Then we should check the general non-per-file owners.
- if (paths.size() == savedSizeOfPaths) {
+ // Unless foundNoParentGlob, we should check the general non-per-file owners.
+ if (!foundNoParentGlob) {
foundStar |= findStarOwner(dirPath + "/", distance, paths, distances);
}
if (stopLooking.contains(dirPath + "/") // stop looking parent
+ || foundNoParentGlob // per-file "set noparent"
|| !dirPath.contains("/") /* root */) {
break;
}
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 5c6e050..c58a9ef 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/findowners/OwnersValidator.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/OwnersValidator.java
@@ -256,9 +256,11 @@
// no email address to check
} else if ((email = Parser.parseEmail(line)) != null) {
collectEmail(email2lines, email, path, lineNumber);
- } else if ((emails = Parser.parsePerFileEmails(line)) != null) {
+ } else if ((emails = Parser.parsePerFileOwners(line)) != null) {
for (String e : emails) {
- collectEmail(email2lines, e, path, lineNumber);
+ if (!e.equals(Parser.TOK_SET_NOPARENT)) {
+ collectEmail(email2lines, e, path, lineNumber);
+ }
}
} else if (Parser.isInclude(line)) {
// Included "OWNERS" files will be checked by themselves.
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 317bea7..ad2fe74 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/findowners/Parser.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/Parser.java
@@ -31,35 +31,14 @@
/**
* Parse lines in an OWNERS file and put them into an OwnersDb.
*
- * <p>OWNERS file syntax:
- *
- * <pre>
- * lines := (\s* line? \s* "\n")*
- * line := "set" \s+ "noparent"
- * | "per-file" \s+ globs \s* "=" \s* directives
- * | "file:" \s* glob
- * | "include" SPACE+ (project SPACE* ":" SPACE*)? filePath
- * | comment
- * | directive
- * project := a Gerrit absolute project path name without space or column character
- * filePath := a file path name without space or column character
- * directives := directive (comma directive)*
- * directive := email_address
- * | "*"
- * globs := glob (comma glob)*
- * glob := [a-zA-Z0-9_-*?.]+
- * comma := \s* "," \s*
- * comment := "#" [^"\n"]*
- * </pre>
- *
- * <p>The "file:" directive is not implemented yet.
- *
- * <p>"per-file globs = directives" applies each directive to files matching any of the globs.
- * A glob does not contain directory path.
+ * The syntax, semantics, and some examples are included in syntax.md.
*/
class Parser {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ // Artifical owner token for "set noparent" when used in per-file.
+ protected static final String TOK_SET_NOPARENT = "set noparent";
+
// Globs and emails are separated by commas with optional spaces around a comma.
protected static final String COMMA = "[\\s]*,[\\s]*"; // used in unit tests
@@ -72,6 +51,8 @@
// TODO: have a more precise email address pattern.
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 + ")?";
@@ -81,21 +62,24 @@
private static final String PROJECT_AND_FILE = PROJECT_NAME + FILE_PATH;
+ private static final String SET_NOPARENT = "set[\\s]+noparent";
+
// Simple input lines with 0 or 1 sub-pattern.
private static final Pattern PAT_COMMENT = Pattern.compile(BOL + EOL);
private static final Pattern PAT_EMAIL = Pattern.compile(BOL + EMAIL_OR_STAR + EOL);
private static final Pattern PAT_FILE = Pattern.compile(BOL + "file:.*" + EOL);
private static final Pattern PAT_INCLUDE =
Pattern.compile(BOL + "include[\\s]+" + PROJECT_AND_FILE + EOL);
- private static final Pattern PAT_NO_PARENT = Pattern.compile(BOL + "set[\\s]+noparent" + EOL);
+ private static final Pattern PAT_NO_PARENT = Pattern.compile(BOL + SET_NOPARENT + EOL);
- // Patterns to match trimmed globs and emails in per-file lines.
- private static final Pattern PAT_EMAIL_LIST =
- Pattern.compile("^(" + EMAIL_OR_STAR + "(" + COMMA + EMAIL_OR_STAR + ")*)$");
+ // Patterns to match trimmed globs, emails, and set noparent in per-file lines.
+ private static final Pattern PAT_PER_FILE_OWNERS =
+ Pattern.compile("^(" + EMAIL_LIST + "|(" + SET_NOPARENT + "))$");
private static final Pattern PAT_GLOBS =
Pattern.compile("^(" + GLOB + "(" + COMMA + GLOB + ")*)$");
// PAT_PER_FILE matches a line to two groups: (1) globs, (2) emails
- // Trimmed 1st group should match PAT_GLOBS; trimmed 2nd group should match PAT_EMAIL_LIST.
+ // Trimmed 1st group should match PAT_GLOBS;
+ // trimmed 2nd group should match PAT_PER_FILE_OWNERS.
private static final Pattern PAT_PER_FILE =
Pattern.compile(BOL + "per-file[\\s]+([^=#]+)=[\\s]*([^#]+)" + EOL);
@@ -165,18 +149,22 @@
return parts;
}
+ static String removeExtraSpaces(String s) {
+ return s.trim().replaceAll("[\\s]+", " ");
+ }
+
static String[] parsePerFile(String line) {
Matcher m = PAT_PER_FILE.matcher(line);
if (!m.matches() || !isGlobs(m.group(1).trim())
- || !PAT_EMAIL_LIST.matcher(m.group(2).trim()).matches()) {
+ || !PAT_PER_FILE_OWNERS.matcher(m.group(2).trim()).matches()) {
return null;
}
- return new String[]{m.group(1).trim(), m.group(2).trim()};
+ return new String[]{removeExtraSpaces(m.group(1)), removeExtraSpaces(m.group(2))};
}
- static String[] parsePerFileEmails(String line) {
- String[] globsAndEmails = parsePerFile(line);
- return (globsAndEmails != null) ? globsAndEmails[1].split(COMMA, -1) : null;
+ static String[] parsePerFileOwners(String line) {
+ String[] globsAndOwners = parsePerFile(line);
+ return (globsAndOwners != null) ? globsAndOwners[1].split(COMMA, -1) : null;
}
static class Result {
@@ -184,23 +172,24 @@
List<String> warnings; // warning messages
List<String> errors; // error messages
Map<String, Set<String>> owner2paths; // maps from owner email to pathGlobs
+ Set<String> noParentGlobs; // per-file dirpath+glob with "set noparent"
Result() {
stopLooking = false;
warnings = new ArrayList<>();
errors = new ArrayList<>();
owner2paths = new HashMap<>();
+ noParentGlobs = new HashSet<>();
}
void append(Result r) {
warnings.addAll(r.warnings);
- errors.addAll(r.warnings);
+ errors.addAll(r.errors);
stopLooking |= r.stopLooking; // included file's "set noparent" applies to the including file
for (String key : r.owner2paths.keySet()) {
- for (String dir : r.owner2paths.get(key)) {
- Util.addToMap(owner2paths, key, dir);
- }
+ Util.addAllToMap(owner2paths, key, r.owner2paths.get(key));
}
+ noParentGlobs.addAll(r.noParentGlobs);
}
}
@@ -260,7 +249,7 @@
*/
void parseLine(Result result, String dir, String line, int num) {
String email;
- String[] globsAndEmails;
+ String[] globsAndOwners;
String[] projectAndFile;
if (isNoParent(line)) {
result.stopLooking = true;
@@ -268,11 +257,15 @@
// ignore comment and empty lines.
} else if ((email = parseEmail(line)) != null) {
Util.addToMap(result.owner2paths, email, dir);
- } else if ((globsAndEmails = parsePerFile(line)) != null) {
- String[] emails = globsAndEmails[1].split(COMMA, -1);
- for (String glob : globsAndEmails[0].split(COMMA, -1)) {
- for (String e : emails) {
- Util.addToMap(result.owner2paths, e, dir + glob);
+ } else if ((globsAndOwners = parsePerFile(line)) != null) {
+ String[] owners = globsAndOwners[1].split(COMMA, -1);
+ for (String glob : globsAndOwners[0].split(COMMA, -1)) {
+ for (String e : owners) {
+ if (e.equals(Parser.TOK_SET_NOPARENT)) {
+ result.noParentGlobs.add(dir + glob);
+ } else {
+ Util.addToMap(result.owner2paths, e, dir + glob);
+ }
}
}
} else if (isFile(line)) {
diff --git a/src/main/resources/Documentation/syntax.md b/src/main/resources/Documentation/syntax.md
index 946b362..1470ed2 100644
--- a/src/main/resources/Documentation/syntax.md
+++ b/src/main/resources/Documentation/syntax.md
@@ -8,17 +8,19 @@
```java
lines := (SPACE* line? SPACE* EOL)*
-line := "set" SPACE+ "noparent"
- | "per-file" SPACE+ globs SPACE* "=" SPACE* directives
+line := comment
+ | noparent
+ | ownerEmail
+ | "per-file" SPACE+ globs SPACE* "=" SPACE* owners
| "include" SPACE+ (project SPACE* ":" SPACE*)? filePath
- | comment
- | directive
-directives := directive (SPACE* "," SPACE* directive)*
-directive := email
+comment := "#" ANYCHAR*
+noparent := "set" SPACE+ "noparent"
+ownerEmail := email
| "*"
+owners := noparent
+ | ownerEmail (SPACE* "," SPACE* ownerEmail)*
globs := glob (SPACE* "," SPACE* glob)*
glob := [a-zA-Z0-9_-*?.]+
-comment := "#" ANYCHAR*
email := [^ @]+@[^ #]+
project := a Gerrit project name without space or column character
filePath := a file path name without space or column character
@@ -27,29 +29,39 @@
SPACE := any white space character
```
-* An OWNERS file can include another file with the "include filePath"
- or "include project:filePath" line.
- When the "project:" is not specified, the OWNERS file's project is used.
- The included file is given with the "filePath".
+* An OWNERS file can include another file with the `include filePath`
+ or `include project:filePath` line.
+ When the `project:` is not specified, the OWNERS file's project is used.
+ The included file is given with the `filePath`.
* If the filePath starts with "/", it is an absolute path starting from
the project root directory. Otherwise, the filePath is added a prefix
of the current including file directory and then searched from the
(given) project root directory.
-* `per-file globs = directives` applies each `directive` only to files
- matching any of the `globs`. Number of globs does not need to be equal
- to the number of directives.
+* An OWNERS file inherit rules in OWNERS files of parent directories
+ by default, unless `set noparent` is specified.
-* A 'glob' does not contain directory path.
+* A "glob" is UNIX globbing pathname without the directory path.
+
+* `per-file globs = owners` applies `owners` only to files
+ matching any of the `globs`. Number of globs does not need to be equal
+ to the number of `ownerEmail` in `owners`.
+
+* `per-file globs = set noparent` is like `set noparent` but applies only to
+ files matching any of the `globs`. OWNERS files in parent directories
+ are not considrered for files matching those globs. Even default ownerEmails
+ specified in the current OWNERS file are not included.
+
+* Without the `per-file globs = set noparent`, all global ownerEmails also
+ apply to files matching those globs.
* The email addresses should belong to registered Gerrit users.
A group mailing address can be used as long as it is associated to
a Gerrit user account.
-* The `*` directive means that no owner is specified for the directory
- or file. Any user can approve that directory or files. All other specified
- owner email addresses for the same directory or files are ignored.
+* The `*` is a special ownerEmail meaning that
+ any user can approve that directory or files.
### Examples
@@ -63,16 +75,22 @@
include P1/P2:/core/OWNERS # include file core/OWNERS of project P1/P2
include ../base/OWNERS # include <this_owner_file_dir>/../base/OWNERS
+include /OWNERS # include OWNERS at root directory of this repository
-per-file *.c,*.cpp = x@g.com,y@g.com,z@g.com
+per-file *.c, *.cpp = x@g.com, y@g.com, z@g.com
# x@, y@ and z@ are owners of all *.c or *.cpp files
per-file *.c = c@g.com
# c@, x@, y@ and z@ are owners of all *.c files
-per-file *.xml,README:*
-# no owner for *.xml and README files
+per-file *.xml,README=*,x@g.com
+# in additional to x@g.com, anyone can be owner for *.xml and README files
abc@g.com # one default owner
xyz@g.com # another default owner
# abc@ and xyz@ are owners for all files in this directory,
-# except *.c, *.cpp, *.xml, and README files
+# including *.c, *.cpp, *.xml, and README files
+
+per-file *.txt,*.java = set noparent
+per-file *.txt,*.java = jj@g.com
+# Only jj@g.com is the owner of *.txt and *.java files,
+# not even abc@g.com or xyz@g.com
```
diff --git a/src/main/resources/static/find-owners.js b/src/main/resources/static/find-owners.js
index 4f3522f..c773aa9 100644
--- a/src/main/resources/static/find-owners.js
+++ b/src/main/resources/static/find-owners.js
@@ -577,7 +577,7 @@
changeActions.addTapListener(actionKey,
() => popupFindOwnersPage(null, change, revision, false));
}
- function onClick(e) {
+ function onClick(event) {
if (pageDiv.style.visibility != 'hidden' && !useContextPopup) {
var x = event.clientX;
var y = event.clientY;
diff --git a/src/test/java/com/googlesource/gerrit/plugins/findowners/FindOwnersIT.java b/src/test/java/com/googlesource/gerrit/plugins/findowners/FindOwnersIT.java
index 7559da9..d50a1c0 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/findowners/FindOwnersIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/findowners/FindOwnersIT.java
@@ -327,12 +327,12 @@
String ownerA = ownerJson("a@a");
String ownerB = ownerJson("b@b");
String ownerC = ownerJson("c@c");
+ String ownerABC = "owners:[ " +ownerA + ", " + ownerB + ", " + ownerC;
String ownerX = ownerJson("x@x");
- assertThat(getOwnersResponse(c2)).contains("owners:[ " + ownerX + " ], files:[ t.c ]");
- // Add "t.txt" file, which has new owners.
+ assertThat(getOwnersResponse(c2)).contains(ownerABC + ", " + ownerX + " ], files:[ t.c ]");
+ // Add "t.txt" file, which has only global default owners.
PushOneCommit.Result c3 = createChange("3", "t.txt", "Test!");
- assertThat(getOwnersResponse(c3))
- .contains("owners:[ " + ownerA + ", " + ownerB + ", " + ownerC + " ], files:[ t.txt ]");
+ assertThat(getOwnersResponse(c3)).contains(ownerABC + " ], files:[ t.txt ]");
}
@Test
@@ -342,9 +342,31 @@
PushOneCommit.Result c2 = addFile("2", "d1/OWNERS", "d1@g\nper-file OWNERS=d1o@g\n");
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");
- assertThat(getOwnersResponse(c1)).contains("{ ./d1/d1/OWNERS:[ d1d1o@g, d1o@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(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(c4)).contains("{ ./d2/OWNERS:[ d2@g, d2o@g ] }");
+ }
+
+ @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");
+ // 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 ] }");
assertThat(getOwnersResponse(c2)).contains("{ ./d1/OWNERS:[ d1o@g ] }");
- assertThat(getOwnersResponse(c3)).contains("{ ./d2/d1/OWNERS:[ d1d1o@g, d2o@g ] }");
+ assertThat(getOwnersResponse(c3)).contains("{ ./d2/d1/OWNERS:[ d1d1o@g, d2d1o@g ] }");
assertThat(getOwnersResponse(c4)).contains("{ ./d2/OWNERS:[ d2o@g ] }");
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/findowners/OwnersValidatorTest.java b/src/test/java/com/googlesource/gerrit/plugins/findowners/OwnersValidatorTest.java
index cc60789..f81e465 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/findowners/OwnersValidatorTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/findowners/OwnersValidatorTest.java
@@ -144,6 +144,7 @@
+ "per-file *.py=u2.m@g.com \n"
+ "per-file *.c, *.java ,A*bp = u1@g.com, u1+review@g.com ,u2.m@g.com#comment\n"
+ " per-file *.txt = * # everyone can approve # \n"
+ + "per-file *.java = set noparent # \n"
+ " set noparent # comment#\n");
private static final ImmutableSet<String> EXPECTED_VERBOSE_OUTPUT =
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 d5c31de..a26e952 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/findowners/ParserTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/findowners/ParserTest.java
@@ -71,6 +71,39 @@
}
@Test
+ public void appendResultTest() {
+ String s1 = "a@b@c";
+ String s2 = "**";
+ String e1 = testLineErrorMsg(s1);
+ String e2 = testLineErrorMsg(s2);
+ String w1 = testLineWarningMsg("w1");
+ String w2 = testLineWarningMsg("w2");
+ String b1 = "d1/*.c";
+ String b2 = "d2/*.java";
+ Parser.Result r1 = testLine(s1);
+ Parser.Result r2 = testLine(s2);
+ assertThat(r1.warnings).isEmpty();
+ assertThat(r2.warnings).isEmpty();
+ assertThat(r1.noParentGlobs).isEmpty();
+ assertThat(r2.noParentGlobs).isEmpty();
+ assertThat(r1.errors).containsExactly(e1);
+ assertThat(r2.errors).containsExactly(e2);
+ r1.warnings.add(w1);
+ r2.warnings.add(w2);
+ r1.noParentGlobs.add(b1);
+ r2.noParentGlobs.add(b2);
+ r2.append(r1);
+ assertThat(r2.warnings).containsExactly(w2, w1);
+ assertThat(r2.noParentGlobs).containsExactly(b2, b1);
+ assertThat(r1.noParentGlobs).containsExactly(b1);
+ assertThat(r2.errors).containsExactly(e2, e1);
+ r1.append(r2);
+ assertThat(r1.warnings).containsExactly(w1, w2, w1);
+ assertThat(r1.noParentGlobs).containsExactly(b2, b1); // set union
+ assertThat(r1.errors).containsExactly(e1, e2, e1);
+ }
+
+ @Test
public void commentLineTest() {
String[] lines = {"", " ", "# comment #data", "#any", " # comment"};
for (String s : lines) {
@@ -120,22 +153,35 @@
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 "
+ "a@g.com , xyz@gmail.com , * # comment", "*,*#comment", " a@b,c@d ",
+ " set noparent ", "\tset\t\tnoparent\t"
};
String[] globsList = {"*", "*,*.c", " *test*.java , *.cc, *.cpp ", "*.bp,*.mk ,A* "};
for (String directive : directives) {
for (String globs : globsList) {
String line = "per-file " + globs + "=" + directive;
Parser.Result result = testLine(line);
- String[] emailList = directive.replaceAll("#.*$", "").trim().split(Parser.COMMA, -1);
+ String[] directiveList = directive.replaceAll("#.*$", "").trim().split(Parser.COMMA, -1);
String[] globList = globs.trim().split(Parser.COMMA);
Arrays.sort(globList);
- for (String email : emailList) {
- String[] paths = result.owner2paths.get(email).toArray(new String[1]);
- assertThat(paths).hasLength(globList.length);
- Arrays.sort(paths);
- for (int g = 0; g < globList.length; g++) {
- assertThat(paths[g]).isEqualTo(mockedTestDir() + globList[g]);
+ String[] owners = Parser.parsePerFileOwners(line);
+ assertThat(owners).hasLength(directiveList.length);
+ for (String email : owners) {
+ String e = email.trim();
+ assertThat(result.stopLooking).isFalse();
+ if (e.equals(Parser.TOK_SET_NOPARENT)) {
+ assertThat(result.owner2paths).isEmpty(); // no other owners in this per-file
+ assertThat(result.noParentGlobs).hasSize(globList.length);
+ for (String glob : globList) {
+ assertThat(result.noParentGlobs).contains(mockedTestDir() + glob);
+ }
+ } else {
+ String[] paths = result.owner2paths.get(e).toArray(new String[1]);
+ 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]);
+ }
}
}
}
@@ -145,8 +191,8 @@
@Test
public void perFileBadDirectiveTest() {
String[] directives = {
- "file://OWNERS", " ** ", "a b@c .co", "a@b@c #com", "a.<b>@zc#", " set noparent ",
- " , a@b ", "a@b, , c@d #"
+ "file://OWNERS", " ** ", "a b@c .co", "a@b@c #com", "a.<b>@zc#",
+ " , a@b ", "a@b, , c@d #", "a@b, set noparent"
};
for (String directive : directives) {
String line = "per-file *test*.c=" + directive;
