Skip to content
This repository was archived by the owner on Mar 14, 2023. It is now read-only.

Commit b5bfd37

Browse files
committed
Don't use startswith when comparing mentions paths.
1 parent 7b24638 commit b5bfd37

File tree

4 files changed

+80
-9
lines changed

4 files changed

+80
-9
lines changed

highfive/newpr.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,11 @@ def get_to_mention(self, diff, author):
340340
cur_dir = None
341341
if len(full_dir) > 0:
342342
for entry in mentions:
343-
if full_dir.startswith(entry):
343+
# Check if this entry is a prefix
344+
eparts = entry.split("/")
345+
if (len(eparts) <= len(parts) and
346+
all(a==b for a,b in zip(parts, eparts))
347+
):
344348
to_mention.add(entry)
345349
elif entry.endswith('.rs') and full_dir.endswith(entry):
346350
to_mention.add(entry)

highfive/tests/fakes.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,24 @@ def get_repo_configs():
4848
"groups": {"all": []},
4949
"dirs": {},
5050
},
51+
'mentions': {
52+
"groups": {"all": []},
53+
"dirs": {"compiler": ["@pnkfelix"]},
54+
"mentions": {
55+
"src/tools/cargo": {
56+
"message": "this should not match startswith",
57+
"reviewers": ["@ehuss"],
58+
},
59+
"error_codes.rs": {
60+
"message": "bare .rs file should match endswith",
61+
"reviewers": ["@GuillaumeGomez"],
62+
},
63+
"compiler/rustc": {
64+
"message": "directory should match all files within",
65+
"reviewers": ["@pnkfelix"],
66+
}
67+
}
68+
}
5169
}
5270

5371

highfive/tests/fakes/mentions.diff

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
diff --git a/compiler/rustc/src/main.rs b/compiler/rustc/src/main.rs
2+
index 6bc5aa6382c..4fc3e397508 100644
3+
--- a/compiler/rustc/src/main.rs
4+
+++ b/compiler/rustc/src/main.rs
5+
@@ -1,3 +1,4 @@
6+
+// Hello!
7+
fn main() {
8+
// Pull in jemalloc when enabled.
9+
//
10+
diff --git a/compiler/rustc_error_codes/src/error_codes.rs b/compiler/rustc_error_codes/src/error_codes.rs
11+
index 636c37142ad..2172d8eaec9 100644
12+
--- a/compiler/rustc_error_codes/src/error_codes.rs
13+
+++ b/compiler/rustc_error_codes/src/error_codes.rs
14+
@@ -463,6 +463,7 @@
15+
E0777: include_str!("./error_codes/E0777.md"),
16+
E0778: include_str!("./error_codes/E0778.md"),
17+
E0779: include_str!("./error_codes/E0779.md"),
18+
+E0779: include_str!("./error_codes/E0780.md"),
19+
;
20+
// E0006, // merged with E0005
21+
// E0008, // cannot bind by-move into a pattern guard
22+
diff --git a/src/tools/cargotest/main.rs b/src/tools/cargotest/main.rs
23+
index 8aabe077cf1..0a6bac48ebb 100644
24+
--- a/src/tools/cargotest/main.rs
25+
+++ b/src/tools/cargotest/main.rs
26+
@@ -36,7 +36,7 @@ struct Test {
27+
Test {
28+
name: "xsv",
29+
repo: "https://github.com/BurntSushi/xsv",
30+
- sha: "66956b6bfd62d6ac767a6b6499c982eae20a2c9f",
31+
+ sha: "3de6c04269a7d315f7e9864b9013451cd9580a08",
32+
lock: None,
33+
packages: &[],
34+
},

highfive/tests/test_newpr.py

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1032,6 +1032,7 @@ def make_fakes(cls):
10321032
'diff': {
10331033
'normal': load_fake('normal.diff'),
10341034
'travis-yml': load_fake('travis-yml.diff'),
1035+
'mentions': load_fake('mentions.diff'),
10351036
},
10361037
'config': fakes.get_repo_configs(),
10371038
'global_': fakes.get_global_configs(),
@@ -1073,7 +1074,9 @@ def choose_reviewers(self, diff, author, global_=None):
10731074
)
10741075
mentions = self.get_to_mention(diff, global_)
10751076
chosen_reviewers.add(reviewer)
1076-
mention_list.add(tuple(mentions))
1077+
for mention in mentions:
1078+
for reviewer in mention['reviewers']:
1079+
mention_list.add(reviewer)
10771080
return chosen_reviewers, mention_list
10781081

10791082
def test_individuals_no_dirs_1(self):
@@ -1087,7 +1090,7 @@ def test_individuals_no_dirs_1(self):
10871090
self.fakes['diff']['normal'], "nikomatsakis"
10881091
)
10891092
assert set(["pnkfelix", "nrc"]) == chosen_reviewers
1090-
assert set([()]) == mentions
1093+
assert set() == mentions
10911094

