Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions evergreen.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,9 @@ def main(): # pragma: no cover
if repo.archived:
print(f"Skipping {repo.full_name} (archived)")
continue
if is_empty_repo(repo):
print(f"Skipping {repo.full_name} (empty repository)")
continue
if repo.visibility.lower() not in config.filter_visibility:
print(f"Skipping {repo.full_name} (visibility-filtered)")
continue
Expand Down Expand Up @@ -315,6 +318,17 @@ def is_repo_created_date_before(
)


def is_empty_repo(repo):
"""Check whether the repository is empty (has no commits).

Empty repositories cannot hold or receive a dependabot configuration, and
every ``repo.get_contents(...)`` call against them returns a 404
("This repository is empty."). Detecting them up front lets the caller skip
them before any content lookup, rather than crashing partway through.
"""
return repo.size == 0


def is_dependabot_security_updates_enabled(ghe, ghe_api_url, owner, repo, access_token):
"""
Check if Dependabot security updates are enabled at the /repos/:owner/:repo/automated-security-fixes endpoint using the requests library
Expand Down
12 changes: 11 additions & 1 deletion exceptions.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"""Custom exceptions for the evergreen application."""

from github import UnknownObjectException
from github import GithubException, UnknownObjectException


class OptionalFileNotFoundError(UnknownObjectException):
Expand Down Expand Up @@ -47,3 +47,13 @@ def check_optional_file(repo, filename):
raise OptionalFileNotFoundError(
status=e.status, data=e.data, headers=e.headers
) from e
except GithubException as e:
# An empty repository (one with no commits) returns a 404 that PyGithub
# raises as the base GithubException ("This repository is empty.")
# rather than UnknownObjectException. Treat it the same as a missing
# optional file so the repository is skipped instead of crashing the run.
if e.status == 404:
raise OptionalFileNotFoundError(
status=e.status, data=e.data, headers=e.headers
) from e
Comment on lines +50 to +58

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thank you. You're right that hardening check_optional_file alone just moves the crash: an empty repo returns None there, the loop doesn't skip it, and it then hits repo.get_contents("/") / .github/workflows / .devcontainer in dependabot_file.py, which only catch UnknownObjectException.

I've pushed a follow-up that skips empty repos up front instead:

  • Added an is_empty_repo(repo) helper (repo.size == 0, no extra API call since size is already on the listed repo object).
  • The main loop now skips empty repos right after the archived check, before any get_contents lookup, so none of the dependabot_file.py paths are reached for them.
  • Kept the check_optional_file GithubException 404 handling as a defensive backstop.
  • Added unit tests for is_empty_repo; full suite passes (184 passed).

This felt cleaner than broadening the exception handling at each get_contents call site, but happy to switch to a shared safe_get_contents-style helper if you'd prefer that centralization.

raise
17 changes: 17 additions & 0 deletions test_evergreen.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
get_global_project_id,
get_repos_iterator,
is_dependabot_security_updates_enabled,
is_empty_repo,
is_repo_created_date_before,
link_item_to_project,
)
Expand Down Expand Up @@ -362,6 +363,22 @@ def test_check_pending_issues_for_duplicates_with_duplicates(self):
self.assertTrue(result)


class TestIsEmptyRepo(unittest.TestCase):
"""Test the is_empty_repo function in evergreen.py"""

def test_is_empty_repo_true_for_size_zero(self):
"""A repository with no commits reports size 0 and is considered empty."""
mock_repo = MagicMock()
mock_repo.size = 0
self.assertTrue(is_empty_repo(mock_repo))

def test_is_empty_repo_false_for_non_zero_size(self):
"""A repository with content is not considered empty."""
mock_repo = MagicMock()
mock_repo.size = 42
self.assertFalse(is_empty_repo(mock_repo))


class TestGetReposIterator(unittest.TestCase):
"""Test the get_repos_iterator function in evergreen.py"""

Expand Down
36 changes: 35 additions & 1 deletion test_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from unittest.mock import Mock

from exceptions import OptionalFileNotFoundError, check_optional_file
from github import UnknownObjectException
from github import GithubException, UnknownObjectException


class TestOptionalFileNotFoundError(unittest.TestCase):
Expand Down Expand Up @@ -94,6 +94,40 @@ def test_check_optional_file_with_missing_file(self):
self.assertEqual(context.exception.__cause__, original_error)
mock_repo.get_contents.assert_called_once_with("missing.yml")

def test_check_optional_file_with_empty_repository(self):
"""Test check_optional_file when the repository is empty.

An empty repository returns a 404 that PyGithub raises as the base
GithubException rather than UnknownObjectException. It should be
translated to OptionalFileNotFoundError so the repository is skipped.
"""
mock_repo = Mock()

original_error = GithubException(
status=404, data={"message": "This repository is empty."}
)
mock_repo.get_contents.side_effect = original_error

with self.assertRaises(OptionalFileNotFoundError) as context:
check_optional_file(mock_repo, "config.yml")

self.assertEqual(context.exception.__cause__, original_error)
mock_repo.get_contents.assert_called_once_with("config.yml")

def test_check_optional_file_reraises_non_404_github_exception(self):
"""Test check_optional_file re-raises non-404 GithubExceptions unchanged."""
mock_repo = Mock()

original_error = GithubException(status=403, data={"message": "Forbidden"})
mock_repo.get_contents.side_effect = original_error

with self.assertRaises(GithubException) as context:
check_optional_file(mock_repo, "config.yml")

self.assertNotIsInstance(context.exception, OptionalFileNotFoundError)
self.assertEqual(context.exception.status, 403)
mock_repo.get_contents.assert_called_once_with("config.yml")

def test_check_optional_file_can_catch_as_unknown_object_exception(self):
"""Test that OptionalFileNotFoundError from check_optional_file can be caught as UnknownObjectException."""
mock_repo = Mock()
Expand Down