Skip to content

Commit

Permalink
fix: len(None) bug (#643)
Browse files Browse the repository at this point in the history
* 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.

* quick fix

* fix: update tests

* fix: do not publish to pypi

---------

Co-authored-by: Tom Hu <[email protected]>
Co-authored-by: Tom Hu <[email protected]>
  • Loading branch information
3 people authored Feb 20, 2025
1 parent 63bd5f2 commit b1a69ee
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 6 deletions.
1 change: 1 addition & 0 deletions .github/workflows/release_flow.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 16 additions & 4 deletions codecov_cli/helpers/versioning_systems.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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")

Expand All @@ -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 []
2 changes: 1 addition & 1 deletion codecov_cli/services/upload/network_finder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
7 changes: 7 additions & 0 deletions tests/factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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"]
Expand Down
32 changes: 31 additions & 1 deletion tests/services/upload/test_upload_collector.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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 0 network files to report, (0 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

0 comments on commit b1a69ee

Please sign in to comment.