Skip to content

"comparison of String with nil failed" when base pipeline is present but coverage is nil

Summary

Bug identified on customer call ZD link(internal)

Related to #324281 (closed) and introduced in !63079 (merged)

When evaluating whether to require approval for a merge request, we check whether the pipeline coverage is less than the base pipeline coverage if base_pipeline.present?


    def merge_requests_approved_coverage
      pipeline.merge_requests_as_head_pipeline.reject do |merge_request|
        base_pipeline = merge_request.base_pipeline

        # if base pipeline is missing we just default to not require approval.
        pipeline.coverage < base_pipeline.coverage if base_pipeline.present?
      end
    end

The problem is base_pipeline.present? only checks whether there's a base pipeline, and doesn't check whether there's coverage. As a result base_pipeline.coverage evaluates to nil and we get a NoMethodError

Steps to reproduce

From gitlab-rails console:

# Pick MR where base pipeline coverage is nil
# pipeline is current pipeline

`pipeline.coverage < base_pipeline.coverage if base_pipeline.present?`

The output will be:

comparison of String with nil failed if pipeline.coverage isn't nil NoMethodError (undefined method < for nil:NilClass) if they're both nil

Example from my console:

irb(main):036:0> current_pipeline.coverage < merge_request.base_pipeline.coverage if merge_request.base_pipeline.present?
Traceback (most recent call last):
        1: from (irb):36
NoMethodError (undefined method `<' for nil:NilClass)
irb(main):037:0>

With workaround:

irb(main):037:0> current_pipeline.coverage < merge_request.base_pipeline.coverage if merge_request.base_pipeline.coverage.present?
=> nil
irb(main):038:0>

Possible fixes

The current workaround is to check for the presence of base_pipeline.coverage rather than the presence ofbase_pipeline.

pipeline.coverage < base_pipeline.coverage if base_pipeline.coverage.present?

Edited by Brie Carranza