Skip to content

Commit

Permalink
fix tests and more code removal related to partial sessions
Browse files Browse the repository at this point in the history
  • Loading branch information
@Swatinem
Swatinem committed Oct 14, 2024
1 parent f2b8e07 commit 78671e3
Show file tree
Hide file tree
Showing 10 changed files with 49 additions and 770 deletions.
16 changes: 6 additions & 10 deletions services/report/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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
Expand Down
42 changes: 0 additions & 42 deletions services/report/languages/tests/unit/test_pycoverage.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand All @@ -228,7 +227,6 @@ def test_process_pycoverage(self):
[[0, 1, None, None, None]],
None,
None,
[(0, 1, None, ["Th2dMtk4M_codecov"])],
),
(
2,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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": [
Expand All @@ -272,7 +261,6 @@ def test_process_pycoverage(self):
[[0, 1, None, None, None]],
None,
None,
[(0, 1, None, ["Th2dMtk4M_codecov"])],
),
(
3,
Expand All @@ -281,7 +269,6 @@ def test_process_pycoverage(self):
[[0, 1, None, None, None]],
None,
None,
[(0, 1, None, ["Th2dMtk4M_codecov"])],
),
(
4,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -308,7 +293,6 @@ def test_process_pycoverage(self):
[[0, 0, None, None, None]],
None,
None,
[],
),
(
9,
Expand All @@ -317,7 +301,6 @@ def test_process_pycoverage(self):
[[0, 1, None, None, None]],
None,
None,
[(0, 1, None, ["Th2dMtk4M_codecov"])],
),
(
10,
Expand All @@ -326,7 +309,6 @@ def test_process_pycoverage(self):
[[0, 0, None, None, None]],
None,
None,
[],
),
],
"test_another.py": [
Expand All @@ -337,7 +319,6 @@ def test_process_pycoverage(self):
[[0, 1, None, None, None]],
None,
None,
[(0, 1, None, ["Th2dMtk4M_codecov"])],
),
(
3,
Expand All @@ -346,7 +327,6 @@ def test_process_pycoverage(self):
[[0, 1, None, None, None]],
None,
None,
[(0, 1, None, ["Th2dMtk4M_codecov"])],
),
(
4,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -373,7 +351,6 @@ def test_process_pycoverage(self):
[[0, 1, None, None, None]],
None,
None,
[(0, 1, None, ["Th2dMtk4M_codecov"])],
),
(
8,
Expand All @@ -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": [
Expand All @@ -393,7 +369,6 @@ def test_process_pycoverage(self):
[[0, 1, None, None, None]],
None,
None,
[(0, 1, None, ["Th2dMtk4M_codecov"])],
),
(
4,
Expand All @@ -402,7 +377,6 @@ def test_process_pycoverage(self):
[[0, 1, None, None, None]],
None,
None,
[(0, 1, None, ["Th2dMtk4M_codecov"])],
),
(
5,
Expand All @@ -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"])],
),
],
},
Expand Down Expand Up @@ -488,7 +461,6 @@ def test_process_compressed_report(self):
[[0, 1, None, None, None]],
None,
None,
[(0, 1, None, ["Th2dMtk4M_codecov"])],
),
(
2,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -521,7 +485,6 @@ def test_process_compressed_report(self):
[[0, 0, None, None, None]],
None,
None,
[],
),
(
5,
Expand All @@ -530,7 +493,6 @@ def test_process_compressed_report(self):
[[0, 1, None, None, None]],
None,
None,
[(0, 1, None, ["label_5"])],
),
],
"__init__.py": [
Expand All @@ -541,7 +503,6 @@ def test_process_compressed_report(self):
[[0, 0, None, None, None]],
None,
None,
[],
),
(
3,
Expand All @@ -550,7 +511,6 @@ def test_process_compressed_report(self):
[[0, 0, None, None, None]],
None,
None,
[],
),
(
4,
Expand All @@ -559,7 +519,6 @@ def test_process_compressed_report(self):
[[0, 0, None, None, None]],
None,
None,
[],
),
(
5,
Expand All @@ -568,7 +527,6 @@ def test_process_compressed_report(self):
[[0, 0, None, None, None]],
None,
None,
[],
),
],
},
Expand Down
25 changes: 9 additions & 16 deletions services/report/raw_upload_processor.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -121,37 +115,36 @@ 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
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
3 changes: 1 addition & 2 deletions services/report/tests/unit/test_process.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit 78671e3

Please sign in to comment.