summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMayra Cabrera <mcabrera@gitlab.com>2018-04-05 21:04:42 +0000
committerGrzegorz Bizon <grzegorz@gitlab.com>2018-04-05 21:04:42 +0000
commitd54cf868f81ca957c8322661b11e6755d9ea5a85 (patch)
tree39ef104a65a13fc41a5fcc3a79b9e1ec9c154d3d
parentdd271e246001a06609592eef109d154291305d32 (diff)
downloadgitlab-ce-d54cf868f81ca957c8322661b11e6755d9ea5a85.tar.gz
Resolve "Show `failure_reason` and upgrade tooltips of jobs"
-rw-r--r--app/assets/javascripts/pipelines/components/graph/job_component.vue9
-rw-r--r--app/assets/stylesheets/pages/builds.scss2
-rw-r--r--app/controllers/projects/jobs_controller.rb8
-rw-r--r--app/models/concerns/presentable.rb8
-rw-r--r--app/presenters/ci/build_presenter.rb16
-rw-r--r--app/serializers/status_entity.rb2
-rw-r--r--app/views/ci/status/_badge.html.haml4
-rw-r--r--app/views/ci/status/_dropdown_graph_badge.html.haml7
-rw-r--r--app/views/projects/jobs/_sidebar.html.haml8
-rw-r--r--app/views/projects/jobs/show.html.haml2
-rw-r--r--changelogs/unreleased/44269-show-failure-reason-on-upgrade-tooltip-of-jobs.yml5
-rw-r--r--lib/gitlab/ci/status/build/factory.rb4
-rw-r--r--lib/gitlab/ci/status/build/failed.rb40
-rw-r--r--lib/gitlab/ci/status/build/failed_allowed.rb12
-rw-r--r--lib/gitlab/ci/status/build/retried.rb17
-rw-r--r--lib/gitlab/ci/status/core.rb10
-rw-r--r--spec/factories/ci/builds.rb5
-rw-r--r--spec/features/projects/jobs/user_browses_job_spec.rb22
-rw-r--r--spec/features/projects/jobs/user_browses_jobs_spec.rb11
-rw-r--r--spec/features/projects/pipelines/pipeline_spec.rb16
-rw-r--r--spec/features/projects/pipelines/pipelines_spec.rb17
-rw-r--r--spec/javascripts/pipelines/graph/job_component_spec.js2
-rw-r--r--spec/lib/gitlab/ci/status/build/action_spec.rb10
-rw-r--r--spec/lib/gitlab/ci/status/build/cancelable_spec.rb18
-rw-r--r--spec/lib/gitlab/ci/status/build/factory_spec.rb8
-rw-r--r--spec/lib/gitlab/ci/status/build/failed_allowed_spec.rb23
-rw-r--r--spec/lib/gitlab/ci/status/build/failed_spec.rb83
-rw-r--r--spec/lib/gitlab/ci/status/build/play_spec.rb16
-rw-r--r--spec/lib/gitlab/ci/status/build/retried_spec.rb96
-rw-r--r--spec/lib/gitlab/ci/status/build/retryable_spec.rb18
-rw-r--r--spec/lib/gitlab/ci/status/build/stop_spec.rb20
-rw-r--r--spec/lib/gitlab/ci/status/success_warning_spec.rb4
-rw-r--r--spec/presenters/ci/build_presenter_spec.rb93
-rw-r--r--spec/serializers/build_serializer_spec.rb22
-rw-r--r--spec/serializers/job_entity_spec.rb26
-rw-r--r--spec/serializers/pipeline_entity_spec.rb2
-rw-r--r--spec/serializers/stage_entity_spec.rb2
-rw-r--r--spec/serializers/status_entity_spec.rb2
-rw-r--r--spec/views/projects/jobs/show.html.haml_spec.rb3
39 files changed, 635 insertions, 38 deletions
diff --git a/app/assets/javascripts/pipelines/components/graph/job_component.vue b/app/assets/javascripts/pipelines/components/graph/job_component.vue
index 9b136573135..d501c465a96 100644
--- a/app/assets/javascripts/pipelines/components/graph/job_component.vue
+++ b/app/assets/javascripts/pipelines/components/graph/job_component.vue
@@ -17,6 +17,7 @@
* "text": "passed",
* "label": "passed",
* "group": "success",
+ * "tooltip": "passed",
* "details_path": "/root/ci-mock/builds/4256",
* "action": {
* "icon": "retry",
@@ -69,12 +70,12 @@
textBuilder.push(this.job.name);
}
- if (this.job.name && this.status.label) {
+ if (this.job.name && this.status.tooltip) {
textBuilder.push('-');
}
- if (this.status.label) {
- textBuilder.push(`${this.job.status.label}`);
+ if (this.status.tooltip) {
+ textBuilder.push(`${this.job.status.tooltip}`);
}
return textBuilder.join(' ');
@@ -100,6 +101,7 @@
:title="tooltipText"
:class="cssClassJobName"
data-container="body"
+ data-html="true"
class="js-pipeline-graph-job-link"
>
@@ -115,6 +117,7 @@
class="js-job-component-tooltip"
:title="tooltipText"
:class="cssClassJobName"
+ data-html="true"
data-container="body"
>
diff --git a/app/assets/stylesheets/pages/builds.scss b/app/assets/stylesheets/pages/builds.scss
index 98d460339cd..7a6352e45f1 100644
--- a/app/assets/stylesheets/pages/builds.scss
+++ b/app/assets/stylesheets/pages/builds.scss
@@ -391,7 +391,7 @@
}
&:hover {
- background-color: $row-hover;
+ background-color: $dropdown-item-hover-bg;
}
.icon-retry {
diff --git a/app/controllers/projects/jobs_controller.rb b/app/controllers/projects/jobs_controller.rb
index ac9eb2047a0..dd12d30a085 100644
--- a/app/controllers/projects/jobs_controller.rb
+++ b/app/controllers/projects/jobs_controller.rb
@@ -2,7 +2,6 @@ class Projects::JobsController < Projects::ApplicationController
include SendFileUpload
before_action :build, except: [:index, :cancel_all]
-
before_action :authorize_read_build!,
only: [:index, :show, :status, :raw, :trace]
before_action :authorize_update_build!,
@@ -45,8 +44,11 @@ class Projects::JobsController < Projects::ApplicationController
end
def show
- @builds = @project.pipelines.find_by_sha(@build.sha).builds.order('id DESC')
- @builds = @builds.where("id not in (?)", @build.id)
+ @builds = @project.pipelines
+ .find_by_sha(@build.sha)
+ .builds
+ .order('id DESC')
+ .present(current_user: current_user)
@pipeline = @build.pipeline
respond_to do |format|
diff --git a/app/models/concerns/presentable.rb b/app/models/concerns/presentable.rb
index 7b33b837004..bc4fbd19a02 100644
--- a/app/models/concerns/presentable.rb
+++ b/app/models/concerns/presentable.rb
@@ -1,4 +1,12 @@
module Presentable
+ extend ActiveSupport::Concern
+
+ class_methods do
+ def present(attributes)
+ all.map { |klass_object| klass_object.present(attributes) }
+ end
+ end
+
def present(**attributes)
Gitlab::View::Presenter::Factory
.new(self, attributes)
diff --git a/app/presenters/ci/build_presenter.rb b/app/presenters/ci/build_presenter.rb
index 255475e1fe6..9afebda19be 100644
--- a/app/presenters/ci/build_presenter.rb
+++ b/app/presenters/ci/build_presenter.rb
@@ -15,6 +15,8 @@ module Ci
def status_title
if auto_canceled?
"Job is redundant and is auto-canceled by Pipeline ##{auto_canceled_by_id}"
+ else
+ tooltip_for_badge
end
end
@@ -28,5 +30,19 @@ module Ci
trigger_request.user_variables
end
end
+
+ def tooltip_message
+ "#{subject.name} - #{detailed_status.status_tooltip}"
+ end
+
+ private
+
+ def tooltip_for_badge
+ detailed_status.badge_tooltip.capitalize
+ end
+
+ def detailed_status
+ @detailed_status ||= subject.detailed_status(user)
+ end
end
end
diff --git a/app/serializers/status_entity.rb b/app/serializers/status_entity.rb
index a7c2e21e92b..8e8bda2f9df 100644
--- a/app/serializers/status_entity.rb
+++ b/app/serializers/status_entity.rb
@@ -2,7 +2,7 @@ class StatusEntity < Grape::Entity
include RequestAwareEntity
expose :icon, :text, :label, :group
-
+ expose :status_tooltip, as: :tooltip
expose :has_details?, as: :has_details
expose :details_path
diff --git a/app/views/ci/status/_badge.html.haml b/app/views/ci/status/_badge.html.haml
index 35a3563dff1..5114387984b 100644
--- a/app/views/ci/status/_badge.html.haml
+++ b/app/views/ci/status/_badge.html.haml
@@ -4,10 +4,10 @@
- css_classes = "ci-status ci-#{status.group} #{'has-tooltip' if title.present?}"
- if link && status.has_details?
- = link_to status.details_path, class: css_classes, title: title do
+ = link_to status.details_path, class: css_classes, title: title, data: { html: title.present? } do
= sprite_icon(status.icon)
= status.text
- else
- %span{ class: css_classes, title: title }
+ %span{ class: css_classes, title: title, data: { html: title.present? } }
= sprite_icon(status.icon)
= status.text
diff --git a/app/views/ci/status/_dropdown_graph_badge.html.haml b/app/views/ci/status/_dropdown_graph_badge.html.haml
index c5b4439e273..db2040110fa 100644
--- a/app/views/ci/status/_dropdown_graph_badge.html.haml
+++ b/app/views/ci/status/_dropdown_graph_badge.html.haml
@@ -3,14 +3,15 @@
- subject = local_assigns.fetch(:subject)
- status = subject.detailed_status(current_user)
- klass = "ci-status-icon ci-status-icon-#{status.group}"
-- tooltip = "#{subject.name} - #{status.label}"
+- tooltip = "#{subject.name} - #{status.status_tooltip}"
- if status.has_details?
- = link_to status.details_path, class: 'mini-pipeline-graph-dropdown-item', data: { toggle: 'tooltip', title: tooltip, container: 'body' } do
+ = link_to status.details_path, class: 'mini-pipeline-graph-dropdown-item', data: { toggle: 'tooltip', title: tooltip, html: true, container: 'body' } do
%span{ class: klass }= sprite_icon(status.icon)
%span.ci-build-text= subject.name
+
- else
- .menu-item.mini-pipeline-graph-dropdown-item{ data: { toggle: 'tooltip', title: tooltip, container: 'body' } }
+ .menu-item.mini-pipeline-graph-dropdown-item{ data: { toggle: 'tooltip', html: true, title: tooltip, container: 'body' } }
%span{ class: klass }= sprite_icon(status.icon)
%span.ci-build-text= subject.name
diff --git a/app/views/projects/jobs/_sidebar.html.haml b/app/views/projects/jobs/_sidebar.html.haml
index ecf186e3dc8..0b57ebedebd 100644
--- a/app/views/projects/jobs/_sidebar.html.haml
+++ b/app/views/projects/jobs/_sidebar.html.haml
@@ -1,5 +1,3 @@
-- builds = @build.pipeline.builds.to_a
-
%aside.right-sidebar.right-sidebar-expanded.build-sidebar.js-build-sidebar.js-right-sidebar{ data: { "offset-top" => "101", "spy" => "affix" } }
.sidebar-container
.blocks-container
@@ -91,7 +89,8 @@
- HasStatus::ORDERED_STATUSES.each do |build_status|
- builds.select{|build| build.status == build_status}.each do |build|
.build-job{ class: sidebar_build_class(build, @build), data: { stage: build.stage } }
- = link_to project_job_path(@project, build) do
+ - tooltip = build.tooltip_message
+ = link_to(project_job_path(@project, build), data: { toggle: 'tooltip', html: true, title: tooltip, container: 'body' }) do
= sprite_icon('arrow-right', size:16, css_class: 'icon-arrow-right')
%span{ class: "ci-status-icon-#{build.status}" }
= ci_icon_for_status(build.status)
@@ -101,5 +100,4 @@
- else
= build.id
- if build.retried?
- %span.has-tooltip{ data: { container: 'body', placement: 'bottom' }, title: 'Job was retried' }
- = sprite_icon('retry', size:16, css_class: 'icon-retry')
+ = sprite_icon('retry', size:16, css_class: 'icon-retry')
diff --git a/app/views/projects/jobs/show.html.haml b/app/views/projects/jobs/show.html.haml
index fa27ded7cc2..dece4dfe167 100644
--- a/app/views/projects/jobs/show.html.haml
+++ b/app/views/projects/jobs/show.html.haml
@@ -107,7 +107,7 @@
illustration_size: 'svg-430',
title: _('This job has not started yet'),
content: _('This job is in pending state and is waiting to be picked by a runner')
- = render "sidebar"
+ = render "sidebar", builds: @builds
.js-build-options{ data: javascript_build_options }
diff --git a/changelogs/unreleased/44269-show-failure-reason-on-upgrade-tooltip-of-jobs.yml b/changelogs/unreleased/44269-show-failure-reason-on-upgrade-tooltip-of-jobs.yml
new file mode 100644
index 00000000000..b3ae8ca7340
--- /dev/null
+++ b/changelogs/unreleased/44269-show-failure-reason-on-upgrade-tooltip-of-jobs.yml
@@ -0,0 +1,5 @@
+---
+title: Display error message on job's tooltip if this one fails
+merge_request: 17782
+author:
+type: added
diff --git a/lib/gitlab/ci/status/build/factory.rb b/lib/gitlab/ci/status/build/factory.rb
index c852d607373..20a319caf86 100644
--- a/lib/gitlab/ci/status/build/factory.rb
+++ b/lib/gitlab/ci/status/build/factory.rb
@@ -6,10 +6,12 @@ module Gitlab
def self.extended_statuses
[[Status::Build::Cancelable,
Status::Build::Retryable],
+ [Status::Build::Failed],
[Status::Build::FailedAllowed,
Status::Build::Play,
Status::Build::Stop],
- [Status::Build::Action]]
+ [Status::Build::Action],
+ [Status::Build::Retried]]
end
def self.common_helpers
diff --git a/lib/gitlab/ci/status/build/failed.rb b/lib/gitlab/ci/status/build/failed.rb
new file mode 100644
index 00000000000..155f4fc1343
--- /dev/null
+++ b/lib/gitlab/ci/status/build/failed.rb
@@ -0,0 +1,40 @@
+module Gitlab
+ module Ci
+ module Status
+ module Build
+ class Failed < Status::Extended
+ REASONS = {
+ 'unknown_failure' => 'unknown failure',
+ 'script_failure' => 'script failure',
+ 'api_failure' => 'API failure',
+ 'stuck_or_timeout_failure' => 'stuck or timeout failure',
+ 'runner_system_failure' => 'runner system failure',
+ 'missing_dependency_failure' => 'missing dependency failure'
+ }.freeze
+
+ def status_tooltip
+ base_message
+ end
+
+ def badge_tooltip
+ base_message
+ end
+
+ def self.matches?(build, user)
+ build.failed?
+ end
+
+ private
+
+ def base_message
+ "#{s_('CiStatusLabel|failed')} #{description}"
+ end
+
+ def description
+ "<br> (#{REASONS[subject.failure_reason]})"
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/ci/status/build/failed_allowed.rb b/lib/gitlab/ci/status/build/failed_allowed.rb
index dc90f398c7e..ca0046fb1f7 100644
--- a/lib/gitlab/ci/status/build/failed_allowed.rb
+++ b/lib/gitlab/ci/status/build/failed_allowed.rb
@@ -4,7 +4,7 @@ module Gitlab
module Build
class FailedAllowed < Status::Extended
def label
- 'failed (allowed to fail)'
+ "failed #{allowed_to_fail_title}"
end
def icon
@@ -15,9 +15,19 @@ module Gitlab
'failed_with_warnings'
end
+ def status_tooltip
+ "#{@status.status_tooltip} #{allowed_to_fail_title}"
+ end
+
def self.matches?(build, user)
build.failed? && build.allow_failure?
end
+
+ private
+
+ def allowed_to_fail_title
+ "(allowed to fail)"
+ end
end
end
end
diff --git a/lib/gitlab/ci/status/build/retried.rb b/lib/gitlab/ci/status/build/retried.rb
new file mode 100644
index 00000000000..6e190e4ee3c
--- /dev/null
+++ b/lib/gitlab/ci/status/build/retried.rb
@@ -0,0 +1,17 @@
+module Gitlab
+ module Ci
+ module Status
+ module Build
+ class Retried < Status::Extended
+ def status_tooltip
+ @status.status_tooltip + " (retried)"
+ end
+
+ def self.matches?(build, user)
+ build.retried?
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/ci/status/core.rb b/lib/gitlab/ci/status/core.rb
index d4fd83b93f8..daab6bb2de5 100644
--- a/lib/gitlab/ci/status/core.rb
+++ b/lib/gitlab/ci/status/core.rb
@@ -57,6 +57,16 @@ module Gitlab
def action_title
raise NotImplementedError
end
+
+ # Hint that appears on all the pipeline graph tooltips and builds on the right sidebar in Job detail view
+ def status_tooltip
+ label
+ end
+
+ # Hint that appears on the build badges
+ def badge_tooltip
+ subject.status
+ end
end
end
end
diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb
index 30235a3a876..fdacbe6c3f1 100644
--- a/spec/factories/ci/builds.rb
+++ b/spec/factories/ci/builds.rb
@@ -238,5 +238,10 @@ FactoryBot.define do
trait :protected do
protected true
end
+
+ trait :script_failure do
+ failed
+ failure_reason 1
+ end
end
end
diff --git a/spec/features/projects/jobs/user_browses_job_spec.rb b/spec/features/projects/jobs/user_browses_job_spec.rb
index 4c49cff30d4..b7eee39052a 100644
--- a/spec/features/projects/jobs/user_browses_job_spec.rb
+++ b/spec/features/projects/jobs/user_browses_job_spec.rb
@@ -34,4 +34,26 @@ describe 'User browses a job', :js do
expect(build.project.running_or_pending_build_count).to eq(build.project.builds.running_or_pending.count(:all))
end
+
+ context 'with a failed job' do
+ let!(:build) { create(:ci_build, :failed, pipeline: pipeline) }
+
+ it 'displays the failure reason' do
+ within('.builds-container') do
+ build_link = first('.build-job > a')
+ expect(build_link['data-title']).to eq('test - failed <br> (unknown failure)')
+ end
+ end
+ end
+
+ context 'when a failed job has been retried' do
+ let!(:build) { create(:ci_build, :failed, :retried, pipeline: pipeline) }
+
+ it 'displays the failure reason and retried label' do
+ within('.builds-container') do
+ build_link = first('.build-job > a')
+ expect(build_link['data-title']).to eq('test - failed <br> (unknown failure) (retried)')
+ end
+ end
+ end
end
diff --git a/spec/features/projects/jobs/user_browses_jobs_spec.rb b/spec/features/projects/jobs/user_browses_jobs_spec.rb
index 767777f3bf9..36ebbeadd4a 100644
--- a/spec/features/projects/jobs/user_browses_jobs_spec.rb
+++ b/spec/features/projects/jobs/user_browses_jobs_spec.rb
@@ -29,4 +29,15 @@ describe 'User browses jobs' do
expect(ci_lint_tool_link[:href]).to end_with(ci_lint_path)
end
end
+
+ context 'with a failed job' do
+ let!(:build) { create(:ci_build, :coverage, :failed, pipeline: pipeline) }
+
+ it 'displays a tooltip with the failure reason' do
+ page.within('.ci-table') do
+ failed_job_link = page.find('.ci-failed')
+ expect(failed_job_link[:title]).to eq('Failed <br> (unknown failure)')
+ end
+ end
+ end
end
diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb
index 266ef693d0b..990e5c4d9df 100644
--- a/spec/features/projects/pipelines/pipeline_spec.rb
+++ b/spec/features/projects/pipelines/pipeline_spec.rb
@@ -115,6 +115,13 @@ describe 'Pipeline', :js do
expect(page).not_to have_content('Retry job')
end
+
+ it 'should include the failure reason' do
+ page.within('#ci-badge-test') do
+ build_link = page.find('.js-pipeline-graph-job-link')
+ expect(build_link['data-original-title']).to eq('test - failed <br> (unknown failure)')
+ end
+ end
end
context 'when pipeline has manual jobs' do
@@ -289,6 +296,15 @@ describe 'Pipeline', :js do
it { expect(build_manual.reload).to be_pending }
end
+
+ context 'failed jobs' do
+ it 'displays a tooltip with the failure reason' do
+ page.within('.ci-table') do
+ failed_job_link = page.find('.ci-failed')
+ expect(failed_job_link[:title]).to eq('Failed <br> (unknown failure)')
+ end
+ end
+ end
end
describe 'GET /:project/pipelines/:id/failures' do
diff --git a/spec/features/projects/pipelines/pipelines_spec.rb b/spec/features/projects/pipelines/pipelines_spec.rb
index 0e81c6c629a..6e63e0f0b49 100644
--- a/spec/features/projects/pipelines/pipelines_spec.rb
+++ b/spec/features/projects/pipelines/pipelines_spec.rb
@@ -394,6 +394,23 @@ describe 'Pipelines', :js do
expect(build.reload).to be_canceled
end
end
+
+ context 'for a failed pipeline' do
+ let!(:build) do
+ create(:ci_build, :failed, pipeline: pipeline,
+ stage: 'build',
+ name: 'build')
+ end
+
+ it 'should display the failure reason' do
+ find('.js-builds-dropdown-button').click
+
+ within('.js-builds-dropdown-list') do
+ build_element = page.find('.mini-pipeline-graph-dropdown-item')
+ expect(build_element['data-title']).to eq('build - failed <br> (unknown failure)')
+ end
+ end
+ end
end
context 'with pagination' do
diff --git a/spec/javascripts/pipelines/graph/job_component_spec.js b/spec/javascripts/pipelines/graph/job_component_spec.js
index ce181a1e515..c9677ae209a 100644
--- a/spec/javascripts/pipelines/graph/job_component_spec.js
+++ b/spec/javascripts/pipelines/graph/job_component_spec.js
@@ -13,6 +13,7 @@ describe('pipeline graph job component', () => {
icon: 'icon_status_success',
text: 'passed',
label: 'passed',
+ tooltip: 'passed',
group: 'success',
details_path: '/root/ci-mock/builds/4256',
has_details: true,
@@ -137,6 +138,7 @@ describe('pipeline graph job component', () => {
status: {
icon: 'icon_status_success',
label: 'success',
+ tooltip: 'success',
},
},
});
diff --git a/spec/lib/gitlab/ci/status/build/action_spec.rb b/spec/lib/gitlab/ci/status/build/action_spec.rb
index d612d29e3e0..bdec582b57b 100644
--- a/spec/lib/gitlab/ci/status/build/action_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/action_spec.rb
@@ -53,4 +53,14 @@ describe Gitlab::Ci::Status::Build::Action do
end
end
end
+
+ describe '#badge_tooltip' do
+ let(:user) { create(:user) }
+ let(:build) { create(:ci_build, :non_playable) }
+ let(:status) { Gitlab::Ci::Status::Core.new(build, user) }
+
+ it 'returns the status' do
+ expect(subject.badge_tooltip).to eq('created')
+ end
+ end
end
diff --git a/spec/lib/gitlab/ci/status/build/cancelable_spec.rb b/spec/lib/gitlab/ci/status/build/cancelable_spec.rb
index 9cdebaa5cf2..3ef0b6817e9 100644
--- a/spec/lib/gitlab/ci/status/build/cancelable_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/cancelable_spec.rb
@@ -40,6 +40,24 @@ describe Gitlab::Ci::Status::Build::Cancelable do
end
end
+ describe '#status_tooltip' do
+ it 'does not override status status_tooltip' do
+ expect(status).to receive(:status_tooltip)
+
+ subject.status_tooltip
+ end
+ end
+
+ describe '#badge_tooltip' do
+ let(:user) { create(:user) }
+ let(:build) { create(:ci_build) }
+ let(:status) { Gitlab::Ci::Status::Core.new(build, user) }
+
+ it 'returns the status' do
+ expect(subject.badge_tooltip).to eq('pending')
+ end
+ end
+
describe 'action details' do
let(:user) { create(:user) }
let(:build) { create(:ci_build) }
diff --git a/spec/lib/gitlab/ci/status/build/factory_spec.rb b/spec/lib/gitlab/ci/status/build/factory_spec.rb
index d196bc6a4c2..bbfa60169a1 100644
--- a/spec/lib/gitlab/ci/status/build/factory_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/factory_spec.rb
@@ -48,11 +48,11 @@ describe Gitlab::Ci::Status::Build::Factory do
it 'matches correct extended statuses' do
expect(factory.extended_statuses)
- .to eq [Gitlab::Ci::Status::Build::Retryable]
+ .to eq [Gitlab::Ci::Status::Build::Retryable, Gitlab::Ci::Status::Build::Failed]
end
- it 'fabricates a retryable build status' do
- expect(status).to be_a Gitlab::Ci::Status::Build::Retryable
+ it 'fabricates a failed build status' do
+ expect(status).to be_a Gitlab::Ci::Status::Build::Failed
end
it 'fabricates status with correct details' do
@@ -60,6 +60,7 @@ describe Gitlab::Ci::Status::Build::Factory do
expect(status.icon).to eq 'status_failed'
expect(status.favicon).to eq 'favicon_status_failed'
expect(status.label).to eq 'failed'
+ expect(status.status_tooltip).to eq 'failed <br> (unknown failure)'
expect(status).to have_details
expect(status).to have_action
end
@@ -75,6 +76,7 @@ describe Gitlab::Ci::Status::Build::Factory do
it 'matches correct extended statuses' do
expect(factory.extended_statuses)
.to eq [Gitlab::Ci::Status::Build::Retryable,
+ Gitlab::Ci::Status::Build::Failed,
Gitlab::Ci::Status::Build::FailedAllowed]
end
diff --git a/spec/lib/gitlab/ci/status/build/failed_allowed_spec.rb b/spec/lib/gitlab/ci/status/build/failed_allowed_spec.rb
index 99a5a7e4aca..bfaa508785e 100644
--- a/spec/lib/gitlab/ci/status/build/failed_allowed_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/failed_allowed_spec.rb
@@ -3,6 +3,7 @@ require 'spec_helper'
describe Gitlab::Ci::Status::Build::FailedAllowed do
let(:status) { double('core status') }
let(:user) { double('user') }
+ let(:build) { create(:ci_build, :failed, :allowed_to_fail) }
subject do
described_class.new(status)
@@ -68,6 +69,28 @@ describe Gitlab::Ci::Status::Build::FailedAllowed do
end
end
+ describe '#badge_tooltip' do
+ let(:user) { create(:user) }
+ let(:failed_status) { Gitlab::Ci::Status::Failed.new(build, user) }
+ let(:build_status) { Gitlab::Ci::Status::Build::Failed.new(failed_status) }
+ let(:status) { described_class.new(build_status) }
+
+ it 'does override badge_tooltip' do
+ expect(status.badge_tooltip).to eq('failed <br> (unknown failure)')
+ end
+ end
+
+ describe '#status_tooltip' do
+ let(:user) { create(:user) }
+ let(:failed_status) { Gitlab::Ci::Status::Failed.new(build, user) }
+ let(:build_status) { Gitlab::Ci::Status::Build::Failed.new(failed_status) }
+ let(:status) { described_class.new(build_status) }
+
+ it 'does override status_tooltip' do
+ expect(status.status_tooltip).to eq 'failed <br> (unknown failure) (allowed to fail)'
+ end
+ end
+
describe '.matches?' do
subject { described_class.matches?(build, user) }
diff --git a/spec/lib/gitlab/ci/status/build/failed_spec.rb b/spec/lib/gitlab/ci/status/build/failed_spec.rb
new file mode 100644
index 00000000000..cadb424ea2c
--- /dev/null
+++ b/spec/lib/gitlab/ci/status/build/failed_spec.rb
@@ -0,0 +1,83 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Status::Build::Failed do
+ let(:build) { create(:ci_build, :script_failure) }
+ let(:status) { double('core status') }
+ let(:user) { double('user') }
+
+ subject { described_class.new(status) }
+
+ describe '#text' do
+ it 'does not override status text' do
+ expect(status).to receive(:text)
+
+ subject.text
+ end
+ end
+
+ describe '#icon' do
+ it 'does not override status icon' do
+ expect(status).to receive(:icon)
+
+ subject.icon
+ end
+ end
+
+ describe '#group' do
+ it 'does not override status group' do
+ expect(status).to receive(:group)
+
+ subject.group
+ end
+ end
+
+ describe '#favicon' do
+ it 'does not override status label' do
+ expect(status).to receive(:favicon)
+
+ subject.favicon
+ end
+ end
+
+ describe '#label' do
+ it 'does not override label' do
+ expect(status).to receive(:label)
+
+ subject.label
+ end
+ end
+
+ describe '#badge_tooltip' do
+ let(:user) { create(:user) }
+ let(:status) { Gitlab::Ci::Status::Failed.new(build, user) }
+
+ it 'does override badge_tooltip' do
+ expect(subject.badge_tooltip).to eq 'failed <br> (script failure)'
+ end
+ end
+
+ describe '#status_tooltip' do
+ let(:user) { create(:user) }
+ let(:status) { Gitlab::Ci::Status::Failed.new(build, user) }
+
+ it 'does override status_tooltip' do
+ expect(subject.status_tooltip).to eq 'failed <br> (script failure)'
+ end
+ end
+
+ describe '.matches?' do
+ context 'with a failed build' do
+ it 'returns true' do
+ expect(described_class.matches?(build, user)).to be_truthy
+ end
+ end
+
+ context 'with any other type of build' do
+ let(:build) { create(:ci_build, :success) }
+
+ it 'returns false' do
+ expect(described_class.matches?(build, user)).to be_falsy
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/ci/status/build/play_spec.rb b/spec/lib/gitlab/ci/status/build/play_spec.rb
index 81d5f553fd1..35e47cd2526 100644
--- a/spec/lib/gitlab/ci/status/build/play_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/play_spec.rb
@@ -14,6 +14,22 @@ describe Gitlab::Ci::Status::Build::Play do
end
end
+ describe '#status_tooltip' do
+ it 'does not override status status_tooltip' do
+ expect(status).to receive(:status_tooltip)
+
+ subject.status_tooltip
+ end
+ end
+
+ describe '#badge_tooltip' do
+ it 'does not override status badge_tooltip' do
+ expect(status).to receive(:badge_tooltip)
+
+ subject.badge_tooltip
+ end
+ end
+
describe '#has_action?' do
context 'when user is allowed to update build' do
context 'when user is allowed to trigger protected action' do
diff --git a/spec/lib/gitlab/ci/status/build/retried_spec.rb b/spec/lib/gitlab/ci/status/build/retried_spec.rb
new file mode 100644
index 00000000000..ee9acaf1c21
--- /dev/null
+++ b/spec/lib/gitlab/ci/status/build/retried_spec.rb
@@ -0,0 +1,96 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Status::Build::Retried do
+ let(:build) { create(:ci_build, :retried) }
+ let(:status) { double('core status') }
+ let(:user) { double('user') }
+
+ subject { described_class.new(status) }
+
+ describe '#text' do
+ it 'does not override status text' do
+ expect(status).to receive(:text)
+
+ subject.text
+ end
+ end
+
+ describe '#icon' do
+ it 'does not override status icon' do
+ expect(status).to receive(:icon)
+
+ subject.icon
+ end
+ end
+
+ describe '#group' do
+ it 'does not override status group' do
+ expect(status).to receive(:group)
+
+ subject.group
+ end
+ end
+
+ describe '#favicon' do
+ it 'does not override status label' do
+ expect(status).to receive(:favicon)
+
+ subject.favicon
+ end
+ end
+
+ describe '#label' do
+ it 'does not override status label' do
+ expect(status).to receive(:label)
+
+ subject.label
+ end
+ end
+
+ describe '#badge_tooltip' do
+ let(:user) { create(:user) }
+ let(:build) { create(:ci_build, :retried) }
+ let(:status) { Gitlab::Ci::Status::Success.new(build, user) }
+
+ it 'returns status' do
+ expect(status.badge_tooltip).to eq('pending')
+ end
+ end
+
+ describe '#status_tooltip' do
+ let(:user) { create(:user) }
+
+ context 'with a failed build' do
+ let(:build) { create(:ci_build, :failed, :retried) }
+ let(:failed_status) { Gitlab::Ci::Status::Failed.new(build, user) }
+ let(:status) { Gitlab::Ci::Status::Build::Failed.new(failed_status) }
+
+ it 'does override status_tooltip' do
+ expect(subject.status_tooltip).to eq 'failed <br> (unknown failure) (retried)'
+ end
+ end
+
+ context 'with another build' do
+ let(:build) { create(:ci_build, :retried) }
+ let(:status) { Gitlab::Ci::Status::Success.new(build, user) }
+
+ it 'does override status_tooltip' do
+ expect(subject.status_tooltip).to eq 'passed (retried)'
+ end
+ end
+ end
+
+ describe '.matches?' do
+ subject { described_class.matches?(build, user) }
+
+ context 'with a retried build' do
+ it { is_expected.to be_truthy }
+ end
+
+ context 'with a build that has not been retried' do
+ let(:build) { create(:ci_build, :success) }
+
+ it { is_expected.to be_falsy }
+ end
+ end
+end
diff --git a/spec/lib/gitlab/ci/status/build/retryable_spec.rb b/spec/lib/gitlab/ci/status/build/retryable_spec.rb
index 14d42e0d70f..0c5099b7da5 100644
--- a/spec/lib/gitlab/ci/status/build/retryable_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/retryable_spec.rb
@@ -40,6 +40,24 @@ describe Gitlab::Ci::Status::Build::Retryable do
end
end
+ describe '#status_tooltip' do
+ it 'does not override status status_tooltip' do
+ expect(status).to receive(:status_tooltip)
+
+ subject.status_tooltip
+ end
+ end
+
+ describe '#badge_tooltip' do
+ let(:user) { create(:user) }
+ let(:build) { create(:ci_build) }
+ let(:status) { Gitlab::Ci::Status::Core.new(build, user) }
+
+ it 'does return status' do
+ expect(status.badge_tooltip).to eq('pending')
+ end
+ end
+
describe 'action details' do
let(:user) { create(:user) }
let(:build) { create(:ci_build) }
diff --git a/spec/lib/gitlab/ci/status/build/stop_spec.rb b/spec/lib/gitlab/ci/status/build/stop_spec.rb
index 18e250772f0..f16fc5c9205 100644
--- a/spec/lib/gitlab/ci/status/build/stop_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/stop_spec.rb
@@ -77,4 +77,24 @@ describe Gitlab::Ci::Status::Build::Stop do
end
end
end
+
+ describe '#status_tooltip' do
+ it 'does not override status status_tooltip' do
+ expect(status).to receive(:status_tooltip)
+
+ subject.status_tooltip
+ end
+ end
+
+ describe '#badge_tooltip' do
+ let(:user) { create(:user) }
+ let(:build) { create(:ci_build, :playable) }
+ let(:status) { Gitlab::Ci::Status::Core.new(build, user) }
+
+ it 'does not override status badge_tooltip' do
+ expect(status).to receive(:badge_tooltip)
+
+ subject.badge_tooltip
+ end
+ end
end
diff --git a/spec/lib/gitlab/ci/status/success_warning_spec.rb b/spec/lib/gitlab/ci/status/success_warning_spec.rb
index 4582354e739..6d05545d1d8 100644
--- a/spec/lib/gitlab/ci/status/success_warning_spec.rb
+++ b/spec/lib/gitlab/ci/status/success_warning_spec.rb
@@ -1,8 +1,10 @@
require 'spec_helper'
describe Gitlab::Ci::Status::SuccessWarning do
+ let(:status) { double('status') }
+
subject do
- described_class.new(double('status'))
+ described_class.new(status)
end
describe '#test' do
diff --git a/spec/presenters/ci/build_presenter_spec.rb b/spec/presenters/ci/build_presenter_spec.rb
index 1a8001be6ab..cc16d0f156b 100644
--- a/spec/presenters/ci/build_presenter_spec.rb
+++ b/spec/presenters/ci/build_presenter_spec.rb
@@ -72,13 +72,44 @@ describe Ci::BuildPresenter do
end
end
- context 'when build is not auto-canceled' do
- before do
- expect(build).to receive(:auto_canceled?).and_return(false)
+ context 'when build failed' do
+ let(:build) { create(:ci_build, :failed, pipeline: pipeline) }
+
+ it 'returns the reason of failure' do
+ status_title = presenter.status_title
+
+ expect(status_title).to eq('Failed <br> (unknown failure)')
end
+ end
+
+ context 'when build has failed && retried' do
+ let(:build) { create(:ci_build, :failed, :retried, pipeline: pipeline) }
- it 'does not have a status title' do
- expect(presenter.status_title).to be_nil
+ it 'does not include retried title' do
+ status_title = presenter.status_title
+
+ expect(status_title).not_to include('(retried)')
+ expect(status_title).to eq('Failed <br> (unknown failure)')
+ end
+ end
+
+ context 'when build has failed and is allowed to' do
+ let(:build) { create(:ci_build, :failed, :allowed_to_fail, pipeline: pipeline) }
+
+ it 'returns the reason of failure' do
+ status_title = presenter.status_title
+
+ expect(status_title).to eq('Failed <br> (unknown failure)')
+ end
+ end
+
+ context 'For any other build' do
+ let(:build) { create(:ci_build, :success, pipeline: pipeline) }
+
+ it 'returns the status' do
+ tooltip_description = presenter.status_title
+
+ expect(tooltip_description).to eq('Success')
end
end
end
@@ -134,4 +165,56 @@ describe Ci::BuildPresenter do
end
end
end
+
+ describe '#tooltip_message' do
+ context 'When build has failed' do
+ let(:build) { create(:ci_build, :script_failure, pipeline: pipeline) }
+
+ it 'returns the reason of failure' do
+ tooltip = subject.tooltip_message
+
+ expect(tooltip).to eq("#{build.name} - failed <br> (script failure)")
+ end
+ end
+
+ context 'When build has failed and retried' do
+ let(:build) { create(:ci_build, :script_failure, :retried, pipeline: pipeline) }
+
+ it 'should include the reason of failure and the retried title' do
+ tooltip = subject.tooltip_message
+
+ expect(tooltip).to eq("#{build.name} - failed <br> (script failure) (retried)")
+ end
+ end
+
+ context 'When build has failed and is allowed to' do
+ let(:build) { create(:ci_build, :script_failure, :allowed_to_fail, pipeline: pipeline) }
+
+ it 'should include the reason of failure' do
+ tooltip = subject.tooltip_message
+
+ expect(tooltip).to eq("#{build.name} - failed <br> (script failure) (allowed to fail)")
+ end
+ end
+
+ context 'For any other build (no retried)' do
+ let(:build) { create(:ci_build, :success, pipeline: pipeline) }
+
+ it 'should include build name and status' do
+ tooltip = subject.tooltip_message
+
+ expect(tooltip).to eq("#{build.name} - passed")
+ end
+ end
+
+ context 'For any other build (retried)' do
+ let(:build) { create(:ci_build, :success, :retried, pipeline: pipeline) }
+
+ it 'should include build name and status' do
+ tooltip = subject.tooltip_message
+
+ expect(tooltip).to eq("#{build.name} - passed (retried)")
+ end
+ end
+ end
end
diff --git a/spec/serializers/build_serializer_spec.rb b/spec/serializers/build_serializer_spec.rb
index 9673b11c2a2..98cd15e248b 100644
--- a/spec/serializers/build_serializer_spec.rb
+++ b/spec/serializers/build_serializer_spec.rb
@@ -28,15 +28,31 @@ describe BuildSerializer do
end
describe '#represent_status' do
- context 'when represents only status' do
- let(:resource) { create(:ci_build) }
+ context 'for a failed build' do
+ let(:resource) { create(:ci_build, :failed) }
+ let(:status) { resource.detailed_status(double('user')) }
+
+ subject { serializer.represent_status(resource) }
+
+ it 'serializes only status' do
+ expect(subject[:text]).to eq(status.text)
+ expect(subject[:label]).to eq('failed')
+ expect(subject[:tooltip]).to eq('failed <br> (unknown failure)')
+ expect(subject[:icon]).to eq(status.icon)
+ expect(subject[:favicon]).to match_asset_path("/assets/ci_favicons/#{status.favicon}.ico")
+ end
+ end
+
+ context 'for any other type of build' do
+ let(:resource) { create(:ci_build, :success) }
let(:status) { resource.detailed_status(double('user')) }
subject { serializer.represent_status(resource) }
it 'serializes only status' do
expect(subject[:text]).to eq(status.text)
- expect(subject[:label]).to eq(status.label)
+ expect(subject[:label]).to eq('passed')
+ expect(subject[:tooltip]).to eq('passed')
expect(subject[:icon]).to eq(status.icon)
expect(subject[:favicon]).to match_asset_path("/assets/ci_favicons/#{status.favicon}.ico")
end
diff --git a/spec/serializers/job_entity_spec.rb b/spec/serializers/job_entity_spec.rb
index 026360e91a3..24a6f1a2a8a 100644
--- a/spec/serializers/job_entity_spec.rb
+++ b/spec/serializers/job_entity_spec.rb
@@ -38,7 +38,7 @@ describe JobEntity do
it 'contains details' do
expect(subject).to include :status
- expect(subject[:status]).to include :icon, :favicon, :text, :label
+ expect(subject[:status]).to include :icon, :favicon, :text, :label, :tooltip
end
context 'when job is retryable' do
@@ -126,7 +126,29 @@ describe JobEntity do
it 'contains details' do
expect(subject).to include :status
- expect(subject[:status]).to include :icon, :favicon, :text, :label
+ expect(subject[:status]).to include :icon, :favicon, :text, :label, :tooltip
+ end
+ end
+
+ context 'when job failed' do
+ let(:job) { create(:ci_build, :script_failure) }
+
+ describe 'status' do
+ it 'should contain the failure reason inside label' do
+ expect(subject[:status]).to include :icon, :favicon, :text, :label, :tooltip
+ expect(subject[:status][:label]).to eq('failed')
+ expect(subject[:status][:tooltip]).to eq('failed <br> (script failure)')
+ end
+ end
+ end
+
+ context 'when job passed' do
+ let(:job) { create(:ci_build, :success) }
+
+ describe 'status' do
+ it 'should not contain the failure reason inside label' do
+ expect(subject[:status][:label]).to eq('passed')
+ end
end
end
end
diff --git a/spec/serializers/pipeline_entity_spec.rb b/spec/serializers/pipeline_entity_spec.rb
index 248552d1858..2473c561f4b 100644
--- a/spec/serializers/pipeline_entity_spec.rb
+++ b/spec/serializers/pipeline_entity_spec.rb
@@ -30,7 +30,7 @@ describe PipelineEntity do
expect(subject).to include :details
expect(subject[:details])
.to include :duration, :finished_at
- expect(subject[:details][:status]).to include :icon, :favicon, :text, :label
+ expect(subject[:details][:status]).to include :icon, :favicon, :text, :label, :tooltip
end
it 'contains flags' do
diff --git a/spec/serializers/stage_entity_spec.rb b/spec/serializers/stage_entity_spec.rb
index 40e303f7b89..2034c7891ef 100644
--- a/spec/serializers/stage_entity_spec.rb
+++ b/spec/serializers/stage_entity_spec.rb
@@ -26,7 +26,7 @@ describe StageEntity do
end
it 'contains detailed status' do
- expect(subject[:status]).to include :text, :label, :group, :icon
+ expect(subject[:status]).to include :text, :label, :group, :icon, :tooltip
expect(subject[:status][:label]).to eq 'passed'
end
diff --git a/spec/serializers/status_entity_spec.rb b/spec/serializers/status_entity_spec.rb
index 70402bac2e2..559475e571c 100644
--- a/spec/serializers/status_entity_spec.rb
+++ b/spec/serializers/status_entity_spec.rb
@@ -16,7 +16,7 @@ describe StatusEntity do
subject { entity.as_json }
it 'contains status details' do
- expect(subject).to include :text, :icon, :favicon, :label, :group
+ expect(subject).to include :text, :icon, :favicon, :label, :group, :tooltip
expect(subject).to include :has_details, :details_path
expect(subject[:favicon]).to match_asset_path('/assets/ci_favicons/favicon_status_success.ico')
end
diff --git a/spec/views/projects/jobs/show.html.haml_spec.rb b/spec/views/projects/jobs/show.html.haml_spec.rb
index 6a67da79ec5..9e692159bd0 100644
--- a/spec/views/projects/jobs/show.html.haml_spec.rb
+++ b/spec/views/projects/jobs/show.html.haml_spec.rb
@@ -1,8 +1,10 @@
require 'spec_helper'
describe 'projects/jobs/show' do
+ let(:user) { create(:user) }
let(:project) { create(:project, :repository) }
let(:build) { create(:ci_build, pipeline: pipeline) }
+ let(:builds) { project.builds.present(current_user: user) }
let(:pipeline) do
create(:ci_pipeline, project: project, sha: project.commit.id)
@@ -11,6 +13,7 @@ describe 'projects/jobs/show' do
before do
assign(:build, build.present)
assign(:project, project)
+ assign(:builds, builds)
allow(view).to receive(:can?).and_return(true)
end