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

Enforce code coverage #2455

Closed
Pwuts opened this issue Apr 18, 2023 · 9 comments
Closed

Enforce code coverage #2455

Pwuts opened this issue Apr 18, 2023 · 9 comments

Comments

@Pwuts
Copy link
Member

Pwuts commented Apr 18, 2023

https://app.codecov.io/login/gh

@ntindle
Copy link
Member

ntindle commented Apr 19, 2023

going to be workign on this after my pr is merged for shirts

@7--
Copy link

7-- commented Apr 19, 2023

How about fixing all the obvious bugs before wasting time covering code.

@Pwuts
Copy link
Member Author

Pwuts commented Apr 19, 2023

@7-- you're welcome to help. "Don't just stand there, grab a shovel and dig."

400 issues open, 250 PRs, those need filtering and deduplication before we can even prioritize them.

Also, where do you think the issues come from in the first place? It doesn't help that we don't have coverage and do have multiple "supported" ways to run Auto-GPT.

@Pwuts
Copy link
Member Author

Pwuts commented Apr 20, 2023

Duplicate of #2424

@Pwuts Pwuts marked this as a duplicate of #2424 Apr 20, 2023
@Pwuts Pwuts closed this as not planned Won't fix, can't repro, duplicate, stale Apr 20, 2023
@jerrodcodecov
Copy link

jerrodcodecov commented Apr 25, 2023

Jerrod from Codecov here. Thanks for using the tool. I've been following AutoGPT closely.

Opening discussion here from a convo with @ntindle, but I'm curious @Pwuts.

I noticed you are enforcing / blocking a PR for BOTH patch coverage and project coverage.

If you are enforcing patch coverage (git diff) on every PR, what is the goal of enforcing project coverage (repo coverage) on each PR as well? Are you worried folks are changing/removing tests without realizing it? We'll still help you troubleshoot the current issue(s), but I'm more just curious from a product sense.

Patch coverage alone should give you the "leave things better than you found them" mindset vs. having to have a user care about all of the coverage of the project on every PR. Also, it avoids any weirdness that might occur from comparing a base to a head of a PR where the base and head are not apples to apples for some reason.

@Pwuts
Copy link
Member Author

Pwuts commented Apr 25, 2023

@jerrodcodecov I think it's more that we didn't have a lot of time to set it up so we haven't looked at many of the settings yet. :)

Where should we look to disable the project coverage check?

@jerrodcodecov
Copy link

jerrodcodecov commented Apr 26, 2023

@Pwuts

https://github.com/Significant-Gravitas/Auto-GPT/blob/master/codecov.yml

Codecov is configured here ^

Some common configurations are listed here: https://docs.codecov.com/docs/common-recipe-list#set-non-blocking-status-checks

I'd recommend you change your current YAML

coverage:
  status:
    project:
      default:
        target: auto
        threshold: 1%
      

Replaced by

coverage:
  status:
    project:
      default:
        target: auto
        threshold: 1%
        informational: true
    patch:
      default:
        target: 80%
       

This would set the "project" coverage check to informational only and not block, but will block on the patch coverage (git diff) at 80%. You could set that figure to whatever you'd like of course.

Also, make sure you remove "project coverage check" from the branch protection rules as a repo admin:

https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/about-protected-branches

@jerrodcodecov
Copy link

jerrodcodecov commented Apr 26, 2023

Update from convo with @ntindle and @Pwuts

1.) @vlad-ko and I are going to fork/PR a new change to your Codecov YAML based on the above

2.) @ntindle is going to change the Github Actions flow to upload a set of unit tests and a set of integration tests and then Codecov will use Carryforward Logic to "carryforward" most recent integration test run when a fork doesn't have the appropriate API keys to run the integration tests

3.) @ntindle (or us) will make one more tweak to the Codecov YAML to ingest the flags correctly (we will put it in the current YAML change, commented out)

--

4.) There still may be some residual challenges for accurately comparing coverage from a fork that we are looking into, but we are hopeful #1 through #3 unblock us sufficiently for now

@jerrodcodecov
Copy link

Lastly @ntindle make sure you remove project coverage check from branch protection rules in the Github repo admin. This will ensure that the project coverage check no longer blocks the forks/PRs.

image

https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/about-protected-branches

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