CC @msrb @bgoncalv this seems to be on your side of the net

This is complicated...

This happens when people rebase too quickly -- CI builds 2 scratch-builds in quick succession. Both these scratch-builds are then tested. Which result ends up in Pagure basically depends on which scratch-build/test finishes last. Older scratch-build can finish later, and test results from that older build then appear in the pull request.

This is the older scratch-build: https://osci-jenkins-1.ci.fedoraproject.org/job/fedora-ci/job/dist-git-pipeline/job/master/14264/
And this is the newer scratch-build: https://osci-jenkins-1.ci.fedoraproject.org/job/fedora-ci/job/dist-git-pipeline/job/master/14261/
The newer is green, but it finished first, so it was overriden...

The error "Couldn't find any revision to build." basically means that the branch has been rebased and the commit hash that started the testing is no longer present in the repository. So the CI is unable to locate tests...

Which result ends up in Pagure basically depends on which scratch-build/test finishes last.

Actually no, src.f.p.o is showing both old and new results. Except the green one never appeared there from some reason... so it's even more complicated :D

@msrb what about doing a check before reporting? if the result is still valid to the current state of the branch, and just report it in that case? We need to look at the problem from the user perspective, we should avoid informing him about failures he cannot do anything about. Seems this quick rebase happens, let's try to avoid confusing user.

@msrb what about doing a check before reporting? if the result is still valid to the current state of the branch, and just report it in that case? We need to look at the problem from the user perspective, we should avoid informing him about failures he cannot do anything about. Seems this quick rebase happens, let's try to avoid confusing user.

I've been thinking about this a lot. I think it's not that simple. We would need to check before we send anything, like not even "queued" message. Otherwise users will see queued test which will never be delivered. Or we could send "info" or "skipped" messages when we detect this.
We also don't work with branches or directly with pull requests -- we have the repo and hash, so we would probably need to reach out to Pagure API to figure out what's the latest commit right now. And even if we do all this, users can rebase before actual testing starts in testing farm. So it will blow up the same way some place else.

Pagure is attaching test results to commits, and this information is visible in UI. The problem is that people don't pay attention to this (can't blame them, the text is quite small, and it is not very convenient).

I think the way forward is improving how results are shown in Pagure. In RHEL, we don't show results if they are not for the latest commit in the pull request. We also remove old results when people [citest] or rebase. That helps a lot as what people see is always the latest results. So this issue here is basically non-existent downstream.

It may take some time to improve the situation, but in the mean time we can start sending those info/skipped messages.

I added the check at the beginning, before we call Testing Farm. If the commit is no longer in the repository (i.e. somebody force-pushed), we just skip everything and don't send any result.

There is still a chance that this will blow up later in Testing Farm. If that happens often, we will need to fix the service which updates the PR status in Pagure -- to ignore results that are no longer relevant for the given PR.

Thanks.

@msrb :fireworks: