Add latest changes from gitlab-org/gitlab@master

8 files changed, 238 insertions, 15 deletions

diff --git a/spec/controllers/admin/integrations_controller_spec.rb b/spec/controllers/admin/integrations_controller_spec.rb
index 0641f64b0e3..50748918893 100644
--- a/spec/controllers/admin/integrations_controller_spec.rb
+++ b/spec/controllers/admin/integrations_controller_spec.rb
@@ -3,8 +3,8 @@
 require 'spec_helper'
 
 describe Admin::IntegrationsController do
+  let_it_be(:project) { create(:project) }
   let(:admin) { create(:admin) }
-  let!(:project) { create(:project) }
 
   before do
     sign_in(admin)
@@ -13,7 +13,7 @@ describe Admin::IntegrationsController do
   describe '#edit' do
     context 'when instance_level_integrations not enabled' do
       it 'returns not_found' do
-        allow(Feature).to receive(:enabled?).with(:instance_level_integrations) { false }
+        stub_feature_flags(instance_level_integrations: false)
 
         get :edit, params: { id: Service.available_services_names.sample }
 
diff --git a/spec/controllers/groups/settings/integrations_controller_spec.rb b/spec/controllers/groups/settings/integrations_controller_spec.rb
new file mode 100644
index 00000000000..bbf215a4bb9
--- /dev/null
+++ b/spec/controllers/groups/settings/integrations_controller_spec.rb
@@ -0,0 +1,107 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Groups::Settings::IntegrationsController do
+  let_it_be(:project) { create(:project) }
+  let(:user) { create(:user) }
+  let(:group) { create(:group) }
+
+  before do
+    sign_in(user)
+  end
+
+  describe '#edit' do
+    context 'when group_level_integrations not enabled' do
+      it 'returns not_found' do
+        stub_feature_flags(group_level_integrations: { enabled: false, thing: group })
+
+        get :edit, params: { group_id: group, id: Service.available_services_names.sample }
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+
+    context 'when user is not owner' do
+      it 'renders not_found' do
+        get :edit, params: { group_id: group, id: Service.available_services_names.sample }
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+
+    context 'when user is owner' do
+      before do
+        group.add_owner(user)
+      end
+
+      Service.available_services_names.each do |integration_name|
+        context "#{integration_name}" do
+          it 'successfully displays the template' do
+            get :edit, params: { group_id: group, id: integration_name }
+
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(response).to render_template(:edit)
+          end
+        end
+      end
+    end
+  end
+
+  describe '#update' do
+    let(:integration) { create(:jira_service, project: project) }
+
+    before do
+      group.add_owner(user)
+
+      put :update, params: { group_id: group, id: integration.class.to_param, service: { url: url } }
+    end
+
+    context 'valid params' do
+      let(:url) { 'https://jira.gitlab-example.com' }
+
+      it 'updates the integration' do
+        expect(response).to have_gitlab_http_status(:found)
+        expect(integration.reload.url).to eq(url)
+      end
+    end
+
+    context 'invalid params' do
+      let(:url) { 'ftp://jira.localhost' }
+
+      it 'does not update the integration' do
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(response).to render_template(:edit)
+        expect(integration.reload.url).not_to eq(url)
+      end
+    end
+  end
+
+  describe '#test' do
+    context 'testable' do
+      let(:integration) { create(:jira_service, project: project) }
+
+      before do
+        group.add_owner(user)
+      end
+
+      it 'returns ok' do
+        allow_any_instance_of(integration.class).to receive(:test) { { success: true } }
+
+        put :test, params: { group_id: group, id: integration.class.to_param }
+
+        expect(response).to have_gitlab_http_status(:ok)
+      end
+    end
+
+    context 'not testable' do
+      let(:integration) { create(:alerts_service, project: project) }
+
+      it 'returns not found' do
+        put :test, params: { group_id: group, id: integration.class.to_param }
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+  end
+end
diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb
index a929eaeba3f..74931fcdeb2 100644
--- a/spec/controllers/projects/pipelines_controller_spec.rb
+++ b/spec/controllers/projects/pipelines_controller_spec.rb
@@ -705,13 +705,45 @@ describe Projects::PipelinesController do
   end
 
   describe 'GET test_report.json' do
-    subject(:get_test_report_json) do
-      get :test_report, params: {
-        namespace_id: project.namespace,
-        project_id: project,
-        id: pipeline.id
-      },
-      format: :json
+    let(:pipeline) { create(:ci_pipeline, project: project) }
+
+    context 'with attachments' do
+      let(:blob) do
+        <<~EOF
+          <testsuites>
+            <testsuite>
+              <testcase classname='Calculator' name='sumTest1' time='0.01'>
+                <failure>Some failure</failure>
+                <system-out>[[ATTACHMENT|some/path.png]]</system-out>
+              </testcase>
+            </testsuite>
+          </testsuites>
+        EOF
+      end
+
+      before do
+        allow_any_instance_of(Ci::JobArtifact).to receive(:each_blob).and_yield(blob)
+      end
+
+      it 'does not have N+1 problem with attachments' do
+        get_test_report_json
+
+        create(:ci_build, name: 'rspec', pipeline: pipeline).tap do |build|
+          create(:ci_job_artifact, :junit, job: build)
+        end
+
+        clear_controller_memoization
+
+        control_count = ActiveRecord::QueryRecorder.new { get_test_report_json }.count
+
+        create(:ci_build, name: 'karma', pipeline: pipeline).tap do |build|
+          create(:ci_job_artifact, :junit, job: build)
+        end
+
+        clear_controller_memoization
+
+        expect { get_test_report_json }.not_to exceed_query_limit(control_count)
+      end
     end
 
     context 'when feature is enabled' do
@@ -772,6 +804,20 @@ describe Projects::PipelinesController do
         expect(response.body).to be_empty
       end
     end
+
+    def get_test_report_json
+      get :test_report, params: {
+        namespace_id: project.namespace,
+        project_id: project,
+        id: pipeline.id
+      },
+      format: :json
+    end
+
+    def clear_controller_memoization
+      controller.clear_memoization(:pipeline_test_report)
+      controller.instance_variable_set(:@pipeline, nil)
+    end
   end
 
   describe 'GET test_report_count.json' do
diff --git a/spec/factories/ci/test_case.rb b/spec/factories/ci/test_case.rb
index 8017111bcc7..ce6bd0f3d7d 100644
--- a/spec/factories/ci/test_case.rb
+++ b/spec/factories/ci/test_case.rb
@@ -9,6 +9,7 @@ FactoryBot.define do
     status { "success" }
     system_output { nil }
     attachment { nil }
+    association :job, factory: :ci_build
 
     trait :with_attachment do
       attachment { "some/path.png" }
@@ -24,7 +25,8 @@ FactoryBot.define do
         execution_time: execution_time,
         status: status,
         system_output: system_output,
-        attachment: attachment
+        attachment: attachment,
+        job: job
       )
     end
   end
diff --git a/spec/fixtures/api/schemas/entities/test_case.json b/spec/fixtures/api/schemas/entities/test_case.json
index 70f6edeeeb7..0dd3c5d472f 100644
--- a/spec/fixtures/api/schemas/entities/test_case.json
+++ b/spec/fixtures/api/schemas/entities/test_case.json
@@ -10,7 +10,8 @@
     "classname": { "type": "string" },
     "execution_time": { "type": "float" },
     "system_output": { "type": ["string", "null"] },
-    "stack_trace":  { "type": ["string", "null"] }
+    "stack_trace":  { "type": ["string", "null"] },
+    "attachment_url":  { "type": ["string", "null"] }
   },
   "additionalProperties": false
 }
diff --git a/spec/lib/gitlab/ci/parsers/test/junit_spec.rb b/spec/lib/gitlab/ci/parsers/test/junit_spec.rb
index 9a486c312d4..da168f6daad 100644
--- a/spec/lib/gitlab/ci/parsers/test/junit_spec.rb
+++ b/spec/lib/gitlab/ci/parsers/test/junit_spec.rb
@@ -4,10 +4,11 @@ require 'fast_spec_helper'
 
 describe Gitlab::Ci::Parsers::Test::Junit do
   describe '#parse!' do
-    subject { described_class.new.parse!(junit, test_suite) }
+    subject { described_class.new.parse!(junit, test_suite, args) }
 
     let(:test_suite) { Gitlab::Ci::Reports::TestSuite.new('rspec') }
     let(:test_cases) { flattened_test_cases(test_suite) }
