Save my from myself: Code coverage misses lines that return a literal

Chat / Discussions
,
1

It’s been discussed a few times that code coverage tools do not see lines where a literal value are not flagged as executable lines. To wit:

I’m frustrated by this problem and I’m attempting to fix it in a library I’m building. (See https://github.com/elixir-git/xgit/pull/122.)

I’ll start by saying I don’t like my fix, but I like even less the notion that I have to sacrifice insight into how well my library is covered.

I work that I’m doing in my day job, I have actually seen bugs get masked because we didn’t know the code was poorly covered.

I’m hoping that somebody can point me to a “better” solution that is less hacky, but doesn’t involve compromising on meaningful code coverage.

4 Likes
2

Haven’t looked at your code yet. What is it that you don’t like about it currently?

1 Like
3

What I don’t like is that I use a macro to translate a literal response into something more complicated in the test environment. For example, what was:

def valid?(0o040000), do: true

became:

def valid?(0o040000), do: cover(true)

In the test environment, this evaluates to:

defmacro cover(false = x) do
  quote do
    inspect(unquote(x))
    unquote(x)
  end
end

(There are other variations depending on the value passed to cover, but you get the point.)

Not ideal, but it was the result of a lot of trial-and-error to make the compiler, Credo, and dialyzer all happy and still make the original valid?/1 function (and dozens more like it) show up as executable.

Ultimately, I’d like to see the compiler change so that it is inherently marked as covered. But until / unless that happens, I’m looking for a less messy way to accomplish the goal of more accurate code coverage.

4

Seems you got quite far though. I didn’t even know defmacro cover(false = x) do is valid Elixir!

Recommendation: first try and achieve the goals you want. Then you should probably make an announcement to the community and ask for code reviews.

5

Why not?

What does make you think it were different from def foo(%Foo{} = foo) do … end?

1 Like
6

I would assume that defmacro cover(false) do is the way to go, same as when pattern-matching function heads assert on specific values.

7

But they use the bound x in the body.

This is some heavy lifting to avoid problems in the coverage tools. So if they were just leaving off the binding of x and would use false literally, they might finaly have the same result of literals not beeing visible to the coverage tool or might cause warnings of unused variables.

1 Like
8

I see. That’s why I said he actually got quite ahead. I would not have figured this out immediately. :slight_smile:

9

Thanks for the props. So, in the process, I learned that pattern matching works with macros. (I hadn’t expected that, either.)

Basically, the cover macro is an elaborate trick to prevent the compiler from inlining the literal value, which I think is the root cause for the lack of coverage.

So, to @dimitarvp’s comment, when I first posted this a couple of weeks ago, it was initially this was a call for suggestions. I’ve since merged the PR in question, so we could now consider it a call for reviews.

To @NobbZ’s comment:

This is some heavy lifting to avoid problems in the coverage tools.

Thanks. As I said before, I wish there were a cleaner solution. If there is, I’d love to hear about it. If not, consider this a plea for help from the tool providers. I suspect this is an issue in the compiler itself, but it might be something in excoveralls.

2 Likes