Merge branch 'mo-refactor-test-case-initialization' into 'master'

Refactor TestCase initialization

See merge request gitlab-org/gitlab!28571

lib/gitlab/ci/reports/test_case.rb

@@ -12,20 +12,19 @@ module Gitlab
 
         attr_reader :name, :classname, :execution_time, :status, :file, :system_output, :stack_trace, :key, :attachment, :job
 
-        # rubocop: disable Metrics/ParameterLists
-        def initialize(name:, classname:, execution_time:, status:, file: nil, system_output: nil, stack_trace: nil, attachment: nil, job: nil)
-          @name = name
-          @classname = classname
-          @file = file
-          @execution_time = execution_time.to_f
-          @status = status
-          @system_output = system_output
-          @stack_trace = stack_trace
+        def initialize(params)
+          @name = params.fetch(:name)
+          @classname = params.fetch(:classname)
+          @file = params.fetch(:file, nil)
+          @execution_time = params.fetch(:execution_time).to_f
+          @status = params.fetch(:status)
+          @system_output = params.fetch(:system_output, nil)
+          @stack_trace = params.fetch(:stack_trace, nil)
+          @attachment = params.fetch(:attachment, nil)
+          @job = params.fetch(:job, nil)
+
           @key = sanitize_key_name("#{classname}_#{name}")
-          @attachment = attachment
-          @job = job
         end
-        # rubocop: enable Metrics/ParameterLists
 
         def has_attachment?
           attachment.present?

spec/factories/ci/test_case.rb

@@ -6,17 +6,18 @@ FactoryBot.define do
     classname { "trace" }
     file { "spec/trace_spec.rb" }
     execution_time { 1.23 }
-    status { "success" }
+    status { Gitlab::Ci::Reports::TestCase::STATUS_SUCCESS }
     system_output { nil }
     attachment { nil }
     association :job, factory: :ci_build
 
     trait :failed do
-      status { "failed" }
+      status { Gitlab::Ci::Reports::TestCase::STATUS_FAILED }
+      system_output { "Failure/Error: is_expected.to eq(300) expected: 300 got: -100" }
     end
 
     trait :with_attachment do
-      status { "failed" }
+      status { Gitlab::Ci::Reports::TestCase::STATUS_FAILED }
       attachment { "some/path.png" }
     end
 

spec/lib/gitlab/ci/reports/test_case_spec.rb

@@ -4,21 +4,12 @@ require 'spec_helper'
 
 describe Gitlab::Ci::Reports::TestCase do
   describe '#initialize' do
-    let(:test_case) { described_class.new(**params)}
+    let(:test_case) { described_class.new(params)}
 
     context 'when both classname and name are given' do
       context 'when test case is passed' do
-        let(:params) do
-          {
-            name: 'test-1',
-            classname: 'trace',
-            file: 'spec/trace_spec.rb',
-            execution_time: 1.23,
-            status: described_class::STATUS_SUCCESS,
-            system_output: nil,
-            job: build(:ci_build)
-          }
-        end
+        let(:job) { build(:ci_build) }
+        let(:params) { attributes_for(:test_case).merge!(job: job) }
 
         it 'initializes an instance' do
           expect { test_case }.not_to raise_error
@@ -34,16 +25,8 @@ describe Gitlab::Ci::Reports::TestCase do
       end
 
       context 'when test case is failed' do
-        let(:params) do
-          {
-            name: 'test-1',
-            classname: 'trace',
-            file: 'spec/trace_spec.rb',
-            execution_time: 1.23,
-            status: described_class::STATUS_FAILED,
-            system_output: "Failure/Error: is_expected.to eq(300) expected: 300 got: -100"
-          }
-        end
+        let(:job) { build(:ci_build) }
+        let(:params) { attributes_for(:test_case, :failed).merge!(job: job) }
 
         it 'initializes an instance' do
           expect { test_case }.not_to raise_error
@@ -59,36 +42,23 @@ describe Gitlab::Ci::Reports::TestCase do
       end
     end
 
-    context 'when classname is missing' do
-      let(:params) do
-        {
-          name: 'test-1',
-          file: 'spec/trace_spec.rb',
-          execution_time: 1.23,
-          status: described_class::STATUS_SUCCESS,
-          system_output: nil
-        }
-      end
+    shared_examples 'param is missing' do |param|
+      let(:job) { build(:ci_build) }
+      let(:params) { attributes_for(:test_case).merge!(job: job) }
 
       it 'raises an error' do
-        expect { test_case }.to raise_error(ArgumentError)
+        params.delete(param)
+
+        expect { test_case }.to raise_error(KeyError)
       end
     end
 
-    context 'when name is missing' do
-      let(:params) do
-        {
-          classname: 'trace',
-          file: 'spec/trace_spec.rb',
-          execution_time: 1.23,
-          status: described_class::STATUS_SUCCESS,
-          system_output: nil
-        }
-      end
+    context 'when classname is missing' do
+      it_behaves_like 'param is missing', :classname
+    end
 
-      it 'raises an error' do
-        expect { test_case }.to raise_error(ArgumentError)
-      end
+    context 'when name is missing' do
+      it_behaves_like 'param is missing', :name
     end
 
     context 'when attachment is present' do