alfert / coverex

Coverage Reports for Elixir
Other
101 stars 26 forks source link

Module Ignores list #21

Closed cjimison closed 8 years ago

cjimison commented 8 years ago

It would be great if there was a way to add a list of modules to ignore from the cover report in the mix file. There are many libs out there that macro gen modules/function over basic structs. This makes cover reports look very empty because only one of the macro's functions are called.

For example we have a library that takes in a module struct and adds a bunch of serialize/deserialize macros for thrift. We only may use 2 function API out of 20 possible but the cover report shows only 10% coverage.

Thanks!

-Chris

alfert commented 8 years ago

Your experience shows that coverex works correctly, that is good: Using 2 of 20 methods relates to 10% coverage. Nevertheless, macros generating functions play not nice with coverage reports, in particular for the HTML reports, this is also my experience. Generated modules even more.

What is your request about: Ignoring those modules completely, only for summaries or even making this configurable?

Please be aware that coverage reports always must be interpreted within the architectural context. Coverage rates of 10% may be ok in certain situations, where in other places a coverage of only 80% is not considered as appropriate. Any simple metrics ("all modules have at least 70% coverage") are silly and do not give any proper insight in the system. Ignoring modules means essentially that you state that these modules and their coverage during tests are completely irrelevant to asses the quality of tests and the system under consideration. This is a strong statement.

cjimison commented 8 years ago

I totally agree with you. The only caveat I would give is reaching 100% coverage in a system like Erlang/Elixir is nice because it can catch runtime syntax errors that the compiler misses (like passing the incorrect number of arguments, etc). There is dialyzer, however I feel safest when the code has run at least once before moving into production :) I also see this as a practical problem that should only be used in very specific cases.

The problem we run into is stuff like our thrift support libs. Here is an example:

defmodule UserInfo do
   require ThriftStructs
   defThriftStruct name: "John", 
                    age: 27
                    sex: :male
end

info = UserInfo.new
^info = UserInfo.serialize!(info) |> UserInfo.deserialize!

These structs are super light weight because they are used to serialize/deserialize network traffic and nothing else. However another example is Amnesia:

use Amnesia

# defines a database called Database, it's basically a defmodule with some additional magic
defdatabase Database do
  deftable User
  deftable Message, [:user_id, :content], type: :bag do
    @type t :: %Message{user_id: integer, content: String.t}
  end

These macros generate a lot of helper functions that may or may not be used. There is no real code we need to worry about other than the tests that uses these modules. We have over 100+ of these types of modules in our system and when I generate a coverex report it shows them all around 5% (or none at all for macro defined modules). This makes the reports harder to read and easy to miss large drops in code coverage.

Note: The ThriftStructs libs has a whole set of unit tests to validate the gen'd code.

I would say in our current project at least 60% of the modules defined are examples like above, and being able to ignore this stuff would be very helpful to visualize the reports.

What is your request about: Ignoring those modules completely, only for summaries or even making this configurable?

All the above :) What I was hoping to do was from the Mix.exs file, read a local ignore file and pass that data into coverex so it would not add them to the summaries (and skipping would be nice for speed reasons).

This is a "with great power comes great responsibility" feature request for sure.

Thanks!

-Chris

alfert commented 8 years ago

Ok, let's go for this.

What we can do is to ask :cover about all coverage-compiles modules in the BEAM, and delete all entries from the ignore list. This reduced list is then fed back to analyzing the coverage-data. This would result in less output and only one location where the ignore list needs to be applied.

The simplest form of an ignore list would contain exactly those module names which are to be ignored, e.g.

   ignore_modules: [Database, Database.User, Database.Message]

to stick to your former example. Is that feasible?

If wildcards are required, I would opt for regexp, but then the ignore list requires strings (or regexp's) and we need to handle the Elixir module name prefix in an intelligent way. Thinking about this, I would go for the easy solution first and do wildcards later.

alfert commented 8 years ago

Please take a look the PR #22 Documentation will follow later.

cjimison commented 8 years ago

This is awesome! I just pulled the branch and the new reports are GREAT! They are no longer filled with a bunch of modules no one cares about and I can easily who has not been writing unit tests for their code :)

For us we want to be SUPER selective what goes in this list for all the reasons you have stated before, so wildcards are not needed.

Thanks again!

-Chris

alfert commented 8 years ago

available in 1.4.8 on hex.pm.