Merge branch 'mo-refactor-coverage-finder' into 'master'

Refactor coverage finder [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!54486

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

@@ -71,10 +71,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