From ae98240df2f9a9d11be85d61697532e17c1ed5f8 Mon Sep 17 00:00:00 2001 From: Luke Granger-Brown Date: Mon, 19 Sep 2022 05:25:29 +0000 Subject: feat(gerrit): add code-owners plugin This is the New Thing that is intended to replace the find-owners and owners plugins. In particular: * It inserts a submit requirement rather than providing a Prolog predicate. * The default OWNERS file formats are suspiciously Googley. * It provides a neat UI for finding OWNERS and tracking approval state on a per-file basis. When we fully migrate to using the code-owners plugin, a few things will need to land, which I will likely do "offline" directly to the Gerrit backing Git repos: * Add the corresponding Gerrit config * Replace OWNERS files depot-wide * Add OWNERS files to the refs/meta/config branch * Introduce the Owners-Override label, settable by depot-interventions The enclosed patch adds two extra pieces of functionality that we need in tvldepot but aren't upstream: 1. The ability to just specify usernames rather than email addresses 2. The ability to specify `group:GROUPNAME`, _as long as_ that group is visible to everyone. This is a restriction intended to avoid having the plugin just leak group membership. Change-Id: I27d92b6cb7449af83030b9015f09a1571aa8452f Reviewed-on: https://cl.tvl.fyi/c/depot/+/6664 Tested-by: BuildkiteCI Reviewed-by: tazjin Reviewed-by: sterni --- third_party/gerrit_plugins/builder.nix | 18 +- third_party/gerrit_plugins/code-owners/default.nix | 17 + .../code-owners/using-usernames.patch | 425 +++++++++++++++++++++ 3 files changed, 453 insertions(+), 7 deletions(-) create mode 100644 third_party/gerrit_plugins/code-owners/default.nix create mode 100644 third_party/gerrit_plugins/code-owners/using-usernames.patch diff --git a/third_party/gerrit_plugins/builder.nix b/third_party/gerrit_plugins/builder.nix index 0b6501801cd1..90859756201f 100644 --- a/third_party/gerrit_plugins/builder.nix +++ b/third_party/gerrit_plugins/builder.nix @@ -1,4 +1,4 @@ -{ depot, pkgs, ... }: +{ depot, lib, pkgs, ... }: { buildGerritBazelPlugin = { name @@ -8,7 +8,7 @@ cp -R "${src}" "$out/plugins/${name}" '' , postPatch ? "" - , + , patches ? [ ] }: ((depot.third_party.gerrit.override { name = "${name}.jar"; @@ -26,10 +26,14 @@ installPhase = '' cp "bazel-bin/plugins/${name}/${name}.jar" "$out" ''; - postPatch = - if super ? postPatch then '' - ${super.postPatch} - ${postPatch} - '' else postPatch; + postPatch = '' + ${super.postPatch or ""} + pushd "plugins/${name}" + ${lib.concatMapStringsSep "\n" (patch: '' + patch -p1 < ${patch} + '') patches} + popd + ${postPatch} + ''; })); } 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..377a2a0471cd --- /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:0fpv5yavgki5nv84lg5zykp6v7pv9xll1glmz5dwnz5z11axj4g9"; + 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..738e88f771d7 --- /dev/null +++ b/third_party/gerrit_plugins/code-owners/using-usernames.patch @@ -0,0 +1,425 @@ +diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java +index 07894ced..6a528b50 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; +@@ -38,6 +41,9 @@ 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; +@@ -103,6 +109,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; +@@ -113,6 +121,7 @@ public class CodeOwnerResolver { + private final CodeOwnerMetrics codeOwnerMetrics; + private final UnresolvedImportFormatter unresolvedImportFormatter; + private final TransientCodeOwnerCache transientCodeOwnerCache; ++ private final InternalGroupBackend groupBackend; + + // Enforce visibility by default. + private boolean enforceVisibility = true; +@@ -133,7 +142,8 @@ public class CodeOwnerResolver { + PathCodeOwners.Factory pathCodeOwnersFactory, + CodeOwnerMetrics codeOwnerMetrics, + UnresolvedImportFormatter unresolvedImportFormatter, +- TransientCodeOwnerCache transientCodeOwnerCache) { ++ TransientCodeOwnerCache transientCodeOwnerCache, ++ InternalGroupBackend groupBackend) { + this.codeOwnersPluginConfiguration = codeOwnersPluginConfiguration; + this.permissionBackend = permissionBackend; + this.currentUser = currentUser; +@@ -144,6 +154,7 @@ public class CodeOwnerResolver { + this.codeOwnerMetrics = codeOwnerMetrics; + this.unresolvedImportFormatter = unresolvedImportFormatter; + this.transientCodeOwnerCache = transientCodeOwnerCache; ++ this.groupBackend = groupBackend; + } + + /** +@@ -357,6 +368,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 messageBuilder = ImmutableList.builder(); + AtomicBoolean ownedByAllUsers = new AtomicBoolean(false); +@@ -401,9 +418,32 @@ public class CodeOwnerResolver { + ImmutableMultimap annotations) { + requireNonNull(codeOwnerReferences, "codeOwnerReferences"); + ++ ImmutableSet groupsToResolve = ++ codeOwnerReferences.stream() ++ .map(CodeOwnerReference::email) ++ .filter(ref -> ref.startsWith(GROUP_PREFIX)) ++ .map(ref -> ref.substring(GROUP_PREFIX.length())) ++ .collect(toImmutableSet()); ++ ++ ImmutableSetMultimap 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())))); ++ ImmutableSetMultimap usersToGroups = ++ resolvedGroups.inverse(); ++ + ImmutableSet emailsToResolve = + codeOwnerReferences.stream() + .map(CodeOwnerReference::email) ++ .filter(ref -> !ref.startsWith(GROUP_PREFIX)) + .filter(filterOutAllUsersWildCard(ownedByAllUsers)) + .collect(toImmutableSet()); + +@@ -438,7 +478,8 @@ public class CodeOwnerResolver { + ImmutableMap 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 +493,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> resolvedGroupsCodeOwnersStream = ++ resolvedGroups.entries().stream().map(e -> Pair.of(e.getKey(), e.getValue())); ++ Streams.concat(Streams.concat(newlyResolvedCodeOwnersStream, cachedCodeOwnersStream), resolvedGroupsCodeOwnersStream) + .forEach( + p -> { + ImmutableSet.Builder annotationBuilder = ImmutableSet.builder(); +@@ -463,6 +506,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 +615,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 +630,29 @@ public class CodeOwnerResolver { + */ + private ImmutableMap> lookupExternalIds( + ImmutableList.Builder messages, ImmutableSet emails) { ++ String[] actualEmails = emails.stream() ++ .filter(email -> email.contains("@")) ++ .toArray(String[]::new); ++ ImmutableSet usernames = emails.stream() ++ .filter(email -> !email.contains("@")) ++ .collect(ImmutableSet.toImmutableSet()); + try { +- ImmutableMap> extIdsByEmail = +- externalIds.byEmails(emails.toArray(new String[0])).asMap(); ++ ImmutableMap> extIds = ++ new ImmutableMap.Builder>() ++ .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 +661,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 +878,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; + @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 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 result = +@@ -127,6 +142,18 @@ public class CodeOwnerResolverTest extends AbstractCodeOwnersTest { + CodeOwnerResolver.ALL_USERS_WILDCARD)); + } + ++ @Test ++ public void cannotResolveCodeOwnerReferenceForGroup() throws Exception { ++ OptionalResultWithMessages 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( -- cgit 1.4.1