Skip to content

Add GCOV / LCOV C coverage support#7

Merged
encukou merged 2 commits into
python-ldap:masterfrom
tiran:lcov
Nov 25, 2017
Merged

Add GCOV / LCOV C coverage support#7
encukou merged 2 commits into
python-ldap:masterfrom
tiran:lcov

Conversation

@tiran
Copy link
Copy Markdown
Member

@tiran tiran commented Nov 24, 2017

Signed-off-by: Christian Heimes cheimes@redhat.com

@tiran tiran force-pushed the lcov branch 9 times, most recently from 9ff8616 to 88e5a70 Compare November 25, 2017 11:20
Comment thread Makefile Outdated
lcov-open: $(LCOV_REPORT)
xdg-open $(LCOV_REPORT)/index.html

lcov: lcov-clean lcov-coverage lcov-report
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think make guarantees the order in which it builds dependencies (especially with parallel builds – I have make aliased to make -j4).

IMO it should be something like:

lcov: lcov-clean
    $(MAKE) lcov-coverage
    $(MAKE) lcov-report

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the makefile has .NOTPARALLEL: to disable all parallel builds. But I can change the code.

Comment thread .travis.yml Outdated
- python: 2.7
env:
- TOXENV=py27
- WITH_GCOV=1
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is WITH_GCOV necessary here? It's specified in tox.ini.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, passenv only passes through existing variables, right? In that case I guess it's fine.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tox.ini has passenv = WITH_GCOV. It does not set WITH_GCOV but let tox pass the env vars through. By default tox unsets most env vars.

Comment thread Makefile Outdated
find . -name '*.py[co]' -or -name '*.so*' -or -name '*.dylib' -delete
find . -depth -name __pycache__ -exec rm -rf {} \;

# LCOV report
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will send anyone unfamiliar LCOV to Google. Can you note what this is? e.g. # LCOV report (measuring test coverage for C code)

tiran and others added 2 commits November 25, 2017 15:47
@encukou
Copy link
Copy Markdown
Member

encukou commented Nov 25, 2017

I found a way to simplify the config. Does this look good to you?

(Sorry for the mess I made when pushing it to this PR)

@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 25, 2017

Codecov Report

Merging #7 into master will decrease coverage by 2.14%.
The diff coverage is n/a.

Impacted Files Coverage Δ
Modules/ldapcontrol.c 28.57% <0%> (ø)
Modules/errors.c 79.36% <0%> (ø)
Modules/options.c 34.42% <0%> (ø)
Modules/message.c 32.53% <0%> (ø)
Modules/functions.c 61.66% <0%> (ø)
Modules/common.c 100% <0%> (ø)
Modules/constants.c 98.67% <0%> (ø)
Modules/ldapmodule.c 90.47% <0%> (ø)
Modules/berval.c 78.57% <0%> (ø)
Modules/LDAPObject.c 54.09% <0%> (ø)

@python-ldap python-ldap deleted a comment from codecov Bot Nov 25, 2017
@tiran
Copy link
Copy Markdown
Member Author

tiran commented Nov 25, 2017

ACK

We can move to matrix configuration later in order to run CI for linters and documentation.

UPDATE: There is a typo in setup.py: Instrumentation

@encukou encukou merged commit b91e5d2 into python-ldap:master Nov 25, 2017
@tiran tiran deleted the lcov branch November 25, 2017 15:17
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