Fix floating point bug

coverage_comment/files.py

@@ -110,18 +110,32 @@ def compute_datafile(
     )
 
 
-def parse_datafile(contents: str) -> tuple[coverage.Coverage | None, decimal.Decimal]:
+def parse_datafile(
+    contents: str, current_rate: decimal.Decimal
+) -> tuple[coverage.Coverage | None, decimal.Decimal]:
     file_contents = json.loads_dict(contents)
-    coverage_rate = decimal.Decimal(str(file_contents["coverage"])) / decimal.Decimal(
-        100
-    )
     try:
-        return coverage.extract_info(
+        previous_coverage = coverage.extract_info(
             data=file_contents["raw_data"],  # pyright: ignore[reportArgumentType]
             coverage_path=pathlib.Path(file_contents["coverage_path"]),  # pyright: ignore[reportArgumentType]
-        ), coverage_rate
+        )
     except KeyError:
-        return None, coverage_rate
+        stored_rate = file_contents["coverage"]
+        assert isinstance(stored_rate, int | float)
+        return None, rate_from_stored_float(
+            stored_rate=stored_rate, current_rate=current_rate
+        )
+    return previous_coverage, previous_coverage.info.percent_covered
+
+
+def rate_from_stored_float(
+    stored_rate: float, current_rate: decimal.Decimal
+) -> decimal.Decimal:
+    # The stored float carries less precision than a freshly computed Decimal:
+    # compare in float space so an unchanged rate isn't seen as a tiny delta.
+    if float(current_rate * 100) == stored_rate:
+        return current_rate
+    return decimal.Decimal(str(stored_rate)) / decimal.Decimal(100)
 
 
 class ImageURLs(TypedDict):

coverage_comment/main.py

@@ -146,7 +146,8 @@ def process_pr(
     previous_coverage, previous_coverage_rate = None, None
     if previous_coverage_data_file:
         previous_coverage, previous_coverage_rate = files.parse_datafile(
-            contents=previous_coverage_data_file
+            contents=previous_coverage_data_file,
+            current_rate=coverage.info.percent_covered,
         )
 
     marker = template.get_marker(marker_id=config.SUBPROJECT_ID)

coverage_comment/template_files/comment.md.j2

@@ -6,7 +6,7 @@
 {%- if previous_coverage_rate %}
 {%- set text = "Coverage for the whole project went from " ~ (previous_coverage_rate | pct) ~ " to " ~ (coverage.info.percent_covered | pct) -%}
 {%- set color = (coverage.info.percent_covered - previous_coverage_rate) | get_evolution_color(neutral_color='blue') -%}
-<img title="{{ text }}" src="{{ 'Coverage evolution' | generate_badge(message=(previous_coverage_rate | pct(precision=0)) ~ ' > ' ~ coverage.info.percent_covered | pct(precision=0), color=color) }}">
+<img title="{{ text }}" src="{{ 'Coverage evolution' | generate_badge(message=(previous_coverage_rate | pct(precision=0)) ~ ' → ' ~ coverage.info.percent_covered | pct(precision=0), color=color) }}">
 
 {%- else -%}
 {%- set text = "Coverage for the whole project is " ~ (coverage.info.percent_covered | pct) ~ ". Previous coverage rate is not available, cannot report on evolution." -%}
@@ -93,7 +93,7 @@
 {%- set text = "This PR doesn't change the coverage rate in " ~ path ~ ", which is " ~ percent_covered | pct ~ " (" ~ covered_statements_count ~ "/" ~ statements_count ~ ")." -%}
 {%- endif -%}
 {%- set color = coverage_diff | get_evolution_color() -%}
-{%- set message = "(" ~ previous_covered_statements_count | compact ~ "/" ~ previous_statements_count | compact ~ " > " ~ covered_statements_count | compact ~ "/" ~ statements_count | compact ~ ")" -%}
+{%- set message = "(" ~ previous_covered_statements_count | compact ~ "/" ~ previous_statements_count | compact ~ " → " ~ covered_statements_count | compact ~ "/" ~ statements_count | compact ~ ")" -%}
 {%- else -%}
 {%- set text = "The coverage rate of " ~ path ~ " is " ~ percent_covered | pct ~ " (" ~ covered_statements_count ~ "/" ~ statements_count ~ "). The file did not seem to exist on the base branch." -%}
 {%- set message = "(" ~ covered_statements_count | compact ~ "/" ~ statements_count | compact ~ ")" -%}

tests/unit/test_files.py

@@ -4,6 +4,8 @@
 import json
 import pathlib
 
+import pytest
+
 from coverage_comment import files
 
 
@@ -68,7 +70,9 @@ def test_compute_datafile():
 
 
 def test_parse_datafile():
-    assert files.parse_datafile(contents="""{"coverage": 12.34}""") == (
+    assert files.parse_datafile(
+        contents="""{"coverage": 12.34}""", current_rate=decimal.Decimal("0.5")
+    ) == (
         None,
         decimal.Decimal("0.1234"),
     )
@@ -82,10 +86,65 @@ def test_parse_datafile__previous(coverage_json, coverage_obj):
                 "raw_data": coverage_json,
                 "coverage_path": ".",
             }
+        ),
+        current_rate=decimal.Decimal("0.5"),
+    )
+
+    assert result == (coverage_obj, coverage_obj.info.percent_covered)
+
+
+def test_parse_datafile__previous_rate_is_exact(coverage_json, coverage_obj):
+    current_rate = coverage_obj.info.percent_covered
+    _, previous_rate = files.parse_datafile(
+        contents=files.compute_datafile(
+            raw_coverage_data=coverage_json,
+            line_rate=current_rate * 100,
+            coverage_path=pathlib.Path("."),
+        ),
+        current_rate=current_rate,
+    )
+
+    assert previous_rate == current_rate
+
+
+@pytest.mark.parametrize(
+    "current_rate",
+    [
+        decimal.Decimal(1) / decimal.Decimal(3),
+        decimal.Decimal(2) / decimal.Decimal(3),
+        decimal.Decimal(3931) / decimal.Decimal(4166),
+        decimal.Decimal(3931) / decimal.Decimal(4167),
+    ],
+)
+def test_rate_from_stored_float__unchanged(current_rate):
+    assert (
+        files.rate_from_stored_float(
+            stored_rate=float(current_rate * 100), current_rate=current_rate
         )
+        == current_rate
+    )
+
+
+@pytest.mark.parametrize(
+    "previous_rate, current_rate",
+    [
+        (
+            decimal.Decimal(999_999) / decimal.Decimal(1_000_000),
+            decimal.Decimal(999_998) / decimal.Decimal(999_999),
+        ),
+        (
+            decimal.Decimal(999_998) / decimal.Decimal(999_999),
+            decimal.Decimal(999_999) / decimal.Decimal(1_000_000),
+        ),
+    ],
+)
+def test_rate_from_stored_float__tiny_change(previous_rate, current_rate):
+    result = files.rate_from_stored_float(
+        stored_rate=float(previous_rate * 100), current_rate=current_rate
     )
 
-    assert result == (coverage_obj, decimal.Decimal("0.1234"))
+    assert (current_rate - result > 0) is (current_rate > previous_rate)
+    assert result != current_rate
 
 
 def test_get_urls():