[fix] Fix crash on finding unexpected repo names in multi issues file, fix #5

Author: slarse

repobee_feedback/feedback.py

@@ -24,14 +24,16 @@
 
 
 def callback(args: argparse.Namespace, api: plug.API) -> None:
-    repo_names = plug.generate_repo_names(args.students, args.master_repo_names)
+    repo_names = plug.generate_repo_names(
+        args.students, args.master_repo_names
+    )
     if "multi_issues_file" in args and args.multi_issues_file is not None:
         issues_file = pathlib.Path(args.multi_issues_file).resolve()
-        issues = _parse_multi_issues_file(issues_file)
+        all_issues = _parse_multi_issues_file(issues_file)
     else:
         issues_dir = pathlib.Path(args.issues_dir).resolve()
-        issues = _collect_issues(repo_names, issues_dir)
-    _raise_on_missing_issue_file(issues, repo_names)
+        all_issues = _collect_issues(repo_names, issues_dir)
+    issues = _extract_expected_issues(all_issues, repo_names)
     for repo_name, issue in issues:
         open_issue = args.batch_mode or _ask_for_open(
             issue, repo_name, args.truncation_length
@@ -121,17 +123,31 @@ def _ask_for_open(issue: plug.Issue, repo_name: str, trunc_len: int) -> bool:
         )
     )
     return (
-        input('Open issue "{}" in repo {}? (y/n) '.format(issue.title, repo_name))
+        input(
+            'Open issue "{}" in repo {}? (y/n) '.format(issue.title, repo_name)
+        )
         == "y"
     )
 
 
-def _raise_on_missing_issue_file(repos_and_issues, repo_names):
-    expected = set(repo_names)
-    for repo_name, _ in repos_and_issues:
-        expected.remove(repo_name)
-    if expected:
-        raise plug.PlugError("Missing issues for: " + ", ".join(expected))
+def _extract_expected_issues(
+    repos_and_issues, repo_names
+) -> List[Tuple[str, plug.Issue]]:
+    expected_repo_names = set(repo_names)
+    expected_repos_and_issues = [
+        (repo_name, issue)
+        for repo_name, issue in repos_and_issues
+        if repo_name in expected_repo_names
+    ]
+    missing_repos = expected_repo_names - set(
+        (repo_name for repo_name, _ in expected_repos_and_issues)
+    )
+    if missing_repos:
+        raise plug.PlugError(
+            "Missing issues for: " + ", ".join(expected_repo_names)
+        )
+
+    return expected_repos_and_issues
 
 
 def _collect_issues(
@@ -154,11 +170,15 @@ def _read_issue(issue_path: pathlib.Path) -> plug.Issue:
 def _parse_multi_issues_file(
     issues_file: pathlib.Path
 ) -> Iterable[Tuple[str, plug.Issue]]:
-    with open(str(issues_file), mode="r", encoding=sys.getdefaultencoding()) as file:
+    with open(
+        str(issues_file), mode="r", encoding=sys.getdefaultencoding()
+    ) as file:
         lines = list(file.readlines())
 
     if not lines or not re.match(BEGIN_ISSUE_PATTERN, lines[0], re.IGNORECASE):
-        raise plug.PlugError("first line of multi issues file not #ISSUE# line")
+        raise plug.PlugError(
+            "first line of multi issues file not #ISSUE# line"
+        )
 
     issue_blocks = _extract_issue_blocks(lines)
     return list(_extract_issues(issue_blocks, lines))

tests/test_feedback.py

@@ -195,3 +195,28 @@ def test_opens_issues_from_multi_issues_file(
         feedback.callback(args=parsed_args_multi_issues_file, api=api_mock)
 
         api_mock.open_issue.assert_has_calls(expected_calls)
+
+    def test_skips_unexpected_issues_in_multi_issues_file(
+        self, with_multi_issues_file, parsed_args_multi_issues_file, api_mock
+    ):
+        """Test that an exception is raised if one or more issues are found
+        relating to student repos that ar not in prod(master_repo_names, students).
+        """
+        student_teams = parsed_args_multi_issues_file.students
+        args_dict = vars(parsed_args_multi_issues_file)
+        args_dict["students"] = student_teams[:-1]
+        args = argparse.Namespace(**args_dict)
+        unexpected_repos = plug.generate_repo_names(
+            student_teams[-1:], MASTER_REPO_NAMES
+        )
+
+        _, repos_and_issues = with_multi_issues_file
+        expected_calls = [
+            mock.call(issue.title, issue.body, [repo_name])
+            for repo_name, issue in repos_and_issues
+            if repo_name not in unexpected_repos
+        ]
+
+        feedback.callback(args=args, api=api_mock)
+
+        assert sorted(expected_calls) == sorted(api_mock.open_issue.mock_calls)