parroty / excoveralls

Coverage report tool for Elixir with coveralls.io integration.
MIT License
820 stars 178 forks source link

Cobertura now handles defprotocol and defimpl definitions #306

Closed gorghoa closed 1 year ago

gorghoa commented 1 year ago

A protocol (defprotocol) or implementation (defimpl) definition covered during the tests triggers an error when using the cobertura formatter.

This PR fixes it.

``` ** (MatchError) no match of right hand side value: nil (excoveralls 0.16.0) lib/excoveralls/cobertura.ex:168: ExCoveralls.Cobertura.module_name/1 ``` ``` (excoveralls 0.16.0) lib/excoveralls/cobertura.ex:99: anonymous fn/3 in ExCoveralls.Cobertura.generate_packages/2 (elixir 1.14.2) lib/enum.ex:2468: Enum."-reduce/3-lists^foldl/2-0-"/3 (excoveralls 0.16.0) lib/excoveralls/cobertura.ex:97: ExCoveralls.Cobertura.generate_packages/2 (excoveralls 0.16.0) lib/excoveralls/cobertura.ex:74: ExCoveralls.Cobertura.generate_xml/2 (excoveralls 0.16.0) lib/excoveralls/cobertura.ex:16: ExCoveralls.Cobertura.execute/2 (elixir 1.14.2) lib/enum.ex:975: Enum."-each/2-lists^foreach/1-0-"/2 (excoveralls 0.16.0) lib/excoveralls.ex:65: ExCoveralls.execute/3 (mix 1.14.2) lib/mix/tasks/test.ex:549: Mix.Tasks.Test.do_run/3 (mix 1.14.2) lib/mix/task.ex:421: anonymous fn/3 in Mix.Task.run_task/4 (excoveralls 0.16.0) lib/mix/tasks.ex:54: Mix.Tasks.Coveralls.do_run/2 (mix 1.14.2) lib/mix/task.ex:421: anonymous fn/3 in Mix.Task.run_task/4 (mix 1.14.2) lib/mix/cli.ex:84: Mix.CLI.run_task/2 (elixir 1.14.2) lib/code.ex:1260: Code.require_file/2 ```

ping @albertored for review! :pray:

albertored commented 1 year ago

Hi @gorghoa ! Thanks for the PR, I've just found the same bug today in one of my projects. Just a small comment on your fix and for me it is ready to be merged (@parroty )

parroty commented 1 year ago

Hi. Thank you for the PR and comments.

just a small comment on your fix and for me it is ready to be merged

I just wanted to confirm small comment on your fix part status (it's fixed and ready for merge?). I don't have additional comments, but wanted to confirm before hitting merge button 🙇 .

albertored commented 1 year ago

@parroty yes, it's ready for merge

parroty commented 1 year ago

Thanks!

gorghoa commented 1 year ago

Thank you @albertored and @parroty, have a great day!