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

How to modularize – structure suggestions #215

Closed
latk opened this Issue Feb 14, 2018 · 5 comments

Comments

Projects
None yet
2 participants
@latk
Member

latk commented Feb 14, 2018

When splitting the gcovr script into multiple smaller modules, how should we divide them? In this issue, we can discuss design decisions before settling on a particular choice.

The modularization that was done on the dev branch uses this structure:

Module Contents
gcovr namespace
gcovr.args supplies parse_arguments() function
gcovr.data CoverageData, and processing of gcov data files
gcovr.driver supplies main() function, and a main_() entrypoint
gcovr.path path utilities
gcovr.version __version__ declaration and version_str() function
gcovr.prints namespace
gcovr.prints.html print_html_report() and supporting functions
gcovr.prints.xml print_xml_report() function
gcovr.prints.text print_text_report() and print_summary() functions

I'd prefer a slightly different design:

  • get rid of the gcovr.prints namespace
  • possibly, separate text and summary output
  • put argument parsing, main(), and entrypoint into gcovr.app
  • (later) add gcovr.logging which formats verbose diagnostics and error messages
  • put gcov invocation and parsing into gcovr.gcov

So I'd probably do:

Module Contents
gcovr namespace
gcovr.app parse_arguments(), main(), entrypoint
gcovr.coverage CoverageData data model that is generated by gcovr.gcov and consumed by the output formats
gcovr.gcov invoking gcov and parsing gcov files
gcovr.utils utilities, including path handling and filters
gcovr.version declares __version__ and nothing else, can be exec()d by setup.py
gcovr.html HTML report
gcovr.xml XML report
gcovr.txt text report
gcovr.summary summary report

One thing that bothers me with both of these approaches is the rather generic module names like html. Would html_output be a better name?

Are there any alternative suggestions? Any weaknesses with the proposed structures?

Once we've decided on a design, implementing the modularization is going to be fairly easy. Further refactoring can come afterwards.

@mayeut

This comment has been minimized.

Contributor

mayeut commented Feb 14, 2018

I'd probably go with a slightly different design

Module Contents
gcovr namespace
gcovr.cli parse_arguments()main(), entrypoint
gcovr.driver public API called by gcovr.cli. Can be imported by external modules that wish to get gcovr service programmatically.
gcovr.coverage CoverageData data model that is generated by gcovr.gcov and consumed by the output formats
gcovr.gcov invoking gcov and parsing gcov files
gcovr.utils utilities, including path handling and filters
gcovr.version declares __version__ and nothing else, can be exec()d by setup.py
gcovr.generator manage generator plugins
gcovr.html_generator HTML report generator
gcovr.cobertura_xml_generator XML report generator
gcovr.txt_generator text report generator
gcovr.summary_generator summary report generator

This would allow 3rd party generators if needed. One might even think to get a gcovr.parser parser plugin manager, with gcovr.gcov (or gcovr.gcov_parser) being one of them.

Maybe add a namespace gcovr.generators for all gcovr.*generator:

Module Contents
gcovr.generators namespace
gcovr.generators.generator base class for plugins
gcovr.generators.manager manage generator plugins
gcovr.generators.html HTML report generator
gcovr.generators.cobertura_xml XML report generator
gcovr.generators.text text report generator
gcovr.generators.summary summary report generator
@latk

This comment has been minimized.

Member

latk commented Feb 15, 2018

Regarding plugins and a public API: that is a direction I am open to, but it's too early to implement any of that as part of modularization. Plugins also add many questions of their own, such as “how do we parse command line arguments”. However, I do want to find a structure that allows us to introduce plugins etc. without a second large reorganization. Plugins can be implemented through setuptools entrypoints which does not require namespace packages.

“Generator” is a perfect name for the output formats. I think names like gcovr.html_generator are very good. Good point that the XML format specifically is the Cobertura format. Since output formats are essentially procedural (a single print_xml_report() function and so on), there isn't currently a need for a generator base class.

I'm not sure what the difference between gcovr.cli and gcovr.driver would be. If we add a programmatic interface, it would likely be through the gcovr module itself. Right now, there is very little driver-level behaviour. So I wouldn't know what to put into that module.

@mayeut

This comment has been minimized.

Contributor

mayeut commented Feb 15, 2018

Regarding plugins and a public API: that is a direction I am open to, but it's too early to implement any of that as part of modularization. Plugins also add many questions of their own, such as “how do we parse command line arguments”. However, I do want to find a structure that allows us to introduce plugins etc. without a second large reorganization. Plugins can be implemented through setuptools entrypoints which does not require namespace packages.

What I'm suggesting is of course a direction and I don't expect to get a stable API or Plugin definition for the next version. All I'm saying is that - and as you said yourself - if it's a direction worth taking, it shall be taken under consideration when defining the new design not to redo things all over again. The question you raised regarding plugin & command-line arguments was one that came to my mind when I wrote this. It adds complexity and does not need to be addressed in a first step.

I'm not sure what the difference between gcovr.cli and gcovr.driver would be. If we add a programmatic interface, it would likely be through the gcovr module itself. Right now, there is very little driver-level behaviour. So I wouldn't know what to put into that module.

if gcovr stays a namespace only, then I would create a gcovr.driver but what you're suggesting also makes sense.

What I had in mind was gcovr.cli handles all command-line parser creation/parsing and returns options as expected by the public programmatic API exposed in gcovr.driver.
Let's imagine that the public API only consist of the run method

def run(root='.', filter=None, use_gcov_files=False):

Then the gcovr.cli could have a parse method that can be called like:

root, filter, use_gcov_files = gcovr.cli.parseargs()
gcovr.driver.run(root=root, filter=filter, use_gcov_files=use_gcov_files)

Of course, the logic in gcovr.cli could be implemented instead in gcovr.__main__ module only if there's no need to expose parsing to external integrators.

@latk

This comment has been minimized.

Member

latk commented Feb 18, 2018

I forgot about gcovr.__main__, keeping the entrypoint and CLI stuff there seems most sensible for now.

I'll try to implement modularization today.

@latk latk referenced this issue Feb 18, 2018

Merged

Modularization #225

@latk

This comment has been minimized.

Member

latk commented Feb 19, 2018

Implemented in #225.

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