Skip to content

Commit

Permalink
Merge branch 'main' into matt/fix-brolly-rollup
Browse files Browse the repository at this point in the history
  • Loading branch information
matt-codecov committed Aug 15, 2023
2 parents f8aa611 + 3dbbeed commit 6798714
Show file tree
Hide file tree
Showing 10 changed files with 277 additions and 119 deletions.
30 changes: 30 additions & 0 deletions database/models/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,36 @@ def get_head_commit_notifications(self):
)
return []

def get_repository(self):
return self.repository

def get_commitid(self):
return None

@property
def external_id(self):
return self.pullid

@property
def id(self):
return self.id_

def should_write_to_storage(self) -> bool:
if self.repository is None or self.repository.owner is None:
return False
is_codecov_repo = self.repository.owner.username == "codecov"
return should_write_data_to_storage_config_check(
master_switch_key="pull_flare",
is_codecov_repo=is_codecov_repo,
repoid=self.repository.repoid,
)

_flare = Column("flare", postgresql.JSON)
_flare_storage_path = Column("flare_storage_path", types.Text, nullable=True)
flare = ArchiveField(
should_write_to_storage_fn=should_write_to_storage, default_value={}
)


class CommitNotification(CodecovBaseModel):
__tablename__ = "commit_notifications"
Expand Down
11 changes: 9 additions & 2 deletions database/utils.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import json
import logging
from functools import lru_cache
from typing import Any, Callable
from typing import Any, Callable, Optional

from shared.storage.exceptions import FileNotInStorageError
from shared.utils.ReportEncoder import ReportEncoder
Expand All @@ -18,16 +18,23 @@ def __subclasscheck__(cls, subclass):
and callable(subclass.get_repository)
and hasattr(subclass, "get_commitid")
and callable(subclass.get_commitid)
and hasattr(subclass, "external_id")
)


class ArchiveFieldInterface(metaclass=ArchiveFieldInterfaceMeta):
"""Any class that uses ArchiveField must implement this interface"""

external_id: str

def get_repository(self):
"""Returns the repository object associated with self"""
raise NotImplementedError()

def get_commitid(self) -> str:
def get_commitid(self) -> Optional[str]:
"""Returns the commitid associated with self.
If no commitid is associated return None.
"""
raise NotImplementedError()


