Skip to content

Commit 5eb307a

Browse files
committed
Merge branch 'cs-warn-on-failure' into 'master'
Warn on failure ## What does this MR do? Adds styling and HTML for a `success_with_warnings` status in the MR widget. Unfinished as it needs some plumbing in the relevant CI controllers and models. ## Are there points in the code the reviewer needs to double check? Not at the moment, though I think I'll probably need help getting this to work in the backend. ## What are the relevant issue numbers? #17669 ## Screenshots (if relevant) ![Screen_Shot_2016-06-29_at_12.02.49_PM](/uploads/af4a915689633fe028f44bb34ae7a5b1/Screen_Shot_2016-06-29_at_12.02.49_PM.png) ## Does this MR meet the acceptance criteria? - [ ] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added - [ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md) - [ ] API support added - Tests - [ ] Added for this feature/bug - [ ] All builds are passing - [ ] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides) - [ ] Branch has no merge conflicts with `master` (if you do - rebase it please) - [ ] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits) cc: @ayufan See merge request !5004
2 parents f3ea58e + 2016f36 commit 5eb307a

File tree

9 files changed

+73
-10
lines changed

9 files changed

+73
-10
lines changed

CHANGELOG

+1
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ v 8.10.0 (unreleased)
2727
- Store when and yaml variables in builds table
2828
- Display last commit of deleted branch in push events !4699 (winniehell)
2929
- Escape file extension when parsing search results !5141 (winniehell)
30+
- Add "passing with warnings" to the merge request pipeline possible statuses, this happens when builds that allow failures have failed. !5004
3031
- Apply the trusted_proxies config to the rack request object for use with rack_attack
3132
- Upgrade to Rails 4.2.7. !5236
3233
- Extend exposed environment variables for CI builds

app/assets/javascripts/merge_request_widget.js.coffee

+9-6
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,13 @@ class @MergeRequestWidget
5555
$('.mr-state-widget').replaceWith(data)
5656

5757
ciLabelForStatus: (status) ->
58-
if status is 'success'
59-
'passed'
60-
else
61-
status
58+
switch status
59+
when 'success'
60+
'passed'
61+
when 'success_with_warnings'
62+
'passed with warnings'
63+
else
64+
status
6265

6366
pollCIStatus: ->
6467
@fetchBuildStatusInterval = setInterval ( =>
@@ -116,15 +119,15 @@ class @MergeRequestWidget
116119
showCIStatus: (state) ->
117120
return if not state?
118121
$('.ci_widget').hide()
119-
allowed_states = ["failed", "canceled", "running", "pending", "success", "skipped", "not_found"]
122+
allowed_states = ["failed", "canceled", "running", "pending", "success", "success_with_warnings", "skipped", "not_found"]
120123
if state in allowed_states
121124
$('.ci_widget.ci-' + state).show()
122125
switch state
123126
when "failed", "canceled", "not_found"
124127
@setMergeButtonClass('btn-danger')
125128
when "running"
126129
@setMergeButtonClass('btn-warning')
127-
when "success"
130+
when "success", "success_with_warnings"
128131
@setMergeButtonClass('btn-create')
129132
else
130133
$('.ci_widget.ci-error').show()

app/assets/stylesheets/pages/merge_requests.scss

+8
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,14 @@
7070
color: $gl-success;
7171
}
7272

73+
&.ci-success_with_warnings {
74+
color: $gl-success;
75+
76+
i {
77+
color: $gl-warning;
78+
}
79+
}
80+
7381
&.ci-skipped {
7482
background-color: #eee;
7583
color: #888;

app/assets/stylesheets/pages/status.scss

+6-2
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@
1515
border-color: $gl-danger;
1616
}
1717

18-
&.ci-success {
18+
&.ci-success,
19+
&.ci-success_with_warnings {
1920
color: $gl-success;
2021
border-color: $gl-success;
2122
}
@@ -57,9 +58,12 @@
5758
.ci-status-icon-failed {
5859
color: $gl-danger;
5960
}
60-
.ci-status-icon-pending {
61+
62+
.ci-status-icon-pending,
63+
.ci-status-icon-success_with_warning {
6164
color: $gl-warning;
6265
}
66+
6367
.ci-status-icon-running {
6468
color: $blue-normal;
6569
}

app/controllers/projects/merge_requests_controller.rb

+2
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,8 @@ def ci_status
286286
status = pipeline.status
287287
coverage = pipeline.try(:coverage)
288288

289+
status = "success_with_warnings" if pipeline.success? && pipeline.has_warnings?
290+
289291
status ||= "preparing"
290292
else
291293
ci_service = @merge_request.source_project.ci_service

app/helpers/ci_status_helper.rb

+4-1
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,11 @@ def ci_status_with_icon(status, target = nil)
1515
end
1616

1717
def ci_label_for_status(status)
18-
if status == 'success'
18+
case status
19+
when 'success'
1920
'passed'
21+
when 'success_with_warnings'
22+
'passed with warnings'
2023
else
2124
status
2225
end

app/models/ci/pipeline.rb

+4
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,10 @@ def coverage
146146
end
147147
end
148148

149+
def has_warnings?
150+
builds.latest.ignored.any?
151+
end
152+
149153
def config_processor
150154
return nil unless ci_yaml_file
151155
return @config_processor if defined?(@config_processor)

app/views/projects/merge_requests/widget/_heading.html.haml

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
- if @pipeline
22
.mr-widget-heading
3-
- %w[success skipped canceled failed running pending].each do |status|
3+
- %w[success success_with_warnings skipped canceled failed running pending].each do |status|
44
.ci_widget{ class: "ci-#{status}", style: ("display:none" unless @pipeline.status == status) }
55
= ci_icon_for_status(status)
66
%span

spec/models/ci/pipeline_spec.rb

+38
Original file line numberDiff line numberDiff line change
@@ -504,4 +504,42 @@ def manual_actions
504504
end
505505
end
506506
end
507+
508+
describe '#has_warnings?' do
509+
subject { pipeline.has_warnings? }
510+
511+
context 'build which is allowed to fail fails' do
512+
before do
513+
create :ci_build, :success, pipeline: pipeline, name: 'rspec'
514+
create :ci_build, :allowed_to_fail, :failed, pipeline: pipeline, name: 'rubocop'
515+
end
516+
517+
it 'returns true' do
518+
is_expected.to be_truthy
519+
end
520+
end
521+
522+
context 'build which is allowed to fail succeeds' do
523+
before do
524+
create :ci_build, :success, pipeline: pipeline, name: 'rspec'
525+
create :ci_build, :allowed_to_fail, :success, pipeline: pipeline, name: 'rubocop'
526+
end
527+
528+
it 'returns false' do
529+
is_expected.to be_falsey
530+
end
531+
end
532+
533+
context 'build is retried and succeeds' do
534+
before do
535+
create :ci_build, :success, pipeline: pipeline, name: 'rubocop'
536+
create :ci_build, :failed, pipeline: pipeline, name: 'rspec'
537+
create :ci_build, :success, pipeline: pipeline, name: 'rspec'
538+
end
539+
540+
it 'returns false' do
541+
is_expected.to be_falsey
542+
end
543+
end
544+
end
507545
end

0 commit comments

Comments
 (0)