search

codecov / feedback

A place to discuss feedback about the pull request and web product experience.
38 stars 9 forks source link

[Docs Update] Squash and Merge workflow #369

Open aj-codecov opened 6 months ago

aj-codecov commented 6 months ago

Description

Document how we support workflows that utilize squashing and merging

Page(s)

Might be a new page that specifically focused on how we support different merging workflows

Current State

[X] Missing Information

Additional Information

Originated from a survey of our team plan users where one user called out the lack of any documentation around how we support this workflow, but don't actually let people know that we do or how best to configure Codecov in this use case.

glatterf42 commented 1 month ago

Hi :) Just jumping in here to let you know: I'm looking for exactly this information right now. Is the recommended way to handle this situation still as described here? I.e., save the coverage report as a Github Actions artifact and run a CI job on merge that downloads this artifact and uploads it to Codecov?

drazisil-codecov commented 1 month ago

Hi @glatterf42 ,

Yes, that should work. You may have to manually pass the SHA, since the one for the merge queue never gets saved to the repo.

glatterf42 commented 1 month ago

Thanks for the quick reply! Unfortunately, this workflow doesn't seem to be very straightforward to implement at first glance, so I'll try to piece it together tomorrow. If you have any recommendations or alternatives, they are much appreciated :) Here are my hiccups: GitHub says you can only download artifacts from the same workflow run, so I could not create a workflow that uploads to Codecov on merge to main simply next to my existing workflow (that runs the tests on PRs | unless I write it up manually to download the artifacts via e.g. GitHub CLI and maintain that). I would have to specify my workflow to run on merge and PR and then run the individual jobs conditionally. Also, from the list of Codecov arguments, I'm not immediately sure which of these I should use for specifying a SHA. Would that be the commit_parent or the override_commit? I'm thinking: I want to override the commit for which the upload is stored on Codecov, so likely the latter, but I will then need to figure out how to access the SHA of the commit that was just created by the most recent push to main (which always triggers squash and merge for my current use case).

I'm quite surprised there isn't an easier option like a --keep-report flag or something to the codecov-action that tells commits on main to take the coverage report of the most recent commit on a PR that was just merged. But maybe that would be assuming too much about how workflows are run in general.

drazisil-codecov commented 1 month ago

I'm quite surprised there isn't an easier option like a --keep-report flag or something to the codecov-action that tells commits on main to take the coverage report of the most recent commit on a PR that was just merged. <cc: @rohan-at-sentry / @aj-codecov

drazisil-codecov commented 1 month ago

@glatterf42 In theory, https://github.com/codecov/codecov-cli?tab=readme-ov-file#empty-upload is what is perfect for your case.

We are currently having some timing bugs that is not making it useful. :-/

https://github.com/codecov/engineering-team/issues/2365 is the issue

https://github.com/codecov/feedback/issues/108 is the docs ticket for the command.

glatterf42 commented 1 month ago

Thanks, but I'm not sure how an empty upload is going to help me (just by the name of it). Instead, I'm now thinking that storing the report with GitHub Artifacts is kind of unnecessary since we can also upload it to Codecov instead. So the workflow could go:

  1. Get coverage report on PR
  2. Upload to Codecov
  3. When merging a PR to main: get coverage report of latest commit on PR, then re-upload to Codecov using the SHA of the newly created merge (/squash) commit

This might work using the Codecov API, but unfortunately, when trying to use the codecov-cli, I first ran into the issue linked above.

glatterf42 commented 1 month ago

Or what about this:

  1. Get coverage report on PR
  2. Upload to Codecov
  3. Upload to Codecov again, but override_branch so that the commit seems to belong to main, too

Then subsequent PRs should be able to find these commits on main and compare their changes to them as long as pseudo comparison is enabled, right?

glatterf42 commented 1 month ago

I'm noticing an issue with the latest approach: if you let the checks runs once, then notice you need some further refinement, and run them again, Codecov will likely consider the report uploaded from the first checks as being on main and comparing to that, so the patch result will not reflect the changes compared to main proper.

glatterf42 commented 1 month ago

Just for completeness: I have now decided that the simplest workaround should be to run the test suite/measure coverage twice: on PRs and pushes to main. This is a tad wasteful since there's actually no change between the coverage derived from the latest commit of a PR and the squash commit on main created from that PR. Thus, I'll keep eyes on this issue to see if we can incorporate a better way later on.

drazisil-codecov commented 1 month ago

If you want Codecov to send statuses on a SHA, we have to be told the SHA exists.

As you said, the easiest way to do this it to run tests and upload on the merge SHA.

Another way, if you are using carry forward flags, is to use empty-upload to "ping" Codecov that the SHA exists, which will then populate the "empty report" with the carried forward flag values and send checks.

There are a couple trickier ways to do this, but those are the easiest two.