+    let(:args) { { job: { id: 1, project: "project" } } }
 
     context 'when data is JUnit style XML' do
       context 'when there are no <testcases> in <testsuite>' do
@@ -205,7 +206,7 @@ describe Gitlab::Ci::Parsers::Test::Junit do
       end
     end
 
-    context 'when data contains an attachment tag' do
+    context 'when attachment is specified in failed test case' do
       let(:junit) do
         <<~EOF
           <testsuites>
@@ -219,11 +220,15 @@ describe Gitlab::Ci::Parsers::Test::Junit do
         EOF
       end
 
-      it 'add attachment to a test case' do
+      it 'assigns correct attributes to the test case' do
         expect { subject }.not_to raise_error
 
         expect(test_cases[0].has_attachment?).to be_truthy
         expect(test_cases[0].attachment).to eq("some/path.png")
+
+        expect(test_cases[0].job).to be_present
+        expect(test_cases[0].job[:id]).to eq(1)
+        expect(test_cases[0].job[:project]).to eq("project")
       end
     end
 
diff --git a/spec/lib/gitlab/ci/reports/test_case_spec.rb b/spec/lib/gitlab/ci/reports/test_case_spec.rb
index c13161f3e7c..69fe05d573e 100644
--- a/spec/lib/gitlab/ci/reports/test_case_spec.rb
+++ b/spec/lib/gitlab/ci/reports/test_case_spec.rb
@@ -15,7 +15,8 @@ describe Gitlab::Ci::Reports::TestCase do
             file: 'spec/trace_spec.rb',
             execution_time: 1.23,
             status: described_class::STATUS_SUCCESS,
-            system_output: nil
+            system_output: nil,
+            job: build(:ci_build)
           }
         end
 
@@ -28,6 +29,7 @@ describe Gitlab::Ci::Reports::TestCase do
           expect(test_case.execution_time).to eq(1.23)
           expect(test_case.status).to eq(described_class::STATUS_SUCCESS)
           expect(test_case.system_output).to be_nil
+          expect(test_case.job).to be_present
         end
       end
 
@@ -99,6 +101,22 @@ describe Gitlab::Ci::Reports::TestCase do
       it '#has_attachment?' do
         expect(attachment_test_case.has_attachment?).to be_truthy
       end
+
+      it '#attachment_url' do
+        expect(attachment_test_case.attachment_url).to match(/file\/some\/path.png/)
+      end
+    end
+
+    context 'when attachment is missing' do
+      let(:test_case) { build(:test_case) }
+
+      it '#has_attachment?' do
+        expect(test_case.has_attachment?).to be_falsy
+      end
+
+      it '#attachment_url' do
+        expect(test_case.attachment_url).to be_nil
+      end
     end
   end
 end
diff --git a/spec/serializers/test_case_entity_spec.rb b/spec/serializers/test_case_entity_spec.rb
index 84203adea2c..f16c271be4d 100644
--- a/spec/serializers/test_case_entity_spec.rb
+++ b/spec/serializers/test_case_entity_spec.rb
@@ -31,5 +31,49 @@ describe TestCaseEntity do
         expect(subject[:execution_time]).to eq(2.22)
       end
     end
+
+    context 'when feature is enabled' do
+      before do
+        stub_feature_flags(junit_pipeline_screenshots_view: true)
+      end
+
+      context 'when attachment is present' do
+        let(:test_case) { build(:test_case, :with_attachment) }
+
+        it 'returns the attachment_url' do
+          expect(subject).to include(:attachment_url)
+        end
+      end
+
+      context 'when attachment is not present' do
+        let(:test_case) { build(:test_case) }
+
+        it 'returns a nil attachment_url' do
+          expect(subject[:attachment_url]).to be_nil
+        end
+      end
+    end
+
+    context 'when feature is disabled' do
+      before do
+        stub_feature_flags(junit_pipeline_screenshots_view: false)
+      end
+
+      context 'when attachment is present' do
+        let(:test_case) { build(:test_case, :with_attachment) }
+
+        it 'returns no attachment_url' do
+          expect(subject).not_to include(:attachment_url)
+        end
+      end
+
+      context 'when attachment is not present' do
+        let(:test_case) { build(:test_case) }
+
+        it 'returns no attachment_url' do
+          expect(subject).not_to include(:attachment_url)
+        end
+      end
+    end
   end
 end