Expand Down
35 changes: 22 additions & 13 deletions services/archive.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
class MinioEndpoints(Enum):
chunks = "{version}/repos/{repo_hash}/commits/{commitid}/{chunks_file_name}.txt"
json_data = "{version}/repos/{repo_hash}/commits/{commitid}/json_data/{table}/{field}/{external_id}.json"
json_data_no_commit = (
"{version}/repos/{repo_hash}/json_data/{table}/{field}/{external_id}.json"
)
profiling_summary = "{version}/repos/{repo_hash}/profilingsummaries/{profiling_commit_id}/{location}"
raw = "v4/raw/{date}/{repo_hash}/{commit_sha}/{reportid}.txt"
profiling_collection = "{version}/repos/{repo_hash}/profilingcollections/{profiling_commit_id}/{location}"
Expand Down Expand Up @@ -200,20 +203,26 @@ def write_json_data_to_storage(
*,
encoder=ReportEncoder,
):
path = MinioEndpoints.json_data.get_path(
version="v4",
repo_hash=self.storage_hash,
commitid=commit_id,
table=table,
field=field,
external_id=external_id,
)
stringified_data = json.dumps(data, cls=encoder)
if table == "reports_reportdetails" and not ("meta" in stringified_data):
log.warning(
"Saving files_array data without meta",
extra=dict(commit=commit_id, data=stringified_data, path=path),
if commit_id is None:
# Some classes don't have a commit associated with them
# For example Pull belongs to multiple commits.
path = MinioEndpoints.json_data_no_commit.get_path(
version="v4",
repo_hash=self.storage_hash,
table=table,
field=field,
external_id=external_id,
)
else:
path = MinioEndpoints.json_data.get_path(
version="v4",
repo_hash=self.storage_hash,
commitid=commit_id,
table=table,
field=field,
external_id=external_id,
)
stringified_data = json.dumps(data, cls=encoder)
self.write_file(path, stringified_data)
return path

Expand Down
47 changes: 47 additions & 0 deletions services/notification/notifiers/mixins/message/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,53 @@ def make_metrics(before, after, relative, show_complexity, yaml):
return "".join(("|", coverage, complexity, icon))


def make_patch_only_metrics(before, after, relative, show_complexity, yaml):
if after is None:
# e.g. missing flags
coverage = " `?` |"

elif after is False:
# e.g. file deleted
coverage = " |"

else:
layout = " `{relative}` |"
coverage = layout.format(
relative=format_number_to_str(
yaml, relative.coverage if relative else 0, style="{0}%", if_null="\xF8"
),
)
return "".join(("|", coverage))


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(hide_project_coverage, show_complexity):
if not hide_project_coverage:
table_header = (
"| Coverage \u0394 |"
+ (" Complexity \u0394 |" if show_complexity else "")
+ " |"
)
else:
table_header = "| Coverage |"

return table_header


def get_table_layout(hide_project_coverage, show_complexity):
if hide_project_coverage:
return "|---|---|"
else:
return "|---|---|---|" + ("---|" if show_complexity else "")


def format_number_to_str(
yml, value, if_zero=None, if_null=None, plus=False, style="{0}"
) -> str:
Expand Down
36 changes: 24 additions & 12 deletions services/notification/notifiers/mixins/message/sections.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
diff_to_string,
ellipsis,
escape_markdown,
get_metrics_function,
get_table_header,
get_table_layout,
make_metrics,
sort_by_importance,
)
Expand Down Expand Up @@ -464,11 +467,13 @@ async def do_write_section(self, comparison, diff, changes, links, behind_by=Non
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(
make_metrics_fn(
get_totals_from_file_in_reports(base_report, path) or False,
get_totals_from_file_in_reports(head_report, path) or False,
_diff["totals"],
Expand All @@ -487,12 +492,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 = (
"| Coverage \u0394 |"
+ (" Complexity \u0394 |" if self.show_complexity else "")
+ " |"
)
table_layout = "|---|---|---|" + ("---|" if self.show_complexity else "")
table_header = get_table_header(hide_project_coverage, self.show_complexity)
table_layout = get_table_layout(hide_project_coverage, self.show_complexity)
# add table headers
yield (
"| [Files Changed]({0}?src=pr&el=tree) {1}".format(
Expand Down Expand Up @@ -539,7 +540,7 @@ def tree_cell(typ, path, metrics, _=None):
)
)

if changes:
if changes and not hide_project_coverage:
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 ("")
Expand Down Expand Up @@ -628,7 +629,10 @@ async def do_write_section(self, comparison, diff, changes, links, behind_by=Non
+ ("---|" if has_carriedforward_flags else "")
)

yield ("| Flag " + table_header)
yield (
"| [Flag]({href}/flags?src=pr&el=flags) ".format(href=links["pull"])
+ table_header
)
yield (table_layout)
for flag in sorted(flags, key=lambda f: f["name"]):
carriedforward, carriedforward_from = (
Expand Down Expand Up @@ -660,7 +664,10 @@ async def do_write_section(self, comparison, diff, changes, links, behind_by=Non

yield (
"| {name} {metrics}{cf}".format(
name=flag["name"],
name="[{flag_name}]({href}/flags?src=pr&el=flag)".format(
flag_name=flag["name"],
href=links["pull"],
),
metrics=make_metrics(
flag["before"],
flag["after"],
Expand Down Expand Up @@ -721,13 +728,18 @@ async def do_write_section(
)

# Table header and layout
yield "| Components | Coverage \u0394 | |"
yield "| [Components]({href}/components?src=pr&el=components) | Coverage \u0394 | |".format(
href=links["pull"],
)
yield "|---|---|---|"
# The interesting part
for component_data in component_data_to_show:
yield (
"| {name} {metrics}".format(
name=component_data["name"],
name="[{component_name}]({href}/components?src=pr&el=component)".format(
component_name=component_data["name"],
href=links["pull"],
),
metrics=make_metrics(
component_data["before"],
component_data["after"],
Expand Down
34 changes: 17 additions & 17 deletions services/notification/notifiers/tests/integration/test_comment.py
Original file line number Diff line number Diff line change
Expand Up @@ -364,10 +364,10 @@ async def test_notify(self, sample_comparison, codecov_vcr, mock_configuration):
"- Partials 0 1 +1 ",
"```",
"",
"| Flag | Coverage Δ | Complexity Δ | |",
"| [Flag](https://app.codecov.io/gh/ThiagoCodecov/example-python/pull/15/flags?src=pr&el=flags) | Coverage Δ | Complexity Δ | |",
"|---|---|---|---|",
"| integration | `?` | `?` | |",
"| unit | `100.00% <ø> (?)` | `0.00 <ø> (?)` | |",
"| [integration](https://app.codecov.io/gh/ThiagoCodecov/example-python/pull/15/flags?src=pr&el=flag) | `?` | `?` | |",
"| [unit](https://app.codecov.io/gh/ThiagoCodecov/example-python/pull/15/flags?src=pr&el=flag) | `100.00% <ø> (?)` | `0.00 <ø> (?)` | |",
"",
"Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.",
"",
Expand Down Expand Up @@ -431,7 +431,7 @@ async def test_notify_upload_limited(
):
mock_configuration._params["setup"] = {
"codecov_url": None,
"codecov_dashboard_url": "test.example.br",
"codecov_dashboard_url": "https://app.codecov.io",
}
comparison = sample_comparison_for_limited_upload
notifier = CommentNotifier(
Expand All @@ -447,10 +447,10 @@ async def test_notify_upload_limited(
assert result.notification_successful
assert result.explanation is None
expected_message = [
f"## [Codecov](test.example.br/plan/gh/test-acc9) upload limit reached :warning:",
f"## [Codecov](https://app.codecov.io/plan/gh/test-acc9) upload limit reached :warning:",
f"This org is currently on the free Basic Plan; which includes 250 free private repo uploads each rolling month.\
This limit has been reached and additional reports cannot be generated. For unlimited uploads,\
upgrade to our [pro plan](test.example.br/plan/gh/test-acc9).",
upgrade to our [pro plan](https://app.codecov.io/plan/gh/test-acc9).",
f"",
f"**Do you have questions or need help?** Connect with our sales team today at ` [email protected] `",
]
Expand Down Expand Up @@ -507,10 +507,10 @@ async def test_notify_gitlab(
"- Partials 0 1 +1 ",
"```",
"",
"| Flag | Coverage Δ | Complexity Δ | |",
"| [Flag](https://app.codecov.io/gl/joseph-sentry/example-python/pull/1/flags?src=pr&el=flags) | Coverage Δ | Complexity Δ | |",
"|---|---|---|---|",
"| integration | `?` | `?` | |",
"| unit | `100.00% <ø> (?)` | `0.00 <ø> (?)` | |",
"| [integration](https://app.codecov.io/gl/joseph-sentry/example-python/pull/1/flags?src=pr&el=flag) | `?` | `?` | |",
"| [unit](https://app.codecov.io/gl/joseph-sentry/example-python/pull/1/flags?src=pr&el=flag) | `100.00% <ø> (?)` | `0.00 <ø> (?)` | |",
"",
"Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.",
"",
Expand Down Expand Up @@ -578,10 +578,10 @@ async def test_notify_new_layout(
"- Partials 0 1 +1 ",
"```",
"",
"| Flag | Coverage Δ | Complexity Δ | |",
"| [Flag](https://app.codecov.io/gh/ThiagoCodecov/example-python/pull/15/flags?src=pr&el=flags) | Coverage Δ | Complexity Δ | |",
"|---|---|---|---|",
"| integration | `?` | `?` | |",
"| unit | `100.00% <ø> (?)` | `0.00 <ø> (?)` | |",
"| [integration](https://app.codecov.io/gh/ThiagoCodecov/example-python/pull/15/flags?src=pr&el=flag) | `?` | `?` | |",
"| [unit](https://app.codecov.io/gh/ThiagoCodecov/example-python/pull/15/flags?src=pr&el=flag) | `100.00% <ø> (?)` | `0.00 <ø> (?)` | |",
"",
"Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.",
"",
Expand Down Expand Up @@ -653,18 +653,18 @@ async def test_notify_with_components(
"- Partials 0 1 +1 ",
"```",
"",
"| Flag | Coverage Δ | Complexity Δ | |",
"| [Flag](https://app.codecov.io/gh/codecove2e/example-python/pull/4/flags?src=pr&el=flags) | Coverage Δ | Complexity Δ | |",
"|---|---|---|---|",
"| integration | `?` | `?` | |",
"| unit | `100.00% <ø> (?)` | `0.00 <ø> (?)` | |",
"| [integration](https://app.codecov.io/gh/codecove2e/example-python/pull/4/flags?src=pr&el=flag) | `?` | `?` | |",
"| [unit](https://app.codecov.io/gh/codecove2e/example-python/pull/4/flags?src=pr&el=flag) | `100.00% <ø> (?)` | `0.00 <ø> (?)` | |",
"",
"Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.",
"",
"[see 2 files with indirect coverage changes](https://app.codecov.io/gh/codecove2e/example-python/pull/4/indirect-changes?src=pr&el=tree-more)",
"",
"| Components | Coverage Δ | |",
"| [Components](https://app.codecov.io/gh/codecove2e/example-python/pull/4/components?src=pr&el=components) | Coverage Δ | |",
"|---|---|---|",
"| go_files | `62.50% <ø> (+12.50%)` | :arrow_up: |",
"| [go_files](https://app.codecov.io/gh/codecove2e/example-python/pull/4/components?src=pr&el=component) | `62.50% <ø> (+12.50%)` | :arrow_up: |",
"",
"</details>",
"",
Expand Down
Loading

0 comments on commit 6798714

Please sign in to comment.