summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorOswaldo Ferreira <oswaldo@gitlab.com>2017-10-13 10:17:41 +0200
committerOswaldo Ferreira <oswaldo@gitlab.com>2017-10-13 10:17:41 +0200
commitb78954c13d127d84fa1e10db83f51b23790aa526 (patch)
treeeacba23ba4cbb4f24f2daa2330f72f56343e6619
parent9ab0254085cc4eed5cda322b6d5998c320ceef2c (diff)
downloadgitlab-ce-39032-improve-merge-ongoing-check-consistency.tar.gz
Simplify check for running job on Redis39032-improve-merge-ongoing-check-consistency
-rw-r--r--app/models/merge_request.rb2
-rw-r--r--lib/gitlab/sidekiq_status.rb7
-rw-r--r--spec/lib/gitlab/sidekiq_status_spec.rb12
-rw-r--r--spec/models/merge_request_spec.rb4
4 files changed, 22 insertions, 3 deletions
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index e34714e3f9e..c3fae16d109 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -396,7 +396,7 @@ class MergeRequest < ActiveRecord::Base
end
def merge_ongoing?
- !!merge_jid && !merged? && Gitlab::SidekiqStatus.num_running([merge_jid]) > 0
+ !!merge_jid && !merged? && Gitlab::SidekiqStatus.running?(merge_jid)
end
def closed_without_fork?
diff --git a/lib/gitlab/sidekiq_status.rb b/lib/gitlab/sidekiq_status.rb
index a0a2769cf9e..a1f689d94d9 100644
--- a/lib/gitlab/sidekiq_status.rb
+++ b/lib/gitlab/sidekiq_status.rb
@@ -51,6 +51,13 @@ module Gitlab
self.num_running(job_ids).zero?
end
+ # Returns true if the given job is running
+ #
+ # job_id - The Sidekiq job ID to check.
+ def self.running?(job_id)
+ num_running([job_id]) > 0
+ end
+
# Returns the number of jobs that are running.
#
# job_ids - The Sidekiq job IDs to check.
diff --git a/spec/lib/gitlab/sidekiq_status_spec.rb b/spec/lib/gitlab/sidekiq_status_spec.rb
index c2e77ef6b6c..884f27b212c 100644
--- a/spec/lib/gitlab/sidekiq_status_spec.rb
+++ b/spec/lib/gitlab/sidekiq_status_spec.rb
@@ -39,6 +39,18 @@ describe Gitlab::SidekiqStatus do
end
end
+ describe '.running?', :clean_gitlab_redis_shared_state do
+ it 'returns true if job is running' do
+ described_class.set('123')
+
+ expect(described_class.running?('123')).to be(true)
+ end
+
+ it 'returns false if job is not found' do
+ expect(described_class.running?('123')).to be(false)
+ end
+ end
+
describe '.num_running', :clean_gitlab_redis_shared_state do
it 'returns 0 if all jobs have been completed' do
expect(described_class.num_running(%w(123))).to eq(0)
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 09d7d0dbbe6..73e038b61ed 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -1462,7 +1462,7 @@ describe MergeRequest do
describe '#merge_ongoing?' do
it 'returns true when merge_id, MR is not merged and it has no running job' do
merge_request = build_stubbed(:merge_request, state: :open, merge_jid: 'foo')
- allow(Gitlab::SidekiqStatus).to receive(:num_running).with(['foo']) { 1 }
+ allow(Gitlab::SidekiqStatus).to receive(:running?).with('foo') { true }
expect(merge_request.merge_ongoing?).to be(true)
end
@@ -1481,7 +1481,7 @@ describe MergeRequest do
it 'returns false if there is no merge job running' do
merge_request = build_stubbed(:merge_request, state: :open, merge_jid: 'foo')
- allow(Gitlab::SidekiqStatus).to receive(:num_running).with(['foo']) { 0 }
+ allow(Gitlab::SidekiqStatus).to receive(:running?).with('foo') { false }
expect(merge_request.merge_ongoing?).to be(false)
end