How to have excoveralls consider total testing coverage from all apps in umbrella project?

Questions / Help
, ,
1

Background

I am using excoveralls in an umbrella project with several children.

I have set up the children to use Excoveralls like this:

For normal Elixir applications:

 def project do
    [
      app: :app1,
      version: "3.1.0",
      build_path: "../../_build",
      config_path: "../../config/config.exs",
      deps_path: "../../deps",
      lockfile: "../../mix.lock",
      elixir: "~> 1.16",
      start_permanent: Mix.env() == :prod,
      deps: deps(),
      test_coverage: [tool: ExCoveralls],
      preferred_cli_env: preferred_cli_env()
    ]
  end

And for a Phoenix child, the configuration is slightly different:

def project do
    [
      app: :phoenix_app_1,
      version: "2.2.1",
      build_path: "../../_build",
      config_path: "../../config/config.exs",
      deps_path: "../../deps",
      lockfile: "../../mix.lock",
      elixir: "~> 1.16",
      elixirc_paths: elixirc_paths(Mix.env()),
      compilers: Mix.compilers(),
      start_permanent: Mix.env() == :prod,
      deps: deps(),
      test_coverage: [tool: ExCoveralls],
      preferred_cli_env: preferred_cli_env()
    ]
  end

 defp deps do
    [
      # Phoenix
      # ....

      # I dont know why I have to explicity include excoveralls here
      # but if I dont `mix coveralls` wont work for Phoenix child apps
      {:excoveralls, "~> 0.18", only: :test},
    ]
  end

For both types of apps the preferred_cli_env set to this:

defp preferred_cli_env,
    do: [
      coveralls: :test,
      "coveralls.detail": :test,
      "coveralls.post": :test,
      "coveralls.html": :test,
      "coveralls.github": :test
    ]

Problem

When I run mix coveralls it runs all tests from each child app sequentially, due to my customized mix test alias:

defp aliases do
    child_tests =
      Path.wildcard("apps/*")
      |> Enum.map(&String.replace(&1, "apps/", ""))
      |> Enum.map(fn app -> "cmd --app #{app} mix test --color" end)

    [test: child_tests]
  end

So what ends up happening, is that coveralls will run the coverage for each app, and then only present me with the coverage for the last app run.

This is a problem. I don’t want the coverage of only the last child app, I want the coverage of all child apps, inside the project, so i know the overall test coverage of the project.

Questions

  • Is this possible to do with coveralls?
  • If so, how?
2

Are you running the mix coveralls command with the --umbrella option?
Also, what’s the purpose of your custom test alias?

3

yes

The result is the following:

Randomized with seed 918510
----------------
COV    FILE                                        LINES RELEVANT   MISSED
  0.0% lib/web_interface.ex                          111        2        2
 91.6% lib/web_interface/application.ex               64       12        1
  0.0% lib/web_interface/components/core_compon      907      190      190
  0.0% lib/web_interface/components/layouts.ex         7        0        0
  0.0% lib/web_interface/components/operation_p       39       10       10
  0.0% lib/web_interface/controllers/error_html       19        1        1
  0.0% lib/web_interface/controllers/error_json       15        1        1
  0.0% lib/web_interface/controllers/page_contr       13        3        3
  0.0% lib/web_interface/controllers/page_html.        5        0        0
 40.0% lib/web_interface/desktop/menu_bar.ex          46        5        3
100.0% lib/web_interface/desktop/window_utils.e       33        6        0
  0.0% lib/web_interface/endpoint.ex                  48        0        0
  0.0% lib/web_interface/live/activate_live.ex       232       72       72
  0.0% lib/web_interface/live/deactivate_live.e      206       63       63
  0.0% lib/web_interface/live/login_live.ex           99       22       22
  0.0% lib/web_interface/live/logout_live.ex          40        9        9
  0.0% lib/web_interface/live/profile_live.ex         21        4        4
