Repository: parroty/excoveralls
Pull request #310 (2023-05-24 08:29:33)

Always floor coverage instead of rounding
We do not want to report a 100% coverage when there are lines that are not covered.

albertored: @parroty any news on this?
albertored: PS: @parroty this is an improvement over #257 covering all the cases where percentages are calculated and formatted, if merged #257 can be closed
parroty: Thanks for the follow-up 🙇 . Please let me separate this item from the current `v0.17.0`, as I would like to clarify the compatibility (breaking) portion of this change.
tielur: @parroty interested when this is going out to hex, we are running into this on our CI reporting 100% even though it's not actually 100%
parroty: I'm sorry about not responding to this... I have some concerns around the compatibility part. Could anyone help adding this as configuration option, as opt-in? 🙇 
albertored: Hi @parroty, I understand your concerns about being retro-compatible but I think the actual behaviour is conceptually wrong (showing 100% coverage when some lines are not covered). Are you open to adding a configuration option to restore the previous behaviour and make the floor coverage the default one? I.e. having an opt-out option instead of opt-in as you suggested 
parroty: > Are you open to adding a configuration option to restore the previous behaviour and make the floor coverage the default one? I.e. having an opt-out option instead of opt-in as you suggested

Thank you for the comment 🙇 . I was originally thinking about maintaining the current behavior (`opt-in` for a floor coverage change), but it may be acceptable for the new settings for keeping the behavior considering your point (by noting it as a behavioral change in Changelog).
albertored: Perfect, I'll add the option to this PR in a couple of days, thank you
albertored: @parroty ready

Patch:
diff --git a/README.md b/README.md
index 031fcc7..93cd081 100644
--- a/README.md
+++ b/README.md
@@ -441,6 +441,9 @@ to `false`:
   - When set to a number greater than 0, this setting causes the `mix coveralls` and `mix coveralls.html` tasks to exit with a status code of 1 if test coverage falls below the specified threshold (defaults to 0). This is useful to interrupt CI pipelines with strict code coverage rules. Should be expressed as a number between 0 and 100 signifying the minimum percentage of lines covered.
 - `html_filter_full_covered`
   - A boolean, when `true` files with 100% coverage are not shown in the HTML report. Default to `false`.
+- `floor_coverage`
+  - A boolean, when `false` coverage values are ceiled instead of floored, this means that a project with some lines
+    that are not covered can still have a total 100% coverage. Default to `true`.
 
 Example configuration file:
 
