diff options
Diffstat (limited to 'third_party/gerrit_plugins/code-owners')
| -rw-r--r-- | third_party/gerrit_plugins/code-owners/default.nix | 17 | ||||
| -rw-r--r-- | third_party/gerrit_plugins/code-owners/using-usernames.patch | 481 |
2 files changed, 498 insertions, 0 deletions
diff --git a/third_party/gerrit_plugins/code-owners/default.nix b/third_party/gerrit_plugins/code-owners/default.nix new file mode 100644 index 000000000000..3c0c4bed0fd7 --- /dev/null +++ b/third_party/gerrit_plugins/code-owners/default.nix @@ -0,0 +1,17 @@ +{ depot, pkgs, ... }@args: + +let + inherit (import ../builder.nix args) buildGerritBazelPlugin; +in +buildGerritBazelPlugin rec { + name = "code-owners"; + depsOutputHash = "sha256:07mgvd7fvg1xqlabjn644505yx98vjrmwxx1arwsykir1h82h0b2"; + src = pkgs.fetchgit { + url = "https://gerrit.googlesource.com/plugins/code-owners"; + rev = "6fdf3ce2e52904b35e2a5824a4197155c2c6b4e4"; + sha256 = "sha256:17k6310py71wax3881mf3vsf9zas648j4xzs9h0d7migv5nzsdzs"; + }; + patches = [ + ./using-usernames.patch + ]; +} diff --git a/third_party/gerrit_plugins/code-owners/using-usernames.patch b/third_party/gerrit_plugins/code-owners/using-usernames.patch new file mode 100644 index 000000000000..7383aecc08c3 --- /dev/null +++ b/third_party/gerrit_plugins/code-owners/using-usernames.patch @@ -0,0 +1,481 @@ +From ba76ff8b7cd128383c86aeeacf12d1001670eec4 Mon Sep 17 00:00:00 2001 +From: Luke Granger-Brown <git@lukegb.com> +Date: Wed, 21 Sep 2022 03:15:38 +0100 +Subject: [PATCH] Add support for usernames and groups + +Change-Id: I3ba8527f66216d08e555a6ac4451fe0d1e090de5 +--- + .../codeowners/backend/CodeOwnerResolver.java | 120 ++++++++++++++++-- + .../FindOwnersCodeOwnerConfigParser.java | 3 +- + ...AbstractFileBasedCodeOwnerBackendTest.java | 2 +- + .../backend/CodeOwnerResolverTest.java | 87 ++++++++++++- + .../FindOwnersCodeOwnerConfigParserTest.java | 32 ++++- + 5 files changed, 230 insertions(+), 14 deletions(-) + +diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java +index 07894ced..40943659 100644 +--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java ++++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java +@@ -17,6 +17,8 @@ package com.google.gerrit.plugins.codeowners.backend; + import static com.google.common.base.Preconditions.checkState; + import static com.google.common.collect.ImmutableMap.toImmutableMap; + import static com.google.common.collect.ImmutableSet.toImmutableSet; ++import static com.google.common.collect.ImmutableSetMultimap.flatteningToImmutableSetMultimap; ++import static com.google.common.collect.ImmutableSetMultimap.toImmutableSetMultimap; + import static com.google.gerrit.plugins.codeowners.backend.CodeOwnersInternalServerErrorException.newInternalServerError; + import static java.util.Objects.requireNonNull; + +@@ -25,6 +27,7 @@ import com.google.common.collect.ImmutableList; + import com.google.common.collect.ImmutableMap; + import com.google.common.collect.ImmutableMultimap; + import com.google.common.collect.ImmutableSet; ++import com.google.common.collect.ImmutableSetMultimap; + import com.google.common.collect.Iterables; + import com.google.common.collect.Streams; + import com.google.common.flogger.FluentLogger; +@@ -33,17 +36,24 @@ import com.google.gerrit.entities.Project; + import com.google.gerrit.metrics.Timer0; + import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration; + import com.google.gerrit.plugins.codeowners.metrics.CodeOwnerMetrics; ++import com.google.gerrit.server.AnonymousUser; + import com.google.gerrit.server.CurrentUser; + import com.google.gerrit.server.IdentifiedUser; + import com.google.gerrit.server.account.AccountCache; + import com.google.gerrit.server.account.AccountControl; + import com.google.gerrit.server.account.AccountState; ++import com.google.gerrit.server.account.GroupBackend; ++import com.google.gerrit.server.account.GroupBackends; ++import com.google.gerrit.server.account.InternalGroupBackend; + import com.google.gerrit.server.account.externalids.ExternalId; + import com.google.gerrit.server.account.externalids.ExternalIds; + import com.google.gerrit.server.permissions.GlobalPermission; + import com.google.gerrit.server.permissions.PermissionBackend; + import com.google.gerrit.server.permissions.PermissionBackendException; ++import com.google.gerrit.server.util.RequestContext; ++import com.google.gerrit.server.util.ThreadLocalRequestContext; + import com.google.inject.Inject; ++import com.google.inject.OutOfScopeException; + import com.google.inject.Provider; + import java.io.IOException; + import java.nio.file.Path; +@@ -103,6 +113,8 @@ public class CodeOwnerResolver { + + @VisibleForTesting public static final String ALL_USERS_WILDCARD = "*"; + ++ public static final String GROUP_PREFIX = "group:"; ++ + private final CodeOwnersPluginConfiguration codeOwnersPluginConfiguration; + private final PermissionBackend permissionBackend; + private final Provider<CurrentUser> currentUser; +@@ -113,6 +125,8 @@ public class CodeOwnerResolver { + private final CodeOwnerMetrics codeOwnerMetrics; + private final UnresolvedImportFormatter unresolvedImportFormatter; + private final TransientCodeOwnerCache transientCodeOwnerCache; ++ private final InternalGroupBackend groupBackend; ++ private final ThreadLocalRequestContext context; + + // Enforce visibility by default. + private boolean enforceVisibility = true; +@@ -133,7 +147,9 @@ public class CodeOwnerResolver { + PathCodeOwners.Factory pathCodeOwnersFactory, + CodeOwnerMetrics codeOwnerMetrics, + UnresolvedImportFormatter unresolvedImportFormatter, +- TransientCodeOwnerCache transientCodeOwnerCache) { ++ TransientCodeOwnerCache transientCodeOwnerCache, ++ InternalGroupBackend groupBackend, ++ ThreadLocalRequestContext context) { + this.codeOwnersPluginConfiguration = codeOwnersPluginConfiguration; + this.permissionBackend = permissionBackend; + this.currentUser = currentUser; +@@ -144,6 +160,8 @@ public class CodeOwnerResolver { + this.codeOwnerMetrics = codeOwnerMetrics; + this.unresolvedImportFormatter = unresolvedImportFormatter; + this.transientCodeOwnerCache = transientCodeOwnerCache; ++ this.groupBackend = groupBackend; ++ this.context = context; + } + + /** +@@ -357,6 +375,12 @@ public class CodeOwnerResolver { + "cannot resolve code owner email %s: no account with this email exists", + CodeOwnerResolver.ALL_USERS_WILDCARD)); + } ++ if (codeOwnerReference.email().startsWith(GROUP_PREFIX)) { ++ return OptionalResultWithMessages.createEmpty( ++ String.format( ++ "cannot resolve code owner email %s: this is a group", ++ codeOwnerReference.email())); ++ } + + ImmutableList.Builder<String> messageBuilder = ImmutableList.builder(); + AtomicBoolean ownedByAllUsers = new AtomicBoolean(false); +@@ -401,9 +425,53 @@ public class CodeOwnerResolver { + ImmutableMultimap<CodeOwnerReference, CodeOwnerAnnotation> annotations) { + requireNonNull(codeOwnerReferences, "codeOwnerReferences"); + ++ ImmutableSet<String> groupsToResolve = ++ codeOwnerReferences.stream() ++ .map(CodeOwnerReference::email) ++ .filter(ref -> ref.startsWith(GROUP_PREFIX)) ++ .map(ref -> ref.substring(GROUP_PREFIX.length())) ++ .collect(toImmutableSet()); ++ ++ // When we call GroupBackends.findExactSuggestion we need to ensure that we ++ // have a user in context. This is because the suggestion backend is ++ // likely to want to try to check that we can actually see the group it's ++ // returning (which we also check for explicitly, because I have trust ++ // issues). ++ RequestContext oldCtx = context.getContext(); ++ // Check if we have a user in the context at all... ++ try { ++ oldCtx.getUser(); ++ } catch (OutOfScopeException | NullPointerException e) { ++ // Nope. ++ RequestContext newCtx = () -> { ++ return new AnonymousUser(); ++ }; ++ context.setContext(newCtx); ++ } ++ ImmutableSetMultimap<String, CodeOwner> resolvedGroups = null; ++ try { ++ resolvedGroups = ++ groupsToResolve.stream() ++ .map(groupName -> GroupBackends.findExactSuggestion(groupBackend, groupName)) ++ .filter(groupRef -> groupRef != null) ++ .filter(groupRef -> groupBackend.isVisibleToAll(groupRef.getUUID())) ++ .map(groupRef -> groupBackend.get(groupRef.getUUID())) ++ .collect(flatteningToImmutableSetMultimap( ++ groupRef -> GROUP_PREFIX + groupRef.getName(), ++ groupRef -> accountCache ++ .get(ImmutableSet.copyOf(groupRef.getMembers())) ++ .values().stream() ++ .map(accountState -> CodeOwner.create(accountState.account().id())))); ++ } finally { ++ context.setContext(oldCtx); ++ } ++ ImmutableSetMultimap<CodeOwner, String> usersToGroups = ++ resolvedGroups.inverse(); ++ + ImmutableSet<String> emailsToResolve = + codeOwnerReferences.stream() + .map(CodeOwnerReference::email) ++ .filter(ref -> !ref.startsWith(GROUP_PREFIX)) + .filter(filterOutAllUsersWildCard(ownedByAllUsers)) + .collect(toImmutableSet()); + +@@ -438,7 +506,8 @@ public class CodeOwnerResolver { + ImmutableMap<String, CodeOwner> codeOwnersByEmail = + accountsByEmail.map(mapToCodeOwner()).collect(toImmutableMap(Pair::key, Pair::value)); + +- if (codeOwnersByEmail.keySet().size() < emailsToResolve.size()) { ++ if (codeOwnersByEmail.keySet().size() < emailsToResolve.size() || ++ resolvedGroups.keySet().size() < groupsToResolve.size()) { + hasUnresolvedCodeOwners.set(true); + } + +@@ -452,7 +521,9 @@ public class CodeOwnerResolver { + cachedCodeOwnersByEmail.entrySet().stream() + .filter(e -> e.getValue().isPresent()) + .map(e -> Pair.of(e.getKey(), e.getValue().get())); +- Streams.concat(newlyResolvedCodeOwnersStream, cachedCodeOwnersStream) ++ Stream<Pair<String, CodeOwner>> resolvedGroupsCodeOwnersStream = ++ resolvedGroups.entries().stream().map(e -> Pair.of(e.getKey(), e.getValue())); ++ Streams.concat(Streams.concat(newlyResolvedCodeOwnersStream, cachedCodeOwnersStream), resolvedGroupsCodeOwnersStream) + .forEach( + p -> { + ImmutableSet.Builder<CodeOwnerAnnotation> annotationBuilder = ImmutableSet.builder(); +@@ -463,6 +534,12 @@ public class CodeOwnerResolver { + annotationBuilder.addAll( + annotations.get(CodeOwnerReference.create(ALL_USERS_WILDCARD))); + ++ // annotations for the groups this user is in apply as well ++ for (String group : usersToGroups.get(p.value())) { ++ annotationBuilder.addAll( ++ annotations.get(CodeOwnerReference.create(group))); ++ } ++ + if (!codeOwnersWithAnnotations.containsKey(p.value())) { + codeOwnersWithAnnotations.put(p.value(), new HashSet<>()); + } +@@ -566,7 +643,7 @@ public class CodeOwnerResolver { + } + + messages.add(String.format("email %s has no domain", email)); +- return false; ++ return true; // TVL: we allow domain-less strings which are treated as usernames. + } + + /** +@@ -581,11 +658,29 @@ public class CodeOwnerResolver { + */ + private ImmutableMap<String, Collection<ExternalId>> lookupExternalIds( + ImmutableList.Builder<String> messages, ImmutableSet<String> emails) { ++ String[] actualEmails = emails.stream() ++ .filter(email -> email.contains("@")) ++ .toArray(String[]::new); ++ ImmutableSet<String> usernames = emails.stream() ++ .filter(email -> !email.contains("@")) ++ .collect(ImmutableSet.toImmutableSet()); + try { +- ImmutableMap<String, Collection<ExternalId>> extIdsByEmail = +- externalIds.byEmails(emails.toArray(new String[0])).asMap(); ++ ImmutableMap<String, Collection<ExternalId>> extIds = ++ new ImmutableMap.Builder<String, Collection<ExternalId>>() ++ .putAll(externalIds.byEmails(actualEmails).asMap()) ++ .putAll(externalIds.allByAccount().entries().stream() ++ .map(entry -> entry.getValue()) ++ .filter(externalId -> ++ externalId.key().scheme() != null && ++ externalId.key().isScheme(ExternalId.SCHEME_USERNAME) && ++ usernames.contains(externalId.key().id())) ++ .collect(toImmutableSetMultimap( ++ externalId -> externalId.key().id(), ++ externalId -> externalId)) ++ .asMap()) ++ .build(); + emails.stream() +- .filter(email -> !extIdsByEmail.containsKey(email)) ++ .filter(email -> !extIds.containsKey(email)) + .forEach( + email -> { + transientCodeOwnerCache.cacheNonResolvable(email); +@@ -594,7 +689,7 @@ public class CodeOwnerResolver { + "cannot resolve code owner email %s: no account with this email exists", + email)); + }); +- return extIdsByEmail; ++ return extIds; + } catch (IOException e) { + throw newInternalServerError( + String.format("cannot resolve code owner emails: %s", emails), e); +@@ -811,6 +906,15 @@ public class CodeOwnerResolver { + user != null ? user.getLoggableName() : currentUser.get().getLoggableName())); + return true; + } ++ if (!email.contains("@")) { ++ // the email is the username of the account, or a group, or something else. ++ messages.add( ++ String.format( ++ "account %s is visible to user %s", ++ accountState.account().id(), ++ user != null ? user.getLoggableName() : currentUser.get().getLoggableName())); ++ return true; ++ } + + if (user != null) { + if (user.hasEmailAddress(email)) { +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 5f350998..7977ba55 100644 +--- a/java/com/google/gerrit/plugins/codeowners/backend/findowners/FindOwnersCodeOwnerConfigParser.java ++++ b/java/com/google/gerrit/plugins/codeowners/backend/findowners/FindOwnersCodeOwnerConfigParser.java +@@ -149,7 +149,8 @@ public class FindOwnersCodeOwnerConfigParser implements CodeOwnerConfigParser { + 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<>@#,]+|\\*)"; ++ // Also allows usernames, and group:$GROUP_NAME. ++ private static final String EMAIL_OR_STAR = "([^\\s<>@,]+@[^\\s<>@#,]+?|\\*|[a-zA-Z0-9_\\-]+|group:[a-zA-Z0-9_\\-]+)"; + private static final String EMAIL_LIST = + "(" + EMAIL_OR_STAR + "(" + COMMA + EMAIL_OR_STAR + ")*)"; + +diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/AbstractFileBasedCodeOwnerBackendTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/AbstractFileBasedCodeOwnerBackendTest.java +index 7ec92959..59cf7e05 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 @@ public abstract class AbstractFileBasedCodeOwnerBackendTest extends AbstractCode + .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/CodeOwnerResolverTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverTest.java +index b32c3b5e..6b0f0cf8 100644 +--- a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverTest.java ++++ b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverTest.java +@@ -24,8 +24,10 @@ import com.google.gerrit.acceptance.TestAccount; + import com.google.gerrit.acceptance.TestMetricMaker; + import com.google.gerrit.acceptance.config.GerritConfig; + import com.google.gerrit.acceptance.testsuite.account.AccountOperations; ++import com.google.gerrit.acceptance.testsuite.group.GroupOperations; + import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations; + import com.google.gerrit.entities.Account; ++import com.google.gerrit.entities.AccountGroup; + import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersTest; + import com.google.gerrit.server.ServerInitiated; + import com.google.gerrit.server.account.AccountsUpdate; +@@ -51,6 +53,7 @@ public class CodeOwnerResolverTest extends AbstractCodeOwnersTest { + @Inject private RequestScopeOperations requestScopeOperations; + @Inject @ServerInitiated private Provider<AccountsUpdate> accountsUpdate; + @Inject private AccountOperations accountOperations; ++ @Inject private GroupOperations groupOperations; + @Inject private ExternalIdNotes.Factory externalIdNotesFactory; + @Inject private TestMetricMaker testMetricMaker; + @Inject private ExternalIdFactory externalIdFactory; +@@ -112,6 +115,18 @@ public class CodeOwnerResolverTest extends AbstractCodeOwnersTest { + .contains(String.format("account %s is visible to user %s", admin.id(), admin.username())); + } + ++ @Test ++ public void resolveCodeOwnerReferenceForUsername() throws Exception { ++ OptionalResultWithMessages<CodeOwner> result = ++ codeOwnerResolverProvider ++ .get() ++ .resolveWithMessages(CodeOwnerReference.create(admin.username())); ++ assertThat(result.get()).hasAccountIdThat().isEqualTo(admin.id()); ++ assertThat(result) ++ .hasMessagesThat() ++ .contains(String.format("account %s is visible to user %s", admin.id(), admin.username())); ++ } ++ + @Test + public void cannotResolveCodeOwnerReferenceForStarAsEmail() throws Exception { + OptionalResultWithMessages<CodeOwner> result = +@@ -127,6 +142,18 @@ public class CodeOwnerResolverTest extends AbstractCodeOwnersTest { + CodeOwnerResolver.ALL_USERS_WILDCARD)); + } + ++ @Test ++ public void cannotResolveCodeOwnerReferenceForGroup() throws Exception { ++ OptionalResultWithMessages<CodeOwner> result = ++ codeOwnerResolverProvider ++ .get() ++ .resolveWithMessages(CodeOwnerReference.create("group:Administrators")); ++ assertThat(result).isEmpty(); ++ assertThat(result) ++ .hasMessagesThat() ++ .contains("cannot resolve code owner email group:Administrators: this is a group"); ++ } ++ + @Test + public void resolveCodeOwnerReferenceForAmbiguousEmailIfOtherAccountIsInactive() + throws Exception { +@@ -391,6 +418,64 @@ public class CodeOwnerResolverTest extends AbstractCodeOwnersTest { + assertThat(result.hasUnresolvedCodeOwners()).isFalse(); + } + ++ @Test ++ public void resolvePathCodeOwnersWhenNonVisibleGroupIsUsed() throws Exception { ++ CodeOwnerConfig codeOwnerConfig = ++ CodeOwnerConfig.builder(CodeOwnerConfig.Key.create(project, "master", "/"), TEST_REVISION) ++ .addCodeOwnerSet( ++ CodeOwnerSet.createWithoutPathExpressions("group:Administrators")) ++ .build(); ++ ++ CodeOwnerResolverResult result = ++ codeOwnerResolverProvider ++ .get() ++ .resolvePathCodeOwners(codeOwnerConfig, Paths.get("/README.md")); ++ assertThat(result.codeOwnersAccountIds()).isEmpty(); ++ assertThat(result.ownedByAllUsers()).isFalse(); ++ assertThat(result.hasUnresolvedCodeOwners()).isTrue(); ++ } ++ ++ @Test ++ public void resolvePathCodeOwnersWhenVisibleGroupIsUsed() throws Exception { ++ AccountGroup.UUID createdGroupUUID = groupOperations ++ .newGroup() ++ .name("VisibleGroup") ++ .visibleToAll(true) ++ .addMember(admin.id()) ++ .create(); ++ ++ CodeOwnerConfig codeOwnerConfig = ++ CodeOwnerConfig.builder(CodeOwnerConfig.Key.create(project, "master", "/"), TEST_REVISION) ++ .addCodeOwnerSet( ++ CodeOwnerSet.createWithoutPathExpressions("group:VisibleGroup")) ++ .build(); ++ ++ CodeOwnerResolverResult result = ++ codeOwnerResolverProvider ++ .get() ++ .resolvePathCodeOwners(codeOwnerConfig, Paths.get("/README.md")); ++ assertThat(result.codeOwnersAccountIds()).containsExactly(admin.id()); ++ assertThat(result.ownedByAllUsers()).isFalse(); ++ assertThat(result.hasUnresolvedCodeOwners()).isFalse(); ++ } ++ ++ @Test ++ public void resolvePathCodeOwnersWhenUsernameIsUsed() throws Exception { ++ CodeOwnerConfig codeOwnerConfig = ++ CodeOwnerConfig.builder(CodeOwnerConfig.Key.create(project, "master", "/"), TEST_REVISION) ++ .addCodeOwnerSet( ++ CodeOwnerSet.createWithoutPathExpressions(admin.username())) ++ .build(); ++ ++ CodeOwnerResolverResult result = ++ codeOwnerResolverProvider ++ .get() ++ .resolvePathCodeOwners(codeOwnerConfig, Paths.get("/README.md")); ++ assertThat(result.codeOwnersAccountIds()).containsExactly(admin.id()); ++ assertThat(result.ownedByAllUsers()).isFalse(); ++ assertThat(result.hasUnresolvedCodeOwners()).isFalse(); ++ } ++ + @Test + public void resolvePathCodeOwnersNonResolvableCodeOwnersAreFilteredOut() throws Exception { + CodeOwnerConfig codeOwnerConfig = +@@ -649,7 +734,7 @@ public class CodeOwnerResolverTest extends AbstractCodeOwnersTest { + "domain example.com of email foo@example.org@example.com is allowed"); + assertIsEmailDomainAllowed( + "foo@example.org", false, "domain example.org of email foo@example.org is not allowed"); +- assertIsEmailDomainAllowed("foo", false, "email foo has no domain"); ++ assertIsEmailDomainAllowed("foo", true, "email foo has no domain"); + assertIsEmailDomainAllowed( + "foo@example.com@example.org", + false, +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 260e635e..7aab99d0 100644 +--- a/javatests/com/google/gerrit/plugins/codeowners/backend/findowners/FindOwnersCodeOwnerConfigParserTest.java ++++ b/javatests/com/google/gerrit/plugins/codeowners/backend/findowners/FindOwnersCodeOwnerConfigParserTest.java +@@ -158,16 +158,42 @@ public class FindOwnersCodeOwnerConfigParserTest extends AbstractCodeOwnerConfig + 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!\n" ++ + " invalid line: NOT!AN_EMAIL", + project)); + } + ++ @Test ++ public void codeOwnerConfigWithUsernames() throws Exception { ++ assertParseAndFormat( ++ getCodeOwnerConfig(EMAIL_1, "USERNAME", EMAIL_2), ++ codeOwnerConfig -> ++ assertThat(codeOwnerConfig) ++ .hasCodeOwnerSetsThat() ++ .onlyElement() ++ .hasCodeOwnersEmailsThat() ++ .containsExactly(EMAIL_1, "USERNAME", EMAIL_2), ++ getCodeOwnerConfig(EMAIL_1, "USERNAME", EMAIL_2)); ++ } ++ ++ @Test ++ public void codeOwnerConfigWithGroups() throws Exception { ++ assertParseAndFormat( ++ getCodeOwnerConfig(EMAIL_1, "group:tvl-employees", EMAIL_2), ++ codeOwnerConfig -> ++ assertThat(codeOwnerConfig) ++ .hasCodeOwnerSetsThat() ++ .onlyElement() ++ .hasCodeOwnersEmailsThat() ++ .containsExactly(EMAIL_1, "group:tvl-employees", EMAIL_2), ++ getCodeOwnerConfig(EMAIL_1, "group:tvl-employees", EMAIL_2)); ++ } ++ + @Test + public void codeOwnerConfigWithComment() throws Exception { + assertParseAndFormat( +-- +2.37.3 + |