codecov / engineering-team

This is a general repo to use with GH Projects
1 stars 0 forks source link

Failed check status 20 - [Patch coverage] No report found to compare against. #1625

Open Adal3n3 opened 3 weeks ago

Adal3n3 commented 3 weeks ago

CI failure scenarios:

PR check status message: Your project check has failed due to BASE (commit_id) missing a report. Please upload report for the BASE.

Use-case: Patch coverage

User configurable: yes

aj-codecov commented 3 weeks ago

Should link to the specific base commit in question that's missing a report - this should NOT overwrite the first commit message.

Adal3n3 commented 3 weeks ago

It's a good point - I added a link to the base commit on the message.

giovanni-guidini commented 1 week ago

I took some artistic liberty with the message given the investigation results (in particular the fact that the check didn't fail

AND that we already have a message to indicate that we are missing the BASE report. So I merged both of them with a little ✨dazzle✨

I wanna know if it's acceptable like this or is it too much? (@Adal3n3 @aj-codecov) "upload" link: https://docs.codecov.com/docs/codecov-uploader "learn more" link: https://docs.codecov.io/docs/error-reference#section-missing-base-commit

Image

Adal3n3 commented 6 days ago

The check didn't fail but missing the BASE report. From what I know, customers might not care about this message because it's not a problem to act on so we might not need "❗️" . I think we should use this "Missing BASE" message to explain the "?"

Adal3n3 commented 6 days ago

Just making a note here from Slack 🧵: https://sentry.slack.com/archives/C04DL8SRBUZ/p1715112220384489

The "?" on the coverage table is a bit confusing as customers don't know what it causes to have a "?". Screenshot 2024-05-08 at 11 28 11 AM

Adal3n3 commented 6 days ago

@aj-codecov @giovanni-guidini What about this:

To remove the (?) from the coverage table, please upload report for the BASE (main@0470231). Lean more about missing BASE report.

giovanni-guidini commented 4 days ago

I think these messages are different and independent. It's possible to have the missing base message and not see the flags table. That would make the starting portion of the "missing base" message confusing

The "flags" table should be evolved independently is my opinion.

Adal3n3 commented 14 hours ago

Good callout @giovanni-guidini. To cover both with/without flags, we can shorten it to:

New design:

Please upload report for the BASE (0470231). Lean more about missing BASE report.