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 coverage task & integrate with poe check #4847

Merged
merged 5 commits into from
Jan 2, 2025

Conversation

srjoglekar246
Copy link
Contributor

Why are these changes needed?

Adds code coverage checks for autogen

Related issue number

Closes #4477

Checks

  • Ensured code coverage tests are correctly run.

@srjoglekar246
Copy link
Contributor Author

@ekzhu here you go.

Do we want to enforce some coverage level with something like --cov-fail-under=80?

@ekzhu
Copy link
Collaborator

ekzhu commented Dec 29, 2024

Thank you! @srjoglekar246

I am wondering if we can run this in CI automatically on a PR and publish the result as a message to the PR?

The v0.2 branch does this:

- name: Upload coverage to Codecov
if: matrix.python-version == '3.10'
uses: codecov/codecov-action@v3
with:
file: ./coverage.xml
flags: unittests

More documentation on code cov action: https://github.com/marketplace/actions/codecov

I think we should update this part of checks.yml:

test:
runs-on: ubuntu-latest
strategy:
matrix:
package:
[
"./packages/autogen-core",
"./packages/autogen-magentic-one",
"./packages/autogen-ext",
"./packages/autogen-agentchat",
]
steps:
- uses: actions/checkout@v4
- uses: astral-sh/setup-uv@v5
with:
enable-cache: true
- uses: actions/setup-python@v5
with:
python-version: "3.11"
- name: Run uv sync
run: |
uv sync --locked --all-extras
working-directory: ./python
- name: Run task
run: |
source ${{ github.workspace }}/python/.venv/bin/activate
poe --directory ${{ matrix.package }} test
working-directory: ./python

The code coverage result can be viewed here: https://app.codecov.io/github/microsoft/autogen. Since we haven't been running the v0.2 branch for several months, it is outdated information on it.

@codecov-commenter
Copy link

codecov-commenter commented Dec 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.21%. Comparing base (38cce47) to head (cbc85d6).
Report is 1041 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #4847       +/-   ##
===========================================
+ Coverage   30.49%   68.21%   +37.71%     
===========================================
  Files         113      158       +45     
  Lines       12284     9960     -2324     
  Branches     2602        0     -2602     
===========================================
+ Hits         3746     6794     +3048     
+ Misses       8210     3166     -5044     
+ Partials      328        0      -328     
Flag Coverage Δ
unittests 68.21% <ø> (+37.73%) ⬆️

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.

@srjoglekar246
Copy link
Contributor Author

@ekzhu @jackgerrits Looks like code coverage is working fine, can you take a look?

Copy link
Collaborator

@ekzhu ekzhu left a comment

Choose a reason for hiding this comment

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

Great! Thanks!

cc @victordibia we may need to add studio package to the coverage list also.

@jackgerrits
Copy link
Member

Does the coverage report need to be merged before it is reported to codecov? Or is codecov able to merge the reports across the repo?

@ekzhu
Copy link
Collaborator

ekzhu commented Dec 31, 2024

I think it needs to be merged to main first before we can get new report base. The current base is from several months ago: https://app.codecov.io/gh/microsoft/autogen

@jackgerrits
Copy link
Member

Oh sorry my wording was ambiguous. I meant that this PR is producing many reports and sending them all independently to codecov. Do they need to be combined into a single coverage report for the given pr/commit? What do the codecov docs say on this?

@jackgerrits
Copy link
Member

Looks like it might be automatic: https://docs.codecov.com/docs/github-4a-merging-reports

@jackgerrits
Copy link
Member

We might want to ignore samples and tests though https://docs.codecov.com/docs/code-coverage-with-python

image

@ekzhu
Copy link
Collaborator

ekzhu commented Dec 31, 2024

So adding a codecov.yml in each of the package we are running the code coverage?

@jackgerrits
Copy link
Member

It must be a single file at the root of the repo

https://docs.codecov.com/docs/codecov-yaml#can-i-name-the-file-codecovyml

@jackgerrits jackgerrits merged commit ecdade3 into microsoft:main Jan 2, 2025
56 checks passed
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.

Test coverage for code
4 participants