Repository: rust-lang/highfive

Issue:
highfive should not assign someone to their own PR
In https://github.com/rust-lang/rust/pull/84257 , rust-highfive assigned me to review my own PR.

Discussion:

Patch:
diff --git a/highfive/configs/_global.json b/highfive/configs/_global.json
index 2f8bfeb..8347f0b 100644
--- a/highfive/configs/_global.json
+++ b/highfive/configs/_global.json
@@ -1,7 +1,7 @@
 {
     "groups": {
         "core": ["@Mark-Simulacrum"],
-        "crates": ["@alexcrichton"],
+        "crates": [],
         "doc": ["@steveklabnik", "@GuillaumeGomez"],
         "rust-embedded/cortex-a": ["@andre-richter", "@raw-bin"],
         "rust-embedded/cortex-m": ["@adamgreig", "@therealprof", "@jonas-schievink", "@thalesfragoso"],
diff --git a/highfive/configs/rust-lang/cargo.json b/highfive/configs/rust-lang/cargo.json
index 0927be5..7f1f489 100644
--- a/highfive/configs/rust-lang/cargo.json
+++ b/highfive/configs/rust-lang/cargo.json
@@ -1,6 +1,6 @@
 {
     "groups": {
-        "all": ["@alexcrichton", "@ehuss", "@Eh2406"]
+        "all": ["@ehuss"]
     },
     "contributing": "https://github.com/rust-lang/cargo/blob/master/CONTRIBUTING.md",
     "new_pr_labels": ["S-waiting-on-review"]
diff --git a/highfive/configs/rust-lang/crates.io.json b/highfive/configs/rust-lang/crates.io.json
deleted file mode 100644
index fe9da3c..0000000
--- a/highfive/configs/rust-lang/crates.io.json
+++ /dev/null
@@ -1,18 +0,0 @@
-{
-    "groups": {
-        "all": ["@jtgeibel", "@carols10cents", "@smarnach"],
-        "ember": ["@locks", "@Turbo87"]
-    },
-    "dirs": {
-        "package.json": ["ember"],
-        "package-lock.json": ["ember"],
-        "app": ["ember"],
-        "mirage": ["ember"],
-        "public": ["ember"],
-        "tests": ["ember"],
-        "src": ["all", "@JohnTitor"],
-        "Cargo.lock": ["@JohnTitor"]
-    },
-    "contributing": "https://github.com/rust-lang/crates.io/blob/master/docs/CONTRIBUTING.md",
-    "new_pr_labels": ["S-waiting-on-review"]
-}
diff --git a/highfive/configs/rust-lang/libc.json b/highfive/configs/rust-lang/libc.json
index 78afeff..3125935 100644
--- a/highfive/configs/rust-lang/libc.json
+++ b/highfive/configs/rust-lang/libc.json
@@ -1,6 +1,6 @@
 {
     "groups": {
-        "all": ["@JohnTitor", "@Amanieu"]
+        "all": ["@Amanieu"]
     },
     "dirs": {
     },
@@ -8,6 +8,14 @@
         "src/unix/bsd/netbsdlike/openbsd": {
             "message": "Some changes occurred in OpenBSD module",
             "reviewers": ["@semarie"]
+        },
+        "src/unix/bsd/netbsdlike/mod.rs": {
+            "message": "Some changes occurred in OpenBSD module",
+            "reviewers": ["@semarie"]
+        },
+        "src/unix/solarish": {
+            "message": "Some changes occurred in solarish module",
+            "reviewers": ["@jclulow", "@pfmooney"]
         }
     },
     "contributing": "https://github.com/rust-lang/libc/blob/master/CONTRIBUTING.md",
diff --git a/highfive/configs/rust-lang/rust-installer.json b/highfive/configs/rust-lang/rust-installer.json
index 6c91436..4d95770 100644
--- a/highfive/configs/rust-lang/rust-installer.json
+++ b/highfive/configs/rust-lang/rust-installer.json
@@ -1,6 +1,6 @@
 {
     "groups": {
-        "all": ["@alexcrichton"]
+        "all": []
     },
     "dirs": {
     }
diff --git a/highfive/configs/rust-lang/rust.json b/highfive/configs/rust-lang/rust.json
index 404dde7..1e51742 100644
--- a/highfive/configs/rust-lang/rust.json
+++ b/highfive/configs/rust-lang/rust.json
@@ -7,20 +7,33 @@
             "@petrochenkov",
             "@davidtwco",
             "@oli-obk",
+            "@lcnr",
             "@michaelwoerister",
             "@nagisa",
             "@wesleywiser"
         ],
         "compiler-team-contributors": [
             "@jackh726",
-            "@cjgillot"
+            "@cjgillot",
+            "@compiler-errors",
+            "@fee1-dead"
+        ],
+        "compiler": [
+            "compiler-team",
+            "compiler-team-contributors"
         ],
         "libs": [
-            "@dtolnay",
             "@joshtriplett",
             "@Mark-Simulacrum",
             "@kennytm",
             "@yaahc",
+            "@m-ou-se",
+            "@thomcc"
+        ],
+        "libs-api": [
+            "@dtolnay",
+            "@joshtriplett",
+            "@yaahc",
             "@m-ou-se"
         ],
         "infra-ci": [
@@ -29,54 +42,113 @@
             "@pietroalbini"
         ],
         "rustdoc": [
+            "@jsha",
             "@GuillaumeGomez",
-            "@ollie27",
-            "@CraftSpider"
+            "@CraftSpider",
+            "@notriddle"
+        ],
+        "query-system": [
+            "@cjgillot"
+        ],
+        "incremental": [
+            "@michaelwoerister",
+            "@wesleywiser"
+        ],
+        "typeck": [],
+        "diagnostics": [
+            "@davidtwco",
+            "@estebank",
+            "@oli-obk"
+        ],
+        "parser": [
+            "@davidtwco",
+            "@estebank",
+            "@petrochenkov"
+        ],
+        "lexer": [
+            "@petrochenkov"
+        ],
+        "mir": [
+            "@davidtwco",
+            "@oli-obk"
+        ],
+        "mir-opt": [
+            "@nagisa",
+            "@oli-obk",
+            "@wesleywiser"
+        ],
+        "traits": [
+            "@jackh726",
+            "@matthewjasper"
+        ],
+        "borrowck": [
+            "@davidtwco",
+            "@matthewjasper",
+            "@pnkfelix"
         ]
     },
     "dirs": {
-        ".github/workflows":        ["infra-ci"],
-        "Cargo.lock":               ["@Mark-Simulacrum"],
-        "Cargo.toml":               ["@Mark-Simulacrum"],
-        "compiler":                 ["compiler-team", "compiler-team-contributors"],
-        "compiler/rustc_apfloat":   ["@eddyb"],
-        "compiler/rustc_llvm":      ["@cuviper"],
-        "library/alloc":            ["libs"],
-        "library/core":             ["libs", "@scottmcm"],
-        "library/panic_abort":      ["libs"],
-        "library/panic_unwind":     ["libs"],
-        "library/proc_macro":       ["@petrochenkov"],
-        "library/std":              ["libs"],
-        "library/stdarch":          ["libs"],
-        "library/term":             ["libs"],
-        "library/test":             ["libs"],
-        "src/bootstrap":            ["@Mark-Simulacrum"],
-        "src/build_helper":         ["@Mark-Simulacrum"],
-        "src/ci":                   ["infra-ci"],
-        "src/doc":                  ["doc"],
-        "src/etc":                  ["@Mark-Simulacrum"],
-        "src/librustdoc":           ["rustdoc"],
-        "src/llvm-project":         ["@cuviper"],
-        "src/stage0.txt":           ["@Mark-Simulacrum"],
-        "src/tools/cargo":          ["@ehuss", "@joshtriplett"],
-        "src/tools/compiletest":    ["@Mark-Simulacrum", "@Centril"],
-        "src/tools/linkchecker":    ["@ehuss"],
-        "src/tools/rust-installer": ["@Mark-Simulacrum"],
-        "src/tools/rustbook":       ["doc", "@ehuss"],
-        "src/tools/rustdoc":        ["rustdoc"],
-        "src/tools/rustdoc-js":     ["rustdoc"],
-        "src/tools/rustdoc-js-std": ["rustdoc"],
-        "src/tools/rustdoc-themes": ["rustdoc"],
-        "src/tools/tidy":           ["@Mark-Simulacrum"]
+        ".github/workflows":                     ["infra-ci"],
+        "Cargo.lock":                            ["@Mark-Simulacrum"],
+        "Cargo.toml":                            ["@Mark-Simulacrum"],
+        "compiler":                              ["compiler"],
+        "compiler/rustc_apfloat":                ["@eddyb"],
+        "compiler/rustc_ast/":                   ["compiler", "parser"],
+        "compiler/rustc_lexer":                  ["compiler", "lexer"],
+        "compiler/rustc_llvm":                   ["@cuviper"],
+        "compiler/rustc_middle/src/mir":         ["compiler", "mir"],
+        "compiler/rustc_middle/src/traits":      ["compiler", "traits"],
+        "compiler/rustc_mir/src/interpret":      ["compiler", "mir"],
+        "compiler/rustc_mir/src/transform":      ["compiler", "mir-opt"],
+        "compiler/rustc_mir_build/src/build":    ["compiler", "mir"],
+        "compiler/rustc_typeck":                 ["compiler", "typeck"],
+        "compiler/rustc_traits":                 ["compiler", "traits"],
+        "compiler/rustc_trait_selection":        ["compiler", "traits"],
+        "compiler/rustc_parse":                  ["compiler", "parser"],
+        "compiler/rustc_parse/src/parse/lexer/": ["compiler", "lexer"],
+        "compiler/rustc_query_impl":             ["compiler", "query-system"],
+        "compiler/rustc_query_system":           ["compiler", "query-system"],
+        "library/alloc":                         ["libs"],
+        "library/core":                          ["libs", "@scottmcm"],
+        "library/panic_abort":                   ["libs"],
+        "library/panic_unwind":                  ["libs"],
+        "library/proc_macro":                    ["@petrochenkov"],
+        "library/std":                           ["libs"],
+        "library/std/src/sys/windows":           ["@ChrisDenton"],
+        "library/stdarch":                       ["libs"],
+        "library/term":                          ["libs"],
+        "library/test":                          ["libs"],
+        "src/bootstrap":                         ["@Mark-Simulacrum"],
+        "src/build_helper":                      ["@Mark-Simulacrum"],
+        "src/ci":                                ["infra-ci"],
+        "src/doc":                               ["doc"],
+        "src/etc":                               ["@Mark-Simulacrum"],
+        "src/librustdoc":                        ["rustdoc"],
+        "src/llvm-project":                      ["@cuviper"],
+        "src/stage0.txt":                        ["@Mark-Simulacrum"],
+        "src/tools/cargo":                       ["@ehuss", "@joshtriplett"],
+        "src/tools/compiletest":                 ["@Mark-Simulacrum"],
+        "src/tools/linkchecker":                 ["@ehuss"],
+        "src/tools/rust-installer":              ["@Mark-Simulacrum"],
+        "src/tools/rustbook":                    ["doc", "@ehuss"],
+        "src/tools/rustdoc":                     ["rustdoc"],
+        "src/tools/rustdoc-js":                  ["rustdoc"],
+        "src/tools/rustdoc-js-std":              ["rustdoc"],
+        "src/tools/rustdoc-themes":              ["rustdoc"],
+        "src/tools/tidy":                        ["@Mark-Simulacrum"]
     },
     "mentions": {
         "src/rustdoc-json-types": {
-          "message": "rustdoc-json-types is a **public** (although nightly-only) API. Consider changing `src/librustdoc/json/conversions.rs` instead; otherwise, make sure you update `format_version`.",
+          "message": "rustdoc-json-types is a **public** (although nightly-only) API. If possible, consider changing `src/librustdoc/json/conversions.rs`; otherwise, make sure you bump the `FORMAT_VERSION` constant.",
           "reviewers": ["@CraftSpider", "@aDotInTheVoid"]
         },
+        "src/librustdoc/clean/types.rs": {
+            "message": "Some changes occurred in `clean/types.rs`.",
+            "reviewers": ["@camelid"]
+        },
         "src/librustdoc/html/static": {
             "message": "Some changes occurred in HTML/CSS/JS.",
-            "reviewers": ["@GuillaumeGomez"]
+            "reviewers": ["@GuillaumeGomez", "@Folyd", "@jsha"]
         },
         "src/librustdoc/html/static/themes": {
             "message": "Some changes occurred in HTML/CSS themes.",
@@ -86,10 +158,6 @@
             "message": "A change occurred in the Ayu theme.",
             "reviewers": ["@Cldfire"]
         },
-        "src/librustdoc/passes/collect_intra_doc_links.rs": {
-            "message": "Some changes occurred in intra-doc-links.",
-            "reviewers": ["@jyn514"]
-        },
         "error_codes.rs": {
             "message": "Some changes occurred in diagnostic error codes",
             "reviewers": ["@GuillaumeGomez"]
@@ -106,15 +174,19 @@
             "message": "Some changes occured to rustc_codegen_cranelift",
             "reviewers": ["@bjorn3"]
         },
+        "compiler/rustc_codegen_gcc": {
+            "message": "Some changes occured to rustc_codegen_gcc",
+            "reviewers": ["@antoyo"]
+        },
         "src/tools/rustfmt": {
             "message": "Some changes occurred in src/tools/rustfmt.",
-            "reviewers": ["@calebcartwright"]
+            "reviewers": ["@rust-lang/rustfmt"]
         },
         "src/tools/clippy": {
             "message": "Some changes occurred in src/tools/clippy.",
             "reviewers": ["@rust-lang/clippy"]
         },
-        "compiler/rustc_mir/src/interpret": {
+        "compiler/rustc_const_eval/src/interpret": {
             "message": "Some changes occured to the CTFE / Miri engine",
             "reviewers": ["@rust-lang/miri"]
         },
@@ -125,6 +197,10 @@
         "src/tools/miri": {
             "message": "Some changes occured to the Miri submodule",
             "reviewers": ["@rust-lang/miri"]
+        },
+        "library": {
+            "message": "Hey! It looks like you've submitted a new PR for the library teams!\n\nIf this PR contains changes to any `rust-lang/rust` public library APIs then please comment with `r? rust-lang/libs-api @rustbot label +T-libs-api` to request review from a libs-api team reviewer. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.\n\nExamples of `T-libs-api` changes:\n\n* a stabilization of a library feature\n* introducing new or changes existing unstable library APIs\n* changes to  public documentation in ways that create new stability guarantees",
+            "reviewers": []
         }
     },
     "new_pr_labels": ["S-waiting-on-review"]
diff --git a/highfive/configs/rust-lang/std-dev-guide.json b/highfive/configs/rust-lang/std-dev-guide.json
new file mode 100644
index 0000000..999e83b
--- /dev/null
+++ b/highfive/configs/rust-lang/std-dev-guide.json
@@ -0,0 +1,12 @@
+{
+    "groups": {
+        "all": [
+            "@joshtriplett",
+            "@yaahc",
+            "cuviper",
+            "Amanieu",
+            "@m-ou-se"
+        ]
+    },
+    "new_pr_labels": ["S-waiting-on-review"]
+}
diff --git a/highfive/newpr.py b/highfive/newpr.py
index b0a741b..2919460 100644
--- a/highfive/newpr.py
+++ b/highfive/newpr.py
@@ -33,14 +33,10 @@ surprise_branch_warning = "Pull requests are usually filed against the %s branch
 review_with_reviewer = 'r? @%s\n\n(rust-highfive has picked a reviewer for you, use r? to override)'
 review_without_reviewer = '@%s: no appropriate reviewer found, use r? to override'
 
-reviewer_re = re.compile("\\b[rR]\?[:\- ]*@([a-zA-Z0-9\-]+)")
-reviewer_group_re = re.compile("\\b[rR]\?[:\- ]*@?(?:([a-zA-Z0-9\-]+)/)([a-zA-Z0-9\-]+)")
-submodule_re = re.compile(".*\+Subproject\scommit\s.*", re.DOTALL | re.MULTILINE)
+reviewer_re = re.compile(r"\b[rR]\?[:\- ]*(?:@?([a-zA-Z0-9\-]+)/)?(@?[a-zA-Z0-9\-]+)")
+submodule_re = re.compile(r".*\+Subproject\scommit\s.*", re.DOTALL | re.MULTILINE)
 target_re = re.compile("^[+-]{3} [ab]/compiler/rustc_target/src/spec/", re.MULTILINE)
 
-rustaceans_api_url = "http://www.ncameron.org/rustaceans/user?username={username}"
-
-
 class UnsupportedRepoError(IOError):
     pass
 
@@ -68,10 +64,13 @@ class HighfiveHandler(object):
             return "Ping received! The webhook is configured correctly!\n"
         elif event == "pull_request" and self.payload["action"] == "opened":
             self.new_pr()
-            return 'OK\n'
+            return 'OK, handled new PR\n'
         elif event == "issue_comment" and self.payload["action"] == "created":
-            self.new_comment()
-            return 'OK\n'
+            msg = self.new_comment()
+            if msg is None:
+                return 'OK\n'
+            else:
+                return f"OK: {msg}\n"
         else:
             return 'Unsupported webhook event.\n'
 
@@ -130,11 +129,19 @@ class HighfiveHandler(object):
                 if len(message) > 0:
                     message += '\n\n'
                 msg = mention.get('message')
+                reviewers = [x for x in mention['reviewers'] if x != user]
+
                 if msg is not None:
-                    msg += '\n\n'
+                    if len(reviewers) > 0:
+                        msg += '\n\n'
                 else:
                     msg = ''
-                message += "%scc %s" % (msg, ','.join([x for x in mention['reviewers'] if x != user]))
+
+                if len(reviewers) > 0:
+                    message += "%scc %s" % (msg, ','.join(reviewers))
+                else:
+                    message += msg
+
                 cmd = mention.get('command')
                 if cmd is not None:
                     commands[cmd] = self.payload['pull_request', 'head', 'sha']
@@ -232,7 +239,7 @@ class HighfiveHandler(object):
                 raise e
 
     def get_groups(self):
-        groups = deepcopy(self.repo_config['groups'])
+        groups = deepcopy(self.repo_config['groups'] if 'groups' in self.repo_config else {})
 
         # fill in the default groups, ensuring that overwriting is an
         # error.
@@ -249,16 +256,16 @@ class HighfiveHandler(object):
         None.
         """
         if msg is not None:
-            match = reviewer_group_re.search(msg)
+            match = reviewer_re.search(msg)
             if match:
                 groups = self.get_groups()
                 potential = groups.get(match.group(2)) or groups.get("%s/%s" % (match.group(1), match.group(2))) or []
-                potential.extend(groups["all"])
-                return self.pick_reviewer(groups, potential, exclude)
-
-            match = reviewer_re.search(msg)
-            if match:
-                return match.group(1)
+                picked = self.pick_reviewer(groups, potential, exclude)
+                if picked:
+                    return picked
+                if match.group(1) is None and match.group(2):
+                    if match.group(2).startswith('@'):
+                        return match.group(2)[1:]
 
 
     def choose_reviewer(self, repo, owner, diff, exclude):
@@ -318,7 +325,15 @@ class HighfiveHandler(object):
             p = potential.pop()
             if p.startswith('@'):
                 # remove the '@' prefix from each username
-                reviewers.append(p[1:])
+                username = p[1:]
+
+                # If no one should be excluded add the reviewer
+                if exclude == None:
+                    reviewers.append(username)
+
+                # ensure we don't assign someone to their own PR due with a case-insensitive test
+                elif username.lower() != exclude.lower():
+                    reviewers.append(username)
             elif p in groups:
                 # avoid infinite loops
                 assert p not in seen, "group %s refers to itself" % p
@@ -326,9 +341,6 @@ class HighfiveHandler(object):
                 # we allow groups in groups, so they need to be queued to be resolved
                 potential.extend(groups[p])
 
-        if exclude in reviewers:
-            reviewers.remove(exclude)
-
         if reviewers:
             random.seed()
             return random.choice(reviewers)
@@ -433,12 +445,12 @@ class HighfiveHandler(object):
         # Check the issue is a PR and is open.
         if self.payload['issue', 'state'] != 'open' \
                 or 'pull_request' not in self.payload['issue']:
-            return
+            return "skipped - closed issue"
 
         commenter = self.payload['comment', 'user', 'login']
         # Ignore our own comments.
         if commenter == self.integration_user:
-            return
+            return "skipped - our own comment"
 
         owner = self.payload['repository', 'owner', 'login']
         repo = self.payload['repository', 'name']
@@ -451,7 +463,7 @@ class HighfiveHandler(object):
         )):
             # Check if commenter is a collaborator.
             if not self.is_collaborator(commenter, owner, repo):
-                return
+                return "skipped, comment not by author, collaborator, or assignee"
 
         # Check for r? and set the assignee.
         msg = self.payload['comment', 'body']
@@ -462,3 +474,6 @@ class HighfiveHandler(object):
                 reviewer, owner, repo, issue, self.integration_user,
                 author, None
             )
+            return f"set assignee to {reviewer}"
+        else:
+            return "no reviewer found"


Test patch:
diff --git a/highfive/tests/fakes.py b/highfive/tests/fakes.py
index 7fb5604..10cbdad 100644
--- a/highfive/tests/fakes.py
+++ b/highfive/tests/fakes.py
@@ -44,6 +44,15 @@ def get_repo_configs():
                 "some": ["all"],
             },
         },
+        'nested_groups': {
+            "groups": {
+                "all": [],
+                "a": ["@pnkfelix"],
+                "b": ["@nrc"],
+                "c": ["a", "b"]
+            },
+            "dirs": {"src/librustc_typeck": ["c"]},
+        },
         'empty': {
             "groups": {"all": []},
             "dirs": {},
@@ -77,7 +86,7 @@ def get_repo_configs():
             }
         },
         'teams': {
-            "groups": {"all": [], "a": ["@pnkfelix"], "b/c": ["@nrc"]}
+            "groups": {"all": ["@ehuss"], "a": ["@pnkfelix"], "d": ["@e"], "compiler-team": ["@niko"], "b/c": ["@nrc"]}
         }
     }
 
diff --git a/highfive/tests/test_newpr.py b/highfive/tests/test_newpr.py
index 9062044..d0dee85 100644
--- a/highfive/tests/test_newpr.py
+++ b/highfive/tests/test_newpr.py
@@ -348,7 +348,8 @@ Please see [the contribution instructions](%s) for more information.
             # https://github.com/rust-lang-nursery/highfive/issues/184
             None,
         )
-        handler = HighfiveHandlerMock(Payload({})).handler
+        config = {}
+        handler = HighfiveHandlerMock(Payload({}), repo_config=config).handler
 
         for (msg, reviewer) in found_cases:
             assert handler.find_reviewer(msg, None) == reviewer, \
@@ -563,12 +564,16 @@ class TestSetAssignee(TestNewPR):
                 'message': 'Also important',
                 'reviewers': ['@userD'],
             },
+            {
+                'message': 'last message but this one has nobody to mention associated with it',
+                'reviewers': [],
+            },
         ]
         self.set_assignee(to_mention=to_mention)
 
         self.assert_api_req_call()
         self.mocks['post_comment'].assert_called_once_with(
-            'This is important\n\ncc @userA,@userB,@userC\n\nAlso important\n\ncc @userD',
+            'This is important\n\ncc @userA,@userB,@userC\n\nAlso important\n\ncc @userD\n\nlast message but this one has nobody to mention associated with it',
             self.owner, self.repo, self.issue
         )
 
@@ -980,7 +985,7 @@ class TestNewComment(TestNewPR):
     def test_not_open(self):
         handler = self.make_handler(state='closed')
 
-        assert handler.new_comment() is None
+        assert handler.new_comment() == 'skipped - closed issue'
         self.mocks['is_collaborator'].assert_not_called()
         self.mocks['find_reviewer'].assert_not_called()
         self.mocks['set_assignee'].assert_not_called()
@@ -996,7 +1001,7 @@ class TestNewComment(TestNewPR):
     def test_commenter_is_integration_user(self):
         handler = self.make_handler(commenter='integrationUser')
 
-        assert handler.new_comment() is None
+        assert handler.new_comment() == 'skipped - our own comment'
         self.mocks['is_collaborator'].assert_not_called()
         self.mocks['find_reviewer'].assert_not_called()
         self.mocks['set_assignee'].assert_not_called()
@@ -1007,7 +1012,7 @@ class TestNewComment(TestNewPR):
         )
 
         self.mocks['is_collaborator'].return_value = False
-        assert handler.new_comment() is None
+        assert handler.new_comment() == 'skipped, comment not by author, collaborator, or assignee'
         self.mocks['is_collaborator'].assert_called_with(
             'userB', 'repo-owner', 'repo-name'
         )
@@ -1160,6 +1165,17 @@ class TestChooseReviewer(TestNewPR):
                 'rust', 'rust-lang', self.fakes['diff']['normal'], 'fooauthor'
             )
 
+    def test_nested_groups(self):
+        """Test choosing a reviewer from group with nested groups.
+        """
+        self.handler = HighfiveHandlerMock(
+            Payload({}), repo_config=self.fakes['config']['nested_groups']
+        ).handler
+        (chosen_reviewers, mentions) = self.choose_reviewers(
+            self.fakes['diff']['normal'], "nikomatsakis"
+        )
+        assert set(["pnkfelix", "nrc"]) == chosen_reviewers
+
     def test_global_core(self):
         """Test choosing a reviewer from the core group in the global
         configuration.
@@ -1269,8 +1285,15 @@ class TestChooseReviewer(TestNewPR):
         found_cases = (
             ("r? @foo/a", "pnkfelix"),
             ("r? foo/a", "pnkfelix"),
+            ("r? rust-lang/compiler-team", "niko"),
+            ("r? compiler-team", "niko"),
             ("r? @b/c", "nrc"),
             ("r? b/c", "nrc"),
+
+            # @d goes to the user
+            ("r? @d", "d"),
+            # d goes to the team
+            ("r? d", "e"),
         )
 
         not_found_cases = (
@@ -1304,14 +1327,14 @@ class TestRun(TestNewPR):
     def test_newpr(self):
         payload = Payload({'action': 'opened'})
         m = self.handler_mock(payload)
-        assert m.handler.run('pull_request') == 'OK\n'
+        assert m.handler.run('pull_request') == 'OK, handled new PR\n'
         self.mocks['new_pr'].assert_called_once_with()
         self.mocks['new_comment'].assert_not_called()
 
     def test_new_comment(self):
         payload = Payload({'action': 'created'})
         m = self.handler_mock(payload)
-        assert m.handler.run('issue_comment') == 'OK\n'
+        assert m.handler.run('issue_comment').startswith('OK')
         self.mocks['new_pr'].assert_not_called()
         self.mocks['new_comment'].assert_called_once_with()