Skip to content

Commit

Permalink
Slightly improve FileFinder typing (#509)
Browse files Browse the repository at this point in the history
This fixes a bunch of typing errors, as well as auto-reformats a bunch of stuff via the precommit hook.
  • Loading branch information
Swatinem authored Sep 27, 2024
1 parent 6269e67 commit 14741f0
Show file tree
Hide file tree
Showing 13 changed files with 205 additions and 60 deletions.
24 changes: 12 additions & 12 deletions codecov_cli/helpers/folder_searcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@
import os
import pathlib
import re
import typing
from fnmatch import translate
from typing import Generator, List, Optional, Pattern


def _is_included(
filename_include_regex: typing.Pattern,
multipart_include_regex: typing.Optional[typing.Pattern],
filename_include_regex: Pattern,
multipart_include_regex: Optional[Pattern],
path: pathlib.Path,
):
return filename_include_regex.match(path.name) and (
Expand All @@ -18,8 +18,8 @@ def _is_included(


def _is_excluded(
filename_exclude_regex: typing.Optional[typing.Pattern],
multipart_exclude_regex: typing.Optional[typing.Pattern],
filename_exclude_regex: Optional[Pattern],
multipart_exclude_regex: Optional[Pattern],
path: pathlib.Path,
):
return (
Expand All @@ -31,14 +31,14 @@ def _is_excluded(

def search_files(
folder_to_search: pathlib.Path,
folders_to_ignore: typing.List[str],
folders_to_ignore: List[str],
*,
filename_include_regex: typing.Pattern,
filename_exclude_regex: typing.Optional[typing.Pattern] = None,
multipart_include_regex: typing.Optional[typing.Pattern] = None,
multipart_exclude_regex: typing.Optional[typing.Pattern] = None,
filename_include_regex: Pattern,
filename_exclude_regex: Optional[Pattern] = None,
multipart_include_regex: Optional[Pattern] = None,
multipart_exclude_regex: Optional[Pattern] = None,
search_for_directories: bool = False,
) -> typing.Generator[pathlib.Path, None, None]:
) -> Generator[pathlib.Path, None, None]:
""" "
Searches for files or directories in a given folder
Expand Down Expand Up @@ -85,7 +85,7 @@ def search_files(
yield file_path


def globs_to_regex(patterns: typing.List[str]) -> typing.Optional[typing.Pattern]:
def globs_to_regex(patterns: List[str]) -> Optional[Pattern]:
"""
Converts a list of glob patterns to a combined ORed regex
Expand Down
20 changes: 10 additions & 10 deletions codecov_cli/services/upload/file_finder.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import logging
import os
import typing
from pathlib import Path
from typing import Iterable, List, Optional, Pattern

from codecov_cli.helpers.folder_searcher import globs_to_regex, search_files
from codecov_cli.types import UploadCollectionResultFile
Expand Down Expand Up @@ -183,9 +183,9 @@
class FileFinder(object):
def __init__(
self,
search_root: Path = None,
folders_to_ignore: typing.List[str] = None,
explicitly_listed_files: typing.List[Path] = None,
search_root: Optional[Path] = None,
folders_to_ignore: Optional[List[str]] = None,
explicitly_listed_files: Optional[List[Path]] = None,
disable_search: bool = False,
report_type: str = "coverage",
):
Expand All @@ -195,29 +195,29 @@ def __init__(
self.disable_search = disable_search
self.report_type = report_type

def find_files(self) -> typing.List[UploadCollectionResultFile]:
def find_files(self) -> List[UploadCollectionResultFile]:
if self.report_type == "coverage":
files_excluded_patterns = coverage_files_excluded_patterns
files_patterns = coverage_files_patterns
elif self.report_type == "test_results":
files_excluded_patterns = test_results_files_excluded_patterns
files_patterns = test_results_files_patterns
regex_patterns_to_exclude = globs_to_regex(files_excluded_patterns)
files_paths = []
assert regex_patterns_to_exclude # this is never `None`
files_paths: Iterable[Path] = []
user_files_paths = []
if self.explicitly_listed_files:
user_files_paths = self.get_user_specified_files(regex_patterns_to_exclude)
if not self.disable_search:
regex_patterns_to_include = globs_to_regex(files_patterns)
assert regex_patterns_to_include # this is never `None`
files_paths = search_files(
self.search_root,
default_folders_to_ignore + self.folders_to_ignore,
filename_include_regex=regex_patterns_to_include,
filename_exclude_regex=regex_patterns_to_exclude,
)
result_files = [
UploadCollectionResultFile(path) for path in files_paths if files_paths
]
result_files = [UploadCollectionResultFile(path) for path in files_paths]
user_result_files = [
UploadCollectionResultFile(path)
for path in user_files_paths
Expand All @@ -226,7 +226,7 @@ def find_files(self) -> typing.List[UploadCollectionResultFile]:

return list(set(result_files + user_result_files))

def get_user_specified_files(self, regex_patterns_to_exclude):
def get_user_specified_files(self, regex_patterns_to_exclude: Pattern):
user_filenames_to_include = []
files_excluded_but_user_includes = []
for file in self.explicitly_listed_files:
Expand Down
2 changes: 1 addition & 1 deletion tests/ci_adapters/test_azure_pipelines.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def test_detect(self, env_dict, expected, mocker):
(
{
AzurePipelinesEnvEnum.BUILD_SOURCEVERSION: "123456789000111",
AzurePipelinesEnvEnum.SYSTEM_PULLREQUEST_SOURCECOMMITID: "111000987654321"
AzurePipelinesEnvEnum.SYSTEM_PULLREQUEST_SOURCECOMMITID: "111000987654321",
},
"111000987654321",
),
Expand Down
18 changes: 13 additions & 5 deletions tests/commands/test_process_test_results.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,22 @@ def test_process_test_results(

assert result.exit_code == 0


mocked_post.assert_called_with(
url="https://api.github.com/repos/fake/repo/issues/pull/comments",
data={
"body": "### :x: Failed Test Results: \nCompleted 4 tests with **`1 failed`**, 3 passed and 0 skipped.\n<details><summary>View the full list of failed tests</summary>\n\n| **Test Description** | **Failure message** |\n| :-- | :-- |\n| <pre>Testsuite:<br>api.temp.calculator.test_calculator::test_divide<br><br>Test name:<br>pytest<br></pre> | <pre>def<br> test_divide():<br> &amp;gt; assert Calculator.divide(1, 2) == 0.5<br> E assert 1.0 == 0.5<br> E + where 1.0 = &amp;lt;function Calculator.divide at 0x104c9eb90&amp;gt;(1, 2)<br> E + where &amp;lt;function Calculator.divide at 0x104c9eb90&amp;gt; = Calculator.divide<br> .../temp/calculator/test_calculator.py:30: AssertionError</pre> |",
"cli_args": {'auto_load_params_from': None, 'codecov_yml_path': None, 'enterprise_url': None, 'verbose': False, 'version': 'cli-0.7.4', 'command': 'process-test-results', 'provider_token': 'whatever', 'disable_search': True, 'dir': os.getcwd(), 'exclude_folders': ()},
"cli_args": {
"auto_load_params_from": None,
"codecov_yml_path": None,
"enterprise_url": None,
"verbose": False,
"version": "cli-0.7.4",
"command": "process-test-results",
"provider_token": "whatever",
"disable_search": True,
"dir": os.getcwd(),
"exclude_folders": (),
},
},
headers={
"Accept": "application/vnd.github+json",
Expand All @@ -59,7 +69,6 @@ def test_process_test_results(
)



def test_process_test_results_non_existent_file(mocker, tmpdir):
tmp_file = tmpdir.mkdir("folder").join("summary.txt")

Expand Down Expand Up @@ -94,7 +103,7 @@ def test_process_test_results_non_existent_file(mocker, tmpdir):
assert result.exit_code == 1
expected_logs = [
"ci service found",
'Some files were not found',
"Some files were not found",
]
for log in expected_logs:
assert log in result.output
Expand Down Expand Up @@ -183,7 +192,6 @@ def test_process_test_results_missing_ref(mocker, tmpdir):
assert log in result.output



def test_process_test_results_missing_step_summary(mocker, tmpdir):
tmp_file = tmpdir.mkdir("folder").join("summary.txt")

Expand Down
2 changes: 0 additions & 2 deletions tests/helpers/test_git.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,5 +133,3 @@ def test_get_git_service_class():
assert isinstance(git.get_git_service("github"), Github)
assert git.get_git_service("gitlab") == None
assert git.get_git_service("bitbucket") == None


93 changes: 84 additions & 9 deletions tests/helpers/test_network_finder.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,40 +12,115 @@ def test_find_files(mocker, tmp_path):
mocked_vs = MagicMock()
mocked_vs.list_relevant_files.return_value = filenames

assert NetworkFinder(versioning_system=mocked_vs, network_filter=None, network_prefix=None, network_root_folder=tmp_path).find_files() == filenames
assert NetworkFinder(versioning_system=mocked_vs, network_filter="hello", network_prefix="bello", network_root_folder=tmp_path).find_files(False) == filtered_filenames
assert NetworkFinder(versioning_system=mocked_vs, network_filter="hello", network_prefix="bello", network_root_folder=tmp_path).find_files(True) == filenames
assert (
NetworkFinder(
versioning_system=mocked_vs,
network_filter=None,
network_prefix=None,
network_root_folder=tmp_path,
).find_files()
== filenames
)
assert (
NetworkFinder(
versioning_system=mocked_vs,
network_filter="hello",
network_prefix="bello",
network_root_folder=tmp_path,
).find_files(False)
== filtered_filenames
)
assert (
NetworkFinder(
versioning_system=mocked_vs,
network_filter="hello",
network_prefix="bello",
network_root_folder=tmp_path,
).find_files(True)
== filenames
)
mocked_vs.list_relevant_files.assert_called_with(tmp_path)


def test_find_files_with_filter(mocker, tmp_path):
filenames = ["hello/a.txt", "hello/c.txt", "bello/b.txt"]
filtered_filenames = ["hello/a.txt", "hello/c.txt"]

mocked_vs = MagicMock()
mocked_vs.list_relevant_files.return_value = filenames

assert NetworkFinder(versioning_system=mocked_vs, network_filter="hello", network_prefix=None, network_root_folder=tmp_path).find_files() == filtered_filenames
assert NetworkFinder(versioning_system=mocked_vs, network_filter="hello", network_prefix="bello", network_root_folder=tmp_path).find_files(True) == filenames
assert (
NetworkFinder(
versioning_system=mocked_vs,
network_filter="hello",
network_prefix=None,
network_root_folder=tmp_path,
).find_files()
== filtered_filenames
)
assert (
NetworkFinder(
versioning_system=mocked_vs,
network_filter="hello",
network_prefix="bello",
network_root_folder=tmp_path,
).find_files(True)
== filenames
)
mocked_vs.list_relevant_files.assert_called_with(tmp_path)


def test_find_files_with_prefix(mocker, tmp_path):
filenames = ["hello/a.txt", "hello/c.txt", "bello/b.txt"]
filtered_filenames = ["hellohello/a.txt", "hellohello/c.txt", "hellobello/b.txt"]

mocked_vs = MagicMock()
mocked_vs.list_relevant_files.return_value = filenames

assert NetworkFinder(versioning_system=mocked_vs, network_filter=None, network_prefix="hello", network_root_folder=tmp_path).find_files() == filtered_filenames
assert NetworkFinder(versioning_system=mocked_vs, network_filter="hello", network_prefix="bello", network_root_folder=tmp_path).find_files(True) == filenames
assert (
NetworkFinder(
versioning_system=mocked_vs,
network_filter=None,
network_prefix="hello",
network_root_folder=tmp_path,
).find_files()
== filtered_filenames
)
assert (
NetworkFinder(
versioning_system=mocked_vs,
network_filter="hello",
network_prefix="bello",
network_root_folder=tmp_path,
).find_files(True)
== filenames
)
mocked_vs.list_relevant_files.assert_called_with(tmp_path)


def test_find_files_with_filter_and_prefix(mocker, tmp_path):
filenames = ["hello/a.txt", "hello/c.txt", "bello/b.txt"]
filtered_filenames = ["bellohello/a.txt", "bellohello/c.txt"]

mocked_vs = MagicMock()
mocked_vs.list_relevant_files.return_value = filenames

assert NetworkFinder(versioning_system=mocked_vs, network_filter="hello", network_prefix="bello", network_root_folder=tmp_path).find_files() == filtered_filenames
assert NetworkFinder(versioning_system=mocked_vs, network_filter="hello", network_prefix="bello", network_root_folder=tmp_path).find_files(True) == filenames
assert (
NetworkFinder(
versioning_system=mocked_vs,
network_filter="hello",
network_prefix="bello",
network_root_folder=tmp_path,
).find_files()
== filtered_filenames
)
assert (
NetworkFinder(
versioning_system=mocked_vs,
network_filter="hello",
network_prefix="bello",
network_root_folder=tmp_path,
).find_files(True)
== filenames
)
mocked_vs.list_relevant_files.assert_called_with(tmp_path)
15 changes: 10 additions & 5 deletions tests/helpers/test_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,12 @@ def test_log_result_without_token(mocker):
error=None,
warnings=[],
status_code=201,
text="{\"message\":\"commit\",\"timestamp\":\"2024-03-25T15:41:07Z\",\"ci_passed\":true,\"state\":\"complete\",\"repository\":{\"name\":\"repo\",\"is_private\":false,\"active\":true,\"language\":\"python\",\"yaml\":null},\"author\":{\"avatar_url\":\"https://example.com\",\"service\":\"github\",\"username\":null,\"name\":\"dependabot[bot]\",\"ownerid\":2780265},\"commitid\":\"commit\",\"parent_commit_id\":\"parent\",\"pullid\":1,\"branch\":\"main\"}"
text='{"message":"commit","timestamp":"2024-03-25T15:41:07Z","ci_passed":true,"state":"complete","repository":{"name":"repo","is_private":false,"active":true,"language":"python","yaml":null},"author":{"avatar_url":"https://example.com","service":"github","username":null,"name":"dependabot[bot]","ownerid":2780265},"commitid":"commit","parent_commit_id":"parent","pullid":1,"branch":"main"}',
)
log_warnings_and_errors_if_any(result, "Commit creating", False)
mock_log_debug.assert_called_with('Commit creating result', extra={'extra_log_attributes': {'result': result}})
mock_log_debug.assert_called_with(
"Commit creating result", extra={"extra_log_attributes": {"result": result}}
)


def test_log_result_with_token(mocker):
Expand All @@ -72,18 +74,20 @@ def test_log_result_with_token(mocker):
error=None,
warnings=[],
status_code=201,
text="{\"message\": \"commit\", \"timestamp\": \"2024-07-16T20:51:07Z\", \"ci_passed\": true, \"state\": \"complete\", \"repository\": {\"name\": \"repo\", \"is_private\": false, \"active\": true, \"language\": \"python\", \"yaml\": {\"codecov\": {\"token\": \"faketoken\"}}, \"author\": {\"avatar_url\": \"https://example.com\", \"service\": \"github\", \"username\": \"author\", \"name\": \"author\", \"ownerid\": 3461769}, \"commitid\": \"commit\", \"parent_commit_id\": \"parent_commit\", \"pullid\": null, \"branch\": \"main\"}}"
text='{"message": "commit", "timestamp": "2024-07-16T20:51:07Z", "ci_passed": true, "state": "complete", "repository": {"name": "repo", "is_private": false, "active": true, "language": "python", "yaml": {"codecov": {"token": "faketoken"}}, "author": {"avatar_url": "https://example.com", "service": "github", "username": "author", "name": "author", "ownerid": 3461769}, "commitid": "commit", "parent_commit_id": "parent_commit", "pullid": null, "branch": "main"}}',
)

expected_text = "{\"message\": \"commit\", \"timestamp\": \"2024-07-16T20:51:07Z\", \"ci_passed\": true, \"state\": \"complete\", \"repository\": {\"name\": \"repo\", \"is_private\": false, \"active\": true, \"language\": \"python\", \"yaml\": {\"codecov\": {\"token\": \"f******************\"}}, \"author\": {\"avatar_url\": \"https://example.com\", \"service\": \"github\", \"username\": \"author\", \"name\": \"author\", \"ownerid\": 3461769}, \"commitid\": \"commit\", \"parent_commit_id\": \"parent_commit\", \"pullid\": null, \"branch\": \"main\"}}"
expected_text = '{"message": "commit", "timestamp": "2024-07-16T20:51:07Z", "ci_passed": true, "state": "complete", "repository": {"name": "repo", "is_private": false, "active": true, "language": "python", "yaml": {"codecov": {"token": "f******************"}}, "author": {"avatar_url": "https://example.com", "service": "github", "username": "author", "name": "author", "ownerid": 3461769}, "commitid": "commit", "parent_commit_id": "parent_commit", "pullid": null, "branch": "main"}}'
expected = RequestResult(
error=None,
warnings=[],
status_code=201,
text=expected_text,
)
log_warnings_and_errors_if_any(result, "Commit creating", False)
mock_log_debug.assert_called_with('Commit creating result', extra={'extra_log_attributes': {'result': expected}})
mock_log_debug.assert_called_with(
"Commit creating result", extra={"extra_log_attributes": {"result": expected}}
)


def test_get_token_header_or_fail():
Expand All @@ -102,6 +106,7 @@ def test_get_token_header_or_fail():
== "Codecov token not found. Please provide Codecov token with -t flag."
)


def test_get_token_header():
# Test with a valid UUID token
token = uuid.uuid4()
Expand Down
8 changes: 6 additions & 2 deletions tests/helpers/test_upload_sender.py
Original file line number Diff line number Diff line change
Expand Up @@ -301,10 +301,14 @@ def test_upload_sender_result_fail_post_400(

assert sender.warnings is not None


@pytest.mark.parametrize("error_code", [500, 502])
def test_upload_sender_result_fail_post_500s(
self, mocker, mocked_responses, mocked_legacy_upload_endpoint, capsys, error_code
self,
mocker,
mocked_responses,
mocked_legacy_upload_endpoint,
capsys,
error_code,
):
mocker.patch("codecov_cli.helpers.request.sleep")
mocked_legacy_upload_endpoint.status = error_code
Expand Down
Loading

0 comments on commit 14741f0

Please sign in to comment.