From 0bc9e0b4feb746f1b9fe233bfbb6e9afd70e0b98 Mon Sep 17 00:00:00 2001 From: Mayra Cabrera Date: Thu, 24 May 2018 15:06:58 -0500 Subject: [PATCH] Removes redundant error message for script failures Script failure message was redundant so it was removed. Also 'check your job log' message was removed from all the error messages. Closes #44271 --- app/presenters/commit_status_presenter.rb | 3 +-- app/serializers/job_entity.rb | 2 +- ...-redundant-message-for-failure-reasons.yml | 5 ++++ spec/presenters/ci/build_presenter_spec.rb | 4 +-- spec/serializers/job_entity_spec.rb | 25 +++++++++++++------ 5 files changed, 26 insertions(+), 13 deletions(-) create mode 100644 changelogs/unreleased/46552-fixes-redundant-message-for-failure-reasons.yml diff --git a/app/presenters/commit_status_presenter.rb b/app/presenters/commit_status_presenter.rb index c7f7aa836bd..9a7aaf4ef32 100644 --- a/app/presenters/commit_status_presenter.rb +++ b/app/presenters/commit_status_presenter.rb @@ -1,11 +1,10 @@ class CommitStatusPresenter < Gitlab::View::Presenter::Delegated CALLOUT_FAILURE_MESSAGES = { unknown_failure: 'There is an unknown failure, please try again', - script_failure: 'There has been a script failure. Check the job log for more information', api_failure: 'There has been an API failure, please try again', stuck_or_timeout_failure: 'There has been a timeout failure or the job got stuck. Check your timeout limits or try again', runner_system_failure: 'There has been a runner system failure, please try again', - missing_dependency_failure: 'There has been a missing dependency failure, check the job log for more information' + missing_dependency_failure: 'There has been a missing dependency failure' }.freeze presents :build diff --git a/app/serializers/job_entity.rb b/app/serializers/job_entity.rb index 3076fed1674..30f0bda9418 100644 --- a/app/serializers/job_entity.rb +++ b/app/serializers/job_entity.rb @@ -54,7 +54,7 @@ class JobEntity < Grape::Entity end def failed? - build.failed? + build.failed? && !build.script_failure? end def callout_message diff --git a/changelogs/unreleased/46552-fixes-redundant-message-for-failure-reasons.yml b/changelogs/unreleased/46552-fixes-redundant-message-for-failure-reasons.yml new file mode 100644 index 00000000000..43427aaa242 --- /dev/null +++ b/changelogs/unreleased/46552-fixes-redundant-message-for-failure-reasons.yml @@ -0,0 +1,5 @@ +--- +title: Removes redundant script failure message from Job page +merge_request: 19138 +author: +type: changed diff --git a/spec/presenters/ci/build_presenter_spec.rb b/spec/presenters/ci/build_presenter_spec.rb index efd175247b5..6dfaa3b72f7 100644 --- a/spec/presenters/ci/build_presenter_spec.rb +++ b/spec/presenters/ci/build_presenter_spec.rb @@ -219,11 +219,11 @@ describe Ci::BuildPresenter do end describe '#callout_failure_message' do - let(:build) { create(:ci_build, :failed, :script_failure) } + let(:build) { create(:ci_build, :failed, :api_failure) } it 'returns a verbose failure reason' do description = subject.callout_failure_message - expect(description).to eq('There has been a script failure. Check the job log for more information') + expect(description).to eq('There has been an API failure, please try again') end end diff --git a/spec/serializers/job_entity_spec.rb b/spec/serializers/job_entity_spec.rb index c90396ebb28..a5581a34517 100644 --- a/spec/serializers/job_entity_spec.rb +++ b/spec/serializers/job_entity_spec.rb @@ -131,7 +131,7 @@ describe JobEntity do end context 'when job failed' do - let(:job) { create(:ci_build, :script_failure) } + let(:job) { create(:ci_build, :api_failure) } it 'contains details' do expect(subject[:status]).to include :icon, :favicon, :text, :label, :tooltip @@ -142,20 +142,20 @@ describe JobEntity do end it 'should indicate the failure reason on tooltip' do - expect(subject[:status][:tooltip]).to eq('failed
(script failure)') + expect(subject[:status][:tooltip]).to eq('failed
(API failure)') end it 'should include a callout message with a verbose output' do - expect(subject[:callout_message]).to eq('There has been a script failure. Check the job log for more information') + expect(subject[:callout_message]).to eq('There has been an API failure, please try again') end it 'should state that it is not recoverable' do - expect(subject[:recoverable]).to be_falsy + expect(subject[:recoverable]).to be_truthy end end context 'when job is allowed to fail' do - let(:job) { create(:ci_build, :allowed_to_fail, :script_failure) } + let(:job) { create(:ci_build, :allowed_to_fail, :api_failure) } it 'contains details' do expect(subject[:status]).to include :icon, :favicon, :text, :label, :tooltip @@ -166,15 +166,24 @@ describe JobEntity do end it 'should indicate the failure reason on tooltip' do - expect(subject[:status][:tooltip]).to eq('failed
(script failure) (allowed to fail)') + expect(subject[:status][:tooltip]).to eq('failed
(API failure) (allowed to fail)') end it 'should include a callout message with a verbose output' do - expect(subject[:callout_message]).to eq('There has been a script failure. Check the job log for more information') + expect(subject[:callout_message]).to eq('There has been an API failure, please try again') end it 'should state that it is not recoverable' do - expect(subject[:recoverable]).to be_falsy + expect(subject[:recoverable]).to be_truthy + end + end + + context 'when the job failed with a script failure' do + let(:job) { create(:ci_build, :failed, :script_failure) } + + it 'should not include callout message or recoverable keys' do + expect(subject).not_to include('callout_message') + expect(subject).not_to include('recoverable') end end