From fc29e8fdc2b65bfd1716f30c39b951d857604101 Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Mon, 14 Oct 2024 13:07:29 +0200 Subject: [PATCH 1/5] Remove `label_index` feature flag It turns out this feature flag was never fully rolled out, and is part of ATS (automated test selection) which was also not fully launched. This will now remove this feature flag, as a precursor to removing labels altogether. --- rollouts/__init__.py | 6 - services/report/languages/pycoverage.py | 23 +- .../report/languages/tests/unit/__init__.py | 2 - .../unit/test_pycoverage_encoded_labels.py | 648 --------- services/report/raw_upload_processor.py | 150 +- services/report/report_builder.py | 58 +- .../test_report_builder_encoded_labels.py | 367 ----- services/report/tests/unit/test_sessions.py | 9 +- .../unit/test_sessions_encoded_labels.py | 1217 ----------------- .../tests/unit/test_upload_processing_task.py | 25 - 10 files changed, 30 insertions(+), 2475 deletions(-) delete mode 100644 services/report/languages/tests/unit/test_pycoverage_encoded_labels.py delete mode 100644 services/report/tests/unit/test_report_builder_encoded_labels.py delete mode 100644 services/report/tests/unit/test_sessions_encoded_labels.py diff --git a/rollouts/__init__.py b/rollouts/__init__.py index 67003e32f..e22cf385d 100644 --- a/rollouts/__init__.py +++ b/rollouts/__init__.py @@ -4,12 +4,6 @@ FLAKY_TEST_DETECTION = Feature("flaky_test_detection") FLAKY_SHADOW_MODE = Feature("flaky_shadow_mode") -# Eventually we want all repos to use this -# This flag will just help us with the rollout process -USE_LABEL_INDEX_IN_REPORT_PROCESSING_BY_REPO_ID = Feature( - "use_label_index_in_report_processing" -) - PARALLEL_UPLOAD_PROCESSING_BY_REPO = Feature("parallel_upload_processing") CARRYFORWARD_BASE_SEARCH_RANGE_BY_OWNER = Feature("carryforward_base_search_range") diff --git a/services/report/languages/pycoverage.py b/services/report/languages/pycoverage.py index 16fd0770d..af28bc141 100644 --- a/services/report/languages/pycoverage.py +++ b/services/report/languages/pycoverage.py @@ -28,22 +28,13 @@ def process( ] + [(COVERAGE_MISS, ln) for ln in file_coverage["missing_lines"]] for cov, ln in lines_and_coverage: if ln > 0: - label_list_of_lists: list[list[str]] | list[list[int]] = [] - if report_builder_session.should_use_label_index: - label_list_of_lists = [ - [single_id] - for single_id in labels_table._get_list_of_label_ids( - report_builder_session.label_index, - file_coverage.get("contexts", {}).get(str(ln), []), - ) - ] - else: - label_list_of_lists = [ - [labels_table._normalize_label(testname)] - for testname in file_coverage.get("contexts", {}).get( - str(ln), [] - ) - ] + # label_list_of_lists: list[list[str]] | list[list[int]] = [] + label_list_of_lists = [ + [labels_table._normalize_label(testname)] + for testname in file_coverage.get("contexts", {}).get( + str(ln), [] + ) + ] _file.append( ln, report_builder_session.create_coverage_line( diff --git a/services/report/languages/tests/unit/__init__.py b/services/report/languages/tests/unit/__init__.py index b736d0c8a..d3b12bf7d 100644 --- a/services/report/languages/tests/unit/__init__.py +++ b/services/report/languages/tests/unit/__init__.py @@ -6,7 +6,6 @@ def create_report_builder_session( path_fixer: PathFixer | None = None, filename: str = "filename", current_yaml: dict | None = None, - should_use_label_index: bool = False, ) -> ReportBuilderSession: def fixes(filename, bases_to_try=None): return filename @@ -16,6 +15,5 @@ def fixes(filename, bases_to_try=None): ignored_lines={}, sessionid=0, current_yaml=current_yaml, - should_use_label_index=should_use_label_index, ) return report_builder.create_report_builder_session(filename) diff --git a/services/report/languages/tests/unit/test_pycoverage_encoded_labels.py b/services/report/languages/tests/unit/test_pycoverage_encoded_labels.py deleted file mode 100644 index 134fe59b6..000000000 --- a/services/report/languages/tests/unit/test_pycoverage_encoded_labels.py +++ /dev/null @@ -1,648 +0,0 @@ -from services.report.languages.pycoverage import LabelsTable, PyCoverageProcessor -from services.report.report_builder import SpecialLabelsEnum -from test_utils.base import BaseTestCase - -from . import create_report_builder_session - -SAMPLE = { - "meta": { - "version": "6.4.1", - "timestamp": "2022-06-27T01:44:41.238230", - "branch_coverage": False, - "show_contexts": True, - }, - "files": { - "another.py": { - "executed_lines": [1, 2, 3, 4], - "summary": { - "covered_lines": 4, - "num_statements": 4, - "percent_covered": 100.0, - "percent_covered_display": "100", - "missing_lines": 0, - "excluded_lines": 0, - }, - "missing_lines": [], - "excluded_lines": [], - "contexts": { - "1": [""], - "2": [ - "test_another.py::test_fib_simple_case|run", - "test_another.py::test_fib_bigger_cases|run", - ], - "3": [ - "test_another.py::test_fib_simple_case|run", - "test_another.py::test_fib_bigger_cases|run", - ], - "4": ["test_another.py::test_fib_bigger_cases|run"], - }, - }, - "source.py": { - "executed_lines": [1, 3, 4, 5, 9], - "summary": { - "covered_lines": 5, - "num_statements": 7, - "percent_covered": 71.42857142857143, - "percent_covered_display": "71", - "missing_lines": 2, - "excluded_lines": 0, - }, - "missing_lines": [6, 10], - "excluded_lines": [], - "contexts": { - "1": [""], - "3": [""], - "9": [""], - "4": ["test_source.py::test_some_code|run"], - "5": ["test_source.py::test_some_code|run"], - }, - }, - "test_another.py": { - "executed_lines": [1, 3, 4, 5, 7, 8], - "summary": { - "covered_lines": 6, - "num_statements": 6, - "percent_covered": 100.0, - "percent_covered_display": "100", - "missing_lines": 0, - "excluded_lines": 0, - }, - "missing_lines": [], - "excluded_lines": [], - "contexts": { - "1": [""], - "3": [""], - "7": [""], - "4": ["test_another.py::test_fib_simple_case|run"], - "5": ["test_another.py::test_fib_simple_case|run"], - "8": ["test_another.py::test_fib_bigger_cases|run"], - }, - }, - "test_source.py": { - "executed_lines": [1, 4, 5], - "summary": { - "covered_lines": 3, - "num_statements": 3, - "percent_covered": 100.0, - "percent_covered_display": "100", - "missing_lines": 0, - "excluded_lines": 0, - }, - "missing_lines": [], - "excluded_lines": [], - "contexts": { - "1": [""], - "4": [""], - "5": ["test_source.py::test_some_code|run"], - }, - }, - }, - "totals": { - "covered_lines": 18, - "num_statements": 20, - "percent_covered": 90.0, - "percent_covered_display": "90", - "missing_lines": 2, - "excluded_lines": 0, - }, -} - -COMPRESSED_SAMPLE = { - "meta": { - "version": "6.5.0", - "timestamp": "2023-05-15T18:35:30.641570", - "branch_coverage": False, - "show_contexts": True, - }, - "totals": { - "covered_lines": 4, - "num_statements": 9, - "percent_covered": "44.44444", - "percent_covered_display": "44", - "missing_lines": 5, - "excluded_lines": 0, - }, - "files": { - "awesome.py": { - "executed_lines": [1, 2, 3, 5], - "summary": { - "covered_lines": 4, - "num_statements": 5, - "percent_covered": "80.0", - "percent_covered_display": "80", - "missing_lines": 1, - "excluded_lines": 0, - }, - "missing_lines": [4], - "excluded_lines": [], - "contexts": { - "1": [0], - "2": [1, 2], - "3": [2, 3], - "5": [4], - }, - }, - "__init__.py": { - "executed_lines": [], - "summary": { - "covered_lines": 0, - "num_statements": 4, - "percent_covered": "0.0", - "percent_covered_display": "0", - "missing_lines": 4, - "excluded_lines": 0, - }, - "missing_lines": [1, 3, 4, 5], - "excluded_lines": [], - "contexts": {}, - }, - "services/__init__.py": { - "executed_lines": [0], - "summary": { - "covered_lines": 0, - "num_statements": 0, - "percent_covered": "100.0", - "percent_covered_display": "100", - "missing_lines": 0, - "excluded_lines": 0, - }, - "missing_lines": [], - "excluded_lines": [], - "contexts": {"0": [0]}, - }, - }, - "labels_table": { - "0": "", - "1": "label_1", - "2": "label_2", - "3": "label_3", - "4": "label_5", - }, -} - - -class TestPyCoverageProcessor(BaseTestCase): - def test_matches_content_pycoverage(self): - p = PyCoverageProcessor() - assert p.matches_content(SAMPLE, "", "coverage.json") - assert not p.matches_content({"meta": True}, "", "coverage.json") - assert not p.matches_content({"meta": {}}, "", "coverage.json") - - def test__get_list_of_label_ids(self): - p = LabelsTable(None, {}) - current_label_idx = {} - assert p._get_list_of_label_ids(current_label_idx, [""]) == [1] - assert current_label_idx == { - 1: SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label - } - assert p._get_list_of_label_ids( - current_label_idx, ["test_source.py::test_some_code|run"] - ) == [2] - assert current_label_idx == { - 1: SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label, - 2: "test_source.py::test_some_code", - } - assert p._get_list_of_label_ids( - current_label_idx, ["", "test_source.py::test_some_code|run"] - ) == [1, 2] - assert current_label_idx == { - 1: SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label, - 2: "test_source.py::test_some_code", - } - - def test__get_list_of_label_ids_already_encoded(self): - p = LabelsTable(None, {}) - p.are_labels_already_encoded = True - assert p._get_list_of_label_ids({}, ["2"]) == [2] - assert p._get_list_of_label_ids({}, ["2", "3", "1"]) == [1, 2, 3] - - def test_process_pycoverage(self): - content = SAMPLE - p = PyCoverageProcessor() - report_builder_session = create_report_builder_session( - current_yaml={ - "flag_management": { - "default_rules": { - "carryforward": "true", - "carryforward_mode": "labels", - } - } - }, - should_use_label_index=True, - ) - p.process(content, report_builder_session) - report = report_builder_session.output_report() - - assert report.labels_index == { - 1: SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label, - 2: "test_another.py::test_fib_simple_case", - 3: "test_another.py::test_fib_bigger_cases", - 4: "test_source.py::test_some_code", - } - processed_report = self.convert_report_to_better_readable(report) - - assert processed_report["archive"]["source.py"][0] == ( - 1, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [1])], - ) - assert processed_report == { - "archive": { - "another.py": [ - ( - 1, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [1])], - ), - ( - 2, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [ - (0, 1, None, [2]), - (0, 1, None, [3]), - ], - ), - ( - 3, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [ - (0, 1, None, [2]), - (0, 1, None, [3]), - ], - ), - ( - 4, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [3])], - ), - ], - "source.py": [ - ( - 1, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [1])], - ), - ( - 3, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [1])], - ), - ( - 4, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [4])], - ), - ( - 5, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [4])], - ), - ( - 6, - 0, - None, - [[0, 0, None, None, None]], - None, - None, - [], - ), - ( - 9, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [1])], - ), - ( - 10, - 0, - None, - [[0, 0, None, None, None]], - None, - None, - [], - ), - ], - "test_another.py": [ - ( - 1, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [1])], - ), - ( - 3, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [1])], - ), - ( - 4, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [2])], - ), - ( - 5, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [2])], - ), - ( - 7, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [1])], - ), - ( - 8, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [3])], - ), - ], - "test_source.py": [ - ( - 1, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [1])], - ), - ( - 4, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [1])], - ), - ( - 5, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [4])], - ), - ], - }, - "report": { - "files": { - "another.py": [ - 0, - [0, 4, 4, 0, 0, "100", 0, 0, 0, 0, 0, 0, 0], - None, - None, - ], - "source.py": [ - 1, - [0, 7, 5, 2, 0, "71.42857", 0, 0, 0, 0, 0, 0, 0], - None, - None, - ], - "test_another.py": [ - 2, - [0, 6, 6, 0, 0, "100", 0, 0, 0, 0, 0, 0, 0], - None, - None, - ], - "test_source.py": [ - 3, - [0, 3, 3, 0, 0, "100", 0, 0, 0, 0, 0, 0, 0], - None, - None, - ], - }, - "sessions": {}, - }, - "totals": { - "f": 4, - "n": 20, - "h": 18, - "m": 2, - "p": 0, - "c": "90.00000", - "b": 0, - "d": 0, - "M": 0, - "s": 0, - "C": 0, - "N": 0, - "diff": None, - }, - } - - def test_process_compressed_report(self): - content = COMPRESSED_SAMPLE - p = PyCoverageProcessor() - report_builder_session = create_report_builder_session( - current_yaml={ - "flag_management": { - "default_rules": { - "carryforward": "true", - "carryforward_mode": "labels", - } - } - }, - should_use_label_index=True, - ) - p.process(content, report_builder_session) - report = report_builder_session.output_report() - - assert report.labels_index == { - 0: SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label, - 1: "label_1", - 2: "label_2", - 3: "label_3", - 4: "label_5", - } - processed_report = self.convert_report_to_better_readable(report) - - assert processed_report == { - "archive": { - "awesome.py": [ - ( - 1, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [0])], - ), - ( - 2, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [ - (0, 1, None, [1]), - (0, 1, None, [2]), - ], - ), - ( - 3, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [ - (0, 1, None, [2]), - (0, 1, None, [3]), - ], - ), - ( - 4, - 0, - None, - [[0, 0, None, None, None]], - None, - None, - [], - ), - ( - 5, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [4])], - ), - ], - "__init__.py": [ - ( - 1, - 0, - None, - [[0, 0, None, None, None]], - None, - None, - [], - ), - ( - 3, - 0, - None, - [[0, 0, None, None, None]], - None, - None, - [], - ), - ( - 4, - 0, - None, - [[0, 0, None, None, None]], - None, - None, - [], - ), - ( - 5, - 0, - None, - [[0, 0, None, None, None]], - None, - None, - [], - ), - ], - }, - "report": { - "files": { - "awesome.py": [ - 0, - [0, 5, 4, 1, 0, "80.00000", 0, 0, 0, 0, 0, 0, 0], - None, - None, - ], - "__init__.py": [ - 1, - [0, 4, 0, 4, 0, "0", 0, 0, 0, 0, 0, 0, 0], - None, - None, - ], - }, - "sessions": {}, - }, - "totals": { - "f": 2, - "n": 9, - "h": 4, - "m": 5, - "p": 0, - "c": "44.44444", - "b": 0, - "d": 0, - "M": 0, - "s": 0, - "C": 0, - "N": 0, - "diff": None, - }, - } diff --git a/services/report/raw_upload_processor.py b/services/report/raw_upload_processor.py index cc75f7026..f0fc85069 100644 --- a/services/report/raw_upload_processor.py +++ b/services/report/raw_upload_processor.py @@ -1,5 +1,4 @@ import logging -import typing from dataclasses import dataclass import sentry_sdk @@ -10,19 +9,14 @@ from database.models.reports import Upload from helpers.exceptions import ReportEmptyError, ReportExpiredException from helpers.labels import get_all_report_labels, get_labels_per_session -from rollouts import USE_LABEL_INDEX_IN_REPORT_PROCESSING_BY_REPO_ID from services.path_fixer import PathFixer from services.report.parser.types import ParsedRawReport -from services.report.report_builder import ReportBuilder, SpecialLabelsEnum +from services.report.report_builder import ReportBuilder from services.report.report_processor import process_report from services.yaml import read_yaml_field log = logging.getLogger(__name__) -DEFAULT_LABEL_INDEX = { - SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_index: SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label -} - @dataclass class SessionAdjustmentResult: @@ -84,17 +78,6 @@ def process_raw_upload( temporary_report = Report() - should_use_encoded_labels = ( - upload - and USE_LABEL_INDEX_IN_REPORT_PROCESSING_BY_REPO_ID.check_value( - identifier=upload.report.commit.repository.repoid, default=False - ) - ) - if should_use_encoded_labels: - # We initialize the labels_index (which defaults to {}) to force the special label - # to always be index 0 - temporary_report.labels_index = dict(DEFAULT_LABEL_INDEX) - joined = True for flag in flags or []: if read_yaml_field(commit_yaml, ("flags", flag, "joined")) is False: @@ -120,7 +103,6 @@ def process_raw_upload( sessionid, ignored_lines, path_fixer_to_use, - should_use_encoded_labels, ) try: report_from_file = process_report( @@ -131,24 +113,11 @@ def process_raw_upload( raise if report_from_file: - if should_use_encoded_labels: - # Copies the labels from report into temporary_report - # If needed - make_sure_label_indexes_match(temporary_report, report_from_file) temporary_report.merge(report_from_file, joined=True) if not temporary_report: raise ReportEmptyError("No files found in report.") - if ( - should_use_encoded_labels - and temporary_report.labels_index == DEFAULT_LABEL_INDEX - ): - # This means that, even though this report _could_ use encoded labels, - # none of the reports processed contributed any new labels to it. - # So we assume there are no labels and just reset the _labels_index of temporary_report - temporary_report.labels_index = None - # Now we actually add the session to the original_report # Because we know that the processing was successful _sessionid, session = report.add_session(session, use_id_from_session=True) @@ -166,110 +135,6 @@ def process_raw_upload( return UploadProcessingResult(report=report, session_adjustment=session_adjustment) -@sentry_sdk.trace -def make_sure_orginal_report_is_using_label_ids(original_report: Report): - """Makes sure that the original_report (that was pulled from DB) - has CoverageDatapoints that encode label_ids and not actual labels. - """ - # Always point the special label to index 0 - reverse_index_cache = { - SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label: SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_index - } - if original_report.labels_index is None: - original_report.labels_index = {} - labels_index = original_report.labels_index - - if ( - SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_index - not in labels_index - ): - labels_index[ - SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_index - ] = SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label - - def possibly_translate_label(label_or_id: typing.Union[str, int]) -> int: - if isinstance(label_or_id, int): - return label_or_id - if label_or_id in reverse_index_cache: - return reverse_index_cache[label_or_id] - # Search for label in the report index - for idx, label in labels_index.items(): - if label == label_or_id: - reverse_index_cache[label] = idx - return idx - # Label is not present. Add to index. - # Notice that this never picks index 0, that is reserved for the special label - new_index = max(labels_index.keys()) + 1 - reverse_index_cache[label_or_id] = new_index - # It's OK to update this here because it's inside the - # UploadProcessing lock, so it's exclusive access - labels_index[new_index] = label_or_id - return new_index - - for report_file in original_report: - for _, report_line in report_file.lines: - if report_line.datapoints: - for datapoint in report_line.datapoints: - datapoint.label_ids = [ - possibly_translate_label(label_or_id) - for label_or_id in datapoint.label_ids - ] - report_line.datapoints.sort(key=lambda x: x.key_sorting_tuple()) - - -@sentry_sdk.trace -def make_sure_label_indexes_match( - original_report: Report, to_merge_report: Report -) -> None: - """Makes sure that the indexes of both reports point to the same labels. - Uses the original_report as reference, and fixes the to_merge_report as needed - it also extendes the original_report.labels_index with new labels as needed. - """ - if to_merge_report.labels_index is None or original_report.labels_index is None: - # The new report doesn't have labels to fix - return - - # Map label --> index_in_original_report - reverse_index: typing.Dict[str, int] = { - t[1]: t[0] for t in original_report.labels_index.items() - } - # Map index_in_to_merge_report --> index_in_original_report - indexes_to_fix: typing.Dict[int, int] = {} - next_idx = max(original_report.labels_index.keys()) + 1 - for idx, label in to_merge_report.labels_index.items(): - # Special case for the special label, which is SpecialLabelsEnum in to_merge_report - # But in the original_report it points to a string - if label == SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER: - if ( - idx - != SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_index - ): - indexes_to_fix[idx] = ( - SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_index - ) - if label not in reverse_index: - # It's a new label that doesn't exist in the original_report - original_report.labels_index[next_idx] = label - indexes_to_fix[idx] = next_idx - next_idx += 1 - elif reverse_index[label] == idx: - # This label matches the index on the original report - continue - else: - # Here the label doesn't match the index in the original report - indexes_to_fix[idx] = reverse_index[label] - - # Fix indexes in to_merge_report. - for report_file in to_merge_report: - for _, report_line in report_file.lines: - if report_line.datapoints: - for datapoint in report_line.datapoints: - datapoint.label_ids = [ - indexes_to_fix.get(label_id, label_id) - for label_id in datapoint.label_ids - ] - - @sentry_sdk.trace def clear_carryforward_sessions( original_report: Report, @@ -277,7 +142,7 @@ def clear_carryforward_sessions( to_merge_flags: list[str], current_yaml: UserYaml, upload: Upload | None = None, -): +) -> SessionAdjustmentResult: flags_under_carryforward_rules = { f for f in to_merge_flags if current_yaml.flag_has_carryfoward(f) } @@ -293,17 +158,6 @@ def clear_carryforward_sessions( if upload is None and to_partially_overwrite_flags: log.warning("Upload is None, but there are partial_overwrite_flags present") - if ( - upload - and USE_LABEL_INDEX_IN_REPORT_PROCESSING_BY_REPO_ID.check_value( - identifier=upload.report.commit.repository.repoid, default=False - ) - and to_partially_overwrite_flags - ): - # Make sure that the labels in the reports are in a good state to merge them - make_sure_orginal_report_is_using_label_ids(original_report) - make_sure_label_indexes_match(original_report, to_merge_report) - session_ids_to_fully_delete = [] session_ids_to_partially_delete = [] diff --git a/services/report/report_builder.py b/services/report/report_builder.py index 3b33c8111..4358fb463 100644 --- a/services/report/report_builder.py +++ b/services/report/report_builder.py @@ -32,14 +32,12 @@ def __init__( self, report_builder: "ReportBuilder", report_filepath: str, - should_use_label_index: bool = False, ): self.filepath = report_filepath self._report_builder = report_builder self._report = Report() self.label_index = {} self._present_labels = set() - self.should_use_label_index = should_use_label_index @property def path_fixer(self): @@ -55,15 +53,13 @@ def get_file(self, filename: str) -> ReportFile | None: return self._report.get(filename) def append(self, file: ReportFile): - if not self.should_use_label_index: - # TODO: [codecov/engineering-team#869] This behavior can be removed after label indexing is rolled out for all customers - if file is not None: - for line_number, line in file.lines: - if line.datapoints: - for datapoint in line.datapoints: - if datapoint.label_ids: - for label in datapoint.label_ids: - self._present_labels.add(label) + if file is not None: + for line_number, line in file.lines: + if line.datapoints: + for datapoint in line.datapoints: + if datapoint.label_ids: + for label in datapoint.label_ids: + self._present_labels.add(label) return self._report.append(file) def output_report(self) -> Report: @@ -75,33 +71,21 @@ def output_report(self) -> Report: Returns: Report: The legacy report desired """ - if self.should_use_label_index: - if len(self.label_index) > 0: - if len(self.label_index) == 1 and self.label_index.values() == [ - SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER - ]: - log.warning( - "Report only has SpecialLabels. Might indicate it was not generated with contexts" - ) - self._report._totals = None - self._report.labels_index = self.label_index - else: - if self._present_labels: - if self._present_labels and self._present_labels == { - SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER - }: - log.warning( - "Report only has SpecialLabels. Might indicate it was not generated with contexts" + if self._present_labels: + if self._present_labels and self._present_labels == { + SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER + }: + log.warning( + "Report only has SpecialLabels. Might indicate it was not generated with contexts" + ) + for file in self._report: + for line_number, line in file.lines: + self._possibly_modify_line_to_account_for_special_labels( + file, line_number, line ) - for file in self._report: - for line_number, line in file.lines: - self._possibly_modify_line_to_account_for_special_labels( - file, line_number, line - ) - self._report._totals = None + self._report._totals = None return self._report - # TODO: [codecov/engineering-team#869] This behavior can be removed after label indexing is rolled out for all customers def _possibly_modify_line_to_account_for_special_labels( self, file: ReportFile, line_number: int, line: ReportLine ) -> None: @@ -237,16 +221,14 @@ def __init__( sessionid: int, ignored_lines: dict, path_fixer: PathFixer, - should_use_label_index: bool = False, ): self.current_yaml = current_yaml self.sessionid = sessionid self.ignored_lines = ignored_lines self.path_fixer = path_fixer - self.should_use_label_index = should_use_label_index def create_report_builder_session(self, filepath) -> ReportBuilderSession: - return ReportBuilderSession(self, filepath, self.should_use_label_index) + return ReportBuilderSession(self, filepath) def supports_labels(self) -> bool: """Returns wether a report supports labels. diff --git a/services/report/tests/unit/test_report_builder_encoded_labels.py b/services/report/tests/unit/test_report_builder_encoded_labels.py deleted file mode 100644 index 311b744fb..000000000 --- a/services/report/tests/unit/test_report_builder_encoded_labels.py +++ /dev/null @@ -1,367 +0,0 @@ -import pytest -from shared.reports.resources import LineSession, ReportFile, ReportLine -from shared.reports.types import CoverageDatapoint - -from services.report.report_builder import ( - CoverageType, - ReportBuilder, - SpecialLabelsEnum, -) - - -def test_report_builder_generate_session(mocker): - current_yaml, sessionid, ignored_lines, path_fixer = ( - mocker.MagicMock(), - mocker.MagicMock(), - mocker.MagicMock(), - mocker.MagicMock(), - ) - filepath = "filepath" - builder = ReportBuilder( - current_yaml, sessionid, ignored_lines, path_fixer, should_use_label_index=True - ) - builder_session = builder.create_report_builder_session(filepath) - assert builder_session.path_fixer == path_fixer - - -def test_report_builder_session(mocker): - current_yaml, sessionid, ignored_lines, path_fixer = ( - {}, - mocker.MagicMock(), - mocker.MagicMock(), - mocker.MagicMock(), - ) - filepath = "filepath" - builder = ReportBuilder( - current_yaml, sessionid, ignored_lines, path_fixer, should_use_label_index=True - ) - builder_session = builder.create_report_builder_session(filepath) - first_file = ReportFile("filename.py") - first_file.append(2, ReportLine.create(coverage=0)) - labels_index = { - 0: SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label, - 1: "some_label", - 2: "other", - } - builder_session.label_index = labels_index - first_file.append( - 3, - ReportLine.create( - coverage=0, - datapoints=[ - CoverageDatapoint( - sessionid=0, - coverage=1, - coverage_type=None, - label_ids=[0], - ) - ], - ), - ) - first_file.append( - 10, - ReportLine.create( - coverage=1, - type=None, - sessions=[ - ( - LineSession( - id=0, - coverage=1, - ) - ) - ], - datapoints=[ - CoverageDatapoint( - sessionid=0, - coverage=1, - coverage_type=None, - label_ids=[1, 2], - ), - CoverageDatapoint( - sessionid=0, - coverage=1, - coverage_type=None, - label_ids=None, - ), - ], - complexity=None, - ), - ) - builder_session.append(first_file) - final_report = builder_session.output_report() - assert final_report.labels_index == labels_index - assert final_report.files == ["filename.py"] - assert sorted(final_report.get("filename.py").lines) == [ - ( - 2, - ReportLine.create( - coverage=0, type=None, sessions=None, datapoints=None, complexity=None - ), - ), - ( - 3, - ReportLine.create( - coverage=0, - type=None, - sessions=None, - datapoints=[ - CoverageDatapoint( - sessionid=0, - coverage=1, - coverage_type=None, - label_ids=[0], - ), - ], - complexity=None, - ), - ), - ( - 10, - ReportLine.create( - coverage=1, - type=None, - sessions=[ - LineSession( - id=0, coverage=1, branches=None, partials=None, complexity=None - ) - ], - datapoints=[ - CoverageDatapoint( - sessionid=0, - coverage=1, - coverage_type=None, - label_ids=[1, 2], - ), - CoverageDatapoint( - sessionid=0, - coverage=1, - coverage_type=None, - label_ids=None, - ), - ], - complexity=None, - ), - ), - ] - - -def test_report_builder_session_only_all_labels(mocker): - current_yaml, sessionid, ignored_lines, path_fixer = ( - {}, - mocker.MagicMock(), - mocker.MagicMock(), - mocker.MagicMock(), - ) - labels_index = { - 0: SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label, - } - filepath = "filepath" - builder = ReportBuilder( - current_yaml, sessionid, ignored_lines, path_fixer, should_use_label_index=True - ) - builder_session = builder.create_report_builder_session(filepath) - builder_session.label_index = labels_index - first_file = ReportFile("filename.py") - first_file.append(2, ReportLine.create(coverage=0)) - first_file.append( - 3, - ReportLine.create( - coverage=0, - datapoints=[ - CoverageDatapoint( - sessionid=0, - coverage=1, - coverage_type=None, - label_ids=[0], - ) - ], - ), - ) - first_file.append( - 10, - ReportLine.create( - coverage=1, - type=None, - sessions=[ - ( - LineSession( - id=0, - coverage=1, - ) - ) - ], - datapoints=[ - CoverageDatapoint( - sessionid=0, - coverage=1, - coverage_type=None, - label_ids=[0], - ), - CoverageDatapoint( - sessionid=0, - coverage=1, - coverage_type=None, - label_ids=None, - ), - ], - complexity=None, - ), - ) - builder_session.append(first_file) - final_report = builder_session.output_report() - assert final_report.labels_index == labels_index - assert final_report.files == ["filename.py"] - assert sorted(final_report.get("filename.py").lines) == [ - ( - 2, - ReportLine.create( - coverage=0, type=None, sessions=None, datapoints=None, complexity=None - ), - ), - ( - 3, - ReportLine.create( - coverage=0, - type=None, - sessions=None, - datapoints=[ - CoverageDatapoint( - sessionid=0, - coverage=1, - coverage_type=None, - label_ids=[0], - ), - ], - complexity=None, - ), - ), - ( - 10, - ReportLine.create( - coverage=1, - type=None, - sessions=[ - LineSession( - id=0, coverage=1, branches=None, partials=None, complexity=None - ) - ], - datapoints=[ - CoverageDatapoint( - sessionid=0, - coverage=1, - coverage_type=None, - label_ids=[0], - ), - CoverageDatapoint( - sessionid=0, - coverage=1, - coverage_type=None, - label_ids=None, - ), - ], - complexity=None, - ), - ), - ] - - -def test_report_builder_session_create_line(mocker): - current_yaml, sessionid, ignored_lines, path_fixer = ( - { - "flag_management": { - "default_rules": { - "carryforward": "true", - "carryforward_mode": "labels", - } - } - }, - 45, - mocker.MagicMock(), - mocker.MagicMock(), - ) - filepath = "filepath" - builder = ReportBuilder(current_yaml, sessionid, ignored_lines, path_fixer) - builder_session = builder.create_report_builder_session(filepath) - line = builder_session.create_coverage_line( - 1, - CoverageType.branch, - labels_list_of_lists=[[], [0], [1]], - ) - assert line == ReportLine.create( - coverage=1, - type="b", - sessions=[ - LineSession( - id=45, coverage=1, branches=None, partials=None, complexity=None - ) - ], - datapoints=[ - CoverageDatapoint( - sessionid=45, coverage=1, coverage_type="b", label_ids=[0] - ), - CoverageDatapoint( - sessionid=45, coverage=1, coverage_type="b", label_ids=[1] - ), - ], - complexity=None, - ) - - -@pytest.mark.parametrize( - "current_yaml,expected_result", - [ - ({}, False), - ({"flags": {"oldflag": {"carryforward": "true"}}}, False), - ( - { - "flags": { - "oldflag": {"carryforward": "true", "carryforward_mode": "labels"} - } - }, - True, - ), - ( - { - "flag_management": { - "default_rules": { - "carryforward": "true", - "carryforward_mode": "labels", - } - } - }, - True, - ), - ( - { - "flag_management": { - "default_rules": { - "carryforward": "true", - "carryforward_mode": "all", - } - } - }, - False, - ), - ( - { - "flag_management": { - "default_rules": { - "carryforward": "true", - "carryforward_mode": "all", - }, - "individual_flags": [ - { - "name": "some_flag", - "carryforward_mode": "labels", - } - ], - } - }, - True, - ), - ], -) -def test_report_builder_supports_flags(current_yaml, expected_result): - builder = ReportBuilder(current_yaml, 0, None, None) - assert builder.supports_labels() == expected_result diff --git a/services/report/tests/unit/test_sessions.py b/services/report/tests/unit/test_sessions.py index b7d8d6857..a8ed9beae 100644 --- a/services/report/tests/unit/test_sessions.py +++ b/services/report/tests/unit/test_sessions.py @@ -13,7 +13,6 @@ from shared.yaml import UserYaml from helpers.labels import SpecialLabelsEnum -from rollouts import USE_LABEL_INDEX_IN_REPORT_PROCESSING_BY_REPO_ID from services.report.raw_upload_processor import ( SessionAdjustmentResult, clear_carryforward_sessions, @@ -442,14 +441,8 @@ def test_adjust_sessions_partial_cf_only_no_changes( assert after_result == first_value def test_adjust_sessions_partial_cf_only_no_changes_encoding_labels( - self, sample_first_report, mocker + self, sample_first_report ): - mocker.patch.object( - USE_LABEL_INDEX_IN_REPORT_PROCESSING_BY_REPO_ID, - "check_value", - return_value=False, - ) - first_to_merge_session = Session(flags=["enterprise"], id=3) second_report = Report( sessions={first_to_merge_session.id: first_to_merge_session} diff --git a/services/report/tests/unit/test_sessions_encoded_labels.py b/services/report/tests/unit/test_sessions_encoded_labels.py deleted file mode 100644 index 7e73ed061..000000000 --- a/services/report/tests/unit/test_sessions_encoded_labels.py +++ /dev/null @@ -1,1217 +0,0 @@ -import pytest -from mock import MagicMock -from shared.reports.editable import EditableReport, EditableReportFile -from shared.reports.resources import ( - LineSession, - Report, - ReportFile, - ReportLine, - Session, - SessionType, -) -from shared.reports.types import CoverageDatapoint -from shared.yaml import UserYaml - -from helpers.labels import SpecialLabelsEnum -from rollouts import USE_LABEL_INDEX_IN_REPORT_PROCESSING_BY_REPO_ID -from services.report.raw_upload_processor import ( - SessionAdjustmentResult, - clear_carryforward_sessions, - make_sure_label_indexes_match, -) -from test_utils.base import BaseTestCase - -# Not calling add_sessions here on purpose, so it doesnt -# interfere with this logic - - -class TestAdjustSession(BaseTestCase): - @pytest.fixture - def sample_first_report(self): - report_label_idx = { - 0: SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label, - 1: "one_label", - 2: "another_label", - } - first_report = EditableReport( - sessions={ - 0: Session( - flags=["enterprise"], - id=0, - session_type=SessionType.carriedforward, - ), - 1: Session( - flags=["enterprise"], id=1, session_type=SessionType.uploaded - ), - 2: Session( - flags=["unit"], id=2, session_type=SessionType.carriedforward - ), - 3: Session( - flags=["unrelated"], id=3, session_type=SessionType.uploaded - ), - } - ) - first_file = EditableReportFile("first_file.py") - c = 0 - for list_of_lists_of_labels in [ - [[1]], - [[2]], - [[2], [1]], - [[2, 1]], - [[0]], - ]: - for sessionid in range(4): - first_file.append( - c % 7 + 1, - self.create_sample_line( - coverage=c, - sessionid=sessionid, - list_of_lists_of_labels=list_of_lists_of_labels, - ), - ) - c += 1 - second_file = EditableReportFile("second_file.py") - first_report.append(first_file) - first_report.append(second_file) - - assert self.convert_report_to_better_readable(first_report)["archive"] == { - "first_file.py": [ - ( - 1, - 14, - None, - [ - [0, 0, None, None, None], - [3, 7, None, None, None], - [2, 14, None, None, None], - ], - None, - None, - [ - (0, 0, None, [1]), - (2, 14, None, [2, 1]), - (3, 7, None, [2]), - ], - ), - ( - 2, - 15, - None, - [ - [1, 1, None, None, None], - [0, 8, None, None, None], - [3, 15, None, None, None], - ], - None, - None, - [ - (0, 8, None, [1]), - (0, 8, None, [2]), - (1, 1, None, [1]), - (3, 15, None, [2, 1]), - ], - ), - ( - 3, - 16, - None, - [ - [2, 2, None, None, None], - [1, 9, None, None, None], - [0, 16, None, None, None], - ], - None, - None, - [ - (0, 16, None, [0]), - (1, 9, None, [1]), - (1, 9, None, [2]), - (2, 2, None, [1]), - ], - ), - ( - 4, - 17, - None, - [ - [3, 3, None, None, None], - [2, 10, None, None, None], - [1, 17, None, None, None], - ], - None, - None, - [ - (1, 17, None, [0]), - (2, 10, None, [1]), - (2, 10, None, [2]), - (3, 3, None, [1]), - ], - ), - ( - 5, - 18, - None, - [ - [0, 4, None, None, None], - [3, 11, None, None, None], - [2, 18, None, None, None], - ], - None, - None, - [ - (0, 4, None, [2]), - (2, 18, None, [0]), - (3, 11, None, [1]), - (3, 11, None, [2]), - ], - ), - ( - 6, - 19, - None, - [ - [1, 5, None, None, None], - [0, 12, None, None, None], - [3, 19, None, None, None], - ], - None, - None, - [ - (0, 12, None, [2, 1]), - (1, 5, None, [2]), - (3, 19, None, [0]), - ], - ), - ( - 7, - 13, - None, - [[2, 6, None, None, None], [1, 13, None, None, None]], - None, - None, - [ - (1, 13, None, [2, 1]), - (2, 6, None, [2]), - ], - ), - ] - } - first_report.labels_index = report_label_idx - return first_report - - @pytest.fixture - def sample_first_report_no_encoded_labels(self): - first_report = EditableReport( - sessions={ - 0: Session( - flags=["enterprise"], - id=0, - session_type=SessionType.carriedforward, - ), - 1: Session( - flags=["enterprise"], id=1, session_type=SessionType.uploaded - ), - 2: Session( - flags=["unit"], id=2, session_type=SessionType.carriedforward - ), - 3: Session( - flags=["unrelated"], id=3, session_type=SessionType.uploaded - ), - } - ) - first_file = EditableReportFile("first_file.py") - c = 0 - for list_of_lists_of_labels in [ - [["one_label"]], - [["another_label"]], - [["another_label"], ["one_label"]], - [["another_label", "one_label"]], - [["Th2dMtk4M_codecov"]], - ]: - for sessionid in range(4): - first_file.append( - c % 7 + 1, - self.create_sample_line( - coverage=c, - sessionid=sessionid, - list_of_lists_of_labels=list_of_lists_of_labels, - ), - ) - c += 1 - second_file = EditableReportFile("second_file.py") - first_report.append(first_file) - first_report.append(second_file) - - assert self.convert_report_to_better_readable(first_report)["archive"] == { - "first_file.py": [ - ( - 1, - 14, - None, - [ - [0, 0, None, None, None], - [3, 7, None, None, None], - [2, 14, None, None, None], - ], - None, - None, - [ - (0, 0, None, ["one_label"]), - (2, 14, None, ["another_label", "one_label"]), - (3, 7, None, ["another_label"]), - ], - ), - ( - 2, - 15, - None, - [ - [1, 1, None, None, None], - [0, 8, None, None, None], - [3, 15, None, None, None], - ], - None, - None, - [ - (0, 8, None, ["another_label"]), - (0, 8, None, ["one_label"]), - (1, 1, None, ["one_label"]), - (3, 15, None, ["another_label", "one_label"]), - ], - ), - ( - 3, - 16, - None, - [ - [2, 2, None, None, None], - [1, 9, None, None, None], - [0, 16, None, None, None], - ], - None, - None, - [ - (0, 16, None, ["Th2dMtk4M_codecov"]), - (1, 9, None, ["another_label"]), - (1, 9, None, ["one_label"]), - (2, 2, None, ["one_label"]), - ], - ), - ( - 4, - 17, - None, - [ - [3, 3, None, None, None], - [2, 10, None, None, None], - [1, 17, None, None, None], - ], - None, - None, - [ - (1, 17, None, ["Th2dMtk4M_codecov"]), - (2, 10, None, ["another_label"]), - (2, 10, None, ["one_label"]), - (3, 3, None, ["one_label"]), - ], - ), - ( - 5, - 18, - None, - [ - [0, 4, None, None, None], - [3, 11, None, None, None], - [2, 18, None, None, None], - ], - None, - None, - [ - (0, 4, None, ["another_label"]), - (2, 18, None, ["Th2dMtk4M_codecov"]), - (3, 11, None, ["another_label"]), - (3, 11, None, ["one_label"]), - ], - ), - ( - 6, - 19, - None, - [ - [1, 5, None, None, None], - [0, 12, None, None, None], - [3, 19, None, None, None], - ], - None, - None, - [ - (0, 12, None, ["another_label", "one_label"]), - (1, 5, None, ["another_label"]), - (3, 19, None, ["Th2dMtk4M_codecov"]), - ], - ), - ( - 7, - 13, - None, - [[2, 6, None, None, None], [1, 13, None, None, None]], - None, - None, - [ - (1, 13, None, ["another_label", "one_label"]), - (2, 6, None, ["another_label"]), - ], - ), - ] - } - return first_report - - def create_sample_line( - self, *, coverage, sessionid=None, list_of_lists_of_labels=None - ): - datapoints = [ - CoverageDatapoint( - sessionid=sessionid, - coverage=coverage, - coverage_type=None, - label_ids=label_ids, - ) - for label_ids in (list_of_lists_of_labels or [[]]) - ] - return ReportLine.create( - coverage=coverage, - sessions=[ - ( - LineSession( - id=sessionid, - coverage=coverage, - ) - ) - ], - datapoints=datapoints, - ) - - @pytest.mark.parametrize("report_idx", [0, 1]) - def test_adjust_sessions_no_cf( - self, sample_first_report, sample_first_report_no_encoded_labels, report_idx - ): - report_under_test = [ - sample_first_report, - sample_first_report_no_encoded_labels, - ][report_idx] - first_value = self.convert_report_to_better_readable(report_under_test) - first_to_merge_session = Session(flags=["enterprise"], id=3) - second_report = Report(sessions={3: first_to_merge_session}) - current_yaml = UserYaml({}) - # No change to the report cause there's no session to CF - assert clear_carryforward_sessions( - report_under_test, second_report, ["enterprise"], current_yaml - ) == SessionAdjustmentResult([], []) - assert first_value == self.convert_report_to_better_readable(report_under_test) - - def test_adjust_sessions_full_cf_only(self, sample_first_report): - first_to_merge_session = Session(flags=["enterprise"], id=3) - second_report = Report(sessions={3: first_to_merge_session}) - current_yaml = UserYaml( - { - "flag_management": { - "individual_flags": [{"name": "enterprise", "carryforward": True}] - } - } - ) - assert clear_carryforward_sessions( - sample_first_report, second_report, ["enterprise"], current_yaml - ) == SessionAdjustmentResult([0], []) - assert self.convert_report_to_better_readable(sample_first_report) == { - "archive": { - "first_file.py": [ - ( - 1, - 14, - None, - [[3, 7, None, None, None], [2, 14, None, None, None]], - None, - None, - [ - (2, 14, None, [2, 1]), - (3, 7, None, [2]), - ], - ), - ( - 2, - 15, - None, - [[1, 1, None, None, None], [3, 15, None, None, None]], - None, - None, - [ - (1, 1, None, [1]), - (3, 15, None, [2, 1]), - ], - ), - ( - 3, - 9, - None, - [[2, 2, None, None, None], [1, 9, None, None, None]], - None, - None, - [ - (1, 9, None, [1]), - (1, 9, None, [2]), - (2, 2, None, [1]), - ], - ), - ( - 4, - 17, - None, - [ - [3, 3, None, None, None], - [2, 10, None, None, None], - [1, 17, None, None, None], - ], - None, - None, - [ - (1, 17, None, [0]), - (2, 10, None, [1]), - (2, 10, None, [2]), - (3, 3, None, [1]), - ], - ), - ( - 5, - 18, - None, - [[3, 11, None, None, None], [2, 18, None, None, None]], - None, - None, - [ - (2, 18, None, [0]), - (3, 11, None, [1]), - (3, 11, None, [2]), - ], - ), - ( - 6, - 19, - None, - [[1, 5, None, None, None], [3, 19, None, None, None]], - None, - None, - [ - (1, 5, None, [2]), - (3, 19, None, [0]), - ], - ), - ( - 7, - 13, - None, - [[2, 6, None, None, None], [1, 13, None, None, None]], - None, - None, - [ - (1, 13, None, [2, 1]), - (2, 6, None, [2]), - ], - ), - ] - }, - "report": { - "files": { - "first_file.py": [ - 0, - [0, 7, 7, 0, 0, "100", 0, 0, 0, 0, 0, 0, 0], - None, - None, - ] - }, - "sessions": { - "1": { - "t": None, - "d": None, - "a": None, - "f": ["enterprise"], - "c": None, - "n": None, - "N": None, - "j": None, - "u": None, - "p": None, - "e": None, - "st": "uploaded", - "se": {}, - }, - "2": { - "t": None, - "d": None, - "a": None, - "f": ["unit"], - "c": None, - "n": None, - "N": None, - "j": None, - "u": None, - "p": None, - "e": None, - "st": "carriedforward", - "se": {}, - }, - "3": { - "t": None, - "d": None, - "a": None, - "f": ["unrelated"], - "c": None, - "n": None, - "N": None, - "j": None, - "u": None, - "p": None, - "e": None, - "st": "uploaded", - "se": {}, - }, - }, - }, - "totals": { - "f": 1, - "n": 7, - "h": 7, - "m": 0, - "p": 0, - "c": "100", - "b": 0, - "d": 0, - "M": 0, - "s": 3, - "C": 0, - "N": 0, - "diff": None, - }, - } - - @pytest.mark.parametrize("report_idx", [0, 1]) - def test_adjust_sessions_partial_cf_only_no_changes( - self, - sample_first_report, - sample_first_report_no_encoded_labels, - mocker, - report_idx, - ): - report_under_test = [ - sample_first_report, - sample_first_report_no_encoded_labels, - ][report_idx] - - mocker.patch.object( - USE_LABEL_INDEX_IN_REPORT_PROCESSING_BY_REPO_ID, - "check_value", - return_value=True, - ) - first_to_merge_session = Session(flags=["enterprise"], id=3) - second_report = Report( - sessions={first_to_merge_session.id: first_to_merge_session} - ) - upload = MagicMock( - name="fake_upload", - **{ - "report": MagicMock( - name="fake_commit_report", - **{ - "code": None, - "commit": MagicMock( - name="fake_commit", - **{"repository": MagicMock(name="fake_repo")}, - ), - }, - ) - }, - ) - current_yaml = UserYaml( - { - "flag_management": { - "individual_flags": [ - { - "name": "enterprise", - "carryforward_mode": "labels", - "carryforward": True, - } - ] - } - } - ) - - first_value = self.convert_report_to_better_readable(sample_first_report) - # This makes changes to the not-label-encoded original report, encoding them - assert clear_carryforward_sessions( - report_under_test, - second_report, - ["enterprise"], - current_yaml, - upload=upload, - ) == SessionAdjustmentResult([], [0]) - # The after result should always be the encoded labels one - after_result = self.convert_report_to_better_readable(report_under_test) - assert after_result == first_value - - def test_make_sure_label_indexes_match(self, sample_first_report): - first_to_merge_session = Session(flags=["enterprise"], id=3) - second_report = Report( - sessions={first_to_merge_session.id: first_to_merge_session} - ) - second_report.labels_index = { - 0: SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label, - # Different from the original report - 2: "one_label", - 1: "another_label", - # New labels - 3: "new_label", - } - second_report_file = ReportFile("unrelatedfile.py") - second_report_file.append( - 90, - self.create_sample_line( - coverage=90, sessionid=3, list_of_lists_of_labels=[[2]] - ), - ) - second_report_file.append( - 89, - self.create_sample_line( - coverage=89, sessionid=3, list_of_lists_of_labels=[[1, 3]] - ), - ) - second_report.append(second_report_file) - assert self.convert_report_to_better_readable(second_report)["archive"] == { - "unrelatedfile.py": [ - ( - 89, - 89, - None, - [[3, 89, None, None, None]], - None, - None, - [(3, 89, None, [1, 3])], - ), - ( - 90, - 90, - None, - [[3, 90, None, None, None]], - None, - None, - [(3, 90, None, [2])], - ), - ] - } - assert sample_first_report.labels_index == { - 0: SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label, - 1: "one_label", - 2: "another_label", - } - # This changes the label indexes in the 2nd report AND adds new labels to the original one. - # So when we merge them we can be sure the indexes point to the same labels - # And all labels are accounted for - make_sure_label_indexes_match(sample_first_report, second_report) - assert sample_first_report.labels_index == { - 0: SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label, - 1: "one_label", - 2: "another_label", - 3: "new_label", - } - assert self.convert_report_to_better_readable(second_report)["archive"] == { - "unrelatedfile.py": [ - ( - 89, - 89, - None, - [[3, 89, None, None, None]], - None, - None, - [(3, 89, None, [2, 3])], - ), - ( - 90, - 90, - None, - [[3, 90, None, None, None]], - None, - None, - [(3, 90, None, [1])], - ), - ] - } - - def test_adjust_sessions_partial_cf_only_some_changes( - self, - sample_first_report, - mocker, - ): - first_to_merge_session = Session(flags=["enterprise"], id=3) - second_report = Report( - sessions={first_to_merge_session.id: first_to_merge_session} - ) - mocker.patch.object( - USE_LABEL_INDEX_IN_REPORT_PROCESSING_BY_REPO_ID, - "check_value", - return_value=True, - ) - upload = MagicMock( - name="fake_upload", - **{ - "report": MagicMock( - name="fake_commit_report", - **{ - "code": None, - "commit": MagicMock( - name="fake_commit", - **{"repository": MagicMock(name="fake_repo")}, - ), - }, - ) - }, - ) - current_yaml = UserYaml( - { - "flag_management": { - "individual_flags": [ - { - "name": "enterprise", - "carryforward_mode": "labels", - "carryforward": True, - } - ] - } - } - ) - second_report_file = ReportFile("unrelatedfile.py") - second_report_file.append( - 90, - self.create_sample_line( - coverage=90, sessionid=3, list_of_lists_of_labels=[[1]] - ), - ) - second_report.append(second_report_file) - assert clear_carryforward_sessions( - sample_first_report, - second_report, - ["enterprise"], - current_yaml, - upload=upload, - ) == SessionAdjustmentResult([], [0]) - - assert self.convert_report_to_better_readable(sample_first_report) == { - "archive": { - "first_file.py": [ - ( - 1, - 14, - None, - [[3, 7, None, None, None], [2, 14, None, None, None]], - None, - None, - [ - (2, 14, None, [2, 1]), - (3, 7, None, [2]), - ], - ), - ( - 2, - 15, - None, - [ - [1, 1, None, None, None], - [0, 8, None, None, None], - [3, 15, None, None, None], - ], - None, - None, - [ - (0, 8, None, [2]), - (1, 1, None, [1]), - (3, 15, None, [2, 1]), - ], - ), - ( - 3, - 16, - None, - [ - [2, 2, None, None, None], - [1, 9, None, None, None], - [0, 16, None, None, None], - ], - None, - None, - [ - (0, 16, None, [0]), - (1, 9, None, [1]), - (1, 9, None, [2]), - (2, 2, None, [1]), - ], - ), - ( - 4, - 17, - None, - [ - [3, 3, None, None, None], - [2, 10, None, None, None], - [1, 17, None, None, None], - ], - None, - None, - [ - (1, 17, None, [0]), - (2, 10, None, [1]), - (2, 10, None, [2]), - (3, 3, None, [1]), - ], - ), - ( - 5, - 18, - None, - [ - [0, 4, None, None, None], - [3, 11, None, None, None], - [2, 18, None, None, None], - ], - None, - None, - [ - (0, 4, None, [2]), - (2, 18, None, [0]), - (3, 11, None, [1]), - (3, 11, None, [2]), - ], - ), - ( - 6, - 19, - None, - [[1, 5, None, None, None], [3, 19, None, None, None]], - None, - None, - [ - (1, 5, None, [2]), - (3, 19, None, [0]), - ], - ), - ( - 7, - 13, - None, - [[2, 6, None, None, None], [1, 13, None, None, None]], - None, - None, - [ - (1, 13, None, [2, 1]), - (2, 6, None, [2]), - ], - ), - ] - }, - "report": { - "files": { - "first_file.py": [ - 0, - [0, 7, 7, 0, 0, "100", 0, 0, 0, 0, 0, 0, 0], - None, - None, - ] - }, - "sessions": { - "0": { - "t": None, - "d": None, - "a": None, - "f": ["enterprise"], - "c": None, - "n": None, - "N": None, - "j": None, - "u": None, - "p": None, - "e": None, - "st": "carriedforward", - "se": {}, - }, - "1": { - "t": None, - "d": None, - "a": None, - "f": ["enterprise"], - "c": None, - "n": None, - "N": None, - "j": None, - "u": None, - "p": None, - "e": None, - "st": "uploaded", - "se": {}, - }, - "2": { - "t": None, - "d": None, - "a": None, - "f": ["unit"], - "c": None, - "n": None, - "N": None, - "j": None, - "u": None, - "p": None, - "e": None, - "st": "carriedforward", - "se": {}, - }, - "3": { - "t": None, - "d": None, - "a": None, - "f": ["unrelated"], - "c": None, - "n": None, - "N": None, - "j": None, - "u": None, - "p": None, - "e": None, - "st": "uploaded", - "se": {}, - }, - }, - }, - "totals": { - "f": 1, - "n": 7, - "h": 7, - "m": 0, - "p": 0, - "c": "100", - "b": 0, - "d": 0, - "M": 0, - "s": 4, - "C": 0, - "N": 0, - "diff": None, - }, - } - - def test_adjust_sessions_partial_cf_only_full_deletion_due_to_lost_labels( - self, sample_first_report, mocker - ): - first_to_merge_session = Session(flags=["enterprise"], id=3) - second_report = Report(sessions={3: first_to_merge_session}) - current_yaml = UserYaml( - { - "flag_management": { - "individual_flags": [ - { - "name": "enterprise", - "carryforward_mode": "labels", - "carryforward": True, - } - ] - } - } - ) - upload = MagicMock( - name="fake_upload", - **{ - "report": MagicMock( - name="fake_commit_report", - **{ - "code": None, - "commit": MagicMock( - name="fake_commit", - **{"repository": MagicMock(name="fake_repo")}, - ), - }, - ) - }, - ) - mocker.patch.object( - USE_LABEL_INDEX_IN_REPORT_PROCESSING_BY_REPO_ID, - "check_value", - return_value=True, - ) - - second_report_file = ReportFile("unrelatedfile.py") - second_report_file.append( - 90, - self.create_sample_line( - coverage=90, sessionid=3, list_of_lists_of_labels=[[1]] - ), - ) - a_report_file = ReportFile("first_file.py") - a_report_file.append( - 90, - self.create_sample_line( - coverage=90, - sessionid=3, - list_of_lists_of_labels=[ - [2], - [0], - ], - ), - ) - second_report.append(second_report_file) - second_report.append(a_report_file) - assert clear_carryforward_sessions( - sample_first_report, - second_report, - ["enterprise"], - current_yaml, - upload=upload, - ) == SessionAdjustmentResult([0], []) - - res = self.convert_report_to_better_readable(sample_first_report) - assert res["report"]["sessions"] == { - "1": { - "t": None, - "d": None, - "a": None, - "f": ["enterprise"], - "c": None, - "n": None, - "N": None, - "j": None, - "u": None, - "p": None, - "e": None, - "st": "uploaded", - "se": {}, - }, - "2": { - "t": None, - "d": None, - "a": None, - "f": ["unit"], - "c": None, - "n": None, - "N": None, - "j": None, - "u": None, - "p": None, - "e": None, - "st": "carriedforward", - "se": {}, - }, - "3": { - "t": None, - "d": None, - "a": None, - "f": ["unrelated"], - "c": None, - "n": None, - "N": None, - "j": None, - "u": None, - "p": None, - "e": None, - "st": "uploaded", - "se": {}, - }, - } - assert res["archive"] == { - "first_file.py": [ - ( - 1, - 14, - None, - [[3, 7, None, None, None], [2, 14, None, None, None]], - None, - None, - [ - (2, 14, None, [2, 1]), - (3, 7, None, [2]), - ], - ), - ( - 2, - 15, - None, - [[1, 1, None, None, None], [3, 15, None, None, None]], - None, - None, - [ - (1, 1, None, [1]), - (3, 15, None, [2, 1]), - ], - ), - ( - 3, - 9, - None, - [[2, 2, None, None, None], [1, 9, None, None, None]], - None, - None, - [ - (1, 9, None, [1]), - (1, 9, None, [2]), - (2, 2, None, [1]), - ], - ), - ( - 4, - 17, - None, - [ - [3, 3, None, None, None], - [2, 10, None, None, None], - [1, 17, None, None, None], - ], - None, - None, - [ - (1, 17, None, [0]), - (2, 10, None, [1]), - (2, 10, None, [2]), - (3, 3, None, [1]), - ], - ), - ( - 5, - 18, - None, - [[3, 11, None, None, None], [2, 18, None, None, None]], - None, - None, - [ - (2, 18, None, [0]), - (3, 11, None, [1]), - (3, 11, None, [2]), - ], - ), - ( - 6, - 19, - None, - [[1, 5, None, None, None], [3, 19, None, None, None]], - None, - None, - [ - (1, 5, None, [2]), - (3, 19, None, [0]), - ], - ), - ( - 7, - 13, - None, - [[2, 6, None, None, None], [1, 13, None, None, None]], - None, - None, - [ - (1, 13, None, [2, 1]), - (2, 6, None, [2]), - ], - ), - ] - } diff --git a/tasks/tests/unit/test_upload_processing_task.py b/tasks/tests/unit/test_upload_processing_task.py index bb5624c1d..fe51da7af 100644 --- a/tasks/tests/unit/test_upload_processing_task.py +++ b/tasks/tests/unit/test_upload_processing_task.py @@ -19,7 +19,6 @@ RepositoryWithoutValidBotError, ) from helpers.parallel import ParallelProcessing -from rollouts import USE_LABEL_INDEX_IN_REPORT_PROCESSING_BY_REPO_ID from services.archive import ArchiveService from services.report import ProcessingError, RawReportInfo, ReportService from services.report.parser.legacy import LegacyReportParser @@ -53,12 +52,6 @@ def test_upload_processor_task_call( mock_storage, celery_app, ): - mocker.patch.object( - USE_LABEL_INDEX_IN_REPORT_PROCESSING_BY_REPO_ID, - "check_value", - return_value=False, - ) - mocked_1 = mocker.patch.object(ArchiveService, "read_chunks") mocked_1.return_value = None url = "v4/raw/2019-05-22/C3C4715CA57C910D11D5EB899FC86A7E/4c4e4654ac25037ae869caeb3619d485970b6304/a84d445c-9c1e-434f-8275-f18f1f320f81.txt" @@ -160,12 +153,6 @@ def test_upload_processor_task_call_should_delete( mock_storage, celery_app, ): - mocker.patch.object( - USE_LABEL_INDEX_IN_REPORT_PROCESSING_BY_REPO_ID, - "check_value", - return_value=False, - ) - mock_configuration.set_params( {"services": {"minio": {"expire_raw_after_n_days": True}}} ) @@ -270,12 +257,6 @@ def test_upload_processor_task_call_should_delete( def test_upload_processor_call_with_upload_obj( self, mocker, dbsession, mock_storage ): - mocker.patch.object( - USE_LABEL_INDEX_IN_REPORT_PROCESSING_BY_REPO_ID, - "check_value", - return_value=False, - ) - mocked_1 = mocker.patch.object(ArchiveService, "read_chunks") mocked_1.return_value = None commit = CommitFactory.create( @@ -390,12 +371,6 @@ def test_upload_task_call_existing_chunks( mock_storage, celery_app, ): - mocker.patch.object( - USE_LABEL_INDEX_IN_REPORT_PROCESSING_BY_REPO_ID, - "check_value", - return_value=False, - ) - mocked_1 = mocker.patch.object(ArchiveService, "read_chunks") with open(here.parent.parent / "samples" / "sample_chunks_1.txt") as f: content = f.read() From c7b887304025fcfe71c27a129b2ca1e056400797 Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Mon, 14 Oct 2024 13:26:23 +0200 Subject: [PATCH 2/5] Remove most of the "labels" concept --- services/report/languages/pycoverage.py | 72 +-- services/report/report_builder.py | 124 +---- .../report/tests/unit/test_report_builder.py | 165 +----- services/report/tests/unit/test_sessions.py | 171 +------ services/tests/test_report.py | 481 ------------------ tasks/tests/unit/test_clean_labels_index.py | 379 -------------- tasks/upload_clean_labels_index.py | 217 -------- tasks/upload_finisher.py | 39 -- 8 files changed, 31 insertions(+), 1617 deletions(-) delete mode 100644 tasks/tests/unit/test_clean_labels_index.py delete mode 100644 tasks/upload_clean_labels_index.py diff --git a/services/report/languages/pycoverage.py b/services/report/languages/pycoverage.py index af28bc141..d85f4ce4c 100644 --- a/services/report/languages/pycoverage.py +++ b/services/report/languages/pycoverage.py @@ -1,7 +1,7 @@ import sentry_sdk from services.report.languages.base import BaseLanguageProcessor -from services.report.report_builder import ReportBuilderSession, SpecialLabelsEnum +from services.report.report_builder import ReportBuilderSession COVERAGE_HIT = 1 COVERAGE_MISS = 0 @@ -16,8 +16,6 @@ def matches_content(self, content: dict, first_line: str, name: str) -> bool: def process( self, content: dict, report_builder_session: ReportBuilderSession ) -> None: - labels_table = LabelsTable(report_builder_session, content) - for filename, file_coverage in content["files"].items(): _file = report_builder_session.create_coverage_file(filename) if _file is None: @@ -28,70 +26,6 @@ def process( ] + [(COVERAGE_MISS, ln) for ln in file_coverage["missing_lines"]] for cov, ln in lines_and_coverage: if ln > 0: - # label_list_of_lists: list[list[str]] | list[list[int]] = [] - label_list_of_lists = [ - [labels_table._normalize_label(testname)] - for testname in file_coverage.get("contexts", {}).get( - str(ln), [] - ) - ] - _file.append( - ln, - report_builder_session.create_coverage_line( - cov, - labels_list_of_lists=label_list_of_lists, - ), - ) + _line = report_builder_session.create_coverage_line(cov) + _file.append(ln, _line) report_builder_session.append(_file) - - -class LabelsTable: - def __init__( - self, report_builder_session: ReportBuilderSession, content: dict - ) -> None: - self.labels_table: dict[str, str] = {} - self.reverse_table: dict[str, int] = {} - self.are_labels_already_encoded = False - - # Compressed pycoverage files will include a labels_table - if "labels_table" in content: - self.labels_table = content["labels_table"] - # We can pre-populate some of the indexes that will be used - for idx, testname in self.labels_table.items(): - clean_label = self._normalize_label(testname) - report_builder_session.label_index[int(idx)] = clean_label - self.are_labels_already_encoded = True - - def _normalize_label(self, testname: int | float | str) -> str: - if isinstance(testname, int) or isinstance(testname, float): - # This is from a compressed report. - # Pull label from the labels_table - # But the labels_table keys are strings, because of JSON format - testname = self.labels_table[str(testname)] - if testname == "": - return SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label - return testname.split("|", 1)[0] - - def _get_list_of_label_ids( - self, - current_label_idx: dict[int, str], - line_contexts: list[str | int], - ) -> list[int]: - if self.are_labels_already_encoded: - # The line contexts already include indexes in the table. - # We can re-use the table and don't have to do anything with contexts. - return sorted(map(int, line_contexts)) - - # In this case we do need to fix the labels - label_ids_for_line = set() - for testname in line_contexts: - clean_label = self._normalize_label(testname) - if clean_label in self.reverse_table: - label_ids_for_line.add(self.reverse_table[clean_label]) - else: - label_id = max([*current_label_idx.keys(), 0]) + 1 - current_label_idx[label_id] = clean_label - self.reverse_table[clean_label] = label_id - label_ids_for_line.add(label_id) - - return sorted(label_ids_for_line) diff --git a/services/report/report_builder.py b/services/report/report_builder.py index 4358fb463..8eb6ef87f 100644 --- a/services/report/report_builder.py +++ b/services/report/report_builder.py @@ -1,13 +1,10 @@ -import dataclasses import logging from enum import Enum -from typing import Any, List, Sequence +from typing import Any, Sequence from shared.reports.resources import LineSession, Report, ReportFile, ReportLine -from shared.reports.types import CoverageDatapoint from shared.yaml.user_yaml import UserYaml -from helpers.labels import SpecialLabelsEnum from services.path_fixer import PathFixer from services.yaml.reader import read_yaml_field @@ -36,8 +33,6 @@ def __init__( self.filepath = report_filepath self._report_builder = report_builder self._report = Report() - self.label_index = {} - self._present_labels = set() @property def path_fixer(self): @@ -53,109 +48,11 @@ def get_file(self, filename: str) -> ReportFile | None: return self._report.get(filename) def append(self, file: ReportFile): - if file is not None: - for line_number, line in file.lines: - if line.datapoints: - for datapoint in line.datapoints: - if datapoint.label_ids: - for label in datapoint.label_ids: - self._present_labels.add(label) return self._report.append(file) def output_report(self) -> Report: - """ - Outputs a Report. - This function applies all the needed modifications before a report - can be output - - Returns: - Report: The legacy report desired - """ - if self._present_labels: - if self._present_labels and self._present_labels == { - SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER - }: - log.warning( - "Report only has SpecialLabels. Might indicate it was not generated with contexts" - ) - for file in self._report: - for line_number, line in file.lines: - self._possibly_modify_line_to_account_for_special_labels( - file, line_number, line - ) - self._report._totals = None return self._report - def _possibly_modify_line_to_account_for_special_labels( - self, file: ReportFile, line_number: int, line: ReportLine - ) -> None: - """Possibly modify the report line in the file - to account for any label in the SpecialLabelsEnum - - Args: - file (ReportFile): The file we want to modify - line_number (int): The line number in case we - need to set the new line back into the files - line (ReportLine): The original line - """ - if not line.datapoints: - return - - new_datapoints = [ - item - for datapoint in line.datapoints - for item in self._possibly_convert_datapoints(datapoint) - ] - if new_datapoints and new_datapoints != line.datapoints: - # A check to avoid unnecessary replacement - file[line_number] = dataclasses.replace( - line, - datapoints=sorted( - new_datapoints, - key=lambda x: ( - x.sessionid, - x.coverage, - x.coverage_type, - ), - ), - ) - file._totals = None - - # TODO: This can be removed after label indexing is rolled out for all customers - def _possibly_convert_datapoints( - self, datapoint: CoverageDatapoint - ) -> List[CoverageDatapoint]: - """Possibly convert datapoints - The datapoint that might need to be converted - - Args: - datapoint (CoverageDatapoint): The datapoint to convert - """ - if datapoint.label_ids and any( - label == SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER - for label in datapoint.label_ids - ): - new_label = ( - SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label - ) - return [ - dataclasses.replace( - datapoint, - label_ids=sorted( - set( - [ - label - for label in datapoint.label_ids - if label - != SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER - ] - + [new_label] - ) - ), - ) - ] - return [datapoint] - def create_coverage_file( self, path: str, do_fix_path: bool = True ) -> ReportFile | None: @@ -171,30 +68,12 @@ def create_coverage_line( self, coverage: int | str, coverage_type: CoverageType | None = None, - labels_list_of_lists: list[list[str | SpecialLabelsEnum]] - | list[list[int]] - | None = None, partials=None, missing_branches=None, complexity=None, ) -> ReportLine: sessionid = self._report_builder.sessionid coverage_type_str = coverage_type.map_to_string() if coverage_type else None - datapoints = ( - [ - CoverageDatapoint( - sessionid=sessionid, - coverage=coverage, - coverage_type=coverage_type_str, - label_ids=label_ids, - ) - # Avoid creating datapoints that don't contain any labels - for label_ids in (labels_list_of_lists or []) - if label_ids - ] - if self._report_builder.supports_labels() - else None - ) return ReportLine.create( coverage=coverage, type=coverage_type_str, @@ -209,7 +88,6 @@ def create_coverage_line( ) ) ], - datapoints=datapoints, complexity=complexity, ) diff --git a/services/report/tests/unit/test_report_builder.py b/services/report/tests/unit/test_report_builder.py index e282c8f42..2aa890f8b 100644 --- a/services/report/tests/unit/test_report_builder.py +++ b/services/report/tests/unit/test_report_builder.py @@ -1,12 +1,7 @@ import pytest from shared.reports.resources import LineSession, ReportFile, ReportLine -from shared.reports.types import CoverageDatapoint -from services.report.report_builder import ( - CoverageType, - ReportBuilder, - SpecialLabelsEnum, -) +from services.report.report_builder import CoverageType, ReportBuilder def test_report_builder_generate_session(mocker): @@ -36,23 +31,12 @@ def test_report_builder_session(mocker): first_file.append(2, ReportLine.create(coverage=0)) first_file.append( 3, - ReportLine.create( - coverage=0, - datapoints=[ - CoverageDatapoint( - sessionid=0, - coverage=1, - coverage_type=None, - label_ids=[SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER], - ) - ], - ), + ReportLine.create(coverage=0), ) first_file.append( 10, ReportLine.create( coverage=1, - type=None, sessions=[ ( LineSession( @@ -61,21 +45,6 @@ def test_report_builder_session(mocker): ) ) ], - datapoints=[ - CoverageDatapoint( - sessionid=0, - coverage=1, - coverage_type=None, - label_ids=["some_label", "other"], - ), - CoverageDatapoint( - sessionid=0, - coverage=1, - coverage_type=None, - label_ids=None, - ), - ], - complexity=None, ), ) builder_session.append(first_file) @@ -84,53 +53,15 @@ def test_report_builder_session(mocker): assert sorted(final_report.get("filename.py").lines) == [ ( 2, - ReportLine.create( - coverage=0, type=None, sessions=None, datapoints=None, complexity=None - ), + ReportLine.create(coverage=0), ), ( 3, - ReportLine.create( - coverage=0, - type=None, - sessions=None, - datapoints=[ - CoverageDatapoint( - sessionid=0, - coverage=1, - coverage_type=None, - label_ids=["Th2dMtk4M_codecov"], - ), - ], - complexity=None, - ), + ReportLine.create(coverage=0), ), ( 10, - ReportLine.create( - coverage=1, - type=None, - sessions=[ - LineSession( - id=0, coverage=1, branches=None, partials=None, complexity=None - ) - ], - datapoints=[ - CoverageDatapoint( - sessionid=0, - coverage=1, - coverage_type=None, - label_ids=["some_label", "other"], - ), - CoverageDatapoint( - sessionid=0, - coverage=1, - coverage_type=None, - label_ids=None, - ), - ], - complexity=None, - ), + ReportLine.create(coverage=1, sessions=[LineSession(id=0, coverage=1)]), ), ] @@ -149,17 +80,7 @@ def test_report_builder_session_only_all_labels(mocker): first_file.append(2, ReportLine.create(coverage=0)) first_file.append( 3, - ReportLine.create( - coverage=0, - datapoints=[ - CoverageDatapoint( - sessionid=0, - coverage=1, - coverage_type=None, - label_ids=[SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER], - ) - ], - ), + ReportLine.create(coverage=0), ) first_file.append( 10, @@ -174,21 +95,6 @@ def test_report_builder_session_only_all_labels(mocker): ) ) ], - datapoints=[ - CoverageDatapoint( - sessionid=0, - coverage=1, - coverage_type=None, - label_ids=[SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER], - ), - CoverageDatapoint( - sessionid=0, - coverage=1, - coverage_type=None, - label_ids=None, - ), - ], - complexity=None, ), ) builder_session.append(first_file) @@ -197,52 +103,17 @@ def test_report_builder_session_only_all_labels(mocker): assert sorted(final_report.get("filename.py").lines) == [ ( 2, - ReportLine.create( - coverage=0, type=None, sessions=None, datapoints=None, complexity=None - ), + ReportLine.create(coverage=0), ), ( 3, - ReportLine.create( - coverage=0, - type=None, - sessions=None, - datapoints=[ - CoverageDatapoint( - sessionid=0, - coverage=1, - coverage_type=None, - label_ids=["Th2dMtk4M_codecov"], - ), - ], - complexity=None, - ), + ReportLine.create(coverage=0), ), ( 10, ReportLine.create( coverage=1, - type=None, - sessions=[ - LineSession( - id=0, coverage=1, branches=None, partials=None, complexity=None - ) - ], - datapoints=[ - CoverageDatapoint( - sessionid=0, - coverage=1, - coverage_type=None, - label_ids=["Th2dMtk4M_codecov"], - ), - CoverageDatapoint( - sessionid=0, - coverage=1, - coverage_type=None, - label_ids=None, - ), - ], - complexity=None, + sessions=[LineSession(id=0, coverage=1)], ), ), ] @@ -274,8 +145,6 @@ def test_report_builder_session_create_line(mocker): id=45, coverage=1, branches=None, partials=None, complexity=None ) ], - datapoints=[], - complexity=None, ) @@ -299,7 +168,6 @@ def test_report_builder_session_create_line_mixed_labels(mocker): line = builder_session.create_coverage_line( 1, CoverageType.branch, - labels_list_of_lists=[["label1"], [], ["label2"], None], ) assert line == ReportLine.create( coverage=1, @@ -309,21 +177,6 @@ def test_report_builder_session_create_line_mixed_labels(mocker): id=45, coverage=1, branches=None, partials=None, complexity=None ) ], - datapoints=[ - CoverageDatapoint( - sessionid=45, - coverage=1, - coverage_type="b", - label_ids=["label1"], - ), - CoverageDatapoint( - sessionid=45, - coverage=1, - coverage_type="b", - label_ids=["label2"], - ), - ], - complexity=None, ) diff --git a/services/report/tests/unit/test_sessions.py b/services/report/tests/unit/test_sessions.py index a8ed9beae..f1fe7d318 100644 --- a/services/report/tests/unit/test_sessions.py +++ b/services/report/tests/unit/test_sessions.py @@ -9,10 +9,8 @@ Session, SessionType, ) -from shared.reports.types import CoverageDatapoint from shared.yaml import UserYaml -from helpers.labels import SpecialLabelsEnum from services.report.raw_upload_processor import ( SessionAdjustmentResult, clear_carryforward_sessions, @@ -46,27 +44,19 @@ def sample_first_report(self): ) first_file = EditableReportFile("first_file.py") c = 0 - for list_of_lists_of_labels in [ - [["one_label"]], - [["another_label"]], - [["another_label"], ["one_label"]], - [["another_label", "one_label"]], - [[SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label]], - ]: - for sessionid in range(4): - first_file.append( - c % 7 + 1, - self.create_sample_line( - coverage=c, - sessionid=sessionid, - list_of_lists_of_labels=list_of_lists_of_labels, - ), - ) - c += 1 + for sessionid in range(4): + first_file.append( + c % 7 + 1, + self.create_sample_line( + coverage=c, + sessionid=sessionid, + ), + ) + c += 1 second_file = EditableReportFile("second_file.py") first_report.append(first_file) first_report.append(second_file) - # print(self.convert_report_to_better_readable(first_report)["archive"]) + assert self.convert_report_to_better_readable(first_report)["archive"] == { "first_file.py": [ ( @@ -191,29 +181,10 @@ def sample_first_report(self): } return first_report - def create_sample_line( - self, *, coverage, sessionid=None, list_of_lists_of_labels=None - ): - datapoints = [ - CoverageDatapoint( - sessionid=sessionid, - coverage=coverage, - coverage_type=None, - label_ids=labels, - ) - for labels in (list_of_lists_of_labels or [[]]) - ] + def create_sample_line(self, *, coverage, sessionid=None): return ReportLine.create( coverage=coverage, - sessions=[ - ( - LineSession( - id=sessionid, - coverage=coverage, - ) - ) - ], - datapoints=datapoints, + sessions=[(LineSession(id=sessionid, coverage=coverage))], ) def test_adjust_sessions_no_cf(self, sample_first_report): @@ -241,7 +212,7 @@ def test_adjust_sessions_full_cf_only(self, sample_first_report): assert clear_carryforward_sessions( sample_first_report, second_report, ["enterprise"], current_yaml ) == SessionAdjustmentResult([0], []) - print(self.convert_report_to_better_readable(sample_first_report)) + assert self.convert_report_to_better_readable(sample_first_report) == { "archive": { "first_file.py": [ @@ -515,7 +486,7 @@ def test_adjust_sessions_partial_cf_only_some_changes(self, sample_first_report) assert clear_carryforward_sessions( sample_first_report, second_report, ["enterprise"], current_yaml ) == SessionAdjustmentResult([], [0]) - print(self.convert_report_to_better_readable(sample_first_report)) + assert self.convert_report_to_better_readable(sample_first_report) == { "archive": { "first_file.py": [ @@ -739,23 +710,12 @@ def test_adjust_sessions_partial_cf_only_full_deletion_due_to_lost_labels( second_report_file = ReportFile("unrelatedfile.py") second_report_file.append( 90, - self.create_sample_line( - coverage=90, sessionid=3, list_of_lists_of_labels=[["one_label"]] - ), + self.create_sample_line(coverage=90, sessionid=3), ) a_report_file = ReportFile("first_file.py") a_report_file.append( 90, - self.create_sample_line( - coverage=90, - sessionid=3, - list_of_lists_of_labels=[ - ["another_label"], - [ - SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label - ], - ], - ), + self.create_sample_line(coverage=90, sessionid=3), ) second_report.append(second_report_file) second_report.append(a_report_file) @@ -763,7 +723,7 @@ def test_adjust_sessions_partial_cf_only_full_deletion_due_to_lost_labels( sample_first_report, second_report, ["enterprise"], current_yaml ) == SessionAdjustmentResult([0], []) res = self.convert_report_to_better_readable(sample_first_report) - # print(res["report"]["sessions"]) + assert res["report"]["sessions"] == { "1": { "t": None, @@ -811,7 +771,7 @@ def test_adjust_sessions_partial_cf_only_full_deletion_due_to_lost_labels( "se": {}, }, } - print(self.convert_report_to_better_readable(sample_first_report)["archive"]) + assert self.convert_report_to_better_readable(sample_first_report)[ "archive" ] == { @@ -910,98 +870,3 @@ def test_adjust_sessions_partial_cf_only_full_deletion_due_to_lost_labels( ), ] } - - -{ - "first_file.py": [ - ( - 1, - 14, - None, - [[3, 7, None, None, None], [2, 14, None, None, None]], - None, - None, - [ - (2, 14, None, ["another_label", "one_label"]), - (3, 7, None, ["another_label"]), - ], - ), - ( - 2, - 15, - None, - [[1, 1, None, None, None], [3, 15, None, None, None]], - None, - None, - [ - (1, 1, None, ["one_label"]), - (3, 15, None, ["another_label", "one_label"]), - ], - ), - ( - 3, - 9, - None, - [[2, 2, None, None, None], [1, 9, None, None, None]], - None, - None, - [ - (1, 9, None, ["another_label"]), - (1, 9, None, ["one_label"]), - (2, 2, None, ["one_label"]), - ], - ), - ( - 4, - 17, - None, - [ - [3, 3, None, None, None], - [2, 10, None, None, None], - [1, 17, None, None, None], - ], - None, - None, - [ - (1, 17, None, ["Th2dMtk4M_codecov"]), - (2, 10, None, ["another_label"]), - (2, 10, None, ["one_label"]), - (3, 3, None, ["one_label"]), - ], - ), - ( - 5, - 18, - None, - [[3, 11, None, None, None], [2, 18, None, None, None]], - None, - None, - [ - (2, 18, None, ["Th2dMtk4M_codecov"]), - (3, 11, None, ["another_label"]), - (3, 11, None, ["one_label"]), - ], - ), - ( - 6, - 19, - None, - [[1, 5, None, None, None], [3, 19, None, None, None]], - None, - None, - [(1, 5, None, ["another_label"]), (3, 19, None, ["Th2dMtk4M_codecov"])], - ), - ( - 7, - 13, - None, - [[2, 6, None, None, None], [1, 13, None, None, None]], - None, - None, - [ - (1, 13, None, ["another_label", "one_label"]), - (2, 6, None, ["another_label"]), - ], - ), - ] -} diff --git a/services/tests/test_report.py b/services/tests/test_report.py index 79e4ef607..94dce7654 100644 --- a/services/tests/test_report.py +++ b/services/tests/test_report.py @@ -218,51 +218,6 @@ def sample_commit_with_report_big(dbsession, mock_storage): return commit -@pytest.fixture -def sample_commit_with_report_big_with_labels(dbsession, mock_storage): - sessions_dict = { - "0": { - "N": None, - "a": None, - "c": None, - "d": None, - "e": None, - "f": ["enterprise"], - "j": None, - "n": None, - "p": None, - "st": "uploaded", - "t": None, - "u": None, - }, - } - file_headers = { - "file_00.py": [ - 0, - [0, 4, 0, 4, 0, "0", 0, 0, 0, 0, 0, 0, 0], - [[0, 4, 0, 4, 0, "0", 0, 0, 0, 0, 0, 0, 0]], - None, - ], - "file_01.py": [ - 1, - [0, 32, 32, 0, 0, "100", 0, 0, 0, 0, 0, 0, 0], - [[0, 32, 32, 0, 0, "100", 0, 0, 0, 0, 0, 0, 0]], - None, - ], - } - commit = CommitFactory.create( - _report_json={"sessions": sessions_dict, "files": file_headers} - ) - dbsession.add(commit) - dbsession.flush() - with open("tasks/tests/samples/sample_chunks_with_header.txt") as f: - content = f.read().encode() - archive_hash = ArchiveService.get_archive_hash(commit.repository) - chunks_url = f"v4/repos/{archive_hash}/commits/{commit.commitid}/chunks.txt" - mock_storage.write_file("archive", chunks_url, content) - return commit - - @pytest.fixture def sample_commit_with_report_big_already_carriedforward(dbsession, mock_storage): sessions_dict = { @@ -1436,442 +1391,6 @@ def test_create_new_report_for_commit( assert expected_results["report"] == readable_report["report"] assert expected_results == readable_report - @pytest.mark.django_db(databases={"default", "timeseries"}) - def test_create_new_report_for_commit_with_labels( - self, dbsession, sample_commit_with_report_big_with_labels - ): - parent_commit = sample_commit_with_report_big_with_labels - commit = CommitFactory.create( - repository=parent_commit.repository, - parent_commit_id=parent_commit.commitid, - _report_json=None, - ) - dbsession.add(commit) - dbsession.flush() - dbsession.add(CommitReport(commit_id=commit.id_)) - dbsession.flush() - yaml_dict = {"flags": {"enterprise": {"carryforward": True}}} - report = ReportService(UserYaml(yaml_dict)).create_new_report_for_commit(commit) - assert report is not None - assert report.labels_index == { - 0: "Th2dMtk4M_codecov", - 1: "core/tests/test_menu_interface.py::TestMenuInterface::test_init", - 2: "core/tests/test_main.py::TestMainMenu::test_init_values", - 3: "core/tests/test_main.py::TestMainMenu::test_invalid_menu_choice", - 4: "core/tests/test_menu_interface.py::TestMenuInterface::test_menu_options", - 5: "core/tests/test_menu_interface.py::TestMenuInterface::test_set_loop", - 6: "core/tests/test_main.py::TestMainMenu::test_menu_choice_emotions", - 7: "core/tests/test_menu_interface.py::TestMenuInterface::test_name", - 8: "core/tests/test_menu_interface.py::TestMenuInterface::test_parent", - 9: "core/tests/test_main.py::TestMainMenu::test_menu_choice_fruits", - 10: "core/tests/test_main.py::TestMainMenu::test_menu_options", - } - assert sorted(report.files) == sorted( - [ - "file_00.py", - "file_01.py", - ] - ) - assert report.totals == ReportTotals( - files=2, - lines=36, - hits=32, - misses=4, - partials=0, - coverage="88.88889", - branches=0, - methods=0, - messages=0, - sessions=1, - complexity=0, - complexity_total=0, - diff=0, - ) - readable_report = self.convert_report_to_better_readable(report) - expected_results = { - "archive": { - "file_00.py": [ - ( - 1, - 0, - None, - [[0, 0, None, None, None]], - None, - None, - [(0, 0, None, [])], - ), - ( - 3, - 0, - None, - [[0, 0, None, None, None]], - None, - None, - [(0, 0, None, [])], - ), - ( - 4, - 0, - None, - [[0, 0, None, None, None]], - None, - None, - [(0, 0, None, [])], - ), - ( - 5, - 0, - None, - [[0, 0, None, None, None]], - None, - None, - [(0, 0, None, [])], - ), - ], - "file_01.py": [ - ( - 1, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [0])], - ), - ( - 2, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [0])], - ), - ( - 5, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [0])], - ), - ( - 6, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [0])], - ), - ( - 7, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [0])], - ), - ( - 8, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [0])], - ), - ( - 9, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [0])], - ), - ( - 12, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [0])], - ), - ( - 13, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [0])], - ), - ( - 14, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [0])], - ), - ( - 16, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [0])], - ), - ( - 17, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [1, 2, 3, 4, 5, 6, 7, 8, 9, 10])], - ), - ( - 18, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [1, 2, 3, 4, 5, 6, 7, 8, 9, 10])], - ), - ( - 19, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [1, 2, 3, 4, 5, 6, 7, 8, 9, 10])], - ), - ( - 21, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [0])], - ), - ( - 22, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [0])], - ), - ( - 23, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [1, 3, 5, 6, 9])], - ), - ( - 25, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [0])], - ), - ( - 26, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [0])], - ), - ( - 27, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [1, 2, 8])], - ), - ( - 29, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [0])], - ), - ( - 30, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [0])], - ), - ( - 31, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [1, 2, 7])], - ), - ( - 33, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [0])], - ), - ( - 34, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [3, 5, 6, 9])], - ), - ( - 36, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [0])], - ), - ( - 37, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [0])], - ), - ( - 38, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [3, 4, 6, 9, 10])], - ), - ( - 39, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [4])], - ), - ( - 41, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [3, 4, 6, 9, 10])], - ), - ( - 43, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [0])], - ), - ( - 44, - 0, - None, - [[0, 0, None, None, None]], - None, - None, - [(0, 0, None, [])], - ), - ], - }, - "report": { - "files": { - "file_00.py": [ - 0, - [0, 4, 0, 4, 0, "0", 0, 0, 0, 0, 0, 0, 0], - None, - None, - ], - "file_01.py": [ - 1, - [0, 32, 32, 0, 0, "100", 0, 0, 0, 0, 0, 0, 0], - None, - None, - ], - }, - "sessions": { - "0": { - "N": "Carriedforward", - "a": None, - "c": None, - "d": None, - "e": None, - "f": ["enterprise"], - "j": None, - "n": None, - "p": None, - "se": {"carriedforward_from": parent_commit.commitid}, - "st": "carriedforward", - "t": None, - "u": None, - } - }, - }, - "totals": { - "C": 0, - "M": 0, - "N": 0, - "b": 0, - "c": "88.88889", - "d": 0, - "diff": None, - "f": 2, - "h": 32, - "m": 4, - "n": 36, - "p": 0, - "s": 1, - }, - } - assert expected_results["report"]["files"] == readable_report["report"]["files"] - assert expected_results["report"] == readable_report["report"] - assert expected_results == readable_report - @pytest.mark.django_db(databases={"default", "timeseries"}) def test_build_report_from_commit_carriedforward_add_sessions( self, dbsession, sample_commit_with_report_big, mocker diff --git a/tasks/tests/unit/test_clean_labels_index.py b/tasks/tests/unit/test_clean_labels_index.py deleted file mode 100644 index 8c029484b..000000000 --- a/tasks/tests/unit/test_clean_labels_index.py +++ /dev/null @@ -1,379 +0,0 @@ -import pytest -from celery.exceptions import Retry -from mock import MagicMock -from redis.exceptions import LockError -from shared.reports.resources import Report, ReportFile -from shared.reports.types import ReportLine - -from database.tests.factories.core import CommitFactory -from helpers.labels import SpecialLabelsEnum -from tasks.upload_clean_labels_index import ( - CleanLabelsIndexTask, - OwnerContext, - ReadOnlyArgs, - ReportService, - UserYaml, -) -from tasks.upload_processor import UPLOAD_PROCESSING_LOCK_NAME -from test_utils.base import BaseTestCase - - -@pytest.fixture -def sample_report_with_labels(): - report = Report() - # All labels are being used - labels_index = { - SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_index: SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label, - 1: "some_test", - 2: "another_test", - } - report.header = {"labels_index": labels_index} - report_file = ReportFile("file.py") - report_file._lines = [ - ReportLine.create(1, None, [[0, 1]], None, None, [[0, 1, None, [0]]]), - None, - ReportLine.create(1, None, [[0, 1]], None, None, [[0, 1, None, [1, 2]]]), - ReportLine.create( - "1/2", None, [[0, "1/2"]], None, None, [[0, "1/2", None, [1]]] - ), - ReportLine.create( - "1/2", None, [[0, "1/2"]], None, None, [[0, "1/2", None, [2]]] - ), - ] - report.append(report_file) - return report - - -@pytest.fixture -def sample_report_with_labels_and_renames(): - report = Report() - labels_index = { - SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_index: SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label, - 1: "some_test", # This label isn't being used - 2: "another_test", - 3: "some_test_renamed", - } - report.header = {"labels_index": labels_index} - report_file = ReportFile("file.py") - report_file._lines = [ - ReportLine.create(1, None, [[0, 1]], None, None, [[0, 1, None, [0]]]), - None, - ReportLine.create(1, None, [[0, 1]], None, None, [[0, 1, None, [2, 3]]]), - ReportLine.create( - "1/2", None, [[0, "1/2"]], None, None, [[0, "1/2", None, [3]]] - ), - ReportLine.create( - "1/2", None, [[0, "1/2"]], None, None, [[0, "1/2", None, [2]]] - ), - ] - report.append(report_file) - return report - - -# Simplified version of the FakeRedis class defined in tasks/tests/unit/test_upload_task.py -class FakeRedis(object): - """ - This is a fake, very rudimentary redis implementation to ease the managing - of mocking `exists`, `lpop` and whatnot in the context of Upload jobs - """ - - def __init__(self, mocker): - self.keys = {} - self.lock = mocker.MagicMock() - self.sismember = mocker.MagicMock() - self.hdel = mocker.MagicMock() - - def get(self, key): - res = None - if self.keys.get(key) is not None: - res = self.keys.get(key) - if res is None: - return None - if not isinstance(res, (str, bytes)): - return str(res).encode() - if not isinstance(res, bytes): - return res.encode() - return res - - -@pytest.fixture -def mock_redis(mocker): - m = mocker.patch("services.redis._get_redis_instance_from_url") - redis_server = FakeRedis(mocker) - m.return_value = redis_server - yield redis_server - - -class TestCleanLabelsIndexSyncronization(object): - @pytest.mark.parametrize( - "commitid, is_currently_processing", [("sha_1", True), ("sha_2", False)] - ) - def test__is_currently_processing( - self, mock_redis, commitid, is_currently_processing - ): - mock_redis.keys = {"upload_processing_lock_1_sha_1": True} - lock_name = UPLOAD_PROCESSING_LOCK_NAME(1, commitid) - task = CleanLabelsIndexTask() - assert ( - task._is_currently_processing(mock_redis, lock_name) - == is_currently_processing - ) - - def test_retry_currently_processing(self, dbsession, mocker, mock_redis): - commit = CommitFactory() - dbsession.add(commit) - dbsession.flush() - mock_currently_processing = mocker.patch.object( - CleanLabelsIndexTask, "_is_currently_processing", return_value=True - ) - task = CleanLabelsIndexTask() - with pytest.raises(Retry): - task.run_impl(dbsession, commit.repository.repoid, commit.commitid) - lock_name = UPLOAD_PROCESSING_LOCK_NAME( - commit.repository.repoid, commit.commitid - ) - mock_currently_processing.assert_called_with(mock_redis, lock_name) - - def test_retry_failed_to_get_lock(self, dbsession, mocker, mock_redis): - commit = CommitFactory() - dbsession.add(commit) - dbsession.flush() - mock_currently_processing = mocker.patch.object( - CleanLabelsIndexTask, "_is_currently_processing", return_value=False - ) - mock_run_impl_within_lock = mocker.patch.object( - CleanLabelsIndexTask, "run_impl_within_lock" - ) - # Mock the getters of read_only_args - mocker.patch.object( - CleanLabelsIndexTask, "_get_best_effort_commit_yaml", return_value={} - ) - mock_redis.lock.side_effect = LockError - task = CleanLabelsIndexTask() - with pytest.raises(Retry): - task.run_impl(dbsession, commit.repository.repoid, commit.commitid) - mock_currently_processing.assert_called() - mock_run_impl_within_lock.assert_not_called() - - def test_call_actual_logic(self, dbsession, mocker, mock_redis): - commit = CommitFactory() - dbsession.add(commit) - dbsession.flush() - mock_currently_processing = mocker.patch.object( - CleanLabelsIndexTask, "_is_currently_processing", return_value=False - ) - mock_run_impl_within_lock = mocker.patch.object( - CleanLabelsIndexTask, "run_impl_within_lock", return_value="return_value" - ) - mocker.patch.object( - CleanLabelsIndexTask, "_get_best_effort_commit_yaml", return_value={} - ) - task = CleanLabelsIndexTask() - task.run_impl( - dbsession, commit.repository.repoid, commit.commitid, report_code="code" - ) - mock_currently_processing.assert_called() - mock_run_impl_within_lock.assert_called_with( - dbsession, - ReadOnlyArgs(commit=commit, commit_yaml={}, report_code="code"), - ) - - -class TestCleanLabelsIndexReadOnlyArgs(object): - def test__get_commit_or_fail_success(self, dbsession): - commit = CommitFactory() - dbsession.add(commit) - dbsession.flush() - task = CleanLabelsIndexTask() - assert ( - task._get_commit_or_fail( - dbsession, commit.repository.repoid, commit.commitid - ) - == commit - ) - - def test__get_commit_or_fail_fail(self, dbsession): - task = CleanLabelsIndexTask() - with pytest.raises(AssertionError) as exp: - task._get_commit_or_fail(dbsession, 10000, "commit_that_dont_exist") - assert str(exp.value) == "Commit not found in database." - - def test__get_best_effort_commit_yaml_from_provider(self, dbsession, mocker): - commit = CommitFactory() - dbsession.add(commit) - mock_repo_service = MagicMock(name="repo_provider_service") - mock_fetch_commit = mocker.patch( - "tasks.upload_clean_labels_index.fetch_commit_yaml_from_provider", - return_value={ - "yaml_for": f"commit_{commit.commitid}", - "origin": "git_provider", - }, - ) - task = CleanLabelsIndexTask() - res = task._get_best_effort_commit_yaml(commit, mock_repo_service) - assert res == { - "yaml_for": f"commit_{commit.commitid}", - "origin": "git_provider", - } - mock_fetch_commit.assert_called_with(commit, mock_repo_service) - - def test__get_best_effort_commit_yaml_from_db(self, dbsession, mocker): - commit = CommitFactory() - dbsession.add(commit) - mock_fetch_commit = mocker.patch( - "tasks.upload_clean_labels_index.fetch_commit_yaml_from_provider" - ) - mock_final_yaml = mocker.patch.object( - UserYaml, - "get_final_yaml", - return_value=UserYaml.from_dict( - {"yaml_for": f"commit_{commit.commitid}", "origin": "database"} - ), - ) - task = CleanLabelsIndexTask() - res = task._get_best_effort_commit_yaml(commit, None) - assert res == {"yaml_for": f"commit_{commit.commitid}", "origin": "database"} - owner = commit.repository.owner - mock_fetch_commit.assert_not_called() - mock_final_yaml.assert_called_with( - owner_yaml=commit.repository.owner.yaml, - repo_yaml=commit.repository.yaml, - commit_yaml=None, - owner_context=OwnerContext( - ownerid=owner.ownerid, - owner_plan=owner.plan, - owner_onboarding_date=owner.createstamp, - ), - ) - - -class TestCleanLabelsIndexLogic(BaseTestCase): - def test_clean_labels_report_not_found(self, dbsession, mocker): - commit = CommitFactory() - dbsession.add(commit) - mock_get_report = mocker.patch.object( - ReportService, "get_existing_report_for_commit", return_value=None - ) - task = CleanLabelsIndexTask() - read_only_args = ReadOnlyArgs(commit=commit, report_code=None, commit_yaml={}) - res = task.run_impl_within_lock(read_only_args) - assert res == {"success": False, "error": "Report not found"} - mock_get_report.assert_called_with(commit, report_code=None) - - def test_clean_labels_no_labels_index_in_report(self, dbsession, mocker): - commit = CommitFactory() - dbsession.add(commit) - report = Report() - assert report.header == {} - mock_get_report = mocker.patch.object( - ReportService, "get_existing_report_for_commit", return_value=report - ) - task = CleanLabelsIndexTask() - read_only_args = ReadOnlyArgs(commit=commit, report_code=None, commit_yaml={}) - res = task.run_impl_within_lock(read_only_args) - assert res == { - "success": False, - "error": "Labels index is empty, nothing to do", - } - mock_get_report.assert_called_with(commit, report_code=None) - - def test_clean_labels_no_change_needed( - self, dbsession, mocker, sample_report_with_labels - ): - commit = CommitFactory() - dbsession.add(commit) - mock_get_report = mocker.patch.object( - ReportService, - "get_existing_report_for_commit", - return_value=sample_report_with_labels, - ) - mock_save_report = mocker.patch.object( - ReportService, "save_report", return_value={"url": "the_storage_path"} - ) - task = CleanLabelsIndexTask() - read_only_args = ReadOnlyArgs(commit=commit, report_code=None, commit_yaml={}) - sample_report_better_read_original = self.convert_report_to_better_readable( - sample_report_with_labels - ) - res = task.run_impl_within_lock(read_only_args) - assert res == {"success": True} - mock_get_report.assert_called_with(commit, report_code=None) - mock_save_report.assert_called() - assert sample_report_with_labels.labels_index == { - SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_index: SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label, - 1: "some_test", - 2: "another_test", - } - assert ( - self.convert_report_to_better_readable(sample_report_with_labels) - == sample_report_better_read_original - ) - - def test_clean_labels_with_renames( - self, dbsession, mocker, sample_report_with_labels_and_renames - ): - commit = CommitFactory() - dbsession.add(commit) - mock_get_report = mocker.patch.object( - ReportService, - "get_existing_report_for_commit", - return_value=sample_report_with_labels_and_renames, - ) - mock_save_report = mocker.patch.object( - ReportService, "save_report", return_value={"url": "the_storage_path"} - ) - task = CleanLabelsIndexTask() - read_only_args = ReadOnlyArgs(commit=commit, report_code=None, commit_yaml={}) - res = task.run_impl_within_lock(read_only_args) - assert res == {"success": True} - mock_get_report.assert_called_with(commit, report_code=None) - mock_save_report.assert_called() - assert sample_report_with_labels_and_renames.labels_index == { - SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_index: SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label, - 1: "another_test", - 2: "some_test_renamed", - } - readable_report = self.convert_report_to_better_readable( - sample_report_with_labels_and_renames - )["archive"] - print(readable_report) - assert readable_report == { - "file.py": [ - ( - 1, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [0])], - ), - ( - 3, - 1, - None, - [[0, 1, None, None, None]], - None, - None, - [(0, 1, None, [1, 2])], - ), - ( - 4, - "1/2", - None, - [[0, "1/2", None, None, None]], - None, - None, - [(0, "1/2", None, [2])], - ), - ( - 5, - "1/2", - None, - [[0, "1/2", None, None, None]], - None, - None, - [(0, "1/2", None, [1])], - ), - ] - } diff --git a/tasks/upload_clean_labels_index.py b/tasks/upload_clean_labels_index.py deleted file mode 100644 index 9aff29efd..000000000 --- a/tasks/upload_clean_labels_index.py +++ /dev/null @@ -1,217 +0,0 @@ -import logging -from collections import defaultdict -from typing import Dict, List, Optional, Set, TypedDict - -from asgiref.sync import async_to_sync -from redis.exceptions import LockError -from shared.reports.resources import Report -from shared.reports.types import CoverageDatapoint -from shared.torngit.base import TorngitBaseAdapter -from shared.utils.enums import TaskConfigGroup -from shared.yaml import UserYaml -from shared.yaml.user_yaml import OwnerContext - -from app import celery_app -from database.models.core import Commit -from services.redis import get_redis_connection -from services.report import ReportService -from services.repository import get_repo_provider_service -from services.yaml.fetcher import fetch_commit_yaml_from_provider -from tasks.base import BaseCodecovTask -from tasks.upload_processor import UPLOAD_PROCESSING_LOCK_NAME - -log = logging.getLogger(__name__) - - -def _prepare_kwargs_for_retry(repoid, commitid, report_code, kwargs): - kwargs.update( - { - "repoid": repoid, - "commitid": commitid, - "report_code": report_code, - } - ) - - -class ReadOnlyArgs(TypedDict): - commit: Commit - report_code: str - commit_yaml: Optional[Dict] - - -# TODO: Move to shared -task_name = f"app.tasks.{TaskConfigGroup.upload.value}.UploadCleanLabelsIndex" - - -class CleanLabelsIndexTask( - BaseCodecovTask, - name=task_name, -): - def run_impl(self, db_session, repoid, commitid, report_code=None, *args, **kwargs): - redis_connection = get_redis_connection() - repoid = int(repoid) - lock_name = UPLOAD_PROCESSING_LOCK_NAME(repoid, commitid) - if self._is_currently_processing(redis_connection, lock_name): - log.info( - "Currently processing upload. Retrying in 300s.", - extra=dict( - repoid=repoid, - commit=commitid, - ), - ) - _prepare_kwargs_for_retry(repoid, commitid, report_code, kwargs) - self.retry(countdown=300, kwargs=kwargs) - # Collect as much info as possible outside the lock - # so that the time we stay with the lock is as small as possible - commit = self._get_commit_or_fail(db_session, repoid, commitid) - repository_service = get_repo_provider_service(commit.repository) - commit_yaml = self._get_best_effort_commit_yaml(commit, repository_service) - read_only_args = ReadOnlyArgs( - commit=commit, commit_yaml=commit_yaml, report_code=report_code - ) - try: - with redis_connection.lock( - UPLOAD_PROCESSING_LOCK_NAME(repoid, commitid), - timeout=max(300, self.hard_time_limit_task), - blocking_timeout=5, - ): - return self.run_impl_within_lock( - db_session, - read_only_args, - *args, - **kwargs, - ) - except LockError: - log.warning( - "Unable to acquire lock for key %s.", - lock_name, - extra=dict(commit=commitid, repoid=repoid), - ) - retry_countdown = 20 * 2**self.request.retries + 280 - log.warning( - "Retrying clean labels index task", - extra=dict(commit=commitid, repoid=repoid, countdown=int(retry_countdown)), - ) - _prepare_kwargs_for_retry(repoid, commitid, report_code, kwargs) - self.retry(max_retries=3, countdown=retry_countdown, kwargs=kwargs) - - def run_impl_within_lock( - self, - read_only_args: ReadOnlyArgs, - *args, - **kwargs, - ): - commit = read_only_args["commit"] - report_code = read_only_args["report_code"] - log.info( - "Starting cleanup of labels index", - extra=dict( - repoid=commit.repository.repoid, - commit=commit.commitid, - report_code=report_code, - ), - ) - - # Get the report - report_service = ReportService(read_only_args["commit_yaml"]) - - report = report_service.get_existing_report_for_commit( - commit, report_code=report_code - ) - if report is None: - log.error( - "Report not found", - extra=dict(commit=commit.commitid, report_code=report_code), - ) - return {"success": False, "error": "Report not found"} - # Get the labels index and prep report for changes - if not report.labels_index: - log.error( - "Labels index is empty, nothing to do", - extra=dict(commit=commit.commitid, report_code=report_code), - ) - return {"success": False, "error": "Labels index is empty, nothing to do"} - # Make the changes - self.cleanup_report_labels_index(report) - # Save changes - report_service.save_report(report) - return {"success": True} - - def cleanup_report_labels_index(self, report: Report): - used_labels_set: Set[int] = set() - # This is used later as a reference to the datapoints that might be - # updated for a given label. - # it works because it actually saves a reference to the CoverageDatapoint - # so changing the ref changes the datapoint in the Report - datapoints_with_label: Dict[int, List[CoverageDatapoint]] = defaultdict(list) - - # Collect all the labels that are being used - for report_file in report: - for _, report_line in report_file.lines: - if report_line.datapoints: - for datapoint in report_line.datapoints: - for label_id in datapoint.label_ids: - used_labels_set.add(label_id) - datapoints_with_label[label_id].append(datapoint) - - labels_stored_max_index = max(report.labels_index.keys()) - offset = 0 - # The 0 index is special, let's not touch that. - # It's important that we iterate in order so that we always replace labels - # with a SMALLER index - for label_index in range(1, labels_stored_max_index + 1): - if label_index in used_labels_set: - report.labels_index[label_index - offset] = report.labels_index[ - label_index - ] - for datapoint in datapoints_with_label[label_index]: - idx_to_change = datapoint.label_ids.index(label_index) - datapoint.label_ids[idx_to_change] = label_index - offset - else: - # This label is no longer in the report. We can reuse this index - offset += 1 - # After fixing all indexes we can remove the last 'offset' ones - while offset: - del report.labels_index[labels_stored_max_index] - labels_stored_max_index -= 1 - offset -= 1 - - def _get_best_effort_commit_yaml( - self, commit: Commit, repository_service: TorngitBaseAdapter - ) -> Dict: - repository = commit.repository - commit_yaml = None - if repository_service: - commit_yaml = async_to_sync(fetch_commit_yaml_from_provider)( - commit, repository_service - ) - if commit_yaml is None: - context = OwnerContext( - owner_onboarding_date=repository.owner.createstamp, - owner_plan=repository.owner.plan, - ownerid=repository.ownerid, - ) - commit_yaml = UserYaml.get_final_yaml( - owner_yaml=repository.owner.yaml, - repo_yaml=repository.yaml, - commit_yaml=None, - owner_context=context, - ).to_dict() - return commit_yaml - - def _get_commit_or_fail(self, db_session, repoid: int, commitid: str) -> Commit: - commits = db_session.query(Commit).filter( - Commit.repoid == repoid, Commit.commitid == commitid - ) - commit = commits.first() - assert commit, "Commit not found in database." - return commit - - def _is_currently_processing(self, redis_connection, lock_name: str): - if redis_connection.get(lock_name): - return True - return False - - -RegisteredCleanLabelsIndexTask = celery_app.register_task(CleanLabelsIndexTask()) -clean_labels_index_task = celery_app.tasks[RegisteredCleanLabelsIndexTask.name] diff --git a/tasks/upload_finisher.py b/tasks/upload_finisher.py index 19b186ef3..18cd0f6e0 100644 --- a/tasks/upload_finisher.py +++ b/tasks/upload_finisher.py @@ -44,7 +44,6 @@ from services.yaml import read_yaml_field from tasks.base import BaseCodecovTask from tasks.parallel_verification import parallel_verification_task -from tasks.upload_clean_labels_index import task_name as clean_labels_index_task_name from tasks.upload_processor import UPLOAD_PROCESSING_LOCK_NAME, UploadProcessorTask log = logging.getLogger(__name__) @@ -390,24 +389,6 @@ def finish_reports_processing( else: commit.state = "skipped" - if self.should_clean_labels_index(commit_yaml, processing_results): - task = self.app.tasks[clean_labels_index_task_name].apply_async( - kwargs={ - "repoid": repoid, - "commitid": commitid, - "report_code": report_code, - }, - ) - log.info( - "Scheduling clean_labels_index task", - extra=dict( - repoid=repoid, - commit=commitid, - clean_labels_index_task_id=task.id, - parent_task=self.request.parent_id, - ), - ) - if checkpoints: checkpoints.log(UploadFlow.PROCESSING_COMPLETE) if not notifications_called: @@ -415,26 +396,6 @@ def finish_reports_processing( return {"notifications_called": notifications_called} - def should_clean_labels_index( - self, commit_yaml: UserYaml, processing_results: dict - ): - """Returns True if any of the successful processings was uploaded using a flag - that implies labels were uploaded with the report. - """ - - def should_clean_for_flag(flag: str): - config = commit_yaml.get_flag_configuration(flag) - return config and config.get("carryforward_mode", "") == "labels" - - def should_clean_for_processing_result(results): - args = results.get("arguments", {}) - flags_str = args.get("flags", "") - flags = flags_str.split(",") if flags_str else [] - return results["successful"] and any(map(should_clean_for_flag, flags)) - - actual_processing_results = processing_results.get("processings_so_far", []) - return any(map(should_clean_for_processing_result, actual_processing_results)) - def should_call_notifications( self, commit: Commit, From a862064d02542a24f21e87c2cbd2c8376316929c Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Mon, 14 Oct 2024 13:40:23 +0200 Subject: [PATCH 3/5] rip out more of labels --- helpers/labels.py | 73 -- services/report/raw_upload_processor.py | 42 +- services/report/report_builder.py | 39 - .../report/tests/unit/test_report_builder.py | 60 - services/report/tests/unit/test_sessions.py | 79 +- services/tests/test_report.py | 6 +- tasks/label_analysis.py | 591 ---------- tasks/tests/unit/test_label_analysis.py | 1001 ---------------- .../test_label_analysis_encoded_labels.py | 1047 ----------------- tasks/upload_finisher.py | 7 +- 10 files changed, 17 insertions(+), 2928 deletions(-) delete mode 100644 helpers/labels.py delete mode 100644 tasks/label_analysis.py delete mode 100644 tasks/tests/unit/test_label_analysis.py delete mode 100644 tasks/tests/unit/test_label_analysis_encoded_labels.py diff --git a/helpers/labels.py b/helpers/labels.py deleted file mode 100644 index 078440995..000000000 --- a/helpers/labels.py +++ /dev/null @@ -1,73 +0,0 @@ -from enum import Enum - -from shared.reports.resources import Report - -# The SpecialLabelsEnum enum is place to hold sentinels for labels with special -# meanings -# One example is CODECOV_ALL_LABELS_PLACEHOLDER: it's a sentinel for -# "all the labels from this report apply here" -# Imagine a suite, with many tests, that import a particular file -# The imports all happen before any of the tests is executed -# So on this imported file, there might be some global code -# Because it's global, it runs during this import phase -# So it's not attached directly to any test, because it ran -# outside of any tests -# But it is responsible for those tests in a way, since this global variable -# is used in the functions themselves (which run during the tests). At least -# there is no simple way to guarantee the global variable didn't affect -# anything that imported that file -# So, from the coverage perspective, this global-level line was an indirect -# part of every test. So, in the end, if we see this constant in the report -# datapoints, we will replace it with all the labels (tests) that we saw in -# that report -# This is what CODECOV_ALL_LABELS_PLACEHOLDER is - - -class SpecialLabelsEnum(Enum): - CODECOV_ALL_LABELS_PLACEHOLDER = ("Th2dMtk4M_codecov", 0) - - def __init__(self, label, index): - self.corresponding_label = label - self.corresponding_index = index - - -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[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 - { - SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label, - SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_index, - } - - -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[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 - { - SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label, - SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_index, - } diff --git a/services/report/raw_upload_processor.py b/services/report/raw_upload_processor.py index f0fc85069..3437152ec 100644 --- a/services/report/raw_upload_processor.py +++ b/services/report/raw_upload_processor.py @@ -8,7 +8,6 @@ from database.models.reports import Upload from helpers.exceptions import ReportEmptyError, ReportExpiredException -from helpers.labels import get_all_report_labels, get_labels_per_session from services.path_fixer import PathFixer from services.report.parser.types import ParsedRawReport from services.report.report_builder import ReportBuilder @@ -124,7 +123,7 @@ def process_raw_upload( # Adjust sessions removed carryforward sessions that are being replaced if session.flags: session_adjustment = clear_carryforward_sessions( - report, temporary_report, session.flags, commit_yaml, upload + report, session.flags, commit_yaml ) else: session_adjustment = SessionAdjustmentResult([], []) @@ -138,52 +137,21 @@ def process_raw_upload( @sentry_sdk.trace def clear_carryforward_sessions( original_report: Report, - to_merge_report: Report, to_merge_flags: list[str], current_yaml: UserYaml, - upload: Upload | None = None, ) -> SessionAdjustmentResult: - flags_under_carryforward_rules = { + to_fully_overwrite_flags = { f for f in to_merge_flags if current_yaml.flag_has_carryfoward(f) } - to_partially_overwrite_flags = { - f - for f in flags_under_carryforward_rules - if current_yaml.get_flag_configuration(f).get("carryforward_mode") == "labels" - } - to_fully_overwrite_flags = flags_under_carryforward_rules.difference( - to_partially_overwrite_flags - ) - - if upload is None and to_partially_overwrite_flags: - log.warning("Upload is None, but there are partial_overwrite_flags present") session_ids_to_fully_delete = [] - session_ids_to_partially_delete = [] - - if to_fully_overwrite_flags or to_partially_overwrite_flags: + if to_fully_overwrite_flags: for session_id, session in original_report.sessions.items(): if session.session_type == SessionType.carriedforward and session.flags: if any(f in to_fully_overwrite_flags for f in session.flags): session_ids_to_fully_delete.append(session_id) - if any(f in to_partially_overwrite_flags for f in session.flags): - session_ids_to_partially_delete.append(session_id) - actually_fully_deleted_sessions = set() if session_ids_to_fully_delete: original_report.delete_multiple_sessions(session_ids_to_fully_delete) - actually_fully_deleted_sessions.update(session_ids_to_fully_delete) - - if session_ids_to_partially_delete: - all_labels = get_all_report_labels(to_merge_report) - original_report.delete_labels(session_ids_to_partially_delete, all_labels) - for s in session_ids_to_partially_delete: - labels_now = get_labels_per_session(original_report, s) - if not labels_now: - actually_fully_deleted_sessions.add(s) - original_report.delete_session(s) - - return SessionAdjustmentResult( - sorted(actually_fully_deleted_sessions), - sorted(set(session_ids_to_partially_delete) - actually_fully_deleted_sessions), - ) + + return SessionAdjustmentResult(sorted(session_ids_to_fully_delete), []) diff --git a/services/report/report_builder.py b/services/report/report_builder.py index 8eb6ef87f..17ea98563 100644 --- a/services/report/report_builder.py +++ b/services/report/report_builder.py @@ -107,42 +107,3 @@ def __init__( def create_report_builder_session(self, filepath) -> ReportBuilderSession: return ReportBuilderSession(self, filepath) - - def supports_labels(self) -> bool: - """Returns wether a report supports labels. - This is true if the client has configured some flag with carryforward_mode == "labels" - """ - if self.current_yaml is None or self.current_yaml == {}: - return False - old_flag_style = self.current_yaml.get("flags") - flag_management = self.current_yaml.get("flag_management") - # Check if some of the old style flags uses labels - old_flag_with_carryforward_labels = False - if old_flag_style: - old_flag_with_carryforward_labels = any( - map( - lambda flag_definition: flag_definition.get("carryforward_mode") - == "labels", - old_flag_style.values(), - ) - ) - # Check if some of the flags or default rules use labels - flag_management_default_rule_carryforward_labels = False - flag_management_flag_with_carryforward_labels = False - if flag_management: - flag_management_default_rule_carryforward_labels = ( - flag_management.get("default_rules", {}).get("carryforward_mode") - == "labels" - ) - flag_management_flag_with_carryforward_labels = any( - map( - lambda flag_definition: flag_definition.get("carryforward_mode") - == "labels", - flag_management.get("individual_flags", []), - ) - ) - return ( - old_flag_with_carryforward_labels - or flag_management_default_rule_carryforward_labels - or flag_management_flag_with_carryforward_labels - ) diff --git a/services/report/tests/unit/test_report_builder.py b/services/report/tests/unit/test_report_builder.py index 2aa890f8b..7a19bc422 100644 --- a/services/report/tests/unit/test_report_builder.py +++ b/services/report/tests/unit/test_report_builder.py @@ -1,4 +1,3 @@ -import pytest from shared.reports.resources import LineSession, ReportFile, ReportLine from services.report.report_builder import CoverageType, ReportBuilder @@ -178,62 +177,3 @@ def test_report_builder_session_create_line_mixed_labels(mocker): ) ], ) - - -@pytest.mark.parametrize( - "current_yaml,expected_result", - [ - ({}, False), - ({"flags": {"oldflag": {"carryforward": "true"}}}, False), - ( - { - "flags": { - "oldflag": {"carryforward": "true", "carryforward_mode": "labels"} - } - }, - True, - ), - ( - { - "flag_management": { - "default_rules": { - "carryforward": "true", - "carryforward_mode": "labels", - } - } - }, - True, - ), - ( - { - "flag_management": { - "default_rules": { - "carryforward": "true", - "carryforward_mode": "all", - } - } - }, - False, - ), - ( - { - "flag_management": { - "default_rules": { - "carryforward": "true", - "carryforward_mode": "all", - }, - "individual_flags": [ - { - "name": "some_flag", - "carryforward_mode": "labels", - } - ], - } - }, - True, - ), - ], -) -def test_report_builder_supports_flags(current_yaml, expected_result): - builder = ReportBuilder(current_yaml, 0, None, None) - assert builder.supports_labels() == expected_result diff --git a/services/report/tests/unit/test_sessions.py b/services/report/tests/unit/test_sessions.py index f1fe7d318..2ec2a1254 100644 --- a/services/report/tests/unit/test_sessions.py +++ b/services/report/tests/unit/test_sessions.py @@ -1,14 +1,6 @@ import pytest -from mock import MagicMock from shared.reports.editable import EditableReport, EditableReportFile -from shared.reports.resources import ( - LineSession, - Report, - ReportFile, - ReportLine, - Session, - SessionType, -) +from shared.reports.resources import LineSession, ReportLine, Session, SessionType from shared.yaml import UserYaml from services.report.raw_upload_processor import ( @@ -189,19 +181,15 @@ def create_sample_line(self, *, coverage, sessionid=None): def test_adjust_sessions_no_cf(self, sample_first_report): first_value = self.convert_report_to_better_readable(sample_first_report) - first_to_merge_session = Session(flags=["enterprise"], id=3) - second_report = Report(sessions={3: first_to_merge_session}) current_yaml = UserYaml({}) assert clear_carryforward_sessions( - sample_first_report, second_report, ["enterprise"], current_yaml + sample_first_report, ["enterprise"], current_yaml ) == SessionAdjustmentResult([], []) assert first_value == self.convert_report_to_better_readable( sample_first_report ) def test_adjust_sessions_full_cf_only(self, sample_first_report): - first_to_merge_session = Session(flags=["enterprise"], id=3) - second_report = Report(sessions={3: first_to_merge_session}) current_yaml = UserYaml( { "flag_management": { @@ -210,7 +198,7 @@ def test_adjust_sessions_full_cf_only(self, sample_first_report): } ) assert clear_carryforward_sessions( - sample_first_report, second_report, ["enterprise"], current_yaml + sample_first_report, ["enterprise"], current_yaml ) == SessionAdjustmentResult([0], []) assert self.convert_report_to_better_readable(sample_first_report) == { @@ -387,10 +375,6 @@ def test_adjust_sessions_full_cf_only(self, sample_first_report): def test_adjust_sessions_partial_cf_only_no_changes( self, sample_first_report, mocker ): - first_to_merge_session = Session(flags=["enterprise"], id=3) - second_report = Report( - sessions={first_to_merge_session.id: first_to_merge_session} - ) current_yaml = UserYaml( { "flag_management": { @@ -406,7 +390,7 @@ def test_adjust_sessions_partial_cf_only_no_changes( ) first_value = self.convert_report_to_better_readable(sample_first_report) assert clear_carryforward_sessions( - sample_first_report, second_report, ["enterprise"], current_yaml + sample_first_report, ["enterprise"], current_yaml ) == SessionAdjustmentResult([], [0]) after_result = self.convert_report_to_better_readable(sample_first_report) assert after_result == first_value @@ -414,10 +398,6 @@ def test_adjust_sessions_partial_cf_only_no_changes( def test_adjust_sessions_partial_cf_only_no_changes_encoding_labels( self, sample_first_report ): - first_to_merge_session = Session(flags=["enterprise"], id=3) - second_report = Report( - sessions={first_to_merge_session.id: first_to_merge_session} - ) current_yaml = UserYaml( { "flag_management": { @@ -432,36 +412,13 @@ def test_adjust_sessions_partial_cf_only_no_changes_encoding_labels( } ) first_value = self.convert_report_to_better_readable(sample_first_report) - upload = MagicMock( - name="fake_upload", - **{ - "report": MagicMock( - name="fake_commit_report", - **{ - "code": None, - "commit": MagicMock( - name="fake_commit", - **{"repository": MagicMock(name="fake_repo")}, - ), - }, - ) - }, - ) assert clear_carryforward_sessions( - sample_first_report, - second_report, - ["enterprise"], - current_yaml, - upload=upload, + sample_first_report, ["enterprise"], current_yaml ) == SessionAdjustmentResult([], [0]) after_result = self.convert_report_to_better_readable(sample_first_report) assert after_result == first_value def test_adjust_sessions_partial_cf_only_some_changes(self, sample_first_report): - first_to_merge_session = Session(flags=["enterprise"], id=3) - second_report = Report( - sessions={first_to_merge_session.id: first_to_merge_session} - ) current_yaml = UserYaml( { "flag_management": { @@ -475,16 +432,8 @@ def test_adjust_sessions_partial_cf_only_some_changes(self, sample_first_report) } } ) - second_report_file = ReportFile("unrelatedfile.py") - second_report_file.append( - 90, - self.create_sample_line( - coverage=90, sessionid=3, list_of_lists_of_labels=[["one_label"]] - ), - ) - second_report.append(second_report_file) assert clear_carryforward_sessions( - sample_first_report, second_report, ["enterprise"], current_yaml + sample_first_report, ["enterprise"], current_yaml ) == SessionAdjustmentResult([], [0]) assert self.convert_report_to_better_readable(sample_first_report) == { @@ -691,8 +640,6 @@ def test_adjust_sessions_partial_cf_only_some_changes(self, sample_first_report) def test_adjust_sessions_partial_cf_only_full_deletion_due_to_lost_labels( self, sample_first_report ): - first_to_merge_session = Session(flags=["enterprise"], id=3) - second_report = Report(sessions={3: first_to_merge_session}) current_yaml = UserYaml( { "flag_management": { @@ -707,20 +654,8 @@ def test_adjust_sessions_partial_cf_only_full_deletion_due_to_lost_labels( } ) - second_report_file = ReportFile("unrelatedfile.py") - second_report_file.append( - 90, - self.create_sample_line(coverage=90, sessionid=3), - ) - a_report_file = ReportFile("first_file.py") - a_report_file.append( - 90, - self.create_sample_line(coverage=90, sessionid=3), - ) - second_report.append(second_report_file) - second_report.append(a_report_file) assert clear_carryforward_sessions( - sample_first_report, second_report, ["enterprise"], current_yaml + sample_first_report, ["enterprise"], current_yaml ) == SessionAdjustmentResult([0], []) res = self.convert_report_to_better_readable(sample_first_report) diff --git a/services/tests/test_report.py b/services/tests/test_report.py index 94dce7654..1999c64bd 100644 --- a/services/tests/test_report.py +++ b/services/tests/test_report.py @@ -1423,7 +1423,7 @@ def fake_possibly_shift(report, base, head): report.add_session(to_merge_session) assert sorted(report.sessions.keys()) == [2, 3, 4] assert clear_carryforward_sessions( - report, Report(), ["enterprise"], UserYaml(yaml_dict) + report, ["enterprise"], UserYaml(yaml_dict) ) == SessionAdjustmentResult( fully_deleted_sessions=[2, 3], partially_deleted_sessions=[] ) @@ -1495,7 +1495,7 @@ def test_get_existing_report_for_commit_already_carriedforward_add_sessions( report.add_session(first_to_merge_session) assert sorted(report.sessions.keys()) == [0, 1, 2, 3, 4] assert clear_carryforward_sessions( - report, Report(), ["enterprise"], UserYaml(yaml_dict) + report, ["enterprise"], UserYaml(yaml_dict) ) == SessionAdjustmentResult( fully_deleted_sessions=[2, 3], partially_deleted_sessions=[] ) @@ -1571,7 +1571,7 @@ def test_get_existing_report_for_commit_already_carriedforward_add_sessions( report.add_session(second_to_merge_session) assert sorted(report.sessions.keys()) == [0, 1, 3, 4] assert clear_carryforward_sessions( - report, Report(), ["unit"], UserYaml(yaml_dict) + report, ["unit"], UserYaml(yaml_dict) ) == SessionAdjustmentResult( fully_deleted_sessions=[], partially_deleted_sessions=[] ) diff --git a/tasks/label_analysis.py b/tasks/label_analysis.py deleted file mode 100644 index 476d7c235..000000000 --- a/tasks/label_analysis.py +++ /dev/null @@ -1,591 +0,0 @@ -import logging -from typing import Dict, List, NamedTuple, Optional, Set, Tuple, TypedDict, Union - -import sentry_sdk -from asgiref.sync import async_to_sync -from shared.celery_config import label_analysis_task_name -from shared.labelanalysis import LabelAnalysisRequestState -from sqlalchemy.orm import Session - -from app import celery_app -from database.models.labelanalysis import ( - LabelAnalysisProcessingError, - LabelAnalysisProcessingErrorCode, - LabelAnalysisRequest, -) -from database.models.staticanalysis import StaticAnalysisSuite -from helpers.labels import get_all_report_labels, get_labels_per_session -from helpers.metrics import metrics -from helpers.telemetry import MetricContext -from services.report import Report, ReportService -from services.report.report_builder import SpecialLabelsEnum -from services.repository import get_repo_provider_service -from services.static_analysis import StaticAnalysisComparisonService -from services.static_analysis.git_diff_parser import DiffChange, parse_git_diff_json -from services.yaml import get_repo_yaml -from tasks.base import BaseCodecovTask - -log = logging.getLogger(__name__) - - -GLOBAL_LEVEL_LABEL = ( - SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label -) - -GLOBAL_LEVEL_LABEL_IDX = ( - SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_index -) - - -class LinesRelevantToChangeInFile(TypedDict): - all: bool - lines: Set[int] - - -class LinesRelevantToChange(TypedDict): - all: bool - files: Dict[str, Optional[LinesRelevantToChangeInFile]] - - -class ExistingLabelSetsEncoded(NamedTuple): - all_report_labels: Set[int] - executable_lines_labels: Set[int] - global_level_labels: Set[int] - are_labels_encoded: bool = True - - -class ExistingLabelSetsNotEncoded(NamedTuple): - all_report_labels: Set[str] - executable_lines_labels: Set[str] - global_level_labels: Set[str] - are_labels_encoded: bool = False - - -ExistingLabelSets = Union[ExistingLabelSetsEncoded, ExistingLabelSetsNotEncoded] -PossiblyEncodedLabelSet = Union[Set[str], Set[int]] - - -class LabelAnalysisRequestProcessingTask( - BaseCodecovTask, name=label_analysis_task_name -): - errors: List[LabelAnalysisProcessingError] = None - dbsession: Session = None - metrics_context: MetricContext = None - - def reset_task_context(self): - """Resets the task's attributes to None to avoid spilling information - between task calls in the same process. - https://docs.celeryq.dev/en/latest/userguide/tasks.html#instantiation - """ - self.errors = None - self.dbsession = None - self.metrics_context = None - - def run_impl(self, db_session, request_id, *args, **kwargs): - self.errors = [] - self.dbsession = db_session - label_analysis_request = ( - db_session.query(LabelAnalysisRequest) - .filter(LabelAnalysisRequest.id_ == request_id) - .first() - ) - if label_analysis_request is None: - metrics.incr("label_analysis_task.failed_to_calculate.larq_not_found") - log.error( - "LabelAnalysisRequest not found", extra=dict(request_id=request_id) - ) - self.add_processing_error( - larq_id=request_id, - error_code=LabelAnalysisProcessingErrorCode.NOT_FOUND, - error_msg="LabelAnalysisRequest not found", - error_extra=dict(), - ) - response = { - "success": False, - "present_report_labels": [], - "present_diff_labels": [], - "absent_labels": [], - "global_level_labels": [], - "errors": self.errors, - } - self.reset_task_context() - return response - log.info( - "Starting label analysis request", - extra=dict( - request_id=request_id, - external_id=label_analysis_request.external_id, - commit=label_analysis_request.head_commit.commitid, - ), - ) - self.metrics_context = MetricContext( - repo_id=label_analysis_request.head_commit.repository.repoid, - commit_id=label_analysis_request.head_commit.id, - ) - - if label_analysis_request.state_id == LabelAnalysisRequestState.FINISHED.db_id: - # Indicates that this request has been calculated already - # We might need to update the requested labels - response = self._handle_larq_already_calculated(label_analysis_request) - self.reset_task_context() - return response - - try: - lines_relevant_to_diff: Optional[LinesRelevantToChange] = ( - self._get_lines_relevant_to_diff(label_analysis_request) - ) - base_report = self._get_base_report(label_analysis_request) - - if lines_relevant_to_diff and base_report: - existing_labels: ExistingLabelSets = self._get_existing_labels( - base_report, lines_relevant_to_diff - ) - if existing_labels.are_labels_encoded: - # Translate label_ids - def partial_fn_to_apply(label_id_set): - return self._lookup_label_ids( - report=base_report, label_ids=label_id_set - ) - - existing_labels = ExistingLabelSetsNotEncoded( - all_report_labels=partial_fn_to_apply( - existing_labels.all_report_labels - ), - executable_lines_labels=partial_fn_to_apply( - existing_labels.executable_lines_labels - ), - global_level_labels=partial_fn_to_apply( - existing_labels.global_level_labels - ), - are_labels_encoded=False, - ) - - requested_labels = self._get_requested_labels(label_analysis_request) - result = self.calculate_final_result( - requested_labels=requested_labels, - existing_labels=existing_labels, - commit_sha=label_analysis_request.head_commit.commitid, - ) - label_analysis_request.result = result - label_analysis_request.state_id = ( - LabelAnalysisRequestState.FINISHED.db_id - ) - metrics.incr("label_analysis_task.success") - response = { - "success": True, - "present_report_labels": result["present_report_labels"], - "present_diff_labels": result["present_diff_labels"], - "absent_labels": result["absent_labels"], - "global_level_labels": result["global_level_labels"], - "errors": self.errors, - } - self.reset_task_context() - return response - except Exception: - # temporary general catch while we find possible problems on this - metrics.incr("label_analysis_task.failed_to_calculate.exception") - log.exception( - "Label analysis failed to calculate", - extra=dict( - request_id=request_id, - commit=label_analysis_request.head_commit.commitid, - external_id=label_analysis_request.external_id, - ), - ) - label_analysis_request.result = None - label_analysis_request.state_id = LabelAnalysisRequestState.ERROR.db_id - self.add_processing_error( - larq_id=request_id, - error_code=LabelAnalysisProcessingErrorCode.FAILED, - error_msg="Failed to calculate", - error_extra=dict(), - ) - response = { - "success": False, - "present_report_labels": [], - "present_diff_labels": [], - "absent_labels": [], - "global_level_labels": [], - "errors": self.errors, - } - self.reset_task_context() - return response - metrics.incr("label_analysis_task.failed_to_calculate.missing_info") - log.warning( - "We failed to get some information that was important to label analysis", - extra=dict( - has_relevant_lines=(lines_relevant_to_diff is not None), - has_base_report=(base_report is not None), - commit=label_analysis_request.head_commit.commitid, - external_id=label_analysis_request.external_id, - request_id=request_id, - ), - ) - label_analysis_request.state_id = LabelAnalysisRequestState.FINISHED.db_id - result_to_save = { - "success": True, - "present_report_labels": [], - "present_diff_labels": [], - "absent_labels": label_analysis_request.requested_labels, - "global_level_labels": [], - } - label_analysis_request.result = result_to_save - result_to_return = {**result_to_save, "errors": self.errors} - self.reset_task_context() - return result_to_return - - def add_processing_error( - self, - larq_id: int, - error_code: LabelAnalysisProcessingErrorCode, - error_msg: str, - error_extra: dict, - ): - error = LabelAnalysisProcessingError( - label_analysis_request_id=larq_id, - error_code=error_code.value, - error_params=dict(message=error_msg, extra=error_extra), - ) - self.errors.append(error.to_representation()) - self.dbsession.add(error) - - def _handle_larq_already_calculated(self, larq: LabelAnalysisRequest): - # This means we already calculated everything - # Except possibly the absent labels - log.info( - "Label analysis request was already calculated", - extra=dict( - request_id=larq.id, - external_id=larq.external_id, - commit=larq.head_commit.commitid, - ), - ) - if larq.requested_labels: - saved_result = larq.result - all_saved_labels = set( - saved_result.get("present_report_labels", []) - + saved_result.get("present_diff_labels", []) - + saved_result.get("global_level_labels", []) - ) - executable_lines_saved_labels = set( - saved_result.get("present_diff_labels", []) - ) - global_saved_labels = set(saved_result.get("global_level_labels", [])) - result = self.calculate_final_result( - requested_labels=larq.requested_labels, - existing_labels=ExistingLabelSetsNotEncoded( - all_saved_labels, executable_lines_saved_labels, global_saved_labels - ), - commit_sha=larq.head_commit.commitid, - ) - larq.result = result # Save the new result - metrics.incr("label_analysis_task.already_calculated.new_result") - return {**result, "success": True, "errors": []} - # No requested labels mean we don't have any new information - # So we don't need to calculate again - # This shouldn't actually happen - metrics.incr("label_analysis_task.already_calculated.same_result") - return {**larq.result, "success": True, "errors": []} - - def _lookup_label_ids(self, report: Report, label_ids: Set[int]) -> Set[str]: - labels: Set[str] = set() - for label_id in label_ids: - # This can raise shared.reports.exceptions.LabelNotFoundError - # But (1) we shouldn't let that happen and (2) there's no recovering from it - # So we should let that happen to surface bugs to us - labels.add(report.lookup_label_by_id(label_id)) - return labels - - def _get_requested_labels(self, label_analysis_request: LabelAnalysisRequest): - if label_analysis_request.requested_labels: - return label_analysis_request.requested_labels - # This is the case where the CLI PATCH the requested labels after collecting them - self.dbsession.refresh(label_analysis_request, ["requested_labels"]) - return label_analysis_request.requested_labels - - @sentry_sdk.trace - def _get_existing_labels( - self, report: Report, lines_relevant_to_diff: LinesRelevantToChange - ) -> ExistingLabelSets: - all_report_labels = self.get_all_report_labels(report) - ( - executable_lines_labels, - global_level_labels, - ) = self.get_executable_lines_labels(report, lines_relevant_to_diff) - - if len(all_report_labels) > 0: - # Check if report labels are encoded or not - test_label = all_report_labels.pop() - are_labels_encoded = isinstance(test_label, int) - all_report_labels.add(test_label) - else: - # There are no labels in the report - are_labels_encoded = False - - class_to_use = ( - ExistingLabelSetsEncoded - if are_labels_encoded - else ExistingLabelSetsNotEncoded - ) - - return class_to_use( - all_report_labels=all_report_labels, - executable_lines_labels=executable_lines_labels, - global_level_labels=global_level_labels, - ) - - @sentry_sdk.trace - def _get_lines_relevant_to_diff(self, label_analysis_request: LabelAnalysisRequest): - parsed_git_diff = self._get_parsed_git_diff(label_analysis_request) - if parsed_git_diff: - executable_lines_relevant_to_diff = self.get_relevant_executable_lines( - label_analysis_request, parsed_git_diff - ) - # This line will be useful for debugging - # And to tweak the heuristics - log.info( - "Lines relevant to diff", - extra=dict( - lines_relevant_to_diff=executable_lines_relevant_to_diff, - commit=label_analysis_request.head_commit.commitid, - external_id=label_analysis_request.external_id, - request_id=label_analysis_request.id_, - ), - ) - return executable_lines_relevant_to_diff - return None - - @sentry_sdk.trace - def _get_parsed_git_diff( - self, label_analysis_request: LabelAnalysisRequest - ) -> Optional[List[DiffChange]]: - try: - repo_service = get_repo_provider_service( - label_analysis_request.head_commit.repository - ) - git_diff = async_to_sync(repo_service.get_compare)( - label_analysis_request.base_commit.commitid, - label_analysis_request.head_commit.commitid, - ) - return list(parse_git_diff_json(git_diff)) - except Exception: - # temporary general catch while we find possible problems on this - log.exception( - "Label analysis failed to parse git diff", - extra=dict( - request_id=label_analysis_request.id, - external_id=label_analysis_request.external_id, - commit=label_analysis_request.head_commit.commitid, - ), - ) - self.add_processing_error( - larq_id=label_analysis_request.id, - error_code=LabelAnalysisProcessingErrorCode.FAILED, - error_msg="Failed to parse git diff", - error_extra=dict( - head_commit=label_analysis_request.head_commit.commitid, - base_commit=label_analysis_request.base_commit.commitid, - ), - ) - return None - - @sentry_sdk.trace - def _get_base_report( - self, label_analysis_request: LabelAnalysisRequest - ) -> Optional[Report]: - base_commit = label_analysis_request.base_commit - current_yaml = get_repo_yaml(base_commit.repository) - report_service = ReportService(current_yaml) - report: Report = report_service.get_existing_report_for_commit(base_commit) - if report is None: - log.warning( - "No report found for label analysis", - extra=dict( - request_id=label_analysis_request.id, - commit=label_analysis_request.head_commit.commitid, - ), - ) - self.add_processing_error( - larq_id=label_analysis_request.id, - error_code=LabelAnalysisProcessingErrorCode.MISSING_DATA, - error_msg="Missing base report", - error_extra=dict( - head_commit=label_analysis_request.head_commit.commitid, - base_commit=label_analysis_request.base_commit.commitid, - ), - ) - return report - - @sentry_sdk.trace - def calculate_final_result( - self, - *, - requested_labels: Optional[List[str]], - existing_labels: ExistingLabelSetsNotEncoded, - commit_sha: str, - ): - all_report_labels = existing_labels.all_report_labels - executable_lines_labels = existing_labels.executable_lines_labels - global_level_labels = existing_labels.global_level_labels - log.info( - "Final info", - extra=dict( - executable_lines_labels=sorted(executable_lines_labels), - all_report_labels=all_report_labels, - requested_labels=requested_labels, - global_level_labels=sorted(global_level_labels), - commit=commit_sha, - ), - ) - self.metrics_context.log_simple_metric( - "label_analysis.tests_saved_count", len(all_report_labels) - ) - self.metrics_context.log_simple_metric( - "label_analysis.requests_with_requested_labels", - float(requested_labels is not None), - ) - if requested_labels is not None: - requested_labels = set(requested_labels) - ans = { - "present_report_labels": sorted(all_report_labels & requested_labels), - "present_diff_labels": sorted( - executable_lines_labels & requested_labels - ), - "absent_labels": sorted(requested_labels - all_report_labels), - "global_level_labels": sorted(global_level_labels & requested_labels), - } - self.metrics_context.log_simple_metric( - "label_analysis.requested_labels_count", len(requested_labels) - ) - self.metrics_context.log_simple_metric( - "label_analysis.tests_to_run_count", - len( - ans["present_diff_labels"] - + ans["global_level_labels"] - + ans["absent_labels"] - ), - ) - return ans - self.metrics_context.log_simple_metric( - "label_analysis.tests_to_run_count", - len(executable_lines_labels | global_level_labels), - ) - return { - "present_report_labels": sorted(all_report_labels), - "present_diff_labels": sorted(executable_lines_labels), - "absent_labels": [], - "global_level_labels": sorted(global_level_labels), - } - - @sentry_sdk.trace - def get_relevant_executable_lines( - self, label_analysis_request: LabelAnalysisRequest, parsed_git_diff - ): - db_session = label_analysis_request.get_db_session() - base_static_analysis: StaticAnalysisSuite = ( - db_session.query(StaticAnalysisSuite) - .filter( - StaticAnalysisSuite.commit_id == label_analysis_request.base_commit_id, - ) - .first() - ) - head_static_analysis: StaticAnalysisSuite = ( - db_session.query(StaticAnalysisSuite) - .filter( - StaticAnalysisSuite.commit_id == label_analysis_request.head_commit_id, - ) - .first() - ) - if not base_static_analysis or not head_static_analysis: - # TODO : Proper handling of this case - log.info( - "Trying to make prediction where there are no static analyses", - extra=dict( - base_static_analysis=base_static_analysis.id_ - if base_static_analysis is not None - else None, - head_static_analysis=head_static_analysis.id_ - if head_static_analysis is not None - else None, - commit=label_analysis_request.head_commit.commitid, - ), - ) - self.add_processing_error( - larq_id=label_analysis_request.id, - error_code=LabelAnalysisProcessingErrorCode.MISSING_DATA, - error_msg="Missing static analysis info", - error_extra=dict( - head_commit=label_analysis_request.head_commit.commitid, - base_commit=label_analysis_request.base_commit.commitid, - has_base_static_analysis=(base_static_analysis is not None), - has_head_static_analysis=(head_static_analysis is not None), - ), - ) - return None - static_analysis_comparison_service = StaticAnalysisComparisonService( - base_static_analysis, - head_static_analysis, - parsed_git_diff, - ) - return static_analysis_comparison_service.get_base_lines_relevant_to_change() - - @sentry_sdk.trace - def get_executable_lines_labels( - self, report: Report, executable_lines: LinesRelevantToChange - ) -> Tuple[PossiblyEncodedLabelSet, PossiblyEncodedLabelSet]: - if executable_lines["all"]: - return (self.get_all_report_labels(report), set()) - full_sessions = set() - labels: PossiblyEncodedLabelSet = set() - global_level_labels = set() - # Prime piece of code to be rust-ifyied - for name, file_executable_lines in executable_lines["files"].items(): - rf = report.get(name) - if rf and file_executable_lines: - if file_executable_lines["all"]: - for line_number, line in rf.lines: - if line and line.datapoints: - for datapoint in line.datapoints: - dp_labels = datapoint.label_ids or [] - labels.update(dp_labels) - if ( - # If labels are encoded - GLOBAL_LEVEL_LABEL_IDX in dp_labels - # If labels are NOT encoded - or GLOBAL_LEVEL_LABEL in dp_labels - ): - full_sessions.add(datapoint.sessionid) - else: - for line_number in file_executable_lines["lines"]: - line = rf.get(line_number) - if line and line.datapoints: - for datapoint in line.datapoints: - dp_labels = datapoint.label_ids or [] - labels.update(dp_labels) - if ( - # If labels are encoded - GLOBAL_LEVEL_LABEL_IDX in dp_labels - # If labels are NOT encoded - or GLOBAL_LEVEL_LABEL in dp_labels - ): - full_sessions.add(datapoint.sessionid) - for sess_id in full_sessions: - global_level_labels.update(self.get_labels_per_session(report, sess_id)) - return ( - labels - set([GLOBAL_LEVEL_LABEL_IDX, GLOBAL_LEVEL_LABEL]), - global_level_labels, - ) - - def get_labels_per_session(self, report: Report, sess_id: int): - return get_labels_per_session(report, sess_id) - - def get_all_report_labels(self, report: Report) -> set: - return get_all_report_labels(report) - - -RegisteredLabelAnalysisRequestProcessingTask = celery_app.register_task( - LabelAnalysisRequestProcessingTask() -) -label_analysis_task = celery_app.tasks[ - RegisteredLabelAnalysisRequestProcessingTask.name -] diff --git a/tasks/tests/unit/test_label_analysis.py b/tasks/tests/unit/test_label_analysis.py deleted file mode 100644 index 9c44e9a6e..000000000 --- a/tasks/tests/unit/test_label_analysis.py +++ /dev/null @@ -1,1001 +0,0 @@ -import json - -import pytest -from mock import patch -from shared.reports.resources import LineSession, Report, ReportFile, ReportLine -from shared.reports.types import CoverageDatapoint - -from database.models.labelanalysis import LabelAnalysisRequest -from database.tests.factories import RepositoryFactory -from database.tests.factories.labelanalysis import LabelAnalysisRequestFactory -from database.tests.factories.staticanalysis import ( - StaticAnalysisSingleFileSnapshotFactory, - StaticAnalysisSuiteFactory, - StaticAnalysisSuiteFilepathFactory, -) -from services.report import ReportService -from services.static_analysis import StaticAnalysisComparisonService -from tasks.label_analysis import ( - LabelAnalysisRequestProcessingTask, - LabelAnalysisRequestState, -) - -sample_head_static_analysis_dict = { - "empty_lines": [2, 3, 11], - "warnings": [], - "filename": "source.py", - "functions": [ - { - "identifier": "some_function", - "start_line": 6, - "end_line": 10, - "code_hash": "e69c18eff7d24f8bad3370db87f64333", - "complexity_metrics": { - "conditions": 1, - "mccabe_cyclomatic_complexity": 2, - "returns": 1, - "max_nested_conditional": 1, - }, - } - ], - "hash": "84d371ab1c57d2349038ac3671428803", - "language": "python", - "number_lines": 11, - "statements": [ - ( - 1, - { - "line_surety_ancestorship": None, - "start_column": 0, - "line_hash": "55c30cf01e202728b6952e9cba304798", - "len": 0, - "extra_connected_lines": (), - }, - ), - ( - 5, - { - "line_surety_ancestorship": None, - "start_column": 4, - "line_hash": "1d7be9f2145760a59513a4049fcd0d1c", - "len": 0, - "extra_connected_lines": (), - }, - ), - ( - 6, - { - "line_surety_ancestorship": 5, - "start_column": 4, - "line_hash": "f802087a854c26782ee8d4ece7214425", - "len": 0, - "extra_connected_lines": (), - }, - ), - ( - 7, - { - "line_surety_ancestorship": None, - "start_column": 8, - "line_hash": "6ae3393fa7880fe8a844c03256cac37b", - "len": 0, - "extra_connected_lines": (), - }, - ), - ( - 8, - { - "line_surety_ancestorship": 6, - "start_column": 4, - "line_hash": "5b099d1822e9236c540a5701a657225e", - "len": 0, - "extra_connected_lines": (), - }, - ), - ( - 9, - { - "line_surety_ancestorship": 8, - "start_column": 4, - "line_hash": "e5d4915bb7dddeb18f53dc9fde9a3064", - "len": 0, - "extra_connected_lines": (), - }, - ), - ( - 10, - { - "line_surety_ancestorship": 9, - "start_column": 4, - "line_hash": "e70ce43136171575ee525375b10f91a1", - "len": 0, - "extra_connected_lines": (), - }, - ), - ], - "definition_lines": [(4, 6)], - "import_lines": [], -} - -sample_base_static_analysis_dict = { - "empty_lines": [2, 3, 11], - "warnings": [], - "filename": "source.py", - "functions": [ - { - "identifier": "some_function", - "start_line": 6, - "end_line": 10, - "code_hash": "e4b52b6da12184142fcd7ff2c8412662", - "complexity_metrics": { - "conditions": 1, - "mccabe_cyclomatic_complexity": 2, - "returns": 1, - "max_nested_conditional": 1, - }, - } - ], - "hash": "811d0016249a5b1400a685164e5295de", - "language": "python", - "number_lines": 11, - "statements": [ - ( - 1, - { - "line_surety_ancestorship": None, - "start_column": 0, - "line_hash": "55c30cf01e202728b6952e9cba304798", - "len": 0, - "extra_connected_lines": (), - }, - ), - ( - 5, - { - "line_surety_ancestorship": None, - "start_column": 4, - "line_hash": "1d7be9f2145760a59513a4049fcd0d1c", - "len": 0, - "extra_connected_lines": (), - }, - ), - ( - 6, - { - "line_surety_ancestorship": 5, - "start_column": 4, - "line_hash": "52f98812dca4687f18373b87433df695", - "len": 0, - "extra_connected_lines": (), - }, - ), - ( - 7, - { - "line_surety_ancestorship": None, - "start_column": 8, - "line_hash": "6ae3393fa7880fe8a844c03256cac37b", - "len": 0, - "extra_connected_lines": (), - }, - ), - ( - 8, - { - "line_surety_ancestorship": 7, - "start_column": 8, - "line_hash": "5b099d1822e9236c540a5701a657225e", - "len": 0, - "extra_connected_lines": (), - }, - ), - ( - 9, - { - "line_surety_ancestorship": 6, - "start_column": 4, - "line_hash": "e5d4915bb7dddeb18f53dc9fde9a3064", - "len": 0, - "extra_connected_lines": (), - }, - ), - ( - 10, - { - "line_surety_ancestorship": 9, - "start_column": 4, - "line_hash": "e70ce43136171575ee525375b10f91a1", - "len": 0, - "extra_connected_lines": (), - }, - ), - ], - "definition_lines": [(4, 6)], - "import_lines": [], -} - - -@pytest.fixture -def sample_report_with_labels(): - r = Report() - first_rf = ReportFile("source.py") - first_rf.append( - 5, - ReportLine.create( - coverage=1, - type=None, - sessions=[ - ( - LineSession( - id=1, - coverage=1, - ) - ) - ], - datapoints=[ - CoverageDatapoint( - sessionid=1, - coverage=1, - coverage_type=None, - label_ids=["apple", "label_one", "pineapple", "banana"], - ) - ], - complexity=None, - ), - ) - first_rf.append( - 6, - ReportLine.create( - coverage=1, - type=None, - sessions=[ - ( - LineSession( - id=1, - coverage=1, - ) - ) - ], - datapoints=[ - CoverageDatapoint( - sessionid=1, - coverage=1, - coverage_type=None, - label_ids=["label_one", "pineapple", "banana"], - ) - ], - complexity=None, - ), - ) - first_rf.append( - 7, - ReportLine.create( - coverage=1, - type=None, - sessions=[ - ( - LineSession( - id=1, - coverage=1, - ) - ) - ], - datapoints=[ - CoverageDatapoint( - sessionid=1, - coverage=1, - coverage_type=None, - label_ids=["banana"], - ) - ], - complexity=None, - ), - ) - first_rf.append( - 8, - ReportLine.create( - coverage=1, - type=None, - sessions=[ - ( - LineSession( - id=1, - coverage=1, - ) - ) - ], - datapoints=[ - CoverageDatapoint( - sessionid=1, - coverage=1, - coverage_type=None, - label_ids=["banana"], - ), - CoverageDatapoint( - sessionid=5, - coverage=1, - coverage_type=None, - label_ids=["orangejuice"], - ), - ], - complexity=None, - ), - ) - first_rf.append( - 99, - ReportLine.create( - coverage=1, - type=None, - sessions=[ - ( - LineSession( - id=5, - coverage=1, - ) - ) - ], - datapoints=[ - CoverageDatapoint( - sessionid=5, - coverage=1, - coverage_type=None, - label_ids=["justjuice"], - ), - ], - complexity=None, - ), - ) - first_rf.append( - 8, - ReportLine.create( - coverage=1, - type=None, - sessions=[ - ( - LineSession( - id=1, - coverage=1, - ) - ) - ], - datapoints=[ - CoverageDatapoint( - sessionid=1, - coverage=1, - coverage_type=None, - label_ids=["label_one", "pineapple", "banana"], - ), - CoverageDatapoint( - sessionid=5, - coverage=1, - coverage_type=None, - label_ids=["Th2dMtk4M_codecov", "applejuice"], - ), - ], - complexity=None, - ), - ) - second_rf = ReportFile("path/from/additionsonly.py") - second_rf.append( - 6, - ReportLine.create( - coverage=1, - type=None, - sessions=[ - ( - LineSession( - id=1, - coverage=1, - ) - ) - ], - datapoints=[ - CoverageDatapoint( - sessionid=1, - coverage=1, - coverage_type=None, - label_ids=["whatever", "here"], - ) - ], - complexity=None, - ), - ) - random_rf = ReportFile("path/from/randomfile_no_static_analysis.html") - random_rf.append( - 1, - ReportLine.create( - coverage=1, - type=None, - sessions=[(LineSession(id=1, coverage=1))], - datapoints=None, - complexity=None, - ), - ) - r.append(first_rf) - r.append(second_rf) - r.append(random_rf) - - return r - - -def test_simple_call_without_requested_labels_then_with_requested_labels( - dbsession, mock_storage, mocker, sample_report_with_labels, mock_repo_provider -): - mock_metrics = mocker.patch("tasks.label_analysis.metrics") - mock_metrics_context = mocker.patch("tasks.label_analysis.MetricContext") - mocker.patch.object( - LabelAnalysisRequestProcessingTask, - "_get_lines_relevant_to_diff", - return_value={ - "all": False, - "files": {"source.py": {"all": False, "lines": {8, 6}}}, - }, - ) - mocker.patch.object( - ReportService, - "get_existing_report_for_commit", - return_value=sample_report_with_labels, - ) - repository = RepositoryFactory.create() - larf = LabelAnalysisRequestFactory.create( - base_commit__repository=repository, head_commit__repository=repository - ) - dbsession.add(larf) - dbsession.flush() - base_sasf = StaticAnalysisSuiteFactory.create(commit=larf.base_commit) - head_sasf = StaticAnalysisSuiteFactory.create(commit=larf.head_commit) - dbsession.add(base_sasf) - dbsession.add(head_sasf) - dbsession.flush() - first_path = "abdkasdauchudh.txt" - second_path = "0diao9u3qdsdu.txt" - mock_storage.write_file( - "archive", - first_path, - json.dumps(sample_base_static_analysis_dict), - ) - mock_storage.write_file( - "archive", - second_path, - json.dumps(sample_head_static_analysis_dict), - ) - first_snapshot = StaticAnalysisSingleFileSnapshotFactory.create( - repository=repository, content_location=first_path - ) - second_snapshot = StaticAnalysisSingleFileSnapshotFactory.create( - repository=repository, content_location=second_path - ) - dbsession.add(first_snapshot) - dbsession.add(second_snapshot) - dbsession.flush() - first_base_file = StaticAnalysisSuiteFilepathFactory.create( - file_snapshot=first_snapshot, - analysis_suite=base_sasf, - filepath="source.py", - ) - first_head_file = StaticAnalysisSuiteFilepathFactory.create( - file_snapshot=second_snapshot, - analysis_suite=head_sasf, - filepath="source.py", - ) - dbsession.add(first_base_file) - dbsession.add(first_head_file) - dbsession.flush() - - task = LabelAnalysisRequestProcessingTask() - res = task.run_impl(dbsession, larf.id) - expected_present_report_labels = [ - "apple", - "applejuice", - "banana", - "here", - "justjuice", - "label_one", - "orangejuice", - "pineapple", - "whatever", - ] - expected_present_diff_labels = sorted( - ["applejuice", "banana", "label_one", "orangejuice", "pineapple"] - ) - expected_result = { - "absent_labels": [], - "present_diff_labels": expected_present_diff_labels, - "present_report_labels": expected_present_report_labels, - "global_level_labels": ["applejuice", "justjuice", "orangejuice"], - "success": True, - "errors": [], - } - assert res == expected_result - mock_metrics.incr.assert_called_with("label_analysis_task.success") - mock_metrics_context.assert_called_with( - repo_id=repository.repoid, commit_id=larf.head_commit.id - ) - mock_metrics_context.return_value.log_simple_metric.assert_any_call( - "label_analysis.tests_saved_count", 9 - ) - mock_metrics_context.return_value.log_simple_metric.assert_any_call( - "label_analysis.requests_with_requested_labels", 0.0 - ) - mock_metrics_context.return_value.log_simple_metric.assert_any_call( - "label_analysis.tests_to_run_count", 6 - ) - dbsession.flush() - dbsession.refresh(larf) - assert larf.state_id == LabelAnalysisRequestState.FINISHED.db_id - assert larf.result == { - "absent_labels": [], - "present_diff_labels": expected_present_diff_labels, - "present_report_labels": expected_present_report_labels, - "global_level_labels": ["applejuice", "justjuice", "orangejuice"], - } - # Now we call the task again, this time with the requested labels. - # This illustrates what should happen if we patch the labels after calculating - # And trigger the task again to save the new results - larf.requested_labels = ["tangerine", "pear", "banana", "apple"] - dbsession.flush() - res = task.run_impl(dbsession, larf.id) - expected_present_diff_labels = ["banana"] - expected_present_report_labels = ["apple", "banana"] - expected_absent_labels = ["pear", "tangerine"] - assert res == { - "absent_labels": expected_absent_labels, - "present_diff_labels": expected_present_diff_labels, - "present_report_labels": expected_present_report_labels, - "success": True, - "global_level_labels": [], - "errors": [], - } - assert larf.result == { - "absent_labels": expected_absent_labels, - "present_diff_labels": expected_present_diff_labels, - "present_report_labels": expected_present_report_labels, - "global_level_labels": [], - } - mock_metrics.incr.assert_called_with( - "label_analysis_task.already_calculated.new_result" - ) - mock_metrics_context.return_value.log_simple_metric.assert_any_call( - "label_analysis.tests_saved_count", 9 - ) - mock_metrics_context.return_value.log_simple_metric.assert_any_call( - "label_analysis.requests_with_requested_labels", 1.0 - ) - mock_metrics_context.return_value.log_simple_metric.assert_any_call( - "label_analysis.requested_labels_count", 4 - ) - mock_metrics_context.return_value.log_simple_metric.assert_any_call( - "label_analysis.tests_to_run_count", 3 - ) - - -def test_simple_call_with_requested_labels( - dbsession, mock_storage, mocker, sample_report_with_labels, mock_repo_provider -): - mock_metrics = mocker.patch("tasks.label_analysis.metrics") - mock_metrics_context = mocker.patch("tasks.label_analysis.MetricContext") - mocker.patch.object( - LabelAnalysisRequestProcessingTask, - "_get_lines_relevant_to_diff", - return_value={ - "all": False, - "files": {"source.py": {"all": False, "lines": {8, 6}}}, - }, - ) - mocker.patch.object( - ReportService, - "get_existing_report_for_commit", - return_value=sample_report_with_labels, - ) - larf = LabelAnalysisRequestFactory.create( - requested_labels=["tangerine", "pear", "banana", "apple"] - ) - dbsession.add(larf) - dbsession.flush() - task = LabelAnalysisRequestProcessingTask() - res = task.run_impl(dbsession, larf.id) - expected_present_diff_labels = ["banana"] - expected_present_report_labels = ["apple", "banana"] - expected_absent_labels = ["pear", "tangerine"] - assert res == { - "absent_labels": expected_absent_labels, - "present_diff_labels": expected_present_diff_labels, - "present_report_labels": expected_present_report_labels, - "success": True, - "global_level_labels": [], - "errors": [], - } - dbsession.flush() - dbsession.refresh(larf) - assert larf.state_id == LabelAnalysisRequestState.FINISHED.db_id - assert larf.result == { - "absent_labels": expected_absent_labels, - "present_diff_labels": expected_present_diff_labels, - "present_report_labels": expected_present_report_labels, - "global_level_labels": [], - } - mock_metrics.incr.assert_called_with("label_analysis_task.success") - mock_metrics_context.assert_called_with( - repo_id=larf.head_commit.repository.repoid, commit_id=larf.head_commit.id - ) - mock_metrics_context.return_value.log_simple_metric.assert_any_call( - "label_analysis.tests_saved_count", 9 - ) - mock_metrics_context.return_value.log_simple_metric.assert_any_call( - "label_analysis.requests_with_requested_labels", 1.0 - ) - mock_metrics_context.return_value.log_simple_metric.assert_any_call( - "label_analysis.tests_to_run_count", 3 - ) - - -def test_get_requested_labels(dbsession, mocker): - larf = LabelAnalysisRequestFactory.create(requested_labels=[]) - - def side_effect(*args, **kwargs): - larf.requested_labels = ["tangerine", "pear", "banana", "apple"] - - mock_refresh = mocker.patch.object(dbsession, "refresh", side_effect=side_effect) - dbsession.add(larf) - dbsession.flush() - task = LabelAnalysisRequestProcessingTask() - task.dbsession = dbsession - labels = task._get_requested_labels(larf) - mock_refresh.assert_called() - assert labels == ["tangerine", "pear", "banana", "apple"] - - -def test_call_label_analysis_no_request_object(dbsession, mocker): - task = LabelAnalysisRequestProcessingTask() - mock_metrics = mocker.patch("tasks.label_analysis.metrics") - res = task.run_impl(db_session=dbsession, request_id=-1) - assert res == { - "success": False, - "present_report_labels": [], - "present_diff_labels": [], - "absent_labels": [], - "global_level_labels": [], - "errors": [ - { - "error_code": "not found", - "error_params": { - "extra": {}, - "message": "LabelAnalysisRequest not found", - }, - } - ], - } - mock_metrics.incr.assert_called_with( - "label_analysis_task.failed_to_calculate.larq_not_found" - ) - - -def test_get_executable_lines_labels_all_labels(sample_report_with_labels): - executable_lines = {"all": True} - task = LabelAnalysisRequestProcessingTask() - assert task.get_executable_lines_labels( - sample_report_with_labels, executable_lines - ) == ( - { - "banana", - "justjuice", - "here", - "pineapple", - "applejuice", - "apple", - "whatever", - "label_one", - "orangejuice", - }, - set(), - ) - assert task.get_executable_lines_labels( - sample_report_with_labels, executable_lines - ) == (task.get_all_report_labels(sample_report_with_labels), set()) - - -def test_get_executable_lines_labels_all_labels_in_one_file(sample_report_with_labels): - executable_lines = {"all": False, "files": {"source.py": {"all": True}}} - task = LabelAnalysisRequestProcessingTask() - assert task.get_executable_lines_labels( - sample_report_with_labels, executable_lines - ) == ( - { - "apple", - "justjuice", - "applejuice", - "label_one", - "banana", - "orangejuice", - "pineapple", - }, - {"orangejuice", "justjuice", "applejuice"}, - ) - - -def test_get_executable_lines_labels_some_labels_in_one_file(sample_report_with_labels): - executable_lines = { - "all": False, - "files": {"source.py": {"all": False, "lines": set([5, 6])}}, - } - task = LabelAnalysisRequestProcessingTask() - assert task.get_executable_lines_labels( - sample_report_with_labels, executable_lines - ) == ( - {"apple", "label_one", "pineapple", "banana"}, - set(), - ) - - -def test_get_executable_lines_labels_some_labels_in_one_file_with_globals( - sample_report_with_labels, -): - executable_lines = { - "all": False, - "files": {"source.py": {"all": False, "lines": set([6, 8])}}, - } - task = LabelAnalysisRequestProcessingTask() - assert task.get_executable_lines_labels( - sample_report_with_labels, executable_lines - ) == ( - {"label_one", "pineapple", "banana", "orangejuice", "applejuice"}, - {"applejuice", "justjuice", "orangejuice"}, - ) - - -def test_get_executable_lines_labels_some_labels_in_one_file_other_null( - sample_report_with_labels, -): - executable_lines = { - "all": False, - "files": { - "source.py": {"all": False, "lines": set([5, 6])}, - "path/from/randomfile_no_static_analysis.html": None, - }, - } - task = LabelAnalysisRequestProcessingTask() - assert task.get_executable_lines_labels( - sample_report_with_labels, executable_lines - ) == ( - {"apple", "label_one", "pineapple", "banana"}, - set(), - ) - - -def test_get_all_labels_one_session(sample_report_with_labels): - task = LabelAnalysisRequestProcessingTask() - assert task.get_labels_per_session(sample_report_with_labels, 1) == { - "apple", - "banana", - "here", - "label_one", - "pineapple", - "whatever", - } - assert task.get_labels_per_session(sample_report_with_labels, 2) == set() - assert task.get_labels_per_session(sample_report_with_labels, 5) == { - "orangejuice", - "justjuice", - "applejuice", - } - - -def test_get_relevant_executable_lines_nothing_found(dbsession, mocker): - repository = RepositoryFactory.create() - dbsession.add(repository) - dbsession.flush() - larf = LabelAnalysisRequestFactory.create( - base_commit__repository=repository, head_commit__repository=repository - ) - dbsession.add(larf) - dbsession.flush() - task = LabelAnalysisRequestProcessingTask() - task.errors = [] - task.dbsession = dbsession - parsed_git_diff = [] - assert task.get_relevant_executable_lines(larf, parsed_git_diff) is None - - -def test_get_relevant_executable_lines_with_static_analyses(dbsession, mocker): - repository = RepositoryFactory.create() - dbsession.add(repository) - dbsession.flush() - larf = LabelAnalysisRequestFactory.create( - base_commit__repository=repository, head_commit__repository=repository - ) - dbsession.add(larf) - dbsession.flush() - base_sasf = StaticAnalysisSuiteFactory.create(commit=larf.base_commit) - head_sasf = StaticAnalysisSuiteFactory.create(commit=larf.head_commit) - dbsession.add(base_sasf) - dbsession.add(head_sasf) - dbsession.flush() - task = LabelAnalysisRequestProcessingTask() - parsed_git_diff = [] - mocked_res = mocker.patch.object( - StaticAnalysisComparisonService, "get_base_lines_relevant_to_change" - ) - assert ( - task.get_relevant_executable_lines(larf, parsed_git_diff) - == mocked_res.return_value - ) - - -def test_run_impl_with_error( - dbsession, mock_storage, mocker, sample_report_with_labels, mock_repo_provider -): - mock_metrics = mocker.patch("tasks.label_analysis.metrics") - mocker.patch.object( - LabelAnalysisRequestProcessingTask, - "_get_lines_relevant_to_diff", - side_effect=Exception("Oh no"), - ) - larf = LabelAnalysisRequestFactory.create( - requested_labels=["tangerine", "pear", "banana", "apple"] - ) - dbsession.add(larf) - dbsession.flush() - task = LabelAnalysisRequestProcessingTask() - res = task.run_impl(dbsession, larf.id) - expected_result = { - "absent_labels": [], - "present_diff_labels": [], - "present_report_labels": [], - "success": False, - "global_level_labels": [], - "errors": [ - { - "error_code": "failed", - "error_params": {"extra": {}, "message": "Failed to calculate"}, - } - ], - } - assert res == expected_result - dbsession.flush() - dbsession.refresh(larf) - assert larf.state_id == LabelAnalysisRequestState.ERROR.db_id - assert larf.result is None - mock_metrics.incr.assert_called_with( - "label_analysis_task.failed_to_calculate.exception" - ) - - -def test_calculate_result_no_report( - dbsession, mock_storage, mocker, sample_report_with_labels, mock_repo_provider -): - mock_metrics = mocker.patch("tasks.label_analysis.metrics") - larf: LabelAnalysisRequest = LabelAnalysisRequestFactory.create( - # This being not-ordered is important in the test - # TO make sure we go through the warning at the bottom of run_impl - requested_labels=["tangerine", "pear", "banana", "apple"] - ) - dbsession.add(larf) - dbsession.flush() - mocker.patch.object( - ReportService, - "get_existing_report_for_commit", - return_value=None, - ) - mocker.patch.object( - LabelAnalysisRequestProcessingTask, - "_get_lines_relevant_to_diff", - return_value=(set(), set(), set()), - ) - task = LabelAnalysisRequestProcessingTask() - res = task.run_impl(dbsession, larf.id) - assert res == { - "success": True, - "absent_labels": larf.requested_labels, - "present_diff_labels": [], - "present_report_labels": [], - "global_level_labels": [], - "errors": [ - { - "error_code": "missing data", - "error_params": { - "extra": { - "base_commit": larf.base_commit.commitid, - "head_commit": larf.head_commit.commitid, - }, - "message": "Missing base report", - }, - } - ], - } - mock_metrics.incr.assert_called_with( - "label_analysis_task.failed_to_calculate.missing_info" - ) - - -@patch("tasks.label_analysis.parse_git_diff_json", return_value=["parsed_git_diff"]) -def test__get_parsed_git_diff(mock_parse_diff, dbsession, mock_repo_provider): - repository = RepositoryFactory.create() - dbsession.add(repository) - dbsession.flush() - larq = LabelAnalysisRequestFactory.create( - base_commit__repository=repository, head_commit__repository=repository - ) - dbsession.add(larq) - dbsession.flush() - mock_repo_provider.get_compare.return_value = {"diff": "json"} - task = LabelAnalysisRequestProcessingTask() - task.errors = [] - parsed_diff = task._get_parsed_git_diff(larq) - assert parsed_diff == ["parsed_git_diff"] - mock_parse_diff.assert_called_with({"diff": "json"}) - mock_repo_provider.get_compare.assert_called_with( - larq.base_commit.commitid, larq.head_commit.commitid - ) - - -@patch("tasks.label_analysis.parse_git_diff_json", return_value=["parsed_git_diff"]) -def test__get_parsed_git_diff_error(mock_parse_diff, dbsession, mock_repo_provider): - repository = RepositoryFactory.create() - dbsession.add(repository) - dbsession.flush() - larq = LabelAnalysisRequestFactory.create( - base_commit__repository=repository, head_commit__repository=repository - ) - dbsession.add(larq) - dbsession.flush() - mock_repo_provider.get_compare.side_effect = Exception("Oh no") - task = LabelAnalysisRequestProcessingTask() - task.errors = [] - task.dbsession = dbsession - parsed_diff = task._get_parsed_git_diff(larq) - assert parsed_diff is None - mock_parse_diff.assert_not_called() - mock_repo_provider.get_compare.assert_called_with( - larq.base_commit.commitid, larq.head_commit.commitid - ) - - -@patch( - "tasks.label_analysis.LabelAnalysisRequestProcessingTask.get_relevant_executable_lines", - return_value=[{"all": False, "files": {}}], -) -@patch( - "tasks.label_analysis.LabelAnalysisRequestProcessingTask._get_parsed_git_diff", - return_value=["parsed_git_diff"], -) -def test__get_lines_relevant_to_diff( - mock_parse_diff, mock_get_relevant_lines, dbsession -): - repository = RepositoryFactory.create() - dbsession.add(repository) - dbsession.flush() - larq = LabelAnalysisRequestFactory.create( - base_commit__repository=repository, head_commit__repository=repository - ) - dbsession.add(larq) - dbsession.flush() - task = LabelAnalysisRequestProcessingTask() - lines = task._get_lines_relevant_to_diff(larq) - assert lines == [{"all": False, "files": {}}] - mock_parse_diff.assert_called_with(larq) - mock_get_relevant_lines.assert_called_with(larq, ["parsed_git_diff"]) - - -@patch( - "tasks.label_analysis.LabelAnalysisRequestProcessingTask.get_relevant_executable_lines" -) -@patch( - "tasks.label_analysis.LabelAnalysisRequestProcessingTask._get_parsed_git_diff", - return_value=None, -) -def test__get_lines_relevant_to_diff_error( - mock_parse_diff, mock_get_relevant_lines, dbsession -): - repository = RepositoryFactory.create() - dbsession.add(repository) - dbsession.flush() - larq = LabelAnalysisRequestFactory.create( - base_commit__repository=repository, head_commit__repository=repository - ) - dbsession.add(larq) - dbsession.flush() - task = LabelAnalysisRequestProcessingTask() - lines = task._get_lines_relevant_to_diff(larq) - assert lines is None - mock_parse_diff.assert_called_with(larq) - mock_get_relevant_lines.assert_not_called() diff --git a/tasks/tests/unit/test_label_analysis_encoded_labels.py b/tasks/tests/unit/test_label_analysis_encoded_labels.py deleted file mode 100644 index db841bc15..000000000 --- a/tasks/tests/unit/test_label_analysis_encoded_labels.py +++ /dev/null @@ -1,1047 +0,0 @@ -import json - -import pytest -from mock import MagicMock, patch -from shared.reports.resources import LineSession, Report, ReportFile, ReportLine -from shared.reports.types import CoverageDatapoint - -from database.models.labelanalysis import LabelAnalysisRequest -from database.tests.factories import RepositoryFactory -from database.tests.factories.core import ReportFactory -from database.tests.factories.labelanalysis import LabelAnalysisRequestFactory -from database.tests.factories.staticanalysis import ( - StaticAnalysisSingleFileSnapshotFactory, - StaticAnalysisSuiteFactory, - StaticAnalysisSuiteFilepathFactory, -) -from helpers.labels import SpecialLabelsEnum -from services.report import ReportService -from services.static_analysis import StaticAnalysisComparisonService -from tasks.label_analysis import ( - ExistingLabelSetsNotEncoded, - LabelAnalysisRequestProcessingTask, - LabelAnalysisRequestState, -) - -sample_head_static_analysis_dict = { - "empty_lines": [2, 3, 11], - "warnings": [], - "filename": "source.py", - "functions": [ - { - "identifier": "some_function", - "start_line": 6, - "end_line": 10, - "code_hash": "e69c18eff7d24f8bad3370db87f64333", - "complexity_metrics": { - "conditions": 1, - "mccabe_cyclomatic_complexity": 2, - "returns": 1, - "max_nested_conditional": 1, - }, - } - ], - "hash": "84d371ab1c57d2349038ac3671428803", - "language": "python", - "number_lines": 11, - "statements": [ - ( - 1, - { - "line_surety_ancestorship": None, - "start_column": 0, - "line_hash": "55c30cf01e202728b6952e9cba304798", - "len": 0, - "extra_connected_lines": (), - }, - ), - ( - 5, - { - "line_surety_ancestorship": None, - "start_column": 4, - "line_hash": "1d7be9f2145760a59513a4049fcd0d1c", - "len": 0, - "extra_connected_lines": (), - }, - ), - ( - 6, - { - "line_surety_ancestorship": 5, - "start_column": 4, - "line_hash": "f802087a854c26782ee8d4ece7214425", - "len": 0, - "extra_connected_lines": (), - }, - ), - ( - 7, - { - "line_surety_ancestorship": None, - "start_column": 8, - "line_hash": "6ae3393fa7880fe8a844c03256cac37b", - "len": 0, - "extra_connected_lines": (), - }, - ), - ( - 8, - { - "line_surety_ancestorship": 6, - "start_column": 4, - "line_hash": "5b099d1822e9236c540a5701a657225e", - "len": 0, - "extra_connected_lines": (), - }, - ), - ( - 9, - { - "line_surety_ancestorship": 8, - "start_column": 4, - "line_hash": "e5d4915bb7dddeb18f53dc9fde9a3064", - "len": 0, - "extra_connected_lines": (), - }, - ), - ( - 10, - { - "line_surety_ancestorship": 9, - "start_column": 4, - "line_hash": "e70ce43136171575ee525375b10f91a1", - "len": 0, - "extra_connected_lines": (), - }, - ), - ], - "definition_lines": [(4, 6)], - "import_lines": [], -} - -sample_base_static_analysis_dict = { - "empty_lines": [2, 3, 11], - "warnings": [], - "filename": "source.py", - "functions": [ - { - "identifier": "some_function", - "start_line": 6, - "end_line": 10, - "code_hash": "e4b52b6da12184142fcd7ff2c8412662", - "complexity_metrics": { - "conditions": 1, - "mccabe_cyclomatic_complexity": 2, - "returns": 1, - "max_nested_conditional": 1, - }, - } - ], - "hash": "811d0016249a5b1400a685164e5295de", - "language": "python", - "number_lines": 11, - "statements": [ - ( - 1, - { - "line_surety_ancestorship": None, - "start_column": 0, - "line_hash": "55c30cf01e202728b6952e9cba304798", - "len": 0, - "extra_connected_lines": (), - }, - ), - ( - 5, - { - "line_surety_ancestorship": None, - "start_column": 4, - "line_hash": "1d7be9f2145760a59513a4049fcd0d1c", - "len": 0, - "extra_connected_lines": (), - }, - ), - ( - 6, - { - "line_surety_ancestorship": 5, - "start_column": 4, - "line_hash": "52f98812dca4687f18373b87433df695", - "len": 0, - "extra_connected_lines": (), - }, - ), - ( - 7, - { - "line_surety_ancestorship": None, - "start_column": 8, - "line_hash": "6ae3393fa7880fe8a844c03256cac37b", - "len": 0, - "extra_connected_lines": (), - }, - ), - ( - 8, - { - "line_surety_ancestorship": 7, - "start_column": 8, - "line_hash": "5b099d1822e9236c540a5701a657225e", - "len": 0, - "extra_connected_lines": (), - }, - ), - ( - 9, - { - "line_surety_ancestorship": 6, - "start_column": 4, - "line_hash": "e5d4915bb7dddeb18f53dc9fde9a3064", - "len": 0, - "extra_connected_lines": (), - }, - ), - ( - 10, - { - "line_surety_ancestorship": 9, - "start_column": 4, - "line_hash": "e70ce43136171575ee525375b10f91a1", - "len": 0, - "extra_connected_lines": (), - }, - ), - ], - "definition_lines": [(4, 6)], - "import_lines": [], -} - - -@pytest.fixture -def sample_report_with_labels(): - r = Report() - report_labels_index = { - 0: SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label, - 1: "apple", - 2: "label_one", - 3: "pineapple", - 4: "banana", - 5: "orangejuice", - 6: "justjuice", - 7: "whatever", - 8: "here", - 9: "applejuice", - } - first_rf = ReportFile("source.py") - first_rf.append( - 5, - ReportLine.create( - coverage=1, - type=None, - sessions=[ - ( - LineSession( - id=1, - coverage=1, - ) - ) - ], - datapoints=[ - CoverageDatapoint( - sessionid=1, - coverage=1, - coverage_type=None, - label_ids=[1, 2, 3, 4], - ) - ], - complexity=None, - ), - ) - first_rf.append( - 6, - ReportLine.create( - coverage=1, - type=None, - sessions=[ - ( - LineSession( - id=1, - coverage=1, - ) - ) - ], - datapoints=[ - CoverageDatapoint( - sessionid=1, - coverage=1, - coverage_type=None, - label_ids=[2, 3, 4], - ) - ], - complexity=None, - ), - ) - first_rf.append( - 7, - ReportLine.create( - coverage=1, - type=None, - sessions=[ - ( - LineSession( - id=1, - coverage=1, - ) - ) - ], - datapoints=[ - CoverageDatapoint( - sessionid=1, - coverage=1, - coverage_type=None, - label_ids=[4], - ) - ], - complexity=None, - ), - ) - first_rf.append( - 8, - ReportLine.create( - coverage=1, - type=None, - sessions=[ - ( - LineSession( - id=1, - coverage=1, - ) - ) - ], - datapoints=[ - CoverageDatapoint( - sessionid=1, - coverage=1, - coverage_type=None, - label_ids=[4], - ), - CoverageDatapoint( - sessionid=5, - coverage=1, - coverage_type=None, - label_ids=[5], - ), - ], - complexity=None, - ), - ) - first_rf.append( - 99, - ReportLine.create( - coverage=1, - type=None, - sessions=[ - ( - LineSession( - id=5, - coverage=1, - ) - ) - ], - datapoints=[ - CoverageDatapoint( - sessionid=5, - coverage=1, - coverage_type=None, - label_ids=[6], - ), - ], - complexity=None, - ), - ) - first_rf.append( - 8, - ReportLine.create( - coverage=1, - type=None, - sessions=[ - ( - LineSession( - id=1, - coverage=1, - ) - ) - ], - datapoints=[ - CoverageDatapoint( - sessionid=1, - coverage=1, - coverage_type=None, - label_ids=[2, 3, 4], - ), - CoverageDatapoint( - sessionid=5, - coverage=1, - coverage_type=None, - label_ids=[0, 9], - ), - ], - complexity=None, - ), - ) - second_rf = ReportFile("path/from/additionsonly.py") - second_rf.append( - 6, - ReportLine.create( - coverage=1, - type=None, - sessions=[ - ( - LineSession( - id=1, - coverage=1, - ) - ) - ], - datapoints=[ - CoverageDatapoint( - sessionid=1, - coverage=1, - coverage_type=None, - label_ids=[7, 8], - ) - ], - complexity=None, - ), - ) - random_rf = ReportFile("path/from/randomfile_no_static_analysis.html") - random_rf.append( - 1, - ReportLine.create( - coverage=1, - type=None, - sessions=[(LineSession(id=1, coverage=1))], - datapoints=None, - complexity=None, - ), - ) - r.append(first_rf) - r.append(second_rf) - r.append(random_rf) - r.labels_index = report_labels_index - return r - - -def test_simple_call_without_requested_labels_then_with_requested_labels( - dbsession, mock_storage, mocker, sample_report_with_labels, mock_repo_provider -): - mock_metrics = mocker.patch("tasks.label_analysis.metrics") - mock_metrics_context = mocker.patch("tasks.label_analysis.MetricContext") - mocker.patch.object( - LabelAnalysisRequestProcessingTask, - "_get_lines_relevant_to_diff", - return_value={ - "all": False, - "files": {"source.py": {"all": False, "lines": {8, 6}}}, - }, - ) - mocker.patch.object( - ReportService, - "get_existing_report_for_commit", - return_value=sample_report_with_labels, - ) - repository = RepositoryFactory.create() - larf = LabelAnalysisRequestFactory.create( - base_commit__repository=repository, head_commit__repository=repository - ) - dbsession.add(larf) - dbsession.flush() - base_sasf = StaticAnalysisSuiteFactory.create(commit=larf.base_commit) - head_sasf = StaticAnalysisSuiteFactory.create(commit=larf.head_commit) - dbsession.add(base_sasf) - dbsession.add(head_sasf) - dbsession.flush() - first_path = "abdkasdauchudh.txt" - second_path = "0diao9u3qdsdu.txt" - mock_storage.write_file( - "archive", - first_path, - json.dumps(sample_base_static_analysis_dict), - ) - mock_storage.write_file( - "archive", - second_path, - json.dumps(sample_head_static_analysis_dict), - ) - first_snapshot = StaticAnalysisSingleFileSnapshotFactory.create( - repository=repository, content_location=first_path - ) - second_snapshot = StaticAnalysisSingleFileSnapshotFactory.create( - repository=repository, content_location=second_path - ) - dbsession.add(first_snapshot) - dbsession.add(second_snapshot) - dbsession.flush() - first_base_file = StaticAnalysisSuiteFilepathFactory.create( - file_snapshot=first_snapshot, - analysis_suite=base_sasf, - filepath="source.py", - ) - first_head_file = StaticAnalysisSuiteFilepathFactory.create( - file_snapshot=second_snapshot, - analysis_suite=head_sasf, - filepath="source.py", - ) - dbsession.add(first_base_file) - dbsession.add(first_head_file) - dbsession.flush() - - task = LabelAnalysisRequestProcessingTask() - assert sample_report_with_labels.labels_index is not None - res = task.run_impl(dbsession, larf.id) - expected_present_report_labels = [ - "apple", - "applejuice", - "banana", - "here", - "justjuice", - "label_one", - "orangejuice", - "pineapple", - "whatever", - ] - expected_present_diff_labels = sorted( - ["applejuice", "banana", "label_one", "orangejuice", "pineapple"] - ) - expected_result = { - "absent_labels": [], - "present_diff_labels": expected_present_diff_labels, - "present_report_labels": expected_present_report_labels, - "global_level_labels": ["applejuice", "justjuice", "orangejuice"], - "success": True, - "errors": [], - } - assert res == expected_result - mock_metrics.incr.assert_called_with("label_analysis_task.success") - mock_metrics_context.assert_called_with( - repo_id=repository.repoid, commit_id=larf.head_commit.id - ) - mock_metrics_context.return_value.log_simple_metric.assert_any_call( - "label_analysis.tests_saved_count", 9 - ) - mock_metrics_context.return_value.log_simple_metric.assert_any_call( - "label_analysis.requests_with_requested_labels", 0.0 - ) - mock_metrics_context.return_value.log_simple_metric.assert_any_call( - "label_analysis.tests_to_run_count", 6 - ) - # It's zero because the report has the _labels_index already - dbsession.flush() - dbsession.refresh(larf) - assert larf.state_id == LabelAnalysisRequestState.FINISHED.db_id - assert larf.result == { - "absent_labels": [], - "present_diff_labels": expected_present_diff_labels, - "present_report_labels": expected_present_report_labels, - "global_level_labels": ["applejuice", "justjuice", "orangejuice"], - } - # Now we call the task again, this time with the requested labels. - # This illustrates what should happen if we patch the labels after calculating - # And trigger the task again to save the new results - larf.requested_labels = ["tangerine", "pear", "banana", "apple"] - dbsession.flush() - res = task.run_impl(dbsession, larf.id) - expected_present_diff_labels = ["banana"] - expected_present_report_labels = ["apple", "banana"] - expected_absent_labels = ["pear", "tangerine"] - assert res == { - "absent_labels": expected_absent_labels, - "present_diff_labels": expected_present_diff_labels, - "present_report_labels": expected_present_report_labels, - "success": True, - "global_level_labels": [], - "errors": [], - } - assert larf.result == { - "absent_labels": expected_absent_labels, - "present_diff_labels": expected_present_diff_labels, - "present_report_labels": expected_present_report_labels, - "global_level_labels": [], - } - mock_metrics.incr.assert_called_with( - "label_analysis_task.already_calculated.new_result" - ) - mock_metrics.incr.assert_called_with( - "label_analysis_task.already_calculated.new_result" - ) - mock_metrics_context.return_value.log_simple_metric.assert_any_call( - "label_analysis.tests_saved_count", 9 - ) - mock_metrics_context.return_value.log_simple_metric.assert_any_call( - "label_analysis.requests_with_requested_labels", 1.0 - ) - mock_metrics_context.return_value.log_simple_metric.assert_any_call( - "label_analysis.requested_labels_count", 4 - ) - mock_metrics_context.return_value.log_simple_metric.assert_any_call( - "label_analysis.tests_to_run_count", 3 - ) - - -def test_simple_call_with_requested_labels( - dbsession, mock_storage, mocker, sample_report_with_labels, mock_repo_provider -): - mock_metrics = mocker.patch("tasks.label_analysis.metrics") - mock_metrics_context = mocker.patch("tasks.label_analysis.MetricContext") - mocker.patch.object( - LabelAnalysisRequestProcessingTask, - "_get_lines_relevant_to_diff", - return_value={ - "all": False, - "files": {"source.py": {"all": False, "lines": {8, 6}}}, - }, - ) - mocker.patch.object( - ReportService, - "get_existing_report_for_commit", - return_value=sample_report_with_labels, - ) - larf = LabelAnalysisRequestFactory.create( - requested_labels=["tangerine", "pear", "banana", "apple"] - ) - ReportFactory(commit=larf.base_commit) - dbsession.add(larf) - dbsession.flush() - task = LabelAnalysisRequestProcessingTask() - res = task.run_impl(dbsession, larf.id) - expected_present_diff_labels = ["banana"] - expected_present_report_labels = ["apple", "banana"] - expected_absent_labels = ["pear", "tangerine"] - assert res == { - "absent_labels": expected_absent_labels, - "present_diff_labels": expected_present_diff_labels, - "present_report_labels": expected_present_report_labels, - "success": True, - "global_level_labels": [], - "errors": [], - } - dbsession.flush() - dbsession.refresh(larf) - assert larf.state_id == LabelAnalysisRequestState.FINISHED.db_id - assert larf.result == { - "absent_labels": expected_absent_labels, - "present_diff_labels": expected_present_diff_labels, - "present_report_labels": expected_present_report_labels, - "global_level_labels": [], - } - mock_metrics.incr.assert_called_with("label_analysis_task.success") - mock_metrics.incr.assert_called_with("label_analysis_task.success") - mock_metrics_context.assert_called_with( - repo_id=larf.head_commit.repository.repoid, commit_id=larf.head_commit.id - ) - mock_metrics_context.return_value.log_simple_metric.assert_any_call( - "label_analysis.tests_saved_count", 9 - ) - mock_metrics_context.return_value.log_simple_metric.assert_any_call( - "label_analysis.requests_with_requested_labels", 1.0 - ) - mock_metrics_context.return_value.log_simple_metric.assert_any_call( - "label_analysis.tests_to_run_count", 3 - ) - - -def test_get_requested_labels(dbsession, mocker): - larf = LabelAnalysisRequestFactory.create(requested_labels=[]) - - def side_effect(*args, **kwargs): - larf.requested_labels = ["tangerine", "pear", "banana", "apple"] - - mock_refresh = mocker.patch.object(dbsession, "refresh", side_effect=side_effect) - dbsession.add(larf) - dbsession.flush() - task = LabelAnalysisRequestProcessingTask() - task.dbsession = dbsession - labels = task._get_requested_labels(larf) - mock_refresh.assert_called() - assert labels == ["tangerine", "pear", "banana", "apple"] - - -def test_call_label_analysis_no_request_object(dbsession, mocker): - task = LabelAnalysisRequestProcessingTask() - mock_metrics = mocker.patch("tasks.label_analysis.metrics") - res = task.run_impl(db_session=dbsession, request_id=-1) - assert res == { - "success": False, - "present_report_labels": [], - "present_diff_labels": [], - "absent_labels": [], - "global_level_labels": [], - "errors": [ - { - "error_code": "not found", - "error_params": { - "extra": {}, - "message": "LabelAnalysisRequest not found", - }, - } - ], - } - mock_metrics.incr.assert_called_with( - "label_analysis_task.failed_to_calculate.larq_not_found" - ) - - -def test_get_executable_lines_labels_all_labels(sample_report_with_labels): - executable_lines = {"all": True} - task = LabelAnalysisRequestProcessingTask() - assert task.get_executable_lines_labels( - sample_report_with_labels, executable_lines - ) == ( - { - 4, - 6, - 8, - 3, - 9, - 1, - 7, - 2, - 5, - }, - set(), - ) - assert task.get_executable_lines_labels( - sample_report_with_labels, executable_lines - ) == (task.get_all_report_labels(sample_report_with_labels), set()) - - -def test_get_executable_lines_labels_all_labels_in_one_file(sample_report_with_labels): - executable_lines = {"all": False, "files": {"source.py": {"all": True}}} - task = LabelAnalysisRequestProcessingTask() - assert task.get_executable_lines_labels( - sample_report_with_labels, executable_lines - ) == ( - { - 1, - 6, - 9, - 2, - 4, - 5, - 3, - }, - {5, 6, 9}, - ) - - -def test_get_executable_lines_labels_some_labels_in_one_file(sample_report_with_labels): - executable_lines = { - "all": False, - "files": {"source.py": {"all": False, "lines": set([5, 6])}}, - } - task = LabelAnalysisRequestProcessingTask() - assert task.get_executable_lines_labels( - sample_report_with_labels, executable_lines - ) == ( - {1, 2, 3, 4}, - set(), - ) - - -def test_get_executable_lines_labels_some_labels_in_one_file_with_globals( - sample_report_with_labels, -): - executable_lines = { - "all": False, - "files": {"source.py": {"all": False, "lines": set([6, 8])}}, - } - task = LabelAnalysisRequestProcessingTask() - assert task.get_executable_lines_labels( - sample_report_with_labels, executable_lines - ) == ( - {2, 3, 4, 5, 9}, - {9, 6, 5}, - ) - - -def test_get_executable_lines_labels_some_labels_in_one_file_other_null( - sample_report_with_labels, -): - executable_lines = { - "all": False, - "files": { - "source.py": {"all": False, "lines": set([5, 6])}, - "path/from/randomfile_no_static_analysis.html": None, - }, - } - task = LabelAnalysisRequestProcessingTask() - assert task.get_executable_lines_labels( - sample_report_with_labels, executable_lines - ) == ( - {1, 2, 3, 4}, - set(), - ) - - -def test_get_all_labels_one_session(sample_report_with_labels): - task = LabelAnalysisRequestProcessingTask() - assert task.get_labels_per_session(sample_report_with_labels, 1) == { - 1, - 4, - 8, - 2, - 3, - 7, - } - assert task.get_labels_per_session(sample_report_with_labels, 2) == set() - assert task.get_labels_per_session(sample_report_with_labels, 5) == { - 5, - 6, - 9, - } - - -def test_get_relevant_executable_lines_nothing_found(dbsession, mocker): - repository = RepositoryFactory.create() - dbsession.add(repository) - dbsession.flush() - larf = LabelAnalysisRequestFactory.create( - base_commit__repository=repository, head_commit__repository=repository - ) - dbsession.add(larf) - dbsession.flush() - task = LabelAnalysisRequestProcessingTask() - task.errors = [] - task.dbsession = dbsession - parsed_git_diff = [] - assert task.get_relevant_executable_lines(larf, parsed_git_diff) is None - - -def test_get_relevant_executable_lines_with_static_analyses(dbsession, mocker): - repository = RepositoryFactory.create() - dbsession.add(repository) - dbsession.flush() - larf = LabelAnalysisRequestFactory.create( - base_commit__repository=repository, head_commit__repository=repository - ) - dbsession.add(larf) - dbsession.flush() - base_sasf = StaticAnalysisSuiteFactory.create(commit=larf.base_commit) - head_sasf = StaticAnalysisSuiteFactory.create(commit=larf.head_commit) - dbsession.add(base_sasf) - dbsession.add(head_sasf) - dbsession.flush() - task = LabelAnalysisRequestProcessingTask() - parsed_git_diff = [] - mocked_res = mocker.patch.object( - StaticAnalysisComparisonService, "get_base_lines_relevant_to_change" - ) - assert ( - task.get_relevant_executable_lines(larf, parsed_git_diff) - == mocked_res.return_value - ) - - -def test_run_impl_with_error( - dbsession, mock_storage, mocker, sample_report_with_labels, mock_repo_provider -): - mock_metrics = mocker.patch("tasks.label_analysis.metrics") - mocker.patch.object( - LabelAnalysisRequestProcessingTask, - "_get_lines_relevant_to_diff", - side_effect=Exception("Oh no"), - ) - larf = LabelAnalysisRequestFactory.create( - requested_labels=["tangerine", "pear", "banana", "apple"] - ) - dbsession.add(larf) - dbsession.flush() - task = LabelAnalysisRequestProcessingTask() - res = task.run_impl(dbsession, larf.id) - expected_result = { - "absent_labels": [], - "present_diff_labels": [], - "present_report_labels": [], - "success": False, - "global_level_labels": [], - "errors": [ - { - "error_code": "failed", - "error_params": {"extra": {}, "message": "Failed to calculate"}, - } - ], - } - assert res == expected_result - dbsession.flush() - dbsession.refresh(larf) - assert larf.state_id == LabelAnalysisRequestState.ERROR.db_id - assert larf.result is None - mock_metrics.incr.assert_called_with( - "label_analysis_task.failed_to_calculate.exception" - ) - - -def test_calculate_result_no_report( - dbsession, mock_storage, mocker, sample_report_with_labels, mock_repo_provider -): - mock_metrics = mocker.patch("tasks.label_analysis.metrics") - larf: LabelAnalysisRequest = LabelAnalysisRequestFactory.create( - # This being not-ordered is important in the test - # TO make sure we go through the warning at the bottom of run_impl - requested_labels=["tangerine", "pear", "banana", "apple"] - ) - dbsession.add(larf) - dbsession.flush() - mocker.patch.object( - ReportService, - "get_existing_report_for_commit", - return_value=None, - ) - mocker.patch.object( - LabelAnalysisRequestProcessingTask, - "_get_lines_relevant_to_diff", - return_value=(set(), set(), set()), - ) - task = LabelAnalysisRequestProcessingTask() - res = task.run_impl(dbsession, larf.id) - assert res == { - "success": True, - "absent_labels": larf.requested_labels, - "present_diff_labels": [], - "present_report_labels": [], - "global_level_labels": [], - "errors": [ - { - "error_code": "missing data", - "error_params": { - "extra": { - "base_commit": larf.base_commit.commitid, - "head_commit": larf.head_commit.commitid, - }, - "message": "Missing base report", - }, - } - ], - } - mock_metrics.incr.assert_called_with( - "label_analysis_task.failed_to_calculate.missing_info" - ) - - -@patch("tasks.label_analysis.parse_git_diff_json", return_value=["parsed_git_diff"]) -def test__get_parsed_git_diff(mock_parse_diff, dbsession, mock_repo_provider): - repository = RepositoryFactory.create() - dbsession.add(repository) - dbsession.flush() - larq = LabelAnalysisRequestFactory.create( - base_commit__repository=repository, head_commit__repository=repository - ) - dbsession.add(larq) - dbsession.flush() - mock_repo_provider.get_compare.return_value = {"diff": "json"} - task = LabelAnalysisRequestProcessingTask() - task.errors = [] - parsed_diff = task._get_parsed_git_diff(larq) - assert parsed_diff == ["parsed_git_diff"] - mock_parse_diff.assert_called_with({"diff": "json"}) - mock_repo_provider.get_compare.assert_called_with( - larq.base_commit.commitid, larq.head_commit.commitid - ) - - -@patch("tasks.label_analysis.parse_git_diff_json", return_value=["parsed_git_diff"]) -def test__get_parsed_git_diff_error(mock_parse_diff, dbsession, mock_repo_provider): - repository = RepositoryFactory.create() - dbsession.add(repository) - dbsession.flush() - larq = LabelAnalysisRequestFactory.create( - base_commit__repository=repository, head_commit__repository=repository - ) - dbsession.add(larq) - dbsession.flush() - mock_repo_provider.get_compare.side_effect = Exception("Oh no") - task = LabelAnalysisRequestProcessingTask() - task.errors = [] - task.dbsession = dbsession - parsed_diff = task._get_parsed_git_diff(larq) - assert parsed_diff is None - mock_parse_diff.assert_not_called() - mock_repo_provider.get_compare.assert_called_with( - larq.base_commit.commitid, larq.head_commit.commitid - ) - - -@patch( - "tasks.label_analysis.LabelAnalysisRequestProcessingTask.get_relevant_executable_lines", - return_value=[{"all": False, "files": {}}], -) -@patch( - "tasks.label_analysis.LabelAnalysisRequestProcessingTask._get_parsed_git_diff", - return_value=["parsed_git_diff"], -) -def test__get_lines_relevant_to_diff( - mock_parse_diff, mock_get_relevant_lines, dbsession -): - repository = RepositoryFactory.create() - dbsession.add(repository) - dbsession.flush() - larq = LabelAnalysisRequestFactory.create( - base_commit__repository=repository, head_commit__repository=repository - ) - dbsession.add(larq) - dbsession.flush() - task = LabelAnalysisRequestProcessingTask() - lines = task._get_lines_relevant_to_diff(larq) - assert lines == [{"all": False, "files": {}}] - mock_parse_diff.assert_called_with(larq) - mock_get_relevant_lines.assert_called_with(larq, ["parsed_git_diff"]) - - -@patch( - "tasks.label_analysis.LabelAnalysisRequestProcessingTask.get_relevant_executable_lines" -) -@patch( - "tasks.label_analysis.LabelAnalysisRequestProcessingTask._get_parsed_git_diff", - return_value=None, -) -def test__get_lines_relevant_to_diff_error( - mock_parse_diff, mock_get_relevant_lines, dbsession -): - repository = RepositoryFactory.create() - dbsession.add(repository) - dbsession.flush() - larq = LabelAnalysisRequestFactory.create( - base_commit__repository=repository, head_commit__repository=repository - ) - dbsession.add(larq) - dbsession.flush() - task = LabelAnalysisRequestProcessingTask() - lines = task._get_lines_relevant_to_diff(larq) - assert lines is None - mock_parse_diff.assert_called_with(larq) - mock_get_relevant_lines.assert_not_called() - - -@patch( - "tasks.label_analysis.LabelAnalysisRequestProcessingTask.get_all_report_labels", - return_value=set(), -) -@patch( - "tasks.label_analysis.LabelAnalysisRequestProcessingTask.get_executable_lines_labels", - return_value=(set(), set()), -) -def test___get_existing_labels_no_labels_in_report( - mock_get_executable_lines_labels, mock_get_all_report_labels -): - report = MagicMock(name="fake_report") - lines_relevant = MagicMock(name="fake_lines_relevant_to_diff") - task = LabelAnalysisRequestProcessingTask() - res = task._get_existing_labels(report, lines_relevant) - expected = ExistingLabelSetsNotEncoded( - all_report_labels=set(), - executable_lines_labels=set(), - global_level_labels=set(), - ) - assert isinstance(res, ExistingLabelSetsNotEncoded) - assert res == expected diff --git a/tasks/upload_finisher.py b/tasks/upload_finisher.py index 18cd0f6e0..56439b069 100644 --- a/tasks/upload_finisher.py +++ b/tasks/upload_finisher.py @@ -588,7 +588,7 @@ def merge_report(cumulative_report: Report, obj): session_adjustment = SessionAdjustmentResult([], []) if flags := session.flags: session_adjustment = clear_carryforward_sessions( - cumulative_report, incremental_report, flags, UserYaml(commit_yaml) + cumulative_report, flags, UserYaml(commit_yaml) ) cumulative_report.merge(incremental_report) @@ -717,9 +717,6 @@ def change_sessionid(report: Report, old_id: int, new_id: int): session.id = new_id all_sessions.add(session.id) - if line.datapoints: - for point in line.datapoints: - if point.sessionid == old_id: - point.sessionid = new_id + line.datapoints = None report_file._details["present_sessions"] = all_sessions From f2b8e075e3439e2d58e0fc86ab53b62f51317dd9 Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Mon, 14 Oct 2024 13:54:56 +0200 Subject: [PATCH 4/5] unregister removed tasks --- tasks/__init__.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tasks/__init__.py b/tasks/__init__.py index 387c3e121..08a047155 100644 --- a/tasks/__init__.py +++ b/tasks/__init__.py @@ -26,7 +26,6 @@ from tasks.health_check import health_check_task from tasks.hourly_check import hourly_check_task from tasks.http_request import http_request_task -from tasks.label_analysis import label_analysis_task from tasks.manual_trigger import manual_trigger_task from tasks.new_user_activated import new_user_activated_task from tasks.notify import notify_task @@ -58,6 +57,5 @@ from tasks.trial_expiration_cron import trial_expiration_cron_task from tasks.update_branches import update_branches_task_name from tasks.upload import upload_task -from tasks.upload_clean_labels_index import clean_labels_index_task from tasks.upload_finisher import upload_finisher_task from tasks.upload_processor import upload_processor_task From 78671e3d7b883b762207649f5583b2bfb6f5ad8a Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Mon, 14 Oct 2024 14:26:47 +0200 Subject: [PATCH 5/5] fix tests and more code removal related to partial sessions --- services/report/__init__.py | 16 +- .../languages/tests/unit/test_pycoverage.py | 42 -- services/report/raw_upload_processor.py | 25 +- services/report/tests/unit/test_process.py | 3 +- services/report/tests/unit/test_sessions.py | 542 +----------------- services/tests/test_report.py | 24 +- tasks/tests/unit/test_upload_finisher_task.py | 138 ----- .../tests/unit/test_upload_processing_task.py | 13 +- tasks/upload_finisher.py | 14 +- tasks/upload_processor.py | 2 +- 10 files changed, 49 insertions(+), 770 deletions(-) diff --git a/services/report/__init__.py b/services/report/__init__.py index 4d9d469bc..d21c34905 100644 --- a/services/report/__init__.py +++ b/services/report/__init__.py @@ -50,10 +50,7 @@ RAW_UPLOAD_RAW_REPORT_COUNT, RAW_UPLOAD_SIZE, ) -from services.report.raw_upload_processor import ( - SessionAdjustmentResult, - process_raw_upload, -) +from services.report.raw_upload_processor import process_raw_upload from services.repository import get_repo_provider_service from services.yaml.reader import get_paths_from_flags, read_yaml_field @@ -73,7 +70,7 @@ class ProcessingResult: session: Session report: Report | None = None error: ProcessingError | None = None - session_adjustment: SessionAdjustmentResult | None = None + deleted_sessions: set[int] | None = None @dataclass @@ -717,7 +714,7 @@ def build_report_from_raw_content( upload=upload, ) result.report = process_result.report - result.session_adjustment = process_result.session_adjustment + result.deleted_sessions = process_result.deleted_sessions log.info( "Successfully processed report", @@ -812,11 +809,10 @@ def update_upload_with_processing_result( # delete all the carryforwarded `Upload` records corresponding to `Session`s # which have been removed from the report. # we always have a `session_adjustment` in the non-error case. - assert processing_result.session_adjustment - deleted_sessions = ( - processing_result.session_adjustment.fully_deleted_sessions + assert processing_result.deleted_sessions is not None + delete_uploads_by_sessionid( + upload, list(processing_result.deleted_sessions) ) - delete_uploads_by_sessionid(upload, deleted_sessions) else: error = processing_result.error diff --git a/services/report/languages/tests/unit/test_pycoverage.py b/services/report/languages/tests/unit/test_pycoverage.py index 7455c7d25..ed1546b73 100644 --- a/services/report/languages/tests/unit/test_pycoverage.py +++ b/services/report/languages/tests/unit/test_pycoverage.py @@ -216,7 +216,6 @@ def test_process_pycoverage(self): [[0, 1, None, None, None]], None, None, - [(0, 1, None, ["Th2dMtk4M_codecov"])], ) assert processed_report == { "archive": { @@ -228,7 +227,6 @@ def test_process_pycoverage(self): [[0, 1, None, None, None]], None, None, - [(0, 1, None, ["Th2dMtk4M_codecov"])], ), ( 2, @@ -237,10 +235,6 @@ def test_process_pycoverage(self): [[0, 1, None, None, None]], None, None, - [ - (0, 1, None, ["test_another.py::test_fib_simple_case"]), - (0, 1, None, ["test_another.py::test_fib_bigger_cases"]), - ], ), ( 3, @@ -249,10 +243,6 @@ def test_process_pycoverage(self): [[0, 1, None, None, None]], None, None, - [ - (0, 1, None, ["test_another.py::test_fib_simple_case"]), - (0, 1, None, ["test_another.py::test_fib_bigger_cases"]), - ], ), ( 4, @@ -261,7 +251,6 @@ def test_process_pycoverage(self): [[0, 1, None, None, None]], None, None, - [(0, 1, None, ["test_another.py::test_fib_bigger_cases"])], ), ], "source.py": [ @@ -272,7 +261,6 @@ def test_process_pycoverage(self): [[0, 1, None, None, None]], None, None, - [(0, 1, None, ["Th2dMtk4M_codecov"])], ), ( 3, @@ -281,7 +269,6 @@ def test_process_pycoverage(self): [[0, 1, None, None, None]], None, None, - [(0, 1, None, ["Th2dMtk4M_codecov"])], ), ( 4, @@ -290,7 +277,6 @@ def test_process_pycoverage(self): [[0, 1, None, None, None]], None, None, - [(0, 1, None, ["test_source.py::test_some_code"])], ), ( 5, @@ -299,7 +285,6 @@ def test_process_pycoverage(self): [[0, 1, None, None, None]], None, None, - [(0, 1, None, ["test_source.py::test_some_code"])], ), ( 6, @@ -308,7 +293,6 @@ def test_process_pycoverage(self): [[0, 0, None, None, None]], None, None, - [], ), ( 9, @@ -317,7 +301,6 @@ def test_process_pycoverage(self): [[0, 1, None, None, None]], None, None, - [(0, 1, None, ["Th2dMtk4M_codecov"])], ), ( 10, @@ -326,7 +309,6 @@ def test_process_pycoverage(self): [[0, 0, None, None, None]], None, None, - [], ), ], "test_another.py": [ @@ -337,7 +319,6 @@ def test_process_pycoverage(self): [[0, 1, None, None, None]], None, None, - [(0, 1, None, ["Th2dMtk4M_codecov"])], ), ( 3, @@ -346,7 +327,6 @@ def test_process_pycoverage(self): [[0, 1, None, None, None]], None, None, - [(0, 1, None, ["Th2dMtk4M_codecov"])], ), ( 4, @@ -355,7 +335,6 @@ def test_process_pycoverage(self): [[0, 1, None, None, None]], None, None, - [(0, 1, None, ["test_another.py::test_fib_simple_case"])], ), ( 5, @@ -364,7 +343,6 @@ def test_process_pycoverage(self): [[0, 1, None, None, None]], None, None, - [(0, 1, None, ["test_another.py::test_fib_simple_case"])], ), ( 7, @@ -373,7 +351,6 @@ def test_process_pycoverage(self): [[0, 1, None, None, None]], None, None, - [(0, 1, None, ["Th2dMtk4M_codecov"])], ), ( 8, @@ -382,7 +359,6 @@ def test_process_pycoverage(self): [[0, 1, None, None, None]], None, None, - [(0, 1, None, ["test_another.py::test_fib_bigger_cases"])], ), ], "test_source.py": [ @@ -393,7 +369,6 @@ def test_process_pycoverage(self): [[0, 1, None, None, None]], None, None, - [(0, 1, None, ["Th2dMtk4M_codecov"])], ), ( 4, @@ -402,7 +377,6 @@ def test_process_pycoverage(self): [[0, 1, None, None, None]], None, None, - [(0, 1, None, ["Th2dMtk4M_codecov"])], ), ( 5, @@ -411,7 +385,6 @@ def test_process_pycoverage(self): [[0, 1, None, None, None]], None, None, - [(0, 1, None, ["test_source.py::test_some_code"])], ), ], }, @@ -488,7 +461,6 @@ def test_process_compressed_report(self): [[0, 1, None, None, None]], None, None, - [(0, 1, None, ["Th2dMtk4M_codecov"])], ), ( 2, @@ -497,10 +469,6 @@ def test_process_compressed_report(self): [[0, 1, None, None, None]], None, None, - [ - (0, 1, None, ["label_1"]), - (0, 1, None, ["label_2"]), - ], ), ( 3, @@ -509,10 +477,6 @@ def test_process_compressed_report(self): [[0, 1, None, None, None]], None, None, - [ - (0, 1, None, ["label_2"]), - (0, 1, None, ["label_3"]), - ], ), ( 4, @@ -521,7 +485,6 @@ def test_process_compressed_report(self): [[0, 0, None, None, None]], None, None, - [], ), ( 5, @@ -530,7 +493,6 @@ def test_process_compressed_report(self): [[0, 1, None, None, None]], None, None, - [(0, 1, None, ["label_5"])], ), ], "__init__.py": [ @@ -541,7 +503,6 @@ def test_process_compressed_report(self): [[0, 0, None, None, None]], None, None, - [], ), ( 3, @@ -550,7 +511,6 @@ def test_process_compressed_report(self): [[0, 0, None, None, None]], None, None, - [], ), ( 4, @@ -559,7 +519,6 @@ def test_process_compressed_report(self): [[0, 0, None, None, None]], None, None, - [], ), ( 5, @@ -568,7 +527,6 @@ def test_process_compressed_report(self): [[0, 0, None, None, None]], None, None, - [], ), ], }, diff --git a/services/report/raw_upload_processor.py b/services/report/raw_upload_processor.py index 3437152ec..382d676e4 100644 --- a/services/report/raw_upload_processor.py +++ b/services/report/raw_upload_processor.py @@ -17,16 +17,10 @@ log = logging.getLogger(__name__) -@dataclass -class SessionAdjustmentResult: - fully_deleted_sessions: list[int] - partially_deleted_sessions: list[int] - - @dataclass class UploadProcessingResult: report: Report # NOTE: this is just returning the input argument, and primarily used in tests - session_adjustment: SessionAdjustmentResult + deleted_sessions: set[int] @sentry_sdk.trace @@ -121,17 +115,16 @@ def process_raw_upload( # Because we know that the processing was successful _sessionid, session = report.add_session(session, use_id_from_session=True) # Adjust sessions removed carryforward sessions that are being replaced + deleted_sessions = set() if session.flags: - session_adjustment = clear_carryforward_sessions( + deleted_sessions = clear_carryforward_sessions( report, session.flags, commit_yaml ) - else: - session_adjustment = SessionAdjustmentResult([], []) report.merge(temporary_report, joined=joined) session.totals = temporary_report.totals - return UploadProcessingResult(report=report, session_adjustment=session_adjustment) + return UploadProcessingResult(report=report, deleted_sessions=deleted_sessions) @sentry_sdk.trace @@ -139,19 +132,19 @@ def clear_carryforward_sessions( original_report: Report, to_merge_flags: list[str], current_yaml: UserYaml, -) -> SessionAdjustmentResult: +) -> set[int]: + session_ids_to_fully_delete = set() to_fully_overwrite_flags = { f for f in to_merge_flags if current_yaml.flag_has_carryfoward(f) } - session_ids_to_fully_delete = [] if to_fully_overwrite_flags: for session_id, session in original_report.sessions.items(): if session.session_type == SessionType.carriedforward and session.flags: if any(f in to_fully_overwrite_flags for f in session.flags): - session_ids_to_fully_delete.append(session_id) + session_ids_to_fully_delete.add(session_id) if session_ids_to_fully_delete: - original_report.delete_multiple_sessions(session_ids_to_fully_delete) + original_report.delete_multiple_sessions(list(session_ids_to_fully_delete)) - return SessionAdjustmentResult(sorted(session_ids_to_fully_delete), []) + return session_ids_to_fully_delete diff --git a/services/report/tests/unit/test_process.py b/services/report/tests/unit/test_process.py index 172cf8931..0686d1778 100644 --- a/services/report/tests/unit/test_process.py +++ b/services/report/tests/unit/test_process.py @@ -1109,8 +1109,7 @@ def test_process_raw_upload_with_carryforwarded_flags(self): session=session, ) report = result.report - assert result.session_adjustment.fully_deleted_sessions == [1] - assert result.session_adjustment.partially_deleted_sessions == [] + assert result.deleted_sessions == {1} assert sorted(report.sessions.keys()) == [0, session.id] assert session.id == 2 assert report.sessions[session.id] == session diff --git a/services/report/tests/unit/test_sessions.py b/services/report/tests/unit/test_sessions.py index 2ec2a1254..99376c574 100644 --- a/services/report/tests/unit/test_sessions.py +++ b/services/report/tests/unit/test_sessions.py @@ -3,15 +3,9 @@ from shared.reports.resources import LineSession, ReportLine, Session, SessionType from shared.yaml import UserYaml -from services.report.raw_upload_processor import ( - SessionAdjustmentResult, - clear_carryforward_sessions, -) +from services.report.raw_upload_processor import clear_carryforward_sessions from test_utils.base import BaseTestCase -# Not calling add_sessions here on purpose, so it doesnt -# interfere with this logic - class TestAdjustSession(BaseTestCase): @pytest.fixture @@ -36,15 +30,16 @@ def sample_first_report(self): ) first_file = EditableReportFile("first_file.py") c = 0 - for sessionid in range(4): - first_file.append( - c % 7 + 1, - self.create_sample_line( - coverage=c, - sessionid=sessionid, - ), - ) - c += 1 + for _ in range(5): + for sessionid in range(4): + first_file.append( + c % 7 + 1, + self.create_sample_line( + coverage=c, + sessionid=sessionid, + ), + ) + c += 1 second_file = EditableReportFile("second_file.py") first_report.append(first_file) first_report.append(second_file) @@ -62,11 +57,6 @@ def sample_first_report(self): ], None, None, - [ - (0, 0, None, ["one_label"]), - (2, 14, None, ["another_label", "one_label"]), - (3, 7, None, ["another_label"]), - ], ), ( 2, @@ -79,12 +69,6 @@ def sample_first_report(self): ], None, None, - [ - (0, 8, None, ["another_label"]), - (0, 8, None, ["one_label"]), - (1, 1, None, ["one_label"]), - (3, 15, None, ["another_label", "one_label"]), - ], ), ( 3, @@ -97,12 +81,6 @@ def sample_first_report(self): ], None, None, - [ - (0, 16, None, ["Th2dMtk4M_codecov"]), - (1, 9, None, ["another_label"]), - (1, 9, None, ["one_label"]), - (2, 2, None, ["one_label"]), - ], ), ( 4, @@ -115,12 +93,6 @@ def sample_first_report(self): ], None, None, - [ - (1, 17, None, ["Th2dMtk4M_codecov"]), - (2, 10, None, ["another_label"]), - (2, 10, None, ["one_label"]), - (3, 3, None, ["one_label"]), - ], ), ( 5, @@ -133,12 +105,6 @@ def sample_first_report(self): ], None, None, - [ - (0, 4, None, ["another_label"]), - (2, 18, None, ["Th2dMtk4M_codecov"]), - (3, 11, None, ["another_label"]), - (3, 11, None, ["one_label"]), - ], ), ( 6, @@ -151,11 +117,6 @@ def sample_first_report(self): ], None, None, - [ - (0, 12, None, ["another_label", "one_label"]), - (1, 5, None, ["another_label"]), - (3, 19, None, ["Th2dMtk4M_codecov"]), - ], ), ( 7, @@ -164,10 +125,6 @@ def sample_first_report(self): [[2, 6, None, None, None], [1, 13, None, None, None]], None, None, - [ - (1, 13, None, ["another_label", "one_label"]), - (2, 6, None, ["another_label"]), - ], ), ] } @@ -182,9 +139,12 @@ def create_sample_line(self, *, coverage, sessionid=None): def test_adjust_sessions_no_cf(self, sample_first_report): first_value = self.convert_report_to_better_readable(sample_first_report) current_yaml = UserYaml({}) - assert clear_carryforward_sessions( - sample_first_report, ["enterprise"], current_yaml - ) == SessionAdjustmentResult([], []) + assert ( + clear_carryforward_sessions( + sample_first_report, ["enterprise"], current_yaml + ) + == set() + ) assert first_value == self.convert_report_to_better_readable( sample_first_report ) @@ -199,7 +159,7 @@ def test_adjust_sessions_full_cf_only(self, sample_first_report): ) assert clear_carryforward_sessions( sample_first_report, ["enterprise"], current_yaml - ) == SessionAdjustmentResult([0], []) + ) == {0} assert self.convert_report_to_better_readable(sample_first_report) == { "archive": { @@ -211,10 +171,6 @@ def test_adjust_sessions_full_cf_only(self, sample_first_report): [[3, 7, None, None, None], [2, 14, None, None, None]], None, None, - [ - (2, 14, None, ["another_label", "one_label"]), - (3, 7, None, ["another_label"]), - ], ), ( 2, @@ -223,10 +179,6 @@ def test_adjust_sessions_full_cf_only(self, sample_first_report): [[1, 1, None, None, None], [3, 15, None, None, None]], None, None, - [ - (1, 1, None, ["one_label"]), - (3, 15, None, ["another_label", "one_label"]), - ], ), ( 3, @@ -235,11 +187,6 @@ def test_adjust_sessions_full_cf_only(self, sample_first_report): [[2, 2, None, None, None], [1, 9, None, None, None]], None, None, - [ - (1, 9, None, ["another_label"]), - (1, 9, None, ["one_label"]), - (2, 2, None, ["one_label"]), - ], ), ( 4, @@ -252,12 +199,6 @@ def test_adjust_sessions_full_cf_only(self, sample_first_report): ], None, None, - [ - (1, 17, None, ["Th2dMtk4M_codecov"]), - (2, 10, None, ["another_label"]), - (2, 10, None, ["one_label"]), - (3, 3, None, ["one_label"]), - ], ), ( 5, @@ -266,11 +207,6 @@ def test_adjust_sessions_full_cf_only(self, sample_first_report): [[3, 11, None, None, None], [2, 18, None, None, None]], None, None, - [ - (2, 18, None, ["Th2dMtk4M_codecov"]), - (3, 11, None, ["another_label"]), - (3, 11, None, ["one_label"]), - ], ), ( 6, @@ -279,10 +215,6 @@ def test_adjust_sessions_full_cf_only(self, sample_first_report): [[1, 5, None, None, None], [3, 19, None, None, None]], None, None, - [ - (1, 5, None, ["another_label"]), - (3, 19, None, ["Th2dMtk4M_codecov"]), - ], ), ( 7, @@ -291,10 +223,6 @@ def test_adjust_sessions_full_cf_only(self, sample_first_report): [[2, 6, None, None, None], [1, 13, None, None, None]], None, None, - [ - (1, 13, None, ["another_label", "one_label"]), - (2, 6, None, ["another_label"]), - ], ), ] }, @@ -371,437 +299,3 @@ def test_adjust_sessions_full_cf_only(self, sample_first_report): "diff": None, }, } - - def test_adjust_sessions_partial_cf_only_no_changes( - self, sample_first_report, mocker - ): - current_yaml = UserYaml( - { - "flag_management": { - "individual_flags": [ - { - "name": "enterprise", - "carryforward_mode": "labels", - "carryforward": True, - } - ] - } - } - ) - first_value = self.convert_report_to_better_readable(sample_first_report) - assert clear_carryforward_sessions( - sample_first_report, ["enterprise"], current_yaml - ) == SessionAdjustmentResult([], [0]) - after_result = self.convert_report_to_better_readable(sample_first_report) - assert after_result == first_value - - def test_adjust_sessions_partial_cf_only_no_changes_encoding_labels( - self, sample_first_report - ): - current_yaml = UserYaml( - { - "flag_management": { - "individual_flags": [ - { - "name": "enterprise", - "carryforward_mode": "labels", - "carryforward": True, - } - ] - } - } - ) - first_value = self.convert_report_to_better_readable(sample_first_report) - assert clear_carryforward_sessions( - sample_first_report, ["enterprise"], current_yaml - ) == SessionAdjustmentResult([], [0]) - after_result = self.convert_report_to_better_readable(sample_first_report) - assert after_result == first_value - - def test_adjust_sessions_partial_cf_only_some_changes(self, sample_first_report): - current_yaml = UserYaml( - { - "flag_management": { - "individual_flags": [ - { - "name": "enterprise", - "carryforward_mode": "labels", - "carryforward": True, - } - ] - } - } - ) - assert clear_carryforward_sessions( - sample_first_report, ["enterprise"], current_yaml - ) == SessionAdjustmentResult([], [0]) - - assert self.convert_report_to_better_readable(sample_first_report) == { - "archive": { - "first_file.py": [ - ( - 1, - 14, - None, - [[3, 7, None, None, None], [2, 14, None, None, None]], - None, - None, - [ - (2, 14, None, ["another_label", "one_label"]), - (3, 7, None, ["another_label"]), - ], - ), - ( - 2, - 15, - None, - [ - [1, 1, None, None, None], - [0, 8, None, None, None], - [3, 15, None, None, None], - ], - None, - None, - [ - (0, 8, None, ["another_label"]), - (1, 1, None, ["one_label"]), - (3, 15, None, ["another_label", "one_label"]), - ], - ), - ( - 3, - 16, - None, - [ - [2, 2, None, None, None], - [1, 9, None, None, None], - [0, 16, None, None, None], - ], - None, - None, - [ - (0, 16, None, ["Th2dMtk4M_codecov"]), - (1, 9, None, ["another_label"]), - (1, 9, None, ["one_label"]), - (2, 2, None, ["one_label"]), - ], - ), - ( - 4, - 17, - None, - [ - [3, 3, None, None, None], - [2, 10, None, None, None], - [1, 17, None, None, None], - ], - None, - None, - [ - (1, 17, None, ["Th2dMtk4M_codecov"]), - (2, 10, None, ["another_label"]), - (2, 10, None, ["one_label"]), - (3, 3, None, ["one_label"]), - ], - ), - ( - 5, - 18, - None, - [ - [0, 4, None, None, None], - [3, 11, None, None, None], - [2, 18, None, None, None], - ], - None, - None, - [ - (0, 4, None, ["another_label"]), - (2, 18, None, ["Th2dMtk4M_codecov"]), - (3, 11, None, ["another_label"]), - (3, 11, None, ["one_label"]), - ], - ), - ( - 6, - 19, - None, - [[1, 5, None, None, None], [3, 19, None, None, None]], - None, - None, - [ - (1, 5, None, ["another_label"]), - (3, 19, None, ["Th2dMtk4M_codecov"]), - ], - ), - ( - 7, - 13, - None, - [[2, 6, None, None, None], [1, 13, None, None, None]], - None, - None, - [ - (1, 13, None, ["another_label", "one_label"]), - (2, 6, None, ["another_label"]), - ], - ), - ] - }, - "report": { - "files": { - "first_file.py": [ - 0, - [0, 7, 7, 0, 0, "100", 0, 0, 0, 0, 0, 0, 0], - None, - None, - ] - }, - "sessions": { - "0": { - "t": None, - "d": None, - "a": None, - "f": ["enterprise"], - "c": None, - "n": None, - "N": None, - "j": None, - "u": None, - "p": None, - "e": None, - "st": "carriedforward", - "se": {}, - }, - "1": { - "t": None, - "d": None, - "a": None, - "f": ["enterprise"], - "c": None, - "n": None, - "N": None, - "j": None, - "u": None, - "p": None, - "e": None, - "st": "uploaded", - "se": {}, - }, - "2": { - "t": None, - "d": None, - "a": None, - "f": ["unit"], - "c": None, - "n": None, - "N": None, - "j": None, - "u": None, - "p": None, - "e": None, - "st": "carriedforward", - "se": {}, - }, - "3": { - "t": None, - "d": None, - "a": None, - "f": ["unrelated"], - "c": None, - "n": None, - "N": None, - "j": None, - "u": None, - "p": None, - "e": None, - "st": "uploaded", - "se": {}, - }, - }, - }, - "totals": { - "f": 1, - "n": 7, - "h": 7, - "m": 0, - "p": 0, - "c": "100", - "b": 0, - "d": 0, - "M": 0, - "s": 4, - "C": 0, - "N": 0, - "diff": None, - }, - } - - def test_adjust_sessions_partial_cf_only_full_deletion_due_to_lost_labels( - self, sample_first_report - ): - current_yaml = UserYaml( - { - "flag_management": { - "individual_flags": [ - { - "name": "enterprise", - "carryforward_mode": "labels", - "carryforward": True, - } - ] - } - } - ) - - assert clear_carryforward_sessions( - sample_first_report, ["enterprise"], current_yaml - ) == SessionAdjustmentResult([0], []) - res = self.convert_report_to_better_readable(sample_first_report) - - assert res["report"]["sessions"] == { - "1": { - "t": None, - "d": None, - "a": None, - "f": ["enterprise"], - "c": None, - "n": None, - "N": None, - "j": None, - "u": None, - "p": None, - "e": None, - "st": "uploaded", - "se": {}, - }, - "2": { - "t": None, - "d": None, - "a": None, - "f": ["unit"], - "c": None, - "n": None, - "N": None, - "j": None, - "u": None, - "p": None, - "e": None, - "st": "carriedforward", - "se": {}, - }, - "3": { - "t": None, - "d": None, - "a": None, - "f": ["unrelated"], - "c": None, - "n": None, - "N": None, - "j": None, - "u": None, - "p": None, - "e": None, - "st": "uploaded", - "se": {}, - }, - } - - assert self.convert_report_to_better_readable(sample_first_report)[ - "archive" - ] == { - "first_file.py": [ - ( - 1, - 14, - None, - [[3, 7, None, None, None], [2, 14, None, None, None]], - None, - None, - [ - (2, 14, None, ["another_label", "one_label"]), - (3, 7, None, ["another_label"]), - ], - ), - ( - 2, - 15, - None, - [[1, 1, None, None, None], [3, 15, None, None, None]], - None, - None, - [ - (1, 1, None, ["one_label"]), - (3, 15, None, ["another_label", "one_label"]), - ], - ), - ( - 3, - 9, - None, - [[2, 2, None, None, None], [1, 9, None, None, None]], - None, - None, - [ - (1, 9, None, ["another_label"]), - (1, 9, None, ["one_label"]), - (2, 2, None, ["one_label"]), - ], - ), - ( - 4, - 17, - None, - [ - [3, 3, None, None, None], - [2, 10, None, None, None], - [1, 17, None, None, None], - ], - None, - None, - [ - (1, 17, None, ["Th2dMtk4M_codecov"]), - (2, 10, None, ["another_label"]), - (2, 10, None, ["one_label"]), - (3, 3, None, ["one_label"]), - ], - ), - ( - 5, - 18, - None, - [[3, 11, None, None, None], [2, 18, None, None, None]], - None, - None, - [ - (2, 18, None, ["Th2dMtk4M_codecov"]), - (3, 11, None, ["another_label"]), - (3, 11, None, ["one_label"]), - ], - ), - ( - 6, - 19, - None, - [[1, 5, None, None, None], [3, 19, None, None, None]], - None, - None, - [ - (1, 5, None, ["another_label"]), - (3, 19, None, ["Th2dMtk4M_codecov"]), - ], - ), - ( - 7, - 13, - None, - [[2, 6, None, None, None], [1, 13, None, None, None]], - None, - None, - [ - (1, 13, None, ["another_label", "one_label"]), - (2, 6, None, ["another_label"]), - ], - ), - ] - } diff --git a/services/tests/test_report.py b/services/tests/test_report.py index 1999c64bd..51277e9a5 100644 --- a/services/tests/test_report.py +++ b/services/tests/test_report.py @@ -20,10 +20,7 @@ ReportService, ) from services.report import log as report_log -from services.report.raw_upload_processor import ( - SessionAdjustmentResult, - clear_carryforward_sessions, -) +from services.report.raw_upload_processor import clear_carryforward_sessions from test_utils.base import BaseTestCase @@ -1424,9 +1421,7 @@ def fake_possibly_shift(report, base, head): assert sorted(report.sessions.keys()) == [2, 3, 4] assert clear_carryforward_sessions( report, ["enterprise"], UserYaml(yaml_dict) - ) == SessionAdjustmentResult( - fully_deleted_sessions=[2, 3], partially_deleted_sessions=[] - ) + ) == {2, 3} assert sorted(report.sessions.keys()) == [4] readable_report = self.convert_report_to_better_readable(report) expected_results = { @@ -1496,9 +1491,7 @@ def test_get_existing_report_for_commit_already_carriedforward_add_sessions( assert sorted(report.sessions.keys()) == [0, 1, 2, 3, 4] assert clear_carryforward_sessions( report, ["enterprise"], UserYaml(yaml_dict) - ) == SessionAdjustmentResult( - fully_deleted_sessions=[2, 3], partially_deleted_sessions=[] - ) + ) == {2, 3} assert sorted(report.sessions.keys()) == [0, 1, 4] readable_report = self.convert_report_to_better_readable(report) expected_sessions_dict = { @@ -1570,10 +1563,8 @@ def test_get_existing_report_for_commit_already_carriedforward_add_sessions( second_to_merge_session = Session(flags=["unit"]) report.add_session(second_to_merge_session) assert sorted(report.sessions.keys()) == [0, 1, 3, 4] - assert clear_carryforward_sessions( - report, ["unit"], UserYaml(yaml_dict) - ) == SessionAdjustmentResult( - fully_deleted_sessions=[], partially_deleted_sessions=[] + assert ( + clear_carryforward_sessions(report, ["unit"], UserYaml(yaml_dict)) == set() ) assert sorted(report.sessions.keys()) == [0, 1, 3, 4] new_readable_report = self.convert_report_to_better_readable(report) @@ -3576,10 +3567,7 @@ def test_update_upload_with_processing_result_success(self, mocker, dbsession): dbsession.add(upload_obj) dbsession.flush() assert len(upload_obj.errors) == 0 - processing_result = ProcessingResult( - session=Session(), - session_adjustment=SessionAdjustmentResult([], []), - ) + processing_result = ProcessingResult(session=Session(), deleted_sessions={}) assert ( ReportService({}).update_upload_with_processing_result( upload_obj, processing_result diff --git a/tasks/tests/unit/test_upload_finisher_task.py b/tasks/tests/unit/test_upload_finisher_task.py index 32c311a18..4dd2df606 100644 --- a/tasks/tests/unit/test_upload_finisher_task.py +++ b/tasks/tests/unit/test_upload_finisher_task.py @@ -426,7 +426,6 @@ def test_finish_reports_processing_with_pull(self, dbsession, mocker): "app.tasks.notify.Notify": mocker.MagicMock(), "app.tasks.pulls.Sync": mocker.MagicMock(), "app.tasks.compute_comparison.ComputeComparison": mocker.MagicMock(), - "app.tasks.upload.UploadCleanLabelsIndex": mocker.MagicMock(), }, ) repository = RepositoryFactory.create( @@ -481,70 +480,6 @@ def test_finish_reports_processing_with_pull(self, dbsession, mocker): mocked_app.tasks[ "app.tasks.compute_comparison.ComputeComparison" ].apply_async.assert_called_once() - mocked_app.tasks[ - "app.tasks.upload.UploadCleanLabelsIndex" - ].apply_async.assert_not_called() - - def test_finish_reports_processing_call_clean_labels(self, dbsession, mocker): - commit_yaml = { - "flag_management": { - "individual_flags": [ - { - "name": "smart-tests", - "carryforward": True, - "carryforward_mode": "labels", - }, - { - "name": "just-tests", - "carryforward": True, - }, - ] - } - } - mocked_app = mocker.patch.object(UploadFinisherTask, "app") - commit = CommitFactory.create( - message="dsidsahdsahdsa", - commitid="abf6d4df662c47e32460020ab14abf9303581429", - repository__owner__unencrypted_oauth_token="testulk3d54rlhxkjyzomq2wh8b7np47xabcrkx8", - repository__owner__username="ThiagoCodecov", - repository__yaml=commit_yaml, - ) - processing_results = { - "processings_so_far": [ - {"successful": True, "arguments": {"flags": "smart-tests"}} - ] - } - dbsession.add(commit) - dbsession.flush() - - checkpoints = _create_checkpoint_logger(mocker) - res = UploadFinisherTask().finish_reports_processing( - dbsession, - commit, - UserYaml(commit_yaml), - processing_results, - None, - checkpoints, - ) - assert res == {"notifications_called": True} - mocked_app.tasks["app.tasks.notify.Notify"].apply_async.assert_any_call( - kwargs={ - "commitid": commit.commitid, - "current_yaml": commit_yaml, - "repoid": commit.repoid, - _kwargs_key(UploadFlow): ANY, - }, - ) - mocked_app.tasks[ - "app.tasks.upload.UploadCleanLabelsIndex" - ].apply_async.assert_called_with( - kwargs={ - "repoid": commit.repoid, - "commitid": commit.commitid, - "report_code": None, - }, - ) - assert mocked_app.send_task.call_count == 0 @pytest.mark.parametrize( "notify_error", @@ -630,76 +565,3 @@ def test_upload_finisher_task_calls_save_commit_measurements_task( "dataset_names": None, } ) - - -class TestShouldCleanLabelsIndex(object): - @pytest.mark.parametrize( - "processing_results, expected", - [ - ( - { - "processings_so_far": [ - {"successful": True, "arguments": {"flags": "smart-tests"}} - ] - }, - True, - ), - ( - { - "processings_so_far": [ - {"successful": True, "arguments": {"flags": "just-tests"}} - ] - }, - False, - ), - ( - { - "processings_so_far": [ - { - "successful": True, - "arguments": {"flags": "just-tests,smart-tests"}, - } - ] - }, - True, - ), - ( - { - "processings_so_far": [ - {"successful": False, "arguments": {"flags": "smart-tests"}} - ] - }, - False, - ), - ( - { - "processings_so_far": [ - {"successful": True, "arguments": {"flags": "just-tests"}}, - {"successful": True, "arguments": {"flags": "smart-tests"}}, - ] - }, - True, - ), - ], - ) - def test_should_clean_labels_index(self, processing_results, expected): - commit_yaml = UserYaml( - { - "flag_management": { - "individual_flags": [ - { - "name": "smart-tests", - "carryforward": True, - "carryforward_mode": "labels", - }, - { - "name": "just-tests", - "carryforward": True, - }, - ] - } - } - ) - task = UploadFinisherTask() - result = task.should_clean_labels_index(commit_yaml, processing_results) - assert result == expected diff --git a/tasks/tests/unit/test_upload_processing_task.py b/tasks/tests/unit/test_upload_processing_task.py index fe51da7af..96f9ad37c 100644 --- a/tasks/tests/unit/test_upload_processing_task.py +++ b/tasks/tests/unit/test_upload_processing_task.py @@ -22,10 +22,7 @@ from services.archive import ArchiveService from services.report import ProcessingError, RawReportInfo, ReportService from services.report.parser.legacy import LegacyReportParser -from services.report.raw_upload_processor import ( - SessionAdjustmentResult, - UploadProcessingResult, -) +from services.report.raw_upload_processor import UploadProcessingResult from tasks.upload_processor import UploadProcessorTask here = Path(__file__) @@ -574,9 +571,7 @@ def test_upload_task_call_with_expired_report( false_report_file.append(18, ReportLine.create(1, [])) false_report.append(false_report_file) mocked_2.side_effect = [ - UploadProcessingResult( - report=false_report, session_adjustment=SessionAdjustmentResult([], []) - ), + UploadProcessingResult(report=false_report, deleted_sessions={}), ReportExpiredException(), ] # Mocking retry to also raise the exception so we can see how it is called @@ -730,9 +725,7 @@ def test_upload_task_call_with_empty_report( false_report_file.append(18, ReportLine.create(1, [])) false_report.append(false_report_file) mocked_2.side_effect = [ - UploadProcessingResult( - report=false_report, session_adjustment=SessionAdjustmentResult([], []) - ), + UploadProcessingResult(report=false_report, deleted_sessions={}), ReportEmptyError(), ] mocker.patch.object(UploadProcessorTask, "app", celery_app) diff --git a/tasks/upload_finisher.py b/tasks/upload_finisher.py index 56439b069..9853eeb75 100644 --- a/tasks/upload_finisher.py +++ b/tasks/upload_finisher.py @@ -37,10 +37,7 @@ from services.processing.state import ProcessingState, should_trigger_postprocessing from services.redis import get_redis_connection from services.report import ReportService, delete_uploads_by_sessionid -from services.report.raw_upload_processor import ( - SessionAdjustmentResult, - clear_carryforward_sessions, -) +from services.report.raw_upload_processor import clear_carryforward_sessions from services.yaml import read_yaml_field from tasks.base import BaseCodecovTask from tasks.parallel_verification import parallel_verification_task @@ -585,9 +582,9 @@ def merge_report(cumulative_report: Report, obj): session, use_id_from_session=True ) - session_adjustment = SessionAdjustmentResult([], []) + deleted_sessions = set() if flags := session.flags: - session_adjustment = clear_carryforward_sessions( + deleted_sessions = clear_carryforward_sessions( cumulative_report, flags, UserYaml(commit_yaml) ) @@ -605,9 +602,8 @@ def merge_report(cumulative_report: Report, obj): upload.state_id = UploadState.PROCESSED.db_id upload.state = "processed" upload.order_number = new_sessionid - delete_uploads_by_sessionid( - upload, session_adjustment.fully_deleted_sessions - ) + if deleted_sessions: + delete_uploads_by_sessionid(upload, list(deleted_sessions)) db_session.flush() return cumulative_report diff --git a/tasks/upload_processor.py b/tasks/upload_processor.py index e5a2c888b..cba80e43f 100644 --- a/tasks/upload_processor.py +++ b/tasks/upload_processor.py @@ -43,7 +43,7 @@ def UPLOAD_PROCESSING_LOCK_NAME(repoid: int, commitid: str) -> str: Only a single processing task may possess this lock at a time, because merging reports requires exclusive access to the report. - This is used by the Upload, Notify and UploadCleanLabelsIndex tasks as well to + This is used by the Upload and Notify tasks as well to verify if an upload for the commit is currently being processed. """ return f"upload_processing_lock_{repoid}_{commitid}"