Skip to content

Modifying coverage reporting for python files is more difficult than need be. #646

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

Open
nedbat opened this issue Feb 28, 2018 · 4 comments
Labels
enhancement New feature or request

Comments

@nedbat
Copy link
Owner

nedbat commented Feb 28, 2018

Originally reported by John Sirois (Bitbucket: jsirois, GitHub: jsirois)


N.B.: Obsoletes https://bitbucket.org/ned/coveragepy/issues/645/modifying-coverage-reporting-for-python

Currently the plugin api for providing a custom reporter is def file_reporter(filename) -> FileReporter. This API makes it hard to subclass PythonFileReporter since its constructor takes and optional coverage argument in addition to the morf to report on. In particular, the Coverage instance is not optional for the successful execution of fr.parser and fr.no_branch_lines. This can be worked around ~like so:

class MyFileReporter(PythonFileReporter):
  def __init__(self, morf, fancy_rel_filename, coverage=None):
    super(MyFileReporter, self).__init__(morf, coverage=coverage)
   self._fancy_rel_filename = fancy_rel_filename

  # What I want to customize:
  def relative_filename(self):
    return self._fancy_rel_filename

  # What I have to implement to workaround:
  @property
  def parser(self):
    """Lazily create a :class:`PythonParser`."""
    if self._parser is None:
      self._parser = PythonParser(
        filename=self.filename,
        exclude=None,  # Lost configurability here.
      )
      self._parser.parse_source()
    return self._parser

  @expensive
  def no_branch_lines(self):
    no_branch = self.parser.lines_matching(
      join_regex(DEFAULT_PARTIAL[:]),  # Lost configurability here.
      join_regex(DEFAULT_PARTIAL_ALWAYS[:])  # Lost configurability here.
    )
    return no_branch

The workaround is necessitated by the plugin lifecycle as it stands, where there is no access to the active Coverage instance FWICT. It looks like configuring plugins set up a bit of chicken/egg if Coverage passed itself into coverage_init; although, this would work in the example case since by the time MyFileReporter actually used the passed-in Coverage instance, it would be fulliy _init'ed. Perhaps the CoveragePlugin API could have def python_file_reporter(filename, coverage) -> PythonFileReporter method added or else there could be a PythonCoveragePlugin or CoveragePlugin2 API that amended the existing file_reporter API signature. ...Or, perhaps I'm missing something! That would be best.

I'm happy to whip up a patch if one is needed and any of these ideas or others would work.


@nedbat
Copy link
Owner Author

nedbat commented Feb 28, 2018

Issue #645 was marked as a duplicate of this issue.

@nedbat
Copy link
Owner Author

nedbat commented Feb 28, 2018

Original comment by John Sirois (Bitbucket: jsirois, GitHub: jsirois)


Downstream issue for extra context: pantsbuild/pants#5426

@nedbat
Copy link
Owner Author

nedbat commented Feb 28, 2018

@jsirois Thanks for the report, and for trying the plugins! Help me understand what you are trying to accomplish with your plugin: what's the larger problem you are trying to solve?

@nedbat
Copy link
Owner Author

nedbat commented Feb 28, 2018

Original comment by John Sirois (Bitbucket: jsirois, GitHub: jsirois)


Ah, looks like combine doesn't have any include/exclude filtering, so I think running 1 combine per source root won't do the trick. The 1st run will just map all symlink farm files to the 1st source root and then all remaining combines will no-op.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant