Skip to content
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

platform specific configuration #563

Closed
nedbat opened this issue Feb 7, 2017 · 24 comments
Closed

platform specific configuration #563

nedbat opened this issue Feb 7, 2017 · 24 comments
Labels
enhancement New feature or request

Comments

@nedbat
Copy link
Owner

nedbat commented Feb 7, 2017

Originally reported by John Vandenberg (Bitbucket: jayvdb, GitHub: jayvdb)


If a block has two platform/OS-specific branches, currently the only way to reach 100% with a single configuration is to use #pragma: no cover on both branches, ignoring the block when coverage is run on any platform/OS.

It is useful for each platform/OS to require coverage for their own branch.

This can be currently be achieved by using a different configuration file for each platformOS.

It would be helpful that the configuration allows for different ignore patterns for platform/OS, so only one configuration file is needed for all platforms.

One easy way to do this is to iterate the section names in the ini file, extracting any section with pytest in the name, and then finding the most appropriate section for the host platform/OS.

Another approach is to allow the ignore regex to be specified on the command line.


@nedbat
Copy link
Owner Author

nedbat commented Feb 7, 2017

You can already get the effect of the regex being specified on the command line, because environment variables are expanded in the .coveragerc values. So you could use TOXENV in your coverage pragma, or some other environment variable you create just for that purpose.

@nedbat
Copy link
Owner Author

nedbat commented Feb 7, 2017

Original comment by John Vandenberg (Bitbucket: jayvdb, GitHub: jayvdb)


Perfect. I'll implement that in my current project, and then submit documentation to close this issue.

@nedbat
Copy link
Owner Author

nedbat commented Feb 9, 2017

Original comment by John Vandenberg (Bitbucket: jayvdb, GitHub: jayvdb)


I have env vars working.

As platform specific code is one of the most common and unavoidable uses of ignore_lines, it would be nice if we could build a mini PEP 496 language into config.py 's HandyConfigParser.get .
Then it works out of the box, without any need for setting platform specific env vars before running coverage.

I've build that type of thing into flake8-putty, and would be happy to contribute it to coverage if it is desired.

@nedbat
Copy link
Owner Author

nedbat commented Feb 9, 2017

I'm not sure what that would look like, or how it would affect backward compatibility.

@nedbat
Copy link
Owner Author

nedbat commented Feb 9, 2017

Original comment by John Vandenberg (Bitbucket: jayvdb, GitHub: jayvdb)


How about pseudo environment variables, using lower case to avoid conflicts with any env vars which might actually be set.

  • os_name
  • sys_platform
  • platform_release
  • implementation_name
  • platform_machine
  • platform_python_implementation

And if the env actually includes any of those keys, all are disabled...?

@nedbat
Copy link
Owner Author

nedbat commented Jun 7, 2017

Original comment by John Vandenberg (Bitbucket: jayvdb, GitHub: jayvdb)


I creates two plugins which in concert solve this problem:

In action at coala/coala#4337

@nedbat
Copy link
Owner Author

nedbat commented Nov 17, 2017

Help me understand how you use these plugins? Why do you need to reload the configuration settings? Aren't your different environments just different runs of coverage, with different config files?

@nedbat
Copy link
Owner Author

nedbat commented Nov 17, 2017

Original comment by John Vandenberg (Bitbucket: jayvdb, GitHub: jayvdb)


https://github.com/coala/coala/blob/master/setup.cfg#L62

exclude_lines =
    pragma: no ?cover
    pragma ${PLATFORM_SYSTEM}: no cover
    pragma ${OS_NAME}: no cover
    pragma Python [0-9.,]*${PYTHON_VERSION}[0-9.,]*: no cover

coverage_env_plugin adds all the environment markers into the coverage environment , and the other reloads the config so that exclude_lines is re-parsed (as environment variables are replaced when the config is first loaded, before plugins are loaded.)

@nedbat
Copy link
Owner Author

nedbat commented Nov 25, 2017

Original comment by Toshio Kuratomi (Bitbucket: toshio, GitHub: toshio)


