Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add coverage reporting #25

Closed
hauleth opened this issue Oct 21, 2021 · 12 comments · Fixed by #26
Closed

Add coverage reporting #25

hauleth opened this issue Oct 21, 2021 · 12 comments · Fixed by #26
Labels
good first issue Good for newcomers
Milestone

Comments

@hauleth
Copy link
Owner

hauleth commented Oct 21, 2021

No description provided.

@hauleth hauleth added the good first issue Good for newcomers label Oct 21, 2021
@marc0s
Copy link
Contributor

marc0s commented Oct 23, 2021

Hi, how would you like this to be done? Calling mix test --cover from some Github action? Or maybe using ExCoveralls?

@hauleth
Copy link
Owner Author

hauleth commented Oct 23, 2021

I was thinking about ExCoveralls or Covertool and upload them to CodeCov.io

@marc0s
Copy link
Contributor

marc0s commented Oct 23, 2021

Ok! Will have a look into it.

@marc0s
Copy link
Contributor

marc0s commented Oct 23, 2021

I have configured mix_unused to depend on ExCoveralls but I'm unable to run it because the following errors. I've done the same exact steps in another personal project without any troubles.

➜ MIX_ENV=test mix coveralls
...............................................

Finished in 2.7 seconds (0.1s async, 2.5s sync)
6 doctests, 41 tests, 0 failures

Randomized with seed 681095

** (FunctionClauseError) no function clause matching in IO.chardata_to_string/1    
    
    The following arguments were given to IO.chardata_to_string/1:
    
        # 1
        nil
    
    Attempted function clauses (showing 2 out of 2):
    
        def chardata_to_string(string) when is_binary(string)
        def chardata_to_string(list) when is_list(list)
    
    (elixir 1.12.1) lib/io.ex:607: IO.chardata_to_string/1
    (elixir 1.12.1) lib/path.ex:422: Path.dirname/1
    (excoveralls 0.14.3) lib/excoveralls/cover.ex:21: ExCoveralls.Cover.module_path/1
    (excoveralls 0.14.3) lib/excoveralls/stats.ex:54: ExCoveralls.Stats.add_counts/4
    (elixir 1.12.1) lib/enum.ex:2356: Enum."-reduce/3-lists^foldl/2-0-"/3
    (excoveralls 0.14.3) lib/excoveralls/stats.ex:28: ExCoveralls.Stats.report/1
    (excoveralls 0.14.3) lib/excoveralls.ex:47: ExCoveralls.execute/2
    (mix 1.12.1) lib/mix/tasks/test.ex:520: Mix.Tasks.Test.do_run/3
    (mix 1.12.1) lib/mix/task.ex:394: anonymous fn/3 in Mix.Task.run_task/3
    (excoveralls 0.14.3) lib/mix/tasks.ex:54: Mix.Tasks.Coveralls.do_run/2
    (mix 1.12.1) lib/mix/task.ex:394: anonymous fn/3 in Mix.Task.run_task/3
    (mix 1.12.1) lib/mix/cli.ex:84: Mix.CLI.run_task/2
    (elixir 1.12.1) lib/code.ex:1261: Code.require_file/2

Do you think that this might be related to the way mix_unused hooks itself into the compiler options or some of that stuff?

The culprit of the error is that while running mix coveralls the output of Mix.Project.config_files() returns a list like this one, without mix.es:

["/home/marc0s/Projects/Personal/mix_unused/_build/dev/.mix/compile.lock"]

What I find strange is that I'm running all mix tasks with MIX_ENV=test and the path above contains _build/dev instead.

Any hint would be appreciated.

Thanks

@hauleth
Copy link
Owner Author

hauleth commented Oct 23, 2021

So you have branch that I can look into?

@marc0s
Copy link
Contributor

marc0s commented Oct 23, 2021

@marc0s
Copy link
Contributor

marc0s commented Oct 23, 2021

For running the coverage analysis and report, run MIX_ENV=test mix coveralls

(branch updated with before missing coveralls.json file)

@hauleth
Copy link
Owner Author

hauleth commented Oct 23, 2021

It seems like bug in ExCoveralls triggered by the fact that we are using Mix.Project.in_project/3 to run some tests against other Mix projects.

@hauleth
Copy link
Owner Author

hauleth commented Oct 23, 2021

This patch to lib/excoveralls/path_reader.ex seems to fix it:

<       :error -> Mix.Project.config_files() |> Enum.find(&(&1 =~ ~r/mix.exs/)) |> Path.dirname()
---
>       :error -> File.cwd!()

And as mix must be ran from the same directory as the mix.exs file is, it should work in all cases (IIRC it will work in umbrella as well, as Mix changes directory before running actions for sub projects).

hauleth added a commit to hauleth/excoveralls that referenced this issue Oct 23, 2021
Otherwise it will fail if tests use [`in_project/3`][1] with confusing
error about the fact that `nil` cannot be used in `Path.dirname/1`.

It should work 100% of the time as Mix will work only when ran in
directory with `mix.exs` in it, and when running in umbrella it will
change directory to the sub project before running tasks.

Ref hauleth/mix_unused#25

[1]: https://hexdocs.pm/mix/Mix.Project.html#in_project/3
@hauleth
Copy link
Owner Author

hauleth commented Oct 23, 2021

@marc0s see parroty/excoveralls#271

@marc0s
Copy link
Contributor

marc0s commented Oct 23, 2021

Looks like your fix makes it work:

➜ MIX_ENV=test mix coveralls
Compiling 8 files (.ex)
Generated mix_unused app
...............................................

Finished in 2.5 seconds (0.1s async, 2.4s sync)
6 doctests, 41 tests, 0 failures

Randomized with seed 721296
----------------
COV    FILE                                        LINES RELEVANT   MISSED
 97.1% lib/mix/tasks/compile.unused.ex               189       34        1
100.0% lib/mix_unused/analyze.ex                      39       10        0
100.0% lib/mix_unused/analyzers/private.ex            29        6        0
100.0% lib/mix_unused/analyzers/unused.ex             18        3        0
100.0% lib/mix_unused/config.ex                       51        7        0
 90.0% lib/mix_unused/exports.ex                      91       20        2
 90.0% lib/mix_unused/filter.ex                      130       10        1
 92.9% lib/mix_unused/tracer.ex                       93       14        1
[TOTAL]  95.2%
----------------

@hauleth
Copy link
Owner Author

hauleth commented Oct 24, 2021

@marc0s if you want you can replace ExCoveralls with covertool (which is IMHO much simpler). This one works without any problems.

marc0s added a commit to marc0s/mix_unused that referenced this issue Oct 24, 2021
marc0s added a commit to marc0s/mix_unused that referenced this issue Oct 24, 2021
hauleth pushed a commit that referenced this issue Oct 24, 2021
@hauleth hauleth added this to the v0.4.0 milestone Oct 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants