ignore_skipped not working as expected

Summary

If you want the pipeline status badge to only display the last non-skipped status, the "ignore_skipped" feature is not working as expected.

Steps to reproduce

  1. push to branch and pipeline succeeds -> passed
  2. push to branch with "[ci skip]" in commit message -> skipped

The badge for the pipeline should be "passed" if "?ignore_skipped=true" is appended to the badge url.

According to ~~~~~~~~~~~~https://docs.gitlab.com/ee/ci/pipelines/settings.html#display-only-non-skipped-status

According to https://docs.gitlab.com/ee/user/project/badges.html#display-only-non-skipped-status

Example Project

https://gitlab.com/benjaminfuchs/test/-/commits/master

What is the current bug behavior?

Badge shows "unknown"

What is the expected correct behavior?

"unknown" should be "passed"

Relevant logs and/or screenshots

image

Output of checks

This bug happens on GitLab.com

Possible fixes

Original Suggestion:

https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/badge/pipeline/status.rb#L31

 I seems to me "pipelines" only contains pipelines for the same commit hash. But what I would expect is to look for all pipelines of the branch and return the latest status that is not "skipped".So instead of ... 
pipelines = @project.ci_pipelines
            .where(sha: @sha)

... this would be correct:

pipelines = @project.ci_pipelines
            .where(ref: @ref)

Implementation Guide

From the changes and review notes in !59593 (closed), remove the sha filter from Gitlab::Ci::Badge::Pipeline::Status#status

-         pipelines = @project.ci_pipelines
-           .where(sha: @sha)
-
-         relation = @ignore_skipped ? pipelines.without_statuses([:skipped]) : pipelines
+         relation = @project.all_pipelines.ci_sources.for_branch(@ref)
+         relation = relation.without_statuses([:skipped]) if @ignore_skipped
          relation.latest_status(@ref) || 'unknown'

(New)* We can also do,

relation = @project.ci_pipelines.for_branch(@ref)
relation = relation.without_statuses([:skipped]) if @ignore_skipped

because @project.ci_pipelines == @project.all_pipelines.ci_sources

ci_sources:

for_branch:

latest_status:

There is a test that fails and it needs to be updated because the old_pipeline is created after the pipeline that's expected:

[6] pry(#<RSpec::ExampleGroups::GitlabCiBadgePipelineStatus::PipelineExists::WhenOutdatedPipelineForGivenRefExists>)> pipeline.id
=> 3
[7] pry(#<RSpec::ExampleGroups::GitlabCiBadgePipelineStatus::PipelineExists::WhenOutdatedPipelineForGivenRefExists>)> old_pipeline.id
=> 4

By removing the need to load commits out of the repository to compare the SHA, we can broaden our search across recent statuses in a more performant way: just searching through pipelines in the database. Since we've restricted the source types to non-dangling CI sources, sorting by pipeline ID becomes a pretty effective approximation of commit order on the ref. This was recently discussed in a Verify Technical Discussions call and we agreed it was effective enough to use.

Edited by Max Fan