Commit 77a43827 authored by Kamil Trzciński's avatar Kamil Trzciński

Merge branch 'fix/do-not-add-todo-when-build-allowed-to-fail' into 'master'

Do not create TODO when build is allowed to fail

## What does this MR do?

Do not create a TODO when build that failed is allowed to fail.

## Does this MR meet the acceptance criteria?

- [x] [Changelog entry](https://docs.gitlab.com/ce/development/changelog.html) added
- [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- Tests
  - [x] Added for this feature/bug
  - [ ] All builds are passing

## What are the relevant issue numbers?

Closes #22280

See merge request !7618
parents 671c6d7d d840c462
...@@ -70,7 +70,11 @@ module Ci ...@@ -70,7 +70,11 @@ module Ci
environment: build.environment, environment: build.environment,
status_event: 'enqueue' status_event: 'enqueue'
) )
MergeRequests::AddTodoWhenBuildFailsService.new(build.project, nil).close(new_build)
MergeRequests::AddTodoWhenBuildFailsService
.new(build.project, nil)
.close(new_build)
build.pipeline.mark_as_processable_after_stage(build.stage_idx) build.pipeline.mark_as_processable_after_stage(build.stage_idx)
new_build new_build
end end
......
module MergeRequests module MergeRequests
class AddTodoWhenBuildFailsService < MergeRequests::BaseService class AddTodoWhenBuildFailsService < MergeRequests::BaseService
# Adds a todo to the parent merge_request when a CI build fails # Adds a todo to the parent merge_request when a CI build fails
#
def execute(commit_status) def execute(commit_status)
return if commit_status.allow_failure?
commit_status_merge_requests(commit_status) do |merge_request| commit_status_merge_requests(commit_status) do |merge_request|
todo_service.merge_request_build_failed(merge_request) todo_service.merge_request_build_failed(merge_request)
end end
end end
# Closes any pending build failed todos for the parent MRs when a build is retried # Closes any pending build failed todos for the parent MRs when a
# build is retried
#
def close(commit_status) def close(commit_status)
commit_status_merge_requests(commit_status) do |merge_request| commit_status_merge_requests(commit_status) do |merge_request|
todo_service.merge_request_build_retried(merge_request) todo_service.merge_request_build_retried(merge_request)
......
---
title: Do not create a new TODO when failed build is allowed to fail
merge_request: 7618
author:
...@@ -24,9 +24,11 @@ you still have open. ...@@ -24,9 +24,11 @@ you still have open.
A Todo appears in your Todos dashboard when: A Todo appears in your Todos dashboard when:
- an issue or merge request is assigned to you - an issue or merge request is assigned to you,
- you are `@mentioned` in an issue or merge request, be it the description of - you are `@mentioned` in an issue or merge request, be it the description of
the issue/merge request or in a comment the issue/merge request or in a comment,
- build in the CI pipeline running for your merge request failed, but this
build is not allowed to fail.
>**Note:** Commenting on a commit will _not_ trigger a Todo. >**Note:** Commenting on a commit will _not_ trigger a Todo.
......
require 'spec_helper' require 'spec_helper'
# Write specs in this file.
describe MergeRequests::AddTodoWhenBuildFailsService do describe MergeRequests::AddTodoWhenBuildFailsService do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:merge_request) { create(:merge_request) } let(:merge_request) { create(:merge_request) }
let(:project) { create(:project) } let(:project) { create(:project) }
let(:sha) { '1234567890abcdef1234567890abcdef12345678' } let(:sha) { '1234567890abcdef1234567890abcdef12345678' }
let(:pipeline) { create(:ci_pipeline_with_one_job, ref: merge_request.source_branch, project: project, sha: sha) } let(:ref) { merge_request.source_branch }
let(:service) { MergeRequests::AddTodoWhenBuildFailsService.new(project, user, commit_message: 'Awesome message') }
let(:pipeline) do
create(:ci_pipeline_with_one_job, ref: ref,
project: project,
sha: sha)
end
let(:service) do
described_class.new(project, user, commit_message: 'Awesome message')
end
let(:todo_service) { TodoService.new } let(:todo_service) { TodoService.new }
let(:merge_request) do let(:merge_request) do
...@@ -23,7 +32,9 @@ describe MergeRequests::AddTodoWhenBuildFailsService do ...@@ -23,7 +32,9 @@ describe MergeRequests::AddTodoWhenBuildFailsService do
describe '#execute' do describe '#execute' do
context 'commit status with ref' do context 'commit status with ref' do
let(:commit_status) { create(:generic_commit_status, ref: merge_request.source_branch, pipeline: pipeline) } let(:commit_status) do
create(:generic_commit_status, ref: ref, pipeline: pipeline)
end
it 'notifies the todo service' do it 'notifies the todo service' do
expect(todo_service).to receive(:merge_request_build_failed).with(merge_request) expect(todo_service).to receive(:merge_request_build_failed).with(merge_request)
...@@ -32,7 +43,7 @@ describe MergeRequests::AddTodoWhenBuildFailsService do ...@@ -32,7 +43,7 @@ describe MergeRequests::AddTodoWhenBuildFailsService do
end end
context 'commit status with non-HEAD ref' do context 'commit status with non-HEAD ref' do
let(:commit_status) { create(:generic_commit_status, ref: merge_request.source_branch) } let(:commit_status) { create(:generic_commit_status, ref: ref) }
it 'does not notify the todo service' do it 'does not notify the todo service' do
expect(todo_service).not_to receive(:merge_request_build_failed) expect(todo_service).not_to receive(:merge_request_build_failed)
...@@ -48,6 +59,18 @@ describe MergeRequests::AddTodoWhenBuildFailsService do ...@@ -48,6 +59,18 @@ describe MergeRequests::AddTodoWhenBuildFailsService do
service.execute(commit_status) service.execute(commit_status)
end end
end end
context 'when commit status is a build allowed to fail' do
let(:commit_status) do
create(:ci_build, :allowed_to_fail, ref: ref, pipeline: pipeline)
end
it 'does not create todo' do
expect(todo_service).not_to receive(:merge_request_build_failed)
service.execute(commit_status)
end
end
end end
describe '#close' do describe '#close' do
......
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