Commit 7403df6c authored by Stan Hu's avatar Stan Hu

Merge branch 'suppress-allow-failure-builds' into 'master'

Suppress e-mails on failed builds if allow_failure is set

Every time I push to GitLab, I get > 2 emails saying a spec failed when I don't care about the benchmarks and others that have `allow_failure` set to `true`.

@ayufan mentioned creating a summary e-mail to prevent getting one e-mail per build, but the latter might actually be desirable. For example, I do want to know if Rubocop errors fail right away.

See merge request !2178
parents 8429f6f4 69209612
...@@ -32,6 +32,7 @@ v 8.4.0 (unreleased) ...@@ -32,6 +32,7 @@ v 8.4.0 (unreleased)
v 8.3.3 (unreleased) v 8.3.3 (unreleased)
- Get "Merge when build succeeds" to work when commits were pushed to MR target branch while builds were running - Get "Merge when build succeeds" to work when commits were pushed to MR target branch while builds were running
- Suppress e-mails on failed builds if allow_failure is set (Stan Hu)
- Fix project transfer e-mail sending incorrect paths in e-mail notification (Stan Hu) - Fix project transfer e-mail sending incorrect paths in e-mail notification (Stan Hu)
- Enable "Add key" button when user fills in a proper key (Stan Hu) - Enable "Add key" button when user fills in a proper key (Stan Hu)
......
...@@ -73,12 +73,16 @@ class BuildsEmailService < Service ...@@ -73,12 +73,16 @@ class BuildsEmailService < Service
when 'success' when 'success'
!notify_only_broken_builds? !notify_only_broken_builds?
when 'failed' when 'failed'
true !allow_failure?(data)
else else
false false
end end
end end
def allow_failure?(data)
data[:build_allow_failure] == true
end
def all_recipients(data) def all_recipients(data)
all_recipients = recipients.split(',') all_recipients = recipients.split(',')
......
...@@ -23,6 +23,7 @@ module Gitlab ...@@ -23,6 +23,7 @@ module Gitlab
build_started_at: build.started_at, build_started_at: build.started_at,
build_finished_at: build.finished_at, build_finished_at: build.finished_at,
build_duration: build.duration, build_duration: build.duration,
build_allow_failure: build.allow_failure,
# TODO: do we still need it? # TODO: do we still need it?
project_id: project.id, project_id: project.id,
......
...@@ -14,6 +14,7 @@ describe 'Gitlab::BuildDataBuilder' do ...@@ -14,6 +14,7 @@ describe 'Gitlab::BuildDataBuilder' do
it { expect(data[:tag]).to eq(build.tag) } it { expect(data[:tag]).to eq(build.tag) }
it { expect(data[:build_id]).to eq(build.id) } it { expect(data[:build_id]).to eq(build.id) }
it { expect(data[:build_status]).to eq(build.status) } it { expect(data[:build_status]).to eq(build.status) }
it { expect(data[:build_allow_failure]).to eq(false) }
it { expect(data[:project_id]).to eq(build.project.id) } it { expect(data[:project_id]).to eq(build.project.id) }
it { expect(data[:project_name]).to eq(build.project.name_with_namespace) } it { expect(data[:project_name]).to eq(build.project.name_with_namespace) }
end end
......
require 'spec_helper'
describe BuildsEmailService do
let(:build) { create(:ci_build) }
let(:data) { Gitlab::BuildDataBuilder.build(build) }
let(:service) { BuildsEmailService.new }
describe :execute do
it "sends email" do
service.recipients = 'test@gitlab.com'
data[:build_status] = 'failed'
expect(BuildEmailWorker).to receive(:perform_async)
service.execute(data)
end
it "does not sends email with failed build and allowed_failure on" do
data[:build_status] = 'failed'
data[:build_allow_failure] = true
expect(BuildEmailWorker).not_to receive(:perform_async)
service.execute(data)
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