diff --git a/lib/conf/coveralls.json b/lib/conf/coveralls.json
index f6cfa93..0d1c485 100644
--- a/lib/conf/coveralls.json
+++ b/lib/conf/coveralls.json
@@ -16,7 +16,8 @@
   "coverage_options": {
     "treat_no_relevant_lines_as_covered": false,
     "output_dir": "cover/",
-    "minimum_coverage": 0
+    "minimum_coverage": 0,
+    "floor_coverage": true
   },
 
   "terminal_options": {
diff --git a/lib/excoveralls/local.ex b/lib/excoveralls/local.ex
index 2317f71..f262d79 100644
--- a/lib/excoveralls/local.ex
+++ b/lib/excoveralls/local.ex
@@ -2,8 +2,6 @@ defmodule ExCoveralls.Local do
   @moduledoc """
   Locally displays the result to screen.
   """
-  
-  
 
   defmodule Count do
     @moduledoc """
@@ -102,7 +100,7 @@ defmodule ExCoveralls.Local do
         Enum.map(count_info, fn original ->
           [stat, count] = original
           %{
-            "cov" => get_coverage(count),
+            "cov" => ExCoveralls.Stats.get_coverage(count.relevant, count.covered),
             "file" => stat[:name],
             "lines" => count.lines,
             "relevant" => count.relevant,
@@ -145,16 +143,16 @@ defmodule ExCoveralls.Local do
   end
 
   defp format_info([stat, count]) do
-    coverage = get_coverage(count)
+    coverage = ExCoveralls.Stats.get_coverage(count.relevant, count.covered)
     file_width = ExCoveralls.Settings.get_file_col_width
-    print_string("~5.1f% ~-#{file_width}s ~8w ~8w ~8w",
+    print_string("~5w% ~-#{file_width}s ~8w ~8w ~8w",
       [coverage, stat[:name], count.lines, count.relevant, count.relevant - count.covered])
   end
 
   defp format_total(info) do
     totals   = Enum.reduce(info, %Count{}, fn([_, count], acc) -> append(count, acc) end)
-    coverage = get_coverage(totals)
-    print_string("[TOTAL] ~5.1f%", [coverage])
+    coverage = ExCoveralls.Stats.get_coverage(totals.relevant, totals.covered)
+    print_string("[TOTAL] ~5w%", [coverage])
   end
 
   defp append(a, b) do
@@ -165,21 +163,6 @@ defmodule ExCoveralls.Local do
     }
   end
 
-  defp get_coverage(count) do
-    case count.relevant do
-      0 -> default_coverage_value()
-      _ -> (count.covered / count.relevant) * 100
-    end
-  end
-
-  defp default_coverage_value do
-    options = ExCoveralls.Settings.get_coverage_options
-    case Map.fetch(options, "treat_no_relevant_lines_as_covered") do
-      {:ok, false} -> 0.0
-      _            -> 100.0
-    end
-  end
-
   @doc """
   Calculate count information from the coverage stats.
   """
diff --git a/lib/excoveralls/settings.ex b/lib/excoveralls/settings.ex
index c30f26e..46a54ae 100644
--- a/lib/excoveralls/settings.ex
+++ b/lib/excoveralls/settings.ex
@@ -30,12 +30,12 @@ defmodule ExCoveralls.Settings do
   Get default coverage value for lines marked as not relevant.
   """
   def default_coverage_value do
-    case Map.fetch(get_coverage_options(), "treat_no_relevant_lines_as_covered") do
-      {:ok, true} -> 100.0
-      _           -> 0.0
-    end
+    get_coverage_options() |> default_coverage_value()
   end
 
+  def default_coverage_value(%{"treat_no_relevant_lines_as_covered" => true}), do: 100.0
+  def default_coverage_value(_), do: 0.0
+
   @doc """
   Get terminal output options from the json file.
   """
diff --git a/lib/excoveralls/stats.ex b/lib/excoveralls/stats.ex
index 75e0b20..eb504ee 100644
--- a/lib/excoveralls/stats.ex
+++ b/lib/excoveralls/stats.ex
@@ -198,16 +198,17 @@ defmodule ExCoveralls.Stats do
     {s+sloc, h+hits, m+misses}
   end
 
-  defp get_coverage(relevant, covered) do
-    value = case relevant do
-      0 -> Settings.default_coverage_value
-      _ -> (covered / relevant) * 100
+  def get_coverage(relevant, covered) do
+    coverage_options = Settings.get_coverage_options()
+
+    approximate_fn = case coverage_options do
+      %{"floor_coverage" => false} -> &Float.round(&1, 1)
+      _ -> &Float.floor(&1, 1)
     end
 
-    if value == trunc(value) do
-      trunc(value)
-    else
-      Float.round(value, 1)
+    case relevant do
+      0 -> Settings.default_coverage_value(coverage_options)
+      _ -> approximate_fn.((covered / relevant) * 100)
     end
   end
 
@@ -226,7 +227,7 @@ defmodule ExCoveralls.Stats do
   Exit the process with a status of 1 if coverage is below the minimum.
   """
   def ensure_minimum_coverage(stats) do
-    coverage_options = ExCoveralls.Settings.get_coverage_options
+    coverage_options = Settings.get_coverage_options
     minimum_coverage = coverage_options["minimum_coverage"] || 0
     if minimum_coverage > 0, do: check_coverage_threshold(stats, minimum_coverage)
   end


Test patch:
diff --git a/test/html_test.exs b/test/html_test.exs
index 571ad24..c78ae79 100644
--- a/test/html_test.exs
+++ b/test/html_test.exs
@@ -5,7 +5,7 @@ defmodule ExCoveralls.HtmlTest do
   alias ExCoveralls.Html
 
   @file_name "excoveralls.html"
-  @file_size 20375
+  @file_size 20381
   @test_output_dir "cover_test/"
   @test_template_path "lib/templates/html/htmlcov/"
 
@@ -35,7 +35,7 @@ defmodule ExCoveralls.HtmlTest do
         File.rm!(path)
         File.rmdir!(@test_output_dir)
       end
-      
+
       ExCoveralls.ConfServer.clear()
     end
 
@@ -78,7 +78,7 @@ defmodule ExCoveralls.HtmlTest do
     output = capture_io(fn ->
       assert catch_exit(Html.execute(@source_info)) == {:shutdown, 1}
     end)
-    assert String.contains?(output, "FAILED: Expected minimum coverage of 100%, got 50%.")
+    assert String.contains?(output, "FAILED: Expected minimum coverage of 100%, got 50.0%.")
   end
 
   test_with_mock "Exit status code is 0 when actual coverage reaches the minimum",
diff --git a/test/local_test.exs b/test/local_test.exs
index 8d13f63..27cfc7f 100644
--- a/test/local_test.exs
+++ b/test/local_test.exs
@@ -37,7 +37,7 @@ defmodule ExCoveralls.LocalTest do
       "\e[31mdefmodule Test do\e[m\n\e[32m  def test do\e[m\n" <>
       "  end\n" <>
       "end"
-      
+
   setup do
     ExCoveralls.ConfServer.clear()
     on_exit(fn -> ExCoveralls.ConfServer.clear() end)
@@ -75,11 +75,11 @@ defmodule ExCoveralls.LocalTest do
   end
 
   test "Empty (no relevant lines) file is calculated as 0.0%" do
-    assert String.contains?(Local.coverage(@empty_source_info), "[TOTAL] 100.0%")
+    assert String.contains?(Local.coverage(@empty_source_info), "[TOTAL]   0.0%")
   end
 
   test_with_mock "Empty (no relevant lines) file with treat_no_relevant_lines_as_covered=true option is calculated as 100.0%",
-    ExCoveralls.Settings, [
+    ExCoveralls.Settings, [:passthrough], [
       get_coverage_options: fn -> %{"treat_no_relevant_lines_as_covered" => true} end,
       get_file_col_width: fn -> 40 end,
       get_print_files: fn -> true end
@@ -88,7 +88,7 @@ defmodule ExCoveralls.LocalTest do
   end
 
   test_with_mock "Empty (no relevant lines) file with treat_no_relevant_lines_as_covered=false option is calculated as 0.0%",
-      ExCoveralls.Settings, [
+      ExCoveralls.Settings, [:passthrough], [
         get_coverage_options: fn -> %{"treat_no_relevant_lines_as_covered" => false} end,
         get_file_col_width: fn -> 40 end,
         get_print_files: fn -> true end
@@ -97,7 +97,6 @@ defmodule ExCoveralls.LocalTest do
   end
 
   test_with_mock "Exit status code is 1 when actual coverage does not reach the minimum",
-
       ExCoveralls.Settings, [
         get_coverage_options: fn -> %{"minimum_coverage" => 100} end,
         get_file_col_width: fn -> 40 end,
@@ -107,7 +106,7 @@ defmodule ExCoveralls.LocalTest do
     output = capture_io(fn ->
       assert catch_exit(Local.execute(@source_info)) == {:shutdown, 1}
     end)
-    assert String.contains?(output, "FAILED: Expected minimum coverage of 100%, got 50%.")
+    assert String.contains?(output, "FAILED: Expected minimum coverage of 100%, got 50.0%.")
   end
 
   test_with_mock "Exit status code is 0 when actual coverage reaches the minimum",
diff --git a/test/poster_test.exs b/test/poster_test.exs
index e7d9f8f..a808fd5 100644
--- a/test/poster_test.exs
+++ b/test/poster_test.exs
@@ -1,19 +1,19 @@
 defmodule PosterTest do
   use ExUnit.Case
   import ExUnit.CaptureIO
-  
+
   setup do
     bypass = Bypass.open()
     %{bypass: bypass, endpoint: "http://localhost:#{bypass.port}"}
   end
-  
+
   test "successfully posting JSON", %{bypass: bypass, endpoint: endpoint} do
     Bypass.expect(bypass, fn conn ->
       assert conn.method == "POST"
       assert {"host", "localhost"} in conn.req_headers
       Plug.Conn.resp(conn, 200, "")
     end)
-    
+
     assert capture_io(fn ->
       ExCoveralls.Poster.execute("{}", endpoint: endpoint)
     end) =~ "Successfully uploaded"
@@ -21,7 +21,7 @@ defmodule PosterTest do
 
   test "post JSON fails", %{bypass: bypass, endpoint: endpoint} do
     Bypass.down(bypass)
-    
+
     assert_raise ExCoveralls.ReportUploadError, fn ->
       ExCoveralls.Poster.execute("{}", endpoint: endpoint)
     end
@@ -32,7 +32,7 @@ defmodule PosterTest do
       assert conn.method == "POST"
       Plug.Conn.resp(conn, 500, "")
     end)
-      
+
     assert capture_io(fn ->
       assert ExCoveralls.Poster.execute("{}", endpoint: endpoint) == :ok
     end) =~ ~r/internal server error/
@@ -43,7 +43,7 @@ defmodule PosterTest do
       assert conn.method == "POST"
       Plug.Conn.resp(conn, 405, "")
     end)
-  
+
     assert capture_io(fn ->
       assert ExCoveralls.Poster.execute("{}", endpoint: endpoint) == :ok
     end) =~ ~r/maintenance/
@@ -58,7 +58,7 @@ defmodule PosterTest do
 
     Bypass.expect_once(bypass, "POST", "/api/v1/jobs", fn conn ->
       conn
-      |> Plug.Conn.put_resp_header("location", Path.join(endpoint, "redirected") |> IO.inspect())
+      |> Plug.Conn.put_resp_header("location", Path.join(endpoint, "redirected"))
       |> Plug.Conn.resp(302, "")
     end)
 
diff --git a/test/stats_test.exs b/test/stats_test.exs
index e6d6867..5570c2c 100644
--- a/test/stats_test.exs
+++ b/test/stats_test.exs
@@ -125,7 +125,7 @@ defmodule ExCoveralls.StatsTest do
   end
 
   test_with_mock "Empty (no relevant lines) file with treat_no_relevant_lines_as_covered option is calculated as 100.0%",
-    ExCoveralls.Settings, [default_coverage_value: fn -> 100 end] do
+    ExCoveralls.Settings, [:passthrough], [default_coverage_value: fn _ -> 100 end] do
 
     results = Stats.source(@empty_source_info)
     assert(results.coverage == 100)
@@ -133,7 +133,7 @@ defmodule ExCoveralls.StatsTest do
 
   test "coverage stats are rounded to one decimal place" do
     results = Stats.source(@fractional_source_info)
-    assert(results.coverage == 66.7)
+    assert(results.coverage == 66.6)
   end
 
   describe "update_stats/2" do