diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index ef22b429df9c91191bfc4d7abdad919d3424e633..6d32301ff444aa7a412c10a14376e661d3248e53 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -72,6 +72,7 @@ class Pipeline < ApplicationRecord }, class_name: 'Ci::Ref', inverse_of: :pipelines has_one :chat_data, class_name: 'Ci::PipelineChatData' + has_one :processed_coverage_report, class_name: 'Ci::PipelineProcessedReport' has_many :triggered_pipelines, through: :sourced_pipelines, source: :pipeline has_many :child_pipelines, -> { merge(Ci::Sources::Pipeline.same_project) }, through: :sourced_pipelines, source: :pipeline @@ -216,6 +217,10 @@ class Pipeline < ApplicationRecord AutoMergeProcessWorker.perform_async(merge_request.id) end + if pipeline.has_reports?(Ci::JobArtifact.coverage_reports) + Ci::PipelineCoverageReportWorker.perform_async(pipeline.id) + end + if pipeline.auto_devops_source? self.class.auto_devops_pipelines_completed_total.increment(status: pipeline.status) end diff --git a/app/models/ci/pipeline_processed_report.rb b/app/models/ci/pipeline_processed_report.rb new file mode 100644 index 0000000000000000000000000000000000000000..fc232a8c6769f1d3d8fa297530ac1edff63e7083 --- /dev/null +++ b/app/models/ci/pipeline_processed_report.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +module Ci + class PipelineProcessedReport < ApplicationRecord + include ObjectStorage::BackgroundMove + extend Gitlab::Ci::Model + + belongs_to :pipeline + + mount_uploader :processed_report, PipelineProcessedReportUploader + end +end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 412d0fa4ec84a1656957ba9c86bb739198acf61c..5e521938be38535d4ba2a97807a225ee6226b047 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -1300,9 +1300,7 @@ def compare_test_reports end def has_coverage_reports? - return false unless Feature.enabled?(:coverage_report_view, project) - - actual_head_pipeline&.has_reports?(Ci::JobArtifact.coverage_reports) + actual_head_pipeline&.processed_coverage_report.present? end # TODO: this method and compare_test_reports use the same @@ -1314,7 +1312,7 @@ def find_coverage_reports return { status: :error, status_reason: 'This merge request does not have coverage reports' } end - compare_reports(Ci::GenerateCoverageReportsService) + compare_reports(Ci::FindCoverageReportsService) end def has_exposed_artifacts? diff --git a/app/services/ci/generate_coverage_reports_service.rb b/app/services/ci/find_coverage_reports_service.rb similarity index 63% rename from app/services/ci/generate_coverage_reports_service.rb rename to app/services/ci/find_coverage_reports_service.rb index ebd1eaf0bade177b6874f241ce42d49001752c23..d7de3ee987898609682d89aaf48bcbe889c2615f 100644 --- a/app/services/ci/generate_coverage_reports_service.rb +++ b/app/services/ci/find_coverage_reports_service.rb @@ -6,14 +6,17 @@ module Ci # - it's not a report comparison and some comparing features must be turned off. # see CompareReportsBaseService for more notes. # issue: https://gitlab.com/gitlab-org/gitlab/issues/34224 - class GenerateCoverageReportsService < CompareReportsBaseService + class FindCoverageReportsService < CompareReportsBaseService def execute(base_pipeline, head_pipeline) merge_request = MergeRequest.find_by_id(params[:id]) - { - status: :parsed, - key: key(base_pipeline, head_pipeline), - data: head_pipeline.coverage_reports.pick(merge_request.new_paths) - } + head_pipeline.processed_coverage_report.processed_report.open do |file| + raw_coverage = JSON.parse(file.read) + { + status: :parsed, + key: key(base_pipeline, head_pipeline), + data: { files: pick(raw_coverage, merge_request.new_paths) } + } + end rescue => e Gitlab::ErrorTracking.track_exception(e, project_id: project.id) { @@ -26,5 +29,13 @@ def execute(base_pipeline, head_pipeline) def latest?(base_pipeline, head_pipeline, data) data&.fetch(:key, nil) == key(base_pipeline, head_pipeline) end + + private + + def pick(files, keys) + files.select do |key| + keys.include?(key) + end + end end end diff --git a/app/uploaders/pipeline_processed_report_uploader.rb b/app/uploaders/pipeline_processed_report_uploader.rb new file mode 100644 index 0000000000000000000000000000000000000000..74976359b026ffadc0bd8135d2eedfe0deb59d02 --- /dev/null +++ b/app/uploaders/pipeline_processed_report_uploader.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class PipelineProcessedReportUploader < GitlabUploader + include ObjectStorage::Concern + + storage_options Gitlab.config.artifacts + + alias_method :upload, :model + + def filename + "report-#{model.id}" + end + + def store_dir + File.join(model.model_name.plural, "pipeline-#{model.pipeline_id}") + end +end diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index dd0eeaa93590cffd220f09520e2b5a4e40768f7c..b6e1eedd7df558843d05f083723e221b0452e01e 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -661,6 +661,13 @@ :resource_boundary: :unknown :weight: 1 :idempotent: true +- :name: pipeline_background:ci_pipeline_coverage_report + :feature_category: :continuous_integration + :has_external_dependencies: + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true - :name: pipeline_cache:expire_job_cache :feature_category: :continuous_integration :has_external_dependencies: diff --git a/app/workers/ci/pipeline_coverage_report_worker.rb b/app/workers/ci/pipeline_coverage_report_worker.rb new file mode 100644 index 0000000000000000000000000000000000000000..b2261e11a7f5eda6722153553f4e6389d75c84d6 --- /dev/null +++ b/app/workers/ci/pipeline_coverage_report_worker.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module Ci + class PipelineCoverageReportWorker + include ApplicationWorker + include PipelineBackgroundQueue + + idempotent! + + def perform(pipeline_id) + Ci::Pipeline.find_by_id(pipeline_id).try do |pipeline| + report = pipeline.create_processed_coverage_report + report_file = CarrierWaveStringFile.new(pipeline.coverage_reports.json_string) + report.update(processed_report: report_file) + end + end + end +end diff --git a/db/migrate/20200321084319_create_ci_pipeline_processed_report_table.rb b/db/migrate/20200321084319_create_ci_pipeline_processed_report_table.rb new file mode 100644 index 0000000000000000000000000000000000000000..e63c026429fb0e6f14e73097fe47ee5babb869d9 --- /dev/null +++ b/db/migrate/20200321084319_create_ci_pipeline_processed_report_table.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class CreateCiPipelineProcessedReportTable < ActiveRecord::Migration[6.0] + DOWNTIME = false + + def change + create_table :ci_pipeline_processed_reports do |t| + t.references :pipeline, index: false, null: false, foreign_key: { to_table: :ci_pipelines, on_delete: :cascade } + t.string :processed_report # rubocop:disable Migration/AddLimitToStringColumns + + t.index :pipeline_id + end + end +end diff --git a/db/structure.sql b/db/structure.sql index 2af6dcba3fdd3bfa4b5a2f1586d5c47d498f10ac..66b353321d2a06f08c8d9696f2675401c6b32560 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -1086,6 +1086,21 @@ CREATE SEQUENCE public.ci_pipeline_chat_data_id_seq ALTER SEQUENCE public.ci_pipeline_chat_data_id_seq OWNED BY public.ci_pipeline_chat_data.id; +CREATE TABLE public.ci_pipeline_processed_reports ( + id bigint NOT NULL, + pipeline_id bigint NOT NULL, + processed_report character varying +); + +CREATE SEQUENCE public.ci_pipeline_processed_reports_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE public.ci_pipeline_processed_reports_id_seq OWNED BY public.ci_pipeline_processed_reports.id; + CREATE TABLE public.ci_pipeline_schedule_variables ( id integer NOT NULL, key character varying NOT NULL, @@ -6847,6 +6862,8 @@ ALTER TABLE ONLY public.ci_job_variables ALTER COLUMN id SET DEFAULT nextval('pu ALTER TABLE ONLY public.ci_pipeline_chat_data ALTER COLUMN id SET DEFAULT nextval('public.ci_pipeline_chat_data_id_seq'::regclass); +ALTER TABLE ONLY public.ci_pipeline_processed_reports ALTER COLUMN id SET DEFAULT nextval('public.ci_pipeline_processed_reports_id_seq'::regclass); + ALTER TABLE ONLY public.ci_pipeline_schedule_variables ALTER COLUMN id SET DEFAULT nextval('public.ci_pipeline_schedule_variables_id_seq'::regclass); ALTER TABLE ONLY public.ci_pipeline_schedules ALTER COLUMN id SET DEFAULT nextval('public.ci_pipeline_schedules_id_seq'::regclass); @@ -7482,6 +7499,9 @@ ALTER TABLE ONLY public.ci_job_variables ALTER TABLE ONLY public.ci_pipeline_chat_data ADD CONSTRAINT ci_pipeline_chat_data_pkey PRIMARY KEY (id); +ALTER TABLE ONLY public.ci_pipeline_processed_reports + ADD CONSTRAINT ci_pipeline_processed_reports_pkey PRIMARY KEY (id); + ALTER TABLE ONLY public.ci_pipeline_schedule_variables ADD CONSTRAINT ci_pipeline_schedule_variables_pkey PRIMARY KEY (id); @@ -8606,6 +8626,8 @@ CREATE INDEX index_ci_pipeline_chat_data_on_chat_name_id ON public.ci_pipeline_c CREATE UNIQUE INDEX index_ci_pipeline_chat_data_on_pipeline_id ON public.ci_pipeline_chat_data USING btree (pipeline_id); +CREATE INDEX index_ci_pipeline_processed_reports_on_pipeline_id ON public.ci_pipeline_processed_reports USING btree (pipeline_id); + CREATE UNIQUE INDEX index_ci_pipeline_schedule_variables_on_schedule_id_and_key ON public.ci_pipeline_schedule_variables USING btree (pipeline_schedule_id, key); CREATE INDEX index_ci_pipeline_schedules_on_next_run_at_and_active ON public.ci_pipeline_schedules USING btree (next_run_at, active); @@ -11676,6 +11698,9 @@ ALTER TABLE ONLY public.ci_runner_namespaces ALTER TABLE ONLY public.board_project_recent_visits ADD CONSTRAINT fk_rails_fb6fc419cb FOREIGN KEY (user_id) REFERENCES public.users(id) ON DELETE CASCADE; +ALTER TABLE ONLY public.ci_pipeline_processed_reports + ADD CONSTRAINT fk_rails_fba6cb330d FOREIGN KEY (pipeline_id) REFERENCES public.ci_pipelines(id) ON DELETE CASCADE; + ALTER TABLE ONLY public.serverless_domain_cluster ADD CONSTRAINT fk_rails_fbdba67eb1 FOREIGN KEY (creator_id) REFERENCES public.users(id) ON DELETE SET NULL; @@ -12708,5 +12733,6 @@ INSERT INTO "schema_migrations" (version) VALUES ('20200318163148'), ('20200318164448'), ('20200318165448'), -('20200319203901'); +('20200319203901'), +('20200321084319'); diff --git a/doc/user/project/merge_requests/test_coverage_visualization.md b/doc/user/project/merge_requests/test_coverage_visualization.md index a0a4c5d3743b80457718499705ef64ac2249fae7..20c706839249d304bd5b69f5c32d4a7128dc17d3 100644 --- a/doc/user/project/merge_requests/test_coverage_visualization.md +++ b/doc/user/project/merge_requests/test_coverage_visualization.md @@ -62,17 +62,3 @@ test: reports: cobertura: coverage/cobertura-coverage.xml ``` - -## Enabling the feature - -This feature comes with the `:coverage_report_view` feature flag disabled by -default. This feature is disabled due to some performance issues with very large -data sets. When [the performance issue](https://gitlab.com/gitlab-org/gitlab/issues/37725) -is resolved, the feature will be enabled by default. - -To enable this feature, ask a GitLab administrator with Rails console access to -run the following command: - -```ruby -Feature.enable(:coverage_report_view) -``` diff --git a/lib/gitlab/ci/reports/coverage_reports.rb b/lib/gitlab/ci/reports/coverage_reports.rb index 31afb636d2f39d6640db7fb8390773e15359fb9a..5e3e446430113ad2905f6a16b055377854c24504 100644 --- a/lib/gitlab/ci/reports/coverage_reports.rb +++ b/lib/gitlab/ci/reports/coverage_reports.rb @@ -10,12 +10,8 @@ def initialize @files = {} end - def pick(keys) - coverage_files = files.select do |key| - keys.include?(key) - end - - { files: coverage_files } + def json_string + JSON.generate(files) end def add_file(name, line_coverage)