Refactor coverage finder

This commits gets rid of our existing coverage finder and
the feature flag associated with our new coverage finder.
All codepaths will now use the new finder.

app/controllers/projects/ci/daily_build_group_report_results_controller.rb

@@ -3,7 +3,6 @@
 class Projects::Ci::DailyBuildGroupReportResultsController < Projects::ApplicationController
   include Gitlab::Utils::StrongMemoize
 
-  MAX_ITEMS = 1000
   REPORT_WINDOW = 90.days
 
   before_action :authorize_read_build_report_results!
@@ -40,17 +39,13 @@ class Projects::Ci::DailyBuildGroupReportResultsController < Projects::Applicati
   end
 
   def report_results
-    if ::Gitlab::Ci::Features.use_coverage_data_new_finder?(project)
-      ::Ci::Testing::DailyBuildGroupReportResultsFinder.new(
-        params: new_finder_params,
-        current_user: current_user
-      ).execute
-    else
-      Ci::DailyBuildGroupReportResultsFinder.new(**finder_params).execute
-    end
+    ::Ci::DailyBuildGroupReportResultsFinder.new(
+      params: finder_params,
+      current_user: current_user
+    ).execute
   end
 
-  def new_finder_params
+  def finder_params
     {
       project: project,
       coverage: true,
@@ -61,17 +56,6 @@ class Projects::Ci::DailyBuildGroupReportResultsController < Projects::Applicati
     }
   end
 
-  def finder_params
-    {
-      current_user: current_user,
-      project: project,
-      ref_path: params.require(:ref_path),
-      start_date: start_date,
-      end_date: end_date,
-      limit: MAX_ITEMS
-    }
-  end
-
   def start_date
     strong_memoize(:start_date) do
       start_date = Date.parse(params.require(:start_date))

app/finders/ci/daily_build_group_report_results_finder.rb

 # frozen_string_literal: true
 
+# DailyBuildGroupReportResultsFinder
+#
+# Used to filter DailyBuildGroupReportResults by set of params
+#
+# Arguments:
+#   current_user
+#   params:
+#     project: integer
+#     group: integer
+#     coverage: boolean
+#     ref_path: string
+#     start_date: date
+#     end_date: date
+#     sort: boolean
+#     limit: integer
+
 module Ci
   class DailyBuildGroupReportResultsFinder
     include Gitlab::Allowable
 
-    def initialize(current_user:, project:, ref_path: nil, start_date:, end_date:, limit: nil)
+    MAX_ITEMS = 1_000
+
+    attr_reader :params, :current_user
+
+    def initialize(params: {}, current_user: nil)
+      @params = params
       @current_user = current_user
-      @project = project
-      @ref_path = ref_path
-      @start_date = start_date
-      @end_date = end_date
-      @limit = limit
     end
 
     def execute
-      return none unless query_allowed?
+      return Ci::DailyBuildGroupReportResult.none unless query_allowed?
 
-      query
+      collection = Ci::DailyBuildGroupReportResult.by_projects(params[:project])
+      collection = filter_report_results(collection)
+      collection
     end
 
-    protected
+    private
 
-    attr_reader :current_user, :project, :ref_path, :start_date, :end_date, :limit
+    def query_allowed?
+      can?(current_user, :read_build_report_results, params[:project])
+    end
+
+    def filter_report_results(collection)
+      collection = by_coverage(collection)
+      collection = by_ref_path(collection)
+      collection = by_dates(collection)
 
-    def query
-      Ci::DailyBuildGroupReportResult.recent_results(
-        query_params,
-        limit: limit
-      )
+      collection = sort(collection)
+      collection = limit_by(collection)
+      collection
     end
 
-    def query_allowed?
-      can?(current_user, :read_build_report_results, project)
+    def by_coverage(items)
+      params[:coverage].present? ? items.with_coverage : items
     end
 
-    def query_params
-      params = {
-        project_id: project,
-        date: start_date..end_date
-      }
+    def by_ref_path(items)
+      params[:ref_path].present? ? items.by_ref_path(params[:ref_path]) : items.with_default_branch
+    end
+
+    def by_dates(items)
+      params[:start_date].present? && params[:end_date].present? ? items.by_dates(params[:start_date], params[:end_date]) : items
+    end
 
-      if ref_path.present?
-        params[:ref_path] = ref_path
-      else
-        params[:default_branch] = true
-      end
+    def sort(items)
+      params[:sort].present? ? items.ordered_by_date_and_group_name : items
+    end
 
-      params
+    # rubocop: disable CodeReuse/ActiveRecord
+    def limit_by(items)
+      items.limit(limit)
     end
+    # rubocop: enable CodeReuse/ActiveRecord
+
+    def limit
+      return MAX_ITEMS unless params[:limit].present?
 
-    def none
-      Ci::DailyBuildGroupReportResult.none
+      [params[:limit].to_i, MAX_ITEMS].min
     end
   end
 end
+
+Ci::DailyBuildGroupReportResultsFinder.prepend_if_ee('::EE::Ci::DailyBuildGroupReportResultsFinder')

app/finders/ci/testing/daily_build_group_report_results_finder.rb

-# frozen_string_literal: true
-
-# DailyBuildGroupReportResultsFinder
-#
-# Used to filter DailyBuildGroupReportResults by set of params
-#
-# Arguments:
-#   current_user
-#   params:
-#     project: integer
-#     group: integer
-#     coverage: boolean
-#     ref_path: string
-#     start_date: date
-#     end_date: date
-#     sort: boolean
-#     limit: integer
-
-module Ci
-  module Testing
-    class DailyBuildGroupReportResultsFinder
-      include Gitlab::Allowable
-
-      MAX_ITEMS = 1_000
-
-      attr_reader :params, :current_user
-
-      def initialize(params: {}, current_user: nil)
-        @params = params
-        @current_user = current_user
-      end
-
-      def execute
-        return Ci::DailyBuildGroupReportResult.none unless query_allowed?
-
-        collection = Ci::DailyBuildGroupReportResult.by_projects(params[:project])
-        collection = filter_report_results(collection)
-        collection
-      end
-
-      private
-
-      def query_allowed?
-        can?(current_user, :read_build_report_results, params[:project])
-      end
-
-      def filter_report_results(collection)
-        collection = by_coverage(collection)
-        collection = by_ref_path(collection)
-        collection = by_dates(collection)
-
-        collection = sort(collection)
-        collection = limit_by(collection)
-        collection
-      end
-
-      def by_coverage(items)
-        params[:coverage].present? ? items.with_coverage : items
-      end
-
-      def by_ref_path(items)
-        params[:ref_path].present? ? items.by_ref_path(params[:ref_path]) : items.with_default_branch
-      end
-
-      def by_dates(items)
-        params[:start_date].present? && params[:end_date].present? ? items.by_dates(params[:start_date], params[:end_date]) : items
-      end
-
-      def sort(items)
-        params[:sort].present? ? items.ordered_by_date_and_group_name : items
-      end
-
-      # rubocop: disable CodeReuse/ActiveRecord
-      def limit_by(items)
-        items.limit(limit)
-      end
-      # rubocop: enable CodeReuse/ActiveRecord
-
-      def limit
-        return MAX_ITEMS unless params[:limit].present?
-
-        [params[:limit].to_i, MAX_ITEMS].min
-      end
-    end
-  end
-end
-
-Ci::Testing::DailyBuildGroupReportResultsFinder.prepend_if_ee('::EE::Ci::Testing::DailyBuildGroupReportResultsFinder')

app/models/ci/daily_build_group_report_result.rb

@@ -30,10 +30,6 @@ module Ci
         upsert_all(data, unique_by: :index_daily_build_group_report_results_unique_columns) if data.any?
       end
 
-      def recent_results(attrs, limit: nil)
-        where(attrs).order(date: :desc, group_name: :asc).limit(limit)
-      end
-
       def report_window(start_date)
         default_date = REPORT_WINDOW.ago.to_date
         date = Date.parse(start_date) rescue default_date

changelogs/unreleased/mo-refactor-coverage-finder.yml

+---
+title: Remove coverage_data_new_finder feature flag
+merge_request: 54486
+author:
+type: performance

config/feature_flags/development/coverage_data_new_finder.yml

----
-name: coverage_data_new_finder
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/53670
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/301093
-milestone: '13.9'
-type: development
-group: group::testing
-default_enabled: false

ee/app/controllers/groups/analytics/coverage_reports_controller.rb

@@ -32,17 +32,13 @@ class Groups::Analytics::CoverageReportsController < Groups::Analytics::Applicat
   end
 
   def report_results
