Revert defaulting to an existing Report in Notify (#894)

services/comparison/types.py

@@ -39,8 +39,9 @@ class Comparison(object):
     enriched_pull: EnrichedPull
     current_yaml: Optional[UserYaml] = None
 
-    # FIXME: The functions down below do not make sense given that we assume
+    # FIXME: The functions down below would not make sense given that we assume
     # a `FullCommit` and its `report` to always exist.
+    # Which they don't, so these checks do make sense, contrary to declared types.
     # Similarly, we also expect an `EnrichedPull` to exist, which does not match
     # the reality.
 

tasks/notify.py

@@ -48,7 +48,7 @@
 from services.lock_manager import LockManager, LockRetry, LockType
 from services.notification import NotificationService
 from services.redis import Redis, get_redis_connection
-from services.report import Report, ReportService
+from services.report import ReportService
 from services.repository import (
     EnrichedPull,
     _get_repo_provider_service_instance,
@@ -607,15 +607,12 @@ def submit_third_party_notifications(
             )
             patch_coverage_base_commitid = None
 
-        # FIXME: The input types can be `None`, though the types of `Comparison`,
-        # and all its users assume that a report exists, so just create an empty
-        # one here.
-        head_report = head_report or ReadOnlyReport.create_from_report(Report())
-        base_report = base_report or ReadOnlyReport.create_from_report(Report())
-
-        # FIXME: A similar problem is that `base_commit` can be `None`, which is
-        # also being flagged by `mypy`, even though the downstream code expects
-        # it to exist.
+        # FIXME: Both the `commit` as well as the `report` on `FullCommit`
+        # (both `head` and `project_coverage_base`) are declared to be non-`None`.
+        # Though you will see type errors below because they indeed can be `None`.
+        # Downstream code seems to be very fragile in this regard.
+        # Some code wrongly assumes things are non-`None` and will error.
+        # Other code checks `report` and then errors on `commit`.
 
         comparison = ComparisonProxy(
             Comparison(

tasks/tests/integration/test_notify_task.py

@@ -2,7 +2,7 @@
 from unittest.mock import patch
 
 import pytest
-from mock import ANY, AsyncMock, PropertyMock
+from mock import AsyncMock, PropertyMock
 from shared.validation.types import CoverageCommentRequiredChanges
 
 from database.models import Pull
@@ -102,9 +102,9 @@ def test_simple_call_no_notifiers(
                             "repository": "example-python",
                             "owner": "ThiagoCodecov",
                             "comparison": {
-                                "url": ANY,
-                                "message": "no change",
-                                "coverage": "0.00",
+                                "url": None,
+                                "message": "unknown",
+                                "coverage": None,
                                 "notation": "",
                                 "head_commit": {
                                     "commitid": "649eaaf2924e92dc7fd8d370ddb857033231e67a",