Skip to content

Add Git actions for Codecov #487

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

Open
3 tasks done
Shaunak01 opened this issue Apr 3, 2025 · 15 comments
Open
3 tasks done

Add Git actions for Codecov #487

Shaunak01 opened this issue Apr 3, 2025 · 15 comments
Assignees

Comments

@Shaunak01
Copy link
Contributor

Shaunak01 commented Apr 3, 2025

We want to integrate Codecov to track and display coverage for our test suite. This will help us keep track of code quality and maintain a healthy testing culture. Below is the plan:

Steps and info for activating CodeCov: https://app.codecov.io/github/causify-ai/helpers/new

Steps:

  • Add codecove_requirements.txt and run pytest coverage action

Requirements

coverage
codecov

In a GitHub Action, run tests and generate a coverage report:

pytest --cov --cov-branch --cov-report=xml
  • Select an upload token to add as a secret on GitHub
  • Add Codecov to GitHub Actions workflow yaml file
- name: Upload coverage reports to Codecov
    uses: codecov/codecov-action@v5
    with:
      token: ${{ secrets.CODECOV_TOKEN }}

@samarth9008 @dremdem as you are working in restructuring our git actions can you guide as to where we should add codecov actions? The token is already added to helpers so we just have to add the actions.

This is just for test purposes, we have not bought this service yet. I will document this once tested or we solidify the plan.

FYI @gpsaggese pls guide if you can as well.

@samarth9008
Copy link
Contributor

@dremdem

do you mind taking a lead and mentor @Shaunak01 on this issue?

@samarth9008
Copy link
Contributor

@Shaunak01

Is there a reason to create a new issue? I think we can continue with the old one - https://github.com/causify-ai/cmamp/issues/11539

If there is no reason, feel free to transfer info to the old one and close this issue

@dremdem
Copy link
Contributor

dremdem commented Apr 8, 2025

@dremdem

do you mind taking a lead and mentor @Shaunak01 on this issue?

Yes, sure.

@dremdem
Copy link
Contributor

dremdem commented Apr 8, 2025

where we should add codecov actions?

Let's not mix workflow responsibilities - let's keep this as a separate workflow.
We already have the
https://github.com/causify-ai/cmamp/actions/workflows/test_coverage.yml
I'd suggest modifying that one.

@Shaunak01
Copy link
Contributor Author

Shaunak01 commented Apr 8, 2025

If there is no reason, feel free to transfer info to the old one and close this issue

We wanted to test in helpers only so opening the issue here from cmamp.

Let's not mix workflow responsibilities - let's keep this as a separate workflow. We already have the https://github.com/causify-ai/cmamp/actions/workflows/test_coverage.yml I'd suggest modifying that one.

Sure, can we start by modifying the helpers repo only

@dremdem
Copy link
Contributor

dremdem commented Apr 8, 2025

Sure, can we start by modifying the helpers repo only

Yes, feel free to use test_coverage.yml as a base.
First, I’d take a look at that GitHub Actions workflow and invoke action that is calling the fast_tests with the coverage parameter.
Then, develop the new GitHub Actions workflow in the helpers repo.

@Shaunak01
Copy link
Contributor Author

PR: #516
WIP

@Shaunak01
Copy link
Contributor Author

Take a look at how the coverage looks: https://app.codecov.io/github/causify-ai/helpers/tree/HelpersTask487_Add_Git_actions_for_Codecov_3/

We get a 14 day free trial for complete org so we can do that if we like this service.

@gpsaggese
Copy link
Contributor

  1. The view of current coverage is pretty cool
  2. Ofc, if we want to have a change in coverage for every PR we need to enable the coverage build for every PR, right?
    • This looks a bit too complicated. An alternative approach is to detect when a PR decreases the coverage after the fact
    • Is there a way to detect that automatically (e.g., an API for codecov or just a check on the files we generate to say "the number of lines covered by test can't decrease", maybe with a little buffer for the inevitable jitter)

@Shaunak01
Copy link
Contributor Author

Codecov supports a configuration file (typically named codecov.yml) where you can set targets and thresholds for your project’s overall coverage. When you configure these settings, Codecov will automatically compare the new coverage data to the baseline for each PR and flag (or even fail) PRs if the coverage drop exceeds your allowed buffer.

@gpsaggese
Copy link
Contributor

Ok pretty cool.

Let's create a short markdown document about this system explaining how it works, how to configure it, etc.

We can start with //helpers and then use the same approach in all repos.

@dremdem
Copy link
Contributor

dremdem commented Apr 15, 2025

@gpsaggese @samarth9008
Can we reiterate the high-level requirements for this issue:

Initially, I thought that:

  • It would run every Monday for the fast, slow and superslow
  • After every run, the code coverage would be updated and accessible at this link
    • My proposal is to add the link to the Master_buildmeister_dashboard, under the [Allure Reports(http://172.30.2.44/build/buildmeister_dashboard/Master_buildmeister_dashboard.latest.html#Allure-reports)

However, considering: #487 (comment)

if we want to have a change in coverage for every PR we need to enable the coverage build for every PR

My questions is:

  • Should we run it for every PR? And what should be displayed? The percentage decrease in test coverage?
  • Do we need to block merging if the coverage decreases too much?

Update:
Found the example how it can be implemented:
codecov/example-python#293
Image

We already have something in the #516
Image

It probably needs some setup to make it work.

@Shaunak01
Copy link
Contributor Author

@gpsaggese

Can we re iterate on the high level goals as suggested by @dremdem

  1. Do we want the coverage to run every Monday for the fast, slow and superslow (all 3 or just fast-slow) OR do we want to run with every PR?
  2. If we run on every PR, do we need to block merging if the overage reduces below certain threshold?

@gpsaggese
Copy link
Contributor

  1. Let's start with running the coverage once every week for the 3 builds.
  2. Is there a way to merge the 3 coverages to get the total one?
  3. Let's replace the old coverage system, adding a link in the BM view
  4. Let's put together a short doc of how the system works, etc.
  5. Let's start with helpers and then we extend to the other repos.
  6. We can report a break if the coverage decreases more than 1% in a week. Ideally we want to only have coverage increase.

Makes sense?

@dremdem
Copy link
Contributor

dremdem commented Apr 21, 2025

  1. Let's start with running the coverage once every week for the 3 builds.

@Shaunak01
I'd use the #516 as a reference and create a new one from scratch.
In the new PR, we can remove all the redundant code we discussed in the last meeting.

After that, we can proceed with the next steps.
Feel free to create sub-issues for each step.

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

No branches or pull requests

4 participants