100.0% lib/web_interface/persistence.ex               24        5        0
  0.0% lib/web_interface/persistence/button.ex        32        8        8
 92.3% lib/web_interface/persistence/strategy.e       45       13        1
 96.0% lib/web_interface/persistence/syndicate.      152       50        2
 87.5% lib/web_interface/persistence/user.ex          33        8        1
  0.0% lib/web_interface/router.ex                    35        8        8
 80.0% lib/web_interface/telemetry.ex                 71        5        1
  0.0% test/support/conn_case.ex                      37        2        2
[TOTAL]  19.0%
----------------
----------------
COV    FILE                                        LINES RELEVANT   MISSED

[TOTAL]   0.0%
----------------

As you can see, I am only presented with the coverage table for the last child app.
After that, the total is 0.0% for the umbrella project (which is clearly incorrect).

Some of the children have dependencies on other children. Because mix test runs all children’s tests in parallel when invoked inside an umbrella project, this means that my test suite would fail constantly, due to dependencies, race conditions and the typical “This app is already launched” error.

The fix for this was to run the children’s tests sequentially. To force this I had to create an alias, since mix test does not support this functionality by default for umbrella projects.

4

Tbh that sounds like you’re manually starting applications, which you generally shouldn’t need to be doing in the first place. If you have your dependencies setup correctly between your umbrella applications (using {:app, in_umbrella: true}) all necessary applications should start up no matter what mix task you run and if it’s for the whole umbrella or just a single app within.

2 Likes
5

Lets have an example. Imagine I have a child application called “auction_house” which is the API for an auction house of items.

I want to do a test that mocks the 3rd party, so I end up using bypass and the code looks like this:

defmodule AuctionHouseTest do
  @moduledoc false

  use ExUnit.Case, async: false

  alias AuctionHouse
  alias AuctionHouse.Runtime.Server # a GenServer
  alias Bypass

  @test_port 8082

  setup do
    {:ok, pid} = start_supervised(Server)
    bypass = Bypass.open(port: @test_port)
    %{bypass: bypass, server: pid}
  end

  describe "place_oder/1" do
    test "returns {:ok, placed_order} if order was placed correctly", %{
      bypass: bypass,
      server: server
    } do
      # Arrange
      Bypass.expect(bypass, "POST", "/v1/profile/orders", fn conn ->
        response = %{
          "payload" => "some really big payload"
        }

        Plug.Conn.resp(conn, 200, Jason.encode!(response))
      end)

      order = Order.new(bananas, 10, "euros")

      login_info = %Authorization{token: "token"}
      :sys.replace_state(server, fn state -> Map.put(state, :authorization, login_info) end)

      assert AuctionHouse.place_order(order) == {:ok, PlacedOrder.new(%{"item_id" => order.item_id, "order_id" => "5ee71a2604d55c0a5cbdc3c2"})}

    end
  end
end

In this example I am creating a GenServer and acting upon it. As this is an integration test, I dont have any other way of doing it.

Now, if another application, say “WebInterface” launches child app “B”, which in turn launches “AuctionHouse” (because it is a dependency), you can see we have a problem, as my GenServer for AuctionHouse is a named one.

The issue here is so hard to track, that not even using mix test --trace helps:

Randomized with seed 709532
==> store
13:08:57.292 request_id=F6mYQKIYvID6w48AAAGJ [info] GET /
13:08:57.339 request_id=F6mYQKIYvID6w48AAAGJ [debug] Processing with WebInterface.PageController.home/2
  Parameters: %{"k" => "6G3ZX53TAZPH72RRL6WTKYDA7J3QFV5CFWOQ362LI24ARBFDILAA"}
  Pipelines: [:browser]
13:08:57.372 request_id=F6mYQKIYvID6w48AAAGJ [info] Sent 302 in 74ms
13:08:57.372 request_id=F6mYQKcjXIBxu0AAAAHJ [info] GET /login
13:08:57.372 request_id=F6mYQKcjXIBxu0AAAAHJ [debug] Processing with WebInterface.LoginLive.Elixir.WebInterface.LoginLive/2
  Parameters: %{}
  Pipelines: [:browser]

