Check run and Pull Reuqest

It seems to me that when a check run is created, it referenced the based and target sha. So for each commit on a branch, we have several check runs that are being shown. However, when I create a pull request for that branch, only the results from the last check run is being surfaced and shown. If I want to show all the check runs being produced on that pull request, I need to look at each commit individually.

So the question is: I would like to have the ability to see per commit but I would also live to have the pull request to reflect the globality of all the check runs. That will then give a good overview to the user of all the issues that have been surfaced on all the code modified during that code review.

How can I achieve that?

Thanks!

I’m not sure I understand what it is you’re trying to achieve. Let me describe a situation and see if I’m understanding correctly:

  1. Alice creates a branch
  2. Alice makes three commits on the branch
  3. Alice creates a pull request using that branch
  4. A check run executes on the last commit in that branch which fails with three errors
  5. Alice creates another commit and pushes to the branch
  6. A check run executes on the latest commit and fails with two errors (one from step 4 and one new one)
  7. Alice creates another commit and pushes to the branch
  8. A check run executes on the latest commit and passes

All of this can be seen on the pull request in the timeline view and details for each failure can be obtained by clicking the red X.

How would surfacing the fact that there were failures that have been fixed be better communicated? For example, on this pull request.

Let’s take the example mentioned and I will add some additional information in bold:

  1. Alice creates a branch
  2. Alice makes three commits on the branch (let’s call them commit 1, commit2 and commit 3)
  3. Alice creates a pull request using that branch. When the pull request will be created, a check suite event will be sent with revision X and Y. X is the sha of commit2, Y is the sha of commit 3). Other commits are NOT included in the check_suite event.
  4. A check run executes on the last commit in that branch which fails with three errors.  If we use the information from the check_suite to annotate the commit, we annotate only the difference between commit2 and commit3. And we annotate in that case commit3.
  5. Alice creates another commit and pushes to the branch. Let’s call that commit commit 4. That creates a check_suite event on the webhook with commit3 and commit4. Previous commit are not included in the check run.
  6. A check run executes on the latest commit and fails with two errors (one from step 4 and one new one). If we rely on the check_suite event, commit from step4 (commit 3) is not included.
  7. Alice creates another commit and pushes to the branch. Let’s call is commit5. That will also trigger a check_suite even on the webhook with commit4 and commit 5.
  8. A check run executes on the latest commit and passes.

So there is the issue: at each commit on a branch, a check suite is created. The check suite is only the diff between the previous change and the current one. And check_run are supposed to be added to the check suite. These check run are associated with the sha of the change. And *also* surfaced in the pull request view. In other words: the pull request contains check runs that can be related only to one commit (check run coming from the check suite) but also contains check runs from the pull request.

The problem is that all check runs (from check suite AND pull requests) are surfaced on the same view, giving incorrect results (these checks do not apply to the same scope of change, one taking only one commit, another takes multiple). So my question are rather:

  1. How check suite and check runs are related to a pull request? According to the documentation: "A check suite is a collection of the check runs created by a single GitHub App for a specific commit." However, how do all the check runs related to a pull request? I would expect the pull request (that contains multiple commits) view to aggregate the results of all check suite (that is only for one commit)

  2. What event and data to use to anlyze a pull request? Based on your comment, is seems that the only useful information is the pull request information and not the check suite. So what is the purpose of the check suite and can it be useful?

Thanks.

I feel like you may be misunderstanding how Git works. A commit hash, or SHA, refers to a commit in the repository. A commit in the repository contains the state of the entire repository at that point in time, it  does not reference a difference between two states. Older source control systems, such as SVN and CVS worked on a diff-based system like you describe. You can still get the difference between two commits out of Git, but that is not what is stored in Git or what is represented by a commit.

So check suites and check runs are created for specific commits, not the difference between two commits, as stated in the documentation.

To answer your questions:

How are check suites and check runs associated to a pull request?

By strict definition, they’re not. They’re associated to single commits. Some commits are associated with pull requests and there are visualizations on the PR timeline view for those check suites.

What event and data to use to analyze a pull request?

The way that GitHub is designed is that the result state of the list of check suites executed against the last commit of the pull request is the state of the pull request itself. This is because when the pull request is merged, the repository will either equal the last commit of the pull request, or very closely resemble it (in the case that there have been changes on the default branch since the pull request branch was created). So the state of the check suites executed against the last commit is the most valid predictor of the success or failure of the pull request after merging. The state of the check suites executed against any previous commit are no longer valid predictors because any problems in those previous commits could have been fixed in the latest commit. This is why the state of the check suites of the last commit are surfaced in the GitHub Pull Request UI:

Pull Request from the above screenshot

In the example above, when eff2f76 was the last commit (March 20), the state of the pull request was that it was not ready to be merged because one or more check suites that were associated with eff2f76 failed. Then, on May 31, rafeca added some more commits to the pull request that resulted in df3352c. All of the check suites associated with df3352c passed, so the pull request at df3352c is ready to be merged. Or I could go in and add some more commits that include a problem and it would go back to being not ready to merge. And so on.

