lemurheavy / coveralls-public

The public issue tracker for coveralls.io
http://coveralls.io
124 stars 7 forks source link

Github PRs: Comments working but Checks not. #1700

Open alanmcruickshank opened 1 year ago

alanmcruickshank commented 1 year ago

We're an open source project so I can share a link to live PR examples:

You'll see that Coveralls PR comment is being added and updated correctly, but in the GitHub "checks" section, there is no coveralls report. For comparison, you can see that codecov does currently have a section in the checks (that we'd love to replace with coveralls if we can, because coveralls has been more reliable in reporting back).

Ultimate aim here is to require coverage (as reported by coveralls) to be 100% to allow PRs to be merged.

Can you advise on whether this is a configuration issue or a bug? I've tried switching a the settings in the account and am unable to work out what the issue is.

afinetooth commented 1 year ago

Hi @alanmcruickshank.

The issue is that your repo is missing an owner. We leverage the OAuth token of the repo "owner," typically the user who added the repo to Coveralls.io, to perform certain Github API based actions, including sending status updates, so, without an owner, those will fail.

If you have a look at your repo settings at Coveralls: https://coveralls.io/github/sqlfluff/sqlfluff/settings

You'll see that the repo needs a new owner:

Screen Shot 2023-03-25 at 1 33 09 PM

As long as you have Admin access to the repo, you can claim ownership, and that will probably resolve the issue for you. If not, let us know and we'll reassign ownership for you. Then, once any user with a valid token and access to the repo is assigned ownership, status updates will start working again.

afinetooth commented 1 year ago

BTW, we'll be releasing a beta of a new Github-coveralls integration next month that's based on a Github App and won't require these permissions, so please look for that and, when you migrate your repos over to the new integration, you won't need to worry about this happening again.

byron-janrain commented 1 year ago

We are experiencing a similar problem but only for PR checks (push checks appear as expected). We are not missing "owners" of any repos.

afinetooth commented 1 year ago

Hi @byron-janrain, I'm aware of your issue and, If I recall correctly, it was caused by a race condition between two statuses on the same commit (you were building both push and pull_request commits on PR branches, so each commit was getting two status messages, one for push and one for pull_request, but both had the same "context" (coverage/coveralls), so one was overwriting the other. (We think the push status was overriding the pull_request status most of the time.)

While I think we suggested a temporary workaround (a CI config file change to stop building push commits on open PRs), we also started working on a change to differentiate the context of statuses if more than one is sent for the same commit, and deployed that, as a fix for the above, on Mon, Apr 24.

Either here, or (since your repo is private) to support@coveralls.io, can you please send an example of the current behavior you're seeing?

Our fix should ensure that, if we are creating both a push build and a pull_request build for you on the same commit (HEAD of PR branch), the two statuses should be distinct; and, in such a case, the status type will be shown for the push status, so the two statuses would be, for example:

  1. coverage/coveralls: (-0.1%) to 85.5%
  2. coverage/coveralls (push): (+0.25) to 85.5%

Hi @alanmcruickshank. I just checked your settings again and noticed that your repo still has no owner, which currently still prevents us from sending status updates (because the request we make to the Github API to create the status update requires a valid OAuth token, for which we use the token of your repo owner.)

Again, we are releasing a new Github Integration based on a Github App, and the beta for that actually opens next week, on Wed, May 3, so if you'd like to participate in that, let us know.

But the quick fix is to give your repo an owner.

Update: Understanding that you may not have had time to get familiar with your Repo Settings, I went ahead and made your user the owner if the repo to demonstrate that this will cause status updates to start working. (You were the only collaborator on the repo and had Admin access, so it seemed reasonable for you to be the owner.)

After doing this, I force-resent status updated for your last four (4) builds (3 pull_request builds on two (2) PRs, and one push build on main). So these PRs and this commit should now have status updates:

  1. PR #4620
  2. PR #4625
  3. Commit 48d33fd
Screenshot 2023-04-27 at 12 39 37 PM Screenshot 2023-04-27 at 12 40 00 PM Screenshot 2023-04-27 at 12 40 36 PM
byron-janrain commented 1 year ago

@afinetooth Thanks for responding so promptly! We did have that race condition issue on the coverage/coveralls check.

While I think we suggested a temporary workaround (a CI config file change to stop building push commits on open PRs), we also started working on a change to differentiate the context of statuses if more than one is sent for the same commit, and deployed that, as a fix for the above, on Mon, Apr 24.

Given the description of the fix, I'd expect to see a check for coverage/coveralls (push) and coverage/coveralls (pull_request) now right? The repos I'm looking at have workflows that are basically copies of your current actions docs on: [push, pull_request] but only (push) ever shows up. When I change it to on: [pull_request] no checks show up at all. I can't imagine that's the intended behavior.

Thank you again for your time, I've emailed support, and will pursue private details through that channel. 🙇

afinetooth commented 1 year ago

Hi @byron-janrain. Just got your email to support@coveralls.io. I'll pick up the thread there!