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

Investigating Codecov #3561

Closed
TimothyMothra opened this issue Aug 6, 2022 · 2 comments
Closed

Investigating Codecov #3561

TimothyMothra opened this issue Aug 6, 2022 · 2 comments
Assignees
Labels
enhancement New feature or request Stale Issues and pull requests which have been flagged for closing due to inactivity

Comments

@TimothyMothra
Copy link
Contributor

I'm investigating why Codecov often reports Impacted Files that are not modified by the PR.

Take this PR as an example: #3546.

This PR changes 3 files related to Serilog, but Codecov is reporting 19 Impacted Files from unrelated projects! (Codecov)

SCREENSHOT: CODECOV FOR PR 3546

image

Two things stand out to me...

  1. Codecov is reporting all the files that I recently increased coverage to 100%. (PR 3476)
    The reason for this is that 3546 was forked off an earlier version of main, before 3476 was merged.
    I think it's safe to ignore these as they should be omitted from future PRs, or if the current PR is rebased.

    SCREENSHOT: COMMIT IDs

    Codecov shows the base commit (a789bc0):
    image

    Commit history:
    image

  2. There are a few EventSource classes in this report!
    This is reporting that some methods were added to coverage, and some were removed.

    SCREENSHOT: src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs

    image

    SCREENSHOT: src/OpenTelemetry.Exporter.ZPages/Implementation/ZPagesExporterEventSource.cs

    image

    SCREENSHOT: src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusExporterEventSource.cs

    image

    The current theory is that some of our functional/integration tests are experiencing concurrency issues and executing different blocks during their tests.

Next Steps

  1. Can Codecov be configured to compare PRs against the current head of main?
    Currently comparing from where a PR was branched creates noise in PRs.
  2. Monitor which files are showing up in Codecov Reports for future investigations.
    • There may be a bug in either the test or the product code.
    • If these were 100% covered by dedicated unit tests, they shouldn't show up in the variability of other tests.
  3. Consider removing Integration Tests from Codecov.
    Assuming these are the source of the variability, this would help reduce noise in PRs.
@TimothyMothra TimothyMothra added the enhancement New feature or request label Aug 6, 2022
@TimothyMothra
Copy link
Contributor Author

Shared my findings in today's SIG meeting (2022-08-09).

Will continue to work to improve code coverage, this will remove some of the variability.
I'll also follow up with Eddy if he knows any better configurations for Codecov.

Copy link
Contributor

This issue was marked stale due to lack of activity and will be closed in 7 days. Commenting will instruct the bot to automatically remove the label. This bot runs once per day.

@github-actions github-actions bot added the Stale Issues and pull requests which have been flagged for closing due to inactivity label Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Stale Issues and pull requests which have been flagged for closing due to inactivity
Projects
None yet
Development

No branches or pull requests

1 participant