10921095
def test_individuals_no_dirs_2(self):
10931096
"""Test choosing a reviewer from a list of individual reviewers, no
@@ -1100,7 +1103,7 @@ def test_individuals_no_dirs_2(self):
11001103
self.fakes['diff']['normal'], "nrc"
11011104
)
11021105
assert set(["pnkfelix"]) == chosen_reviewers
1103-
assert set([()]) == mentions
1106+
assert set() == mentions
11041107

11051108
def test_circular_groups(self):
11061109
"""Test choosing a reviewer from groups that have circular references.
@@ -1125,7 +1128,7 @@ def test_global_core(self):
11251128
self.fakes['global_']['base']
11261129
)
11271130
assert set(['alexcrichton']) == chosen_reviewers
1128-
assert set([()]) == mentions
1131+
assert set() == mentions
11291132

11301133
@mock.patch('highfive.newpr.HighfiveHandler._load_json_file')
11311134
def test_global_group_overlap(self, mock_load_json):
@@ -1152,7 +1155,7 @@ def test_no_potential_reviewers(self):
11521155
self.fakes['global_']['base']
11531156
)
11541157
assert set([None]) == chosen_reviewers
1155-
assert set([()]) == mentions
1158+
assert set() == mentions
11561159

11571160
def test_with_dirs(self):
11581161
"""Test choosing a reviewer when directory reviewers are defined that
@@ -1165,7 +1168,7 @@ def test_with_dirs(self):
11651168
self.fakes['diff']['normal'], "nikomatsakis"
11661169
)
11671170
assert set(["pnkfelix", "nrc"]) == chosen_reviewers
1168-
assert set([()]) == mentions
1171+
assert set() == mentions
11691172

11701173
def test_with_dirs_no_intersection(self):
11711174
"""Test choosing a reviewer when directory reviewers are defined that
@@ -1178,7 +1181,7 @@ def test_with_dirs_no_intersection(self):
11781181
self.fakes['diff']['normal'], "nikomatsakis"
11791182
)
11801183
assert set(["pnkfelix", "nrc"]) == chosen_reviewers
1181-
assert set([()]) == mentions
1184+
assert set() == mentions
11821185

11831186
def test_with_files(self):
11841187
"""Test choosing a reviewer when a file os changed."""
@@ -1189,7 +1192,19 @@ def test_with_files(self):
11891192
self.fakes['diff']['travis-yml'], "nikomatsakis"
11901193
)
11911194
assert set(["pnkfelix", "nrc", "aturon"]) == chosen_reviewers
1192-
assert set([()]) == mentions
1195+
assert set() == mentions
1196+
1197+
def test_mentions(self):
1198+
"""Test tagging people listed in the mentions list."""
1199+
self.handler = HighfiveHandlerMock(
1200+
Payload({}), repo_config=self.fakes['config']['mentions']
1201+
).handler
1202+
(chosen_reviewers, mentions) = self.choose_reviewers(
1203+
self.fakes['diff']['mentions'], "nikomatsakis",
1204+
)
1205+
assert set(["pnkfelix"]) == chosen_reviewers
1206+
# @ehuss should not be listed here
1207+
assert set(["@pnkfelix", "@GuillaumeGomez"]) == mentions
11931208

11941209

11951210
class TestRun(TestNewPR):

0 commit comments

Comments
 (0)