diff options
author | Kamil Trzciński <ayufan@ayufan.eu> | 2018-08-07 17:44:12 +0300 |
---|---|---|
committer | Kamil Trzciński <ayufan@ayufan.eu> | 2018-08-07 17:44:12 +0300 |
commit | 0e90f27ff79d1743d8ec5e49e003d4c68a689f78 (patch) | |
tree | efcf4c80c547f95e4b8170dc2a305ef1a5f7319d /spec | |
parent | 0d729d5c54b8c1f87f915eb365e8abc189828b68 (diff) | |
parent | 8fa361b2d9cce92595e5804b9c7f4a04b5df25fb (diff) |
Merge branch 'improve-junit-support-be' into 'master'
Improve JUnit test reports in merge request widgets
Closes #49966
See merge request gitlab-org/gitlab-ce!21039
Diffstat (limited to 'spec')
-rw-r--r-- | spec/controllers/projects/merge_requests_controller_spec.rb | 24 | ||||
-rw-r--r-- | spec/features/merge_request/user_sees_merge_widget_spec.rb | 226 | ||||
-rw-r--r-- | spec/models/ci/pipeline_spec.rb | 43 | ||||
-rw-r--r-- | spec/models/concerns/reactive_caching_spec.rb | 8 | ||||
-rw-r--r-- | spec/models/merge_request_spec.rb | 13 | ||||
-rw-r--r-- | spec/services/ci/compare_test_reports_service_spec.rb | 32 |
6 files changed, 334 insertions, 12 deletions
diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 375018e2229..d9bb3981539 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -597,6 +597,12 @@ describe Projects::MergeRequestsController do context 'when comparison is being processed' do let(:comparison_status) { { status: :parsing } } + it 'sends polling interval' do + expect(Gitlab::PollingInterval).to receive(:set_header) + + subject + end + it 'returns 204 HTTP status' do subject @@ -607,6 +613,12 @@ describe Projects::MergeRequestsController do context 'when comparison is done' do let(:comparison_status) { { status: :parsed, data: { summary: 1 } } } + it 'does not send polling interval' do + expect(Gitlab::PollingInterval).not_to receive(:set_header) + + subject + end + it 'returns 200 HTTP status' do subject @@ -618,6 +630,12 @@ describe Projects::MergeRequestsController do context 'when user created corrupted test reports' do let(:comparison_status) { { status: :error, status_reason: 'Failed to parse test reports' } } + it 'does not send polling interval' do + expect(Gitlab::PollingInterval).not_to receive(:set_header) + + subject + end + it 'returns 400 HTTP status' do subject @@ -629,6 +647,12 @@ describe Projects::MergeRequestsController do context 'when something went wrong on our system' do let(:comparison_status) { {} } + it 'does not send polling interval' do + expect(Gitlab::PollingInterval).not_to receive(:set_header) + + subject + end + it 'returns 500 HTTP status' do subject 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 b6b3844f2ae..b285cd7a7ac 100644 --- a/spec/features/merge_request/user_sees_merge_widget_spec.rb +++ b/spec/features/merge_request/user_sees_merge_widget_spec.rb @@ -2,6 +2,7 @@ require 'rails_helper' describe 'Merge request > User sees merge widget', :js do include ProjectForksHelper + include TestReportsHelper let(:project) { create(:project, :repository) } let(:project_only_mwps) { create(:project, :repository, only_allow_merge_if_pipeline_succeeds: true) } @@ -325,4 +326,229 @@ describe 'Merge request > User sees merge widget', :js do expect(page).to have_content('This merge request is in the process of being merged') end end + + context 'when merge request has test reports' do + let!(:head_pipeline) do + create(:ci_pipeline, + :success, + project: project, + ref: merge_request.source_branch, + sha: merge_request.diff_head_sha) + end + + let!(:build) { create(:ci_build, :success, pipeline: head_pipeline, project: project) } + + before do + merge_request.update!(head_pipeline_id: head_pipeline.id) + end + + context 'when result has not been parsed yet' do + let!(:job_artifact) { create(:ci_job_artifact, :junit, job: build, project: project) } + + before do + visit project_merge_request_path(project, merge_request) + end + + it 'shows parsing status' do + expect(page).to have_content('Test summary results are being parsed') + end + end + + context 'when result has already been parsed' do + context 'when JUnit xml is correctly formatted' do + let!(:job_artifact) { create(:ci_job_artifact, :junit, job: build, project: project) } + + before do + allow_any_instance_of(MergeRequest).to receive(:compare_test_reports).and_return(compared_data) + + visit project_merge_request_path(project, merge_request) + end + + it 'shows parsed results' do + expect(page).to have_content('Test summary contained') + end + end + + context 'when JUnit xml is corrupted' do + let!(:job_artifact) { create(:ci_job_artifact, :junit_with_corrupted_data, job: build, project: project) } + + before do + allow_any_instance_of(MergeRequest).to receive(:compare_test_reports).and_return(compared_data) + + visit project_merge_request_path(project, merge_request) + end + + it 'shows the error state' do + expect(page).to have_content('Test summary failed loading results') + end + end + + def compared_data + Ci::CompareTestReportsService.new(project).execute(nil, head_pipeline) + end + end + + context 'when test reports have been parsed correctly' do + let(:serialized_data) do + { + status: :parsed, + data: TestReportsComparerSerializer + .new(project: project) + .represent(comparer) + } + end + + before do + allow_any_instance_of(MergeRequest) + .to receive(:has_test_reports?).and_return(true) + allow_any_instance_of(MergeRequest) + .to receive(:compare_test_reports).and_return(serialized_data) + + visit project_merge_request_path(project, merge_request) + end + + context 'when a new failures exists' do + let(:base_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_success) + end + end + + 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_failed) + end + end + + it 'shows test reports summary which includes the new failure' do + within(".mr-section-container") do + click_button 'Expand' + + expect(page).to have_content('Test summary contained 1 failed test result out of 2 total tests') + 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 failed test result out of 1 total test') + expect(page).to have_content('New') + expect(page).to have_content('subtractTest') + end + end + end + + context 'when user clicks the new failure' do + it 'shows the test report detail' do + within(".mr-section-container") do + click_button 'Expand' + + within(".js-report-section-container") do + click_button 'subtractTest' + + expect(page).to have_content('6.66') + expect(page).to have_content(sample_java_failed_message) + end + end + end + end + end + + context 'when an existing failure exists' do + let(:base_reports) do + Gitlab::Ci::Reports::TestReports.new.tap do |reports| + reports.get_suite('rspec').add_test_case(create_test_case_rspec_failed) + reports.get_suite('junit').add_test_case(create_test_case_java_success) + end + end + + let(:head_reports) do + Gitlab::Ci::Reports::TestReports.new.tap do |reports| + reports.get_suite('rspec').add_test_case(create_test_case_rspec_failed) + reports.get_suite('junit').add_test_case(create_test_case_java_success) + end + end + + it 'shows test reports summary which includes the existing failure' do + within(".mr-section-container") do + click_button 'Expand' + + expect(page).to have_content('Test summary contained 1 failed test result out of 2 total tests') + within(".js-report-section-container") 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') + end + end + end + + context 'when user clicks the existing failure' do + it 'shows test report detail of it' do + within(".mr-section-container") do + click_button 'Expand' + + within(".js-report-section-container") do + click_button 'Test#sum when a is 2 and b is 2 returns summary' + + expect(page).to have_content('2.22') + expect(page).to have_content(sample_rspec_failed_message) + end + end + end + end + end + + context 'when a resolved failure exists' do + let(:base_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_failed) + end + end + + 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) + end + end + + it 'shows test reports summary which includes the resolved failure' do + within(".mr-section-container") do + click_button 'Expand' + + expect(page).to have_content('Test summary contained 1 fixed test result out of 2 total tests') + 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') + end + end + end + + context 'when user clicks the resolved failure' do + it 'shows test report detail of it' do + within(".mr-section-container") do + click_button 'Expand' + + within(".js-report-section-container") do + click_button 'subtractTest' + + expect(page).to have_content('6.66') + end + end + end + end + end + + def comparer + Gitlab::Ci::Reports::TestReportsComparer.new(base_reports, head_reports) + end + end + end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 3512ba6aee5..77b7332a761 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -1856,9 +1856,7 @@ describe Ci::Pipeline, :mailer do context 'when pipeline has builds with test reports' do before do - create(:ci_build, pipeline: pipeline, project: project).tap do |build| - create(:ci_job_artifact, :junit, job: build, project: build.project) - end + create(:ci_build, :test_reports, pipeline: pipeline, project: project) end context 'when pipeline status is running' do @@ -1875,6 +1873,22 @@ describe Ci::Pipeline, :mailer do end context 'when pipeline does not have builds with test reports' do + before do + create(:ci_build, :artifacts, pipeline: pipeline, project: project) + end + + let(:pipeline) { create(:ci_pipeline, :success, project: project) } + + it { is_expected.to be_falsey } + end + + context 'when retried build has test reports' do + before do + create(:ci_build, :retried, :test_reports, pipeline: pipeline, project: project) + end + + let(:pipeline) { create(:ci_pipeline, :success, project: project) } + it { is_expected.to be_falsey } end end @@ -1883,14 +1897,12 @@ describe Ci::Pipeline, :mailer do subject { pipeline.test_reports } context 'when pipeline has multiple builds with test reports' do - before do - create(:ci_build, :success, name: 'rspec', pipeline: pipeline, project: project).tap do |build| - create(:ci_job_artifact, :junit, job: build, project: build.project) - end + let!(:build_rspec) { create(:ci_build, :success, name: 'rspec', pipeline: pipeline, project: project) } + let!(:build_java) { create(:ci_build, :success, name: 'java', pipeline: pipeline, project: project) } - create(:ci_build, :success, name: 'java', pipeline: pipeline, project: project).tap do |build| - create(:ci_job_artifact, :junit_with_ant, job: build, project: build.project) - end + before do + create(:ci_job_artifact, :junit, job: build_rspec, project: project) + create(:ci_job_artifact, :junit_with_ant, job: build_java, project: project) end it 'returns test reports with collected data' do @@ -1898,6 +1910,17 @@ describe Ci::Pipeline, :mailer do expect(subject.success_count).to be(5) expect(subject.failed_count).to be(2) end + + context 'when builds are retried' do + let!(:build_rspec) { create(:ci_build, :retried, :success, name: 'rspec', pipeline: pipeline, project: project) } + let!(:build_java) { create(:ci_build, :retried, :success, name: 'java', pipeline: pipeline, project: project) } + + it 'does not take retried builds into account' do + expect(subject.total_count).to be(0) + expect(subject.success_count).to be(0) + expect(subject.failed_count).to be(0) + end + end end context 'when pipeline does not have any builds with test reports' do diff --git a/spec/models/concerns/reactive_caching_spec.rb b/spec/models/concerns/reactive_caching_spec.rb index 79f75c0ffa0..97a4c212f1c 100644 --- a/spec/models/concerns/reactive_caching_spec.rb +++ b/spec/models/concerns/reactive_caching_spec.rb @@ -85,6 +85,14 @@ describe ReactiveCaching, :use_clean_rails_memory_store_caching do it { is_expected.to be_nil } end + + context 'when cache was invalidated' do + it 'refreshes cache' do + expect(ReactiveCachingWorker).to receive(:perform_async).with(CacheTest, 666) + + instance.with_reactive_cache { raise described_class::InvalidateReactiveCache } + end + end end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 3ab6a20cd55..6258bfa232f 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1204,10 +1204,21 @@ describe MergeRequest do it 'returns status and data' do expect_any_instance_of(Ci::CompareTestReportsService) - .to receive(:execute).with(base_pipeline.iid, head_pipeline.iid) + .to receive(:execute).with(base_pipeline, head_pipeline).and_call_original subject end + + context 'when cached results is not latest' do + before do + allow_any_instance_of(Ci::CompareTestReportsService) + .to receive(:latest?).and_return(false) + end + + it 'raises and InvalidateReactiveCache error' do + expect { subject }.to raise_error(ReactiveCaching::InvalidateReactiveCache) + end + end end end diff --git a/spec/services/ci/compare_test_reports_service_spec.rb b/spec/services/ci/compare_test_reports_service_spec.rb index d3bbf17cc5c..a26c970a8f0 100644 --- a/spec/services/ci/compare_test_reports_service_spec.rb +++ b/spec/services/ci/compare_test_reports_service_spec.rb @@ -5,7 +5,7 @@ describe Ci::CompareTestReportsService do let(:project) { create(:project, :repository) } describe '#execute' do - subject { service.execute(base_pipeline&.iid, head_pipeline.iid) } + subject { service.execute(base_pipeline, head_pipeline) } context 'when head pipeline has test reports' do let!(:base_pipeline) { nil } @@ -42,4 +42,34 @@ describe Ci::CompareTestReportsService do end end end + + describe '#latest?' do + subject { service.latest?(base_pipeline, head_pipeline, data) } + + let!(:base_pipeline) { nil } + let!(:head_pipeline) { create(:ci_pipeline, :with_test_reports, project: project) } + let!(:key) { service.send(:key, base_pipeline, head_pipeline) } + + context 'when cache key is latest' do + let(:data) { { key: key } } + + it { is_expected.to be_truthy } + end + + context 'when cache key is outdated' do + before do + head_pipeline.update_column(:updated_at, 10.minutes.ago) + end + + let(:data) { { key: key } } + + it { is_expected.to be_falsy } + end + + context 'when cache key is empty' do + let(:data) { { key: nil } } + + it { is_expected.to be_falsy } + end + end end |