linkedin / pygradle

Using Gradle to build Python projects
https://github.com/linkedin/pygradle
Apache License 2.0
586 stars 147 forks source link

'term:skip-covered' should be used in preference to 'term' #324

Open wadetregaskis-linkedin opened 4 years ago

wadetregaskis-linkedin commented 4 years ago

In reporting to the terminal, the pertinent information is the missing coverage - I don't need to see a long list of files with 100% coverage. I still want that in the other reports, because e.g. my CI pipeline might set requirements on overall coverage - so I can't just put 'skip_covered = true' in my setup.cfg.

warsaw commented 4 years ago

Can you just add

[report]
skip_covered=true

to your setup.cfg?

wadetregaskis-linkedin commented 4 years ago

Unfortunately not - I tried that, but it applies to all reports, not just the one output to the terminal. That's what I was alluding to in my original comment here.

I also tried re-setting it to false specifically in the [coverage:html] et al sections, but unfortunately it's apparently not supported in those sections (evidently they're not logical 'subclasses' of the 'report' section).

warsaw commented 4 years ago

Ah, sorry; you wrote skip_missing so that threw me off.

FWIW, I like the 100% coverage output.

Unfortunately, I'm not sure if what you want is supported by coverage.

wadetregaskis-linkedin commented 4 years ago

Whoops! Sorry for the confusion. Properties - they're all the same, right? ;)

wadetregaskis-linkedin commented 4 years ago

It is supported - AFAICT that's why 'term:missing' exists, as a special-case way to override skip_covered only for terminal output. It unfortunately appears to be the only way to do that, and I don't see any way to otherwise modify what the coverage pygradle plugin passes to pytest w.r.t. these parameters.

warsaw commented 4 years ago

FYI, this change would have to be made in PyCoverageTask.groovy:

    public void preExecution() {
        // using subArgs as these must be added after py.test
        subArgs(
            '--cov',
            project.file(component.srcDir).getAbsolutePath(),
            '--cov-report=xml',
            '--cov-report=html',
            '--cov-report=term'
        )

        super.preExecution()
    }

We would have to think about how to make this configurable since we still want to support --cov-report=term.

wadetregaskis-linkedin commented 4 years ago

Agreed making it configurable would be wise. I have no objection if I have to explicitly turn this on within build.gradle or similar - so long as there's some way for me to do so.