Merge the Report._files/_chunks (#553)

* Merge the `Report._files/_chunks`

Instead of maintaining two separate fields, this merges the previous `_files` which was a dict from filename to `totals`, and the `_chunks` which was either a fully parsed `ReportFile`, or just a shallow `list` of lines.

Maintaining only a single structure for the list of files makes a ton of internals of the `Report` simpler.

It also solves the problem of `file_totals` getting out of sync with the `ReportFile.totals`.

* make parsing/serializing more on-demand

* Move parsing of the `details` to on-demand

* review

Author: Swatinem

shared/reports/carryforward.py

@@ -2,7 +2,6 @@
 import re
 from typing import Mapping, Sequence
 
-from shared.reports.editable import EditableReport
 from shared.reports.resources import Report
 from shared.utils.match import Matcher
 from shared.utils.sessions import SessionType
@@ -36,49 +35,23 @@ def generate_carryforward_report(
     flags: Sequence[str],
     paths: Sequence[str],
     session_extras: Mapping[str, str] | None = None,
-) -> EditableReport:
+) -> Report:
     """
-        Generates a carriedforward report starting from report `report`, flags `flags`
-            and paths `paths`
+    Generates a carriedforward report by filtering the given `report` in-place,
+    to only those files and sessions matching the given `flags` and `paths`.
 
-    What this function does it basically take a report `report` and creates a new report
-        from it (so no changes are done in-place). On this new report, it adds all the information
-        from `report` that relates to sessions that have any of the flags `f`
-
-    This way, for example, if none of the sessions in `report` have a flag in `flags`,
-        it will just produce an empty report
-
-    If there are sessions with any of the flags in `flags`, let's call them `relevant_sessions`,
-        this function will go through all files in `report` that match any of the paths `paths
-        and build a new 'carriedforward' ReportFile from it, with only the ReportLines
-        that had at least one LineSession among the `relevant_sessions` (and proper filter out
-        all the the other sessions from that line). Then all the new EditableReportFile will
-        be added to the report.
-
-    Also, the old sessions are copied over to the new report, with their numbering changed to match
-        the new session order they are in now (they could be the fifth session before,
-        and the first session now)
-
-    Args:
-        report (Report): Description
-        flags (Sequence[str]): Description
-
-    Returns:
-        EditableReport: A new report with only the info related to `flags` on it, as described above
-    """
-    new_report = EditableReport(
-        chunks=report._chunks,
-        files=report._files,
-        sessions=report.sessions,
-        totals=None,
-    )
+    The sessions that are matching the `flags` are being flagged as `carriedforward`,
+    and other sessions are removed from the report."""
     if paths:
         matcher = Matcher(paths)
-        for filename in new_report.files:
-            if not matcher.match(filename):
-                del new_report[filename]
+        files_to_delete = {
+            filename for filename in report._files.keys() if not matcher.match(filename)
+        }
+        for filename in files_to_delete:
+            del report[filename]
+
     sessions_to_delete = []
-    for sid, session in new_report.sessions.items():
+    for sid, session in report.sessions.items():
         if not contain_any_of_the_flags(flags, session.flags):
             sessions_to_delete.append(int(sid))
         else:
@@ -89,8 +62,8 @@ def generate_carryforward_report(
         "Removing sessions that are not supposed to carryforward",
         extra=dict(deleted_sessions=sessions_to_delete),
     )
-    new_report.delete_multiple_sessions(sessions_to_delete)
-    return new_report
+    report.delete_multiple_sessions(sessions_to_delete)
+    return report
 
 
 def contain_any_of_the_flags(expected_flags, actual_flags):

shared/reports/filtered.py

@@ -165,9 +165,9 @@ def network(self):
             if file:
                 yield fname, make_network_file(file.totals)
 
-    def get(self, filename, _else=None):
+    def get(self, filename):
         if not self.should_include(filename):
-            return _else
+            return None
         if not self.flags:
             return self.report.get(filename)
         r = self.report.get(filename)

shared/reports/readonly.py

@@ -65,8 +65,7 @@ def from_chunks(cls, files=None, sessions=None, totals=None, chunks=None):
         )
         totals = inner_report._totals
         filename_mapping = {
-            filename: file_summary.file_index
-            for (filename, file_summary) in inner_report._files.items()
+            filename: idx for idx, filename in enumerate(inner_report._files.keys())
         }
         session_mapping = {
             sid: (session.flags or []) for sid, session in inner_report.sessions.items()
@@ -116,10 +115,6 @@ def get_flag_names(self) -> list[str]:
     def sessions(self):
         return self.inner_report.sessions
 
-    @property
-    def size(self):
-        return self.inner_report.size
-
     def apply_diff(self, *args, **kwargs):
         return self.inner_report.apply_diff(*args, **kwargs)
 

shared/reports/reportfile.py

@@ -13,12 +13,21 @@
 log = logging.getLogger(__name__)
 
 
-class ReportFile(object):
+class ReportFile:
+    name: str
+    _totals: ReportTotals | None
+    diff_totals: ReportTotals | None
+    _raw_lines: str | None
+    _parsed_lines: list[None | str | ReportLine]
+    _details: dict[str, Any]
+    __present_sessions: set[int] | None
+
     def __init__(
         self,
         name: str,
         totals: ReportTotals | list | None = None,
         lines: list[None | str | ReportLine] | str | None = None,
+        diff_totals: ReportTotals | list | None = None,
         ignore=None,
     ):
         """
