codecov / codecov-browser-extension

A browser extension for Codecov. Works for Chrome and Firefox
Apache License 2.0
36 stars 11 forks source link

Coverage is offset due to fetching wrong commit. #17

Closed eliatcodecov closed 1 year ago

eliatcodecov commented 1 year ago

Compare this section of code in both the extension and in the Codecov UI. Coverage appears to be offset

Extension

URL: https://github.com/codecov/codecov-cli/blob/master/codecov_cli/main.py

Screenshot 2023-05-12 at 8 46 46 AM

Codecov UI

URL: https://app.codecov.io/gh/codecov/codecov-cli/blob/master/codecov_cli/main.py

Screenshot 2023-05-12 at 8 47 45 AM

Problem

I believe the actual issue here is that GitHub is showing a commit that's ahead of the coverage being displayed because that coverage has not yet been uploaded to Codecov. you can see here that the commit in questione76d9b9 is in a processing state on Codecov:

Screenshot 2023-05-12 at 8 52 59 AM

(Source: https://app.codecov.io/gh/codecov/codecov-cli/commit/e76d9b907c78c26a27899654bf094d021067378c)

I think the extension is just fetching the most recent coverage from the branch, which is ec1c3ef (link: https://app.codecov.io/gh/codecov/codecov-cli/commit/ec1c3ef82c9abc53ed1fecd74e340a7d4e5b202c). Therefore, the coverage is incorrect because we're overlaying coverage from a prior commit on top of more recent code.

Potential Solution

We should probably fail to fetch coverage if we cannot find coverage for the specific commit the user is trying to view in GitHub.

scott-codecov commented 1 year ago

You can pass ?sha={current sha from GH} to the report endpoint of the Codecov API. It will return a 404 if there's no coverage for that SHA yet.

anukul commented 1 year ago

I was able to reproduce this today, while randomly browsing through some code.

The way the extension works right now, is:

  1. Read "ref" from URL (regex)
  2. Since this can be either a branch name or a commit hash, request coverage report for both cases, and use whichever request returns a successful response.

I noticed that if I request the coverage report by branch name, Codecov returns a report for a different commit hash than what the branch points to on GitHub.

Considering this, it looks like we should switch to a commit hash only approach. But if the user loads the page through a branch name, the associated commit hash is not visible anywhere on the page.

So the problem is reduced to fetching the HEAD commit on GitHub for a given branch name. This is possible through the GitHub API, but we need an access token for private repositories. Does Codecov have access to this information?

eliatcodecov commented 1 year ago

We definitely have access tokens for users that login to Codecov using GitHub. Would it make sense for there to be some new endpoint (or modify an existing one) in the Codecov API that can better fetch this information? Unsure, I'm going to defer to @scott-codecov on this one.

scott-codecov commented 1 year ago

I noticed that if I request the coverage report by branch name, Codecov returns a report for a different commit hash than what the branch points to on GitHub.

This probably means we don't have a coverage upload for the HEAD commit of that branch. Not sure there's much we can do in that case. Even if we resolved the branch name to the latest commit on GitHub, if we don't have that commit's coverage in Codecov then we can't return correct coverage info.

I was able to reproduce this today, while randomly browsing through some code.

@anukul Do you have a link where I could check this out? Just want to confirm that we're indeed missing the branch's HEAD commit in Codecov.

anukul commented 1 year ago

This probably means we don't have a coverage upload for the HEAD commit of that branch.

That makes sense. If we can switch to only using commit hashes, then the extension would show Coverage: N/A, which would be preferable to the current state.

@anukul Do you have a link where I could check this out?

Sure, here's GitHub and Codecov links.

scott-codecov commented 1 year ago

Yeah, so for that example you linked we currently have 2739f839d068241b8c58691ce6684d6ffbfd793f as the HEAD of main. GitHub says it's f7d78a6176aaff3b749daa71d82182ee40cfca30 but we don't have that commit in our database b/c a coverage upload was never made for that commit.

If I make a request to that same GitHub URL (https://github.com/cosmos/cosmos-sdk/blob/main/x/gov/genesis.go) with an Accept: application/json header I get back some info that includes:

{
    "payload": {
        "refInfo": {
            "name": "main",
            "listCacheKey": "v0:1690460331.0",
            "canEdit": true,
            "refType": "branch",
            "currentOid": "f7d78a6176aaff3b749daa71d82182ee40cfca30"
        },
        ...
    }
}

Would it be possible for the browser extension to make a fetch request like that in order to extract the branch's HEAD SHA? If we add credentials: "include" I think it will pass the GitHub user's cookies along with the request and so should presumably work for private repos as well. Could be wrong here - let me know what you think.

anukul commented 1 year ago

@scott-codecov thanks! that was very helpful. fetching commit hash from API vs extracting it from the DOM also made this part way less error prone.