Skip to content

G
gitlab-ce
Commit c54e08cf authored by Tiago Botelho's avatar Tiago Botelho
Browse files
Options

refactors git push service spec code

parent be0f6c42
Hide whitespace changes
Inline Side-by-side
Showing with 123 additions and 100 deletions
app/models/remote_mirror.rb
View file @ c54e08cf
......@@ -76,7 +76,7 @@ class RemoteMirror < ActiveRecord::Base
def sync
return unless project && enabled
schedule_update_job
RepositoryUpdateRemoteMirrorWorker.perform_in(BACKOFF_DELAY, self.id, Time.now) if project&.repository_exists?
end
def updated_since?(timestamp)
......@@ -133,10 +133,6 @@ class RemoteMirror < ActiveRecord::Base
)
end
def schedule_update_job
RepositoryUpdateRemoteMirrorWorker.perform_in(BACKOFF_DELAY, self.id, Time.now) if project&.repository_exists?
end
def refresh_remote
return unless project
......
app/workers/repository_update_remote_mirror_worker.rb
View file @ c54e08cf
......@@ -5,7 +5,7 @@ class RepositoryUpdateRemoteMirrorWorker
include Sidekiq::Worker
include Gitlab::ShellAdapter
sidekiq_options queue: :project_mirror, retry: 3
sidekiq_options queue: :project_mirror, retry: 3, dead: false
sidekiq_retry_in { |count| 30 * count }
......
doc/workflow/repository_mirroring.md
View file @ c54e08cf
......@@ -72,7 +72,7 @@ repository to push to. Hit **Save changes** for the changes to take effect.
Similarly to the pull mirroring, since the upstream repository functions as a
mirror to the repository in GitLab, you are advised not to push commits directly
to the mirrored repository. Instead, all changes will end up in the mirrored repository
whenever commits are be pushed to GitLab, or when a [forced update](#forcing-an-update) is initiated.
whenever commits are pushed to GitLab, or when a [forced update](#forcing-an-update) is initiated.
Pushes into GitLab are automatically pushed to the remote mirror 5 minutes after they come in.
......
spec/models/remote_mirror_spec.rb
View file @ c54e08cf
......@@ -85,6 +85,38 @@ describe RemoteMirror do
end
end
context '#sync' do
let(:remote_mirror) { create(:project, :remote_mirror).remote_mirrors.first }
before do
Timecop.freeze(Time.now)
end
context 'with remote mirroring enabled' do
it 'schedules a RepositoryUpdateRemoteMirrorWorker to run within a certain backoff delay' do
expect(RepositoryUpdateRemoteMirrorWorker).to receive(:perform_in).with(RemoteMirror::BACKOFF_DELAY, remote_mirror.id, Time.now)
remote_mirror.sync
end
end
context 'with remote mirroring disabled' do
it 'returns nil' do
remote_mirror.update_attributes(enabled: false)
expect(remote_mirror.sync).to be_nil
end
end
context 'without project' do
it 'returns nil' do
allow_any_instance_of(RemoteMirror).to receive(:project).and_return(nil)
expect(remote_mirror.sync).to be_nil
end
end
end
context '#updated_since?' do
let(:remote_mirror) { create(:project, :remote_mirror).remote_mirrors.first }
let(:timestamp) { Time.now - 5.minutes }
......
spec/services/git_push_service_spec.rb
View file @ c54e08cf
......@@ -3,22 +3,19 @@ require 'spec_helper'
describe GitPushService, services: true do
include RepoHelpers
let(:user) { create(:user) }
let(:project) { create(:project, :repository) }
let(:user) { create(:user) }
let(:project) { create(:project, :repository) }
let(:blankrev) { Gitlab::Git::BLANK_SHA }
let(:oldrev) { sample_commit.parent_id }
let(:newrev) { sample_commit.id }
let(:ref) { 'refs/heads/master' }
before do
project.team << [user, :master]
@blankrev = Gitlab::Git::BLANK_SHA
@oldrev = sample_commit.parent_id
@newrev = sample_commit.id
@ref = 'refs/heads/master'
end
describe 'with remote mirrors' do
let(:project) { create(:project, :remote_mirror) }
let(:oldrev) { @oldrev }
let(:newrev) { @newrev }
let(:ref) { @ref }
subject do
described_class.new(project, user, oldrev: oldrev, newrev: newrev, ref: ref)
......@@ -39,15 +36,12 @@ describe GitPushService, services: true do
end
describe 'Push branches' do
let(:oldrev) { @oldrev }
let(:newrev) { @newrev }
subject do
execute_service(project, user, oldrev, newrev, @ref)
execute_service(project, user, oldrev, newrev, ref)
end
context 'new branch' do
let(:oldrev) { @blankrev }
let(:oldrev) { blankrev }
it { is_expected.to be_truthy }
......@@ -75,7 +69,7 @@ describe GitPushService, services: true do
end
context 'rm branch' do
let(:newrev) { @blankrev }
let(:newrev) { blankrev }
it { is_expected.to be_truthy }
......@@ -94,24 +88,20 @@ describe GitPushService, services: true do
end
describe "Git Push Data" do
before do
service = execute_service(project, user, @oldrev, @newrev, @ref )
@push_data = service.push_data
@commit = project.commit(@newrev)
end
let(:commit) { project.commit(newrev) }
subject { @push_data }
subject { push_data_from_service(project, user, oldrev, newrev, ref) }
it { is_expected.to include(object_kind: 'push') }
it { is_expected.to include(before: @oldrev) }
it { is_expected.to include(after: @newrev) }
it { is_expected.to include(ref: @ref) }
it { is_expected.to include(before: oldrev) }
it { is_expected.to include(after: newrev) }
it { is_expected.to include(ref: ref) }
it { is_expected.to include(user_id: user.id) }
it { is_expected.to include(user_name: user.name) }
it { is_expected.to include(project_id: project.id) }
context "with repository data" do
subject { @push_data[:repository] }
subject { push_data_from_service(project, user, oldrev, newrev, ref)[:repository] }
it { is_expected.to include(name: project.name) }
it { is_expected.to include(url: project.url_to_repo) }
......@@ -120,7 +110,7 @@ describe GitPushService, services: true do
end
context "with commits" do
subject { @push_data[:commits] }
subject { push_data_from_service(project, user, oldrev, newrev, ref)[:commits] }
it { is_expected.to be_an(Array) }
it 'has 1 element' do
......@@ -128,11 +118,11 @@ describe GitPushService, services: true do
end
context "the commit" do
subject { @push_data[:commits].first }
subject { push_data_from_service(project, user, oldrev, newrev, ref)[:commits].first }
it { is_expected.to include(id: @commit.id) }
it { is_expected.to include(message: @commit.safe_message) }
it { is_expected.to include(timestamp: @commit.date.xmlschema) }
it { is_expected.to include(id: commit.id) }
it { is_expected.to include(message: commit.safe_message) }
it { is_expected.to include(timestamp: commit.date.xmlschema) }
it do
is_expected.to include(
url: [
......@@ -140,16 +130,16 @@ describe GitPushService, services: true do
project.namespace.to_param,
project.to_param,
'commit',
@commit.id
commit.id
].join('/')
)
end
context "with a author" do
subject { @push_data[:commits].first[:author] }
subject { push_data_from_service(project, user, oldrev, newrev, ref)[:commits].first[:author] }
it { is_expected.to include(name: @commit.author_name) }
it { is_expected.to include(email: @commit.author_email) }
it { is_expected.to include(name: commit.author_name) }
it { is_expected.to include(email: commit.author_email) }
end
end
end
......@@ -167,40 +157,37 @@ describe GitPushService, services: true do
it "does not trigger indexer when push to non-default branch" do
expect_any_instance_of(Gitlab::Elastic::Indexer).not_to receive(:run)
execute_service(project, user, @oldrev, @newrev, 'refs/heads/other')
execute_service(project, user, oldrev, newrev, 'refs/heads/other')
end
it "triggers indexer when push to default branch" do
expect_any_instance_of(Gitlab::Elastic::Indexer).to receive(:run)
execute_service(project, user, @oldrev, @newrev, 'refs/heads/master')
execute_service(project, user, oldrev, newrev, ref)
end
end
describe "Push Event" do
before do
service = execute_service(project, user, @oldrev, @newrev, @ref )
@event = Event.find_by_action(Event::PUSHED)
@push_data = service.push_data
end
let!(:push_data) { push_data_from_service(project, user, oldrev, newrev, ref) }
let(:event) { Event.find_by_action(Event::PUSHED) }
it { expect(@event).not_to be_nil }
it { expect(@event.project).to eq(project) }
it { expect(@event.action).to eq(Event::PUSHED) }
it { expect(@event.data).to eq(@push_data) }
it { expect(event).not_to be_nil }
it { expect(event.project).to eq(project) }
it { expect(event.action).to eq(Event::PUSHED) }
it { expect(event.data).to eq(push_data) }
context "Updates merge requests" do
it "when pushing a new branch for the first time" do
expect(UpdateMergeRequestsWorker).to receive(:perform_async).
with(project.id, user.id, @blankrev, 'newrev', 'refs/heads/master')
execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' )
with(project.id, user.id, blankrev, 'newrev', ref)
execute_service(project, user, blankrev, 'newrev', ref)
end
end
context "Sends System Push data" do
it "when pushing on a branch" do
expect(SystemHookPushWorker).to receive(:perform_async).with(@push_data, :push_hooks)
execute_service(project, user, @oldrev, @newrev, @ref )
expect(SystemHookPushWorker).to receive(:perform_async).with(push_data, :push_hooks)
execute_service(project, user, oldrev, newrev, ref)
end
end
end
......@@ -210,13 +197,13 @@ describe GitPushService, services: true do
it "calls the copy attributes method for the first push to the default branch" do
expect(project.repository).to receive(:copy_gitattributes).with('master')
execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master')
execute_service(project, user, blankrev, 'newrev', ref)
end
it "calls the copy attributes method for changes to the default branch" do
expect(project.repository).to receive(:copy_gitattributes).with('refs/heads/master')
expect(project.repository).to receive(:copy_gitattributes).with(ref)
execute_service(project, user, 'oldrev', 'newrev', 'refs/heads/master')
execute_service(project, user, 'oldrev', 'newrev', ref)
end
end
......@@ -229,7 +216,7 @@ describe GitPushService, services: true do
it "does not call copy attributes method" do
expect(project.repository).not_to receive(:copy_gitattributes)
execute_service(project, user, @oldrev, @newrev, @ref)
execute_service(project, user, oldrev, newrev, ref)
end
end
end
......@@ -239,7 +226,7 @@ describe GitPushService, services: true do
it "when pushing a branch for the first time" do
expect(project).to receive(:execute_hooks)
expect(project.default_branch).to eq("master")
execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' )
execute_service(project, user, blankrev, 'newrev', ref)
expect(project.protected_branches).not_to be_empty
expect(project.protected_branches.first.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::MASTER])
expect(project.protected_branches.first.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::MASTER])
......@@ -250,7 +237,7 @@ describe GitPushService, services: true do
expect(project).to receive(:execute_hooks)
expect(project.default_branch).to eq("master")
execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' )
execute_service(project, user, blankrev, 'newrev', ref)
expect(project.protected_branches).to be_empty
end
......@@ -260,7 +247,7 @@ describe GitPushService, services: true do
expect(project).to receive(:execute_hooks)
expect(project.default_branch).to eq("master")
execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' )
execute_service(project, user, blankrev, 'newrev', ref)
expect(project.protected_branches).not_to be_empty
expect(project.protected_branches.last.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER])
......@@ -277,7 +264,7 @@ describe GitPushService, services: true do
expect(project.default_branch).to eq("master")
expect_any_instance_of(ProtectedBranches::CreateService).not_to receive(:execute)
execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' )
execute_service(project, user, blankrev, 'newrev', ref)
expect(project.protected_branches).not_to be_empty
expect(project.protected_branches.last.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::NO_ACCESS])
......@@ -289,7 +276,7 @@ describe GitPushService, services: true do
expect(project).to receive(:execute_hooks)
expect(project.default_branch).to eq("master")
execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' )
execute_service(project, user, blankrev, 'newrev', ref)
expect(project.protected_branches).not_to be_empty
expect(project.protected_branches.first.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::MASTER])
expect(project.protected_branches.first.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER])
......@@ -297,7 +284,7 @@ describe GitPushService, services: true do
it "when pushing new commits to existing branch" do
expect(project).to receive(:execute_hooks)
execute_service(project, user, 'oldrev', 'newrev', 'refs/heads/master' )
execute_service(project, user, 'oldrev', 'newrev', ref)
end
end
end
......@@ -327,7 +314,7 @@ describe GitPushService, services: true do
it "creates a note if a pushed commit mentions an issue" do
expect(SystemNoteService).to receive(:cross_reference).with(issue, commit, commit_author)
execute_service(project, user, @oldrev, @newrev, @ref )
execute_service(project, user, oldrev, newrev, ref)
end
it "only creates a cross-reference note if one doesn't already exist" do
......@@ -335,7 +322,7 @@ describe GitPushService, services: true do
expect(SystemNoteService).not_to receive(:cross_reference).with(issue, commit, commit_author)
execute_service(project, user, @oldrev, @newrev, @ref )
execute_service(project, user, oldrev, newrev, ref)
end
it "defaults to the pushing user if the commit's author is not known" do
......@@ -345,16 +332,16 @@ describe GitPushService, services: true do
)
expect(SystemNoteService).to receive(:cross_reference).with(issue, commit, user)
execute_service(project, user, @oldrev, @newrev, @ref )
execute_service(project, user, oldrev, newrev, ref)
end
it "finds references in the first push to a non-default branch" do
allow(project.repository).to receive(:commits_between).with(@blankrev, @newrev).and_return([])
allow(project.repository).to receive(:commits_between).with("master", @newrev).and_return([commit])
allow(project.repository).to receive(:commits_between).with(blankrev, newrev).and_return([])
allow(project.repository).to receive(:commits_between).with("master", newrev).and_return([commit])
expect(SystemNoteService).to receive(:cross_reference).with(issue, commit, commit_author)
execute_service(project, user, @blankrev, @newrev, 'refs/heads/other' )
execute_service(project, user, blankrev, newrev, 'refs/heads/other')
end
end
......@@ -384,14 +371,14 @@ describe GitPushService, services: true do
context "while saving the 'first_mentioned_in_commit_at' metric for an issue" do
it 'sets the metric for referenced issues' do
execute_service(project, user, @oldrev, @newrev, @ref)
execute_service(project, user, oldrev, newrev, ref)
expect(issue.reload.metrics.first_mentioned_in_commit_at).to be_like_time(commit_time)
end
it 'does not set the metric for non-referenced issues' do
non_referenced_issue = create(:issue, project: project)
execute_service(project, user, @oldrev, @newrev, @ref)
execute_service(project, user, oldrev, newrev, ref)
expect(non_referenced_issue.reload.metrics.first_mentioned_in_commit_at).to be_nil
end
......@@ -423,18 +410,18 @@ describe GitPushService, services: true do
context "to default branches" do
it "closes issues" do
execute_service(project, commit_author, @oldrev, @newrev, @ref )
execute_service(project, commit_author, oldrev, newrev, ref)
expect(Issue.find(issue.id)).to be_closed
end
it "adds a note indicating that the issue is now closed" do
expect(SystemNoteService).to receive(:change_status).with(issue, project, commit_author, "closed", closing_commit)
execute_service(project, commit_author, @oldrev, @newrev, @ref )
execute_service(project, commit_author, oldrev, newrev, ref)
end
it "doesn't create additional cross-reference notes" do
expect(SystemNoteService).not_to receive(:cross_reference)
execute_service(project, commit_author, @oldrev, @newrev, @ref )
execute_service(project, commit_author, oldrev, newrev, ref)
end
it "doesn't close issues when external issue tracker is in use" do
......@@ -445,7 +432,7 @@ describe GitPushService, services: true do
# The push still shouldn't create cross-reference notes.
expect do
execute_service(project, commit_author, @oldrev, @newrev, 'refs/heads/hurf' )
execute_service(project, commit_author, oldrev, newrev, 'refs/heads/hurf')
end.not_to change { Note.where(project_id: project.id, system: true).count }
end
end
......@@ -458,11 +445,11 @@ describe GitPushService, services: true do
it "creates cross-reference notes" do
expect(SystemNoteService).to receive(:cross_reference).with(issue, closing_commit, commit_author)
execute_service(project, user, @oldrev, @newrev, @ref )
execute_service(project, user, oldrev, newrev, ref)
end
it "doesn't close issues" do
execute_service(project, user, @oldrev, @newrev, @ref )
execute_service(project, user, oldrev, newrev, ref)
expect(Issue.find(issue.id)).to be_opened
end
end
......@@ -496,7 +483,7 @@ describe GitPushService, services: true do
let(:message) { "this is some work.\n\nrelated to JIRA-1" }
it "initiates one api call to jira server to mention the issue" do
execute_service(project, user, @oldrev, @newrev, @ref)
execute_service(project, user, oldrev, newrev, ref)
expect(WebMock).to have_requested(:post, jira_api_comment_url('JIRA-1')).with(
body: /mentioned this issue in/
......@@ -506,7 +493,11 @@ describe GitPushService, services: true do
context "closing an issue" do
let(:message) { "this is some work.\n\ncloses JIRA-1" }
let(:comment_body) { { body: "Issue solved with [#{closing_commit.id}|http://#{Gitlab.config.gitlab.host}/#{project.path_with_namespace}/commit/#{closing_commit.id}]." }.to_json }
let(:comment_body) do
{
body: "Issue solved with [#{closing_commit.id}|http://#{Gitlab.config.gitlab.host}/#{project.path_with_namespace}/commit/#{closing_commit.id}]."
}.to_json
end
before do
open_issue = JIRA::Resource::Issue.new(jira_tracker.client, attrs: { "id" => "JIRA-1" })
......@@ -520,13 +511,13 @@ describe GitPushService, services: true do
context "using right markdown" do
it "initiates one api call to jira server to close the issue" do
execute_service(project, commit_author, @oldrev, @newrev, @ref )
execute_service(project, commit_author, oldrev, newrev, ref)
expect(WebMock).to have_requested(:post, jira_api_transition_url('JIRA-1')).once
end
it "initiates one api call to jira server to comment on the issue" do
execute_service(project, commit_author, @oldrev, @newrev, @ref )
execute_service(project, commit_author, oldrev, newrev, ref)
expect(WebMock).to have_requested(:post, jira_api_comment_url('JIRA-1')).with(
body: comment_body
......@@ -538,13 +529,13 @@ describe GitPushService, services: true do
let(:message) { "this is some work.\n\ncloses #1" }
it "does not initiates one api call to jira server to close the issue" do
execute_service(project, commit_author, @oldrev, @newrev, @ref )
execute_service(project, commit_author, oldrev, newrev, ref)
expect(WebMock).not_to have_requested(:post, jira_api_transition_url('JIRA-1'))
end
it "does not initiates one api call to jira server to comment on the issue" do
execute_service(project, commit_author, @oldrev, @newrev, @ref )
execute_service(project, commit_author, oldrev, newrev, ref)
expect(WebMock).not_to have_requested(:post, jira_api_comment_url('JIRA-1')).with(
body: comment_body
......@@ -557,7 +548,7 @@ describe GitPushService, services: true do
describe "empty project" do
let(:project) { create(:project_empty_repo) }
let(:new_ref) { 'refs/heads/feature'}
let(:new_ref) { 'refs/heads/feature' }
before do
allow(project).to receive(:default_branch).and_return('feature')
......@@ -565,7 +556,7 @@ describe GitPushService, services: true do
end
it 'push to first branch updates HEAD' do
execute_service(project, user, @blankrev, @newrev, new_ref )
execute_service(project, user, blankrev, newrev, new_ref)
end
end
......@@ -586,7 +577,7 @@ describe GitPushService, services: true do
it 'does not perform housekeeping when not needed' do
expect(housekeeping).not_to receive(:execute)
execute_service(project, user, @oldrev, @newrev, @ref)
execute_service(project, user, oldrev, newrev, ref)
end
context 'when housekeeping is needed' do
......@@ -597,20 +588,20 @@ describe GitPushService, services: true do
it 'performs housekeeping' do
expect(housekeeping).to receive(:execute)
execute_service(project, user, @oldrev, @newrev, @ref)
execute_service(project, user, oldrev, newrev, ref)
end
it 'does not raise an exception' do
allow(housekeeping).to receive(:try_obtain_lease).and_return(false)
execute_service(project, user, @oldrev, @newrev, @ref)
execute_service(project, user, oldrev, newrev, ref)
end
end
it 'increments the push counter' do
expect(housekeeping).to receive(:increment!)
execute_service(project, user, @oldrev, @newrev, @ref)
execute_service(project, user, oldrev, newrev, ref)
end
end
......@@ -618,9 +609,9 @@ describe GitPushService, services: true do
let(:service) do
described_class.new(project,
user,
oldrev: sample_commit.parent_id,
newrev: sample_commit.id,
ref: 'refs/heads/master')
oldrev: oldrev,
newrev: newrev,
ref: ref)
end
context 'on the default branch' do
......@@ -663,9 +654,9 @@ describe GitPushService, services: true do
let(:service) do
described_class.new(project,
user,
oldrev: sample_commit.parent_id,
newrev: sample_commit.id,
ref: 'refs/heads/master')
oldrev: oldrev,
newrev: newrev,
ref: ref)
end
it 'only schedules a limited number of commits' do
......@@ -679,8 +670,12 @@ describe GitPushService, services: true do
end
def execute_service(project, user, oldrev, newrev, ref)
service = described_class.new(project, user, oldrev: oldrev, newrev: newrev, ref: ref )
service = described_class.new(project, user, oldrev: oldrev, newrev: newrev, ref: ref)
service.execute
service
end
def push_data_from_service(project, user, oldrev, newrev, ref)
execute_service(project, user, oldrev, newrev, ref).push_data
end
end
spec/workers/repository_update_remote_mirror_worker_spec.rb
View file @ c54e08cf
......@@ -28,7 +28,7 @@ describe RepositoryUpdateRemoteMirrorWorker do
expect_any_instance_of(Projects::UpdateRemoteMirrorService).to receive(:execute).with(remote_mirror).and_return(status: :error, message: error_message)
expect do
subject.perform(remote_mirror.id, Time.now)
end.to raise_error(RepositoryUpdateRemoteMirrorWorker::UpdateError, 'fail!')
end.to raise_error(RepositoryUpdateRemoteMirrorWorker::UpdateError, error_message)
expect(remote_mirror.reload.update_status).to eq('failed')
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7