@@ -32,19 +41,18 @@ def __init__(
             {eof:N, lines:[1,10]}
         """
         self.name = name
-        self._details: dict[str, Any] = {}
+        self._totals = None
+        self.diff_totals = None
+        self._raw_lines = None
+        self._parsed_lines = []
+        self._details = {}
+        self.__present_sessions = None
 
-        # lines = [<details dict()>, <Line #1>, ....]
-        self._lines: list[None | str | ReportLine] = []
         if lines:
             if isinstance(lines, list):
-                self._lines = lines
-
+                self._parsed_lines = lines
             else:
-                lines = lines.splitlines()
-                if detailsline := lines.pop(0):
-                    self._details = orjson.loads(detailsline) or {}
-                self._lines = lines
+                self._raw_lines = lines
 
         self._ignore = _ignore_to_func(ignore) if ignore else None
 
@@ -54,22 +62,38 @@ def __init__(
         # All mutating methods (like `append`, `merge`, etc) will either re-calculate these values
         # directly, or clear them so the `@property` accessors re-calculate them when needed.
 
-        self._totals: ReportTotals | None = None
         if isinstance(totals, ReportTotals):
             self._totals = totals
         elif totals:
             self._totals = ReportTotals(*totals)
 
-        self.__present_sessions: set[int] | None = None
-        if present_sessions := self._details.get("present_sessions"):
-            self.__present_sessions = set(present_sessions)
+        if isinstance(diff_totals, ReportTotals):
+            self.diff_totals = diff_totals
+        elif diff_totals:
+            self.diff_totals = ReportTotals(*diff_totals)
 
     def _invalidate_caches(self):
         self._totals = None
+        self.diff_totals = None
         self.__present_sessions = None
 
+    @property
+    def _lines(self):
+        if self._raw_lines:
+            self._parsed_lines = self._raw_lines.splitlines()
+            detailsline = self._parsed_lines.pop(0)
+
+            self._details = orjson.loads(detailsline or "null") or {}
+            if present_sessions := self._details.get("present_sessions"):
+                self.__present_sessions = set(present_sessions)
+
+            self._raw_lines = None
+
+        return self._parsed_lines
+
     @property
     def _present_sessions(self):
+        _ensure_is_parsed = self._lines
         if self.__present_sessions is None:
             self.__present_sessions = set()
             for _, line in self.lines:
@@ -78,18 +102,16 @@ def _present_sessions(self):
 
     @property
     def details(self):
+        _ensure_is_parsed = self._lines
         self._details["present_sessions"] = sorted(self._present_sessions)
         return self._details
 
     @property
     def totals(self):
         if not self._totals:
-            self._totals = self._process_totals()
+            self._totals = get_line_totals(line for _ln, line in self.lines)
         return self._totals
 
-    def _process_totals(self) -> ReportTotals:
-        return get_line_totals(line for _ln, line in self.lines)
-
     def __repr__(self):
         try:
             return "<%s name=%s lines=%s>" % (
@@ -282,7 +304,8 @@ def merge(self, other_file, joined=True):
         ):
             # previous file was boil-the-ocean
             # OR previous file had END issue
-            self._lines = other_file._lines
+            self._parsed_lines = other_file._lines.copy()
+            self._raw_lines = None
             log.warning(
                 "Doing something weird because of weird .rb logic",
                 extra=dict(report_filename=self.name),
@@ -302,10 +325,11 @@ def merge(self, other_file, joined=True):
 
         else:
             # set new lines object
-            self._lines = [
+            self._parsed_lines = [
                 merge_line(before, after, joined)
                 for before, after in zip_longest(self, other_file)
             ]
+            self._raw_lines = None
 
         self._invalidate_caches()
         return True
@@ -460,7 +484,9 @@ def delete_multiple_sessions(self, session_ids_to_delete: set[int]):
         self._invalidate_caches()
 
         if not new_sessions:
-            self._lines = []  # no remaining sessions means no line data
+            # no remaining sessions means no line data
+            self._parsed_lines = []
+            self._raw_lines = None
             return
 
         for index, line in self.lines: