Commit 9a455e95 authored by Grzegorz Bizon's avatar Grzegorz Bizon

Merge branch '46552-fixes-redundant-message-for-failure-reasons' into 'master'

Fixes redudant script failure message

Closes #46552 and #44271

See merge request gitlab-org/gitlab-ce!19138
parents 68ef6ae2 7a7a4356
class CommitStatusPresenter < Gitlab::View::Presenter::Delegated class CommitStatusPresenter < Gitlab::View::Presenter::Delegated
CALLOUT_FAILURE_MESSAGES = { CALLOUT_FAILURE_MESSAGES = {
unknown_failure: 'There is an unknown failure, please try again', 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', 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', 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', 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 }.freeze
presents :build presents :build
......
...@@ -26,7 +26,7 @@ class JobEntity < Grape::Entity ...@@ -26,7 +26,7 @@ class JobEntity < Grape::Entity
expose :created_at expose :created_at
expose :updated_at expose :updated_at
expose :detailed_status, as: :status, with: StatusEntity expose :detailed_status, as: :status, with: StatusEntity
expose :callout_message, if: -> (*) { failed? } expose :callout_message, if: -> (*) { failed? && !build.script_failure? }
expose :recoverable, if: -> (*) { failed? } expose :recoverable, if: -> (*) { failed? }
private private
......
---
title: Removes redundant script failure message from Job page
merge_request: 19138
author:
type: changed
...@@ -219,11 +219,11 @@ describe Ci::BuildPresenter do ...@@ -219,11 +219,11 @@ describe Ci::BuildPresenter do
end end
describe '#callout_failure_message' do 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 it 'returns a verbose failure reason' do
description = subject.callout_failure_message 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
end end
......
...@@ -131,7 +131,7 @@ describe JobEntity do ...@@ -131,7 +131,7 @@ describe JobEntity do
end end
context 'when job failed' do context 'when job failed' do
let(:job) { create(:ci_build, :script_failure) } let(:job) { create(:ci_build, :api_failure) }
it 'contains details' do it 'contains details' do
expect(subject[:status]).to include :icon, :favicon, :text, :label, :tooltip expect(subject[:status]).to include :icon, :favicon, :text, :label, :tooltip
...@@ -142,20 +142,20 @@ describe JobEntity do ...@@ -142,20 +142,20 @@ describe JobEntity do
end end
it 'should indicate the failure reason on tooltip' do it 'should indicate the failure reason on tooltip' do
expect(subject[:status][:tooltip]).to eq('failed <br> (script failure)') expect(subject[:status][:tooltip]).to eq('failed <br> (API failure)')
end end
it 'should include a callout message with a verbose output' do 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 end
it 'should state that it is not recoverable' do it 'should state that it is not recoverable' do
expect(subject[:recoverable]).to be_falsy expect(subject[:recoverable]).to be_truthy
end end
end end
context 'when job is allowed to fail' do 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 it 'contains details' do
expect(subject[:status]).to include :icon, :favicon, :text, :label, :tooltip expect(subject[:status]).to include :icon, :favicon, :text, :label, :tooltip
...@@ -166,15 +166,24 @@ describe JobEntity do ...@@ -166,15 +166,24 @@ describe JobEntity do
end end
it 'should indicate the failure reason on tooltip' do it 'should indicate the failure reason on tooltip' do
expect(subject[:status][:tooltip]).to eq('failed <br> (script failure) (allowed to fail)') expect(subject[:status][:tooltip]).to eq('failed <br> (API failure) (allowed to fail)')
end end
it 'should include a callout message with a verbose output' do 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 end
it 'should state that it is not recoverable' do 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
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment