diff options
Diffstat (limited to 'spec/lib')
20 files changed, 823 insertions, 120 deletions
diff --git a/spec/lib/gitlab/danger/helper_spec.rb b/spec/lib/gitlab/danger/helper_spec.rb index 32b90041c64..f7642182a17 100644 --- a/spec/lib/gitlab/danger/helper_spec.rb +++ b/spec/lib/gitlab/danger/helper_spec.rb @@ -2,7 +2,6 @@ require 'fast_spec_helper' require 'rspec-parameterized' -require 'webmock/rspec' require 'gitlab/danger/helper' @@ -19,39 +18,6 @@ describe Gitlab::Danger::Helper do end end - let(:teammate_json) do - <<~JSON - [ - { - "username": "in-gitlab-ce", - "name": "CE maintainer", - "projects":{ "gitlab-ce": "maintainer backend" } - }, - { - "username": "in-gitlab-ee", - "name": "EE reviewer", - "projects":{ "gitlab-ee": "reviewer frontend" } - } - ] - JSON - end - - let(:ce_teammate_matcher) do - satisfy do |teammate| - teammate.username == 'in-gitlab-ce' && - teammate.name == 'CE maintainer' && - teammate.projects == { 'gitlab-ce' => 'maintainer backend' } - end - end - - let(:ee_teammate_matcher) do - satisfy do |teammate| - teammate.username == 'in-gitlab-ee' && - teammate.name == 'EE reviewer' && - teammate.projects == { 'gitlab-ee' => 'reviewer frontend' } - end - end - let(:fake_git) { double('fake-git') } subject(:helper) { FakeDanger.new(git: fake_git) } @@ -119,69 +85,6 @@ describe Gitlab::Danger::Helper do end end - describe '#team' do - subject(:team) { helper.team } - - context 'HTTP failure' do - before do - WebMock - .stub_request(:get, 'https://about.gitlab.com/roulette.json') - .to_return(status: 404) - end - - it 'raises a pretty error' do - expect { team }.to raise_error(/Failed to read/) - end - end - - context 'JSON failure' do - before do - WebMock - .stub_request(:get, 'https://about.gitlab.com/roulette.json') - .to_return(body: 'INVALID JSON') - end - - it 'raises a pretty error' do - expect { team }.to raise_error(/Failed to parse/) - end - end - - context 'success' do - before do - WebMock - .stub_request(:get, 'https://about.gitlab.com/roulette.json') - .to_return(body: teammate_json) - end - - it 'returns an array of teammates' do - is_expected.to contain_exactly(ce_teammate_matcher, ee_teammate_matcher) - end - - it 'memoizes the result' do - expect(team.object_id).to eq(helper.team.object_id) - end - end - end - - describe '#project_team' do - subject { helper.project_team } - - before do - WebMock - .stub_request(:get, 'https://about.gitlab.com/roulette.json') - .to_return(body: teammate_json) - end - - it 'filters team by project_name' do - expect(helper) - .to receive(:project_name) - .at_least(:once) - .and_return('gitlab-ce') - - is_expected.to contain_exactly(ce_teammate_matcher) - end - end - describe '#changes_by_category' do it 'categorizes changed files' do expect(fake_git).to receive(:added_files) { %w[foo foo.md foo.rb foo.js db/foo qa/foo ee/changelogs/foo.yml] } diff --git a/spec/lib/gitlab/danger/roulette_spec.rb b/spec/lib/gitlab/danger/roulette_spec.rb new file mode 100644 index 00000000000..40dce0c5378 --- /dev/null +++ b/spec/lib/gitlab/danger/roulette_spec.rb @@ -0,0 +1,101 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'webmock/rspec' + +require 'gitlab/danger/roulette' + +describe Gitlab::Danger::Roulette do + let(:teammate_json) do + <<~JSON + [ + { + "username": "in-gitlab-ce", + "name": "CE maintainer", + "projects":{ "gitlab-ce": "maintainer backend" } + }, + { + "username": "in-gitlab-ee", + "name": "EE reviewer", + "projects":{ "gitlab-ee": "reviewer frontend" } + } + ] + JSON + end + + let(:ce_teammate_matcher) do + satisfy do |teammate| + teammate.username == 'in-gitlab-ce' && + teammate.name == 'CE maintainer' && + teammate.projects == { 'gitlab-ce' => 'maintainer backend' } + end + end + + let(:ee_teammate_matcher) do + satisfy do |teammate| + teammate.username == 'in-gitlab-ee' && + teammate.name == 'EE reviewer' && + teammate.projects == { 'gitlab-ee' => 'reviewer frontend' } + end + end + + subject(:roulette) { Object.new.extend(described_class) } + + describe '#team' do + subject(:team) { roulette.team } + + context 'HTTP failure' do + before do + WebMock + .stub_request(:get, described_class::ROULETTE_DATA_URL) + .to_return(status: 404) + end + + it 'raises a pretty error' do + expect { team }.to raise_error(/Failed to read/) + end + end + + context 'JSON failure' do + before do + WebMock + .stub_request(:get, described_class::ROULETTE_DATA_URL) + .to_return(body: 'INVALID JSON') + end + + it 'raises a pretty error' do + expect { team }.to raise_error(/Failed to parse/) + end + end + + context 'success' do + before do + WebMock + .stub_request(:get, described_class::ROULETTE_DATA_URL) + .to_return(body: teammate_json) + end + + it 'returns an array of teammates' do + is_expected.to contain_exactly(ce_teammate_matcher, ee_teammate_matcher) + end + + it 'memoizes the result' do + expect(team.object_id).to eq(roulette.team.object_id) + end + end + end + + describe '#project_team' do + subject { roulette.project_team('gitlab-ce') } + + before do + WebMock + .stub_request(:get, described_class::ROULETTE_DATA_URL) + .to_return(body: teammate_json) + end + + it 'filters team by project_name' do + is_expected.to contain_exactly(ce_teammate_matcher) + end + end +end diff --git a/spec/lib/gitlab/danger/teammate_spec.rb b/spec/lib/gitlab/danger/teammate_spec.rb index 4bc0a4c1398..753c74ff814 100644 --- a/spec/lib/gitlab/danger/teammate_spec.rb +++ b/spec/lib/gitlab/danger/teammate_spec.rb @@ -1,5 +1,9 @@ # frozen_string_literal: true +require 'fast_spec_helper' + +require 'gitlab/danger/teammate' + describe Gitlab::Danger::Teammate do subject { described_class.new({ 'projects' => projects }) } let(:projects) { { project => capabilities } } @@ -9,15 +13,15 @@ describe Gitlab::Danger::Teammate do let(:capabilities) { ['reviewer backend', 'maintainer frontend', 'trainee_maintainer database'] } it '#reviewer? supports multiple roles per project' do - expect(subject.reviewer?(project, 'backend')).to be_truthy + expect(subject.reviewer?(project, :backend)).to be_truthy end it '#traintainer? supports multiple roles per project' do - expect(subject.traintainer?(project, 'database')).to be_truthy + expect(subject.traintainer?(project, :database)).to be_truthy end it '#maintainer? supports multiple roles per project' do - expect(subject.maintainer?(project, 'frontend')).to be_truthy + expect(subject.maintainer?(project, :frontend)).to be_truthy end end @@ -25,15 +29,15 @@ describe Gitlab::Danger::Teammate do let(:capabilities) { 'reviewer backend' } it '#reviewer? supports one role per project' do - expect(subject.reviewer?(project, 'backend')).to be_truthy + expect(subject.reviewer?(project, :backend)).to be_truthy end it '#traintainer? supports one role per project' do - expect(subject.traintainer?(project, 'database')).to be_falsey + expect(subject.traintainer?(project, :database)).to be_falsey end it '#maintainer? supports one role per project' do - expect(subject.maintainer?(project, 'frontend')).to be_falsey + expect(subject.maintainer?(project, :frontend)).to be_falsey end end end diff --git a/spec/lib/gitlab/github_import/parallel_importer_spec.rb b/spec/lib/gitlab/github_import/parallel_importer_spec.rb index f5df38c9aaf..ecab64a372a 100644 --- a/spec/lib/gitlab/github_import/parallel_importer_spec.rb +++ b/spec/lib/gitlab/github_import/parallel_importer_spec.rb @@ -25,18 +25,9 @@ describe Gitlab::GithubImport::ParallelImporter do end it 'sets the JID in Redis' do - expect(Gitlab::SidekiqStatus) - .to receive(:set) - .with("github-importer/#{project.id}", StuckImportJobsWorker::IMPORT_JOBS_EXPIRATION) - .and_call_original + expect(Gitlab::Import::SetAsyncJid).to receive(:set_jid).with(project).and_call_original importer.execute end - - it 'updates the import JID of the project' do - importer.execute - - expect(project.import_state.reload.jid).to eq("github-importer/#{project.id}") - end end end diff --git a/spec/lib/gitlab/import/set_async_jid_spec.rb b/spec/lib/gitlab/import/set_async_jid_spec.rb new file mode 100644 index 00000000000..51397280138 --- /dev/null +++ b/spec/lib/gitlab/import/set_async_jid_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Gitlab::Import::SetAsyncJid do + describe '.set_jid', :clean_gitlab_redis_shared_state do + let(:project) { create(:project, :import_scheduled) } + + it 'sets the JID in Redis' do + expect(Gitlab::SidekiqStatus) + .to receive(:set) + .with("async-import/#{project.id}", StuckImportJobsWorker::IMPORT_JOBS_EXPIRATION) + .and_call_original + + described_class.set_jid(project) + end + + it 'updates the import JID of the project' do + described_class.set_jid(project) + + expect(project.import_state.reload.jid).to eq("async-import/#{project.id}") + end + end +end diff --git a/spec/lib/gitlab/import_sources_spec.rb b/spec/lib/gitlab/import_sources_spec.rb index 94abf9679c4..8060b5d4448 100644 --- a/spec/lib/gitlab/import_sources_spec.rb +++ b/spec/lib/gitlab/import_sources_spec.rb @@ -14,7 +14,8 @@ describe Gitlab::ImportSources do 'Repo by URL' => 'git', 'GitLab export' => 'gitlab_project', 'Gitea' => 'gitea', - 'Manifest file' => 'manifest' + 'Manifest file' => 'manifest', + 'Phabricator' => 'phabricator' } expect(described_class.options).to eq(expected) @@ -35,6 +36,7 @@ describe Gitlab::ImportSources do gitlab_project gitea manifest + phabricator ) expect(described_class.values).to eq(expected) @@ -53,6 +55,7 @@ describe Gitlab::ImportSources do fogbugz gitlab_project gitea + phabricator ) expect(described_class.importer_names).to eq(expected) @@ -70,7 +73,8 @@ describe Gitlab::ImportSources do 'git' => nil, 'gitlab_project' => Gitlab::ImportExport::Importer, 'gitea' => Gitlab::LegacyGithubImport::Importer, - 'manifest' => nil + 'manifest' => nil, + 'phabricator' => Gitlab::PhabricatorImport::Importer } import_sources.each do |name, klass| @@ -91,7 +95,8 @@ describe Gitlab::ImportSources do 'git' => 'Repo by URL', 'gitlab_project' => 'GitLab export', 'gitea' => 'Gitea', - 'manifest' => 'Manifest file' + 'manifest' => 'Manifest file', + 'phabricator' => 'Phabricator' } import_sources.each do |name, title| @@ -102,7 +107,7 @@ describe Gitlab::ImportSources do end describe 'imports_repository? checker' do - let(:allowed_importers) { %w[github gitlab_project bitbucket_server] } + let(:allowed_importers) { %w[github gitlab_project bitbucket_server phabricator] } it 'fails if any importer other than the allowed ones implements this method' do current_importers = described_class.values.select { |kind| described_class.importer(kind).try(:imports_repository?) } diff --git a/spec/lib/gitlab/lets_encrypt/client_spec.rb b/spec/lib/gitlab/lets_encrypt/client_spec.rb index d63a2fbee04..5454d9c1af4 100644 --- a/spec/lib/gitlab/lets_encrypt/client_spec.rb +++ b/spec/lib/gitlab/lets_encrypt/client_spec.rb @@ -5,14 +5,12 @@ require 'spec_helper' describe ::Gitlab::LetsEncrypt::Client do include LetsEncryptHelpers - set(:private_key) { OpenSSL::PKey::RSA.new(4096).to_pem } let(:client) { described_class.new } before do stub_application_setting( lets_encrypt_notification_email: 'myemail@test.example.com', - lets_encrypt_terms_of_service_accepted: true, - lets_encrypt_private_key: private_key + lets_encrypt_terms_of_service_accepted: true ) end @@ -28,6 +26,36 @@ describe ::Gitlab::LetsEncrypt::Client do ) end + it 'generates and stores private key and initialize acme client with it' do + expect(Gitlab::CurrentSettings.lets_encrypt_private_key).to eq(nil) + + subject + + saved_private_key = Gitlab::CurrentSettings.lets_encrypt_private_key + + expect(saved_private_key).to be + expect(Acme::Client).to have_received(:new).with( + hash_including(private_key: eq_pem(saved_private_key)) + ) + end + + context 'when private key is saved in settings' do + let!(:saved_private_key) do + key = OpenSSL::PKey::RSA.new(4096).to_pem + Gitlab::CurrentSettings.current_application_settings.update(lets_encrypt_private_key: key) + key + end + + it 'uses current value of private key' do + subject + + expect(Acme::Client).to have_received(:new).with( + hash_including(private_key: eq_pem(saved_private_key)) + ) + expect(Gitlab::CurrentSettings.lets_encrypt_private_key).to eq(saved_private_key) + end + end + context 'when acme integration is disabled' do before do stub_application_setting(lets_encrypt_terms_of_service_accepted: false) @@ -94,6 +122,18 @@ describe ::Gitlab::LetsEncrypt::Client do context 'when terms of service are accepted' do it { is_expected.to eq(true) } + context "when private_key isn't present and database is read only" do + before do + allow(::Gitlab::Database).to receive(:read_only?).and_return(true) + end + + it 'returns false' do + expect(::Gitlab::CurrentSettings.lets_encrypt_private_key).to eq(nil) + + is_expected.to eq(false) + end + end + context 'when feature flag is disabled' do before do stub_feature_flags(pages_auto_ssl: false) diff --git a/spec/lib/gitlab/phabricator_import/base_worker_spec.rb b/spec/lib/gitlab/phabricator_import/base_worker_spec.rb new file mode 100644 index 00000000000..d46d908a3e3 --- /dev/null +++ b/spec/lib/gitlab/phabricator_import/base_worker_spec.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Gitlab::PhabricatorImport::BaseWorker do + let(:subclass) do + # Creating an anonymous class for a worker is complicated, as we generate the + # queue name from the class name. + Gitlab::PhabricatorImport::ImportTasksWorker + end + + describe '.schedule' do + let(:arguments) { %w[project_id the_next_page] } + + it 'schedules the job' do + expect(subclass).to receive(:perform_async).with(*arguments) + + subclass.schedule(*arguments) + end + + it 'counts the scheduled job', :clean_gitlab_redis_shared_state do + state = Gitlab::PhabricatorImport::WorkerState.new('project_id') + + allow(subclass).to receive(:remove_job) # otherwise the job is removed before we saw it + + expect { subclass.schedule(*arguments) }.to change { state.running_count }.by(1) + end + end + + describe '#perform' do + let(:project) { create(:project, :import_started, import_url: "https://a.phab.instance") } + let(:worker) { subclass.new } + let(:state) { Gitlab::PhabricatorImport::WorkerState.new(project.id) } + + before do + allow(worker).to receive(:import) + end + + it 'does not break for a non-existing project' do + expect { worker.perform('not a thing') }.not_to raise_error + end + + it 'does not do anything when the import is not in progress' do + project = create(:project, :import_failed) + + expect(worker).not_to receive(:import) + + worker.perform(project.id) + end + + it 'calls import for the project' do + expect(worker).to receive(:import).with(project, 'other_arg') + + worker.perform(project.id, 'other_arg') + end + + it 'marks the project as imported if there was only one job running' do + worker.perform(project.id) + + expect(project.import_state.reload).to be_finished + end + + it 'does not mark the job as finished when there are more scheduled jobs' do + 2.times { state.add_job } + + worker.perform(project.id) + + expect(project.import_state.reload).to be_in_progress + end + + it 'decrements the job counter' do + expect { worker.perform(project.id) }.to change { state.running_count }.by(-1) + end + end +end diff --git a/spec/lib/gitlab/phabricator_import/cache/map_spec.rb b/spec/lib/gitlab/phabricator_import/cache/map_spec.rb new file mode 100644 index 00000000000..52c7a02219f --- /dev/null +++ b/spec/lib/gitlab/phabricator_import/cache/map_spec.rb @@ -0,0 +1,66 @@ +require 'spec_helper' + +describe Gitlab::PhabricatorImport::Cache::Map, :clean_gitlab_redis_cache do + set(:project) { create(:project) } + let(:redis) { Gitlab::Redis::Cache } + subject(:map) { described_class.new(project) } + + describe '#get_gitlab_model' do + it 'returns nil if there was nothing cached for the phabricator id' do + expect(map.get_gitlab_model('does not exist')).to be_nil + end + + it 'returns the object if it was set in redis' do + issue = create(:issue, project: project) + set_in_redis('exists', issue) + + expect(map.get_gitlab_model('exists')).to eq(issue) + end + + it 'extends the TTL for the cache key' do + set_in_redis('extend', create(:issue, project: project)) do |redis| + redis.expire(cache_key('extend'), 10.seconds.to_i) + end + + map.get_gitlab_model('extend') + + ttl = redis.with { |redis| redis.ttl(cache_key('extend')) } + + expect(ttl).to be > 10.seconds + end + end + + describe '#set_gitlab_model' do + around do |example| + Timecop.freeze { example.run } + end + + it 'sets the class and id in redis with a ttl' do + issue = create(:issue, project: project) + + map.set_gitlab_model(issue, 'it is set') + + set_data, ttl = redis.with do |redis| + redis.pipelined do |p| + p.mapped_hmget(cache_key('it is set'), :classname, :database_id) + p.ttl(cache_key('it is set')) + end + end + + expect(set_data).to eq({ classname: 'Issue', database_id: issue.id.to_s }) + expect(ttl).to be_within(1.second).of(StuckImportJobsWorker::IMPORT_JOBS_EXPIRATION) + end + end + + def set_in_redis(key, object) + redis.with do |redis| + redis.mapped_hmset(cache_key(key), + { classname: object.class, database_id: object.id }) + yield(redis) if block_given? + end + end + + def cache_key(phabricator_id) + subject.__send__(:cache_key_for_phabricator_id, phabricator_id) + end +end diff --git a/spec/lib/gitlab/phabricator_import/conduit/client_spec.rb b/spec/lib/gitlab/phabricator_import/conduit/client_spec.rb new file mode 100644 index 00000000000..542b3cd060f --- /dev/null +++ b/spec/lib/gitlab/phabricator_import/conduit/client_spec.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Gitlab::PhabricatorImport::Conduit::Client do + let(:client) do + described_class.new('https://see-ya-later.phabricator', 'api-token') + end + + describe '#get' do + it 'performs and parses a request' do + params = { some: 'extra', values: %w[are passed] } + stub_valid_request(params) + + response = client.get('test', params: params) + + expect(response).to be_a(Gitlab::PhabricatorImport::Conduit::Response) + expect(response).to be_success + end + + it 'wraps request errors in an `ApiError`' do + stub_timeout + + expect { client.get('test') }.to raise_error(Gitlab::PhabricatorImport::Conduit::ApiError) + end + + it 'raises response error' do + stub_error_response + + expect { client.get('test') } + .to raise_error(Gitlab::PhabricatorImport::Conduit::ResponseError, /has the wrong length/) + end + end + + def stub_valid_request(params = {}) + WebMock.stub_request( + :get, 'https://see-ya-later.phabricator/api/test' + ).with( + body: CGI.unescape(params.reverse_merge('api.token' => 'api-token').to_query) + ).and_return( + status: 200, + body: fixture_file('phabricator_responses/maniphest.search.json') + ) + end + + def stub_timeout + WebMock.stub_request( + :get, 'https://see-ya-later.phabricator/api/test' + ).to_timeout + end + + def stub_error_response + WebMock.stub_request( + :get, 'https://see-ya-later.phabricator/api/test' + ).and_return( + status: 200, + body: fixture_file('phabricator_responses/auth_failed.json') + ) + end +end diff --git a/spec/lib/gitlab/phabricator_import/conduit/maniphest_spec.rb b/spec/lib/gitlab/phabricator_import/conduit/maniphest_spec.rb new file mode 100644 index 00000000000..0d7714649b9 --- /dev/null +++ b/spec/lib/gitlab/phabricator_import/conduit/maniphest_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Gitlab::PhabricatorImport::Conduit::Maniphest do + let(:maniphest) do + described_class.new(phabricator_url: 'https://see-ya-later.phabricator', api_token: 'api-token') + end + + describe '#tasks' do + let(:fake_client) { double('Phabricator client') } + + before do + allow(maniphest).to receive(:client).and_return(fake_client) + end + + it 'calls the api with the correct params' do + expected_params = { + after: '123', + attachments: { + projects: 1, subscribers: 1, columns: 1 + } + } + + expect(fake_client).to receive(:get).with('maniphest.search', + params: expected_params) + + maniphest.tasks(after: '123') + end + + it 'returns a parsed response' do + response = Gitlab::PhabricatorImport::Conduit::Response + .new(fixture_file('phabricator_responses/maniphest.search.json')) + + allow(fake_client).to receive(:get).and_return(response) + + expect(maniphest.tasks).to be_a(Gitlab::PhabricatorImport::Conduit::TasksResponse) + end + end +end diff --git a/spec/lib/gitlab/phabricator_import/conduit/response_spec.rb b/spec/lib/gitlab/phabricator_import/conduit/response_spec.rb new file mode 100644 index 00000000000..a8596968f14 --- /dev/null +++ b/spec/lib/gitlab/phabricator_import/conduit/response_spec.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Gitlab::PhabricatorImport::Conduit::Response do + let(:response) { described_class.new(JSON.parse(fixture_file('phabricator_responses/maniphest.search.json')))} + let(:error_response) { described_class.new(JSON.parse(fixture_file('phabricator_responses/auth_failed.json'))) } + + describe '.parse!' do + it 'raises a ResponseError if the http response was not successfull' do + fake_response = double(:http_response, success?: false, status: 401) + + expect { described_class.parse!(fake_response) } + .to raise_error(Gitlab::PhabricatorImport::Conduit::ResponseError, /responded with 401/) + end + + it 'raises a ResponseError if the response contained a Phabricator error' do + fake_response = double(:http_response, + success?: true, + status: 200, + body: fixture_file('phabricator_responses/auth_failed.json')) + + expect { described_class.parse!(fake_response) } + .to raise_error(Gitlab::PhabricatorImport::Conduit::ResponseError, /ERR-INVALID-AUTH: API token/) + end + + it 'raises a ResponseError if JSON parsing failed' do + fake_response = double(:http_response, + success?: true, + status: 200, + body: 'This is no JSON') + + expect { described_class.parse!(fake_response) } + .to raise_error(Gitlab::PhabricatorImport::Conduit::ResponseError, /unexpected token at/) + end + + it 'returns a parsed response for valid input' do + fake_response = double(:http_response, + success?: true, + status: 200, + body: fixture_file('phabricator_responses/maniphest.search.json')) + + expect(described_class.parse!(fake_response)).to be_a(described_class) + end + end + + describe '#success?' do + it { expect(response).to be_success } + it { expect(error_response).not_to be_success } + end + + describe '#error_code' do + it { expect(error_response.error_code).to eq('ERR-INVALID-AUTH') } + it { expect(response.error_code).to be_nil } + end + + describe '#error_info' do + it 'returns the correct error info' do + expected_message = 'API token "api-token" has the wrong length. API tokens should be 32 characters long.' + + expect(error_response.error_info).to eq(expected_message) + end + + it { expect(response.error_info).to be_nil } + end + + describe '#data' do + it { expect(error_response.data).to be_nil } + it { expect(response.data).to be_an(Array) } + end + + describe '#pagination' do + it { expect(error_response.pagination).to be_nil } + + it 'builds the pagination correctly' do + expect(response.pagination).to be_a(Gitlab::PhabricatorImport::Conduit::Pagination) + expect(response.pagination.next_page).to eq('284') + end + end +end diff --git a/spec/lib/gitlab/phabricator_import/conduit/tasks_response_spec.rb b/spec/lib/gitlab/phabricator_import/conduit/tasks_response_spec.rb new file mode 100644 index 00000000000..4b4c2a6276e --- /dev/null +++ b/spec/lib/gitlab/phabricator_import/conduit/tasks_response_spec.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Gitlab::PhabricatorImport::Conduit::TasksResponse do + let(:conduit_response) do + Gitlab::PhabricatorImport::Conduit::Response + .new(JSON.parse(fixture_file('phabricator_responses/maniphest.search.json'))) + end + + subject(:response) { described_class.new(conduit_response) } + + describe '#pagination' do + it 'delegates to the conduit reponse' do + expect(response.pagination).to eq(conduit_response.pagination) + end + end + + describe '#tasks' do + it 'builds the correct tasks representation' do + tasks = response.tasks + + titles = tasks.map(&:issue_attributes).map { |attrs| attrs[:title] } + + expect(titles).to contain_exactly('Things are slow', 'Things are broken') + end + end +end diff --git a/spec/lib/gitlab/phabricator_import/import_tasks_worker_spec.rb b/spec/lib/gitlab/phabricator_import/import_tasks_worker_spec.rb new file mode 100644 index 00000000000..1e38ef8aaa5 --- /dev/null +++ b/spec/lib/gitlab/phabricator_import/import_tasks_worker_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Gitlab::PhabricatorImport::ImportTasksWorker do + describe '#perform' do + it 'calls the correct importer' do + project = create(:project, :import_started, import_url: "https://the.phab.ulr") + fake_importer = instance_double(Gitlab::PhabricatorImport::Issues::Importer) + + expect(Gitlab::PhabricatorImport::Issues::Importer).to receive(:new).with(project).and_return(fake_importer) + expect(fake_importer).to receive(:execute) + + described_class.new.perform(project.id) + end + end +end diff --git a/spec/lib/gitlab/phabricator_import/importer_spec.rb b/spec/lib/gitlab/phabricator_import/importer_spec.rb new file mode 100644 index 00000000000..bf14010a187 --- /dev/null +++ b/spec/lib/gitlab/phabricator_import/importer_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' + +describe Gitlab::PhabricatorImport::Importer do + it { expect(described_class).to be_async } + + it "acts like it's importing repositories" do + expect(described_class).to be_imports_repository + end + + describe '#execute' do + let(:project) { create(:project, :import_scheduled) } + subject(:importer) { described_class.new(project) } + + it 'sets a custom jid that will be kept up to date' do + expect { importer.execute }.to change { project.import_state.reload.jid } + end + + it 'starts importing tasks' do + expect(Gitlab::PhabricatorImport::ImportTasksWorker).to receive(:schedule).with(project.id) + + importer.execute + end + + it 'marks the import as failed when something goes wrong' do + allow(importer).to receive(:schedule_first_tasks_page).and_raise('Stuff is broken') + + importer.execute + + expect(project.import_state).to be_failed + end + end +end diff --git a/spec/lib/gitlab/phabricator_import/issues/importer_spec.rb b/spec/lib/gitlab/phabricator_import/issues/importer_spec.rb new file mode 100644 index 00000000000..2412cf76f79 --- /dev/null +++ b/spec/lib/gitlab/phabricator_import/issues/importer_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Gitlab::PhabricatorImport::Issues::Importer do + set(:project) { create(:project) } + + let(:response) do + Gitlab::PhabricatorImport::Conduit::TasksResponse.new( + Gitlab::PhabricatorImport::Conduit::Response + .new(JSON.parse(fixture_file('phabricator_responses/maniphest.search.json'))) + ) + end + + subject(:importer) { described_class.new(project, nil) } + + before do + client = instance_double(Gitlab::PhabricatorImport::Conduit::Maniphest) + + allow(client).to receive(:tasks).and_return(response) + allow(importer).to receive(:client).and_return(client) + end + + describe '#execute' do + it 'imports each task in the response' do + response.tasks.each do |task| + task_importer = instance_double(Gitlab::PhabricatorImport::Issues::TaskImporter) + + expect(task_importer).to receive(:execute) + expect(Gitlab::PhabricatorImport::Issues::TaskImporter) + .to receive(:new).with(project, task) + .and_return(task_importer) + end + + importer.execute + end + + it 'schedules the next batch if there is one' do + expect(Gitlab::PhabricatorImport::ImportTasksWorker) + .to receive(:schedule).with(project.id, response.pagination.next_page) + + importer.execute + end + + it 'does not reschedule when there is no next page' do + allow(response.pagination).to receive(:has_next_page?).and_return(false) + + expect(Gitlab::PhabricatorImport::ImportTasksWorker) + .not_to receive(:schedule) + + importer.execute + end + end +end diff --git a/spec/lib/gitlab/phabricator_import/issues/task_importer_spec.rb b/spec/lib/gitlab/phabricator_import/issues/task_importer_spec.rb new file mode 100644 index 00000000000..1625604e754 --- /dev/null +++ b/spec/lib/gitlab/phabricator_import/issues/task_importer_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Gitlab::PhabricatorImport::Issues::TaskImporter do + set(:project) { create(:project) } + let(:task) do + Gitlab::PhabricatorImport::Representation::Task.new( + { + 'phid' => 'the-phid', + 'fields' => { + 'name' => 'Title', + 'description' => { + 'raw' => '# This is markdown\n it can contain more text.' + }, + 'dateCreated' => '1518688921', + 'dateClosed' => '1518789995' + } + } + ) + end + + describe '#execute' do + it 'creates the issue with the expected attributes' do + issue = described_class.new(project, task).execute + + expect(issue.project).to eq(project) + expect(issue).to be_persisted + expect(issue.author).to eq(User.ghost) + expect(issue.title).to eq('Title') + expect(issue.description).to eq('# This is markdown\n it can contain more text.') + expect(issue).to be_closed + expect(issue.created_at).to eq(Time.at(1518688921)) + expect(issue.closed_at).to eq(Time.at(1518789995)) + end + + it 'does not recreate the issue when called multiple times' do + expect { described_class.new(project, task).execute } + .to change { project.issues.reload.size }.from(0).to(1) + expect { described_class.new(project, task).execute } + .not_to change { project.issues.reload.size } + end + + it 'does not trigger a save when the object did not change' do + existing_issue = create(:issue, + task.issue_attributes.merge(author: User.ghost)) + importer = described_class.new(project, task) + allow(importer).to receive(:issue).and_return(existing_issue) + + expect(existing_issue).not_to receive(:save!) + + importer.execute + end + end +end diff --git a/spec/lib/gitlab/phabricator_import/project_creator_spec.rb b/spec/lib/gitlab/phabricator_import/project_creator_spec.rb new file mode 100644 index 00000000000..e9455b866ac --- /dev/null +++ b/spec/lib/gitlab/phabricator_import/project_creator_spec.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Gitlab::PhabricatorImport::ProjectCreator do + let(:user) { create(:user) } + let(:params) do + { path: 'new-phab-import', + phabricator_server_url: 'http://phab.example.com', + api_token: 'the-token' } + end + subject(:creator) { described_class.new(user, params) } + + describe '#execute' do + it 'creates a project correctly and schedule an import' do + expect_next_instance_of(Gitlab::PhabricatorImport::Importer) do |importer| + expect(importer).to receive(:execute) + end + + project = creator.execute + + expect(project).to be_persisted + expect(project).to be_import + expect(project.import_type).to eq('phabricator') + expect(project.import_data.credentials).to match(a_hash_including(api_token: 'the-token')) + expect(project.import_data.data).to match(a_hash_including('phabricator_url' => 'http://phab.example.com')) + expect(project.import_url).to eq(Project::UNKNOWN_IMPORT_URL) + expect(project.namespace).to eq(user.namespace) + end + + context 'when import params are missing' do + let(:params) do + { path: 'new-phab-import', + phabricator_server_url: 'http://phab.example.com', + api_token: '' } + end + + it 'returns nil' do + expect(creator.execute).to be_nil + end + end + + context 'when import params are invalid' do + let(:params) do + { path: 'new-phab-import', + namespace_id: '-1', + phabricator_server_url: 'http://phab.example.com', + api_token: 'the-token' } + end + + it 'returns an unpersisted project' do + project = creator.execute + + expect(project).not_to be_persisted + expect(project).not_to be_valid + end + end + end +end diff --git a/spec/lib/gitlab/phabricator_import/representation/task_spec.rb b/spec/lib/gitlab/phabricator_import/representation/task_spec.rb new file mode 100644 index 00000000000..dfbd8c546eb --- /dev/null +++ b/spec/lib/gitlab/phabricator_import/representation/task_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' + +describe Gitlab::PhabricatorImport::Representation::Task do + subject(:task) do + described_class.new( + { + 'phid' => 'the-phid', + 'fields' => { + 'name' => 'Title'.ljust(257, '.'), # A string padded to 257 chars + 'description' => { + 'raw' => '# This is markdown\n it can contain more text.' + }, + 'dateCreated' => '1518688921', + 'dateClosed' => '1518789995' + } + } + ) + end + + describe '#issue_attributes' do + it 'contains the expected values' do + expected_attributes = { + title: 'Title'.ljust(255, '.'), + description: '# This is markdown\n it can contain more text.', + state: :closed, + created_at: Time.at(1518688921), + closed_at: Time.at(1518789995) + } + + expect(task.issue_attributes).to eq(expected_attributes) + end + end +end diff --git a/spec/lib/gitlab/phabricator_import/worker_state_spec.rb b/spec/lib/gitlab/phabricator_import/worker_state_spec.rb new file mode 100644 index 00000000000..a44947445c9 --- /dev/null +++ b/spec/lib/gitlab/phabricator_import/worker_state_spec.rb @@ -0,0 +1,46 @@ +require 'spec_helper' + +describe Gitlab::PhabricatorImport::WorkerState, :clean_gitlab_redis_shared_state do + subject(:state) { described_class.new('weird-project-id') } + let(:key) { 'phabricator-import/jobs/project-weird-project-id/job-count' } + + describe '#add_job' do + it 'increments the counter for jobs' do + set_value(3) + + expect { state.add_job }.to change { get_value }.from('3').to('4') + end + end + + describe '#remove_job' do + it 'decrements the counter for jobs' do + set_value(3) + + expect { state.remove_job }.to change { get_value }.from('3').to('2') + end + end + + describe '#running_count' do + it 'reads the value' do + set_value(9) + + expect(state.running_count).to eq(9) + end + + it 'returns 0 when nothing was set' do + expect(state.running_count).to eq(0) + end + end + + def set_value(value) + redis.with { |r| r.set(key, value) } + end + + def get_value + redis.with { |r| r.get(key) } + end + + def redis + Gitlab::Redis::SharedState + end +end |