From 68a9fc55db05b362c277d87b355d0c89aa0870f5 Mon Sep 17 00:00:00 2001 From: Juan Pasutti Date: Tue, 15 Sep 2026 23:54:07 -0300 Subject: [PATCH 1/2] Ignore empty GitHub API keys Signed-off-by: Juan Pasutti --- .../github/util/github_api_key_handler.py | 7 ++- tests/test_classes/test_github_api_keys.py | 58 +++++++++++++++++++ 2 files changed, 63 insertions(+), 2 deletions(-) create mode 100644 tests/test_classes/test_github_api_keys.py diff --git a/collectoss/tasks/github/util/github_api_key_handler.py b/collectoss/tasks/github/util/github_api_key_handler.py index 5cabe1fab..93570ed79 100644 --- a/collectoss/tasks/github/util/github_api_key_handler.py +++ b/collectoss/tasks/github/util/github_api_key_handler.py @@ -102,8 +102,11 @@ def get_api_keys(self) -> List[str]: time.sleep(5) attempts += 1 - if self.config_key is not None: - keys += [self.config_key] + if self.config_key is not None: # Leave out None values + if self.config_key.strip(): # Leave out empty strings + keys += [self.config_key] + else: + self.logger.warning("GitHub API key is an empty string. Please, add a valid one.") if len(keys) == 0: return [] diff --git a/tests/test_classes/test_github_api_keys.py b/tests/test_classes/test_github_api_keys.py new file mode 100644 index 000000000..92eac261d --- /dev/null +++ b/tests/test_classes/test_github_api_keys.py @@ -0,0 +1,58 @@ +# SPDX-License-Identifier: MIT +import pytest +from unittest.mock import Mock, patch + +from collectoss.tasks.github.util.github_api_key_handler import GithubApiKeyHandler + + +github_whitespace_api_keys_list = ["", " "] +github_none_api_key = None +github_valid_api_key = "ghp_1234567890abcdef1234567890abcdef12345678" + +def build_handler(config_key, db_keys): + logger = Mock() + + with patch("collectoss.tasks.github.util.github_api_key_handler.RedisList"), \ + patch.object(GithubApiKeyHandler, "get_config_key", return_value=config_key), \ + patch.object(GithubApiKeyHandler, "get_api_keys_from_database", return_value=db_keys), \ + patch.object(GithubApiKeyHandler, "is_bad_api_key", return_value=False) as probe: + handler = GithubApiKeyHandler(logger) + + return handler, probe, logger + +@pytest.mark.unit +class TestConfigKeys: + + @pytest.mark.parametrize("github_whitespace_api_key", github_whitespace_api_keys_list) + def test_whitespace_config_key_with_no_db_keys(self, github_whitespace_api_key): + db_keys = [] + handler, probe, logger = build_handler(github_whitespace_api_key, db_keys) + + assert handler.keys == [] + assert probe.call_count == 0 + logger.warning.assert_called_once() + + def test_none_config_key_with_no_db_keys(self): + db_keys = [] + handler, probe, logger = build_handler(github_none_api_key, db_keys) + + assert handler.keys == [] + assert probe.call_count == 0 + logger.warning.assert_not_called() + + def test_valid_config_key_with_no_db_keys(self): + db_keys = [] + handler, probe, logger = build_handler(github_valid_api_key, db_keys) + + assert handler.keys == [github_valid_api_key] + assert probe.call_count == 1 + logger.warning.assert_not_called() + + @pytest.mark.parametrize("github_whitespace_api_key", github_whitespace_api_keys_list) + def test_whitespace_config_key_with_db_keys(self, github_whitespace_api_key): + db_keys = ["ghp_abcdef1234567890abcdef1234567890abcdef12"] + handler, probe, logger = build_handler(github_whitespace_api_key, db_keys) + + assert handler.keys == db_keys + assert probe.call_count == 1 + logger.warning.assert_called_once() From 27a4b7a1647d5d7185771377f110261c6df92a30 Mon Sep 17 00:00:00 2001 From: Juan Pasutti Date: Tue, 22 Sep 2026 12:00:56 -0300 Subject: [PATCH 2/2] test: assert on the redis key list, not just the warning Signed-off-by: Juan Pasutti --- tests/test_classes/test_github_api_keys.py | 33 ++++++++++++++-------- 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/tests/test_classes/test_github_api_keys.py b/tests/test_classes/test_github_api_keys.py index 92eac261d..a1212a1f5 100644 --- a/tests/test_classes/test_github_api_keys.py +++ b/tests/test_classes/test_github_api_keys.py @@ -8,6 +8,7 @@ github_whitespace_api_keys_list = ["", " "] github_none_api_key = None github_valid_api_key = "ghp_1234567890abcdef1234567890abcdef12345678" +github_valid_db_api_key = "ghp_abcdef1234567890abcdef1234567890abcdef12" def build_handler(config_key, db_keys): logger = Mock() @@ -15,10 +16,10 @@ def build_handler(config_key, db_keys): with patch("collectoss.tasks.github.util.github_api_key_handler.RedisList"), \ patch.object(GithubApiKeyHandler, "get_config_key", return_value=config_key), \ patch.object(GithubApiKeyHandler, "get_api_keys_from_database", return_value=db_keys), \ - patch.object(GithubApiKeyHandler, "is_bad_api_key", return_value=False) as probe: + patch.object(GithubApiKeyHandler, "is_bad_api_key", return_value=False) as mock_is_bad_api_key: handler = GithubApiKeyHandler(logger) - return handler, probe, logger + return handler, mock_is_bad_api_key, logger @pytest.mark.unit class TestConfigKeys: @@ -26,33 +27,41 @@ class TestConfigKeys: @pytest.mark.parametrize("github_whitespace_api_key", github_whitespace_api_keys_list) def test_whitespace_config_key_with_no_db_keys(self, github_whitespace_api_key): db_keys = [] - handler, probe, logger = build_handler(github_whitespace_api_key, db_keys) + handler, mock_is_bad_api_key, logger = build_handler(github_whitespace_api_key, db_keys) assert handler.keys == [] - assert probe.call_count == 0 + assert mock_is_bad_api_key.call_count == 0 + # with no keys left, get_api_keys returns before it reaches redis + handler.redis_key_list.clear.assert_not_called() + handler.redis_key_list.extend.assert_not_called() logger.warning.assert_called_once() def test_none_config_key_with_no_db_keys(self): db_keys = [] - handler, probe, logger = build_handler(github_none_api_key, db_keys) + handler, mock_is_bad_api_key, logger = build_handler(github_none_api_key, db_keys) assert handler.keys == [] - assert probe.call_count == 0 + assert mock_is_bad_api_key.call_count == 0 + handler.redis_key_list.clear.assert_not_called() + handler.redis_key_list.extend.assert_not_called() logger.warning.assert_not_called() def test_valid_config_key_with_no_db_keys(self): db_keys = [] - handler, probe, logger = build_handler(github_valid_api_key, db_keys) + handler, mock_is_bad_api_key, logger = build_handler(github_valid_api_key, db_keys) assert handler.keys == [github_valid_api_key] - assert probe.call_count == 1 + assert mock_is_bad_api_key.call_count == 1 + handler.redis_key_list.extend.assert_called_once_with([github_valid_api_key]) logger.warning.assert_not_called() @pytest.mark.parametrize("github_whitespace_api_key", github_whitespace_api_keys_list) def test_whitespace_config_key_with_db_keys(self, github_whitespace_api_key): - db_keys = ["ghp_abcdef1234567890abcdef1234567890abcdef12"] - handler, probe, logger = build_handler(github_whitespace_api_key, db_keys) + expected_keys = [github_valid_db_api_key] + # get_api_keys appends to the list it gets back, so hand it a copy + handler, mock_is_bad_api_key, logger = build_handler(github_whitespace_api_key, list(expected_keys)) - assert handler.keys == db_keys - assert probe.call_count == 1 + assert handler.keys == expected_keys + assert mock_is_bad_api_key.call_count == 1 + handler.redis_key_list.extend.assert_called_once_with(expected_keys) logger.warning.assert_called_once()