So, to summarize:

  1. A check run is associated with a single commit and returns a status when complete

  2. A check suite is a collection of check runs associated with a single GitHub App and a single commit that summarizes the status of the individual check runs

  3. Multiple check suites can be executed against an individual commit

  4. The results of the check suites on any commit can be used to determine what problems might exist on that commit

  5. For example, perhaps you’re building a cross-platform application. One of the check suites might indicate a problem with the Windows build, whereas the macOS and Linux build check suites both pass.

  6. The mergeability of a pull request can be determined based on the states of the check suites of the last commit on the pull request

  7. GitHub has ways that you can indicate that certain check suites are required while others are not. For example, perhaps the build verification test check suite is required to pass whereas the check suite that calculates code coverage is not required to pass

I hope that helps and let me know if you have any more questions.

1 Like

Thank you for your detailed answer. I will read/process it and reply later.

First of all, thanks a lot for taking the time to make such a detailed reply.

I will then explain my case study so that it will be more clear. What I am doing is a code review tool: I want to make sure that the code that is shipped in a pull request is good. So the code review reviews all the code being shipped/written in a code review, surfaces all the potential problems and ultimately, gives a green/red light for shipping.

Therefore, I need to check that the code is good for all the commits for a pull request. Not only the latest one.

Let me give a simple example:

  • Step1: Alice starts a new branch. Alice add a file (code.py) that contains errors. She commits, pushes the branch and start a pull request. The check run results in a lot of warning, it does not pass.

  • Step2: Alice commit a new file on the branch and adds a new file (code2.py) that does not contains any error. The check passes.

The pull request will reflect the status of the last commit only and therefore, shows a pass (because it takes only the value of the latest commit) instead of showing the value of all the commits in the pull requests.

Does that explain the problem here? I think the problem is that the check for a code review is that we only want to analyze the subset of code being modified and not the whole repository (and this is what is also specified by a check_suite with the before and after parameters).

Note: Replied with the account that holds the application I develop :slight_smile:

Yes, I understand your problem description. You want:

  1. A method to surface the results of a check on every commit because
  2. For optimization purposes, you want to only analyze the code that was changed in your check
  3. Also, you want the pull request UI to surface all of that information

The goal of the checks system is to give feedback to the developer of the health of the codebase at the point in time of a given commit. This result can then be used to inform the developer on whether a pull request is ready to be merged or help a team enforce rules around what code is acceptable to be merged into various stages of development (signified by branches).

Keeping the above goal in mind, if we somehow surfaced in the pull request UI that the pull request consists of 38 “bad” commits and 42 “good” commits, does this help the developer make the decision of whether or not to press the Merge button?

Current

Proposed

Or maybe I’m misunderstanding your vision of what you want?

Hi @juli1

The example that you gave doesn’t seem to be correct.

Let me give a simple example:

  • Step1: Alice starts a new branch. Alice add a file (code.py) that contains errors. She commits, pushes the branch and start a pull request. The check run results in a lot of warning, it does not pass.
  • Step2: Alice commit a new file on the branch and adds a new file (code2.py) that does not contains any error. The check passes.

In this example, the check on Step2 will fail unless you have corrected the errors included in code.py in the first step. This is because commits are layered sequentially and don’t stand independent from one another. That’s why only the last commit in a chain is checked.

Hope that helps!

@lee-dohmyou totally understand the use case and got the problem :slight_smile:

Many thanks for taking the time to understand the underlying problem, I can understand it is not the common way of using checks!

I am not sure what is the “best” solution. Right now, what I am doing is to run check runs on pull-request events: anytime I have a pull-request event, my engine runs and finds issues that have been introduced between the source and target sha. Not sure this is the best way to do it tho, that is why I started this discussion.

@juli1 wrote:

I am not sure what is the “best” solution. Right now, what I am doing is to run check runs on pull-request events: anytime I have a pull-request event, my engine runs and finds issues that have been introduced between the source and target sha. Not sure this is the best way to do it tho, that is why I started this discussion.

One weakness I can see with this approach is that your engine wouldn’t find issues that existed before the source commit. So let’s say that you enhance your engine to detect new types of problems. Your engine would detect those problems when someone introduced a new one but old ones lingering in the code base would go unnoticed.

Another weakness is that sometimes issues in one piece of code can be introduced by changing some other piece of code. A simple example would be that I reference a function foo in many places in the code but then delete the function foo. Every single one of those references to foo would be an issue even though they haven’t been changed.

Perhaps how you’re analyzing things aren’t subject to these or related pitfalls, though? :man_shrugging: 

You are correct: with such an approach, we do not find previous issues. But that is not the goal: we want to find issues from what is sent. This is the approach taken by usual code review tools, such as phabricator (https://www.phacility.com/phabricator/) or reviewboard (https://www.reviewboard.org/).

The reason is the following: somebody might start to install my GitHub app on a old, legacy repository that already has 10,000 issues. No one wants to have all these issues flagged at each code review. Instead, one wants to make sure they are not adding more issues at each commit. This is why we are only inspecting the diff.

So I am staying with the current solutions but might still think of integrating the check_suite event when a change is not part of a pull request (for example, when somebody commit a change on a branch without a pull request.

Many kudos for your help and time to clarify how check_run and check_suite work

1 Like