summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/serializers/test_suite_comparer_entity.rb29
-rw-r--r--app/services/ci/compare_reports_base_service.rb6
-rw-r--r--changelogs/unreleased/limit-amount-of-tests-returned.yml5
-rw-r--r--spec/features/merge_request/user_sees_merge_widget_spec.rb62
-rw-r--r--spec/lib/gitlab/ci/reports/test_reports_comparer_spec.rb8
-rw-r--r--spec/lib/gitlab/ci/reports/test_suite_comparer_spec.rb23
-rw-r--r--spec/serializers/test_case_entity_spec.rb2
-rw-r--r--spec/serializers/test_reports_comparer_entity_spec.rb8
-rw-r--r--spec/serializers/test_reports_comparer_serializer_spec.rb8
-rw-r--r--spec/serializers/test_suite_comparer_entity_spec.rb92
-rw-r--r--spec/support/test_reports/test_reports_helper.rb34
11 files changed, 189 insertions, 88 deletions
diff --git a/app/serializers/test_suite_comparer_entity.rb b/app/serializers/test_suite_comparer_entity.rb
index 9fa3a897ebe..d402a4d5718 100644
--- a/app/serializers/test_suite_comparer_entity.rb
+++ b/app/serializers/test_suite_comparer_entity.rb
@@ -1,6 +1,9 @@
# frozen_string_literal: true
class TestSuiteComparerEntity < Grape::Entity
+ DEFAULT_MAX_TESTS = 100
+ DEFAULT_MIN_TESTS = 10
+
expose :name
expose :total_status, as: :status
@@ -10,7 +13,27 @@ class TestSuiteComparerEntity < Grape::Entity
expose :failed_count, as: :failed
end
- expose :new_failures, using: TestCaseEntity
- expose :resolved_failures, using: TestCaseEntity
- expose :existing_failures, using: TestCaseEntity
+ # rubocop: disable CodeReuse/ActiveRecord
+ expose :new_failures, using: TestCaseEntity do |suite|
+ suite.new_failures.take(max_tests)
+ end
+
+ expose :existing_failures, using: TestCaseEntity do |suite|
+ suite.existing_failures.take(
+ max_tests(suite.new_failures))
+ end
+
+ expose :resolved_failures, using: TestCaseEntity do |suite|
+ suite.resolved_failures.take(
+ max_tests(suite.new_failures, suite.existing_failures))
+ end
+
+ private
+
+ def max_tests(*used)
+ return Integer::MAX unless Feature.enabled?(:ci_limit_test_reports_size, default_enabled: true)
+
+ [DEFAULT_MAX_TESTS - used.map(&:count).sum, DEFAULT_MIN_TESTS].max
+ end
+ # rubocop: enable CodeReuse/ActiveRecord
end
diff --git a/app/services/ci/compare_reports_base_service.rb b/app/services/ci/compare_reports_base_service.rb
index d5625857599..6c2d80d8f45 100644
--- a/app/services/ci/compare_reports_base_service.rb
+++ b/app/services/ci/compare_reports_base_service.rb
@@ -8,7 +8,7 @@ module Ci
status: :parsed,
key: key(base_pipeline, head_pipeline),
data: serializer_class
- .new(project: project)
+ .new(**serializer_params)
.represent(comparer).as_json
}
rescue Gitlab::Ci::Parsers::ParserError => e
@@ -40,6 +40,10 @@ module Ci
raise NotImplementedError
end
+ def serializer_params
+ { project: project }
+ end
+
def get_report(pipeline)
raise NotImplementedError
end
diff --git a/changelogs/unreleased/limit-amount-of-tests-returned.yml b/changelogs/unreleased/limit-amount-of-tests-returned.yml
new file mode 100644
index 00000000000..0e80a64b6b7
--- /dev/null
+++ b/changelogs/unreleased/limit-amount-of-tests-returned.yml
@@ -0,0 +1,5 @@
+---
+title: Limit amount of JUnit tests returned
+merge_request: 30274
+author:
+type: performance
diff --git a/spec/features/merge_request/user_sees_merge_widget_spec.rb b/spec/features/merge_request/user_sees_merge_widget_spec.rb
index 733e8aa3eba..30e30751693 100644
--- a/spec/features/merge_request/user_sees_merge_widget_spec.rb
+++ b/spec/features/merge_request/user_sees_merge_widget_spec.rb
@@ -519,6 +519,8 @@ describe 'Merge request > User sees merge widget', :js do
end
before do
+ allow_any_instance_of(TestSuiteComparerEntity)
+ .to receive(:max_tests).and_return(2)
allow_any_instance_of(MergeRequest)
.to receive(:has_test_reports?).and_return(true)
allow_any_instance_of(MergeRequest)
@@ -551,7 +553,7 @@ describe 'Merge request > User sees merge widget', :js do
expect(page).to have_content('rspec found no changed test results out of 1 total test')
expect(page).to have_content('junit found 1 failed test result out of 1 total test')
expect(page).to have_content('New')
- expect(page).to have_content('subtractTest')
+ expect(page).to have_content('addTest')
end
end
end
@@ -562,7 +564,7 @@ describe 'Merge request > User sees merge widget', :js do
click_button 'Expand'
within(".js-report-section-container") do
- click_button 'subtractTest'
+ click_button 'addTest'
expect(page).to have_content('6.66')
expect(page).to have_content(sample_java_failed_message.gsub!(/\s+/, ' ').strip)
@@ -596,7 +598,7 @@ describe 'Merge request > User sees merge widget', :js do
expect(page).to have_content('rspec found 1 failed test result out of 1 total test')
expect(page).to have_content('junit found no changed test results out of 1 total test')
expect(page).not_to have_content('New')
- expect(page).to have_content('Test#sum when a is 2 and b is 2 returns summary')
+ expect(page).to have_content('Test#sum when a is 1 and b is 3 returns summary')
end
end
end
@@ -607,7 +609,7 @@ describe 'Merge request > User sees merge widget', :js do
click_button 'Expand'
within(".js-report-section-container") do
- click_button 'Test#sum when a is 2 and b is 2 returns summary'
+ click_button 'Test#sum when a is 1 and b is 3 returns summary'
expect(page).to have_content('2.22')
expect(page).to have_content(sample_rspec_failed_message.gsub!(/\s+/, ' ').strip)
@@ -628,13 +630,7 @@ describe 'Merge request > User sees merge widget', :js do
let(:head_reports) do
Gitlab::Ci::Reports::TestReports.new.tap do |reports|
reports.get_suite('rspec').add_test_case(create_test_case_rspec_success)
- reports.get_suite('junit').add_test_case(create_test_case_java_resolved)
- end
- end
-
- let(:create_test_case_java_resolved) do
- create_test_case_java_failed.tap do |test_case|
- test_case.instance_variable_set("@status", Gitlab::Ci::Reports::TestCase::STATUS_SUCCESS)
+ reports.get_suite('junit').add_test_case(create_test_case_java_success)
end
end
@@ -646,7 +642,7 @@ describe 'Merge request > User sees merge widget', :js do
within(".js-report-section-container") do
expect(page).to have_content('rspec found no changed test results out of 1 total test')
expect(page).to have_content('junit found 1 fixed test result out of 1 total test')
- expect(page).to have_content('subtractTest')
+ expect(page).to have_content('addTest')
end
end
end
@@ -657,15 +653,53 @@ describe 'Merge request > User sees merge widget', :js do
click_button 'Expand'
within(".js-report-section-container") do
- click_button 'subtractTest'
+ click_button 'addTest'
- expect(page).to have_content('6.66')
+ expect(page).to have_content('5.55')
end
end
end
end
end
+ context 'properly truncates the report' do
+ let(:base_reports) do
+ Gitlab::Ci::Reports::TestReports.new.tap do |reports|
+ 10.times do |index|
+ reports.get_suite('rspec').add_test_case(
+ create_test_case_rspec_failed(index))
+ reports.get_suite('junit').add_test_case(
+ create_test_case_java_success(index))
+ end
+ end
+ end
+
+ let(:head_reports) do
+ Gitlab::Ci::Reports::TestReports.new.tap do |reports|
+ 10.times do |index|
+ reports.get_suite('rspec').add_test_case(
+ create_test_case_rspec_failed(index))
+ reports.get_suite('junit').add_test_case(
+ create_test_case_java_failed(index))
+ end
+ end
+ end
+
+ it 'shows test reports summary which includes the resolved failure' do
+ within(".js-reports-container") do
+ click_button 'Expand'
+
+ expect(page).to have_content('Test summary contained 20 failed test results out of 20 total tests')
+ within(".js-report-section-container") do
+ expect(page).to have_content('rspec found 10 failed test results out of 10 total tests')
+ expect(page).to have_content('junit found 10 failed test results out of 10 total tests')
+
+ expect(page).to have_content('Test#sum when a is 1 and b is 3 returns summary', count: 2)
+ end
+ end
+ end
+ end
+
def comparer
Gitlab::Ci::Reports::TestReportsComparer.new(base_reports, head_reports)
end
diff --git a/spec/lib/gitlab/ci/reports/test_reports_comparer_spec.rb b/spec/lib/gitlab/ci/reports/test_reports_comparer_spec.rb
index 71c61e0345f..36582204cc1 100644
--- a/spec/lib/gitlab/ci/reports/test_reports_comparer_spec.rb
+++ b/spec/lib/gitlab/ci/reports/test_reports_comparer_spec.rb
@@ -74,17 +74,11 @@ describe Gitlab::Ci::Reports::TestReportsComparer do
subject { comparer.resolved_count }
context 'when there is a resolved test case in head suites' do
- let(:create_test_case_java_resolved) do
- create_test_case_java_failed.tap do |test_case|
- test_case.instance_variable_set("@status", Gitlab::Ci::Reports::TestCase::STATUS_SUCCESS)
- end
- end
-
before do
base_reports.get_suite('rspec').add_test_case(create_test_case_rspec_success)
base_reports.get_suite('junit').add_test_case(create_test_case_java_failed)
head_reports.get_suite('rspec').add_test_case(create_test_case_rspec_success)
- head_reports.get_suite('junit').add_test_case(create_test_case_java_resolved)
+ head_reports.get_suite('junit').add_test_case(create_test_case_java_success)
end
it 'returns the correct count' do
diff --git a/spec/lib/gitlab/ci/reports/test_suite_comparer_spec.rb b/spec/lib/gitlab/ci/reports/test_suite_comparer_spec.rb
index 6ab16e5518d..579b3e6fd24 100644
--- a/spec/lib/gitlab/ci/reports/test_suite_comparer_spec.rb
+++ b/spec/lib/gitlab/ci/reports/test_suite_comparer_spec.rb
@@ -10,12 +10,6 @@ describe Gitlab::Ci::Reports::TestSuiteComparer do
let(:test_case_success) { create_test_case_rspec_success }
let(:test_case_failed) { create_test_case_rspec_failed }
- let(:test_case_resolved) do
- create_test_case_rspec_failed.tap do |test_case|
- test_case.instance_variable_set("@status", Gitlab::Ci::Reports::TestCase::STATUS_SUCCESS)
- end
- end
-
describe '#new_failures' do
subject { comparer.new_failures }
@@ -44,7 +38,7 @@ describe Gitlab::Ci::Reports::TestSuiteComparer do
context 'when head sutie has a success test case which failed in base' do
before do
base_suite.add_test_case(test_case_failed)
- head_suite.add_test_case(test_case_resolved)
+ head_suite.add_test_case(test_case_success)
end
it 'does not return the failed test case' do
@@ -81,7 +75,7 @@ describe Gitlab::Ci::Reports::TestSuiteComparer do
context 'when head sutie has a success test case which failed in base' do
before do
base_suite.add_test_case(test_case_failed)
- head_suite.add_test_case(test_case_resolved)
+ head_suite.add_test_case(test_case_success)
end
it 'does not return the failed test case' do
@@ -126,11 +120,11 @@ describe Gitlab::Ci::Reports::TestSuiteComparer do
context 'when head sutie has a success test case which failed in base' do
before do
base_suite.add_test_case(test_case_failed)
- head_suite.add_test_case(test_case_resolved)
+ head_suite.add_test_case(test_case_success)
end
it 'does not return the resolved test case' do
- is_expected.to eq([test_case_resolved])
+ is_expected.to eq([test_case_success])
end
it 'returns the correct resolved count' do
@@ -156,13 +150,8 @@ describe Gitlab::Ci::Reports::TestSuiteComparer do
context 'when there are a new failure and an existing failure' do
let(:test_case_1_success) { create_test_case_rspec_success }
- let(:test_case_2_failed) { create_test_case_rspec_failed }
-
- let(:test_case_1_failed) do
- create_test_case_rspec_success.tap do |test_case|
- test_case.instance_variable_set("@status", Gitlab::Ci::Reports::TestCase::STATUS_FAILED)
- end
- end
+ let(:test_case_1_failed) { create_test_case_rspec_failed }
+ let(:test_case_2_failed) { create_test_case_rspec_failed('case2') }
before do
base_suite.add_test_case(test_case_1_success)
diff --git a/spec/serializers/test_case_entity_spec.rb b/spec/serializers/test_case_entity_spec.rb
index 986c9feb07b..cc5f086ca4e 100644
--- a/spec/serializers/test_case_entity_spec.rb
+++ b/spec/serializers/test_case_entity_spec.rb
@@ -24,7 +24,7 @@ describe TestCaseEntity do
it 'contains correct test case details' do
expect(subject[:status]).to eq('failed')
- expect(subject[:name]).to eq('Test#sum when a is 2 and b is 2 returns summary')
+ expect(subject[:name]).to eq('Test#sum when a is 1 and b is 3 returns summary')
expect(subject[:classname]).to eq('spec.test_spec')
expect(subject[:execution_time]).to eq(2.22)
end
diff --git a/spec/serializers/test_reports_comparer_entity_spec.rb b/spec/serializers/test_reports_comparer_entity_spec.rb
index 59c058fe368..4a951bbbde4 100644
--- a/spec/serializers/test_reports_comparer_entity_spec.rb
+++ b/spec/serializers/test_reports_comparer_entity_spec.rb
@@ -53,13 +53,7 @@ describe TestReportsComparerEntity do
base_reports.get_suite('rspec').add_test_case(create_test_case_rspec_success)
base_reports.get_suite('junit').add_test_case(create_test_case_java_failed)
head_reports.get_suite('rspec').add_test_case(create_test_case_rspec_success)
- head_reports.get_suite('junit').add_test_case(create_test_case_java_resolved)
- end
-
- let(:create_test_case_java_resolved) do
- create_test_case_java_failed.tap do |test_case|
- test_case.instance_variable_set("@status", Gitlab::Ci::Reports::TestCase::STATUS_SUCCESS)
- end
+ head_reports.get_suite('junit').add_test_case(create_test_case_java_success)
end
it 'contains correct compared test reports details' do
diff --git a/spec/serializers/test_reports_comparer_serializer_spec.rb b/spec/serializers/test_reports_comparer_serializer_spec.rb
index 9ea86c0dd83..62dc6f486c5 100644
--- a/spec/serializers/test_reports_comparer_serializer_spec.rb
+++ b/spec/serializers/test_reports_comparer_serializer_spec.rb
@@ -44,13 +44,7 @@ describe TestReportsComparerSerializer do
base_reports.get_suite('rspec').add_test_case(create_test_case_rspec_success)
base_reports.get_suite('junit').add_test_case(create_test_case_java_failed)
head_reports.get_suite('rspec').add_test_case(create_test_case_rspec_success)
- head_reports.get_suite('junit').add_test_case(create_test_case_java_resolved)
- end
-
- let(:create_test_case_java_resolved) do
- create_test_case_java_failed.tap do |test_case|
- test_case.instance_variable_set("@status", Gitlab::Ci::Reports::TestCase::STATUS_SUCCESS)
- end
+ head_reports.get_suite('junit').add_test_case(create_test_case_java_success)
end
it 'matches the schema' do
diff --git a/spec/serializers/test_suite_comparer_entity_spec.rb b/spec/serializers/test_suite_comparer_entity_spec.rb
index f61331f53a0..4b2cca2c68c 100644
--- a/spec/serializers/test_suite_comparer_entity_spec.rb
+++ b/spec/serializers/test_suite_comparer_entity_spec.rb
@@ -11,16 +11,10 @@ describe TestSuiteComparerEntity do
let(:test_case_success) { create_test_case_rspec_success }
let(:test_case_failed) { create_test_case_rspec_failed }
- let(:test_case_resolved) do
- create_test_case_rspec_failed.tap do |test_case|
- test_case.instance_variable_set("@status", Gitlab::Ci::Reports::TestCase::STATUS_SUCCESS)
- end
- end
-
describe '#as_json' do
subject { entity.as_json }
- context 'when head sutie has a newly failed test case which does not exist in base' do
+ context 'when head suite has a newly failed test case which does not exist in base' do
before do
base_suite.add_test_case(test_case_success)
head_suite.add_test_case(test_case_failed)
@@ -41,7 +35,7 @@ describe TestSuiteComparerEntity do
end
end
- context 'when head sutie still has a failed test case which failed in base' do
+ context 'when head suite still has a failed test case which failed in base' do
before do
base_suite.add_test_case(test_case_failed)
head_suite.add_test_case(test_case_failed)
@@ -62,10 +56,10 @@ describe TestSuiteComparerEntity do
end
end
- context 'when head sutie has a success test case which failed in base' do
+ context 'when head suite has a success test case which failed in base' do
before do
base_suite.add_test_case(test_case_failed)
- head_suite.add_test_case(test_case_resolved)
+ head_suite.add_test_case(test_case_success)
end
it 'contains correct compared test suite details' do
@@ -74,13 +68,83 @@ describe TestSuiteComparerEntity do
expect(subject[:summary]).to include(total: 1, resolved: 1, failed: 0)
expect(subject[:new_failures]).to be_empty
subject[:resolved_failures].first.tap do |resolved_failure|
- expect(resolved_failure[:status]).to eq(test_case_resolved.status)
- expect(resolved_failure[:name]).to eq(test_case_resolved.name)
- expect(resolved_failure[:execution_time]).to eq(test_case_resolved.execution_time)
- expect(resolved_failure[:system_output]).to eq(test_case_resolved.system_output)
+ expect(resolved_failure[:status]).to eq(test_case_success.status)
+ expect(resolved_failure[:name]).to eq(test_case_success.name)
+ expect(resolved_failure[:execution_time]).to eq(test_case_success.execution_time)
+ expect(resolved_failure[:system_output]).to eq(test_case_success.system_output)
end
expect(subject[:existing_failures]).to be_empty
end
end
+
+ context 'limits amount of tests returned' do
+ before do
+ stub_const('TestSuiteComparerEntity::DEFAULT_MAX_TESTS', 2)
+ stub_const('TestSuiteComparerEntity::DEFAULT_MIN_TESTS', 1)
+ end
+
+ context 'prefers new over existing and resolved' do
+ before do
+ 3.times { add_new_failure }
+ 3.times { add_existing_failure }
+ 3.times { add_resolved_failure }
+ end
+
+ it 'returns 2 new failures, and 1 of resolved and existing' do
+ expect(subject[:summary]).to include(total: 9, resolved: 3, failed: 6)
+ expect(subject[:new_failures].count).to eq(2)
+ expect(subject[:existing_failures].count).to eq(1)
+ expect(subject[:resolved_failures].count).to eq(1)
+ end
+ end
+
+ context 'prefers existing over resolved' do
+ before do
+ 3.times { add_existing_failure }
+ 3.times { add_resolved_failure }
+ end
+
+ it 'returns 2 existing failures, and 1 resolved' do
+ expect(subject[:summary]).to include(total: 6, resolved: 3, failed: 3)
+ expect(subject[:new_failures].count).to eq(0)
+ expect(subject[:existing_failures].count).to eq(2)
+ expect(subject[:resolved_failures].count).to eq(1)
+ end
+ end
+
+ context 'limits amount of resolved' do
+ before do
+ 3.times { add_resolved_failure }
+ end
+
+ it 'returns 2 resolved failures' do
+ expect(subject[:summary]).to include(total: 3, resolved: 3, failed: 0)
+ expect(subject[:new_failures].count).to eq(0)
+ expect(subject[:existing_failures].count).to eq(0)
+ expect(subject[:resolved_failures].count).to eq(2)
+ end
+ end
+
+ private
+
+ def add_new_failure
+ failed_case = create_test_case_rspec_failed(SecureRandom.hex)
+ head_suite.add_test_case(failed_case)
+ end
+
+ def add_existing_failure
+ failed_case = create_test_case_rspec_failed(SecureRandom.hex)
+ base_suite.add_test_case(failed_case)
+ head_suite.add_test_case(failed_case)
+ end
+
+ def add_resolved_failure
+ case_name = SecureRandom.hex
+ failed_case = create_test_case_rspec_failed(case_name)
+ success_case = create_test_case_rspec_success(case_name)
+ base_suite.add_test_case(failed_case)
+ head_suite.add_test_case(success_case)
+ end
+ end
end
end
diff --git a/spec/support/test_reports/test_reports_helper.rb b/spec/support/test_reports/test_reports_helper.rb
index 45c6e04dbf3..6840fb9a860 100644
--- a/spec/support/test_reports/test_reports_helper.rb
+++ b/spec/support/test_reports/test_reports_helper.rb
@@ -1,36 +1,36 @@
module TestReportsHelper
- def create_test_case_rspec_success
+ def create_test_case_rspec_success(name = 'test_spec')
Gitlab::Ci::Reports::TestCase.new(
name: 'Test#sum when a is 1 and b is 3 returns summary',
- classname: 'spec.test_spec',
+ classname: "spec.#{name}",
file: './spec/test_spec.rb',
execution_time: 1.11,
status: Gitlab::Ci::Reports::TestCase::STATUS_SUCCESS)
end
- def create_test_case_rspec_failed
+ def create_test_case_rspec_failed(name = 'test_spec')
Gitlab::Ci::Reports::TestCase.new(
- name: 'Test#sum when a is 2 and b is 2 returns summary',
- classname: 'spec.test_spec',
+ name: 'Test#sum when a is 1 and b is 3 returns summary',
+ classname: "spec.#{name}",
file: './spec/test_spec.rb',
execution_time: 2.22,
system_output: sample_rspec_failed_message,
status: Gitlab::Ci::Reports::TestCase::STATUS_FAILED)
end
- def create_test_case_rspec_skipped
+ def create_test_case_rspec_skipped(name = 'test_spec')
Gitlab::Ci::Reports::TestCase.new(
name: 'Test#sum when a is 3 and b is 3 returns summary',
- classname: 'spec.test_spec',
+ classname: "spec.#{name}",
file: './spec/test_spec.rb',
execution_time: 3.33,
status: Gitlab::Ci::Reports::TestCase::STATUS_SKIPPED)
end
- def create_test_case_rspec_error
+ def create_test_case_rspec_error(name = 'test_spec')
Gitlab::Ci::Reports::TestCase.new(
name: 'Test#sum when a is 4 and b is 4 returns summary',
- classname: 'spec.test_spec',
+ classname: "spec.#{name}",
file: './spec/test_spec.rb',
execution_time: 4.44,
status: Gitlab::Ci::Reports::TestCase::STATUS_ERROR)
@@ -48,34 +48,34 @@ module TestReportsHelper
EOF
end
- def create_test_case_java_success
+ def create_test_case_java_success(name = 'addTest')
Gitlab::Ci::Reports::TestCase.new(
- name: 'addTest',
+ name: name,
classname: 'CalculatorTest',
execution_time: 5.55,
status: Gitlab::Ci::Reports::TestCase::STATUS_SUCCESS)
end
- def create_test_case_java_failed
+ def create_test_case_java_failed(name = 'addTest')
Gitlab::Ci::Reports::TestCase.new(
- name: 'subtractTest',
+ name: name,
classname: 'CalculatorTest',
execution_time: 6.66,
system_output: sample_java_failed_message,
status: Gitlab::Ci::Reports::TestCase::STATUS_FAILED)
end
- def create_test_case_java_skipped
+ def create_test_case_java_skipped(name = 'addTest')
Gitlab::Ci::Reports::TestCase.new(
- name: 'multiplyTest',
+ name: name,
classname: 'CalculatorTest',
execution_time: 7.77,
status: Gitlab::Ci::Reports::TestCase::STATUS_SKIPPED)
end
- def create_test_case_java_error
+ def create_test_case_java_error(name = 'addTest')
Gitlab::Ci::Reports::TestCase.new(
- name: 'divideTest',
+ name: name,
classname: 'CalculatorTest',
execution_time: 8.88,
status: Gitlab::Ci::Reports::TestCase::STATUS_ERROR)