Skip to content

GenerateCoverageReportsService: NoMethodError: undefined method `present' for nil:NilClass

Bug report

https://sentry.gitlab.net/gitlab/gitlabcom/issues/2523536/?referrer=gitlab_plugin

NoMethodError: undefined method `present' for nil:NilClass
Did you mean?  present?
               presence
  ci/generate_coverage_reports_service.rb:15:in `execute'
    data: head_pipeline.pipeline_artifacts.find_by_file_type(:code_coverage).present.for_files(merge_request.new_paths)
  merge_request.rb:1637:in `calculate_reactive_cache'
    service_class.new(project, current_user, id: id, report_type: report_type).execute(comparison_base_pipeline(identifier), actual_head_pipeline)
  gitlab/metrics/instrumentation.rb:162:in `calculate_reactive_cache'
    super
  reactive_caching.rb:94:in `block (2 levels) in exclusively_update_reactive_cache!'
    new_value = calculate_reactive_cache(*args)
  reactive_caching.rb:153:in `enqueuing_update'
    yield
...
(88 additional frame(s) were not displayed)

Proposal

Invalidate the cached data as soon as the new head pipeline is created (current behavior), but delay repopulation of the cache with new data until the new pipeline has completed and has (or definitively does not have) a new CoverageReport artifact. During the execution of the new head pipeline, no diff data will be displayed on a merge request. This is in line with other artifact data comparisons, such as the Ci::CompareTestReportsService that simply returns no data if the head pipeline does not have an artifact.

  class GenerateCoverageReportsService < CompareReportsBaseService
    def execute(base_pipeline, head_pipeline)
+     return unless head_pipeline.has_coverage_reports? # no-op this whole method if we don't have a report. No data written back to the reactive cache.

      merge_request = MergeRequest.find_by_id(params[:id])

      {
        status: :parsed,
        key: key(base_pipeline, head_pipeline),
        data: head_pipeline.pipeline_artifacts.find_by_file_type(:code_coverage).present.for_files(merge_request.new_paths)
      }

By returning no data to be written by ReactiveCache, the with_reactive_cache method call from MergeRequest#compare_reports will simply return nil, and the merge request will return status: parsing, the same as it does with all the other report types.

Homework and the other proposal we decided against are in the comments.

Edited by drew stachon