diff options
author | Rémy Coutable <remy@rymai.me> | 2016-05-19 21:42:24 +0000 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2016-05-19 21:42:24 +0000 |
commit | f73def90a83e7cddda0960b2c780c59835c228fb (patch) | |
tree | 691d299c67ded801d2fd950937561c6756c33092 | |
parent | fa7a682bb05080855c8de29535e9626dde63c145 (diff) | |
parent | b876993677edde4fee3a8ed349c522f1542190f7 (diff) | |
download | gitlab-ce-f73def90a83e7cddda0960b2c780c59835c228fb.tar.gz |
Merge branch 'fix-ci-commit-creation' into 'master'
Fix creation of Ci::Commit object which can lead to pending, failed in some scenarios
## What does this MR do?
If we use a new `project.ci_commits` it will add it to array, and some other services which can do a save on a project can lead to a scenario when `ci_commit` will be saved, where it should not be.
## What are the relevant issue numbers?
https://gitlab.com/gitlab-com/support-forum/issues/717 https://gitlab.com/gitlab-com/support-forum/issues/715 https://gitlab.com/gitlab-org/gitlab-ce/issues/17596 https://gitlab.com/gitlab-com/support-forum/issues/714 https://gitlab.com/gitlab-org/gitlab-ce/issues/13402
cc @rymai
See merge request !4214
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | app/services/create_commit_builds_service.rb | 17 | ||||
-rw-r--r-- | spec/workers/post_receive_spec.rb | 16 |
3 files changed, 24 insertions, 10 deletions
diff --git a/CHANGELOG b/CHANGELOG index 422718dec9a..f07a4f65fd3 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -8,6 +8,7 @@ v 8.8.0 (unreleased) - Toggle sign-up confirmation emails in application settings - Project#open_branches has been cleaned up and no longer loads entire records into memory. - Escape HTML in commit titles in system note messages + - Fix creation of Ci::Commit object which can lead to pending, failed in some scenarios - Improve multiple branch push performance by memoizing permission checking - Log to application.log when an admin starts and stops impersonating a user - Changing the confidentiality of an issue now creates a new system note (Alex Moore-Niemi) diff --git a/app/services/create_commit_builds_service.rb b/app/services/create_commit_builds_service.rb index 0d2aa1ff03d..5b6fefe669e 100644 --- a/app/services/create_commit_builds_service.rb +++ b/app/services/create_commit_builds_service.rb @@ -18,19 +18,16 @@ class CreateCommitBuildsService return false end - commit = project.ci_commit(sha, ref) - unless commit - commit = project.ci_commits.new(sha: sha, ref: ref, before_sha: before_sha, tag: tag) + commit = Ci::Commit.new(project: project, sha: sha, ref: ref, before_sha: before_sha, tag: tag) - # Skip creating ci_commit when no gitlab-ci.yml is found - unless commit.ci_yaml_file - return false - end - - # Create a new ci_commit - commit.save! + # Skip creating ci_commit when no gitlab-ci.yml is found + unless commit.ci_yaml_file + return false end + # Create a new ci_commit + commit.save! + # Skip creating builds for commits that have [ci skip] unless commit.skip_ci? # Create builds for commit diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb index 94ff3457902..2f465bcf1e3 100644 --- a/spec/workers/post_receive_spec.rb +++ b/spec/workers/post_receive_spec.rb @@ -48,6 +48,22 @@ describe PostReceive do PostReceive.new.perform(pwd(project), key_id, base64_changes) end end + + context "gitlab-ci.yml" do + subject { PostReceive.new.perform(pwd(project), key_id, base64_changes) } + + context "creates a Ci::Commit for every change" do + before { stub_ci_commit_to_return_yaml_file } + + it { expect{ subject }.to change{ Ci::Commit.count }.by(2) } + end + + context "does not create a Ci::Commit" do + before { stub_ci_commit_yaml_file(nil) } + + it { expect{ subject }.to_not change{ Ci::Commit.count } } + end + end end context "webhook" do |