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

Add codecov config file, excluded glacier & dao forks, add codecov badge to README #1168

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Carsons-Eels
Copy link
Collaborator

@Carsons-Eels Carsons-Eels commented Mar 21, 2025

What was wrong?

Code coverage unfairly included glacier forks and the dao fork, for which we will not create tests

Closes issues #1139 and #1141

How was it fixed?

Excluded glacier forks in config file

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@Carsons-Eels Carsons-Eels force-pushed the 1139_exclude_glacier_fork_coverage branch from 0ddef70 to 105698e Compare March 21, 2025 01:27
@Carsons-Eels Carsons-Eels force-pushed the 1139_exclude_glacier_fork_coverage branch from 105698e to e7b0a18 Compare March 21, 2025 01:39
@codecov-commenter
Copy link

codecov-commenter commented Mar 22, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.86%. Comparing base (2926c00) to head (b25747b).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1168      +/-   ##
==========================================
- Coverage   84.87%   83.86%   -1.02%     
==========================================
  Files         649      649              
  Lines       36122    36122              
  Branches        0     3224    +3224     
==========================================
- Hits        30659    30293     -366     
  Misses       5463     5463              
- Partials        0      366     +366     
Flag Coverage Δ
unittests 83.86% <ø> (-1.02%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@SamWilsn
Copy link
Contributor

I realize this is still a draft and that you're probably aware, but just in case you haven't seen it yet, arrow_glacier is still included:

1000009594

@SamWilsn
Copy link
Contributor

I hope you don't mind, but I took a quick look at pytest-cov's documentation, and it looks like we can exclude directories earlier in the process. We could maybe add a section like this to pyproject.toml (and add the --cov-config=pyproject.toml option in tox.ini).

[tool.coverage.run]
omit = [
    "src/ethereum/*_glacier/*",
    "src/ethereum/dao_fork/*",
]

@Carsons-Eels
Copy link
Collaborator Author

I hope you don't mind, but I took a quick look at pytest-cov's documentation, and it looks like we can exclude directories earlier in the process. We could maybe add a section like this to pyproject.toml (and add the --cov-config=pyproject.toml option in tox.ini).

[tool.coverage.run]
omit = [
    "src/ethereum/*_glacier/*",
    "src/ethereum/dao_fork/*",
]

Not at all, this is good! I'll update the PR

@Carsons-Eels Carsons-Eels changed the title added codecov config file, excluded glacier dirs, added badge to readme 1139 - Add codecov config file, excluded glacier dirs, add badge to README Mar 25, 2025
@Carsons-Eels Carsons-Eels changed the title 1139 - Add codecov config file, excluded glacier dirs, add badge to README Add codecov config file, excluded glacier & dao forks, add codecov badge to README Mar 25, 2025
@Carsons-Eels Carsons-Eels force-pushed the 1139_exclude_glacier_fork_coverage branch from 01205f5 to 83ee3c3 Compare March 25, 2025 21:49
@Carsons-Eels Carsons-Eels force-pushed the 1139_exclude_glacier_fork_coverage branch from 83ee3c3 to a132f93 Compare March 26, 2025 01:30
@Carsons-Eels Carsons-Eels marked this pull request as ready for review March 27, 2025 16:34
@SamWilsn
Copy link
Contributor

SamWilsn commented Mar 28, 2025

Looks like codecov is still counting the glacier forks...

I wonder if it has something to do with how tox is passing the path? In the console output, we have lines like:

.tox/py3/lib/python3.10/site-packages/ethereum/dao_fork/__init__.py

Perhaps the glob we need to use is */ethereum/dao_fork/*?

@SamWilsn
Copy link
Contributor

For future PRs, prefer rebase over merge commits please!

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.

3 participants