Merge branch '216478-remove-test-report-feature-flag' into 'master'

Remove junit report feature flag

See merge request gitlab-org/gitlab!39260

app/assets/javascripts/pipelines/pipeline_details_bundle.js

@@ -93,10 +93,6 @@ const createPipelineHeaderApp = mediator => {
 };
 
 const createTestDetails = () => {
-  if (!window.gon?.features?.junitPipelineView) {
-    return;
-  }
-
   const el = document.querySelector('#js-pipeline-tests-detail');
   const { summaryEndpoint, suiteEndpoint } = el?.dataset || {};
 

app/assets/javascripts/reports/components/grouped_test_reports_app.vue

@@ -56,7 +56,7 @@ export default {
       return `${this.pipelinePath}/test_report`;
     },
     showViewFullReport() {
-      return Boolean(this.glFeatures.junitPipelineView) && this.pipelinePath.length;
+      return this.pipelinePath.length;
     },
   },
   created() {

app/controllers/projects/merge_requests_controller.rb

@@ -43,7 +43,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
 
   before_action do
     push_frontend_feature_flag(:vue_issuable_sidebar, @project.group)
-    push_frontend_feature_flag(:junit_pipeline_view, @project.group)
   end
 
   around_action :allow_gitaly_ref_name_caching, only: [:index, :show, :discussions]

app/controllers/projects/pipelines_controller.rb

@@ -12,7 +12,6 @@ class Projects::PipelinesController < Projects::ApplicationController
   before_action :authorize_create_pipeline!, only: [:new, :create]
   before_action :authorize_update_pipeline!, only: [:retry, :cancel]
   before_action do
-    push_frontend_feature_flag(:junit_pipeline_view, project)
     push_frontend_feature_flag(:filter_pipelines_search, project, default_enabled: true)
     push_frontend_feature_flag(:dag_pipeline_tab, project, default_enabled: true)
     push_frontend_feature_flag(:pipelines_security_report_summary, project)
@@ -177,8 +176,6 @@ class Projects::PipelinesController < Projects::ApplicationController
   end
 
   def test_report
-    return unless Feature.enabled?(:junit_pipeline_view, project)
-
     respond_to do |format|
       format.html do
         render 'show'

app/views/projects/pipelines/_with_tabs.html.haml

 - return if pipeline_has_errors
-- test_reports_enabled = Feature.enabled?(:junit_pipeline_view, @project)
 - dag_pipeline_tab_enabled = Feature.enabled?(:dag_pipeline_tab, @project, default_enabled: true)
 
 .tabs-holder
@@ -20,11 +19,10 @@
         = link_to failures_project_pipeline_path(@project, @pipeline), data: { target: '#js-tab-failures', action: 'failures', toggle: 'tab' }, class: 'failures-tab' do
           = _('Failed Jobs')
           %span.badge.badge-pill.js-failures-counter= @pipeline.failed_builds.count
-    - if test_reports_enabled
-      %li.js-tests-tab-link
-        = link_to test_report_project_pipeline_path(@project, @pipeline), data: { target: '#js-tab-tests', action: 'test_report', toggle: 'tab' }, class: 'test-tab' do
-          = s_('TestReports|Tests')
-          %span.badge.badge-pill.js-test-report-badge-counter= @pipeline.test_report_summary.total[:count]
+    %li.js-tests-tab-link
+      = link_to test_report_project_pipeline_path(@project, @pipeline), data: { target: '#js-tab-tests', action: 'test_report', toggle: 'tab' }, class: 'test-tab' do
+        = s_('TestReports|Tests')
+        %span.badge.badge-pill.js-test-report-badge-counter= @pipeline.test_report_summary.total[:count]
     = render_if_exists "projects/pipelines/tabs_holder", pipeline: @pipeline, project: @project
 
 .tab-content

changelogs/unreleased/216478-remove-test-report-feature-flag.yml

+---
+title: JUnit test report on pipeline detail page
+merge_request: 39260
+author:
+type: added

doc/administration/feature_flags.md

@@ -103,10 +103,10 @@ Some feature flags can be enabled or disabled on a per project basis:
 Feature.enable(:<feature flag>, Project.find(<project id>))
 ```
 
-For example, to enable the [`:junit_pipeline_view`](../ci/junit_test_reports.md#enabling-the-junit-test-reports-feature-core-only) feature flag for project `1234`:
+For example, to enable the [`:product_analytics`](../operations/product_analytics.md#enable-or-disable-product-analytics) feature flag for project `1234`:
 
 ```ruby
-Feature.enable(:junit_pipeline_view, Project.find(1234))
+Feature.enable(:product_analytics, Project.find(1234))
 ```
 
 `Feature.enable` and `Feature.disable` always return `nil`, this is not an indication that the command failed:

doc/ci/junit_test_reports.md

@@ -239,9 +239,8 @@ Test:
 
 ## Viewing JUnit test reports on GitLab
 
-> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/24792) in GitLab 12.5.
-> - It's deployed behind a feature flag, disabled by default.
-> - To use it in GitLab self-managed instances, ask a GitLab administrator to [enable it](#enabling-the-junit-test-reports-feature-core-only). **(CORE ONLY)**
+> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/24792) in GitLab 12.5 behind a feature flag (`junit_pipeline_view`), disabled by default.
+> - The feature flag was removed and the feature was [made generally available](https://gitlab.com/gitlab-org/gitlab/-/issues/216478) in GitLab 13.3.
 
 If JUnit XML files are generated and uploaded as part of a pipeline, these reports
 can be viewed inside the pipelines details page. The **Tests** tab on this page will
@@ -254,22 +253,6 @@ details, including the cases that make up the suite.
 
 You can also retrieve the reports via the [GitLab API](../api/pipelines.md#get-a-pipelines-test-report).
 
-### Enabling the JUnit test reports feature **(CORE ONLY)**
-
-This feature comes with the `:junit_pipeline_view` feature flag disabled by default. This
-feature is disabled due to some performance issues with very large data sets.
-When [the performance is improved](https://gitlab.com/groups/gitlab-org/-/epics/2854), the feature will be enabled by default.
-
-To enable this feature, ask a GitLab administrator with [Rails console access](../administration/feature_flags.md#how-to-enable-and-disable-features-behind-flags) to run the
-following command:
-
-```ruby
-Feature.enable(:junit_pipeline_view)
-
-# Enable the feature for a specific project, GitLab 13.0 and above only.
-Feature.enable(:junit_pipeline_view, Project.find(<your-project-id-here>))
-```
-
 ## Viewing JUnit screenshots on GitLab
 
 > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/202114) in GitLab 13.0.

lib/api/ci/pipelines.rb

@@ -110,15 +110,13 @@ module API
         end
 
         desc 'Gets the test report for a given pipeline' do
-          detail 'This feature was introduced in GitLab 13.0. Disabled by default behind feature flag `junit_pipeline_view`'
+          detail 'This feature was introduced in GitLab 13.0.'
           success TestReportEntity
         end
         params do
           requires :pipeline_id, type: Integer, desc: 'The pipeline ID'
         end
         get ':id/pipelines/:pipeline_id/test_report' do
-          not_found! unless Feature.enabled?(:junit_pipeline_view, user_project)
-
           authorize! :read_build, pipeline
 
           present pipeline.test_reports, with: TestReportEntity, details: true

spec/controllers/projects/pipelines_controller_spec.rb

@@ -859,113 +859,88 @@ RSpec.describe Projects::PipelinesController do
       end
     end
 
-    context 'when feature is enabled' do
-      before do
-        stub_feature_flags(junit_pipeline_view: project)
-      end
-
-      context 'when pipeline does not have a test report' do
-        it 'renders an empty test report' do
-          get_test_report_json
+    context 'when pipeline does not have a test report' do
+      it 'renders an empty test report' do
+        get_test_report_json
 
-          expect(response).to have_gitlab_http_status(:ok)
-          expect(json_response['total_count']).to eq(0)
-        end
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response['total_count']).to eq(0)
       end
+    end
 
-      context 'when pipeline has a test report' do
-        before do
-          create(:ci_build, name: 'rspec', pipeline: pipeline).tap do |build|
-            create(:ci_job_artifact, :junit, job: build)
-          end
-        end
-
-        it 'renders the test report' do
-          get_test_report_json
-
-          expect(response).to have_gitlab_http_status(:ok)
-          expect(json_response['total_count']).to eq(4)
-        end
+    context 'when pipeline has a test report' do
+      before do
+        create(:ci_build, :test_reports, name: 'rspec', pipeline: pipeline)
       end
 
-      context 'when pipeline has a corrupt test report artifact' do
-        before do
-          create(:ci_build, name: 'rspec', pipeline: pipeline).tap do |build|
-            create(:ci_job_artifact, :junit_with_corrupted_data, job: build)
-          end
+      it 'renders the test report' do
+        get_test_report_json
 
-          get_test_report_json
-        end
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response['total_count']).to eq(4)
+      end
+    end
 
-        it 'renders the test reports' do
-          expect(response).to have_gitlab_http_status(:ok)
-          expect(json_response['test_suites'].count).to eq(1)
-        end
+    context 'when pipeline has a corrupt test report artifact' do
+      before do
+        create(:ci_build, :broken_test_reports, name: 'rspec', pipeline: pipeline)
 
-        it 'returns a suite_error on the suite with corrupted XML' do
-          expect(json_response['test_suites'].first['suite_error']).to eq('JUnit XML parsing failed: 1:1: FATAL: Document is empty')
-        end
+        get_test_report_json
       end
 
-      context 'when junit_pipeline_screenshots_view is enabled' do
-        before do
-          stub_feature_flags(junit_pipeline_screenshots_view: project)
-        end
-
-        context 'when test_report contains attachment and scope is with_attachment as a URL param' do
-          let(:pipeline) { create(:ci_pipeline, :with_test_reports_attachment, project: project) }
+      it 'renders the test reports' do
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response['test_suites'].count).to eq(1)
+      end
 
-          it 'returns a test reports with attachment' do
-            get_test_report_json(scope: 'with_attachment')
+      it 'returns a suite_error on the suite with corrupted XML' do
+        expect(json_response['test_suites'].first['suite_error']).to eq('JUnit XML parsing failed: 1:1: FATAL: Document is empty')
+      end
+    end
 
-            expect(response).to have_gitlab_http_status(:ok)
-            expect(json_response["test_suites"]).to be_present
-            expect(json_response["test_suites"].first["test_cases"].first).to include("attachment_url")
-          end
-        end
+    context 'when junit_pipeline_screenshots_view is enabled' do
+      before do
+        stub_feature_flags(junit_pipeline_screenshots_view: project)
+      end
 
-        context 'when test_report does not contain attachment and scope is with_attachment as a URL param' do
-          let(:pipeline) { create(:ci_pipeline, :with_test_reports, project: project) }
+      context 'when test_report contains attachment and scope is with_attachment as a URL param' do
+        let(:pipeline) { create(:ci_pipeline, :with_test_reports_attachment, project: project) }
 
-          it 'returns a test reports with empty values' do
-            get_test_report_json(scope: 'with_attachment')
+        it 'returns a test reports with attachment' do
+          get_test_report_json(scope: 'with_attachment')
 
-            expect(response).to have_gitlab_http_status(:ok)
-            expect(json_response["test_suites"]).to be_empty
-          end
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response["test_suites"]).to be_present
+          expect(json_response["test_suites"].first["test_cases"].first).to include("attachment_url")
         end
       end
 
-      context 'when junit_pipeline_screenshots_view is disabled' do
-        before do
-          stub_feature_flags(junit_pipeline_screenshots_view: false)
-        end
-
-        context 'when test_report contains attachment and scope is with_attachment as a URL param' do
-          let(:pipeline) { create(:ci_pipeline, :with_test_reports_attachment, project: project) }
+      context 'when test_report does not contain attachment and scope is with_attachment as a URL param' do
+        let(:pipeline) { create(:ci_pipeline, :with_test_reports, project: project) }
 
-          it 'returns a test reports without attachment_url' do
-            get_test_report_json(scope: 'with_attachment')
+        it 'returns a test reports with empty values' do
+          get_test_report_json(scope: 'with_attachment')
 
-            expect(response).to have_gitlab_http_status(:ok)
-            expect(json_response["test_suites"].first["test_cases"].first).not_to include("attachment_url")
-          end
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response["test_suites"]).to be_empty
         end
       end
     end
 
-    context 'when feature is disabled' do
-      let(:pipeline) { create(:ci_empty_pipeline, project: project) }
-
+    context 'when junit_pipeline_screenshots_view is disabled' do
       before do
-        stub_feature_flags(junit_pipeline_view: false)
+        stub_feature_flags(junit_pipeline_screenshots_view: false)
       end
 
-      it 'renders empty response' do
-        get_test_report_json
+      context 'when test_report contains attachment and scope is with_attachment as a URL param' do
+        let(:pipeline) { create(:ci_pipeline, :with_test_reports_attachment, project: project) }
 
-        expect(response).to have_gitlab_http_status(:no_content)
-        expect(response.body).to be_empty
+        it 'returns a test reports without attachment_url' do
+          get_test_report_json(scope: 'with_attachment')
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response["test_suites"].first["test_cases"].first).not_to include("attachment_url")
+        end
       end
     end
 

spec/frontend/fixtures/test_report.rb

@@ -15,7 +15,6 @@ RSpec.describe Projects::PipelinesController, "(JavaScript fixtures)", type: :co
 
   before do
     sign_in(user)
-    stub_feature_flags(junit_pipeline_view: project)
   end
 
   it "pipelines/test_report.json" do

spec/frontend/reports/components/grouped_test_reports_app_spec.js

@@ -20,10 +20,7 @@ describe('Grouped test reports app', () => {
   let wrapper;
   let mockStore;
 
-  const mountComponent = ({
-    glFeatures = { junitPipelineView: false },
-    props = { pipelinePath },
-  } = {}) => {
+  const mountComponent = ({ props = { pipelinePath } } = {}) => {
     wrapper = mount(Component, {
       store: mockStore,
       localVue,
@@ -35,9 +32,6 @@ describe('Grouped test reports app', () => {
       methods: {
         fetchReports: () => {},
       },
-      provide: {
-        glFeatures,
-      },
     });
   };
 
@@ -78,28 +72,17 @@ describe('Grouped test reports app', () => {
   });
 
   describe('`View full report` button', () => {
-    it('should not render the full test report link', () => {
-      expect(findFullTestReportLink().exists()).toBe(false);
-    });
+    it('should render the full test report link', () => {
+      const fullTestReportLink = findFullTestReportLink();
 
-    describe('With junitPipelineView feature flag enabled', () => {
-      beforeEach(() => {
-        mountComponent({ glFeatures: { junitPipelineView: true } });
-      });
-
-      it('should render the full test report link', () => {
-        const fullTestReportLink = findFullTestReportLink();
-
-        expect(fullTestReportLink.exists()).toBe(true);
-        expect(pipelinePath).not.toBe('');
-        expect(fullTestReportLink.attributes('href')).toBe(`${pipelinePath}/test_report`);
-      });
+      expect(fullTestReportLink.exists()).toBe(true);
+      expect(pipelinePath).not.toBe('');
+      expect(fullTestReportLink.attributes('href')).toBe(`${pipelinePath}/test_report`);
     });
 
     describe('Without a pipelinePath', () => {
       beforeEach(() => {
         mountComponent({
-          glFeatures: { junitPipelineView: true },
           props: { pipelinePath: '' },
         });
       });

spec/requests/api/ci/pipelines_spec.rb

@@ -735,55 +735,36 @@ RSpec.describe API::Ci::Pipelines do
 
       let(:pipeline) { create(:ci_pipeline, project: project) }
 
-      context 'when feature is enabled' do
-        before do
-          stub_feature_flags(junit_pipeline_view: true)
-        end
-
-        context 'when pipeline does not have a test report' do
-          it 'returns an empty test report' do
-            subject
-
-            expect(response).to have_gitlab_http_status(:ok)
-            expect(json_response['total_count']).to eq(0)
-          end
-        end
-
-        context 'when pipeline has a test report' do
-          let(:pipeline) { create(:ci_pipeline, :with_test_reports, project: project) }
-
-          it 'returns the test report' do
-            subject
+      context 'when pipeline does not have a test report' do
+        it 'returns an empty test report' do
+          subject
 
-            expect(response).to have_gitlab_http_status(:ok)
-            expect(json_response['total_count']).to eq(4)
-          end
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response['total_count']).to eq(0)
         end
+      end
 
-        context 'when pipeline has corrupt test reports' do
-          before do
-            job = create(:ci_build, pipeline: pipeline)
-            create(:ci_job_artifact, :junit_with_corrupted_data, job: job, project: project)
-          end
+      context 'when pipeline has a test report' do
+        let(:pipeline) { create(:ci_pipeline, :with_test_reports, project: project) }
 
-          it 'returns a suite_error' do
-            subject
+        it 'returns the test report' do
+          subject
 
-            expect(response).to have_gitlab_http_status(:ok)
-            expect(json_response['test_suites'].first['suite_error']).to eq('JUnit XML parsing failed: 1:1: FATAL: Document is empty')
-          end
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response['total_count']).to eq(4)
         end
       end
 
-      context 'when feature is disabled' do
+      context 'when pipeline has corrupt test reports' do
         before do
-          stub_feature_flags(junit_pipeline_view: false)
+          create(:ci_build, :broken_test_reports, name: 'rspec', pipeline: pipeline)
         end
 
-        it 'renders empty response' do
+        it 'returns a suite_error' do
           subject
 
-          expect(response).to have_gitlab_http_status(:not_found)
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response['test_suites'].first['suite_error']).to eq('JUnit XML parsing failed: 1:1: FATAL: Document is empty')
         end
       end
     end