Commit f73def90 authored by Rémy Coutable's avatar Rémy Coutable

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
parents fa7a682b b8769936
...@@ -8,6 +8,7 @@ v 8.8.0 (unreleased) ...@@ -8,6 +8,7 @@ v 8.8.0 (unreleased)
- Toggle sign-up confirmation emails in application settings - Toggle sign-up confirmation emails in application settings
- Project#open_branches has been cleaned up and no longer loads entire records into memory. - Project#open_branches has been cleaned up and no longer loads entire records into memory.
- Escape HTML in commit titles in system note messages - 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 - Improve multiple branch push performance by memoizing permission checking
- Log to application.log when an admin starts and stops impersonating a user - 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) - Changing the confidentiality of an issue now creates a new system note (Alex Moore-Niemi)
......
...@@ -18,19 +18,16 @@ class CreateCommitBuildsService ...@@ -18,19 +18,16 @@ class CreateCommitBuildsService
return false return false
end end
commit = project.ci_commit(sha, ref) commit = Ci::Commit.new(project: project, sha: sha, ref: ref, before_sha: before_sha, tag: tag)
unless commit
commit = project.ci_commits.new(sha: sha, ref: ref, before_sha: before_sha, tag: tag)
# Skip creating ci_commit when no gitlab-ci.yml is found # Skip creating ci_commit when no gitlab-ci.yml is found
unless commit.ci_yaml_file unless commit.ci_yaml_file
return false return false
end
# Create a new ci_commit
commit.save!
end end
# Create a new ci_commit
commit.save!
# Skip creating builds for commits that have [ci skip] # Skip creating builds for commits that have [ci skip]
unless commit.skip_ci? unless commit.skip_ci?
# Create builds for commit # Create builds for commit
......
...@@ -48,6 +48,22 @@ describe PostReceive do ...@@ -48,6 +48,22 @@ describe PostReceive do
PostReceive.new.perform(pwd(project), key_id, base64_changes) PostReceive.new.perform(pwd(project), key_id, base64_changes)
end end
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 end
context "webhook" do context "webhook" 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