MarketManager.Store.FileSystemTest [test/unit/file_system_test.exs]
  * test get_login_data/1 returns nil if read succeeded but authorization token is null [L#358]** (EXIT from #PID<0.98.0>) killed

[0112/130858.448:ERROR:window_impl.cc(120)] Failed to unregister class Chrome_WidgetWin_0. Error = 0

As you can see, in this sample we have output generated for both the “Store” child app and the “WebInterface” child app, which are clumped together (because they are running in parallel).

I know the WebInterface child app is the one to fail and this is visible in the last logged line:

[0112/130858.448:ERROR:window_impl.cc(120)] Failed to unregister class Chrome_WidgetWin_0. Error = 0

Because the “WebInterface” child app is the only app using Elixir Desktop, and this is an error specific to said library I can identify this issue. But this is far from “working properly”.

If you know of a better way to fix this, please do let me know.

6

Fair. You got global state in your application and your tests show you that this is not necessarily the best design.

My general suggestion for global state is refactoring those singleton processes (with a fixed name) to processes, which can be given a name. That way you can start multiple concurrent instances of those processes with different names.

Given that you can start a separate set of processes per test. You can use dependency injection (of various forms) to inject the name (or names) to be used by your business logic when interacting with those processes.

Production can use a single name (or names), which effectively gives you singleton behaviour in production.

The issues you run into are why I generally suggest that test should either not depend on state present on processes started by applications or at least that state should be keyed to the running test. That approach is essentially how the ecto sandbox or mox allow you to run tests concurrently.

While this is talking about processes the same can equally be applied to other global resources.

2 Likes
7

Correct me if I am wrong, but aren’t named genservers a rather common pattern in Elixir?

# Start the server and register it locally with name MyStack
{:ok, _} = GenServer.start_link(Stack, [:hello], name: MyStack)

# Now messages can be sent directly to MyStack
GenServer.call(MyStack, :pop)
#=> :hello

source: GenServer — Elixir v1.12.3

Or the more common form:

{:ok, _} = GenServer.start_link(Stack, [:hello], name: __MODULE__)

I suppose I could give that try. I will let you know if a refactor is viable enough for me to attempt it.
Thanks for the suggestion!

8

Named processes are a common pattern for sure. The issue is also not the named process by itself. The issue is the hardcoded dependencies on those names (and their backing process) into your codebase, which means tests can no longer opt out of using that named process in favor of another instance.

As soon as you look into applications we would generally consider to be libraries you see the pattern of keying a whole tree of processes to a given name all the time. E.g. in ecto processes are keyed to the repo name, in broadway the processes are keyed to the name of the broadway pipeline. E.g. see here how the MyBroadway name determines the names of all child processes: Broadway — Broadway v1.0.7

1 Like
9

Alright, I am now attempting this solution. I do have a question though. Imagine this code, the public API of a Genserver:

  # start_link for normal usage
  def start_link,
    do: GenServer.start_link(__MODULE__, nil, name: __MODULE__)

  # start_link for keyed topology
  def start_link(suffix, count), do: GenServer.start_link(__MODULE__, nil, name: :"#{__MODULE__}_#{suffix}_#{count}")
 
   # get shop orders
  def get_all_orders(item_name),
    do: GenServer.call(?????, {:get_all_orders, item_name}, @genserver_timeout)

You will see an immediate problem: get_all_orders does not know the which GenServer to call.

With __MODULE__ as a name, this was not a problem, but now since the name can be anything, it is.

I am under the impression I now need to use a Registry here, but I am not sure and I believe this would greatly increase the complexity of the solution.

Furthermore, because this Registry would be a Singleton as well, I would be incurring in the same problem as before.

What is the usual recommendation here?

10

You either make it a parameter on the function e.g. get_all_orders(server \\ __MODULE__, item_name) or you can look into other forms of dependency injection like e.g. using Mox to switch out the selection of a genserver to be called.

11 
get_all_orders(server \\ __MODULE__, item_name)

Just out of curiosity, aren’t optional parameters usually last in elixir?

I did consider this, but I am uncomfortable with passing with changing the public api because of tests, even though the parameter is optional.

Mox, however, will replace my genservers, and actually do want to use them, so it is not an option I can use.

12

Usually, but I tend to mirror the order of parameters GenServer.call has as well. It’s imo the more reasonable order if you happen to pass both the server and item.

2 Likes