@jayvdb Do your additions allow excluding coverage reports for just python major version? (Only report on Python2 or only report on Python3)? PYTHON_VERSION looks like it will expand to the full MAJOR.MINOR.MICRO version which seems less helpful. (you needing to specify every Python version that the tests will run on. If the config is to be generic for any random OSS contributor to the project, that quickly gets out of hand.

@nedbat
Copy link
Owner Author

nedbat commented Nov 26, 2017

Original comment by John Vandenberg (Bitbucket: jayvdb, GitHub: jayvdb)


No, we use it for minor versions. It is a bit awkward. However PYTHON_VERSION expands to only MAJOR.MINOR , so .MICRO isnt involved. I see https://www.python.org/dev/peps/pep-0496/#examples gives a bad example with .MICRO, which is useless and actually will break some versions of setuptools/pip/etc.

Here is code which only needs coverage on Python 3.4 , and not 3.5/3.6
https://github.com/coala/coala/blob/6d7512ce9afd562905d1d935f63c9d72c1f89fc9/coalib/misc/Compatibility.py

If we do builds on Python 3.7, we would need to add 3.7 there.
Ideally a better solution is arrived at.

@nedbat
Copy link
Owner Author

nedbat commented Nov 26, 2017

@jayvdb Could you define these environment variables in your actual environment, so they are available to the config parser? It could be done with a Python program that sets the environment and then runs another command (like "coverage"), so that you didn't have to fiddle around in the coverage internals. Then you can define whatever set of environment variables you like.

@nedbat
Copy link
Owner Author

nedbat commented Nov 26, 2017

Original comment by John Vandenberg (Bitbucket: jayvdb, GitHub: jayvdb)


of course. We were doing that before coala/coala#4337 .

Im looking for coveragepy to have a builtin solution for platform specific code.
Otherwise everyone builds their own custom solution.

I did a prototype using plugins, and it is a bit hacky due to how the coverage config is loaded. If the concept is accepted, a patch to core can improve on the prototypes in many ways.

@nedbat
Copy link
Owner Author

nedbat commented Nov 26, 2017

My concern about this path is that the set of conditions is going to vary from project to project. Is 3.6.2 a different platform than 3.6.3? Is Ubuntu different than Arch? Is Ubuntu different than Debian? I would rather have the code that makes those determinations be outside of coverage.py.

One idea (mentioned above) is a program runner that sets the environment variables. Another would be to add a feature to coverage.py so that it reads a specific env file to set variables in addition to reading them from the environment. Then your program could write that file before running coverage. Though again, the existing mechanism (the environment) is specifically designed for this. We shouldn't need to invent new mechanisms to get this to work.

I understand what you mean about wanting the solution in a common place. Your program that makes these determinations can become a tool used by many projects if they need this kind of fine-slicing behavior.

@nedbat
Copy link
Owner Author

nedbat commented Nov 26, 2017

Original comment by Barry Warsaw (Bitbucket: warsaw, GitHub: warsaw)


Here's another case that I've had to deal with recently:

https://github.com/brettcannon/importlib_resources/blob/master/importlib_resources/_py3.py#L18

In this case I need to specify coverage for ranges of Python 3 minor versions. Through a terrible combination of my tox.ini and coverage.ini files, I've managed to make it work, but I don't like it. It's hard to reason about when reading the code, and it isn't very robust.

This may be off-topic for this particular issue, but having minor version ranges would be very helpful.

@nedbat
Copy link
Owner Author

nedbat commented Nov 27, 2017

Original comment by Toshio Kuratomi (Bitbucket: toshio, GitHub: toshio)


Looking at Barry's case, I think we do need some additional support in coverage for a clean solution. Right now coverage gives us a way to specify in coverage that a pattern in the code should be excluded. What Barry is looking at is needing to set a pattern in code (a range of Python versions). My initial thought is that that would need for coverage to have a pattern that it extracts from the code (like '#pragma: .*$') and then coverage would have to parse that to determine what to do. We can't do that with a purely external tool but perhaps coverage could give us hook points to do it in a plugin?

@nedbat
Copy link
Owner Author

nedbat commented Dec 3, 2017

Barry's case just convinces me more and more that the actual conditions needed will vary wildly from project to project. I really don't want coverage.py to be dictating what special conditions you can select for in pragmas. Using environment variables in .coveragerc has already been more powerful than anything I could have built in coverage.py directly.

What I'm wondering is how to add something akin to environment variables in .coveragerc, to provide a more useful configuration parameterization mechanism. Then you can use your own program to determine the pragmas to put into effect, using whatever logic you want.

For example, what if the .coveragerc could name another file to read as part of the configuration? Then your prelude program could write a bunch of [report] exclude_lines = ... configuration into a file, and coverage.py would read it. Or, coverage.py could read a file full of variable definitions, and use them as if they had been environment variables (though I don't know why environment variables don't already work in that case).

A new plugin that could modify the configuration would also do the trick.

@nedbat
Copy link
Owner Author

nedbat commented Dec 4, 2017

Original comment by Toshio Kuratomi (Bitbucket: toshio, GitHub: toshio)


None of that would help for Barry's case. A configuration plugin would help for many other cases (including mine), though. In the non-Barry cases, people are specifying something in our code which can be exactly matched by something that can be provided to coverage. If I put # pragma: py3 in the code and I run on python3.5, I can configure my .coveragerc to exclude "# pragma: py${PY_MAJ_VER}" where I have to set PY_MAJ_VER from some other source prior to reading the configuration file)

(Note: it would help in writing plugins or other tools that deal with this if the environment variables were expanded when the exclude is used. Right now, they are expanded when the configuration is read in which means that tools have to set the environment variables very early in the process. For instance, I wrote a pytest plugin to address this and had to make it a hookwrapper in order to run before the pytest_cov plugin which read the coverage configuration at startup.)

In Barry's case, though, he wants to specify a range like this in his code:

#!python

# pragma: py3.4-3.6

The problem with coverage's current exclude configuration is that it has no way to specify that py3.4-3.6 should be excluded when run on python-3.5.2. So to satisfy his needs we need a plugin that can hook in when the code is being scanned for lines to exclude. The plugin would either need to be passed every line or coverage would have to become opinionated about there being some pattern (For instance, .*# pragma:.*) that would then trigger being passed to the plugins. The plugin would have to check the argument to # pragma: to determine if the line matched its internal criteria for exclusion or not. (In Barry's case, it would have to parse the "py3.4-3.6" string, decide that it should be excluded if the python version was between 3.4 and 3.6 inclusive, and then check that against the running python version to make a final decision).

