Skip to content

Feature/per directory coverage#109

Draft
dothebart wants to merge 4 commits into
aconrad:masterfrom
dothebart:feature/per_directory_coverage
Draft

Feature/per directory coverage#109
dothebart wants to merge 4 commits into
aconrad:masterfrom
dothebart:feature/per_directory_coverage

Conversation

@dothebart

@dothebart dothebart commented Dec 3, 2020

Copy link
Copy Markdown

as discussed in #108 I need another formatter that groups sources by directory and other criteria.

I would make this a draft pr since this is still very hacky, but for some reason github won't let me. [edit it created a draft. hm.]

the yml for grouping looks like this:

Cache:
  coverage/arangod/Cache
Cluster:
  coverage/arangod/Cluster
  coverage/arangod/Network
  coverage/arangod/Sharding

Tabular per directory output achieved by this (using the regular return):

Directories                                         Stmts    Miss    Cover  Missing
------------------------------------------------  -------  ------  -------  ---------
coverage/arangod/Cluster                            14254    3752  73.6776
coverage/arangod/ClusterEngine                        889     275  69.0664
coverage/arangod/Network                              689     169  75.4717

Wild print of per topic collected percentages:

Cluster - +76.04% %

@aconrad

aconrad commented Dec 3, 2020

Copy link
Copy Markdown
Owner

Hi @dothebart, thanks for your pull request!

I took a quick glance at your changes and I think I better understand what you are trying to do; although having a screenshot of the output in the PR description would definitely help. I also acknowledge that you said that it was still a draft.

From my point of view, I worry that this new reporter is a little too specific to your use case and wonder if it should be part of pycobertura. I tried to design pycobertura in a way that allows extensibility by creating custom reporter like you did. I don't have a lot of experience extending the CLI portion of pycobertura, but wouldn't it be possible for you to keep the implementation of a new reporter in its own repository? Say pycobertura-directory-reporter-plugin and then use that to extend the behavior of pycobertura?

I would be happy to take changes to pycobertura to accommodate its extensibility, however. Again, without much experience extending pycobertura myself, there's likely room to make it easier on users. For example, I could see us introduce a way to register new formatters for the command line. Formatters are currently registered in a dictionary {"<reporter_name>": <reporter class>} (like you did in this PR) and there might be better ways to modify that object without monkey patching a global variable.

What do you think?

@dothebart

dothebart commented Dec 3, 2020

Copy link
Copy Markdown
Author

Added the sample output.

In general you see that it sort of plugs nicely into the reporters interface, so having a CLI-Reporter plugin would probably exactly fit my needs. It would probably then be 2 different formatters.

The code here still all a bit rough around the corners, since I was a bit under time pressure, and working with the 20Megs xml took several minutes on each try ;-) So I didn't add tests or such yet.

So I thought we can use this as a discussion start rather than theoretically talking about what could or should be.
I found nice about the CLI that I didn't have to do much to get my things done, it mostly doesn't even interfere
with the rest of the code aside of deriving a readily existing base class.

@aconrad

aconrad commented Dec 3, 2020

Copy link
Copy Markdown
Owner

Absolutely! Totally open for discussion. I'm sharing preliminary thoughts about what crossed my mind at a first glance.

And if you see performance improvements along the way since you are dealing with a large XML file, we welcome change here, too! 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants