Skip to content

Adds test coverage to Cython files#2254

Closed
fenilsuchak wants to merge 0 commit intoMDAnalysis:developfrom
fenilsuchak:cython_cov
Closed

Adds test coverage to Cython files#2254
fenilsuchak wants to merge 0 commit intoMDAnalysis:developfrom
fenilsuchak:cython_cov

Conversation

@fenilsuchak
Copy link
Member

@fenilsuchak fenilsuchak commented Apr 18, 2019

Changes made in this Pull Request:

PR Checklist

  • Tests?
  • Docs?
  • CHANGELOG updated?
  • Issue raised/referenced?

@codecov
Copy link

codecov bot commented Apr 18, 2019

Codecov Report

Merging #2254 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           develop   #2254   +/-   ##
=======================================
  Coverage     89.7%   89.7%           
=======================================
  Files          160     160           
  Lines        19769   19769           
  Branches      2783    2783           
=======================================
  Hits         17733   17733           
  Misses        1440    1440           
  Partials       596     596

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fdeb724...68e6123. Read the comment docs.

@richardjgowers
Copy link
Member

@Fenil3510 we can't add the c/cpp files to ignore because we do include them in the master branch. So releases don't require Cython.

package/setup.py Outdated
"parallelization module".format(
Cython.__version__, required_version))
cython_found = False
cython_linetrace = True
Copy link
Member

Choose a reason for hiding this comment

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

Ideally this isn't on by default as I'm guessing it has a performance cost. We could probably look for one of the travis/CI environment variables to trigger this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this is not a default one and there's an environment variable to trigger this as you had already done in #2029, but I was forcing this to look at the results for now.

@fenilsuchak fenilsuchak changed the title test_cov_cython Adds test coverage to Cython files Apr 18, 2019
@fenilsuchak
Copy link
Member Author

fenilsuchak commented Apr 18, 2019

.pyx still not visible in codecov report. Have to work on it.

@fenilsuchak
Copy link
Member Author

@Fenil3510 we can't add the c/cpp files to ignore because we do include them in the master branch. So releases don't require Cython.

The .c files were due to local build, will have to remove them anyway.

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