From ededb1f6280fa04d0b18223f8093f85e9e6c0621 Mon Sep 17 00:00:00 2001 From: Joseph Sawaya Date: Wed, 19 Feb 2025 15:21:04 -0500 Subject: [PATCH 1/4] fix: len(None) bug the bug was the the NoVersioningSystem class was not defining the list_relevant_files method, which meant it as falling back to the VersioningSystemInterface definition which returns None. This commit make the VersioningSystemInterface an ABC and makes all the relevant methods abstract, then implements the methods on the VersioningSystem implementations. --- codecov_cli/helpers/versioning_systems.py | 20 +++++++++--- codecov_cli/services/upload/network_finder.py | 2 +- .../services/upload/upload_collector.py | 2 +- tests/factory.py | 7 ++++ .../services/upload/test_upload_collector.py | 32 ++++++++++++++++++- 5 files changed, 56 insertions(+), 7 deletions(-) diff --git a/codecov_cli/helpers/versioning_systems.py b/codecov_cli/helpers/versioning_systems.py index 143f8c88c..6b38f1d8a 100644 --- a/codecov_cli/helpers/versioning_systems.py +++ b/codecov_cli/helpers/versioning_systems.py @@ -6,21 +6,27 @@ from codecov_cli.fallbacks import FallbackFieldEnum from codecov_cli.helpers.git import parse_git_service, parse_slug +from abc import ABC, abstractmethod logger = logging.getLogger("codecovcli") -class VersioningSystemInterface(object): +class VersioningSystemInterface(ABC): def __repr__(self) -> str: return str(type(self)) + @abstractmethod def get_fallback_value(self, fallback_field: FallbackFieldEnum) -> t.Optional[str]: pass + @abstractmethod def get_network_root(self) -> t.Optional[Path]: pass - def list_relevant_files(self, directory: t.Optional[Path] = None) -> t.List[str]: + @abstractmethod + def list_relevant_files( + self, directory: t.Optional[Path] = None + ) -> t.Optional[t.List[str]]: pass @@ -119,8 +125,8 @@ def get_network_root(self): return Path(p.stdout.decode().rstrip()) return None - def list_relevant_files(self, root_folder: t.Optional[Path] = None) -> t.List[str]: - dir_to_use = root_folder or self.get_network_root() + def list_relevant_files(self, directory: t.Optional[Path] = None) -> t.List[str]: + dir_to_use = directory or self.get_network_root() if dir_to_use is None: raise ValueError("Can't determine root folder") @@ -145,3 +151,9 @@ def is_available(cls): def get_network_root(self): return Path.cwd() + + def get_fallback_value(self, fallback_field: FallbackFieldEnum): + return None + + def list_relevant_files(self, directory: t.Optional[Path] = None) -> t.List[str]: + return [] diff --git a/codecov_cli/services/upload/network_finder.py b/codecov_cli/services/upload/network_finder.py index 8da568eb1..0bf1aee31 100644 --- a/codecov_cli/services/upload/network_finder.py +++ b/codecov_cli/services/upload/network_finder.py @@ -20,7 +20,7 @@ def __init__( def find_files(self, ignore_filters=False) -> typing.List[str]: files = self.versioning_system.list_relevant_files(self.network_root_folder) - if not ignore_filters: + if files and not ignore_filters: if self.network_filter: files = [file for file in files if file.startswith(self.network_filter)] if self.network_prefix: diff --git a/codecov_cli/services/upload/upload_collector.py b/codecov_cli/services/upload/upload_collector.py index 5ca7c0af7..c3952545d 100644 --- a/codecov_cli/services/upload/upload_collector.py +++ b/codecov_cli/services/upload/upload_collector.py @@ -166,7 +166,7 @@ def generate_upload_data( f"Found {len(report_files)} {report_type.value} files to report" ) logger.debug( - f"Found {len(network)} network files to report, ({len(unfiltered_network)} without filtering)" + f"Found {len(network) if network else None} network files to report, ({len(unfiltered_network) if network else None} without filtering)" ) if not report_files: if report_type == ReportType.TEST_RESULTS: diff --git a/tests/factory.py b/tests/factory.py index bb410266d..4d8385bc9 100644 --- a/tests/factory.py +++ b/tests/factory.py @@ -4,6 +4,7 @@ from codecov_cli.helpers.ci_adapters.base import CIAdapterBase from codecov_cli.helpers.versioning_systems import VersioningSystemInterface from codecov_cli.runners.types import LabelAnalysisRunnerInterface +from pathlib import Path class FakeProvider(CIAdapterBase): @@ -73,6 +74,12 @@ def __init__(self, values_dict: Optional[dict] = None): def get_fallback_value(self, fallback_field: FallbackFieldEnum) -> Optional[str]: return self.values_dict[fallback_field] + def get_network_root(self) -> Optional[Path]: + return None + + def list_relevant_files(self, directory: Optional[Path] = None) -> List[str]: + return [] + class FakeRunner(LabelAnalysisRunnerInterface): dry_run_runner_options = ["--labels"] diff --git a/tests/services/upload/test_upload_collector.py b/tests/services/upload/test_upload_collector.py index 39124d0ec..b2b6e5a98 100644 --- a/tests/services/upload/test_upload_collector.py +++ b/tests/services/upload/test_upload_collector.py @@ -1,7 +1,10 @@ from pathlib import Path from unittest.mock import patch -from codecov_cli.helpers.versioning_systems import GitVersioningSystem +from codecov_cli.helpers.versioning_systems import ( + GitVersioningSystem, + NoVersioningSystem, +) from codecov_cli.services.upload.file_finder import FileFinder from codecov_cli.services.upload.network_finder import NetworkFinder from codecov_cli.services.upload.upload_collector import UploadCollector @@ -174,3 +177,30 @@ def test_generate_upload_data(tmp_path): for file in expected: assert file in res.files + + +@patch("codecov_cli.services.upload.upload_collector.logger") +@patch.object(GitVersioningSystem, "get_network_root", return_value=None) +def test_generate_upload_data_with_none_network( + mock_get_network_root, mock_logger, tmp_path +): + (tmp_path / "coverage.xml").touch() + + file_finder = FileFinder(tmp_path) + network_finder = NetworkFinder(NoVersioningSystem(), None, None, None) + + collector = UploadCollector([], network_finder, file_finder, {}) + + res = collector.generate_upload_data() + + mock_logger.debug.assert_any_call("Collecting relevant files") + mock_logger.debug.assert_any_call( + "Found None network files to report, (None without filtering)" + ) + + mock_logger.info.assert_any_call("Found 1 coverage files to report") + mock_logger.info.assert_any_call("> {}".format(tmp_path / "coverage.xml")) + + assert res.network == [] + assert len(res.files) == 1 + assert len(res.file_fixes) == 0 From 1f983231c03fedfe3b37b4dfc95c21926ae9f889 Mon Sep 17 00:00:00 2001 From: Joseph Sawaya Date: Thu, 20 Feb 2025 12:15:16 -0500 Subject: [PATCH 2/4] quick fix --- codecov_cli/services/upload/upload_collector.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codecov_cli/services/upload/upload_collector.py b/codecov_cli/services/upload/upload_collector.py index c3952545d..5ca7c0af7 100644 --- a/codecov_cli/services/upload/upload_collector.py +++ b/codecov_cli/services/upload/upload_collector.py @@ -166,7 +166,7 @@ def generate_upload_data( f"Found {len(report_files)} {report_type.value} files to report" ) logger.debug( - f"Found {len(network) if network else None} network files to report, ({len(unfiltered_network) if network else None} without filtering)" + f"Found {len(network)} network files to report, ({len(unfiltered_network)} without filtering)" ) if not report_files: if report_type == ReportType.TEST_RESULTS: From 1192c943a8cf755267708e0b39131f8471f675f9 Mon Sep 17 00:00:00 2001 From: Tom Hu Date: Thu, 20 Feb 2025 15:50:32 -0300 Subject: [PATCH 3/4] fix: update tests --- tests/services/upload/test_upload_collector.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/services/upload/test_upload_collector.py b/tests/services/upload/test_upload_collector.py index b2b6e5a98..dce672b38 100644 --- a/tests/services/upload/test_upload_collector.py +++ b/tests/services/upload/test_upload_collector.py @@ -195,7 +195,7 @@ def test_generate_upload_data_with_none_network( mock_logger.debug.assert_any_call("Collecting relevant files") mock_logger.debug.assert_any_call( - "Found None network files to report, (None without filtering)" + "Found 0 network files to report, (0 without filtering)" ) mock_logger.info.assert_any_call("Found 1 coverage files to report") From 1b1176b40b1c42e1dd9a708fe4fda53c0bac4495 Mon Sep 17 00:00:00 2001 From: Tom Hu Date: Thu, 20 Feb 2025 15:57:13 -0300 Subject: [PATCH 4/4] fix: do not publish to pypi --- .github/workflows/release_flow.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/release_flow.yml b/.github/workflows/release_flow.yml index c0e43bdbb..75fa2a288 100644 --- a/.github/workflows/release_flow.yml +++ b/.github/workflows/release_flow.yml @@ -51,6 +51,7 @@ jobs: find . -empty -type d -delete ls -alrt */* - name: Publish package to PyPi + if: false uses: pypa/gh-action-pypi-publish@release/v1 with: verbose: true