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

Measure the test coverage #441

Merged
merged 11 commits into from
Mar 14, 2023
Merged

Measure the test coverage #441

merged 11 commits into from
Mar 14, 2023

Conversation

yakutovicha
Copy link
Member

fixes #440

@codecov
Copy link

codecov bot commented Mar 7, 2023

Codecov Report

❗ No coverage uploaded for pull request base (master@ca48cc4). Click here to learn what that means.
Patch has no changes to coverable lines.

❗ Current head 18094f5 differs from pull request most recent head 322ea71. Consider uploading reports for the commit 322ea71 to get more accurate results

Additional details and impacted files
@@            Coverage Diff            @@
##             master     #441   +/-   ##
=========================================
  Coverage          ?   39.21%           
=========================================
  Files             ?       18           
  Lines             ?     3091           
  Branches          ?        0           
=========================================
  Hits              ?     1212           
  Misses            ?     1879           
  Partials          ?        0           
Flag Coverage Δ
python-3.10 39.21% <0.00%> (?)
python-3.8 39.22% <0.00%> (?)
python-3.9 39.22% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@yakutovicha
Copy link
Member Author

Since the reported coverage is well beyond my original goal of 10-15% (as stated in the issue), I am making this PR ready for the review.

@yakutovicha yakutovicha marked this pull request as ready for review March 14, 2023 18:08
import pytest


@pytest.mark.usefixtures("aiida_profile_clean")
Copy link
Member

@unkcpz unkcpz Mar 14, 2023

Choose a reason for hiding this comment

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

I think we don't need this for these two tests. Even in aiida-core tests this fixture is not used very often. From my experience only when the aiida node stored in DB will break or mess up the logic of other tests.
Nevermind, I see for the second test it requires the database to be empty. This is needed.

Copy link
Member

@unkcpz unkcpz left a comment

Choose a reason for hiding this comment

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

@yakutovicha thanks for adding more tests! I have minor requests and things are not clear to me.

tests/test_structures.py Outdated Show resolved Hide resolved
tests/test_structures.py Show resolved Hide resolved
tests/test_structures.py Outdated Show resolved Hide resolved
@yakutovicha yakutovicha requested a review from unkcpz March 14, 2023 21:37
Copy link
Member

@unkcpz unkcpz left a comment

Choose a reason for hiding this comment

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

Looks all good.

@yakutovicha yakutovicha merged commit 76e31cb into master Mar 14, 2023
@yakutovicha yakutovicha deleted the feature/coverage-reports branch March 14, 2023 22:26
@danielhollas
Copy link
Contributor

@unkcpz @yakutovicha this is great, thanks! 👏

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.

Measure the test coverage
3 participants