-    if ::Gitlab::Ci::Features.use_coverage_data_new_finder?(@group)
-      ::Ci::Testing::DailyBuildGroupReportResultsFinder.new(
-        params: new_finder_params,
-        current_user: current_user
-      ).execute
-    else
-      Ci::DailyBuildGroupReportResultsByGroupFinder.new(**finder_params).execute
-    end
+    ::Ci::DailyBuildGroupReportResultsFinder.new(
+      params: finder_params,
+      current_user: current_user
+    ).execute
   end
 
-  def new_finder_params
+  def finder_params
     {
       group: @group,
       coverage: true,
@@ -53,17 +49,6 @@ class Groups::Analytics::CoverageReportsController < Groups::Analytics::Applicat
     }
   end
 
-  def finder_params
-    {
-      current_user: current_user,
-      group: @group,
-      project_ids: params.permit(project_ids: [])[:project_ids],
-      ref_path: params[:ref_path],
-      start_date: Date.parse(params.require(:start_date)),
-      end_date: Date.parse(params.require(:end_date))
-    }
-  end
-
   def download_tracker_params
     {
       label: 'group_id',

ee/app/finders/ci/daily_build_group_report_results_by_group_finder.rb

-# frozen_string_literal: true
-
-module Ci
-  class DailyBuildGroupReportResultsByGroupFinder < Ci::DailyBuildGroupReportResultsFinder
-    include Gitlab::Allowable
-
-    # We currently impose a maximum of 1000 returned records for performance reasons.
-    # This limit is subject to future removal.
-    # See thread: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/37768#note_386839633
-    GROUP_QUERY_RESULT_LIMIT = 1000.freeze
-
-    def initialize(current_user:, group:, project_ids: [], ref_path: nil, start_date:, end_date:, limit: nil)
-      super(current_user: current_user, project: nil, ref_path: ref_path, start_date: start_date, end_date: end_date, limit: limit)
-
-      @group = group
-      @project_ids = Array(project_ids)
-      @limit = GROUP_QUERY_RESULT_LIMIT unless limit && limit < GROUP_QUERY_RESULT_LIMIT
-    end
-
-    private
-
-    def query
-      Ci::DailyBuildGroupReportResult.with_included_projects.recent_results(
-        query_params,
-        limit: limit
-      )
-    end
-
-    def query_allowed?
-      can?(current_user, :read_group_build_report_results, @group)
-    end
-
-    def query_params
-      super.merge(project_id: project_id_subquery)
-    end
-
-    def project_id_subquery
-      if @project_ids.empty?
-        @group.projects.select(:id)
-      else
-        @group.projects.including_project(@project_ids).select(:id)
-      end
-    end
-  end
-end

ee/app/finders/ee/ci/daily_build_group_report_results_finder.rb

+# frozen_string_literal: true
+
+module EE
+  module Ci
+    # DailyBuildGroupReportResultsFinder
+    #
+    # Extends DailyBuildGroupReportResultsFinder
+    #
+    # Added arguments:
+    #   params:
+    #     group: integer
+    module DailyBuildGroupReportResultsFinder
+      extend ::Gitlab::Utils::Override
+
+      override :execute
+      def execute
+        return super unless params[:group]
+        return ::Ci::DailyBuildGroupReportResult.none unless query_allowed?
+
+        collection = ::Ci::DailyBuildGroupReportResult.by_group(params[:group])
+        collection = filter_report_results(collection)
+        collection
+      end
+
+      private
+
+      override :query_allowed?
+      def query_allowed?
+        return super unless params[:group]
+
+        can?(current_user, :read_group_build_report_results, params[:group])
+      end
+    end
+  end
+end

ee/app/finders/ee/ci/testing/daily_build_group_report_results_finder.rb

-# frozen_string_literal: true
-
-module EE
-  module Ci
-    module Testing
-      # DailyBuildGroupReportResultsFinder
-      #
-      # Extends DailyBuildGroupReportResultsFinder
-      #
-      # Added arguments:
-      #   params:
-      #     group: integer
-      module DailyBuildGroupReportResultsFinder
-        extend ::Gitlab::Utils::Override
-
-        override :execute
-        def execute
-          return super unless params[:group]
-          return ::Ci::DailyBuildGroupReportResult.none unless query_allowed?
-
-          collection = ::Ci::DailyBuildGroupReportResult.by_group(params[:group])
-          collection = filter_report_results(collection)
-          collection
-        end
-
-        private
-
-        override :query_allowed?
-        def query_allowed?
-          return super unless params[:group]
-
-          can?(current_user, :read_group_build_report_results, params[:group])
-        end
-      end
-    end
-  end
-end

ee/spec/controllers/groups/analytics/coverage_reports_controller_spec.rb

@@ -59,96 +59,54 @@ RSpec.describe Groups::Analytics::CoverageReportsController do
         stub_licensed_features(group_coverage_reports: true)
       end
 
-      context 'when feature coverage_data_new_finder is enabled' do
-        before do
-          stub_feature_flags(coverage_data_new_finder: true)
-        end
-
-        it 'responds 200 with CSV coverage data' do
-          get :index, params: valid_request_params
+      it 'responds 200 with CSV coverage data', :snowplow do
+        get :index, params: valid_request_params
 
-          expect(response).to have_gitlab_http_status(:ok)
-          expect(csv_response).to eq([
-            %w[date group_name project_name coverage],
-            [last_coverage.date.to_s, last_coverage.group_name, project.name, last_coverage.data['coverage'].to_s],
-            [first_coverage.date.to_s, first_coverage.group_name, project.name, first_coverage.data['coverage'].to_s]
-          ])
-        end
+        expect_snowplow_event(
+          category: described_class.name,
+          action: 'download_code_coverage_csv',
+          label: 'group_id',
+          value: group.id
+        )
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(csv_response).to eq([
+          %w[date group_name project_name coverage],
+          [last_coverage.date.to_s, last_coverage.group_name, project.name, last_coverage.data['coverage'].to_s],
+          [first_coverage.date.to_s, first_coverage.group_name, project.name, first_coverage.data['coverage'].to_s]
+        ])
       end
 
-      context 'when feature coverage_data_new_finder is disabled' do
-        before do
-          stub_feature_flags(coverage_data_new_finder: false)
-        end
+      context 'when ref_path is nil' do
+        let(:ref_path) { nil }
 
-        it 'responds 200 with CSV coverage data', :snowplow do
+        it 'responds HTTP 200' do
           get :index, params: valid_request_params
 
-          expect_snowplow_event(
-            category: described_class.name,
-            action: 'download_code_coverage_csv',
-            label: 'group_id',
-            value: group.id
-          )
-
           expect(response).to have_gitlab_http_status(:ok)
-          expect(csv_response).to eq([
-            %w[date group_name project_name coverage],
-            [last_coverage.date.to_s, last_coverage.group_name, project.name, last_coverage.data['coverage'].to_s],
-            [first_coverage.date.to_s, first_coverage.group_name, project.name, first_coverage.data['coverage'].to_s]
-          ])
-        end
-
-        context 'with a project_id filter' do
-          let(:params) { valid_request_params.merge(project_ids: [project.id]) }
-
-          it 'responds 200 with CSV coverage data' do
-            expect(Ci::DailyBuildGroupReportResultsByGroupFinder).to receive(:new).with({
-              group: group,
-              current_user: user,
-              project_ids: [project.id.to_s],
-              start_date: Date.parse('2020-03-01'),
-              end_date: Date.parse('2020-03-31'),
-              ref_path: ref_path
-            }).and_call_original
-
-            get :index, params: params
-
-            expect(response).to have_gitlab_http_status(:ok)
-          end
+          expect(csv_response.size).to eq(3)
         end
+      end
 
-        context 'when ref_path is nil' do
-          let(:ref_path) { nil }
-
-          it 'responds HTTP 200' do
-            get :index, params: valid_request_params
-
-            expect(response).to have_gitlab_http_status(:ok)
-            expect(csv_response.size).to eq(3)
-          end
+      it 'executes the same number of queries regardless of the number of records returned' do
+        control = ActiveRecord::QueryRecorder.new do
+          get :index, params: valid_request_params
         end
 
-        it 'executes the same number of queries regardless of the number of records returned' do
-          control = ActiveRecord::QueryRecorder.new do
-            get :index, params: valid_request_params
-          end
-
-          expect(CSV.parse(response.body).length).to eq(3)
+        expect(CSV.parse(response.body).length).to eq(3)
 
-          create_daily_coverage('rspec', project, 79.0, '2020-03-10')
+        create_daily_coverage('rspec', project, 79.0, '2020-03-10', group)
 
-          expect { get :index, params: valid_request_params }.not_to exceed_query_limit(control)
+        expect { get :index, params: valid_request_params }.not_to exceed_query_limit(control)
 
-          expect(csv_response.length).to eq(4)
-        end
+        expect(csv_response.length).to eq(4)
+      end
 
-        context 'with an invalid format' do
-          it 'responds 404' do
-            get :index, params: valid_request_params.merge(format: :json)
+      context 'with an invalid format' do
+        it 'responds 404' do
+          get :index, params: valid_request_params.merge(format: :json)
 
-            expect(response).to have_gitlab_http_status(:not_found)
-          end
+          expect(response).to have_gitlab_http_status(:not_found)
         end
       end
     end

ee/spec/finders/ci/daily_build_group_report_results_by_group_finder_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Ci::DailyBuildGroupReportResultsByGroupFinder do
-  describe '#execute' do
-    let_it_be(:user) { create(:user) }
-
-    let_it_be(:project) { create(:project, :private) }
-    let_it_be(:project_coverage) { create_daily_coverage('rspec', 95.0, '2020-03-10', project) }
-
-    let_it_be(:group) { create(:group, :private) }
-    let_it_be(:rspec_project) { create(:project, namespace: group) }
-    let_it_be(:rspec_project_coverage) { create_daily_coverage('rspec', 79.0, '2020-03-09', rspec_project) }
-
-    let_it_be(:karma_project) { create(:project, namespace: group) }
-    let_it_be(:karma_project_coverage) { create_daily_coverage('karma', 89.0, '2020-03-09', karma_project) }
-
-    let_it_be(:generic_project) { create(:project, namespace: group) }
-    let_it_be(:generic_coverage) { create_daily_coverage('unreported', 95.0, '2020-03-10', generic_project) }
-
-    let_it_be(:subgroup) { create(:group, :private, parent: group) }
-    let_it_be(:subgroup_project) { create(:project, namespace: subgroup) }
-    let_it_be(:subgroup_project_coverage) { create_daily_coverage('rspec', 89.0, '2020-03-09', subgroup_project) }
-
-    let(:limit) { nil }
-    let(:project_ids) { nil }
-
-    let(:attributes) do
-      {
-        current_user: user,
-        group: group,
-        project_ids: project_ids,
-        ref_path: 'refs/heads/master',
-        start_date: '2020-03-09',
-        end_date: '2020-03-10',
-        limit: limit
-      }
-    end
-
-    subject do
-      described_class.new(**attributes).execute
-    end
-
-    context 'when current user is allowed to :read_group_build_report_results' do
-      before do
-        group.add_reporter(user)
-      end
-
-      it 'returns only coverages belonging to the passed group' do
-        expect(subject).to include(rspec_project_coverage, karma_project_coverage)
-        expect(subject).not_to include(project_coverage)
-        expect(subject).not_to include(subgroup_project_coverage)
-      end
-
-      context 'with a limit below 1000' do
-        let(:limit) { 5 }
-
-        it 'uses the provided limit' do
-          expect(subject.limit_value).to eq(5)
-        end
-      end
-
-      context 'with a limit above 1000' do
-        let(:limit) { 1001 }
-
-        it 'uses the max constant' do
-          expect(subject.limit_value).to eq(Ci::DailyBuildGroupReportResultsByGroupFinder::GROUP_QUERY_RESULT_LIMIT)
-        end
-      end
-
-      context 'without a limit' do
-        it 'uses the max constant' do
-          expect(subject.limit_value).to eq(Ci::DailyBuildGroupReportResultsByGroupFinder::GROUP_QUERY_RESULT_LIMIT)
-        end
-      end
-
-      context 'with nil project_ids' do
-        it 'returns only coverages belonging to the passed group' do
-          expect(subject).to match_array([rspec_project_coverage, karma_project_coverage, generic_coverage])
-          expect(subject).not_to include(project_coverage)
-          expect(subject).not_to include(subgroup_project_coverage)
-        end
-      end
-
-      context 'with passed project_ids' do
-        let(:project_ids) { [rspec_project.id] }
-
-        it 'filters out non-specified projects' do
-          expect(subject).to match_array([rspec_project_coverage])
-          expect(subject).not_to include(karma_project_coverage, generic_coverage)
-        end
-      end
-
-      context 'with empty project_ids' do
-        let(:project_ids) { [] }
-
-        it 'returns all projects' do
-          expect(subject).to match_array([rspec_project_coverage, karma_project_coverage, generic_coverage])
-        end
-      end
-    end
-
-    context 'without permmissions' do
-      it 'returns an empty result' do
-        expect(subject).to be_empty
-      end
-    end
-  end
-
-  private
-
-  def create_daily_coverage(group_name, coverage, date, project)
-    create(
-      :ci_daily_build_group_report_result,
-      project: project,
-      ref_path: 'refs/heads/master',
-      group_name: group_name,
-      data: { 'coverage' => coverage },
-      date: date
-    )
-  end
-end

ee/spec/finders/ee/ci/testing/daily_build_group_report_results_finder_spec.rb → ee/spec/finders/ee/ci/daily_build_group_report_results_finder_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Ci::Testing::DailyBuildGroupReportResultsFinder do
+RSpec.describe Ci::DailyBuildGroupReportResultsFinder do
   describe '#execute' do
     let_it_be(:current_user) { create(:user) }
     let_it_be(:group) { create(:group, :private) }
@@ -56,13 +56,13 @@ RSpec.describe Ci::Testing::DailyBuildGroupReportResultsFinder do
         let(:limit) { 1001 }
 
         it 'returns MAX_ITEMS as a limit' do
-          expect(coverages.limit_value).to eq(Ci::Testing::DailyBuildGroupReportResultsFinder::MAX_ITEMS)
+          expect(coverages.limit_value).to eq(Ci::DailyBuildGroupReportResultsFinder::MAX_ITEMS)
         end
       end
 
       context 'without a limit' do
         it 'returns MAX_ITEMS as a limit' do
-          expect(coverages.limit_value).to eq(Ci::Testing::DailyBuildGroupReportResultsFinder::MAX_ITEMS)
+          expect(coverages.limit_value).to eq(Ci::DailyBuildGroupReportResultsFinder::MAX_ITEMS)
         end
       end
     end

lib/gitlab/ci/features.rb

@@ -75,10 +75,6 @@ module Gitlab
       def self.display_codequality_backend_comparison?(project)
         ::Feature.enabled?(:codequality_backend_comparison, project, default_enabled: :yaml)
       end
-
-      def self.use_coverage_data_new_finder?(record)
-        ::Feature.enabled?(:coverage_data_new_finder, record, default_enabled: :yaml)
-      end
     end
   end
 end

spec/controllers/projects/ci/daily_build_group_report_results_controller_spec.rb

@@ -4,29 +4,25 @@ require 'spec_helper'
 
 RSpec.describe Projects::Ci::DailyBuildGroupReportResultsController do
   describe 'GET index' do
-    let(:project) { create(:project, :public, :repository) }
-    let(:ref_path) { 'refs/heads/master' }
-    let(:param_type) { 'coverage' }
-    let(:start_date) { '2019-12-10' }
-    let(:end_date) { '2020-03-09' }
-    let(:allowed_to_read) { true }
-    let(:user) { create(:user) }
-    let(:feature_enabled?) { true }
+    let_it_be(:project) { create(:project, :public, :repository) }
+    let_it_be(:ref_path) { 'refs/heads/master' }
+    let_it_be(:param_type) { 'coverage' }
+    let_it_be(:start_date) { '2019-12-10' }
+    let_it_be(:end_date) { '2020-03-09' }
+    let_it_be(:allowed_to_read) { true }
+    let_it_be(:user) { create(:user) }
+    let_it_be(:rspec_coverage_1) { create_daily_coverage('rspec', 79.0, '2020-03-09') }
+    let_it_be(:rspec_coverage_2) { create_daily_coverage('rspec', 77.0, '2020-03-08') }
+    let_it_be(:karma_coverage) { create_daily_coverage('karma', 81.0, '2019-12-10') }
+    let_it_be(:minitest_coverage) { create_daily_coverage('minitest', 67.0, '2019-12-09') }
+    let_it_be(:mocha_coverage) { create_daily_coverage('mocha', 71.0, '2019-12-09') }
 
     before do
-      create_daily_coverage('rspec', 79.0, '2020-03-09')
-      create_daily_coverage('rspec', 77.0, '2020-03-08')
-      create_daily_coverage('karma', 81.0, '2019-12-10')
-      create_daily_coverage('minitest', 67.0, '2019-12-09')
-      create_daily_coverage('mocha', 71.0, '2019-12-09')
-
       sign_in(user)
 
       allow(Ability).to receive(:allowed?).and_call_original
       allow(Ability).to receive(:allowed?).with(user, :read_build_report_results, project).and_return(allowed_to_read)
 
-      stub_feature_flags(coverage_data_new_finder: feature_enabled?)
-
       get :index, params: {
         namespace_id: project.namespace,
         project_id: project,
@@ -140,33 +136,13 @@ RSpec.describe Projects::Ci::DailyBuildGroupReportResultsController do
     context 'when format is JSON' do
       let(:format) { :json }
 
-      context 'when coverage_data_new_finder flag is enabled' do
-        let(:feature_enabled?) { true }
-
-        it_behaves_like 'JSON results'
-      end
-
-      context 'when coverage_data_new_finder flag is disabled' do
-        let(:feature_enabled?) { false }
-
-        it_behaves_like 'JSON results'
-      end
+      it_behaves_like 'JSON results'
     end
 
     context 'when format is CSV' do
       let(:format) { :csv }
 
-      context 'when coverage_data_new_finder flag is enabled' do
-        let(:feature_enabled?) { true }
-
-        it_behaves_like 'CSV results'
-      end
-
-      context 'when coverage_data_new_finder flag is disabled' do
-        let(:feature_enabled?) { false }
-
-        it_behaves_like 'CSV results'
-      end
+      it_behaves_like 'CSV results'
     end
   end
 

spec/finders/ci/daily_build_group_report_results_finder_spec.rb

@@ -5,10 +5,14 @@ require 'spec_helper'
 RSpec.describe Ci::DailyBuildGroupReportResultsFinder do
   describe '#execute' do
     let_it_be(:project) { create(:project, :private) }
-    let_it_be(:current_user) { project.owner }
+    let(:user_without_permission) { create(:user) }
+    let_it_be(:user_with_permission) { project.owner }
     let_it_be(:ref_path) { 'refs/heads/master' }
     let(:limit) { nil }
     let_it_be(:default_branch) { false }
+    let(:start_date) { '2020-03-09' }
+    let(:end_date) { '2020-03-10' }
+    let(:sort) { true }
 
     let_it_be(:rspec_coverage_1) { create_daily_coverage('rspec', 79.0, '2020-03-09') }
     let_it_be(:karma_coverage_1) { create_daily_coverage('karma', 89.0, '2020-03-09') }
@@ -17,24 +21,35 @@ RSpec.describe Ci::DailyBuildGroupReportResultsFinder do
     let_it_be(:rspec_coverage_3) { create_daily_coverage('rspec', 97.0, '2020-03-11') }
     let_it_be(:karma_coverage_3) { create_daily_coverage('karma', 99.0, '2020-03-11') }
 
-    let(:attributes) do
+    let(:finder) { described_class.new(params: params, current_user: current_user) }
+
+    let(:params) do
       {
-        current_user: current_user,
         project: project,
+        coverage: true,
         ref_path: ref_path,
-        start_date: '2020-03-09',
-        end_date: '2020-03-10',
-        limit: limit
+        start_date: start_date,
+        end_date: end_date,
+        limit: limit,
+        sort: sort
       }
     end
 
-    subject(:coverages) do
-      described_class.new(**attributes).execute
-    end
+    subject(:coverages) { finder.execute }
+
+    context 'when params are provided' do
+      context 'when current user is not allowed to read data' do
+        let(:current_user) { user_without_permission }
+
+        it 'returns an empty collection' do
+          expect(coverages).to be_empty
+        end
+      end
 
-    context 'when ref_path is present' do
-      context 'when current user is allowed to read build report results' do
-        it 'returns all matching results within the given date range' do
+      context 'when current user is allowed to read data' do
+        let(:current_user) { user_with_permission }
+
+        it 'returns matching coverages within the given date range' do
           expect(coverages).to match_array([
             karma_coverage_2,
             rspec_coverage_2,
@@ -43,10 +58,21 @@ RSpec.describe Ci::DailyBuildGroupReportResultsFinder do
           ])
         end
 
-        context 'and limit is specified' do
+        context 'when ref_path is nil' do
+          let(:default_branch) { true }
+          let(:ref_path) { nil }
+
+          it 'returns coverages for the default branch' do
+            rspec_coverage_4 = create_daily_coverage('rspec', 66.0, '2020-03-10')
+
+            expect(coverages).to contain_exactly(rspec_coverage_4)
+          end
+        end
+
+        context 'when limit is specified' do
           let(:limit) { 2 }
 
-          it 'returns limited number of matching results within the given date range' do
+          it 'returns limited number of matching coverages within the given date range' do
             expect(coverages).to match_array([
               karma_coverage_2,
               rspec_coverage_2
@@ -54,28 +80,6 @@ RSpec.describe Ci::DailyBuildGroupReportResultsFinder do
           end
         end
       end
-
-      context 'when current user is not allowed to read build report results' do
-        let(:current_user) { create(:user) }
-
-        it 'returns an empty result' do
-          expect(coverages).to be_empty
-        end
-      end
-    end
-
-    context 'when ref_path query parameter is not present' do
-      let(:ref_path) { nil }
-
-      context 'when records with cover data from the default branch exist' do
-        let(:default_branch) { true }
-
-        it 'returns records with default_branch:true, irrespective of ref_path' do
-          rspec_coverage_4 = create_daily_coverage('rspec', 66.0, '2020-03-10')
-
-          expect(coverages).to contain_exactly(rspec_coverage_4)
-        end
-      end
     end
   end
 

spec/finders/ci/testing/daily_build_group_report_results_finder_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Ci::Testing::DailyBuildGroupReportResultsFinder do
-  describe '#execute' do
-    let_it_be(:project) { create(:project, :private) }
-    let(:user_without_permission) { create(:user) }
-    let_it_be(:user_with_permission) { project.owner }
-    let_it_be(:ref_path) { 'refs/heads/master' }
-    let(:limit) { nil }
-    let_it_be(:default_branch) { false }
-    let(:start_date) { '2020-03-09' }
-    let(:end_date) { '2020-03-10' }
-    let(:sort) { true }
-
-    let_it_be(:rspec_coverage_1) { create_daily_coverage('rspec', 79.0, '2020-03-09') }
-    let_it_be(:karma_coverage_1) { create_daily_coverage('karma', 89.0, '2020-03-09') }
-    let_it_be(:rspec_coverage_2) { create_daily_coverage('rspec', 95.0, '2020-03-10') }
-    let_it_be(:karma_coverage_2) { create_daily_coverage('karma', 92.0, '2020-03-10') }
-    let_it_be(:rspec_coverage_3) { create_daily_coverage('rspec', 97.0, '2020-03-11') }
-    let_it_be(:karma_coverage_3) { create_daily_coverage('karma', 99.0, '2020-03-11') }
-
-    let(:finder) { described_class.new(params: params, current_user: current_user) }
-
-    let(:params) do
-      {
-        project: project,
-        coverage: true,
-        ref_path: ref_path,
-        start_date: start_date,
-        end_date: end_date,
-        limit: limit,
-        sort: sort
-      }
-    end
-
-    subject(:coverages) { finder.execute }
-
-    context 'when params are provided' do
-      context 'when current user is not allowed to read data' do
-        let(:current_user) { user_without_permission }
-
-        it 'returns an empty collection' do
-          expect(coverages).to be_empty
-        end
-      end
-
-      context 'when current user is allowed to read data' do
-        let(:current_user) { user_with_permission }
-
-        it 'returns matching coverages within the given date range' do
-          expect(coverages).to match_array([
-            karma_coverage_2,
-            rspec_coverage_2,
-            karma_coverage_1,
-            rspec_coverage_1
-          ])
-        end
-
-        context 'when ref_path is nil' do
-          let(:default_branch) { true }
-          let(:ref_path) { nil }
-
-          it 'returns coverages for the default branch' do
-            rspec_coverage_4 = create_daily_coverage('rspec', 66.0, '2020-03-10')
-
-            expect(coverages).to contain_exactly(rspec_coverage_4)
-          end
-        end
-
-        context 'when limit is specified' do
-          let(:limit) { 2 }
-
-          it 'returns limited number of matching coverages within the given date range' do
-            expect(coverages).to match_array([
-              karma_coverage_2,
-              rspec_coverage_2
-            ])
-          end
-        end
-      end
-    end
-  end
-
-  private
-
-  def create_daily_coverage(group_name, coverage, date)
-    create(
-      :ci_daily_build_group_report_result,
-      project: project,
-      ref_path: ref_path || 'feature-branch',
-      group_name: group_name,
-      data: { 'coverage' => coverage },
-      date: date,
-      default_branch: default_branch
-    )
-  end
-end