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

Move and rename coverage file #1218

Merged
merged 3 commits into from
Jul 9, 2020

Conversation

arjunsavel
Copy link
Contributor

@arjunsavel arjunsavel commented Jul 2, 2020

Description

I've moved the coverage configuration file to the TARDIS root directory; this should ensure that it is read in correctly (e.g., correct files are omitted from coverage calculation).

Motivation and Context

When running python setup.py test --coverage locally, I noticed that test files were being included in coverage counts. These are explicitly omitted in the coverage configuration file.

How Has This Been Tested?

Checking the coverage page for this PR seems to indicate that it works as expected.

Screenshots (if appropriate):

sunburst
The above image shows the coverage sunburst from our codecov page, which notably includes test folders that are excluded in the configuration file. By changing the location of the coverage file, this will (hopefully!) be fixed.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have assigned/requested two reviewers for this pull request.

@arjunsavel arjunsavel requested review from marxwillia and epassaro July 2, 2020 18:16
@arjunsavel arjunsavel self-assigned this Jul 2, 2020
@codecov
Copy link

codecov bot commented Jul 2, 2020

Codecov Report

Merging #1218 into master will increase coverage by 2.91%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1218      +/-   ##
==========================================
+ Coverage   77.71%   80.63%   +2.91%     
==========================================
  Files          91       41      -50     
  Lines        5727     3423    -2304     
==========================================
- Hits         4451     2760    -1691     
+ Misses       1276      663     -613     
Impacted Files Coverage Δ
tardis/scripts/cmfgen2tardis.py 0.00% <0.00%> (ø)
tardis/io/tests/test_config_reader.py
tardis/tests/fixtures/atom_data.py
tardis/montecarlo/tests/test_spectrum.py
tardis/simulation/setup_package.py
tardis/io/tests/test_config_validator.py
tardis/io/tests/test_csvy_reader.py
tardis/io/__init__.py
...s/plasma/tests/test_tardis_model_density_config.py
tardis/tests/test_tardis_full.py
... and 47 more

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 f47075d...1a183ce. Read the comment docs.

@epassaro
Copy link
Member

epassaro commented Jul 3, 2020

I don't know so much about coverage.

This is the .coveragerc file from carsus: https://github.com/tardis-sn/carsus/blob/master/carsus/tests/coveragerc

Copy link
Contributor

@marxwillia marxwillia left a comment

Choose a reason for hiding this comment

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

Looks good. Just want to make sure you aren't missing an ending '*' in line 47?

@arjunsavel
Copy link
Contributor Author

@marxwillia thanks for the suggestion — done!

@epassaro
Copy link
Member

epassaro commented Jul 8, 2020

How does the sunburst graph looks now?

@arjunsavel
Copy link
Contributor Author

@epassaro can't quite get the sunburst graph to happen — but here's how the tree looks. No tests are being checked for coverage anymore!

@arjunsavel arjunsavel merged commit 2599b0c into tardis-sn:master Jul 9, 2020
@arjunsavel arjunsavel deleted the fix_codecov_config branch July 9, 2020 03:06
atharva-2001 pushed a commit to atharva-2001/tardis that referenced this pull request Oct 1, 2021
* Moved and renamed coverage file

* Omit astropy_init from coverage calculation

* generalize omit of astropy_init to all file types
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants