Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up some carry-forward related code #738

Merged
merged 4 commits into from
Sep 26, 2024
Merged
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
43 changes: 19 additions & 24 deletions helpers/labels.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
from enum import Enum
from typing import Set, Union

import sentry_sdk
from shared.reports.resources import Report

# The SpecialLabelsEnum enum is place to hold sentinels for labels with special
Expand Down Expand Up @@ -33,46 +31,43 @@ def __init__(self, label, index):
self.corresponding_index = index


@sentry_sdk.trace
def get_labels_per_session(report: Report, sess_id: int) -> Union[Set[str], Set[int]]:
def get_labels_per_session(report: Report, sess_id: int) -> set[str | int]:
"""Returns a Set with the labels present in a session from report, EXCLUDING the SpecialLabel.

The return value can either be a set of strings (the labels themselves) OR
a set of ints (the label indexes). The label can be looked up from the index
using Report.lookup_label_by_id(label_id) (assuming Report._labels_idx is set)
"""
all_labels = set()
for rf in report:
for _, line in rf.lines:
all_labels: set[str | int] = set()

for file in report:
for _, line in file.lines:
if line.datapoints:
for datapoint in line.datapoints:
if datapoint.sessionid == sess_id:
all_labels.update(datapoint.label_ids or [])
return all_labels - set(
[
SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label,
SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_index,
]
)

return all_labels - {
SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label,
SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_index,
}


@sentry_sdk.trace
def get_all_report_labels(report: Report) -> Union[Set[str], Set[int]]:
def get_all_report_labels(report: Report) -> set[str | int]:
"""Returns a Set with the labels present in report EXCLUDING the SpecialLabel.

The return value can either be a set of strings (the labels themselves) OR
a set of ints (the label indexes). The label can be looked up from the index
using Report.lookup_label_by_id(label_id) (assuming Report._labels_idx is set)
"""
all_labels = set()
for rf in report:
for _, line in rf.lines:
all_labels: set[str | int] = set()
for file in report:
for _, line in file.lines:
if line.datapoints:
for datapoint in line.datapoints:
all_labels.update(datapoint.label_ids or [])
return all_labels - set(
[
SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label,
SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_index,
]
)

return all_labels - {
SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label,
SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_index,
}
38 changes: 3 additions & 35 deletions services/path_fixer/__init__.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import logging
import os.path
import random
from collections import defaultdict
from pathlib import PurePath

import sentry_sdk
Expand Down Expand Up @@ -77,7 +75,6 @@ def __init__(
toc: list[str],
should_disable_default_pathfixes=False,
) -> None:
self.calculated_paths: dict[str | None, set[str]] = defaultdict(set)
self.toc = toc or []

self.yaml_fixes = yaml_fixes or []
Expand Down Expand Up @@ -114,9 +111,7 @@ def clean_path(self, path: str | None) -> str | None:
return path

def __call__(self, path: str, bases_to_try=None) -> str | None:
res = self.clean_path(path)
self.calculated_paths[res].add(path)
return res
return self.clean_path(path)

def get_relative_path_aware_pathfixer(self, base_path) -> "BasePathAwarePathFixer":
return BasePathAwarePathFixer(original_path_fixer=self, base_path=base_path)
Expand All @@ -125,7 +120,6 @@ def get_relative_path_aware_pathfixer(self, base_path) -> "BasePathAwarePathFixe
class BasePathAwarePathFixer(PathFixer):
def __init__(self, original_path_fixer, base_path) -> None:
self.original_path_fixer = original_path_fixer
self.unexpected_results: list[dict] = []

# base_path argument is the file path after the "# path=" in the report containing report location, if provided.
# to get the base path we use, strip the coverage report from the path to get the base path
Expand All @@ -140,6 +134,7 @@ def __call__(self, path: str, bases_to_try: list[str] | None = None) -> str | No
or not self.original_path_fixer.toc
):
return original_path_fixer_result

if not os.path.isabs(path):
all_base_paths_to_try = self.base_path + (
bases_to_try if bases_to_try is not None else []
Expand All @@ -148,33 +143,6 @@ def __call__(self, path: str, bases_to_try: list[str] | None = None) -> str | No
adjusted_path = os.path.join(base_path, path)
base_path_aware_result = self.original_path_fixer(adjusted_path)
if base_path_aware_result is not None:
self.unexpected_results.append(
{
"original_path": path,
"original_path_fixer_result": original_path_fixer_result,
"base_path_aware_result": base_path_aware_result,
}
)
return base_path_aware_result
return original_path_fixer_result

def log_abnormalities(self) -> bool:
"""
Analyze whether there were abnormalities in this pathfixer processing.
Returns:
bool: Whether abnormalities were noted or not
"""
if len(self.unexpected_results) > 0:
log.info(
"Paths did not match due to the relative path calculation",
extra=dict(
base=self.base_path,
path_patterns=sorted(self.original_path_fixer.path_patterns),
yaml_fixes=self.original_path_fixer.yaml_fixes,
some_cases=random.sample(
self.unexpected_results, min(50, len(self.unexpected_results))
),
),
)
return True
return False
return original_path_fixer_result
16 changes: 0 additions & 16 deletions services/path_fixer/tests/unit/test_path_fixer.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,6 @@ def test_basepath_uses_main_result_if_not_none_when_disagreement(self):
assert base_aware_pf("sample/path.c") == "path.c"
assert base_aware_pf("another/path.py") == "another/path.py"
assert base_aware_pf("/another/path.py") == "another/path.py"
assert len(base_aware_pf.unexpected_results) == 0
assert base_aware_pf.log_abnormalities() is False
assert not base_aware_pf.log_abnormalities()

def test_basepath_uses_own_result_if_main_is_none(self):
toc = ["project/__init__.py", "tests/__init__.py", "tests/test_project.py"]
Expand All @@ -118,13 +115,6 @@ def test_basepath_uses_own_result_if_main_is_none(self):
base_aware_pf = pf.get_relative_path_aware_pathfixer(base_path)
assert pf("__init__.py") is None
assert base_aware_pf("__init__.py") == "project/__init__.py"
assert base_aware_pf.log_abnormalities()
assert len(base_aware_pf.unexpected_results) == 1
assert base_aware_pf.unexpected_results.pop() == {
"original_path": "__init__.py",
"original_path_fixer_result": None,
"base_path_aware_result": "project/__init__.py",
}

def test_basepath_uses_own_result_if_main_is_none_multuple_base_paths(self):
toc = ["project/__init__.py", "tests/__init__.py", "tests/test_project.py"]
Expand All @@ -137,9 +127,3 @@ def test_basepath_uses_own_result_if_main_is_none_multuple_base_paths(self):
base_aware_pf("__init__.py", bases_to_try=["/home/travis/build/project"])
== "project/__init__.py"
)
assert base_aware_pf.log_abnormalities()
assert base_aware_pf.unexpected_results.pop() == {
"original_path": "__init__.py",
"original_path_fixer_result": None,
"base_path_aware_result": "project/__init__.py",
}
Loading
Loading