Improve error message when trying to specify code owners by username
Code owners in OWNERS files need to be specified by email.
It's a common mistake by our users to specify usernames instead (e.g.
'foo' instead of 'foo@google.com').
So far code owners that were specifed by username were rejected with a
general "invalid line: ..." error and users had trouble spotting what
was wrong.
With this change we return a better error message now: "code owner ...
in ... cannot be resolved (must be an email)"
The generic error was thrown from the parser that parses OWNERS files.
We relaxed the matching for code owners there, so that code owners that
are specified by username do not cause a parsing error. These code
owners are not resolvable and hence parsing them is OK. In
CodeOwnerConfigValidator we add a special check for code owners that
are usernames and reject them with the better error message.
Quite some tests check the behavior with non-parseable OWNERS files. So
far to trigger a parsing error these tests created an OWNERS file with a
single line being "INVALID". A line in an OWNERS file is invalid if it
doesn't match a supported rule with defined keywords and if it cannot be
parsed as a code owner. The string "INVALID" doesn't match any keyword
rule. So far it couldn't be parsed as a code owner since it was not a
valid email, but the parsing logic allows usernames for code owners now.
Usernames are just strings that do not contain special characters, so
"INVALID" can be parsed as a username now and doesn't cause a parsing
error. To trigger a parsing error with the new parsing logic the tests
create an OWNERS file with a single line being "@INVALID" now. This
string is neither a valid email nor a valid username (as it contains a
special character), hence it's causing a parsing error.
Bug: Google b/271386354
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I04efc5028a629e017614cc8c1963f586ec4b5d5b
diff --git a/java/com/google/gerrit/plugins/codeowners/acceptance/AbstractCodeOwnersTest.java b/java/com/google/gerrit/plugins/codeowners/acceptance/AbstractCodeOwnersTest.java
index 32c82a1..0e872a9 100644
--- a/java/com/google/gerrit/plugins/codeowners/acceptance/AbstractCodeOwnersTest.java
+++ b/java/com/google/gerrit/plugins/codeowners/acceptance/AbstractCodeOwnersTest.java
@@ -225,7 +225,7 @@
protected void createNonParseableCodeOwnerConfig(String path) throws Exception {
disableCodeOwnersForProject(project);
String changeId =
- createChange("Add invalid code owners file", JgitPath.of(path).get(), "INVALID")
+ createChange("Add invalid code owners file", JgitPath.of(path).get(), "@INVALID")
.getChangeId();
approve(changeId);
gApi.changes().id(changeId).current().submit();
@@ -240,9 +240,9 @@
return getParsingErrorMessage(
ImmutableMap.of(
FindOwnersBackend.class,
- "invalid line: INVALID",
+ "invalid line: @INVALID",
ProtoBackend.class,
- "1:8: Expected \"{\"."));
+ "1:1: Expected identifier. Found '@'"));
}
protected String getParsingErrorMessage(
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/findowners/FindOwnersCodeOwnerConfigParser.java b/java/com/google/gerrit/plugins/codeowners/backend/findowners/FindOwnersCodeOwnerConfigParser.java
index 5f35099..a8562ae 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/findowners/FindOwnersCodeOwnerConfigParser.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/findowners/FindOwnersCodeOwnerConfigParser.java
@@ -149,9 +149,10 @@
private static final String EOL = "[\\s]*(#.*)?$"; // end-of-line
private static final String GLOB = "[^\\s,=]+"; // a file glob
- private static final String EMAIL_OR_STAR = "([^\\s<>@,]+@[^\\s<>@#,]+|\\*)";
- private static final String EMAIL_LIST =
- "(" + EMAIL_OR_STAR + "(" + COMMA + EMAIL_OR_STAR + ")*)";
+ private static final String USERNAME_OR_EMAIL_OR_STAR =
+ "([^\\s<>@#,]+|[^\\s<>@,]+@[^\\s<>@#,]+|\\*)";
+ private static final String USERNAME_OR_EMAIL_LIST =
+ "(" + USERNAME_OR_EMAIL_OR_STAR + "(" + COMMA + USERNAME_OR_EMAIL_OR_STAR + ")*)";
// Optional name of a Gerrit project followed by a colon and optional spaces.
private static final String PROJECT_NAME = "([^\\s:]+" + COLON + ")?";
@@ -172,14 +173,15 @@
// 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_EMAIL = Pattern.compile(BOL + USERNAME_OR_EMAIL_OR_STAR + EOL);
private static final Pattern PAT_ANNOTATION = Pattern.compile("#\\{([A-Za-z_]+)\\}");
private static final Pattern PAT_INCLUDE =
Pattern.compile(BOL + INCLUDE_OR_FILE + PROJECT_BRANCH_AND_FILE + EOL);
private static final Pattern PAT_NO_PARENT = Pattern.compile(BOL + SET_NOPARENT + EOL);
private static final Pattern PAT_PER_FILE_OWNERS =
- Pattern.compile("^(" + EMAIL_LIST + "|" + SET_NOPARENT + "|" + FILE_DIRECTIVE + ")$");
+ Pattern.compile(
+ "^(" + USERNAME_OR_EMAIL_LIST + "|" + SET_NOPARENT + "|" + FILE_DIRECTIVE + ")$");
private static final Pattern PAT_PER_FILE_INCLUDE =
Pattern.compile("^(" + INCLUDE_DIRECTIVE + ")$");
private static final Pattern PAT_GLOBS =
diff --git a/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java b/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java
index 0083497..1c4d42f 100644
--- a/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java
+++ b/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java
@@ -959,6 +959,14 @@
Path codeOwnerConfigFilePath,
CodeOwnerReference codeOwnerReference) {
CodeOwnerResolver codeOwnerResolver = codeOwnerResolverProvider.get().forUser(user);
+ if (!CodeOwnerResolver.ALL_USERS_WILDCARD.equals(codeOwnerReference.email())
+ && !codeOwnerReference.email().contains("@")) {
+ return nonResolvableCodeOwner(
+ branchNameKey,
+ String.format(
+ "code owner '%s' in '%s' cannot be resolved (must be an email)",
+ codeOwnerReference.email(), codeOwnerConfigFilePath));
+ }
if (!codeOwnerResolver.isEmailDomainAllowed(codeOwnerReference.email()).get()) {
return nonResolvableCodeOwner(
branchNameKey,
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerConfigFilesInRevisionIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerConfigFilesInRevisionIT.java
index 980ee02..e061a8d 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerConfigFilesInRevisionIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerConfigFilesInRevisionIT.java
@@ -117,7 +117,7 @@
disableCodeOwnersForProject(project);
String changeId =
- createChange("Add code owners", JgitPath.of(codeOwnerConfigPath).get(), "INVALID")
+ createChange("Add code owners", JgitPath.of(codeOwnerConfigPath).get(), "@INVALID")
.getChangeId();
enableCodeOwnersForProject(project);
@@ -133,9 +133,9 @@
getParsingErrorMessage(
ImmutableMap.of(
FindOwnersBackend.class,
- "invalid line: INVALID",
+ "invalid line: @INVALID",
ProtoBackend.class,
- "1:8: Expected \"{\"."))))));
+ "1:1: Expected identifier. Found '@'"))))));
}
@Test
@@ -440,7 +440,7 @@
"Add code owners",
ImmutableMap.of(
JgitPath.of(pathOfNonParseableCodeOwnerConfig).get(),
- "INVALID",
+ "@INVALID",
JgitPath.of(pathOfInvalidCodeOwnerConfig).get(),
format(
CodeOwnerConfig.builder(keyOfInvalidCodeOwnerConfig, TEST_REVISION)
@@ -463,9 +463,9 @@
getParsingErrorMessage(
ImmutableMap.of(
FindOwnersBackend.class,
- "invalid line: INVALID",
+ "invalid line: @INVALID",
ProtoBackend.class,
- "1:8: Expected \"{\"."))))));
+ "1:1: Expected identifier. Found '@'"))))));
if (verbosity == null
|| ConsistencyProblemInfo.Status.ERROR.equals(verbosity)
|| ConsistencyProblemInfo.Status.WARNING.equals(verbosity)) {
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerConfigValidatorIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerConfigValidatorIT.java
index 51b52da..59e7b52 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerConfigValidatorIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerConfigValidatorIT.java
@@ -91,6 +91,9 @@
public class CodeOwnerConfigValidatorIT extends AbstractCodeOwnersIT {
private static final ObjectId TEST_REVISION =
ObjectId.fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef");
+ private static final String INVALID = "@INVALID";
+ private static final String INVALID_2 = "@INVALID2";
+ private static final String PROTO_SYNTAX_ERROR = "1:1: Expected identifier. Found '@'";
@Inject private RequestScopeOperations requestScopeOperations;
@Inject private ProjectOperations projectOperations;
@@ -110,7 +113,7 @@
@Test
public void nonCodeOwnerConfigFileIsNotValidated() throws Exception {
- PushOneCommit.Result r = createChange("Add arbitrary file", "arbitrary-file.txt", "INVALID");
+ PushOneCommit.Result r = createChange("Add arbitrary file", "arbitrary-file.txt", INVALID);
assertOkWithoutMessages(r);
}
@@ -120,7 +123,7 @@
createChange(
"Add code owner config with file extension",
getCodeOwnerConfigFileName() + ".foo",
- "INVALID");
+ INVALID);
assertOkWithoutMessages(r);
}
@@ -131,7 +134,7 @@
createChange(
"Add code owner config with file extension",
getCodeOwnerConfigFileName() + ".foo",
- "INVALID");
+ INVALID);
String abbreviatedCommit = abbreviateName(r.getCommit());
r.assertErrorStatus(
String.format(
@@ -148,7 +151,7 @@
createChange(
"Add code owner config with file extension",
getCodeOwnerConfigFileName() + ".foo",
- "INVALID");
+ INVALID);
String abbreviatedCommit = abbreviateName(r.getCommit());
r.assertErrorStatus(
String.format(
@@ -431,7 +434,7 @@
@GerritConfig(name = "plugin.code-owners.backend", value = "non-existing-backend")
public void canUploadNonParseableConfigIfCodeOwnersPluginConfigurationIsInvalid()
throws Exception {
- PushOneCommit.Result r = createChange("Add code owners", "OWNERS", "INVALID");
+ PushOneCommit.Result r = createChange("Add code owners", "OWNERS", INVALID);
assertOkWithWarnings(
r,
"skipping validation of code owner config files",
@@ -448,7 +451,7 @@
codeOwnerConfigOperations
.codeOwnerConfig(createCodeOwnerConfigKey("/"))
.getJGitFilePath(),
- "INVALID");
+ INVALID);
assertOkWithHints(
r,
"skipping validation of code owner config files",
@@ -579,7 +582,7 @@
codeOwnerConfigOperations
.codeOwnerConfig(createCodeOwnerConfigKey("/"))
.getJGitFilePath(),
- "INVALID");
+ INVALID);
push.setPushOptions(ImmutableList.copyOf(pushOptions));
return push.to("refs/for/master");
}
@@ -608,7 +611,7 @@
createChange(
"Add code owners",
codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey).getJGitFilePath(),
- "INVALID");
+ INVALID);
assertFatalWithMessages(
r,
"invalid code owner config files",
@@ -619,9 +622,9 @@
getParsingErrorMessage(
ImmutableMap.of(
FindOwnersBackend.class,
- "invalid line: INVALID",
+ "invalid line: " + INVALID,
ProtoBackend.class,
- "1:8: expected \"{\""))));
+ PROTO_SYNTAX_ERROR))));
}
@Test
@@ -633,7 +636,7 @@
createChange(
"Add code owners",
codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey).getJGitFilePath(),
- "INVALID");
+ INVALID);
assertOkWithFatals(
r,
"invalid code owner config files",
@@ -644,9 +647,9 @@
getParsingErrorMessage(
ImmutableMap.of(
FindOwnersBackend.class,
- "invalid line: INVALID",
+ "invalid line: " + INVALID,
ProtoBackend.class,
- "1:8: expected \"{\""))));
+ PROTO_SYNTAX_ERROR))));
}
@Test
@@ -664,7 +667,7 @@
createChange(
"Add code owners",
codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey).getJGitFilePath(),
- "INVALID");
+ INVALID);
assertOkWithFatals(
r,
"invalid code owner config files",
@@ -675,9 +678,9 @@
getParsingErrorMessage(
ImmutableMap.of(
FindOwnersBackend.class,
- "invalid line: INVALID",
+ "invalid line: " + INVALID,
ProtoBackend.class,
- "1:8: expected \"{\""))));
+ PROTO_SYNTAX_ERROR))));
}
@Test
@@ -752,7 +755,7 @@
String path =
codeOwnerConfigOperations.codeOwnerConfig(createCodeOwnerConfigKey("/")).getJGitFilePath();
- PushOneCommit.Result r = createChange("Add code owners", path, "INVALID");
+ PushOneCommit.Result r = createChange("Add code owners", path, INVALID);
r.assertOkStatus();
// re-enable the code owners functionality for the project
@@ -778,7 +781,7 @@
createChange(
"Add code owners",
codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey).getJGitFilePath(),
- "INVALID");
+ INVALID);
r.assertOkStatus();
// re-enable the code owners functionality for the project
@@ -789,7 +792,7 @@
createChange(
"Update code owners",
codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey).getJGitFilePath(),
- "STILL INVALID");
+ INVALID_2);
assertOkWithWarnings(
r,
"invalid code owner config files",
@@ -800,9 +803,9 @@
getParsingErrorMessage(
ImmutableMap.of(
FindOwnersBackend.class,
- "invalid line: STILL INVALID",
+ "invalid line: " + INVALID_2,
ProtoBackend.class,
- "1:7: expected \"{\""))));
+ PROTO_SYNTAX_ERROR))));
}
@Test
@@ -818,7 +821,7 @@
createChange(
"Add code owners",
codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey).getJGitFilePath(),
- "INVALID");
+ INVALID);
r.assertOkStatus();
// re-enable the code owners functionality for the project
@@ -904,7 +907,7 @@
createChange(
"Add code owners",
codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey).getJGitFilePath(),
- "INVALID");
+ INVALID);
assertFatalWithMessages(
r,
"invalid code owner config files",
@@ -915,9 +918,9 @@
getParsingErrorMessage(
ImmutableMap.of(
FindOwnersBackend.class,
- "invalid line: INVALID",
+ "invalid line: " + INVALID,
ProtoBackend.class,
- "1:8: expected \"{\""))));
+ PROTO_SYNTAX_ERROR))));
}
@Test
@@ -939,7 +942,7 @@
createChange(
"Add code owners",
codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey).getJGitFilePath(),
- "INVALID");
+ INVALID);
assertFatalWithMessages(
r,
"invalid code owner config files",
@@ -950,9 +953,9 @@
getParsingErrorMessage(
ImmutableMap.of(
FindOwnersBackend.class,
- "invalid line: INVALID",
+ "invalid line: " + INVALID,
ProtoBackend.class,
- "1:8: expected \"{\""))));
+ PROTO_SYNTAX_ERROR))));
}
@Test
@@ -967,9 +970,9 @@
"Add code owners",
ImmutableMap.of(
codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey1).getJGitFilePath(),
- "INVALID",
+ INVALID,
codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey2).getJGitFilePath(),
- "ALSO-INVALID"));
+ INVALID_2));
PushOneCommit.Result r = push.to("refs/for/master");
assertFatalWithMessages(
r,
@@ -981,9 +984,9 @@
getParsingErrorMessage(
ImmutableMap.of(
FindOwnersBackend.class,
- "invalid line: INVALID",
+ "invalid line: " + INVALID,
ProtoBackend.class,
- "1:8: expected \"{\""))),
+ PROTO_SYNTAX_ERROR))),
String.format(
"invalid code owner config file '%s' (project = %s, branch = master):\n %s",
codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey2).getFilePath(),
@@ -991,9 +994,9 @@
getParsingErrorMessage(
ImmutableMap.of(
FindOwnersBackend.class,
- "invalid line: ALSO-INVALID",
+ "invalid line: " + INVALID_2,
ProtoBackend.class,
- "1:1: expected identifier. found 'ALSO-INVALID'"))));
+ PROTO_SYNTAX_ERROR))));
}
@Test
@@ -1028,6 +1031,101 @@
}
@Test
+ public void cannotUploadConfigWithNonResolvablePerFileCodeOwners() throws Exception {
+ CodeOwnerConfig.Key codeOwnerConfigKey = createCodeOwnerConfigKey("/");
+
+ String unknownEmail1 = "non-existing-email@example.com";
+ String unknownEmail2 = "another-unknown-email@example.com";
+ PushOneCommit.Result r =
+ createChange(
+ "Add code owners",
+ codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey).getJGitFilePath(),
+ format(
+ CodeOwnerConfig.builder(codeOwnerConfigKey, TEST_REVISION)
+ .addCodeOwnerSet(
+ CodeOwnerSet.builder()
+ .addPathExpression("foo")
+ .addCodeOwnerEmail(unknownEmail1)
+ .addCodeOwnerEmail(admin.email())
+ .addCodeOwnerEmail(unknownEmail2)
+ .build())
+ .build()));
+ assertErrorWithMessages(
+ r,
+ "invalid code owner config files",
+ String.format(
+ "code owner email '%s' in '%s' cannot be resolved for %s",
+ unknownEmail1,
+ codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey).getFilePath(),
+ identifiedUserFactory.create(admin.id()).getLoggableName()),
+ String.format(
+ "code owner email '%s' in '%s' cannot be resolved for %s",
+ unknownEmail2,
+ codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey).getFilePath(),
+ identifiedUserFactory.create(admin.id()).getLoggableName()));
+ }
+
+ @Test
+ public void cannotUploadConfigWithNonEmailCodeOwners() throws Exception {
+ CodeOwnerConfig.Key codeOwnerConfigKey = createCodeOwnerConfigKey("/");
+
+ String username1 = "user1";
+ String username2 = "user2";
+ PushOneCommit.Result r =
+ createChange(
+ "Add code owners",
+ codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey).getJGitFilePath(),
+ format(
+ CodeOwnerConfig.builder(codeOwnerConfigKey, TEST_REVISION)
+ .addCodeOwnerSet(
+ CodeOwnerSet.createWithoutPathExpressions(
+ username1, admin.email(), username2))
+ .build()));
+ assertErrorWithMessages(
+ r,
+ "invalid code owner config files",
+ String.format(
+ "code owner '%s' in '%s' cannot be resolved (must be an email)",
+ username1, codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey).getFilePath()),
+ String.format(
+ "code owner '%s' in '%s' cannot be resolved (must be an email)",
+ username2,
+ codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey).getFilePath()));
+ }
+
+ @Test
+ public void cannotUploadConfigWithNonEmailPerFileCodeOwners() throws Exception {
+ CodeOwnerConfig.Key codeOwnerConfigKey = createCodeOwnerConfigKey("/");
+
+ String username1 = "user1";
+ String username2 = "user2";
+ String pathExpression = "foo";
+ PushOneCommit.Result r =
+ createChange(
+ "Add code owners",
+ codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey).getJGitFilePath(),
+ format(
+ CodeOwnerConfig.builder(codeOwnerConfigKey, TEST_REVISION)
+ .addCodeOwnerSet(
+ CodeOwnerSet.builder()
+ .addPathExpression(pathExpression)
+ .addCodeOwnerEmail(username1)
+ .addCodeOwnerEmail(username2)
+ .build())
+ .build()));
+ assertErrorWithMessages(
+ r,
+ "invalid code owner config files",
+ String.format(
+ "code owner '%s' in '%s' cannot be resolved (must be an email)",
+ username1, codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey).getFilePath()),
+ String.format(
+ "code owner '%s' in '%s' cannot be resolved (must be an email)",
+ username2,
+ codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey).getFilePath()));
+ }
+
+ @Test
@GerritConfig(name = "plugin.code-owners.allowedEmailDomain", value = "example.com")
public void canUploadConfigThatAssignsCodeOwnershipToAnEmailWithAnAllowedEmailDomain()
throws Exception {
@@ -2091,7 +2189,7 @@
codeOwnerConfigOperations
.codeOwnerConfig(keyOfImportedCodeOwnerConfig)
.getJGitFilePath(),
- "INVALID");
+ INVALID);
r.assertOkStatus();
approve(r.getChangeId());
gApi.changes().id(r.getChangeId()).current().submit();
@@ -3091,7 +3189,7 @@
codeOwnerConfigOperations
.codeOwnerConfig(createCodeOwnerConfigKey("/"))
.getJGitFilePath(),
- "INVALID");
+ INVALID);
assertOkWithHints(
r,
"skipping validation of code owner config files",
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/GetCodeOwnerStatusRestIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/GetCodeOwnerStatusRestIT.java
index 8f4959d..ec5f069 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/GetCodeOwnerStatusRestIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/GetCodeOwnerStatusRestIT.java
@@ -70,7 +70,7 @@
codeOwnerConfigOperations.codeOwnerConfig(createCodeOwnerConfigKey("/")).getFilePath();
disableCodeOwnersForProject(project);
String changeId =
- createChange("Add code owners", JgitPath.of(filePath).get(), "INVALID").getChangeId();
+ createChange("Add code owners", JgitPath.of(filePath).get(), "@INVALID").getChangeId();
approve(changeId);
gApi.changes().id(changeId).current().submit();
enableCodeOwnersForProject(project);
@@ -84,7 +84,7 @@
.contains(
String.format(
"* invalid code owner config file '%s' (project = %s, branch = master):\n"
- + " invalid line: INVALID",
+ + " invalid line: @INVALID",
filePath, project.get()));
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/AbstractFileBasedCodeOwnerBackendTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/AbstractFileBasedCodeOwnerBackendTest.java
index 7ec9295..743f860 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/AbstractFileBasedCodeOwnerBackendTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/AbstractFileBasedCodeOwnerBackendTest.java
@@ -424,7 +424,7 @@
.commit()
.parent(head)
.message("Add invalid test code owner config")
- .add(JgitPath.of(codeOwnerConfigKey.filePath(getFileName())).get(), "INVALID"));
+ .add(JgitPath.of(codeOwnerConfigKey.filePath(getFileName())).get(), "@INVALID"));
}
// Try to update the code owner config.
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/findowners/FindOwnersCodeOwnerConfigParserTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/findowners/FindOwnersCodeOwnerConfigParserTest.java
index 260e635..f1bbcdf 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/findowners/FindOwnersCodeOwnerConfigParserTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/findowners/FindOwnersCodeOwnerConfigParserTest.java
@@ -158,17 +158,29 @@
codeOwnerConfigParser.parse(
TEST_REVISION,
CodeOwnerConfig.Key.create(project, "master", "/"),
- getCodeOwnerConfig(EMAIL_1, "INVALID", "NOT_AN_EMAIL", EMAIL_2)));
+ getCodeOwnerConfig(EMAIL_1, "@INVALID", "NOT_AN_EMAIL", EMAIL_2)));
assertThat(exception.getFullMessage(FindOwnersBackend.CODE_OWNER_CONFIG_FILE_NAME))
.isEqualTo(
String.format(
"invalid code owner config file '/OWNERS' (project = %s, branch = master):\n"
- + " invalid line: INVALID\n"
- + " invalid line: NOT_AN_EMAIL",
+ + " invalid line: @INVALID",
project));
}
@Test
+ public void codeOwnerConfigWithNonEmailLines() throws Exception {
+ assertParseAndFormat(
+ getCodeOwnerConfig(EMAIL_1, "NOT_AN_EMAIL", EMAIL_3),
+ codeOwnerConfig ->
+ assertThat(codeOwnerConfig)
+ .hasCodeOwnerSetsThat()
+ .onlyElement()
+ .hasCodeOwnersEmailsThat()
+ .containsExactly(EMAIL_1, "NOT_AN_EMAIL", EMAIL_3),
+ getCodeOwnerConfig(EMAIL_1, "NOT_AN_EMAIL", EMAIL_3));
+ }
+
+ @Test
public void codeOwnerConfigWithComment() throws Exception {
assertParseAndFormat(
getCodeOwnerConfig(EMAIL_1, EMAIL_2 + " # some comment", EMAIL_3),
