Skip to content

Commit

Permalink
Clean up some carry-forward related code (#738)
Browse files Browse the repository at this point in the history
  • Loading branch information
Swatinem authored Sep 26, 2024
1 parent 3bee0c1 commit f634f37
Show file tree
Hide file tree
Showing 11 changed files with 276 additions and 511 deletions.
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

0 comments on commit f634f37

Please sign in to comment.