I think the idea of reading part of the configuration from another file largely misses the point. People could write multiple .coveragerc's already. The problem is that selecting which one to use on any particular test run then gets messy. People want something that they can set up once and then just let it do its thing automatically. An external tool can handle some cases but it has the drawback of having to fuss with it every time you set up your testing in something new (tox vs travis vs appveyor vs raw pytest vs [....]). An external plugin to coverage (one which enables the desired behaviour by virtue of being pip installed on the system/virtualenv) seems like the best bet which doesn't put the code directly into coverage itself. But to do that, it needs to have access to hook into coverage's work flow at the appropriate time.

@nedbat
Copy link
Owner Author

nedbat commented Dec 4, 2017

Original comment by Barry Warsaw (Bitbucket: warsaw, GitHub: warsaw)


I concur with @toshio !

@nedbat
Copy link
Owner Author

nedbat commented Dec 23, 2017

OK, here's the plan I have in mind: we'll have a new kind of plugin:

class MyConfigPlugin(CoveragePlugin):
    def configure(self, config):
        opt_name = "report:exclude_lines"
        exclude_lines = config.get_option(opt_name)
        exclude_lines.append(r"pragma: custom")
        exclude_lines.append(r"pragma: or whatever")
        config.set_option(opt_name, exclude_lines)

def coverage_init(reg, options):
    reg.add_configurer(MyConfigPlugin())

The plugin would have a chance to use get_option(option_name) and set_option(option_name, value) before processing starts. It could use any logic it wants to set as many exclude_lines (or any other config value) it wants.

Sound good?

@nedbat
Copy link
Owner Author

nedbat commented Dec 24, 2017

Original comment by Barry Warsaw (Bitbucket: warsaw, GitHub: warsaw)


@nedbat Yeah, that seems like it would work.

@nedbat
Copy link
Owner Author

nedbat commented Jan 6, 2018

I implemented this in 781b3c282758 (bb). Try it out, let me know what you think.

@nedbat
Copy link
Owner Author

nedbat commented Feb 3, 2018

This was included in coverage.py v4.5, released today.

@nedbat
Copy link
Owner Author

nedbat commented Feb 4, 2018

Original comment by Barry Warsaw (Bitbucket: warsaw, GitHub: warsaw)


Cool, thanks! Sorry I didn't get a chance to test it beforehand. I'm seeing something weird with 4.5, but I want to investigate further before I report anything. I'll open new issues for anything I find.

@nedbat nedbat closed this as completed Feb 4, 2018
@nedbat nedbat added major enhancement New feature or request labels Jun 23, 2018
@nedbat
Copy link
Owner Author

nedbat commented Feb 25, 2020

There's a new coverage plugin that can do these sorts of conditionals: https://sobolevn.me/2020/02/conditional-coverage

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