Skip to content

PR test runs record a commit that was never built #1176

Description

@gaurav02081

Summary

For pull request runs, Test.commit stores the PR's head SHA, but the binary that actually runs is built from GitHub's merge ref — a merge of that head into master. The commit shown in the UI and returned by the API was never built.

Evidence

Sampled 22 completed runs from the last 100, using read-only API access only:

recorded commit == binary's own commit
master runs 8 / 8
PR runs 2 / 14

The binary reports its own commit in the build log (Git commit: in the version block). For all 12 mismatching PR runs, that commit is a merge commit whose parents are exactly (a master commit, the recorded head):

run 9490   recorded 6e1e22a3b8   binary e98f1a2f81
  e98f1a2f81  parents 128175ea7b + 6e1e22a3b8
  "Merge 6e1e22a3b84da8190629648690a49e7d9974d9ca into 128175ea7b..."

run 9486   recorded 73a7b90ac6   binary 0531cf3e8f
  0531cf3e8f  parents 128175ea7b + 73a7b90ac6

run 9474   recorded b9d6c45671   binary 5facb55537
  5facb55537  parents 6077cf5c61 + b9d6c45671

The two PR runs that did match (9492, 9493) fit the same explanation: their binary commit aee9693292 has a single parent, 128175ea7b, which was the master tip at the time. That branch was already even with master, so GitHub's merge ref resolved to the head itself and there was nothing to differ.

So: the recorded and built commits diverge whenever GitHub produces a merge commit — that is, whenever the PR branch is not already sitting on the master tip.

Cause

mod_ci/controllers.py:1994 takes the head SHA from the webhook payload:

commit_hash = payload['pull_request']['head']['sha']
...
add_test_entry(g.db, commit_hash, TestType.pull_request, pr_nr=pr_nr)

That value is right as far as it goes — schedule_tests depends on it resolving through repository.get_commit() and on comparing it against test_pr.head.sha. The gap is that nothing records the commit that was actually built.

Impact

  1. Reproduction gives you different code. Checking out the commit shown on the run page and building it does not produce the binary that ran.
  2. Master's changes are present in the tested binary but not in the recorded commit. A PR run's binary contains master commits that the PR's own base does not, so a comparison against a baseline at the recorded commit is not comparing what it appears to compare.

Correction to an earlier version of this issue: I originally cited "unrelated PRs reporting identical sets of new failures" as motivation here. That symptom is explained by #1173 — the expected file rolling forward from the previous run — not by this one. The commit mismatch above stands on its own evidence; I have removed the conflated claim.

Suggested fix

Record the built commit alongside the head rather than instead of it — e.g. a built_commit column on Test, populated from the artifact's workflow run, or parsed from the Git commit: line the binary already prints. The UI can then show "PR head X, tested as merge Y", and API consumers can tell the two apart.

Reproducing

sp run show 9490                              # recorded commit
sp run logs 9490 --all | grep 'Git commit'    # what the binary says

Found while reviewing every open CCExtractor pull request through sp_cli, without shell access to the server.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions