Merge pull request #109 from codecov/dana/new-files-section

create new files section

services/notification/notifiers/mixins/message/__init__.py

@@ -72,19 +72,22 @@ async def create_message(
         write = message.append
         # note: since we're using append, calling write("") will add a newline to the message
 
-        upper_section_name = self.get_upper_section_name(settings)
-        section_writer_class = get_section_class_from_layout_name(upper_section_name)
-        section_writer = section_writer_class(
-            self.repository,
-            upper_section_name,
-            show_complexity,
-            settings,
-            current_yaml,
-        )
-
-        await self.write_section_to_msg(
-            comparison, changes, diff, links, write, section_writer, behind_by
-        )
+        upper_section_names = self.get_upper_section_names(settings)
+        for upper_section_name in upper_section_names:
+            section_writer_class = get_section_class_from_layout_name(
+                upper_section_name
+            )
+            section_writer = section_writer_class(
+                self.repository,
+                upper_section_name,
+                show_complexity,
+                settings,
+                current_yaml,
+            )
+
+            await self.write_section_to_msg(
+                comparison, changes, diff, links, write, section_writer, behind_by
+            )
 
         is_compact_message = should_message_be_compact(comparison, settings)
 
@@ -190,13 +193,19 @@ def get_middle_layout_section_names(self, settings):
         return [
             section
             for section in sections
-            if section not in ["header", "newheader", "newfooter"]
+            if section not in ["header", "newheader", "newfooter", "newfiles"]
         ]
 
-    def get_upper_section_name(self, settings):
-        if "newheader" in settings["layout"]:
-            return "newheader"
-        return "header"
+    def get_upper_section_names(self, settings):
+        sections = list(map(lambda l: l.strip(), (settings["layout"] or "").split(",")))
+        if "newheader" not in sections and "header" not in sections:
+            sections.insert(0, "header")
+
+        return [
+            section
+            for section in sections
+            if section in ["header", "newheader", "newfiles"]
+        ]
 
     def get_lower_section_name(self, settings):
         if "newfooter" in settings["layout"]:

services/notification/notifiers/mixins/message/helpers.py

@@ -149,32 +149,17 @@ def make_patch_only_metrics(before, after, relative, show_complexity, yaml, pull
     return "".join(("|", coverage, missing_line_str))
 
 
-def get_metrics_function(hide_project_coverage):
-    if hide_project_coverage:
-        metrics = make_patch_only_metrics
-    else:
-        metrics = make_metrics
-    return metrics
-
+def get_table_header(show_complexity):
 
-def get_table_header(hide_project_coverage, show_complexity):
-    if not hide_project_coverage:
-        table_header = (
-            "| Coverage \u0394 |"
-            + (" Complexity \u0394 |" if show_complexity else "")
-            + " |"
-        )
-    else:
-        table_header = "| Patch % | Lines |"
-
-    return table_header
+    return (
+        "| Coverage \u0394 |"
+        + (" Complexity \u0394 |" if show_complexity else "")
+        + " |"
+    )
 
 
-def get_table_layout(hide_project_coverage, show_complexity):
-    if hide_project_coverage:
-        return "|---|---|---|"
-    else:
-        return "|---|---|---|" + ("---|" if show_complexity else "")
+def get_table_layout(show_complexity):
+    return "|---|---|---|" + ("---|" if show_complexity else "")
 
 
 def format_number_to_str(

services/notification/notifiers/mixins/message/sections.py

@@ -15,10 +15,10 @@
     diff_to_string,
     ellipsis,
     escape_markdown,
-    get_metrics_function,
     get_table_header,
     get_table_layout,
     make_metrics,
+    make_patch_only_metrics,
     sort_by_importance,
 )
 from services.urls import get_commit_url_from_commit_sha, get_pull_graph_url
@@ -50,6 +50,8 @@ def get_section_class_from_layout_name(layout_name):
         return NewFooterSectionWriter
     if layout_name.startswith("component"):
         return ComponentsSectionWriter
+    if layout_name == "newfiles":
+        return NewFilesSectionWriter
 
 
 class BaseSectionWriter(object):
@@ -397,20 +399,18 @@ async def do_write_section(self, comparison, diff, changes, links, behind_by=Non
         yield ("```")
 
 
-class FileSectionWriter(BaseSectionWriter):
+class NewFilesSectionWriter(BaseSectionWriter):
     async def do_write_section(self, comparison, diff, changes, links, behind_by=None):
         # create list of files changed in diff
         base_report = comparison.base.report
         head_report = comparison.head.report
         if base_report is None:
             base_report = Report()
-        hide_project_coverage = self.settings.get("hide_project_coverage", False)
-        make_metrics_fn = get_metrics_function(hide_project_coverage)
         files_in_diff = [
             (
                 _diff["type"],
                 path,
-                make_metrics_fn(
+                make_patch_only_metrics(
                     get_totals_from_file_in_reports(base_report, path) or False,
                     get_totals_from_file_in_reports(head_report, path) or False,
                     _diff["totals"],
@@ -428,8 +428,8 @@ async def do_write_section(self, comparison, diff, changes, links, behind_by=Non
             c.path for c in changes or []
         )
         if files_in_diff:
-            table_header = get_table_header(hide_project_coverage, self.show_complexity)
-            table_layout = get_table_layout(hide_project_coverage, self.show_complexity)
+            table_header = "| Patch % | Lines |"
+            table_layout = "|---|---|---|"
 
             # get limit of results to show
             limit = int(self.layout.split(":")[1] if ":" in self.layout else 10)
@@ -454,56 +454,106 @@ def tree_cell(typ, path, metrics, _=None):
                         file_tags=" **Critical**" if path in files_in_critical else "",
                     )
 
-            if not hide_project_coverage:
+            remaining_files = 0
+            printed_files = 0
+            changed_files = sorted(
+                files_in_diff, key=lambda a: a[3] or Decimal("0"), reverse=True
+            )
+            changed_files_with_missing_lines = [f for f in changed_files if f[3] > 0]
+            if changed_files_with_missing_lines:
                 yield (
                     "| [Files]({0}?src=pr&el=tree) {1}".format(
                         links["pull"], table_header
                     )
                 )
                 yield (table_layout)
-                for line in starmap(
-                    tree_cell,
-                    sorted(files_in_diff, key=lambda a: a[3] or Decimal("0"))[:limit],
-                ):
-                    yield (line)
-
-                remaining = len(files_in_diff) - limit
-                if remaining > 0:
-                    yield (
-                        "| ... and [{n} more]({href}?src=pr&el=tree-more) | |".format(
-                            n=remaining, href=links["pull"]
-                        )
+            for file in changed_files_with_missing_lines:
+                if printed_files == limit:
+                    remaining_files += 1
+                else:
+                    printed_files += 1
+                    yield (tree_cell(file[0], file[1], file[2]))
+            if remaining_files:
+                yield (
+                    "| ... and [{n} more]({href}?src=pr&el=tree-more) | |".format(
+                        n=remaining_files, href=links["pull"]
                     )
-            else:
-                remaining_files = 0
-                printed_files = 0
-                changed_files = sorted(
-                    files_in_diff, key=lambda a: a[3] or Decimal("0"), reverse=True
                 )
-                changed_files_with_missing_lines = [
-                    f for f in changed_files if f[3] > 0
-                ]
-                if changed_files_with_missing_lines:
-                    yield (
-                        "| [Files]({0}?src=pr&el=tree) {1}".format(
-                            links["pull"], table_header
-                        )
+
+
+class FileSectionWriter(BaseSectionWriter):
+    async def do_write_section(self, comparison, diff, changes, links, behind_by=None):
+        # create list of files changed in diff
+        base_report = comparison.base.report
+        head_report = comparison.head.report
+        if base_report is None:
+            base_report = Report()
+        files_in_diff = [
+            (
+                _diff["type"],
+                path,
+                make_metrics(
+                    get_totals_from_file_in_reports(base_report, path) or False,
+                    get_totals_from_file_in_reports(head_report, path) or False,
+                    _diff["totals"],
+                    self.show_complexity,
+                    self.current_yaml,
+                    links["pull"],
+                ),
+                int(_diff["totals"].misses + _diff["totals"].partials),
+            )
+            for path, _diff in (diff["files"] if diff else {}).items()
+            if _diff.get("totals")
+        ]
+
+        all_files = set(f[1] for f in files_in_diff or []) | set(
+            c.path for c in changes or []
+        )
+        if files_in_diff:
+            table_header = get_table_header(self.show_complexity)
+            table_layout = get_table_layout(self.show_complexity)
+
+            # get limit of results to show
+            limit = int(self.layout.split(":")[1] if ":" in self.layout else 10)
+            mentioned = []
+            files_in_critical = set()
+            if self.settings.get("show_critical_paths", False):
+                overlay = comparison.get_overlay(OverlayType.line_execution_count)
+                files_in_critical = set(
+                    await overlay.search_files_for_critical_changes(all_files)
+                )
+
+            def tree_cell(typ, path, metrics, _=None):
+                if path not in mentioned:
+                    # mentioned: for files that are in diff and changes
+                    mentioned.append(path)
+                    return "| {rm}[{path}]({compare}?src=pr&el=tree#diff-{hash}){rm}{file_tags} {metrics}".format(
+                        rm="~~" if typ == "deleted" else "",
+                        path=escape_markdown(ellipsis(path, 50, False)),
+                        compare=links["pull"],
+                        hash=b64encode(path.encode()).decode(),
+                        metrics=metrics,
+                        file_tags=" **Critical**" if path in files_in_critical else "",
                     )
-                    yield (table_layout)
-                for file in changed_files_with_missing_lines:
-                    if printed_files == limit:
-                        remaining_files += 1
-                    else:
-                        printed_files += 1
-                        yield (tree_cell(file[0], file[1], file[2]))
-
-                if remaining_files:
-                    yield (
-                        "| ... and [{n} more]({href}?src=pr&el=tree-more) | |".format(
-                            n=remaining_files, href=links["pull"]
-                        )
+
+            yield (
+                "| [Files]({0}?src=pr&el=tree) {1}".format(links["pull"], table_header)
+            )
+            yield (table_layout)
+            for line in starmap(
+                tree_cell,
+                sorted(files_in_diff, key=lambda a: a[3] or Decimal("0"))[:limit],
+            ):
+                yield (line)
+            remaining = len(files_in_diff) - limit
+            if remaining > 0:
+                yield (
+                    "| ... and [{n} more]({href}?src=pr&el=tree-more) | |".format(
+                        n=remaining, href=links["pull"]
                     )
-        if changes and not hide_project_coverage:
+                )
+
+        if changes:
             len_changes_not_in_diff = len(all_files or []) - len(files_in_diff or [])
             if files_in_diff and len_changes_not_in_diff > 0:
                 yield ("")

services/notification/notifiers/tests/unit/test_comment.py

@@ -29,6 +29,7 @@
     ComponentsSectionWriter,
     FileSectionWriter,
     ImpactedEntrypointsSectionWriter,
+    NewFilesSectionWriter,
     NewFooterSectionWriter,
     NewHeaderSectionWriter,
 )
@@ -3486,7 +3487,7 @@ async def test_file_with_critical(self, sample_comparison, mocker):
 
     @pytest.mark.asyncio
     async def test_filesection_hide_project_cov(self, sample_comparison, mocker):
-        section_writer = FileSectionWriter(
+        section_writer = NewFilesSectionWriter(
             sample_comparison.head.commit.repository,
             "layout",
             show_complexity=False,
@@ -3570,7 +3571,7 @@ async def test_filesection_hide_project_cov(self, sample_comparison, mocker):
     async def test_filesection_hide_project_cov_with_changed_files_but_no_missing_lines(
         self, sample_comparison, mocker
     ):
-        section_writer = FileSectionWriter(
+        section_writer = NewFilesSectionWriter(
             sample_comparison.head.commit.repository,
             "layout",
             show_complexity=False,
@@ -3649,7 +3650,7 @@ async def test_filesection_hide_project_cov_with_changed_files_but_no_missing_li
     async def test_filesection_hide_project_cov_no_files_changed(
         self, sample_comparison, mocker
     ):
-        section_writer = FileSectionWriter(
+        section_writer = NewFilesSectionWriter(
             sample_comparison.head.commit.repository,
             "layout",
             show_complexity=False,
@@ -4389,7 +4390,7 @@ async def test_build_message_no_project_coverage(
             repository=comparison.head.commit.repository,
             title="title",
             notifier_yaml_settings={
-                "layout": "newheader, files, newfooter",
+                "layout": "newheader, newfiles, newfooter",
                 "hide_project_coverage": True,
             },
             notifier_site_settings=True,