From 0ce6785851510ccb49f0d1edc0220aca46f815f5 Mon Sep 17 00:00:00 2001 From: Jacopo Date: Tue, 3 Oct 2017 10:35:01 +0200 Subject: Replaces `tag: true` into `:tag` in the specs Replaces all the explicit include metadata syntax in the specs (tag: true) into the implicit one (:tag). Added a cop to prevent future errors and handle autocorrection. --- spec/workers/git_garbage_collect_worker_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/workers') diff --git a/spec/workers/git_garbage_collect_worker_spec.rb b/spec/workers/git_garbage_collect_worker_spec.rb index ab656d619f4..47297de738b 100644 --- a/spec/workers/git_garbage_collect_worker_spec.rb +++ b/spec/workers/git_garbage_collect_worker_spec.rb @@ -104,7 +104,7 @@ describe GitGarbageCollectWorker do it_should_behave_like 'flushing ref caches', true end - context "with Gitaly turned off", skip_gitaly_mock: true do + context "with Gitaly turned off", :skip_gitaly_mock do it_should_behave_like 'flushing ref caches', false end -- cgit v1.2.1 From 5b32a4aaffc54c7ea9aca9e2745fcbcdbeae7a22 Mon Sep 17 00:00:00 2001 From: Tiago Botelho Date: Tue, 10 Oct 2017 10:09:49 +0100 Subject: Backports EE 38771 changes to CE. --- spec/workers/repository_fork_worker_spec.rb | 22 ++++++++++++++++++++++ spec/workers/repository_import_worker_spec.rb | 17 +++++++++++++++++ 2 files changed, 39 insertions(+) (limited to 'spec/workers') diff --git a/spec/workers/repository_fork_worker_spec.rb b/spec/workers/repository_fork_worker_spec.rb index d9e9409840f..e881ec37ae5 100644 --- a/spec/workers/repository_fork_worker_spec.rb +++ b/spec/workers/repository_fork_worker_spec.rb @@ -12,6 +12,28 @@ describe RepositoryForkWorker do end describe "#perform" do + describe 'when a worker was reset without cleanup' do + let(:jid) { '12345678' } + let(:started_project) { create(:project, :repository, :import_started) } + + it 'creates a new repository from a fork' do + allow(subject).to receive(:jid).and_return(jid) + + expect(shell).to receive(:fork_repository).with( + '/test/path', + project.full_path, + project.repository_storage_path, + fork_project.namespace.full_path + ).and_return(true) + + subject.perform( + project.id, + '/test/path', + project.full_path, + fork_project.namespace.full_path) + end + end + it "creates a new repository from a fork" do expect(shell).to receive(:fork_repository).with( '/test/path', diff --git a/spec/workers/repository_import_worker_spec.rb b/spec/workers/repository_import_worker_spec.rb index 100dfc32bbe..5cff5108477 100644 --- a/spec/workers/repository_import_worker_spec.rb +++ b/spec/workers/repository_import_worker_spec.rb @@ -6,6 +6,23 @@ describe RepositoryImportWorker do subject { described_class.new } describe '#perform' do + context 'when worker was reset without cleanup' do + let(:jid) { '12345678' } + let(:started_project) { create(:project, :import_started, import_jid: jid) } + + it 'imports the project successfully' do + allow(subject).to receive(:jid).and_return(jid) + + expect_any_instance_of(Projects::ImportService).to receive(:execute) + .and_return({ status: :ok }) + + expect_any_instance_of(Repository).to receive(:expire_emptiness_caches) + expect_any_instance_of(Project).to receive(:import_finish) + + subject.perform(project.id) + end + end + context 'when the import was successful' do it 'imports a project' do expect_any_instance_of(Projects::ImportService).to receive(:execute) -- cgit v1.2.1 From a6c52c4e593850eb270e5bfbd021cdfb56e6eed6 Mon Sep 17 00:00:00 2001 From: Oswaldo Ferreira Date: Fri, 27 Oct 2017 22:11:21 +0200 Subject: Make merge_jid handling less stateful in MergeService --- spec/workers/stuck_merge_jobs_worker_spec.rb | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'spec/workers') diff --git a/spec/workers/stuck_merge_jobs_worker_spec.rb b/spec/workers/stuck_merge_jobs_worker_spec.rb index a5ad78393c9..f8b55e873df 100644 --- a/spec/workers/stuck_merge_jobs_worker_spec.rb +++ b/spec/workers/stuck_merge_jobs_worker_spec.rb @@ -12,8 +12,13 @@ describe StuckMergeJobsWorker do worker.perform - expect(mr_with_sha.reload).to be_merged - expect(mr_without_sha.reload).to be_opened + mr_with_sha.reload + mr_without_sha.reload + + expect(mr_with_sha).to be_merged + expect(mr_without_sha).to be_opened + expect(mr_with_sha.merge_jid).to be_present + expect(mr_without_sha.merge_jid).to be_nil end it 'updates merge request to opened when locked but has not been merged' do -- cgit v1.2.1 From 4934f6078b12072fd62f8065a1b25d961aa2d825 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Tue, 31 Oct 2017 00:18:15 +0900 Subject: specs for workers. --- spec/workers/cluster_provision_worker_spec.rb | 19 +++++-- .../wait_for_cluster_creation_worker_spec.rb | 61 +++++----------------- 2 files changed, 29 insertions(+), 51 deletions(-) (limited to 'spec/workers') diff --git a/spec/workers/cluster_provision_worker_spec.rb b/spec/workers/cluster_provision_worker_spec.rb index 11f208289db..85c7dc20ede 100644 --- a/spec/workers/cluster_provision_worker_spec.rb +++ b/spec/workers/cluster_provision_worker_spec.rb @@ -2,11 +2,22 @@ require 'spec_helper' describe ClusterProvisionWorker do describe '#perform' do - context 'when cluster exists' do - let(:cluster) { create(:gcp_cluster) } + context 'when provider type is gcp' do + let(:cluster) { create(:cluster, provider_type: :gcp, provider_gcp: provider) } + let(:provider) { create(:provider_gcp, :scheduled) } it 'provision a cluster' do - expect_any_instance_of(Ci::ProvisionClusterService).to receive(:execute) + expect_any_instance_of(Clusters::Gcp::ProvisionService).to receive(:execute) + + described_class.new.perform(cluster.id) + end + end + + context 'when provider type is user' do + let(:cluster) { create(:cluster, provider_type: :user) } + + it 'does not provision a cluster' do + expect_any_instance_of(Clusters::Gcp::ProvisionService).not_to receive(:execute) described_class.new.perform(cluster.id) end @@ -14,7 +25,7 @@ describe ClusterProvisionWorker do context 'when cluster does not exist' do it 'does not provision a cluster' do - expect_any_instance_of(Ci::ProvisionClusterService).not_to receive(:execute) + expect_any_instance_of(Clusters::Gcp::ProvisionService).not_to receive(:execute) described_class.new.perform(123) end diff --git a/spec/workers/wait_for_cluster_creation_worker_spec.rb b/spec/workers/wait_for_cluster_creation_worker_spec.rb index dcd4a3b9aec..29812408396 100644 --- a/spec/workers/wait_for_cluster_creation_worker_spec.rb +++ b/spec/workers/wait_for_cluster_creation_worker_spec.rb @@ -2,65 +2,32 @@ require 'spec_helper' describe WaitForClusterCreationWorker do describe '#perform' do - context 'when cluster exists' do - let(:cluster) { create(:gcp_cluster) } - let(:operation) { double } + context 'when provider type is gcp' do + let(:cluster) { create(:cluster, provider_type: :gcp, provider_gcp: provider) } + let(:provider) { create(:provider_gcp, :creating) } - before do - allow(operation).to receive(:status).and_return(status) - allow(operation).to receive(:start_time).and_return(1.minute.ago) - allow(operation).to receive(:status_message).and_return('error') - allow_any_instance_of(Ci::FetchGcpOperationService).to receive(:execute).and_yield(operation) - end - - context 'when operation status is RUNNING' do - let(:status) { 'RUNNING' } - - it 'reschedules worker' do - expect(described_class).to receive(:perform_in) - - described_class.new.perform(cluster.id) - end - - context 'when operation timeout' do - before do - allow(operation).to receive(:start_time).and_return(30.minutes.ago.utc) - end - - it 'sets an error message on cluster' do - described_class.new.perform(cluster.id) + it 'provision a cluster' do + expect_any_instance_of(Clusters::Gcp::VerifyProvisionStatusService).to receive(:execute) - expect(cluster.reload).to be_errored - end - end - end - - context 'when operation status is DONE' do - let(:status) { 'DONE' } - - it 'finalizes cluster creation' do - expect_any_instance_of(Ci::FinalizeClusterCreationService).to receive(:execute) - - described_class.new.perform(cluster.id) - end + described_class.new.perform(cluster.id) end + end - context 'when operation status is others' do - let(:status) { 'others' } + context 'when provider type is user' do + let(:cluster) { create(:cluster, provider_type: :user) } - it 'sets an error message on cluster' do - described_class.new.perform(cluster.id) + it 'does not provision a cluster' do + expect_any_instance_of(Clusters::Gcp::VerifyProvisionStatusService).not_to receive(:execute) - expect(cluster.reload).to be_errored - end + described_class.new.perform(cluster.id) end end context 'when cluster does not exist' do it 'does not provision a cluster' do - expect_any_instance_of(Ci::FetchGcpOperationService).not_to receive(:execute) + expect_any_instance_of(Clusters::Gcp::VerifyProvisionStatusService).not_to receive(:execute) - described_class.new.perform(1234) + described_class.new.perform(123) end end end -- cgit v1.2.1 From a99ad59e655d66fda8af7f2b89aced79b8bc1060 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 6 Nov 2017 23:06:10 +0900 Subject: Remove 10.3 comments (Tracked by a tech debts issue). Refactor spec factory name. Use ArgumentError --- spec/workers/cluster_provision_worker_spec.rb | 2 +- spec/workers/wait_for_cluster_creation_worker_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'spec/workers') diff --git a/spec/workers/cluster_provision_worker_spec.rb b/spec/workers/cluster_provision_worker_spec.rb index 85c7dc20ede..8054ec11a48 100644 --- a/spec/workers/cluster_provision_worker_spec.rb +++ b/spec/workers/cluster_provision_worker_spec.rb @@ -4,7 +4,7 @@ describe ClusterProvisionWorker do describe '#perform' do context 'when provider type is gcp' do let(:cluster) { create(:cluster, provider_type: :gcp, provider_gcp: provider) } - let(:provider) { create(:provider_gcp, :scheduled) } + let(:provider) { create(:cluster_provider_gcp, :scheduled) } it 'provision a cluster' do expect_any_instance_of(Clusters::Gcp::ProvisionService).to receive(:execute) diff --git a/spec/workers/wait_for_cluster_creation_worker_spec.rb b/spec/workers/wait_for_cluster_creation_worker_spec.rb index 29812408396..0e92b298178 100644 --- a/spec/workers/wait_for_cluster_creation_worker_spec.rb +++ b/spec/workers/wait_for_cluster_creation_worker_spec.rb @@ -4,7 +4,7 @@ describe WaitForClusterCreationWorker do describe '#perform' do context 'when provider type is gcp' do let(:cluster) { create(:cluster, provider_type: :gcp, provider_gcp: provider) } - let(:provider) { create(:provider_gcp, :creating) } + let(:provider) { create(:cluster_provider_gcp, :creating) } it 'provision a cluster' do expect_any_instance_of(Clusters::Gcp::VerifyProvisionStatusService).to receive(:execute) -- cgit v1.2.1 From 4dfe26cd8b6863b7e6c81f5c280cdafe9b6e17b6 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Fri, 13 Oct 2017 18:50:36 +0200 Subject: Rewrite the GitHub importer from scratch Prior to this MR there were two GitHub related importers: * Github::Import: the main importer used for GitHub projects * Gitlab::GithubImport: importer that's somewhat confusingly used for importing Gitea projects (apparently they have a compatible API) This MR renames the Gitea importer to Gitlab::LegacyGithubImport and introduces a new GitHub importer in the Gitlab::GithubImport namespace. This new GitHub importer uses Sidekiq for importing multiple resources in parallel, though it also has the ability to import data sequentially should this be necessary. The new code is spread across the following directories: * lib/gitlab/github_import: this directory contains most of the importer code such as the classes used for importing resources. * app/workers/gitlab/github_import: this directory contains the Sidekiq workers, most of which simply use the code from the directory above. * app/workers/concerns/gitlab/github_import: this directory provides a few modules that are included in every GitHub importer worker. == Stages The import work is divided into separate stages, with each stage importing a specific set of data. Stages will schedule the work that needs to be performed, followed by scheduling a job for the "AdvanceStageWorker" worker. This worker will periodically check if all work is completed and schedule the next stage if this is the case. If work is not yet completed this worker will reschedule itself. Using this approach we don't have to block threads by calling `sleep()`, as doing so for large projects could block the thread from doing any work for many hours. == Retrying Work Workers will reschedule themselves whenever necessary. For example, hitting the GitHub API's rate limit will result in jobs rescheduling themselves. These jobs are not processed until the rate limit has been reset. == User Lookups Part of the importing process involves looking up user details in the GitHub API so we can map them to GitLab users. The old importer used an in-memory cache, but this obviously doesn't work when the work is spread across different threads. The new importer uses a Redis cache and makes sure we only perform API/database calls if absolutely necessary. Frequently used keys are refreshed, and lookup misses are also cached; removing the need for performing API/database calls if we know we don't have the data we're looking for. == Performance & Models The new importer in various places uses raw INSERT statements (as generated by `Gitlab::Database.bulk_insert`) instead of using Rails models. This allows us to bypass any validations and callbacks, drastically reducing the number of SQL queries and Gitaly RPC calls necessary to import projects. To ensure the code produces valid data the corresponding tests check if the produced rows are valid according to the model validation rules. --- .../gitlab/github_import/notify_upon_death_spec.rb | 49 +++++++++ .../gitlab/github_import/object_importer_spec.rb | 70 +++++++++++++ .../concerns/gitlab/github_import/queue_spec.rb | 12 +++ .../github_import/rescheduling_methods_spec.rb | 110 ++++++++++++++++++++ .../gitlab/github_import/stage_methods_spec.rb | 77 ++++++++++++++ .../github_import/advance_stage_worker_spec.rb | 115 +++++++++++++++++++++ .../github_import/import_diff_note_worker_spec.rb | 42 ++++++++ .../github_import/import_issue_worker_spec.rb | 45 ++++++++ .../github_import/import_note_worker_spec.rb | 40 +++++++ .../import_pull_request_worker_spec.rb | 51 +++++++++ .../refresh_import_jid_worker_spec.rb | 95 +++++++++++++++++ .../stage/finish_import_worker_spec.rb | 32 ++++++ .../stage/import_base_data_worker_spec.rb | 30 ++++++ .../import_issues_and_diff_notes_worker_spec.rb | 32 ++++++ .../stage/import_notes_worker_spec.rb | 29 ++++++ .../stage/import_pull_requests_worker_spec.rb | 32 ++++++ .../stage/import_repository_worker_spec.rb | 49 +++++++++ spec/workers/repository_import_worker_spec.rb | 23 +++++ 18 files changed, 933 insertions(+) create mode 100644 spec/workers/concerns/gitlab/github_import/notify_upon_death_spec.rb create mode 100644 spec/workers/concerns/gitlab/github_import/object_importer_spec.rb create mode 100644 spec/workers/concerns/gitlab/github_import/queue_spec.rb create mode 100644 spec/workers/concerns/gitlab/github_import/rescheduling_methods_spec.rb create mode 100644 spec/workers/concerns/gitlab/github_import/stage_methods_spec.rb create mode 100644 spec/workers/gitlab/github_import/advance_stage_worker_spec.rb create mode 100644 spec/workers/gitlab/github_import/import_diff_note_worker_spec.rb create mode 100644 spec/workers/gitlab/github_import/import_issue_worker_spec.rb create mode 100644 spec/workers/gitlab/github_import/import_note_worker_spec.rb create mode 100644 spec/workers/gitlab/github_import/import_pull_request_worker_spec.rb create mode 100644 spec/workers/gitlab/github_import/refresh_import_jid_worker_spec.rb create mode 100644 spec/workers/gitlab/github_import/stage/finish_import_worker_spec.rb create mode 100644 spec/workers/gitlab/github_import/stage/import_base_data_worker_spec.rb create mode 100644 spec/workers/gitlab/github_import/stage/import_issues_and_diff_notes_worker_spec.rb create mode 100644 spec/workers/gitlab/github_import/stage/import_notes_worker_spec.rb create mode 100644 spec/workers/gitlab/github_import/stage/import_pull_requests_worker_spec.rb create mode 100644 spec/workers/gitlab/github_import/stage/import_repository_worker_spec.rb (limited to 'spec/workers') diff --git a/spec/workers/concerns/gitlab/github_import/notify_upon_death_spec.rb b/spec/workers/concerns/gitlab/github_import/notify_upon_death_spec.rb new file mode 100644 index 00000000000..4b9aa9a7ef8 --- /dev/null +++ b/spec/workers/concerns/gitlab/github_import/notify_upon_death_spec.rb @@ -0,0 +1,49 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::NotifyUponDeath do + let(:worker_class) do + Class.new do + include Sidekiq::Worker + include Gitlab::GithubImport::NotifyUponDeath + end + end + + describe '.sidekiq_retries_exhausted' do + it 'notifies the JobWaiter when 3 arguments are given and the last is a String' do + job = { 'args' => [12, {}, '123abc'], 'jid' => '123' } + + expect(Gitlab::JobWaiter) + .to receive(:notify) + .with('123abc', '123') + + worker_class.sidekiq_retries_exhausted_block.call(job) + end + + it 'does not notify the JobWaiter when only 2 arguments are given' do + job = { 'args' => [12, {}], 'jid' => '123' } + + expect(Gitlab::JobWaiter) + .not_to receive(:notify) + + worker_class.sidekiq_retries_exhausted_block.call(job) + end + + it 'does not notify the JobWaiter when only 1 argument is given' do + job = { 'args' => [12], 'jid' => '123' } + + expect(Gitlab::JobWaiter) + .not_to receive(:notify) + + worker_class.sidekiq_retries_exhausted_block.call(job) + end + + it 'does not notify the JobWaiter when the last argument is not a String' do + job = { 'args' => [12, {}, 40], 'jid' => '123' } + + expect(Gitlab::JobWaiter) + .not_to receive(:notify) + + worker_class.sidekiq_retries_exhausted_block.call(job) + end + end +end diff --git a/spec/workers/concerns/gitlab/github_import/object_importer_spec.rb b/spec/workers/concerns/gitlab/github_import/object_importer_spec.rb new file mode 100644 index 00000000000..3ccf06f2d7d --- /dev/null +++ b/spec/workers/concerns/gitlab/github_import/object_importer_spec.rb @@ -0,0 +1,70 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::ObjectImporter do + let(:worker) do + Class.new do + include(Gitlab::GithubImport::ObjectImporter) + + def counter_name + :dummy_counter + end + + def counter_description + 'This is a counter' + end + end.new + end + + describe '#import' do + it 'imports the object' do + representation_class = double(:representation_class) + importer_class = double(:importer_class) + importer_instance = double(:importer_instance) + representation = double(:representation) + project = double(:project, path_with_namespace: 'foo/bar') + client = double(:client) + + expect(worker) + .to receive(:representation_class) + .and_return(representation_class) + + expect(worker) + .to receive(:importer_class) + .and_return(importer_class) + + expect(representation_class) + .to receive(:from_json_hash) + .with(an_instance_of(Hash)) + .and_return(representation) + + expect(importer_class) + .to receive(:new) + .with(representation, project, client) + .and_return(importer_instance) + + expect(importer_instance) + .to receive(:execute) + + expect(worker.counter) + .to receive(:increment) + .with(project: 'foo/bar') + .and_call_original + + worker.import(project, client, { 'number' => 10 }) + end + end + + describe '#counter' do + it 'returns a Prometheus counter' do + expect(worker) + .to receive(:counter_name) + .and_call_original + + expect(worker) + .to receive(:counter_description) + .and_call_original + + worker.counter + end + end +end diff --git a/spec/workers/concerns/gitlab/github_import/queue_spec.rb b/spec/workers/concerns/gitlab/github_import/queue_spec.rb new file mode 100644 index 00000000000..321ae3fe978 --- /dev/null +++ b/spec/workers/concerns/gitlab/github_import/queue_spec.rb @@ -0,0 +1,12 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Queue do + it 'sets the Sidekiq options for the worker' do + worker = Class.new do + include Sidekiq::Worker + include Gitlab::GithubImport::Queue + end + + expect(worker.sidekiq_options['queue']).to eq('github_importer') + end +end diff --git a/spec/workers/concerns/gitlab/github_import/rescheduling_methods_spec.rb b/spec/workers/concerns/gitlab/github_import/rescheduling_methods_spec.rb new file mode 100644 index 00000000000..8de4059c4ae --- /dev/null +++ b/spec/workers/concerns/gitlab/github_import/rescheduling_methods_spec.rb @@ -0,0 +1,110 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::ReschedulingMethods do + let(:worker) do + Class.new { include(Gitlab::GithubImport::ReschedulingMethods) }.new + end + + describe '#perform' do + context 'with a non-existing project' do + it 'does not perform any work' do + expect(worker) + .not_to receive(:try_import) + + worker.perform(-1, {}) + end + + it 'notifies any waiters so they do not wait forever' do + expect(worker) + .to receive(:notify_waiter) + .with('123') + + worker.perform(-1, {}, '123') + end + end + + context 'with an existing project' do + let(:project) { create(:project) } + + it 'notifies any waiters upon successfully importing the data' do + expect(worker) + .to receive(:try_import) + .with( + an_instance_of(Project), + an_instance_of(Gitlab::GithubImport::Client), + { 'number' => 2 } + ) + .and_return(true) + + expect(worker) + .to receive(:notify_waiter).with('123') + + worker.perform(project.id, { 'number' => 2 }, '123') + end + + it 'reschedules itself if the data could not be imported' do + expect(worker) + .to receive(:try_import) + .with( + an_instance_of(Project), + an_instance_of(Gitlab::GithubImport::Client), + { 'number' => 2 } + ) + .and_return(false) + + expect(worker) + .not_to receive(:notify_waiter) + + expect_any_instance_of(Gitlab::GithubImport::Client) + .to receive(:rate_limit_resets_in) + .and_return(14) + + expect(worker.class) + .to receive(:perform_in) + .with(14, project.id, { 'number' => 2 }, '123') + + worker.perform(project.id, { 'number' => 2 }, '123') + end + end + end + + describe '#try_import' do + it 'returns true when the import succeeds' do + expect(worker) + .to receive(:import) + .with(10, 20) + + expect(worker.try_import(10, 20)).to eq(true) + end + + it 'returns false when the import fails due to hitting the GitHub API rate limit' do + expect(worker) + .to receive(:import) + .with(10, 20) + .and_raise(Gitlab::GithubImport::RateLimitError) + + expect(worker.try_import(10, 20)).to eq(false) + end + end + + describe '#notify_waiter' do + it 'notifies the waiter if a waiter key is specified' do + expect(worker) + .to receive(:jid) + .and_return('abc123') + + expect(Gitlab::JobWaiter) + .to receive(:notify) + .with('123', 'abc123') + + worker.notify_waiter('123') + end + + it 'does not notify any waiters if no waiter key is specified' do + expect(Gitlab::JobWaiter) + .not_to receive(:notify) + + worker.notify_waiter(nil) + end + end +end diff --git a/spec/workers/concerns/gitlab/github_import/stage_methods_spec.rb b/spec/workers/concerns/gitlab/github_import/stage_methods_spec.rb new file mode 100644 index 00000000000..241e8a2b6d3 --- /dev/null +++ b/spec/workers/concerns/gitlab/github_import/stage_methods_spec.rb @@ -0,0 +1,77 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::StageMethods do + let(:project) { create(:project) } + let(:worker) do + Class.new { include(Gitlab::GithubImport::StageMethods) }.new + end + + describe '#perform' do + it 'returns if no project could be found' do + expect(worker).not_to receive(:try_import) + + worker.perform(-1) + end + + it 'imports the data when the project exists' do + allow(worker) + .to receive(:find_project) + .with(project.id) + .and_return(project) + + expect(worker) + .to receive(:try_import) + .with( + an_instance_of(Gitlab::GithubImport::Client), + an_instance_of(Project) + ) + + worker.perform(project.id) + end + end + + describe '#try_import' do + it 'imports the project' do + client = double(:client) + + expect(worker) + .to receive(:import) + .with(client, project) + + worker.try_import(client, project) + end + + it 'reschedules the worker if RateLimitError was raised' do + client = double(:client, rate_limit_resets_in: 10) + + expect(worker) + .to receive(:import) + .with(client, project) + .and_raise(Gitlab::GithubImport::RateLimitError) + + expect(worker.class) + .to receive(:perform_in) + .with(10, project.id) + + worker.try_import(client, project) + end + end + + describe '#find_project' do + it 'returns a Project for an existing ID' do + project.update_column(:import_status, 'started') + + expect(worker.find_project(project.id)).to eq(project) + end + + it 'returns nil for a project that failed importing' do + project.update_column(:import_status, 'failed') + + expect(worker.find_project(project.id)).to be_nil + end + + it 'returns nil for a non-existing project ID' do + expect(worker.find_project(-1)).to be_nil + end + end +end diff --git a/spec/workers/gitlab/github_import/advance_stage_worker_spec.rb b/spec/workers/gitlab/github_import/advance_stage_worker_spec.rb new file mode 100644 index 00000000000..3be49a0dee8 --- /dev/null +++ b/spec/workers/gitlab/github_import/advance_stage_worker_spec.rb @@ -0,0 +1,115 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::AdvanceStageWorker, :clean_gitlab_redis_shared_state do + let(:project) { create(:project, import_jid: '123') } + let(:worker) { described_class.new } + + describe '#perform' do + context 'when the project no longer exists' do + it 'does not perform any work' do + expect(worker).not_to receive(:wait_for_jobs) + + worker.perform(-1, { '123' => 2 }, :finish) + end + end + + context 'when there are remaining jobs' do + before do + allow(worker) + .to receive(:find_project) + .and_return(project) + end + + it 'reschedules itself' do + expect(worker) + .to receive(:wait_for_jobs) + .with({ '123' => 2 }) + .and_return({ '123' => 1 }) + + expect(described_class) + .to receive(:perform_in) + .with(described_class::INTERVAL, project.id, { '123' => 1 }, :finish) + + worker.perform(project.id, { '123' => 2 }, :finish) + end + end + + context 'when there are no remaining jobs' do + before do + allow(worker) + .to receive(:find_project) + .and_return(project) + + allow(worker) + .to receive(:wait_for_jobs) + .with({ '123' => 2 }) + .and_return({}) + end + + it 'schedules the next stage' do + expect(project) + .to receive(:refresh_import_jid_expiration) + + expect(Gitlab::GithubImport::Stage::FinishImportWorker) + .to receive(:perform_async) + .with(project.id) + + worker.perform(project.id, { '123' => 2 }, :finish) + end + + it 'raises KeyError when the stage name is invalid' do + expect { worker.perform(project.id, { '123' => 2 }, :kittens) } + .to raise_error(KeyError) + end + end + end + + describe '#wait_for_jobs' do + it 'waits for jobs to complete and returns a new pair of keys to wait for' do + waiter1 = double(:waiter1, jobs_remaining: 1, key: '123') + waiter2 = double(:waiter2, jobs_remaining: 0, key: '456') + + expect(Gitlab::JobWaiter) + .to receive(:new) + .ordered + .with(2, '123') + .and_return(waiter1) + + expect(Gitlab::JobWaiter) + .to receive(:new) + .ordered + .with(1, '456') + .and_return(waiter2) + + expect(waiter1) + .to receive(:wait) + .with(described_class::BLOCKING_WAIT_TIME) + + expect(waiter2) + .to receive(:wait) + .with(described_class::BLOCKING_WAIT_TIME) + + new_waiters = worker.wait_for_jobs({ '123' => 2, '456' => 1 }) + + expect(new_waiters).to eq({ '123' => 1 }) + end + end + + describe '#find_project' do + it 'returns a Project' do + project.update_column(:import_status, 'started') + + found = worker.find_project(project.id) + + expect(found).to be_an_instance_of(Project) + + # This test is there to make sure we only select the columns we care + # about. + expect(found.attributes).to eq({ 'id' => nil, 'import_jid' => '123' }) + end + + it 'returns nil if the project import is not running' do + expect(worker.find_project(project.id)).to be_nil + end + end +end diff --git a/spec/workers/gitlab/github_import/import_diff_note_worker_spec.rb b/spec/workers/gitlab/github_import/import_diff_note_worker_spec.rb new file mode 100644 index 00000000000..7c8c665a9b3 --- /dev/null +++ b/spec/workers/gitlab/github_import/import_diff_note_worker_spec.rb @@ -0,0 +1,42 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::ImportDiffNoteWorker do + let(:worker) { described_class.new } + + describe '#import' do + it 'imports a diff note' do + project = double(:project, path_with_namespace: 'foo/bar') + client = double(:client) + importer = double(:importer) + hash = { + 'noteable_id' => 42, + 'path' => 'README.md', + 'commit_id' => '123abc', + 'diff_hunk' => "@@ -1 +1 @@\n-Hello\n+Hello world", + 'user' => { 'id' => 4, 'login' => 'alice' }, + 'note' => 'Hello world', + 'created_at' => Time.zone.now.to_s, + 'updated_at' => Time.zone.now.to_s + } + + expect(Gitlab::GithubImport::Importer::DiffNoteImporter) + .to receive(:new) + .with( + an_instance_of(Gitlab::GithubImport::Representation::DiffNote), + project, + client + ) + .and_return(importer) + + expect(importer) + .to receive(:execute) + + expect(worker.counter) + .to receive(:increment) + .with(project: 'foo/bar') + .and_call_original + + worker.import(project, client, hash) + end + end +end diff --git a/spec/workers/gitlab/github_import/import_issue_worker_spec.rb b/spec/workers/gitlab/github_import/import_issue_worker_spec.rb new file mode 100644 index 00000000000..4116380ff4d --- /dev/null +++ b/spec/workers/gitlab/github_import/import_issue_worker_spec.rb @@ -0,0 +1,45 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::ImportIssueWorker do + let(:worker) { described_class.new } + + describe '#import' do + it 'imports an issue' do + project = double(:project, path_with_namespace: 'foo/bar') + client = double(:client) + importer = double(:importer) + hash = { + 'iid' => 42, + 'title' => 'My Issue', + 'description' => 'This is my issue', + 'milestone_number' => 4, + 'state' => 'opened', + 'assignees' => [{ 'id' => 4, 'login' => 'alice' }], + 'label_names' => %w[bug], + 'user' => { 'id' => 4, 'login' => 'alice' }, + 'created_at' => Time.zone.now.to_s, + 'updated_at' => Time.zone.now.to_s, + 'pull_request' => false + } + + expect(Gitlab::GithubImport::Importer::IssueAndLabelLinksImporter) + .to receive(:new) + .with( + an_instance_of(Gitlab::GithubImport::Representation::Issue), + project, + client + ) + .and_return(importer) + + expect(importer) + .to receive(:execute) + + expect(worker.counter) + .to receive(:increment) + .with(project: 'foo/bar') + .and_call_original + + worker.import(project, client, hash) + end + end +end diff --git a/spec/workers/gitlab/github_import/import_note_worker_spec.rb b/spec/workers/gitlab/github_import/import_note_worker_spec.rb new file mode 100644 index 00000000000..0ca825a722b --- /dev/null +++ b/spec/workers/gitlab/github_import/import_note_worker_spec.rb @@ -0,0 +1,40 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::ImportNoteWorker do + let(:worker) { described_class.new } + + describe '#import' do + it 'imports a note' do + project = double(:project, path_with_namespace: 'foo/bar') + client = double(:client) + importer = double(:importer) + hash = { + 'noteable_id' => 42, + 'noteable_type' => 'issues', + 'user' => { 'id' => 4, 'login' => 'alice' }, + 'note' => 'Hello world', + 'created_at' => Time.zone.now.to_s, + 'updated_at' => Time.zone.now.to_s + } + + expect(Gitlab::GithubImport::Importer::NoteImporter) + .to receive(:new) + .with( + an_instance_of(Gitlab::GithubImport::Representation::Note), + project, + client + ) + .and_return(importer) + + expect(importer) + .to receive(:execute) + + expect(worker.counter) + .to receive(:increment) + .with(project: 'foo/bar') + .and_call_original + + worker.import(project, client, hash) + end + end +end diff --git a/spec/workers/gitlab/github_import/import_pull_request_worker_spec.rb b/spec/workers/gitlab/github_import/import_pull_request_worker_spec.rb new file mode 100644 index 00000000000..d49f560af42 --- /dev/null +++ b/spec/workers/gitlab/github_import/import_pull_request_worker_spec.rb @@ -0,0 +1,51 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::ImportPullRequestWorker do + let(:worker) { described_class.new } + + describe '#import' do + it 'imports a pull request' do + project = double(:project, path_with_namespace: 'foo/bar') + client = double(:client) + importer = double(:importer) + hash = { + 'iid' => 42, + 'title' => 'My Pull Request', + 'description' => 'This is my pull request', + 'source_branch' => 'my-feature', + 'source_branch_sha' => '123abc', + 'target_branch' => 'master', + 'target_branch_sha' => '456def', + 'source_repository_id' => 400, + 'target_repository_id' => 200, + 'source_repository_owner' => 'alice', + 'state' => 'closed', + 'milestone_number' => 4, + 'user' => { 'id' => 4, 'login' => 'alice' }, + 'assignee' => { 'id' => 4, 'login' => 'alice' }, + 'created_at' => Time.zone.now.to_s, + 'updated_at' => Time.zone.now.to_s, + 'merged_at' => Time.zone.now.to_s + } + + expect(Gitlab::GithubImport::Importer::PullRequestImporter) + .to receive(:new) + .with( + an_instance_of(Gitlab::GithubImport::Representation::PullRequest), + project, + client + ) + .and_return(importer) + + expect(importer) + .to receive(:execute) + + expect(worker.counter) + .to receive(:increment) + .with(project: 'foo/bar') + .and_call_original + + worker.import(project, client, hash) + end + end +end diff --git a/spec/workers/gitlab/github_import/refresh_import_jid_worker_spec.rb b/spec/workers/gitlab/github_import/refresh_import_jid_worker_spec.rb new file mode 100644 index 00000000000..073c6d7a2f5 --- /dev/null +++ b/spec/workers/gitlab/github_import/refresh_import_jid_worker_spec.rb @@ -0,0 +1,95 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::RefreshImportJidWorker do + let(:worker) { described_class.new } + + describe '.perform_in_the_future' do + it 'schedules a job in the future' do + expect(described_class) + .to receive(:perform_in) + .with(1.minute.to_i, 10, '123') + + described_class.perform_in_the_future(10, '123') + end + end + + describe '#perform' do + let(:project) { create(:project, import_jid: '123abc') } + + context 'when the project does not exist' do + it 'does nothing' do + expect(Gitlab::SidekiqStatus) + .not_to receive(:running?) + + worker.perform(-1, '123') + end + end + + context 'when the job is running' do + it 'refreshes the import JID and reschedules itself' do + allow(worker) + .to receive(:find_project) + .with(project.id) + .and_return(project) + + expect(Gitlab::SidekiqStatus) + .to receive(:running?) + .with('123') + .and_return(true) + + expect(project) + .to receive(:refresh_import_jid_expiration) + + expect(worker.class) + .to receive(:perform_in_the_future) + .with(project.id, '123') + + worker.perform(project.id, '123') + end + end + + context 'when the job is no longer running' do + it 'returns' do + allow(worker) + .to receive(:find_project) + .with(project.id) + .and_return(project) + + expect(Gitlab::SidekiqStatus) + .to receive(:running?) + .with('123') + .and_return(false) + + expect(project) + .not_to receive(:refresh_import_jid_expiration) + + worker.perform(project.id, '123') + end + end + end + + describe '#find_project' do + it 'returns a Project' do + project = create(:project, import_status: 'started') + + expect(worker.find_project(project.id)).to be_an_instance_of(Project) + end + + it 'only selects the import JID field' do + project = create(:project, import_status: 'started', import_jid: '123abc') + + expect(worker.find_project(project.id).attributes) + .to eq({ 'id' => nil, 'import_jid' => '123abc' }) + end + + it 'returns nil for a project for which the import process failed' do + project = create(:project, import_status: 'failed') + + expect(worker.find_project(project.id)).to be_nil + end + + it 'returns nil for a non-existing project' do + expect(worker.find_project(-1)).to be_nil + end + end +end diff --git a/spec/workers/gitlab/github_import/stage/finish_import_worker_spec.rb b/spec/workers/gitlab/github_import/stage/finish_import_worker_spec.rb new file mode 100644 index 00000000000..91e0cddb5d8 --- /dev/null +++ b/spec/workers/gitlab/github_import/stage/finish_import_worker_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Stage::FinishImportWorker do + let(:project) { create(:project) } + let(:worker) { described_class.new } + + describe '#perform' do + it 'marks the import as finished' do + expect(project).to receive(:after_import) + expect(worker).to receive(:report_import_time).with(project) + + worker.import(double(:client), project) + end + end + + describe '#report_import_time' do + it 'reports the total import time' do + expect(worker.histogram) + .to receive(:observe) + .with({ project: project.path_with_namespace }, a_kind_of(Numeric)) + .and_call_original + + expect(worker.counter) + .to receive(:increment) + .and_call_original + + expect(worker.logger).to receive(:info).with(an_instance_of(String)) + + worker.report_import_time(project) + end + end +end diff --git a/spec/workers/gitlab/github_import/stage/import_base_data_worker_spec.rb b/spec/workers/gitlab/github_import/stage/import_base_data_worker_spec.rb new file mode 100644 index 00000000000..8c80d660287 --- /dev/null +++ b/spec/workers/gitlab/github_import/stage/import_base_data_worker_spec.rb @@ -0,0 +1,30 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Stage::ImportBaseDataWorker do + let(:project) { create(:project) } + let(:worker) { described_class.new } + + describe '#import' do + it 'imports the base data of a project' do + importer = double(:importer) + client = double(:client) + + described_class::IMPORTERS.each do |klass| + expect(klass) + .to receive(:new) + .with(project, client) + .and_return(importer) + + expect(importer).to receive(:execute) + end + + expect(project).to receive(:refresh_import_jid_expiration) + + expect(Gitlab::GithubImport::Stage::ImportPullRequestsWorker) + .to receive(:perform_async) + .with(project.id) + + worker.import(client, project) + end + end +end diff --git a/spec/workers/gitlab/github_import/stage/import_issues_and_diff_notes_worker_spec.rb b/spec/workers/gitlab/github_import/stage/import_issues_and_diff_notes_worker_spec.rb new file mode 100644 index 00000000000..ab347f5b75b --- /dev/null +++ b/spec/workers/gitlab/github_import/stage/import_issues_and_diff_notes_worker_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Stage::ImportIssuesAndDiffNotesWorker do + let(:project) { create(:project) } + let(:worker) { described_class.new } + + describe '#import' do + it 'imports the issues and diff notes' do + client = double(:client) + + described_class::IMPORTERS.each do |klass| + importer = double(:importer) + waiter = Gitlab::JobWaiter.new(2, '123') + + expect(klass) + .to receive(:new) + .with(project, client) + .and_return(importer) + + expect(importer) + .to receive(:execute) + .and_return(waiter) + end + + expect(Gitlab::GithubImport::AdvanceStageWorker) + .to receive(:perform_async) + .with(project.id, { '123' => 2 }, :notes) + + worker.import(client, project) + end + end +end diff --git a/spec/workers/gitlab/github_import/stage/import_notes_worker_spec.rb b/spec/workers/gitlab/github_import/stage/import_notes_worker_spec.rb new file mode 100644 index 00000000000..098d2d55386 --- /dev/null +++ b/spec/workers/gitlab/github_import/stage/import_notes_worker_spec.rb @@ -0,0 +1,29 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Stage::ImportNotesWorker do + let(:project) { create(:project) } + let(:worker) { described_class.new } + + describe '#import' do + it 'imports all the notes' do + importer = double(:importer) + client = double(:client) + waiter = Gitlab::JobWaiter.new(2, '123') + + expect(Gitlab::GithubImport::Importer::NotesImporter) + .to receive(:new) + .with(project, client) + .and_return(importer) + + expect(importer) + .to receive(:execute) + .and_return(waiter) + + expect(Gitlab::GithubImport::AdvanceStageWorker) + .to receive(:perform_async) + .with(project.id, { '123' => 2 }, :finish) + + worker.import(client, project) + end + end +end diff --git a/spec/workers/gitlab/github_import/stage/import_pull_requests_worker_spec.rb b/spec/workers/gitlab/github_import/stage/import_pull_requests_worker_spec.rb new file mode 100644 index 00000000000..2fc91a3e80a --- /dev/null +++ b/spec/workers/gitlab/github_import/stage/import_pull_requests_worker_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Stage::ImportPullRequestsWorker do + let(:project) { create(:project) } + let(:worker) { described_class.new } + + describe '#import' do + it 'imports all the pull requests' do + importer = double(:importer) + client = double(:client) + waiter = Gitlab::JobWaiter.new(2, '123') + + expect(Gitlab::GithubImport::Importer::PullRequestsImporter) + .to receive(:new) + .with(project, client) + .and_return(importer) + + expect(importer) + .to receive(:execute) + .and_return(waiter) + + expect(project) + .to receive(:refresh_import_jid_expiration) + + expect(Gitlab::GithubImport::AdvanceStageWorker) + .to receive(:perform_async) + .with(project.id, { '123' => 2 }, :issues_and_diff_notes) + + worker.import(client, project) + end + end +end diff --git a/spec/workers/gitlab/github_import/stage/import_repository_worker_spec.rb b/spec/workers/gitlab/github_import/stage/import_repository_worker_spec.rb new file mode 100644 index 00000000000..adab535ac05 --- /dev/null +++ b/spec/workers/gitlab/github_import/stage/import_repository_worker_spec.rb @@ -0,0 +1,49 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Stage::ImportRepositoryWorker do + let(:project) { double(:project, id: 4) } + let(:worker) { described_class.new } + + describe '#import' do + before do + expect(Gitlab::GithubImport::RefreshImportJidWorker) + .to receive(:perform_in_the_future) + .with(project.id, '123') + + expect(worker) + .to receive(:jid) + .and_return('123') + end + + context 'when the import succeeds' do + it 'schedules the importing of the base data' do + client = double(:client) + + expect_any_instance_of(Gitlab::GithubImport::Importer::RepositoryImporter) + .to receive(:execute) + .and_return(true) + + expect(Gitlab::GithubImport::Stage::ImportBaseDataWorker) + .to receive(:perform_async) + .with(project.id) + + worker.import(client, project) + end + end + + context 'when the import fails' do + it 'does not schedule the importing of the base data' do + client = double(:client) + + expect_any_instance_of(Gitlab::GithubImport::Importer::RepositoryImporter) + .to receive(:execute) + .and_return(false) + + expect(Gitlab::GithubImport::Stage::ImportBaseDataWorker) + .not_to receive(:perform_async) + + worker.import(client, project) + end + end + end +end diff --git a/spec/workers/repository_import_worker_spec.rb b/spec/workers/repository_import_worker_spec.rb index 5cff5108477..0af537647ad 100644 --- a/spec/workers/repository_import_worker_spec.rb +++ b/spec/workers/repository_import_worker_spec.rb @@ -59,5 +59,28 @@ describe RepositoryImportWorker do expect(project.reload.import_status).to eq('failed') end end + + context 'when using an asynchronous importer' do + it 'does not mark the import process as finished' do + service = double(:service) + + allow(Projects::ImportService) + .to receive(:new) + .and_return(service) + + allow(service) + .to receive(:execute) + .and_return(true) + + allow(service) + .to receive(:async?) + .and_return(true) + + expect_any_instance_of(Project) + .not_to receive(:import_finish) + + subject.perform(project.id) + end + end end end -- cgit v1.2.1 From bae6385bda13f1db0083ef834aedcc89424e0130 Mon Sep 17 00:00:00 2001 From: "micael.bergeron" Date: Mon, 13 Nov 2017 14:56:08 -0500 Subject: add simple logging to UpdateMergeRequestsWorker#perform this is to try to debug #35914 --- spec/workers/update_merge_requests_worker_spec.rb | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'spec/workers') diff --git a/spec/workers/update_merge_requests_worker_spec.rb b/spec/workers/update_merge_requests_worker_spec.rb index 558ff9109ec..5c711399cd5 100644 --- a/spec/workers/update_merge_requests_worker_spec.rb +++ b/spec/workers/update_merge_requests_worker_spec.rb @@ -23,5 +23,11 @@ describe UpdateMergeRequestsWorker do perform end + + it 'logs performance' do + expect(Rails.logger).to receive(:info).with(a_string_matching(/\AUpdateMergeRequestsWorker#perform.*project_id=#{project.id},user_id=#{user.id},oldrev=#{oldrev},newrev=#{newrev},ref=#{ref}/)) + + perform + end end end -- cgit v1.2.1 From 0b4a7d656b224c145072946f06539fcff40cc8ca Mon Sep 17 00:00:00 2001 From: "micael.bergeron" Date: Tue, 14 Nov 2017 09:22:15 -0500 Subject: rework the logging to be simpler and add a threshold --- spec/workers/update_merge_requests_worker_spec.rb | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) (limited to 'spec/workers') diff --git a/spec/workers/update_merge_requests_worker_spec.rb b/spec/workers/update_merge_requests_worker_spec.rb index 5c711399cd5..0fa19ac84bb 100644 --- a/spec/workers/update_merge_requests_worker_spec.rb +++ b/spec/workers/update_merge_requests_worker_spec.rb @@ -24,10 +24,16 @@ describe UpdateMergeRequestsWorker do perform end - it 'logs performance' do - expect(Rails.logger).to receive(:info).with(a_string_matching(/\AUpdateMergeRequestsWorker#perform.*project_id=#{project.id},user_id=#{user.id},oldrev=#{oldrev},newrev=#{newrev},ref=#{ref}/)) + context 'when slow' do + before do + stub_const("UpdateMergeRequestsWorker::LOG_TIME_THRESHOLD", -1) + end - perform + it 'logs debug info' do + expect(Rails.logger).to receive(:info).with(a_string_matching(/\AUpdateMergeRequestsWorker#perform.*project_id=#{project.id},user_id=#{user.id},oldrev=#{oldrev},newrev=#{newrev},ref=#{ref}/)) + + perform + end end end end -- cgit v1.2.1 From 9d372d0da6719a10c41a7021356961e00f9dd23e Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 20 Nov 2017 18:29:59 +0900 Subject: Add a test --- spec/workers/pipeline_schedule_worker_spec.rb | 31 ++++++++++++++++----------- 1 file changed, 19 insertions(+), 12 deletions(-) (limited to 'spec/workers') diff --git a/spec/workers/pipeline_schedule_worker_spec.rb b/spec/workers/pipeline_schedule_worker_spec.rb index 75197039f5a..88b740b7a12 100644 --- a/spec/workers/pipeline_schedule_worker_spec.rb +++ b/spec/workers/pipeline_schedule_worker_spec.rb @@ -22,25 +22,32 @@ describe PipelineScheduleWorker do end context 'when there is a scheduled pipeline within next_run_at' do - it 'creates a new pipeline' do - expect { subject }.to change { project.pipelines.count }.by(1) - expect(Ci::Pipeline.last).to be_schedule + shared_examples 'successful scheduling' do + it 'creates a new pipeline' do + expect { subject }.to change { project.pipelines.count }.by(1) + + pipeline_schedule.reload + expect(Ci::Pipeline.last).to be_schedule + expect(pipeline_schedule.next_run_at).to be > Time.now + expect(pipeline_schedule).to eq(project.pipelines.last.pipeline_schedule) + expect(pipeline_schedule).to be_active + end end - it 'updates the next_run_at field' do - subject + it_behaves_like 'successful scheduling' - expect(pipeline_schedule.reload.next_run_at).to be > Time.now - end - - it 'sets the schedule on the pipeline' do - subject + context 'when the latest commit contains [ci skip]' do + before do + allow_any_instance_of(Ci::Pipeline) + .to receive(:git_commit_message) + .and_return('some commit [ci skip]') + end - expect(project.pipelines.last.pipeline_schedule).to eq(pipeline_schedule) + it_behaves_like 'successful scheduling' end end - context 'inactive schedule' do + context 'when the schedule is deactivated' do before do pipeline_schedule.deactivate! end -- cgit v1.2.1 From 56112b0ae0f6a43335a2fbbd1194c14776377ca0 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 23 Nov 2017 02:55:10 +0900 Subject: Fix testing order --- spec/workers/pipeline_schedule_worker_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/workers') diff --git a/spec/workers/pipeline_schedule_worker_spec.rb b/spec/workers/pipeline_schedule_worker_spec.rb index 88b740b7a12..e7a4ac0f3d6 100644 --- a/spec/workers/pipeline_schedule_worker_spec.rb +++ b/spec/workers/pipeline_schedule_worker_spec.rb @@ -25,9 +25,9 @@ describe PipelineScheduleWorker do shared_examples 'successful scheduling' do it 'creates a new pipeline' do expect { subject }.to change { project.pipelines.count }.by(1) + expect(Ci::Pipeline.last).to be_schedule pipeline_schedule.reload - expect(Ci::Pipeline.last).to be_schedule expect(pipeline_schedule.next_run_at).to be > Time.now expect(pipeline_schedule).to eq(project.pipelines.last.pipeline_schedule) expect(pipeline_schedule).to be_active -- cgit v1.2.1 From 65bd6868d014e23c21e4d5ecff468124b2c72f4c Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Wed, 22 Nov 2017 06:35:53 +0100 Subject: Codestyle changes and Added Exclusive Lease to hashed storage migration --- .../project_migrate_hashed_storage_worker_spec.rb | 44 +++++++++++++++------- 1 file changed, 31 insertions(+), 13 deletions(-) (limited to 'spec/workers') diff --git a/spec/workers/project_migrate_hashed_storage_worker_spec.rb b/spec/workers/project_migrate_hashed_storage_worker_spec.rb index f5226dee0ad..8cacdfa7173 100644 --- a/spec/workers/project_migrate_hashed_storage_worker_spec.rb +++ b/spec/workers/project_migrate_hashed_storage_worker_spec.rb @@ -5,25 +5,43 @@ describe ProjectMigrateHashedStorageWorker do let(:project) { create(:project, :empty_repo) } let(:pending_delete_project) { create(:project, :empty_repo, pending_delete: true) } - it 'skips when project no longer exists' do - nonexistent_id = 999999999999 + context 'when have exclusive lease' do + before do + allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain).and_return(true) + end - expect(::Projects::HashedStorageMigrationService).not_to receive(:new) - subject.perform(nonexistent_id) - end + it 'skips when project no longer exists' do + nonexistent_id = 999999999999 + + expect(::Projects::HashedStorageMigrationService).not_to receive(:new) + subject.perform(nonexistent_id) + end + + it 'skips when project is pending delete' do + expect(::Projects::HashedStorageMigrationService).not_to receive(:new) - it 'skips when project is pending delete' do - expect(::Projects::HashedStorageMigrationService).not_to receive(:new) + subject.perform(pending_delete_project.id) + end - subject.perform(pending_delete_project.id) + it 'delegates removal to service class' do + service = double('service') + expect(::Projects::HashedStorageMigrationService).to receive(:new).with(project, subject.logger).and_return(service) + expect(service).to receive(:execute) + + subject.perform(project.id) + end end - it 'delegates removal to service class' do - service = double('service') - expect(::Projects::HashedStorageMigrationService).to receive(:new).with(project, subject.logger).and_return(service) - expect(service).to receive(:execute) + context 'when dont have exclusive lease' do + before do + allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain).and_return(false) + end + + it 'skips when dont have lease' do + expect(::Projects::HashedStorageMigrationService).not_to receive(:new) - subject.perform(project.id) + subject.perform(project.id) + end end end end -- cgit v1.2.1 From 05876a49c67fd94777801c09129e7dd77873e668 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Wed, 22 Nov 2017 16:29:03 +0100 Subject: fix exclusive lease specs fro hashed storage migration --- spec/workers/project_migrate_hashed_storage_worker_spec.rb | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) (limited to 'spec/workers') diff --git a/spec/workers/project_migrate_hashed_storage_worker_spec.rb b/spec/workers/project_migrate_hashed_storage_worker_spec.rb index 8cacdfa7173..2e3951e7afc 100644 --- a/spec/workers/project_migrate_hashed_storage_worker_spec.rb +++ b/spec/workers/project_migrate_hashed_storage_worker_spec.rb @@ -1,13 +1,16 @@ require 'spec_helper' -describe ProjectMigrateHashedStorageWorker do +describe ProjectMigrateHashedStorageWorker, :clean_gitlab_redis_shared_state do describe '#perform' do let(:project) { create(:project, :empty_repo) } let(:pending_delete_project) { create(:project, :empty_repo, pending_delete: true) } context 'when have exclusive lease' do before do - allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain).and_return(true) + lease = subject.lease_for(project.id) + + allow(Gitlab::ExclusiveLease).to receive(:new).and_return(lease) + allow(lease).to receive(:try_obtain).and_return(true) end it 'skips when project no longer exists' do @@ -34,7 +37,10 @@ describe ProjectMigrateHashedStorageWorker do context 'when dont have exclusive lease' do before do - allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain).and_return(false) + lease = subject.lease_for(project.id) + + allow(Gitlab::ExclusiveLease).to receive(:new).and_return(lease) + allow(lease).to receive(:try_obtain).and_return(false) end it 'skips when dont have lease' do -- cgit v1.2.1 From 07c7ba1bf4a1d0092d07a23e23caf698512d46e0 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 24 Nov 2017 16:30:08 +0100 Subject: Allow to drop jobs for deleted projects --- spec/workers/stuck_ci_jobs_worker_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec/workers') diff --git a/spec/workers/stuck_ci_jobs_worker_spec.rb b/spec/workers/stuck_ci_jobs_worker_spec.rb index ac6f4fefb4e..7f9545177bc 100644 --- a/spec/workers/stuck_ci_jobs_worker_spec.rb +++ b/spec/workers/stuck_ci_jobs_worker_spec.rb @@ -105,8 +105,8 @@ describe StuckCiJobsWorker do job.project.update(pending_delete: true) end - it 'does not drop job' do - expect_any_instance_of(Ci::Build).not_to receive(:drop) + it 'does drop job' do + expect_any_instance_of(Ci::Build).to receive(:drop) worker.perform end end -- cgit v1.2.1 From 6c1a4294209157b43db82678d34e3cba7d2cba3a Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 27 Nov 2017 11:56:06 +0100 Subject: Fix stuck jobs tests --- spec/workers/stuck_ci_jobs_worker_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'spec/workers') diff --git a/spec/workers/stuck_ci_jobs_worker_spec.rb b/spec/workers/stuck_ci_jobs_worker_spec.rb index 7f9545177bc..bdc64c6785b 100644 --- a/spec/workers/stuck_ci_jobs_worker_spec.rb +++ b/spec/workers/stuck_ci_jobs_worker_spec.rb @@ -106,7 +106,7 @@ describe StuckCiJobsWorker do end it 'does drop job' do - expect_any_instance_of(Ci::Build).to receive(:drop) + expect_any_instance_of(Ci::Build).to receive(:drop).and_call_original worker.perform end end @@ -117,7 +117,7 @@ describe StuckCiJobsWorker do let(:worker2) { described_class.new } it 'is guard by exclusive lease when executed concurrently' do - expect(worker).to receive(:drop).at_least(:once) + expect(worker).to receive(:drop).at_least(:once).and_call_original expect(worker2).not_to receive(:drop) worker.perform allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain).and_return(false) @@ -125,8 +125,8 @@ describe StuckCiJobsWorker do end it 'can be executed in sequence' do - expect(worker).to receive(:drop).at_least(:once) - expect(worker2).to receive(:drop).at_least(:once) + expect(worker).to receive(:drop).at_least(:once).and_call_original + expect(worker2).to receive(:drop).at_least(:once).and_call_original worker.perform worker2.perform end -- cgit v1.2.1 From 1d7e3ef1a55991e057213add556926eb13e0bd48 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 27 Nov 2017 21:48:54 +0900 Subject: Duplicate spec for Platform::Kubernetes with kubernetes_project --- spec/workers/reactive_caching_worker_spec.rb | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) (limited to 'spec/workers') diff --git a/spec/workers/reactive_caching_worker_spec.rb b/spec/workers/reactive_caching_worker_spec.rb index 5f4453c15d6..98282af2d18 100644 --- a/spec/workers/reactive_caching_worker_spec.rb +++ b/spec/workers/reactive_caching_worker_spec.rb @@ -1,15 +1,29 @@ require 'spec_helper' describe ReactiveCachingWorker do - let(:project) { create(:kubernetes_project) } let(:service) { project.deployment_service } subject { described_class.new.perform("KubernetesService", service.id) } describe '#perform' do - it 'calls #exclusively_update_reactive_cache!' do - expect_any_instance_of(KubernetesService).to receive(:exclusively_update_reactive_cache!) + shared_examples 'correct behavior with perform' do + it 'calls #exclusively_update_reactive_cache!' do + expect_any_instance_of(KubernetesService).to receive(:exclusively_update_reactive_cache!) - subject + subject + end + end + + context 'when user configured kubernetes from Integration > Kubernetes' do + let(:project) { create(:kubernetes_project) } + + it_behaves_like 'correct behavior with perform' + end + + context 'when user configured kubernetes from CI/CD > Clusters' do + let!(:cluster) { create(:cluster, :project, :provided_by_gcp) } + let(:project) { cluster.project } + + it_behaves_like 'correct behavior with perform' end end end -- cgit v1.2.1 From 53da3d976f3705a87edc50dca41748b5e479fc83 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 27 Nov 2017 22:35:16 +0900 Subject: Replce kubernetes_service and deployment_service to deployment_platform --- spec/workers/reactive_caching_worker_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/workers') diff --git a/spec/workers/reactive_caching_worker_spec.rb b/spec/workers/reactive_caching_worker_spec.rb index 98282af2d18..dd654c941bc 100644 --- a/spec/workers/reactive_caching_worker_spec.rb +++ b/spec/workers/reactive_caching_worker_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe ReactiveCachingWorker do - let(:service) { project.deployment_service } + let(:service) { project.deployment_platform } subject { described_class.new.perform("KubernetesService", service.id) } describe '#perform' do -- cgit v1.2.1 From c36d7842da24e6726705199f178c1324c634bdaf Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 27 Nov 2017 23:19:16 +0900 Subject: Aling shared_exmaples to "same behavior between KubernetesService and Platform::Kubernetes" --- spec/workers/reactive_caching_worker_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'spec/workers') diff --git a/spec/workers/reactive_caching_worker_spec.rb b/spec/workers/reactive_caching_worker_spec.rb index dd654c941bc..225d3e38542 100644 --- a/spec/workers/reactive_caching_worker_spec.rb +++ b/spec/workers/reactive_caching_worker_spec.rb @@ -5,7 +5,7 @@ describe ReactiveCachingWorker do subject { described_class.new.perform("KubernetesService", service.id) } describe '#perform' do - shared_examples 'correct behavior with perform' do + shared_examples 'same behavior between KubernetesService and Platform::Kubernetes' do it 'calls #exclusively_update_reactive_cache!' do expect_any_instance_of(KubernetesService).to receive(:exclusively_update_reactive_cache!) @@ -16,14 +16,14 @@ describe ReactiveCachingWorker do context 'when user configured kubernetes from Integration > Kubernetes' do let(:project) { create(:kubernetes_project) } - it_behaves_like 'correct behavior with perform' + it_behaves_like 'same behavior between KubernetesService and Platform::Kubernetes' end context 'when user configured kubernetes from CI/CD > Clusters' do let!(:cluster) { create(:cluster, :project, :provided_by_gcp) } let(:project) { cluster.project } - it_behaves_like 'correct behavior with perform' + it_behaves_like 'same behavior between KubernetesService and Platform::Kubernetes' end end end -- cgit v1.2.1 From f6d9dcf8382a00a5f2ae2100b13774b01f0328bb Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 27 Nov 2017 23:55:25 +0900 Subject: Fix unit tests --- spec/workers/reactive_caching_worker_spec.rb | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) (limited to 'spec/workers') diff --git a/spec/workers/reactive_caching_worker_spec.rb b/spec/workers/reactive_caching_worker_spec.rb index 225d3e38542..3da851de067 100644 --- a/spec/workers/reactive_caching_worker_spec.rb +++ b/spec/workers/reactive_caching_worker_spec.rb @@ -2,28 +2,27 @@ require 'spec_helper' describe ReactiveCachingWorker do let(:service) { project.deployment_platform } - subject { described_class.new.perform("KubernetesService", service.id) } describe '#perform' do - shared_examples 'same behavior between KubernetesService and Platform::Kubernetes' do + context 'when user configured kubernetes from Integration > Kubernetes' do + let(:project) { create(:kubernetes_project) } + it 'calls #exclusively_update_reactive_cache!' do expect_any_instance_of(KubernetesService).to receive(:exclusively_update_reactive_cache!) - subject + described_class.new.perform("KubernetesService", service.id) end end - context 'when user configured kubernetes from Integration > Kubernetes' do - let(:project) { create(:kubernetes_project) } - - it_behaves_like 'same behavior between KubernetesService and Platform::Kubernetes' - end - context 'when user configured kubernetes from CI/CD > Clusters' do let!(:cluster) { create(:cluster, :project, :provided_by_gcp) } let(:project) { cluster.project } - it_behaves_like 'same behavior between KubernetesService and Platform::Kubernetes' + it 'calls #exclusively_update_reactive_cache!' do + expect_any_instance_of(Clusters::Platforms::Kubernetes).to receive(:exclusively_update_reactive_cache!) + + described_class.new.perform("Clusters::Platforms::Kubernetes", service.id) + end end end end -- cgit v1.2.1 From a4a389a0a7314d011275592474bd974924f86735 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 17 Nov 2017 17:24:40 +0100 Subject: BE for automatic pipeline when enabling Auto DevOps Fix https://gitlab.com/gitlab-org/gitlab-ce/issues/38962 --- spec/workers/create_pipeline_worker_spec.rb | 36 +++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 spec/workers/create_pipeline_worker_spec.rb (limited to 'spec/workers') diff --git a/spec/workers/create_pipeline_worker_spec.rb b/spec/workers/create_pipeline_worker_spec.rb new file mode 100644 index 00000000000..02cb0f46cb4 --- /dev/null +++ b/spec/workers/create_pipeline_worker_spec.rb @@ -0,0 +1,36 @@ +require 'spec_helper' + +describe CreatePipelineWorker do + describe '#perform' do + let(:worker) { described_class.new } + + context 'when a project not found' do + it 'does not call the Service' do + expect(Ci::CreatePipelineService).not_to receive(:new) + expect { worker.perform(99, create(:user).id, 'master', :web) }.to raise_error(ActiveRecord::RecordNotFound) + end + end + + context 'when a user not found' do + let(:project) { create(:project) } + + it 'does not call the Service' do + expect(Ci::CreatePipelineService).not_to receive(:new) + expect { worker.perform(project.id, 99, project.default_branch, :web) }.to raise_error(ActiveRecord::RecordNotFound) + end + end + + context 'when everything is ok' do + let(:project) { create(:project) } + let(:user) { create(:user) } + let(:create_pipeline_service) { instance_double(Ci::CreatePipelineService) } + + it 'calls the Service' do + expect(Ci::CreatePipelineService).to receive(:new).with(project, user, ref: project.default_branch).and_return(create_pipeline_service) + expect(create_pipeline_service).to receive(:execute).with(:web, any_args) + + worker.perform(project.id, user.id, project.default_branch, :web) + end + end + end +end -- cgit v1.2.1