diff --git a/services/report/__init__.py b/services/report/__init__.py index 8290da8e3..1093ef061 100644 --- a/services/report/__init__.py +++ b/services/report/__init__.py @@ -893,7 +893,7 @@ def build_report_from_raw_content( upload_obj=upload, ) except ReportEmptyError: - log.info( + log.warning( "Report %s is empty", reportid, extra=dict(repoid=commit.repoid, commit=commit.commitid), diff --git a/services/report/raw_upload_processor.py b/services/report/raw_upload_processor.py index b93ca2797..795c4d20c 100644 --- a/services/report/raw_upload_processor.py +++ b/services/report/raw_upload_processor.py @@ -90,10 +90,14 @@ def process_raw_upload( else: ignored_file_lines = None + session = session or Session() # Get a sesisonid to merge into # anything merged into the original_report # will take on this sessionid - sessionid, session = original_report.add_session(session or Session()) + # But we don't actually merge yet in case the report is empty. + # This is done to avoid garbage sessions to build up in the report + # How can you be sure this will be the sessionid used when you actually merge it? Remember that this piece of code runs inside a lock u.u + sessionid = original_report.next_session_number() session.id = sessionid if env: session.env = dict([e.split("=", 1) for e in env.split("\n") if "=" in e]) @@ -182,6 +186,10 @@ def process_raw_upload( # 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 = original_report.add_session(session) + # Adjust sessions removed carryforward sessions that are being replaced session_manipulation_result = _adjust_sessions( original_report, temporary_report, diff --git a/services/report/report_processor.py b/services/report/report_processor.py index 5d9d835b7..ae727a71a 100644 --- a/services/report/report_processor.py +++ b/services/report/report_processor.py @@ -172,7 +172,7 @@ def process_report( f"worker.services.report.processors.{processor.name}.failure" ) raise - log.info( + log.warning( "File format could not be recognized", extra=dict( report_filename=name, first_line=first_line[:100], report_type=report_type diff --git a/services/report/tests/unit/test_process.py b/services/report/tests/unit/test_process.py index b2634c6bb..425fb05d6 100644 --- a/services/report/tests/unit/test_process.py +++ b/services/report/tests/unit/test_process.py @@ -197,8 +197,8 @@ def test_process_raw_upload_empty_report(self): session=Session(flags=["fruits"]), flags=[], ) - assert len(original_report.sessions) == 2 - assert sorted(original_report.flags.keys()) == ["fruits", "unit"] + assert len(original_report.sessions) == 1 + assert sorted(original_report.flags.keys()) == ["unit"] assert original_report.files == ["file_1.go", "file_2.py"] assert original_report.flags["unit"].totals == ReportTotals( files=2, @@ -215,21 +215,7 @@ def test_process_raw_upload_empty_report(self): complexity_total=0, diff=0, ) - assert original_report.flags["fruits"].totals == ReportTotals( - files=0, - lines=0, - hits=0, - misses=0, - partials=0, - coverage=None, - branches=0, - methods=0, - messages=0, - sessions=1, - complexity=0, - complexity_total=0, - diff=0, - ) + assert original_report.flags.get("fruits") is None general_totals, json_data = original_report.to_database() assert general_totals == { "f": 2, @@ -241,7 +227,7 @@ def test_process_raw_upload_empty_report(self): "b": 1, "d": 0, "M": 0, - "s": 2, + "s": 1, "C": 10, "N": 2, "diff": None, @@ -279,22 +265,7 @@ def test_process_raw_upload_empty_report(self): "e": None, "st": "uploaded", "se": {}, - }, - "1": { - "t": None, - "d": None, - "a": None, - "f": ["fruits"], - "c": None, - "n": None, - "N": None, - "j": None, - "u": None, - "p": None, - "e": None, - "st": "uploaded", - "se": {}, - }, + } }, } @@ -1015,9 +986,8 @@ def test_process_raw_upload_multiple_raw_reports(self, mocker): class TestProcessRawUploadCarryforwardFlags(BaseTestCase): - def test_process_raw_upload_with_carryforwarded_flags(self): + def _generate_sample_report(self) -> EditableReport: original_report = EditableReport() - upload_flags = ["somethingold", "flag_two"] first_file = EditableReportFile("banana.py") first_file.append(100, ReportLine.create(1, sessions=[LineSession(0, 1)])) first_file.append(200, ReportLine.create(0, sessions=[LineSession(0, 0)])) @@ -1062,6 +1032,11 @@ def test_process_raw_upload_with_carryforwarded_flags(self): ], "third.c": [(2, 1, None, [[1, 1, None, None, None]], None, None)], } + return original_report + + def test_process_raw_upload_with_carryforwarded_flags(self): + original_report = self._generate_sample_report() + upload_flags = ["somethingold", "flag_two"] raw_content = dumps( { "coverage": { @@ -1229,3 +1204,52 @@ def test_process_raw_upload_with_carryforwarded_flags(self): (1, 1, None, [[session.id, 1, None, None, None]], None, None) ], } + + def test_process_raw_upload_carryforward_flag_empty_upload(self): + """Tests that an upload with carryforward flag doesn't create a new session + in the original report for the flag it was uploaded with. + + That is, a new session is created in the commit's report only if the upload + was properly processed + """ + original_report = self._generate_sample_report() + original_sessions = original_report.sessions + expected_sessions = { + 0: Session(flags=["super"]), + 1: Session(flags=["somethingold"], session_type=SessionType.carriedforward), + } + assert isinstance(original_sessions, dict) + assert list(original_sessions.keys()) == [0, 1] + assert original_sessions == expected_sessions + empty_uploaded_report = LegacyParsedRawReport( + toc=None, + env=None, + report_fixes=None, + uploaded_files=[ + ParsedUploadedReportFile( + filename="/Users/path/to/app.coverage.json", + file_contents=BytesIO( + "Bogus report that shouldn't match any parser".encode() + ), + ), + ], + ) + session = Session(flags=["somethingold"]) + with pytest.raises(ReportEmptyError): + process.process_raw_upload( + UserYaml( + { + "flag_management": { + "individual_flags": [ + {"name": "somethingold", "carryforward": True} + ] + } + } + ), + original_report, + empty_uploaded_report, + ["somethingold"], + session=session, + ) + assert list(original_report.sessions.keys()) == [0, 1] + assert original_report.sessions == expected_sessions