diff options
author | Douwe Maan <douwe@gitlab.com> | 2017-11-08 14:52:39 +0300 |
---|---|---|
committer | Winnie Hellmann <winnie@gitlab.com> | 2017-11-10 01:12:15 +0300 |
commit | d1825985d4bd197ff58c7de6f1a64b7871a48365 (patch) | |
tree | d2f7de8ed87dfca2a8102e4bc713ece048e6994e /spec | |
parent | 0946b638d4231a35c036fe2389daea7a23927740 (diff) |
Merge branch 'github-importer-refactor' into 'master'
Rewrite the GitHub importer to perform work in parallel and greatly improve performance
Closes #33135, #38621, and #39361
See merge request gitlab-org/gitlab-ce!14731
(cherry picked from commit 92249f1ac883c2a861235ec49526cbafca73b362)
Diffstat (limited to 'spec')
78 files changed, 5644 insertions, 207 deletions
diff --git a/spec/controllers/import/github_controller_spec.rb b/spec/controllers/import/github_controller_spec.rb index 45c3fa075ef..9bbd97ec305 100644 --- a/spec/controllers/import/github_controller_spec.rb +++ b/spec/controllers/import/github_controller_spec.rb @@ -21,9 +21,9 @@ describe Import::GithubController do describe "GET callback" do it "updates access token" do token = "asdasd12345" - allow_any_instance_of(Gitlab::GithubImport::Client) + allow_any_instance_of(Gitlab::LegacyGithubImport::Client) .to receive(:get_token).and_return(token) - allow_any_instance_of(Gitlab::GithubImport::Client) + allow_any_instance_of(Gitlab::LegacyGithubImport::Client) .to receive(:github_options).and_return({}) stub_omniauth_provider('github') diff --git a/spec/lib/feature_spec.rb b/spec/lib/feature_spec.rb index 1076c63b5f2..10020511bf8 100644 --- a/spec/lib/feature_spec.rb +++ b/spec/lib/feature_spec.rb @@ -13,6 +13,47 @@ describe Feature do end end + describe '.persisted_names' do + it 'returns the names of the persisted features' do + Feature::FlipperFeature.create!(key: 'foo') + + expect(described_class.persisted_names).to eq(%w[foo]) + end + + it 'returns an empty Array when no features are presisted' do + expect(described_class.persisted_names).to be_empty + end + + it 'caches the feature names when request store is active', :request_store do + Feature::FlipperFeature.create!(key: 'foo') + + expect(Feature::FlipperFeature) + .to receive(:feature_names) + .once + .and_call_original + + 2.times do + expect(described_class.persisted_names).to eq(%w[foo]) + end + end + end + + describe '.persisted?' do + it 'returns true for a persisted feature' do + Feature::FlipperFeature.create!(key: 'foo') + + feature = double(:feature, name: 'foo') + + expect(described_class.persisted?(feature)).to eq(true) + end + + it 'returns false for a feature that is not persisted' do + feature = double(:feature, name: 'foo') + + expect(described_class.persisted?(feature)).to eq(false) + end + end + describe '.all' do let(:features) { Set.new } diff --git a/spec/lib/github/client_spec.rb b/spec/lib/github/client_spec.rb deleted file mode 100644 index b846096fe25..00000000000 --- a/spec/lib/github/client_spec.rb +++ /dev/null @@ -1,34 +0,0 @@ -require 'spec_helper' - -describe Github::Client do - let(:connection) { spy } - let(:rate_limit) { double(get: [false, 1]) } - let(:client) { described_class.new({}) } - let(:results) { double } - let(:response) { double } - - before do - allow(Faraday).to receive(:new).and_return(connection) - allow(Github::RateLimit).to receive(:new).with(connection).and_return(rate_limit) - end - - describe '#get' do - before do - allow(Github::Response).to receive(:new).with(results).and_return(response) - end - - it 'uses a default per_page param' do - expect(connection).to receive(:get).with('/foo', per_page: 100).and_return(results) - - expect(client.get('/foo')).to eq(response) - end - - context 'with per_page given' do - it 'overwrites the default per_page' do - expect(connection).to receive(:get).with('/foo', per_page: 30).and_return(results) - - expect(client.get('/foo', per_page: 30)).to eq(response) - end - end - end -end diff --git a/spec/lib/github/import/legacy_diff_note_spec.rb b/spec/lib/github/import/legacy_diff_note_spec.rb deleted file mode 100644 index 8c50b46cacb..00000000000 --- a/spec/lib/github/import/legacy_diff_note_spec.rb +++ /dev/null @@ -1,9 +0,0 @@ -require 'spec_helper' - -describe Github::Import::LegacyDiffNote do - describe '#type' do - it 'returns the original note type' do - expect(described_class.new.type).to eq('LegacyDiffNote') - end - end -end diff --git a/spec/lib/github/import/note_spec.rb b/spec/lib/github/import/note_spec.rb deleted file mode 100644 index fcdccd9e097..00000000000 --- a/spec/lib/github/import/note_spec.rb +++ /dev/null @@ -1,9 +0,0 @@ -require 'spec_helper' - -describe Github::Import::Note do - describe '#type' do - it 'returns the original note type' do - expect(described_class.new.type).to eq('Note') - end - end -end diff --git a/spec/lib/gitlab/database_spec.rb b/spec/lib/gitlab/database_spec.rb index 7aeb85b8f5a..fcddfad3f9f 100644 --- a/spec/lib/gitlab/database_spec.rb +++ b/spec/lib/gitlab/database_spec.rb @@ -202,6 +202,26 @@ describe Gitlab::Database do it 'handles non-UTF-8 data' do expect { described_class.bulk_insert('test', [{ a: "\255" }]) }.not_to raise_error end + + context 'when using PostgreSQL' do + before do + allow(described_class).to receive(:mysql?).and_return(false) + end + + it 'allows the returning of the IDs of the inserted rows' do + result = double(:result, values: [['10']]) + + expect(connection) + .to receive(:execute) + .with(/RETURNING id/) + .and_return(result) + + ids = described_class + .bulk_insert('test', [{ number: 10 }], return_ids: true) + + expect(ids).to eq([10]) + end + end end describe '.create_connection_pool' do diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 96e162ac087..ee14b528ec2 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -648,6 +648,21 @@ describe Gitlab::Git::Repository, seed_helper: true do end end + describe '#remote_exists?' do + before(:all) do + @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') + @repo.add_remote("new_remote", SeedHelper::GITLAB_GIT_TEST_REPO_URL) + end + + it 'returns true for an existing remote' do + expect(@repo.remote_exists?('new_remote')).to eq(true) + end + + it 'returns false for a non-existing remote' do + expect(@repo.remote_exists?('foo')).to eq(false) + end + end + describe "#log" do let(:commit_with_old_name) do Gitlab::Git::Commit.decorate(repository, @commit_with_old_name_id) diff --git a/spec/lib/gitlab/github_import/bulk_importing_spec.rb b/spec/lib/gitlab/github_import/bulk_importing_spec.rb new file mode 100644 index 00000000000..91229d9c7d4 --- /dev/null +++ b/spec/lib/gitlab/github_import/bulk_importing_spec.rb @@ -0,0 +1,62 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::BulkImporting do + let(:importer) do + Class.new { include(Gitlab::GithubImport::BulkImporting) }.new + end + + describe '#build_database_rows' do + it 'returns an Array containing the rows to insert' do + object = double(:object, title: 'Foo') + + expect(importer) + .to receive(:build) + .with(object) + .and_return({ title: 'Foo' }) + + expect(importer) + .to receive(:already_imported?) + .with(object) + .and_return(false) + + enum = [[object, 1]].to_enum + + expect(importer.build_database_rows(enum)).to eq([{ title: 'Foo' }]) + end + + it 'does not import objects that have already been imported' do + object = double(:object, title: 'Foo') + + expect(importer) + .not_to receive(:build) + + expect(importer) + .to receive(:already_imported?) + .with(object) + .and_return(true) + + enum = [[object, 1]].to_enum + + expect(importer.build_database_rows(enum)).to be_empty + end + end + + describe '#bulk_insert' do + it 'bulk inserts rows into the database' do + rows = [{ title: 'Foo' }] * 10 + model = double(:model, table_name: 'kittens') + + expect(Gitlab::Database) + .to receive(:bulk_insert) + .ordered + .with('kittens', rows.first(5)) + + expect(Gitlab::Database) + .to receive(:bulk_insert) + .ordered + .with('kittens', rows.last(5)) + + importer.bulk_insert(model, rows, batch_size: 5) + end + end +end diff --git a/spec/lib/gitlab/github_import/caching_spec.rb b/spec/lib/gitlab/github_import/caching_spec.rb new file mode 100644 index 00000000000..70ecdc16da1 --- /dev/null +++ b/spec/lib/gitlab/github_import/caching_spec.rb @@ -0,0 +1,117 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Caching, :clean_gitlab_redis_cache do + describe '.read' do + it 'reads a value from the cache' do + described_class.write('foo', 'bar') + + expect(described_class.read('foo')).to eq('bar') + end + + it 'returns nil if the cache key does not exist' do + expect(described_class.read('foo')).to be_nil + end + + it 'refreshes the cache key if a value is present' do + described_class.write('foo', 'bar') + + redis = double(:redis) + + expect(redis).to receive(:get).with(/foo/).and_return('bar') + expect(redis).to receive(:expire).with(/foo/, described_class::TIMEOUT) + expect(Gitlab::Redis::Cache).to receive(:with).twice.and_yield(redis) + + described_class.read('foo') + end + + it 'does not refresh the cache key if a value is empty' do + described_class.write('foo', nil) + + redis = double(:redis) + + expect(redis).to receive(:get).with(/foo/).and_return('') + expect(redis).not_to receive(:expire) + expect(Gitlab::Redis::Cache).to receive(:with).and_yield(redis) + + described_class.read('foo') + end + end + + describe '.read_integer' do + it 'returns an Integer' do + described_class.write('foo', '10') + + expect(described_class.read_integer('foo')).to eq(10) + end + + it 'returns nil if no value was found' do + expect(described_class.read_integer('foo')).to be_nil + end + end + + describe '.write' do + it 'writes a value to the cache and returns the written value' do + expect(described_class.write('foo', 10)).to eq(10) + expect(described_class.read('foo')).to eq('10') + end + end + + describe '.set_add' do + it 'adds a value to a set' do + described_class.set_add('foo', 10) + described_class.set_add('foo', 10) + + key = described_class.cache_key_for('foo') + values = Gitlab::Redis::Cache.with { |r| r.smembers(key) } + + expect(values).to eq(['10']) + end + end + + describe '.set_includes?' do + it 'returns false when the key does not exist' do + expect(described_class.set_includes?('foo', 10)).to eq(false) + end + + it 'returns false when the value is not present in the set' do + described_class.set_add('foo', 10) + + expect(described_class.set_includes?('foo', 20)).to eq(false) + end + + it 'returns true when the set includes the given value' do + described_class.set_add('foo', 10) + + expect(described_class.set_includes?('foo', 10)).to eq(true) + end + end + + describe '.write_multiple' do + it 'sets multiple keys' do + mapping = { 'foo' => 10, 'bar' => 20 } + + described_class.write_multiple(mapping) + + mapping.each do |key, value| + full_key = described_class.cache_key_for(key) + found = Gitlab::Redis::Cache.with { |r| r.get(full_key) } + + expect(found).to eq(value.to_s) + end + end + end + + describe '.expire' do + it 'sets the expiration time of a key' do + timeout = 1.hour.to_i + + described_class.write('foo', 'bar', timeout: 2.hours.to_i) + described_class.expire('foo', timeout) + + key = described_class.cache_key_for('foo') + found_ttl = Gitlab::Redis::Cache.with { |r| r.ttl(key) } + + expect(found_ttl).to be <= timeout + end + end +end diff --git a/spec/lib/gitlab/github_import/client_spec.rb b/spec/lib/gitlab/github_import/client_spec.rb index 66273255b6f..6bbdf6bf3b6 100644 --- a/spec/lib/gitlab/github_import/client_spec.rb +++ b/spec/lib/gitlab/github_import/client_spec.rb @@ -1,97 +1,286 @@ require 'spec_helper' describe Gitlab::GithubImport::Client do - let(:token) { '123456' } - let(:github_provider) { Settingslogic.new('app_id' => 'asd123', 'app_secret' => 'asd123', 'name' => 'github', 'args' => { 'client_options' => {} }) } + describe '#parallel?' do + it 'returns true when the client is running in parallel mode' do + client = described_class.new('foo', parallel: true) - subject(:client) { described_class.new(token) } + expect(client).to be_parallel + end + + it 'returns false when the client is running in sequential mode' do + client = described_class.new('foo', parallel: false) - before do - allow(Gitlab.config.omniauth).to receive(:providers).and_return([github_provider]) + expect(client).not_to be_parallel + end end - it 'convert OAuth2 client options to symbols' do - client.client.options.keys.each do |key| - expect(key).to be_kind_of(Symbol) + describe '#user' do + it 'returns the details for the given username' do + client = described_class.new('foo') + + expect(client.octokit).to receive(:user).with('foo') + expect(client).to receive(:with_rate_limit).and_yield + + client.user('foo') end end - it 'does not crash (e.g. Settingslogic::MissingSetting) when verify_ssl config is not present' do - expect { client.api }.not_to raise_error + describe '#repository' do + it 'returns the details of a repository' do + client = described_class.new('foo') + + expect(client.octokit).to receive(:repo).with('foo/bar') + expect(client).to receive(:with_rate_limit).and_yield + + client.repository('foo/bar') + end end - context 'when config is missing' do - before do - allow(Gitlab.config.omniauth).to receive(:providers).and_return([]) + describe '#labels' do + it 'returns the labels' do + client = described_class.new('foo') + + expect(client) + .to receive(:each_object) + .with(:labels, 'foo/bar') + + client.labels('foo/bar') end + end + + describe '#milestones' do + it 'returns the milestones' do + client = described_class.new('foo') + + expect(client) + .to receive(:each_object) + .with(:milestones, 'foo/bar') - it 'is still possible to get an Octokit client' do - expect { client.api }.not_to raise_error + client.milestones('foo/bar') end + end + + describe '#releases' do + it 'returns the releases' do + client = described_class.new('foo') - it 'is not be possible to get an OAuth2 client' do - expect { client.client }.to raise_error(Projects::ImportService::Error) + expect(client) + .to receive(:each_object) + .with(:releases, 'foo/bar') + + client.releases('foo/bar') end end - context 'allow SSL verification to be configurable on API' do + describe '#each_page' do + let(:client) { described_class.new('foo') } + let(:object1) { double(:object1) } + let(:object2) { double(:object2) } + before do - github_provider['verify_ssl'] = false - end + allow(client) + .to receive(:with_rate_limit) + .and_yield + + allow(client.octokit) + .to receive(:public_send) + .and_return([object1]) + + response = double(:response, data: [object2], rels: { next: nil }) + next_page = double(:next_page, get: response) - it 'uses supplied value' do - expect(client.client.options[:connection_opts][:ssl]).to eq({ verify: false }) - expect(client.api.connection_options[:ssl]).to eq({ verify: false }) + allow(client.octokit) + .to receive(:last_response) + .and_return(double(:last_response, rels: { next: next_page })) end - end - describe '#api_endpoint' do - context 'when provider does not specity an API endpoint' do - it 'uses GitHub root API endpoint' do - expect(client.api.api_endpoint).to eq 'https://api.github.com/' + context 'without a block' do + it 'returns an Enumerator' do + expect(client.each_page(:foo)).to be_an_instance_of(Enumerator) + end + + it 'the returned Enumerator returns Page objects' do + enum = client.each_page(:foo) + + page1 = enum.next + page2 = enum.next + + expect(page1).to be_an_instance_of(described_class::Page) + expect(page2).to be_an_instance_of(described_class::Page) + + expect(page1.objects).to eq([object1]) + expect(page1.number).to eq(1) + + expect(page2.objects).to eq([object2]) + expect(page2.number).to eq(2) end end - context 'when provider specify a custom API endpoint' do - before do - github_provider['args']['client_options']['site'] = 'https://github.company.com/' + context 'with a block' do + it 'yields every retrieved page to the supplied block' do + pages = [] + + client.each_page(:foo) { |page| pages << page } + + expect(pages[0]).to be_an_instance_of(described_class::Page) + expect(pages[1]).to be_an_instance_of(described_class::Page) + + expect(pages[0].objects).to eq([object1]) + expect(pages[0].number).to eq(1) + + expect(pages[1].objects).to eq([object2]) + expect(pages[1].number).to eq(2) end - it 'uses the custom API endpoint' do - expect(OmniAuth::Strategies::GitHub).not_to receive(:default_options) - expect(client.api.api_endpoint).to eq 'https://github.company.com/' + it 'starts at the given page' do + pages = [] + + client.each_page(:foo, page: 2) { |page| pages << page } + + expect(pages[0].number).to eq(2) + expect(pages[1].number).to eq(3) end end + end - context 'when given a host' do - subject(:client) { described_class.new(token, host: 'https://try.gitea.io/') } + describe '#with_rate_limit' do + let(:client) { described_class.new('foo') } - it 'builds a endpoint with the given host and the default API version' do - expect(client.api.api_endpoint).to eq 'https://try.gitea.io/api/v3/' - end + it 'yields the supplied block when enough requests remain' do + expect(client).to receive(:requests_remaining?).and_return(true) + + expect { |b| client.with_rate_limit(&b) }.to yield_control end - context 'when given an API version' do - subject(:client) { described_class.new(token, api_version: 'v3') } + it 'waits before yielding if not enough requests remain' do + expect(client).to receive(:requests_remaining?).and_return(false) + expect(client).to receive(:raise_or_wait_for_rate_limit) - it 'does not use the API version without a host' do - expect(client.api.api_endpoint).to eq 'https://api.github.com/' - end + expect { |b| client.with_rate_limit(&b) }.to yield_control end - context 'when given a host and version' do - subject(:client) { described_class.new(token, host: 'https://try.gitea.io/', api_version: 'v3') } + it 'waits and retries the operation if all requests were consumed in the supplied block' do + retries = 0 + + expect(client).to receive(:requests_remaining?).and_return(true) + expect(client).to receive(:raise_or_wait_for_rate_limit) - it 'builds a endpoint with the given options' do - expect(client.api.api_endpoint).to eq 'https://try.gitea.io/api/v3/' + client.with_rate_limit do + if retries.zero? + retries += 1 + raise(Octokit::TooManyRequests) + end end + + expect(retries).to eq(1) + end + + it 'increments the request count counter' do + expect(client.request_count_counter) + .to receive(:increment) + .and_call_original + + expect(client).to receive(:requests_remaining?).and_return(true) + + client.with_rate_limit { } + end + end + + describe '#requests_remaining?' do + let(:client) { described_class.new('foo') } + + it 'returns true if enough requests remain' do + expect(client).to receive(:remaining_requests).and_return(9000) + + expect(client.requests_remaining?).to eq(true) + end + + it 'returns false if not enough requests remain' do + expect(client).to receive(:remaining_requests).and_return(1) + + expect(client.requests_remaining?).to eq(false) + end + end + + describe '#raise_or_wait_for_rate_limit' do + it 'raises RateLimitError when running in parallel mode' do + client = described_class.new('foo', parallel: true) + + expect { client.raise_or_wait_for_rate_limit } + .to raise_error(Gitlab::GithubImport::RateLimitError) + end + + it 'sleeps when running in sequential mode' do + client = described_class.new('foo', parallel: false) + + expect(client).to receive(:rate_limit_resets_in).and_return(1) + expect(client).to receive(:sleep).with(1) + + client.raise_or_wait_for_rate_limit + end + + it 'increments the rate limit counter' do + client = described_class.new('foo', parallel: false) + + expect(client) + .to receive(:rate_limit_resets_in) + .and_return(1) + + expect(client) + .to receive(:sleep) + .with(1) + + expect(client.rate_limit_counter) + .to receive(:increment) + .and_call_original + + client.raise_or_wait_for_rate_limit end end - it 'does not raise error when rate limit is disabled' do - stub_request(:get, /api.github.com/) - allow(client.api).to receive(:rate_limit!).and_raise(Octokit::NotFound) + describe '#remaining_requests' do + it 'returns the number of remaining requests' do + client = described_class.new('foo') + rate_limit = double(remaining: 1) + + expect(client.octokit).to receive(:rate_limit).and_return(rate_limit) + expect(client.remaining_requests).to eq(1) + end + end + + describe '#rate_limit_resets_in' do + it 'returns the number of seconds after which the rate limit is reset' do + client = described_class.new('foo') + rate_limit = double(resets_in: 1) + + expect(client.octokit).to receive(:rate_limit).and_return(rate_limit) + + expect(client.rate_limit_resets_in).to eq(6) + end + end - expect { client.issues {} }.not_to raise_error + describe '#method_missing' do + it 'delegates missing methods to the request method' do + client = described_class.new('foo') + + expect(client).to receive(:milestones).with(state: 'all') + + client.milestones(state: 'all') + end + end + + describe '#respond_to_missing?' do + it 'returns true for methods supported by Octokit' do + client = described_class.new('foo') + + expect(client.respond_to?(:milestones)).to eq(true) + end + + it 'returns false for methods not supported by Octokit' do + client = described_class.new('foo') + + expect(client.respond_to?(:kittens)).to eq(false) + end end end diff --git a/spec/lib/gitlab/github_import/importer/diff_note_importer_spec.rb b/spec/lib/gitlab/github_import/importer/diff_note_importer_spec.rb new file mode 100644 index 00000000000..1568c657a1e --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/diff_note_importer_spec.rb @@ -0,0 +1,152 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Importer::DiffNoteImporter do + let(:project) { create(:project) } + let(:client) { double(:client) } + let(:user) { create(:user) } + let(:created_at) { Time.new(2017, 1, 1, 12, 00) } + let(:updated_at) { Time.new(2017, 1, 1, 12, 15) } + + let(:hunk) do + '@@ -1 +1 @@ + -Hello + +Hello world' + end + + let(:note) do + Gitlab::GithubImport::Representation::DiffNote.new( + noteable_type: 'MergeRequest', + noteable_id: 1, + commit_id: '123abc', + file_path: 'README.md', + diff_hunk: hunk, + author: Gitlab::GithubImport::Representation::User + .new(id: user.id, login: user.username), + note: 'Hello', + created_at: created_at, + updated_at: updated_at, + github_id: 1 + ) + end + + let(:importer) { described_class.new(note, project, client) } + + describe '#execute' do + context 'when the merge request no longer exists' do + it 'does not import anything' do + expect(Gitlab::Database).not_to receive(:bulk_insert) + + importer.execute + end + end + + context 'when the merge request exists' do + let!(:merge_request) do + create(:merge_request, source_project: project, target_project: project) + end + + before do + allow(importer) + .to receive(:find_merge_request_id) + .and_return(merge_request.id) + end + + it 'imports the note' do + allow(importer.user_finder) + .to receive(:author_id_for) + .and_return([user.id, true]) + + expect(Gitlab::Database) + .to receive(:bulk_insert) + .with( + LegacyDiffNote.table_name, + [ + { + noteable_type: 'MergeRequest', + noteable_id: merge_request.id, + project_id: project.id, + author_id: user.id, + note: 'Hello', + system: false, + commit_id: '123abc', + line_code: note.line_code, + type: 'LegacyDiffNote', + created_at: created_at, + updated_at: updated_at, + st_diff: note.diff_hash.to_yaml + } + ] + ) + .and_call_original + + importer.execute + end + + it 'imports the note when the author could not be found' do + allow(importer.user_finder) + .to receive(:author_id_for) + .and_return([project.creator_id, false]) + + expect(Gitlab::Database) + .to receive(:bulk_insert) + .with( + LegacyDiffNote.table_name, + [ + { + noteable_type: 'MergeRequest', + noteable_id: merge_request.id, + project_id: project.id, + author_id: project.creator_id, + note: "*Created by: #{user.username}*\n\nHello", + system: false, + commit_id: '123abc', + line_code: note.line_code, + type: 'LegacyDiffNote', + created_at: created_at, + updated_at: updated_at, + st_diff: note.diff_hash.to_yaml + } + ] + ) + .and_call_original + + importer.execute + end + + it 'produces a valid LegacyDiffNote' do + allow(importer.user_finder) + .to receive(:author_id_for) + .and_return([user.id, true]) + + importer.execute + + note = project.notes.diff_notes.take + + expect(note).to be_valid + expect(note.diff).to be_an_instance_of(Gitlab::Git::Diff) + end + + it 'does not import the note when a foreign key error is raised' do + allow(importer.user_finder) + .to receive(:author_id_for) + .and_return([project.creator_id, false]) + + expect(Gitlab::Database) + .to receive(:bulk_insert) + .and_raise(ActiveRecord::InvalidForeignKey, 'invalid foreign key') + + expect { importer.execute }.not_to raise_error + end + end + end + + describe '#find_merge_request_id' do + it 'returns a merge request ID' do + expect_any_instance_of(Gitlab::GithubImport::IssuableFinder) + .to receive(:database_id) + .and_return(10) + + expect(importer.find_merge_request_id).to eq(10) + end + end +end diff --git a/spec/lib/gitlab/github_import/importer/diff_notes_importer_spec.rb b/spec/lib/gitlab/github_import/importer/diff_notes_importer_spec.rb new file mode 100644 index 00000000000..4713c6795bb --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/diff_notes_importer_spec.rb @@ -0,0 +1,119 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Importer::DiffNotesImporter do + let(:project) { double(:project, id: 4, import_source: 'foo/bar') } + let(:client) { double(:client) } + + let(:github_comment) do + double( + :response, + html_url: 'https://github.com/foo/bar/pull/42', + path: 'README.md', + commit_id: '123abc', + diff_hunk: "@@ -1 +1 @@\n-Hello\n+Hello world", + user: double(:user, id: 4, login: 'alice'), + body: 'Hello world', + created_at: Time.zone.now, + updated_at: Time.zone.now, + id: 1 + ) + end + + describe '#parallel?' do + it 'returns true when running in parallel mode' do + importer = described_class.new(project, client) + expect(importer).to be_parallel + end + + it 'returns false when running in sequential mode' do + importer = described_class.new(project, client, parallel: false) + expect(importer).not_to be_parallel + end + end + + describe '#execute' do + context 'when running in parallel mode' do + it 'imports diff notes in parallel' do + importer = described_class.new(project, client) + + expect(importer).to receive(:parallel_import) + + importer.execute + end + end + + context 'when running in sequential mode' do + it 'imports diff notes in sequence' do + importer = described_class.new(project, client, parallel: false) + + expect(importer).to receive(:sequential_import) + + importer.execute + end + end + end + + describe '#sequential_import' do + it 'imports each diff note in sequence' do + importer = described_class.new(project, client, parallel: false) + diff_note_importer = double(:diff_note_importer) + + allow(importer) + .to receive(:each_object_to_import) + .and_yield(github_comment) + + expect(Gitlab::GithubImport::Importer::DiffNoteImporter) + .to receive(:new) + .with( + an_instance_of(Gitlab::GithubImport::Representation::DiffNote), + project, + client + ) + .and_return(diff_note_importer) + + expect(diff_note_importer).to receive(:execute) + + importer.sequential_import + end + end + + describe '#parallel_import' do + it 'imports each diff note in parallel' do + importer = described_class.new(project, client) + + allow(importer) + .to receive(:each_object_to_import) + .and_yield(github_comment) + + expect(Gitlab::GithubImport::ImportDiffNoteWorker) + .to receive(:perform_async) + .with(project.id, an_instance_of(Hash), an_instance_of(String)) + + waiter = importer.parallel_import + + expect(waiter).to be_an_instance_of(Gitlab::JobWaiter) + expect(waiter.jobs_remaining).to eq(1) + end + end + + describe '#id_for_already_imported_cache' do + it 'returns the ID of the given note' do + importer = described_class.new(project, client) + + expect(importer.id_for_already_imported_cache(github_comment)) + .to eq(1) + end + end + + describe '#collection_options' do + it 'returns an empty Hash' do + # For large projects (e.g. kubernetes/kubernetes) GitHub's API may produce + # HTTP 500 errors when using explicit sorting options, regardless of what + # order you sort in. Not using any sorting options at all allows us to + # work around this. + importer = described_class.new(project, client) + + expect(importer.collection_options).to eq({}) + end + end +end diff --git a/spec/lib/gitlab/github_import/importer/issue_and_label_links_importer_spec.rb b/spec/lib/gitlab/github_import/importer/issue_and_label_links_importer_spec.rb new file mode 100644 index 00000000000..665b31ef244 --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/issue_and_label_links_importer_spec.rb @@ -0,0 +1,27 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Importer::IssueAndLabelLinksImporter do + describe '#execute' do + it 'imports an issue and its labels' do + issue = double(:issue) + project = double(:project) + client = double(:client) + label_links_instance = double(:label_links_importer) + importer = described_class.new(issue, project, client) + + expect(Gitlab::GithubImport::Importer::IssueImporter) + .to receive(:import_if_issue) + .with(issue, project, client) + + expect(Gitlab::GithubImport::Importer::LabelLinksImporter) + .to receive(:new) + .with(issue, project, client) + .and_return(label_links_instance) + + expect(label_links_instance) + .to receive(:execute) + + importer.execute + end + end +end diff --git a/spec/lib/gitlab/github_import/importer/issue_importer_spec.rb b/spec/lib/gitlab/github_import/importer/issue_importer_spec.rb new file mode 100644 index 00000000000..d34ca0b76b8 --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/issue_importer_spec.rb @@ -0,0 +1,201 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Importer::IssueImporter, :clean_gitlab_redis_cache do + let(:project) { create(:project) } + let(:client) { double(:client) } + let(:user) { create(:user) } + let(:milestone) { create(:milestone, project: project) } + let(:created_at) { Time.new(2017, 1, 1, 12, 00) } + let(:updated_at) { Time.new(2017, 1, 1, 12, 15) } + + let(:issue) do + Gitlab::GithubImport::Representation::Issue.new( + iid: 42, + title: 'My Issue', + description: 'This is my issue', + milestone_number: 1, + state: :opened, + assignees: [ + Gitlab::GithubImport::Representation::User.new(id: 4, login: 'alice'), + Gitlab::GithubImport::Representation::User.new(id: 5, login: 'bob') + ], + label_names: %w[bug], + author: Gitlab::GithubImport::Representation::User.new(id: 4, login: 'alice'), + created_at: created_at, + updated_at: updated_at, + pull_request: false + ) + end + + describe '.import_if_issue' do + it 'imports an issuable if it is a regular issue' do + importer = double(:importer) + + expect(described_class) + .to receive(:new) + .with(issue, project, client) + .and_return(importer) + + expect(importer).to receive(:execute) + + described_class.import_if_issue(issue, project, client) + end + + it 'does not import the issuable if it is a pull request' do + expect(issue).to receive(:pull_request?).and_return(true) + + expect(described_class).not_to receive(:new) + + described_class.import_if_issue(issue, project, client) + end + end + + describe '#execute' do + let(:importer) { described_class.new(issue, project, client) } + + it 'creates the issue and assignees' do + expect(importer) + .to receive(:create_issue) + .and_return(10) + + expect(importer) + .to receive(:create_assignees) + .with(10) + + expect(importer.issuable_finder) + .to receive(:cache_database_id) + .with(10) + + importer.execute + end + end + + describe '#create_issue' do + let(:importer) { described_class.new(issue, project, client) } + + before do + allow(importer.milestone_finder) + .to receive(:id_for) + .with(issue) + .and_return(milestone.id) + end + + context 'when the issue author could be found' do + it 'creates the issue with the found author as the issue author' do + allow(importer.user_finder) + .to receive(:author_id_for) + .with(issue) + .and_return([user.id, true]) + + expect(Gitlab::GithubImport) + .to receive(:insert_and_return_id) + .with( + { + iid: 42, + title: 'My Issue', + author_id: user.id, + project_id: project.id, + description: 'This is my issue', + milestone_id: milestone.id, + state: :opened, + created_at: created_at, + updated_at: updated_at + }, + project.issues + ) + .and_call_original + + importer.create_issue + end + end + + context 'when the issue author could not be found' do + it 'creates the issue with the project creator as the issue author' do + allow(importer.user_finder) + .to receive(:author_id_for) + .with(issue) + .and_return([project.creator_id, false]) + + expect(Gitlab::GithubImport) + .to receive(:insert_and_return_id) + .with( + { + iid: 42, + title: 'My Issue', + author_id: project.creator_id, + project_id: project.id, + description: "*Created by: alice*\n\nThis is my issue", + milestone_id: milestone.id, + state: :opened, + created_at: created_at, + updated_at: updated_at + }, + project.issues + ) + .and_call_original + + importer.create_issue + end + end + + context 'when the import fails due to a foreign key error' do + it 'does not raise any errors' do + allow(importer.user_finder) + .to receive(:author_id_for) + .with(issue) + .and_return([user.id, true]) + + expect(Gitlab::GithubImport) + .to receive(:insert_and_return_id) + .and_raise(ActiveRecord::InvalidForeignKey, 'invalid foreign key') + + expect { importer.create_issue }.not_to raise_error + end + end + + it 'produces a valid Issue' do + allow(importer.user_finder) + .to receive(:author_id_for) + .with(issue) + .and_return([user.id, true]) + + importer.create_issue + + expect(project.issues.take).to be_valid + end + + it 'returns the ID of the created issue' do + allow(importer.user_finder) + .to receive(:author_id_for) + .with(issue) + .and_return([user.id, true]) + + expect(importer.create_issue).to be_a_kind_of(Numeric) + end + end + + describe '#create_assignees' do + it 'inserts the issue assignees in bulk' do + importer = described_class.new(issue, project, client) + + allow(importer.user_finder) + .to receive(:user_id_for) + .ordered.with(issue.assignees[0]) + .and_return(4) + + allow(importer.user_finder) + .to receive(:user_id_for) + .ordered.with(issue.assignees[1]) + .and_return(5) + + expect(Gitlab::Database) + .to receive(:bulk_insert) + .with( + IssueAssignee.table_name, + [{ issue_id: 1, user_id: 4 }, { issue_id: 1, user_id: 5 }] + ) + + importer.create_assignees(1) + end + end +end diff --git a/spec/lib/gitlab/github_import/importer/issues_importer_spec.rb b/spec/lib/gitlab/github_import/importer/issues_importer_spec.rb new file mode 100644 index 00000000000..e237e79e94b --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/issues_importer_spec.rb @@ -0,0 +1,111 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Importer::IssuesImporter do + let(:project) { double(:project, id: 4, import_source: 'foo/bar') } + let(:client) { double(:client) } + let(:created_at) { Time.new(2017, 1, 1, 12, 00) } + let(:updated_at) { Time.new(2017, 1, 1, 12, 15) } + + let(:github_issue) do + double( + :response, + number: 42, + title: 'My Issue', + body: 'This is my issue', + milestone: double(:milestone, number: 4), + state: 'open', + assignees: [double(:user, id: 4, login: 'alice')], + labels: [double(:label, name: 'bug')], + user: double(:user, id: 4, login: 'alice'), + created_at: created_at, + updated_at: updated_at, + pull_request: false + ) + end + + describe '#parallel?' do + it 'returns true when running in parallel mode' do + importer = described_class.new(project, client) + expect(importer).to be_parallel + end + + it 'returns false when running in sequential mode' do + importer = described_class.new(project, client, parallel: false) + expect(importer).not_to be_parallel + end + end + + describe '#execute' do + context 'when running in parallel mode' do + it 'imports issues in parallel' do + importer = described_class.new(project, client) + + expect(importer).to receive(:parallel_import) + + importer.execute + end + end + + context 'when running in sequential mode' do + it 'imports issues in sequence' do + importer = described_class.new(project, client, parallel: false) + + expect(importer).to receive(:sequential_import) + + importer.execute + end + end + end + + describe '#sequential_import' do + it 'imports each issue in sequence' do + importer = described_class.new(project, client, parallel: false) + issue_importer = double(:importer) + + allow(importer) + .to receive(:each_object_to_import) + .and_yield(github_issue) + + expect(Gitlab::GithubImport::Importer::IssueAndLabelLinksImporter) + .to receive(:new) + .with( + an_instance_of(Gitlab::GithubImport::Representation::Issue), + project, + client + ) + .and_return(issue_importer) + + expect(issue_importer).to receive(:execute) + + importer.sequential_import + end + end + + describe '#parallel_import' do + it 'imports each issue in parallel' do + importer = described_class.new(project, client) + + allow(importer) + .to receive(:each_object_to_import) + .and_yield(github_issue) + + expect(Gitlab::GithubImport::ImportIssueWorker) + .to receive(:perform_async) + .with(project.id, an_instance_of(Hash), an_instance_of(String)) + + waiter = importer.parallel_import + + expect(waiter).to be_an_instance_of(Gitlab::JobWaiter) + expect(waiter.jobs_remaining).to eq(1) + end + end + + describe '#id_for_already_imported_cache' do + it 'returns the issue number of the given issue' do + importer = described_class.new(project, client) + + expect(importer.id_for_already_imported_cache(github_issue)) + .to eq(42) + end + end +end diff --git a/spec/lib/gitlab/github_import/importer/label_links_importer_spec.rb b/spec/lib/gitlab/github_import/importer/label_links_importer_spec.rb new file mode 100644 index 00000000000..e2a71e78574 --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/label_links_importer_spec.rb @@ -0,0 +1,82 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Importer::LabelLinksImporter do + let(:project) { create(:project) } + let(:client) { double(:client) } + let(:issue) do + double( + :issue, + iid: 4, + label_names: %w[bug], + issuable_type: Issue, + pull_request?: false + ) + end + + let(:importer) { described_class.new(issue, project, client) } + + describe '#execute' do + it 'creates the label links' do + importer = described_class.new(issue, project, client) + + expect(importer).to receive(:create_labels) + + importer.execute + end + end + + describe '#create_labels' do + it 'inserts the label links in bulk' do + expect(importer.label_finder) + .to receive(:id_for) + .with('bug') + .and_return(2) + + expect(importer) + .to receive(:find_target_id) + .and_return(1) + + Timecop.freeze do + expect(Gitlab::Database) + .to receive(:bulk_insert) + .with( + LabelLink.table_name, + [ + { + label_id: 2, + target_id: 1, + target_type: Issue, + created_at: Time.zone.now, + updated_at: Time.zone.now + } + ] + ) + + importer.create_labels + end + end + + it 'does not insert label links for non-existing labels' do + expect(importer.label_finder) + .to receive(:id_for) + .with('bug') + .and_return(nil) + + expect(Gitlab::Database) + .to receive(:bulk_insert) + .with(LabelLink.table_name, []) + + importer.create_labels + end + end + + describe '#find_target_id' do + it 'returns the ID of the issuable to create the label link for' do + expect_any_instance_of(Gitlab::GithubImport::IssuableFinder) + .to receive(:database_id) + .and_return(10) + + expect(importer.find_target_id).to eq(10) + end + end +end diff --git a/spec/lib/gitlab/github_import/importer/labels_importer_spec.rb b/spec/lib/gitlab/github_import/importer/labels_importer_spec.rb new file mode 100644 index 00000000000..156ef96a0fa --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/labels_importer_spec.rb @@ -0,0 +1,107 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Importer::LabelsImporter, :clean_gitlab_redis_cache do + let(:project) { create(:project, import_source: 'foo/bar') } + let(:client) { double(:client) } + let(:importer) { described_class.new(project, client) } + + describe '#execute' do + it 'imports the labels in bulk' do + label_hash = { title: 'bug', color: '#fffaaa' } + + expect(importer) + .to receive(:build_labels) + .and_return([label_hash]) + + expect(importer) + .to receive(:bulk_insert) + .with(Label, [label_hash]) + + expect(importer) + .to receive(:build_labels_cache) + + importer.execute + end + end + + describe '#build_labels' do + it 'returns an Array containnig label rows' do + label = double(:label, name: 'bug', color: 'ffffff') + + expect(importer).to receive(:each_label).and_return([label]) + + rows = importer.build_labels + + expect(rows.length).to eq(1) + expect(rows[0][:title]).to eq('bug') + end + + it 'does not create labels that already exist' do + create(:label, project: project, title: 'bug') + + label = double(:label, name: 'bug', color: 'ffffff') + + expect(importer).to receive(:each_label).and_return([label]) + expect(importer.build_labels).to be_empty + end + end + + describe '#build_labels_cache' do + it 'builds the labels cache' do + expect_any_instance_of(Gitlab::GithubImport::LabelFinder) + .to receive(:build_cache) + + importer.build_labels_cache + end + end + + describe '#build' do + let(:label_hash) do + importer.build(double(:label, name: 'bug', color: 'ffffff')) + end + + it 'returns the attributes of the label as a Hash' do + expect(label_hash).to be_an_instance_of(Hash) + end + + context 'the returned Hash' do + it 'includes the label title' do + expect(label_hash[:title]).to eq('bug') + end + + it 'includes the label color' do + expect(label_hash[:color]).to eq('#ffffff') + end + + it 'includes the project ID' do + expect(label_hash[:project_id]).to eq(project.id) + end + + it 'includes the label type' do + expect(label_hash[:type]).to eq('ProjectLabel') + end + + it 'includes the created timestamp' do + Timecop.freeze do + expect(label_hash[:created_at]).to eq(Time.zone.now) + end + end + + it 'includes the updated timestamp' do + Timecop.freeze do + expect(label_hash[:updated_at]).to eq(Time.zone.now) + end + end + end + end + + describe '#each_label' do + it 'returns the labels' do + expect(client) + .to receive(:labels) + .with('foo/bar') + + importer.each_label + end + end +end diff --git a/spec/lib/gitlab/github_import/importer/milestones_importer_spec.rb b/spec/lib/gitlab/github_import/importer/milestones_importer_spec.rb new file mode 100644 index 00000000000..b1cac3b6e46 --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/milestones_importer_spec.rb @@ -0,0 +1,120 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Importer::MilestonesImporter, :clean_gitlab_redis_cache do + let(:project) { create(:project, import_source: 'foo/bar') } + let(:client) { double(:client) } + let(:importer) { described_class.new(project, client) } + let(:created_at) { Time.new(2017, 1, 1, 12, 00) } + let(:updated_at) { Time.new(2017, 1, 1, 12, 15) } + + let(:milestone) do + double( + :milestone, + number: 1, + title: '1.0', + description: 'The first release', + state: 'open', + created_at: created_at, + updated_at: updated_at + ) + end + + describe '#execute' do + it 'imports the milestones in bulk' do + milestone_hash = { number: 1, title: '1.0' } + + expect(importer) + .to receive(:build_milestones) + .and_return([milestone_hash]) + + expect(importer) + .to receive(:bulk_insert) + .with(Milestone, [milestone_hash]) + + expect(importer) + .to receive(:build_milestones_cache) + + importer.execute + end + end + + describe '#build_milestones' do + it 'returns an Array containnig milestone rows' do + expect(importer) + .to receive(:each_milestone) + .and_return([milestone]) + + rows = importer.build_milestones + + expect(rows.length).to eq(1) + expect(rows[0][:title]).to eq('1.0') + end + + it 'does not create milestones that already exist' do + create(:milestone, project: project, title: '1.0', iid: 1) + + expect(importer) + .to receive(:each_milestone) + .and_return([milestone]) + + expect(importer.build_milestones).to be_empty + end + end + + describe '#build_milestones_cache' do + it 'builds the milestones cache' do + expect_any_instance_of(Gitlab::GithubImport::MilestoneFinder) + .to receive(:build_cache) + + importer.build_milestones_cache + end + end + + describe '#build' do + let(:milestone_hash) { importer.build(milestone) } + + it 'returns the attributes of the milestone as a Hash' do + expect(milestone_hash).to be_an_instance_of(Hash) + end + + context 'the returned Hash' do + it 'includes the milestone number' do + expect(milestone_hash[:iid]).to eq(1) + end + + it 'includes the milestone title' do + expect(milestone_hash[:title]).to eq('1.0') + end + + it 'includes the milestone description' do + expect(milestone_hash[:description]).to eq('The first release') + end + + it 'includes the project ID' do + expect(milestone_hash[:project_id]).to eq(project.id) + end + + it 'includes the milestone state' do + expect(milestone_hash[:state]).to eq(:active) + end + + it 'includes the created timestamp' do + expect(milestone_hash[:created_at]).to eq(created_at) + end + + it 'includes the updated timestamp' do + expect(milestone_hash[:updated_at]).to eq(updated_at) + end + end + end + + describe '#each_milestone' do + it 'returns the milestones' do + expect(client) + .to receive(:milestones) + .with('foo/bar', state: 'all') + + importer.each_milestone + end + end +end diff --git a/spec/lib/gitlab/github_import/importer/note_importer_spec.rb b/spec/lib/gitlab/github_import/importer/note_importer_spec.rb new file mode 100644 index 00000000000..9bdcc42be19 --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/note_importer_spec.rb @@ -0,0 +1,151 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Importer::NoteImporter do + let(:client) { double(:client) } + let(:project) { create(:project) } + let(:user) { create(:user) } + let(:created_at) { Time.new(2017, 1, 1, 12, 00) } + let(:updated_at) { Time.new(2017, 1, 1, 12, 15) } + + let(:github_note) do + Gitlab::GithubImport::Representation::Note.new( + noteable_id: 1, + noteable_type: 'Issue', + author: Gitlab::GithubImport::Representation::User.new(id: 4, login: 'alice'), + note: 'This is my note', + created_at: created_at, + updated_at: updated_at, + github_id: 1 + ) + end + + let(:importer) { described_class.new(github_note, project, client) } + + describe '#execute' do + context 'when the noteable exists' do + let!(:issue_row) { create(:issue, project: project, iid: 1) } + + before do + allow(importer) + .to receive(:find_noteable_id) + .and_return(issue_row.id) + end + + context 'when the author could be found' do + it 'imports the note with the found author as the note author' do + expect(importer.user_finder) + .to receive(:author_id_for) + .with(github_note) + .and_return([user.id, true]) + + expect(Gitlab::Database) + .to receive(:bulk_insert) + .with( + Note.table_name, + [ + { + noteable_type: 'Issue', + noteable_id: issue_row.id, + project_id: project.id, + author_id: user.id, + note: 'This is my note', + system: false, + created_at: created_at, + updated_at: updated_at + } + ] + ) + .and_call_original + + importer.execute + end + end + + context 'when the note author could not be found' do + it 'imports the note with the project creator as the note author' do + expect(importer.user_finder) + .to receive(:author_id_for) + .with(github_note) + .and_return([project.creator_id, false]) + + expect(Gitlab::Database) + .to receive(:bulk_insert) + .with( + Note.table_name, + [ + { + noteable_type: 'Issue', + noteable_id: issue_row.id, + project_id: project.id, + author_id: project.creator_id, + note: "*Created by: alice*\n\nThis is my note", + system: false, + created_at: created_at, + updated_at: updated_at + } + ] + ) + .and_call_original + + importer.execute + end + end + end + + context 'when the noteable does not exist' do + it 'does not import the note' do + expect(Gitlab::Database).not_to receive(:bulk_insert) + + importer.execute + end + end + + context 'when the import fails due to a foreign key error' do + it 'does not raise any errors' do + issue_row = create(:issue, project: project, iid: 1) + + allow(importer) + .to receive(:find_noteable_id) + .and_return(issue_row.id) + + allow(importer.user_finder) + .to receive(:author_id_for) + .with(github_note) + .and_return([user.id, true]) + + expect(Gitlab::Database) + .to receive(:bulk_insert) + .and_raise(ActiveRecord::InvalidForeignKey, 'invalid foreign key') + + expect { importer.execute }.not_to raise_error + end + end + + it 'produces a valid Note' do + issue_row = create(:issue, project: project, iid: 1) + + allow(importer) + .to receive(:find_noteable_id) + .and_return(issue_row.id) + + allow(importer.user_finder) + .to receive(:author_id_for) + .with(github_note) + .and_return([user.id, true]) + + importer.execute + + expect(project.notes.take).to be_valid + end + end + + describe '#find_noteable_id' do + it 'returns the ID of the noteable' do + expect_any_instance_of(Gitlab::GithubImport::IssuableFinder) + .to receive(:database_id) + .and_return(10) + + expect(importer.find_noteable_id).to eq(10) + end + end +end diff --git a/spec/lib/gitlab/github_import/importer/notes_importer_spec.rb b/spec/lib/gitlab/github_import/importer/notes_importer_spec.rb new file mode 100644 index 00000000000..f046d13f879 --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/notes_importer_spec.rb @@ -0,0 +1,116 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Importer::NotesImporter do + let(:project) { double(:project, id: 4, import_source: 'foo/bar') } + let(:client) { double(:client) } + + let(:github_comment) do + double( + :response, + html_url: 'https://github.com/foo/bar/issues/42', + user: double(:user, id: 4, login: 'alice'), + body: 'Hello world', + created_at: Time.zone.now, + updated_at: Time.zone.now, + id: 1 + ) + end + + describe '#parallel?' do + it 'returns true when running in parallel mode' do + importer = described_class.new(project, client) + expect(importer).to be_parallel + end + + it 'returns false when running in sequential mode' do + importer = described_class.new(project, client, parallel: false) + expect(importer).not_to be_parallel + end + end + + describe '#execute' do + context 'when running in parallel mode' do + it 'imports notes in parallel' do + importer = described_class.new(project, client) + + expect(importer).to receive(:parallel_import) + + importer.execute + end + end + + context 'when running in sequential mode' do + it 'imports notes in sequence' do + importer = described_class.new(project, client, parallel: false) + + expect(importer).to receive(:sequential_import) + + importer.execute + end + end + end + + describe '#sequential_import' do + it 'imports each note in sequence' do + importer = described_class.new(project, client, parallel: false) + note_importer = double(:note_importer) + + allow(importer) + .to receive(:each_object_to_import) + .and_yield(github_comment) + + expect(Gitlab::GithubImport::Importer::NoteImporter) + .to receive(:new) + .with( + an_instance_of(Gitlab::GithubImport::Representation::Note), + project, + client + ) + .and_return(note_importer) + + expect(note_importer).to receive(:execute) + + importer.sequential_import + end + end + + describe '#parallel_import' do + it 'imports each note in parallel' do + importer = described_class.new(project, client) + + allow(importer) + .to receive(:each_object_to_import) + .and_yield(github_comment) + + expect(Gitlab::GithubImport::ImportNoteWorker) + .to receive(:perform_async) + .with(project.id, an_instance_of(Hash), an_instance_of(String)) + + waiter = importer.parallel_import + + expect(waiter).to be_an_instance_of(Gitlab::JobWaiter) + expect(waiter.jobs_remaining).to eq(1) + end + end + + describe '#id_for_already_imported_cache' do + it 'returns the ID of the given note' do + importer = described_class.new(project, client) + + expect(importer.id_for_already_imported_cache(github_comment)) + .to eq(1) + end + end + + describe '#collection_options' do + it 'returns an empty Hash' do + # For large projects (e.g. kubernetes/kubernetes) GitHub's API may produce + # HTTP 500 errors when using explicit sorting options, regardless of what + # order you sort in. Not using any sorting options at all allows us to + # work around this. + importer = described_class.new(project, client) + + expect(importer.collection_options).to eq({}) + end + end +end diff --git a/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb new file mode 100644 index 00000000000..35f3fdf8304 --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb @@ -0,0 +1,221 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redis_cache do + let(:project) { create(:project, :repository) } + let(:client) { double(:client) } + let(:user) { create(:user) } + let(:created_at) { Time.new(2017, 1, 1, 12, 00) } + let(:updated_at) { Time.new(2017, 1, 1, 12, 15) } + let(:merged_at) { Time.new(2017, 1, 1, 12, 17) } + + let(:source_commit) { project.repository.commit('feature') } + let(:target_commit) { project.repository.commit('master') } + let(:milestone) { create(:milestone, project: project) } + + let(:pull_request) do + alice = Gitlab::GithubImport::Representation::User.new(id: 4, login: 'alice') + + Gitlab::GithubImport::Representation::PullRequest.new( + iid: 42, + title: 'My Pull Request', + description: 'This is my pull request', + source_branch: 'feature', + source_branch_sha: source_commit.id, + target_branch: 'master', + target_branch_sha: target_commit.id, + source_repository_id: 400, + target_repository_id: 200, + source_repository_owner: 'alice', + state: :closed, + milestone_number: milestone.iid, + author: alice, + assignee: alice, + created_at: created_at, + updated_at: updated_at, + merged_at: merged_at + ) + end + + let(:importer) { described_class.new(pull_request, project, client) } + + describe '#execute' do + it 'imports the pull request' do + expect(importer) + .to receive(:create_merge_request) + .and_return(10) + + expect_any_instance_of(Gitlab::GithubImport::IssuableFinder) + .to receive(:cache_database_id) + .with(10) + + importer.execute + end + end + + describe '#create_merge_request' do + before do + allow(importer.milestone_finder) + .to receive(:id_for) + .with(pull_request) + .and_return(milestone.id) + end + + context 'when the author could be found' do + before do + allow(importer.user_finder) + .to receive(:author_id_for) + .with(pull_request) + .and_return([user.id, true]) + + allow(importer.user_finder) + .to receive(:assignee_id_for) + .with(pull_request) + .and_return(user.id) + end + + it 'imports the pull request with the pull request author as the merge request author' do + expect(Gitlab::GithubImport) + .to receive(:insert_and_return_id) + .with( + { + iid: 42, + title: 'My Pull Request', + description: 'This is my pull request', + source_project_id: project.id, + target_project_id: project.id, + source_branch: 'alice:feature', + target_branch: 'master', + state: :merged, + milestone_id: milestone.id, + author_id: user.id, + assignee_id: user.id, + created_at: created_at, + updated_at: updated_at + }, + project.merge_requests + ) + .and_call_original + + importer.create_merge_request + end + + it 'returns the ID of the created merge request' do + id = importer.create_merge_request + + expect(id).to be_a_kind_of(Numeric) + end + + it 'creates the merge request diffs' do + importer.create_merge_request + + mr = project.merge_requests.take + + expect(mr.merge_request_diffs.exists?).to eq(true) + end + end + + context 'when the author could not be found' do + it 'imports the pull request with the project creator as the merge request author' do + allow(importer.user_finder) + .to receive(:author_id_for) + .with(pull_request) + .and_return([project.creator_id, false]) + + allow(importer.user_finder) + .to receive(:assignee_id_for) + .with(pull_request) + .and_return(user.id) + + expect(Gitlab::GithubImport) + .to receive(:insert_and_return_id) + .with( + { + iid: 42, + title: 'My Pull Request', + description: "*Created by: alice*\n\nThis is my pull request", + source_project_id: project.id, + target_project_id: project.id, + source_branch: 'alice:feature', + target_branch: 'master', + state: :merged, + milestone_id: milestone.id, + author_id: project.creator_id, + assignee_id: user.id, + created_at: created_at, + updated_at: updated_at + }, + project.merge_requests + ) + .and_call_original + + importer.create_merge_request + end + end + + context 'when the source and target branch are identical' do + it 'uses a generated source branch name for the merge request' do + allow(importer.user_finder) + .to receive(:author_id_for) + .with(pull_request) + .and_return([user.id, true]) + + allow(importer.user_finder) + .to receive(:assignee_id_for) + .with(pull_request) + .and_return(user.id) + + allow(pull_request) + .to receive(:source_repository_id) + .and_return(pull_request.target_repository_id) + + allow(pull_request) + .to receive(:source_branch) + .and_return('master') + + expect(Gitlab::GithubImport) + .to receive(:insert_and_return_id) + .with( + { + iid: 42, + title: 'My Pull Request', + description: 'This is my pull request', + source_project_id: project.id, + target_project_id: project.id, + source_branch: 'master-42', + target_branch: 'master', + state: :merged, + milestone_id: milestone.id, + author_id: user.id, + assignee_id: user.id, + created_at: created_at, + updated_at: updated_at + }, + project.merge_requests + ) + .and_call_original + + importer.create_merge_request + end + end + + context 'when the import fails due to a foreign key error' do + it 'does not raise any errors' do + allow(importer.user_finder) + .to receive(:author_id_for) + .with(pull_request) + .and_return([user.id, true]) + + allow(importer.user_finder) + .to receive(:assignee_id_for) + .with(pull_request) + .and_return(user.id) + + expect(Gitlab::GithubImport) + .to receive(:insert_and_return_id) + .and_raise(ActiveRecord::InvalidForeignKey, 'invalid foreign key') + + expect { importer.create_merge_request }.not_to raise_error + end + end + end +end diff --git a/spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb new file mode 100644 index 00000000000..d72572cd510 --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb @@ -0,0 +1,272 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Importer::PullRequestsImporter do + let(:project) { create(:project, import_source: 'foo/bar') } + let(:client) { double(:client) } + + let(:pull_request) do + double( + :response, + number: 42, + title: 'My Pull Request', + body: 'This is my pull request', + state: 'closed', + head: double( + :head, + sha: '123abc', + ref: 'my-feature', + repo: double(:repo, id: 400), + user: double(:user, id: 4, login: 'alice') + ), + base: double( + :base, + sha: '456def', + ref: 'master', + repo: double(:repo, id: 200) + ), + milestone: double(:milestone, number: 4), + user: double(:user, id: 4, login: 'alice'), + assignee: double(:user, id: 4, login: 'alice'), + created_at: Time.zone.now, + updated_at: Time.zone.now, + merged_at: Time.zone.now + ) + end + + describe '#parallel?' do + it 'returns true when running in parallel mode' do + importer = described_class.new(project, client) + expect(importer).to be_parallel + end + + it 'returns false when running in sequential mode' do + importer = described_class.new(project, client, parallel: false) + expect(importer).not_to be_parallel + end + end + + describe '#execute' do + context 'when running in parallel mode' do + it 'imports pull requests in parallel' do + importer = described_class.new(project, client) + + expect(importer).to receive(:parallel_import) + + importer.execute + end + end + + context 'when running in sequential mode' do + it 'imports pull requests in sequence' do + importer = described_class.new(project, client, parallel: false) + + expect(importer).to receive(:sequential_import) + + importer.execute + end + end + end + + describe '#sequential_import' do + it 'imports each pull request in sequence' do + importer = described_class.new(project, client, parallel: false) + pull_request_importer = double(:pull_request_importer) + + allow(importer) + .to receive(:each_object_to_import) + .and_yield(pull_request) + + expect(Gitlab::GithubImport::Importer::PullRequestImporter) + .to receive(:new) + .with( + an_instance_of(Gitlab::GithubImport::Representation::PullRequest), + project, + client + ) + .and_return(pull_request_importer) + + expect(pull_request_importer).to receive(:execute) + + importer.sequential_import + end + end + + describe '#parallel_import' do + it 'imports each note in parallel' do + importer = described_class.new(project, client) + + allow(importer) + .to receive(:each_object_to_import) + .and_yield(pull_request) + + expect(Gitlab::GithubImport::ImportPullRequestWorker) + .to receive(:perform_async) + .with(project.id, an_instance_of(Hash), an_instance_of(String)) + + waiter = importer.parallel_import + + expect(waiter).to be_an_instance_of(Gitlab::JobWaiter) + expect(waiter.jobs_remaining).to eq(1) + end + end + + describe '#each_object_to_import', :clean_gitlab_redis_cache do + let(:importer) { described_class.new(project, client) } + + before do + page = double(:page, objects: [pull_request], number: 1) + + expect(client) + .to receive(:each_page) + .with( + :pull_requests, + 'foo/bar', + { state: 'all', sort: 'created', direction: 'asc', page: 1 } + ) + .and_yield(page) + end + + it 'yields every pull request to the supplied block' do + expect { |b| importer.each_object_to_import(&b) } + .to yield_with_args(pull_request) + end + + it 'updates the repository if a pull request was updated after the last clone' do + expect(importer) + .to receive(:update_repository?) + .with(pull_request) + .and_return(true) + + expect(importer) + .to receive(:update_repository) + + importer.each_object_to_import { } + end + end + + describe '#update_repository' do + it 'updates the repository' do + importer = described_class.new(project, client) + + expect(project.repository) + .to receive(:fetch_remote) + .with('github', forced: false) + + expect(Rails.logger) + .to receive(:info) + .with(an_instance_of(String)) + + expect(importer.repository_updates_counter) + .to receive(:increment) + .with(project: project.path_with_namespace) + .and_call_original + + Timecop.freeze do + importer.update_repository + + expect(project.last_repository_updated_at).to eq(Time.zone.now) + end + end + end + + describe '#update_repository?' do + let(:importer) { described_class.new(project, client) } + + context 'when the pull request was updated after the last update' do + let(:pr) do + double( + :pr, + updated_at: Time.zone.now, + head: double(:head, sha: '123'), + base: double(:base, sha: '456') + ) + end + + before do + allow(project) + .to receive(:last_repository_updated_at) + .and_return(1.year.ago) + end + + it 'returns true when the head SHA is not present' do + expect(importer) + .to receive(:commit_exists?) + .with(pr.head.sha) + .and_return(false) + + expect(importer.update_repository?(pr)).to eq(true) + end + + it 'returns true when the base SHA is not present' do + expect(importer) + .to receive(:commit_exists?) + .with(pr.head.sha) + .and_return(true) + + expect(importer) + .to receive(:commit_exists?) + .with(pr.base.sha) + .and_return(false) + + expect(importer.update_repository?(pr)).to eq(true) + end + + it 'returns false if both the head and base SHAs are present' do + expect(importer) + .to receive(:commit_exists?) + .with(pr.head.sha) + .and_return(true) + + expect(importer) + .to receive(:commit_exists?) + .with(pr.base.sha) + .and_return(true) + + expect(importer.update_repository?(pr)).to eq(false) + end + end + + context 'when the pull request was updated before the last update' do + it 'returns false' do + pr = double(:pr, updated_at: 1.year.ago) + + allow(project) + .to receive(:last_repository_updated_at) + .and_return(Time.zone.now) + + expect(importer.update_repository?(pr)).to eq(false) + end + end + end + + describe '#commit_exists?' do + let(:importer) { described_class.new(project, client) } + + it 'returns true when a commit exists' do + expect(project.repository) + .to receive(:lookup) + .with('123') + .and_return(double(:commit)) + + expect(importer.commit_exists?('123')).to eq(true) + end + + it 'returns false when a commit does not exist' do + expect(project.repository) + .to receive(:lookup) + .with('123') + .and_raise(Rugged::OdbError) + + expect(importer.commit_exists?('123')).to eq(false) + end + end + + describe '#id_for_already_imported_cache' do + it 'returns the PR number of the given PR' do + importer = described_class.new(project, client) + + expect(importer.id_for_already_imported_cache(pull_request)) + .to eq(42) + end + end +end diff --git a/spec/lib/gitlab/github_import/importer/releases_importer_spec.rb b/spec/lib/gitlab/github_import/importer/releases_importer_spec.rb new file mode 100644 index 00000000000..23ae026fb14 --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/releases_importer_spec.rb @@ -0,0 +1,125 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Importer::ReleasesImporter do + let(:project) { create(:project) } + let(:client) { double(:client) } + let(:importer) { described_class.new(project, client) } + let(:created_at) { Time.new(2017, 1, 1, 12, 00) } + let(:updated_at) { Time.new(2017, 1, 1, 12, 15) } + + let(:release) do + double( + :release, + tag_name: '1.0', + body: 'This is my release', + created_at: created_at, + updated_at: updated_at + ) + end + + describe '#execute' do + it 'imports the releases in bulk' do + release_hash = { + tag_name: '1.0', + description: 'This is my release', + created_at: created_at, + updated_at: updated_at + } + + expect(importer).to receive(:build_releases).and_return([release_hash]) + expect(importer).to receive(:bulk_insert).with(Release, [release_hash]) + + importer.execute + end + end + + describe '#build_releases' do + it 'returns an Array containnig release rows' do + expect(importer).to receive(:each_release).and_return([release]) + + rows = importer.build_releases + + expect(rows.length).to eq(1) + expect(rows[0][:tag]).to eq('1.0') + end + + it 'does not create releases that already exist' do + create(:release, project: project, tag: '1.0', description: '1.0') + + expect(importer).to receive(:each_release).and_return([release]) + expect(importer.build_releases).to be_empty + end + + it 'uses a default release description if none is provided' do + expect(release).to receive(:body).and_return('') + expect(importer).to receive(:each_release).and_return([release]) + + release = importer.build_releases.first + + expect(release[:description]).to eq('Release for tag 1.0') + end + end + + describe '#build' do + let(:release_hash) { importer.build(release) } + + it 'returns the attributes of the release as a Hash' do + expect(release_hash).to be_an_instance_of(Hash) + end + + context 'the returned Hash' do + it 'includes the tag name' do + expect(release_hash[:tag]).to eq('1.0') + end + + it 'includes the release description' do + expect(release_hash[:description]).to eq('This is my release') + end + + it 'includes the project ID' do + expect(release_hash[:project_id]).to eq(project.id) + end + + it 'includes the created timestamp' do + expect(release_hash[:created_at]).to eq(created_at) + end + + it 'includes the updated timestamp' do + expect(release_hash[:updated_at]).to eq(updated_at) + end + end + end + + describe '#each_release' do + let(:release) { double(:release) } + + before do + allow(project).to receive(:import_source).and_return('foo/bar') + + allow(client) + .to receive(:releases) + .with('foo/bar') + .and_return([release].to_enum) + end + + it 'returns an Enumerator' do + expect(importer.each_release).to be_an_instance_of(Enumerator) + end + + it 'yields every release to the Enumerator' do + expect(importer.each_release.next).to eq(release) + end + end + + describe '#description_for' do + it 'returns the description when present' do + expect(importer.description_for(release)).to eq(release.body) + end + + it 'returns a generated description when one is not present' do + allow(release).to receive(:body).and_return('') + + expect(importer.description_for(release)).to eq('Release for tag 1.0') + end + end +end diff --git a/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb b/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb new file mode 100644 index 00000000000..80539807711 --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb @@ -0,0 +1,264 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Importer::RepositoryImporter do + let(:repository) { double(:repository) } + let(:client) { double(:client) } + + let(:project) do + double( + :project, + import_url: 'foo.git', + import_source: 'foo/bar', + repository_storage_path: 'foo', + disk_path: 'foo', + repository: repository + ) + end + + let(:importer) { described_class.new(project, client) } + let(:shell_adapter) { Gitlab::Shell.new } + + before do + # The method "gitlab_shell" returns a new instance every call, making + # it harder to set expectations. To work around this we'll stub the method + # and return the same instance on every call. + allow(importer).to receive(:gitlab_shell).and_return(shell_adapter) + end + + describe '#import_wiki?' do + it 'returns true if the wiki should be imported' do + repo = double(:repo, has_wiki: true) + + expect(client) + .to receive(:repository) + .with('foo/bar') + .and_return(repo) + + expect(project) + .to receive(:wiki_repository_exists?) + .and_return(false) + + expect(importer.import_wiki?).to eq(true) + end + + it 'returns false if the GitHub wiki is disabled' do + repo = double(:repo, has_wiki: false) + + expect(client) + .to receive(:repository) + .with('foo/bar') + .and_return(repo) + + expect(importer.import_wiki?).to eq(false) + end + + it 'returns false if the wiki has already been imported' do + repo = double(:repo, has_wiki: true) + + expect(client) + .to receive(:repository) + .with('foo/bar') + .and_return(repo) + + expect(project) + .to receive(:wiki_repository_exists?) + .and_return(true) + + expect(importer.import_wiki?).to eq(false) + end + end + + describe '#execute' do + it 'imports the repository and wiki' do + expect(repository) + .to receive(:empty_repo?) + .and_return(true) + + expect(importer) + .to receive(:import_wiki?) + .and_return(true) + + expect(importer) + .to receive(:import_repository) + .and_return(true) + + expect(importer) + .to receive(:import_wiki_repository) + .and_return(true) + + expect(importer) + .to receive(:update_clone_time) + + expect(importer.execute).to eq(true) + end + + it 'does not import the repository if it already exists' do + expect(repository) + .to receive(:empty_repo?) + .and_return(false) + + expect(importer) + .to receive(:import_wiki?) + .and_return(true) + + expect(importer) + .not_to receive(:import_repository) + + expect(importer) + .to receive(:import_wiki_repository) + .and_return(true) + + expect(importer) + .to receive(:update_clone_time) + + expect(importer.execute).to eq(true) + end + + it 'does not import the wiki if it is disabled' do + expect(repository) + .to receive(:empty_repo?) + .and_return(true) + + expect(importer) + .to receive(:import_wiki?) + .and_return(false) + + expect(importer) + .to receive(:import_repository) + .and_return(true) + + expect(importer) + .to receive(:update_clone_time) + + expect(importer) + .not_to receive(:import_wiki_repository) + + expect(importer.execute).to eq(true) + end + + it 'does not import the wiki if the repository could not be imported' do + expect(repository) + .to receive(:empty_repo?) + .and_return(true) + + expect(importer) + .to receive(:import_wiki?) + .and_return(true) + + expect(importer) + .to receive(:import_repository) + .and_return(false) + + expect(importer) + .not_to receive(:update_clone_time) + + expect(importer) + .not_to receive(:import_wiki_repository) + + expect(importer.execute).to eq(false) + end + end + + describe '#import_repository' do + it 'imports the repository' do + expect(project) + .to receive(:ensure_repository) + + expect(importer) + .to receive(:configure_repository_remote) + + expect(repository) + .to receive(:fetch_remote) + .with('github', forced: true) + + expect(importer.import_repository).to eq(true) + end + + it 'marks the import as failed when an error was raised' do + expect(project).to receive(:ensure_repository) + .and_raise(Gitlab::Git::Repository::NoRepository) + + expect(importer) + .to receive(:fail_import) + .and_return(false) + + expect(importer.import_repository).to eq(false) + end + end + + describe '#configure_repository_remote' do + it 'configures the remote details' do + expect(repository) + .to receive(:remote_exists?) + .with('github') + .and_return(false) + + expect(repository) + .to receive(:add_remote) + .with('github', 'foo.git') + + expect(repository) + .to receive(:set_import_remote_as_mirror) + .with('github') + + expect(repository) + .to receive(:add_remote_fetch_config) + + importer.configure_repository_remote + end + + it 'does not configure the remote if already configured' do + expect(repository) + .to receive(:remote_exists?) + .with('github') + .and_return(true) + + expect(repository) + .not_to receive(:add_remote) + + importer.configure_repository_remote + end + end + + describe '#import_wiki_repository' do + it 'imports the wiki repository' do + expect(importer.gitlab_shell) + .to receive(:import_repository) + .with('foo', 'foo.wiki', 'foo.wiki.git') + + expect(importer.import_wiki_repository).to eq(true) + end + + it 'marks the import as failed if an error was raised' do + expect(importer.gitlab_shell) + .to receive(:import_repository) + .and_raise(Gitlab::Shell::Error) + + expect(importer) + .to receive(:fail_import) + .and_return(false) + + expect(importer.import_wiki_repository).to eq(false) + end + end + + describe '#fail_import' do + it 'marks the import as failed' do + expect(project).to receive(:mark_import_as_failed).with('foo') + + expect(importer.fail_import('foo')).to eq(false) + end + end + + describe '#update_clone_time' do + it 'sets the timestamp for when the cloning process finished' do + Timecop.freeze do + expect(project) + .to receive(:update_column) + .with(:last_repository_updated_at, Time.zone.now) + + importer.update_clone_time + end + end + end +end diff --git a/spec/lib/gitlab/github_import/issuable_finder_spec.rb b/spec/lib/gitlab/github_import/issuable_finder_spec.rb new file mode 100644 index 00000000000..da69911812a --- /dev/null +++ b/spec/lib/gitlab/github_import/issuable_finder_spec.rb @@ -0,0 +1,38 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::IssuableFinder, :clean_gitlab_redis_cache do + let(:project) { double(:project, id: 4) } + let(:issue) do + double(:issue, issuable_type: MergeRequest, iid: 1) + end + + let(:finder) { described_class.new(project, issue) } + + describe '#database_id' do + it 'returns nil when no cache is in place' do + expect(finder.database_id).to be_nil + end + + it 'returns the ID of an issuable when the cache is in place' do + finder.cache_database_id(10) + + expect(finder.database_id).to eq(10) + end + + it 'raises TypeError when the object is not supported' do + finder = described_class.new(project, double(:issue)) + + expect { finder.database_id }.to raise_error(TypeError) + end + end + + describe '#cache_database_id' do + it 'caches the ID of a database row' do + expect(Gitlab::GithubImport::Caching) + .to receive(:write) + .with('github-import/issuable-finder/4/MergeRequest/1', 10) + + finder.cache_database_id(10) + end + end +end diff --git a/spec/lib/gitlab/github_import/label_finder_spec.rb b/spec/lib/gitlab/github_import/label_finder_spec.rb new file mode 100644 index 00000000000..8ba766944d6 --- /dev/null +++ b/spec/lib/gitlab/github_import/label_finder_spec.rb @@ -0,0 +1,61 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::LabelFinder, :clean_gitlab_redis_cache do + let(:project) { create(:project) } + let(:finder) { described_class.new(project) } + let!(:bug) { create(:label, project: project, name: 'Bug') } + let!(:feature) { create(:label, project: project, name: 'Feature') } + + describe '#id_for' do + context 'with a cache in place' do + before do + finder.build_cache + end + + it 'returns the ID of the given label' do + expect(finder.id_for(feature.name)).to eq(feature.id) + end + + it 'returns nil for an empty cache key' do + key = finder.cache_key_for(bug.name) + + Gitlab::GithubImport::Caching.write(key, '') + + expect(finder.id_for(bug.name)).to be_nil + end + + it 'returns nil for a non existing label name' do + expect(finder.id_for('kittens')).to be_nil + end + end + + context 'without a cache in place' do + it 'returns nil for a label' do + expect(finder.id_for(feature.name)).to be_nil + end + end + end + + describe '#build_cache' do + it 'builds the cache of all project labels' do + expect(Gitlab::GithubImport::Caching) + .to receive(:write_multiple) + .with( + { + "github-import/label-finder/#{project.id}/Bug" => bug.id, + "github-import/label-finder/#{project.id}/Feature" => feature.id + } + ) + .and_call_original + + finder.build_cache + end + end + + describe '#cache_key_for' do + it 'returns the cache key for a label name' do + expect(finder.cache_key_for('foo')) + .to eq("github-import/label-finder/#{project.id}/foo") + end + end +end diff --git a/spec/lib/gitlab/github_import/markdown_text_spec.rb b/spec/lib/gitlab/github_import/markdown_text_spec.rb new file mode 100644 index 00000000000..1ff5b9d66b3 --- /dev/null +++ b/spec/lib/gitlab/github_import/markdown_text_spec.rb @@ -0,0 +1,28 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::MarkdownText do + describe '.format' do + it 'formats the text' do + author = double(:author, login: 'Alice') + text = described_class.format('Hello', author) + + expect(text).to eq("*Created by: Alice*\n\nHello") + end + end + + describe '#to_s' do + it 'returns the text when the author was found' do + author = double(:author, login: 'Alice') + text = described_class.new('Hello', author, true) + + expect(text.to_s).to eq('Hello') + end + + it 'returns the text with an extra header when the author was not found' do + author = double(:author, login: 'Alice') + text = described_class.new('Hello', author) + + expect(text.to_s).to eq("*Created by: Alice*\n\nHello") + end + end +end diff --git a/spec/lib/gitlab/github_import/milestone_finder_spec.rb b/spec/lib/gitlab/github_import/milestone_finder_spec.rb new file mode 100644 index 00000000000..dff931a2fe8 --- /dev/null +++ b/spec/lib/gitlab/github_import/milestone_finder_spec.rb @@ -0,0 +1,57 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::MilestoneFinder, :clean_gitlab_redis_cache do + let!(:project) { create(:project) } + let!(:milestone) { create(:milestone, project: project) } + let(:finder) { described_class.new(project) } + + describe '#id_for' do + let(:issuable) { double(:issuable, milestone_number: milestone.iid) } + + context 'with a cache in place' do + before do + finder.build_cache + end + + it 'returns the milestone ID of the given issuable' do + expect(finder.id_for(issuable)).to eq(milestone.id) + end + + it 'returns nil for an empty cache key' do + key = finder.cache_key_for(milestone.iid) + + Gitlab::GithubImport::Caching.write(key, '') + + expect(finder.id_for(issuable)).to be_nil + end + + it 'returns nil for an issuable with a non-existing milestone' do + expect(finder.id_for(double(:issuable, milestone_number: 5))).to be_nil + end + end + + context 'without a cache in place' do + it 'returns nil' do + expect(finder.id_for(issuable)).to be_nil + end + end + end + + describe '#build_cache' do + it 'builds the cache of all project milestones' do + expect(Gitlab::GithubImport::Caching) + .to receive(:write_multiple) + .with("github-import/milestone-finder/#{project.id}/1" => milestone.id) + .and_call_original + + finder.build_cache + end + end + + describe '#cache_key_for' do + it 'returns the cache key for an IID' do + expect(finder.cache_key_for(10)) + .to eq("github-import/milestone-finder/#{project.id}/10") + end + end +end diff --git a/spec/lib/gitlab/github_import/page_counter_spec.rb b/spec/lib/gitlab/github_import/page_counter_spec.rb new file mode 100644 index 00000000000..c2613a9a415 --- /dev/null +++ b/spec/lib/gitlab/github_import/page_counter_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::PageCounter, :clean_gitlab_redis_cache do + let(:project) { double(:project, id: 1) } + let(:counter) { described_class.new(project, :issues) } + + describe '#initialize' do + it 'sets the initial page number to 1 when no value is cached' do + expect(counter.current).to eq(1) + end + + it 'sets the initial page number to the cached value when one is present' do + Gitlab::GithubImport::Caching.write(counter.cache_key, 2) + + expect(described_class.new(project, :issues).current).to eq(2) + end + end + + describe '#set' do + it 'overwrites the page number when the given number is greater than the current number' do + counter.set(4) + expect(counter.current).to eq(4) + end + + it 'does not overwrite the page number when the given number is lower than the current number' do + counter.set(2) + counter.set(1) + + expect(counter.current).to eq(2) + 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 new file mode 100644 index 00000000000..e2a821d4d5c --- /dev/null +++ b/spec/lib/gitlab/github_import/parallel_importer_spec.rb @@ -0,0 +1,40 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::ParallelImporter do + describe '.async?' do + it 'returns true' do + expect(described_class).to be_async + end + end + + describe '#execute', :clean_gitlab_redis_shared_state do + let(:project) { create(:project) } + let(:importer) { described_class.new(project) } + + before do + expect(Gitlab::GithubImport::Stage::ImportRepositoryWorker) + .to receive(:perform_async) + .with(project.id) + .and_return('123') + end + + it 'schedules the importing of the repository' do + expect(importer.execute).to eq(true) + 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 + + importer.execute + end + + it 'updates the import JID of the project' do + importer.execute + + expect(project.import_jid).to eq("github-importer/#{project.id}") + end + end +end diff --git a/spec/lib/gitlab/github_import/parallel_scheduling_spec.rb b/spec/lib/gitlab/github_import/parallel_scheduling_spec.rb new file mode 100644 index 00000000000..98205d3ee25 --- /dev/null +++ b/spec/lib/gitlab/github_import/parallel_scheduling_spec.rb @@ -0,0 +1,296 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::ParallelScheduling do + let(:importer_class) do + Class.new do + include(Gitlab::GithubImport::ParallelScheduling) + + def collection_method + :issues + end + end + end + + let(:project) { double(:project, id: 4, import_source: 'foo/bar') } + let(:client) { double(:client) } + + describe '#parallel?' do + it 'returns true when running in parallel mode' do + expect(importer_class.new(project, client)).to be_parallel + end + + it 'returns false when running in sequential mode' do + importer = importer_class.new(project, client, parallel: false) + + expect(importer).not_to be_parallel + end + end + + describe '#execute' do + it 'imports data in parallel when running in parallel mode' do + importer = importer_class.new(project, client) + waiter = double(:waiter) + + expect(importer) + .to receive(:parallel_import) + .and_return(waiter) + + expect(importer.execute) + .to eq(waiter) + end + + it 'imports data in parallel when running in sequential mode' do + importer = importer_class.new(project, client, parallel: false) + + expect(importer) + .to receive(:sequential_import) + .and_return([]) + + expect(importer.execute) + .to eq([]) + end + + it 'expires the cache used for tracking already imported objects' do + importer = importer_class.new(project, client) + + expect(importer).to receive(:parallel_import) + + expect(Gitlab::GithubImport::Caching) + .to receive(:expire) + .with(importer.already_imported_cache_key, a_kind_of(Numeric)) + + importer.execute + end + end + + describe '#sequential_import' do + let(:importer) { importer_class.new(project, client, parallel: false) } + + it 'imports data in sequence' do + repr_class = double(:representation_class) + repr_instance = double(:representation_instance) + gh_importer = double(:github_importer) + gh_importer_instance = double(:github_importer_instance) + object = double(:object) + + expect(importer) + .to receive(:each_object_to_import) + .and_yield(object) + + expect(importer) + .to receive(:representation_class) + .and_return(repr_class) + + expect(repr_class) + .to receive(:from_api_response) + .with(object) + .and_return(repr_instance) + + expect(importer) + .to receive(:importer_class) + .and_return(gh_importer) + + expect(gh_importer) + .to receive(:new) + .with(repr_instance, project, client) + .and_return(gh_importer_instance) + + expect(gh_importer_instance) + .to receive(:execute) + + importer.sequential_import + end + end + + describe '#parallel_import' do + let(:importer) { importer_class.new(project, client) } + + it 'imports data in parallel' do + repr_class = double(:representation) + worker_class = double(:worker) + object = double(:object) + + expect(importer) + .to receive(:each_object_to_import) + .and_yield(object) + + expect(importer) + .to receive(:representation_class) + .and_return(repr_class) + + expect(importer) + .to receive(:sidekiq_worker_class) + .and_return(worker_class) + + expect(repr_class) + .to receive(:from_api_response) + .with(object) + .and_return({ title: 'Foo' }) + + expect(worker_class) + .to receive(:perform_async) + .with(project.id, { title: 'Foo' }, an_instance_of(String)) + + expect(importer.parallel_import) + .to be_an_instance_of(Gitlab::JobWaiter) + end + end + + describe '#each_object_to_import' do + let(:importer) { importer_class.new(project, client) } + let(:object) { double(:object) } + + before do + expect(importer) + .to receive(:collection_options) + .and_return({ state: 'all' }) + end + + it 'yields every object to import' do + page = double(:page, objects: [object], number: 1) + + expect(client) + .to receive(:each_page) + .with(:issues, 'foo/bar', { state: 'all', page: 1 }) + .and_yield(page) + + expect(importer.page_counter) + .to receive(:set) + .with(1) + .and_return(true) + + expect(importer) + .to receive(:already_imported?) + .with(object) + .and_return(false) + + expect(importer) + .to receive(:mark_as_imported) + .with(object) + + expect { |b| importer.each_object_to_import(&b) } + .to yield_with_args(object) + end + + it 'resumes from the last page' do + page = double(:page, objects: [object], number: 2) + + expect(importer.page_counter) + .to receive(:current) + .and_return(2) + + expect(client) + .to receive(:each_page) + .with(:issues, 'foo/bar', { state: 'all', page: 2 }) + .and_yield(page) + + expect(importer.page_counter) + .to receive(:set) + .with(2) + .and_return(true) + + expect(importer) + .to receive(:already_imported?) + .with(object) + .and_return(false) + + expect(importer) + .to receive(:mark_as_imported) + .with(object) + + expect { |b| importer.each_object_to_import(&b) } + .to yield_with_args(object) + end + + it 'does not yield any objects if the page number was not set' do + page = double(:page, objects: [object], number: 1) + + expect(client) + .to receive(:each_page) + .with(:issues, 'foo/bar', { state: 'all', page: 1 }) + .and_yield(page) + + expect(importer.page_counter) + .to receive(:set) + .with(1) + .and_return(false) + + expect { |b| importer.each_object_to_import(&b) } + .not_to yield_control + end + + it 'does not yield the object if it was already imported' do + page = double(:page, objects: [object], number: 1) + + expect(client) + .to receive(:each_page) + .with(:issues, 'foo/bar', { state: 'all', page: 1 }) + .and_yield(page) + + expect(importer.page_counter) + .to receive(:set) + .with(1) + .and_return(true) + + expect(importer) + .to receive(:already_imported?) + .with(object) + .and_return(true) + + expect(importer) + .not_to receive(:mark_as_imported) + + expect { |b| importer.each_object_to_import(&b) } + .not_to yield_control + end + end + + describe '#already_imported?', :clean_gitlab_redis_cache do + let(:importer) { importer_class.new(project, client) } + + it 'returns false when an object has not yet been imported' do + object = double(:object, id: 10) + + expect(importer) + .to receive(:id_for_already_imported_cache) + .with(object) + .and_return(object.id) + + expect(importer.already_imported?(object)) + .to eq(false) + end + + it 'returns true when an object has already been imported' do + object = double(:object, id: 10) + + allow(importer) + .to receive(:id_for_already_imported_cache) + .with(object) + .and_return(object.id) + + importer.mark_as_imported(object) + + expect(importer.already_imported?(object)) + .to eq(true) + end + end + + describe '#mark_as_imported', :clean_gitlab_redis_cache do + it 'marks an object as already imported' do + object = double(:object, id: 10) + importer = importer_class.new(project, client) + + expect(importer) + .to receive(:id_for_already_imported_cache) + .with(object) + .and_return(object.id) + + expect(Gitlab::GithubImport::Caching) + .to receive(:set_add) + .with(importer.already_imported_cache_key, object.id) + .and_call_original + + importer.mark_as_imported(object) + end + end +end diff --git a/spec/lib/gitlab/github_import/representation/diff_note_spec.rb b/spec/lib/gitlab/github_import/representation/diff_note_spec.rb new file mode 100644 index 00000000000..7b0a1ea4948 --- /dev/null +++ b/spec/lib/gitlab/github_import/representation/diff_note_spec.rb @@ -0,0 +1,164 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Representation::DiffNote do + let(:hunk) do + '@@ -1 +1 @@ + -Hello + +Hello world' + end + + let(:created_at) { Time.new(2017, 1, 1, 12, 00) } + let(:updated_at) { Time.new(2017, 1, 1, 12, 15) } + + shared_examples 'a DiffNote' do + it 'returns an instance of DiffNote' do + expect(note).to be_an_instance_of(described_class) + end + + context 'the returned DiffNote' do + it 'includes the number of the note' do + expect(note.noteable_id).to eq(42) + end + + it 'includes the file path of the diff' do + expect(note.file_path).to eq('README.md') + end + + it 'includes the commit ID' do + expect(note.commit_id).to eq('123abc') + end + + it 'includes the user details' do + expect(note.author) + .to be_an_instance_of(Gitlab::GithubImport::Representation::User) + + expect(note.author.id).to eq(4) + expect(note.author.login).to eq('alice') + end + + it 'includes the note body' do + expect(note.note).to eq('Hello world') + end + + it 'includes the created timestamp' do + expect(note.created_at).to eq(created_at) + end + + it 'includes the updated timestamp' do + expect(note.updated_at).to eq(updated_at) + end + + it 'includes the GitHub ID' do + expect(note.github_id).to eq(1) + end + + it 'returns the noteable type' do + expect(note.noteable_type).to eq('MergeRequest') + end + end + end + + describe '.from_api_response' do + let(:response) do + double( + :response, + html_url: 'https://github.com/foo/bar/pull/42', + path: 'README.md', + commit_id: '123abc', + diff_hunk: hunk, + user: double(:user, id: 4, login: 'alice'), + body: 'Hello world', + created_at: created_at, + updated_at: updated_at, + id: 1 + ) + end + + it_behaves_like 'a DiffNote' do + let(:note) { described_class.from_api_response(response) } + end + + it 'does not set the user if the response did not include a user' do + allow(response) + .to receive(:user) + .and_return(nil) + + note = described_class.from_api_response(response) + + expect(note.author).to be_nil + end + end + + describe '.from_json_hash' do + it_behaves_like 'a DiffNote' do + let(:hash) do + { + 'noteable_type' => 'MergeRequest', + 'noteable_id' => 42, + 'file_path' => 'README.md', + 'commit_id' => '123abc', + 'diff_hunk' => hunk, + 'author' => { 'id' => 4, 'login' => 'alice' }, + 'note' => 'Hello world', + 'created_at' => created_at.to_s, + 'updated_at' => updated_at.to_s, + 'github_id' => 1 + } + end + + let(:note) { described_class.from_json_hash(hash) } + end + + it 'does not convert the author if it was not specified' do + hash = { + 'noteable_type' => 'MergeRequest', + 'noteable_id' => 42, + 'file_path' => 'README.md', + 'commit_id' => '123abc', + 'diff_hunk' => hunk, + 'note' => 'Hello world', + 'created_at' => created_at.to_s, + 'updated_at' => updated_at.to_s, + 'github_id' => 1 + } + + note = described_class.from_json_hash(hash) + + expect(note.author).to be_nil + end + end + + describe '#line_code' do + it 'returns a String' do + note = described_class.new(diff_hunk: hunk, file_path: 'README.md') + + expect(note.line_code).to be_an_instance_of(String) + end + end + + describe '#diff_hash' do + it 'returns a Hash containing the diff details' do + note = described_class.from_json_hash( + 'noteable_type' => 'MergeRequest', + 'noteable_id' => 42, + 'file_path' => 'README.md', + 'commit_id' => '123abc', + 'diff_hunk' => hunk, + 'author' => { 'id' => 4, 'login' => 'alice' }, + 'note' => 'Hello world', + 'created_at' => created_at.to_s, + 'updated_at' => updated_at.to_s, + 'github_id' => 1 + ) + + expect(note.diff_hash).to eq( + diff: hunk, + new_path: 'README.md', + old_path: 'README.md', + a_mode: '100644', + b_mode: '100644', + new_file: false + ) + end + end +end diff --git a/spec/lib/gitlab/github_import/representation/expose_attribute_spec.rb b/spec/lib/gitlab/github_import/representation/expose_attribute_spec.rb new file mode 100644 index 00000000000..15de0fe49ff --- /dev/null +++ b/spec/lib/gitlab/github_import/representation/expose_attribute_spec.rb @@ -0,0 +1,19 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Representation::ExposeAttribute do + it 'defines a getter method that returns an attribute value' do + klass = Class.new do + include Gitlab::GithubImport::Representation::ExposeAttribute + + expose_attribute :number + + attr_reader :attributes + + def initialize + @attributes = { number: 42 } + end + end + + expect(klass.new.number).to eq(42) + end +end diff --git a/spec/lib/gitlab/github_import/representation/issue_spec.rb b/spec/lib/gitlab/github_import/representation/issue_spec.rb new file mode 100644 index 00000000000..99330ce42cb --- /dev/null +++ b/spec/lib/gitlab/github_import/representation/issue_spec.rb @@ -0,0 +1,182 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Representation::Issue do + let(:created_at) { Time.new(2017, 1, 1, 12, 00) } + let(:updated_at) { Time.new(2017, 1, 1, 12, 15) } + + shared_examples 'an Issue' do + it 'returns an instance of Issue' do + expect(issue).to be_an_instance_of(described_class) + end + + context 'the returned Issue' do + it 'includes the issue number' do + expect(issue.iid).to eq(42) + end + + it 'includes the issue title' do + expect(issue.title).to eq('My Issue') + end + + it 'includes the issue description' do + expect(issue.description).to eq('This is my issue') + end + + it 'includes the milestone number' do + expect(issue.milestone_number).to eq(4) + end + + it 'includes the issue state' do + expect(issue.state).to eq(:opened) + end + + it 'includes the issue assignees' do + expect(issue.assignees[0]) + .to be_an_instance_of(Gitlab::GithubImport::Representation::User) + + expect(issue.assignees[0].id).to eq(4) + expect(issue.assignees[0].login).to eq('alice') + end + + it 'includes the label names' do + expect(issue.label_names).to eq(%w[bug]) + end + + it 'includes the author details' do + expect(issue.author) + .to be_an_instance_of(Gitlab::GithubImport::Representation::User) + + expect(issue.author.id).to eq(4) + expect(issue.author.login).to eq('alice') + end + + it 'includes the created timestamp' do + expect(issue.created_at).to eq(created_at) + end + + it 'includes the updated timestamp' do + expect(issue.updated_at).to eq(updated_at) + end + + it 'is not a pull request' do + expect(issue.pull_request?).to eq(false) + end + end + end + + describe '.from_api_response' do + let(:response) do + double( + :response, + number: 42, + title: 'My Issue', + body: 'This is my issue', + milestone: double(:milestone, number: 4), + state: 'open', + assignees: [double(:user, id: 4, login: 'alice')], + labels: [double(:label, name: 'bug')], + user: double(:user, id: 4, login: 'alice'), + created_at: created_at, + updated_at: updated_at, + pull_request: false + ) + end + + it_behaves_like 'an Issue' do + let(:issue) { described_class.from_api_response(response) } + end + + it 'does not set the user if the response did not include a user' do + allow(response) + .to receive(:user) + .and_return(nil) + + issue = described_class.from_api_response(response) + + expect(issue.author).to be_nil + end + end + + describe '.from_json_hash' do + it_behaves_like 'an Issue' do + let(:hash) do + { + 'iid' => 42, + 'title' => 'My Issue', + 'description' => 'This is my issue', + 'milestone_number' => 4, + 'state' => 'opened', + 'assignees' => [{ 'id' => 4, 'login' => 'alice' }], + 'label_names' => %w[bug], + 'author' => { 'id' => 4, 'login' => 'alice' }, + 'created_at' => created_at.to_s, + 'updated_at' => updated_at.to_s, + 'pull_request' => false + } + end + + let(:issue) { described_class.from_json_hash(hash) } + end + + it 'does not convert the author if it was not specified' do + 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], + 'created_at' => created_at.to_s, + 'updated_at' => updated_at.to_s, + 'pull_request' => false + } + + issue = described_class.from_json_hash(hash) + + expect(issue.author).to be_nil + end + end + + describe '#labels?' do + it 'returns true when the issue has labels assigned' do + issue = described_class.new(label_names: %w[bug]) + + expect(issue.labels?).to eq(true) + end + + it 'returns false when the issue has no labels assigned' do + issue = described_class.new(label_names: []) + + expect(issue.labels?).to eq(false) + end + end + + describe '#pull_request?' do + it 'returns false for an issue' do + issue = described_class.new(pull_request: false) + + expect(issue.pull_request?).to eq(false) + end + + it 'returns true for a pull request' do + issue = described_class.new(pull_request: true) + + expect(issue.pull_request?).to eq(true) + end + end + + describe '#truncated_title' do + it 'truncates the title to 255 characters' do + object = described_class.new(title: 'm' * 300) + + expect(object.truncated_title.length).to eq(255) + end + + it 'does not truncate the title if it is shorter than 255 characters' do + object = described_class.new(title: 'foo') + + expect(object.truncated_title).to eq('foo') + end + end +end diff --git a/spec/lib/gitlab/github_import/representation/note_spec.rb b/spec/lib/gitlab/github_import/representation/note_spec.rb new file mode 100644 index 00000000000..f2c1c66b357 --- /dev/null +++ b/spec/lib/gitlab/github_import/representation/note_spec.rb @@ -0,0 +1,107 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Representation::Note do + let(:created_at) { Time.new(2017, 1, 1, 12, 00) } + let(:updated_at) { Time.new(2017, 1, 1, 12, 15) } + + shared_examples 'a Note' do + it 'returns an instance of Note' do + expect(note).to be_an_instance_of(described_class) + end + + context 'the returned Note' do + it 'includes the noteable ID' do + expect(note.noteable_id).to eq(42) + end + + it 'includes the noteable type' do + expect(note.noteable_type).to eq('Issue') + end + + it 'includes the author details' do + expect(note.author) + .to be_an_instance_of(Gitlab::GithubImport::Representation::User) + + expect(note.author.id).to eq(4) + expect(note.author.login).to eq('alice') + end + + it 'includes the note body' do + expect(note.note).to eq('Hello world') + end + + it 'includes the created timestamp' do + expect(note.created_at).to eq(created_at) + end + + it 'includes the updated timestamp' do + expect(note.updated_at).to eq(updated_at) + end + + it 'includes the GitHub ID' do + expect(note.github_id).to eq(1) + end + end + end + + describe '.from_api_response' do + let(:response) do + double( + :response, + html_url: 'https://github.com/foo/bar/issues/42', + user: double(:user, id: 4, login: 'alice'), + body: 'Hello world', + created_at: created_at, + updated_at: updated_at, + id: 1 + ) + end + + it_behaves_like 'a Note' do + let(:note) { described_class.from_api_response(response) } + end + + it 'does not set the user if the response did not include a user' do + allow(response) + .to receive(:user) + .and_return(nil) + + note = described_class.from_api_response(response) + + expect(note.author).to be_nil + end + end + + describe '.from_json_hash' do + it_behaves_like 'a Note' do + let(:hash) do + { + 'noteable_id' => 42, + 'noteable_type' => 'Issue', + 'author' => { 'id' => 4, 'login' => 'alice' }, + 'note' => 'Hello world', + 'created_at' => created_at.to_s, + 'updated_at' => updated_at.to_s, + 'github_id' => 1 + } + end + + let(:note) { described_class.from_json_hash(hash) } + end + + it 'does not convert the author if it was not specified' do + hash = { + 'noteable_id' => 42, + 'noteable_type' => 'Issue', + 'note' => 'Hello world', + 'created_at' => created_at.to_s, + 'updated_at' => updated_at.to_s, + 'github_id' => 1 + } + + note = described_class.from_json_hash(hash) + + expect(note.author).to be_nil + end + end +end diff --git a/spec/lib/gitlab/github_import/representation/pull_request_spec.rb b/spec/lib/gitlab/github_import/representation/pull_request_spec.rb new file mode 100644 index 00000000000..33f6ff0ae6a --- /dev/null +++ b/spec/lib/gitlab/github_import/representation/pull_request_spec.rb @@ -0,0 +1,288 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Representation::PullRequest do + let(:created_at) { Time.new(2017, 1, 1, 12, 00) } + let(:updated_at) { Time.new(2017, 1, 1, 12, 15) } + let(:merged_at) { Time.new(2017, 1, 1, 12, 17) } + + shared_examples 'a PullRequest' do + it 'returns an instance of PullRequest' do + expect(pr).to be_an_instance_of(described_class) + end + + context 'the returned PullRequest' do + it 'includes the pull request number' do + expect(pr.iid).to eq(42) + end + + it 'includes the pull request title' do + expect(pr.title).to eq('My Pull Request') + end + + it 'includes the pull request description' do + expect(pr.description).to eq('This is my pull request') + end + + it 'includes the source branch name' do + expect(pr.source_branch).to eq('my-feature') + end + + it 'includes the source branch SHA' do + expect(pr.source_branch_sha).to eq('123abc') + end + + it 'includes the target branch name' do + expect(pr.target_branch).to eq('master') + end + + it 'includes the target branch SHA' do + expect(pr.target_branch_sha).to eq('456def') + end + + it 'includes the milestone number' do + expect(pr.milestone_number).to eq(4) + end + + it 'includes the user details' do + expect(pr.author) + .to be_an_instance_of(Gitlab::GithubImport::Representation::User) + + expect(pr.author.id).to eq(4) + expect(pr.author.login).to eq('alice') + end + + it 'includes the assignee details' do + expect(pr.assignee) + .to be_an_instance_of(Gitlab::GithubImport::Representation::User) + + expect(pr.assignee.id).to eq(4) + expect(pr.assignee.login).to eq('alice') + end + + it 'includes the created timestamp' do + expect(pr.created_at).to eq(created_at) + end + + it 'includes the updated timestamp' do + expect(pr.updated_at).to eq(updated_at) + end + + it 'includes the merged timestamp' do + expect(pr.merged_at).to eq(merged_at) + end + + it 'includes the source repository ID' do + expect(pr.source_repository_id).to eq(400) + end + + it 'includes the target repository ID' do + expect(pr.target_repository_id).to eq(200) + end + + it 'includes the source repository owner name' do + expect(pr.source_repository_owner).to eq('alice') + end + + it 'includes the pull request state' do + expect(pr.state).to eq(:merged) + end + end + end + + describe '.from_api_response' do + let(:response) do + double( + :response, + number: 42, + title: 'My Pull Request', + body: 'This is my pull request', + state: 'closed', + head: double( + :head, + sha: '123abc', + ref: 'my-feature', + repo: double(:repo, id: 400), + user: double(:user, id: 4, login: 'alice') + ), + base: double( + :base, + sha: '456def', + ref: 'master', + repo: double(:repo, id: 200) + ), + milestone: double(:milestone, number: 4), + user: double(:user, id: 4, login: 'alice'), + assignee: double(:user, id: 4, login: 'alice'), + created_at: created_at, + updated_at: updated_at, + merged_at: merged_at + ) + end + + it_behaves_like 'a PullRequest' do + let(:pr) { described_class.from_api_response(response) } + end + + it 'does not set the user if the response did not include a user' do + allow(response) + .to receive(:user) + .and_return(nil) + + pr = described_class.from_api_response(response) + + expect(pr.author).to be_nil + end + end + + describe '.from_json_hash' do + it_behaves_like 'a PullRequest' do + let(:hash) do + { + '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, + 'author' => { 'id' => 4, 'login' => 'alice' }, + 'assignee' => { 'id' => 4, 'login' => 'alice' }, + 'created_at' => created_at.to_s, + 'updated_at' => updated_at.to_s, + 'merged_at' => merged_at.to_s + } + end + + let(:pr) { described_class.from_json_hash(hash) } + end + + it 'does not convert the author if it was not specified' do + 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, + 'assignee' => { 'id' => 4, 'login' => 'alice' }, + 'created_at' => created_at.to_s, + 'updated_at' => updated_at.to_s, + 'merged_at' => merged_at.to_s + } + + pr = described_class.from_json_hash(hash) + + expect(pr.author).to be_nil + end + end + + describe '#state' do + it 'returns :opened for an open pull request' do + pr = described_class.new(state: :opened) + + expect(pr.state).to eq(:opened) + end + + it 'returns :closed for a closed pull request' do + pr = described_class.new(state: :closed) + + expect(pr.state).to eq(:closed) + end + + it 'returns :merged for a merged pull request' do + pr = described_class.new(state: :closed, merged_at: merged_at) + + expect(pr.state).to eq(:merged) + end + end + + describe '#cross_project?' do + it 'returns false for a pull request submitted from the target project' do + pr = described_class.new(source_repository_id: 1, target_repository_id: 1) + + expect(pr).not_to be_cross_project + end + + it 'returns true for a pull request submitted from a different project' do + pr = described_class.new(source_repository_id: 1, target_repository_id: 2) + + expect(pr).to be_cross_project + end + + it 'returns true if no source repository is present' do + pr = described_class.new(target_repository_id: 2) + + expect(pr).to be_cross_project + end + end + + describe '#formatted_source_branch' do + context 'for a cross-project pull request' do + it 'includes the owner name in the branch name' do + pr = described_class.new( + source_repository_owner: 'foo', + source_branch: 'branch', + target_branch: 'master', + source_repository_id: 1, + target_repository_id: 2 + ) + + expect(pr.formatted_source_branch).to eq('foo:branch') + end + end + + context 'for a regular pull request' do + it 'returns the source branch name' do + pr = described_class.new( + source_repository_owner: 'foo', + source_branch: 'branch', + target_branch: 'master', + source_repository_id: 1, + target_repository_id: 1 + ) + + expect(pr.formatted_source_branch).to eq('branch') + end + end + + context 'for a pull request with the same source and target branches' do + it 'returns a generated source branch name' do + pr = described_class.new( + iid: 1, + source_repository_owner: 'foo', + source_branch: 'branch', + target_branch: 'branch', + source_repository_id: 1, + target_repository_id: 1 + ) + + expect(pr.formatted_source_branch).to eq('branch-1') + end + end + end + + describe '#truncated_title' do + it 'truncates the title to 255 characters' do + object = described_class.new(title: 'm' * 300) + + expect(object.truncated_title.length).to eq(255) + end + + it 'does not truncate the title if it is shorter than 255 characters' do + object = described_class.new(title: 'foo') + + expect(object.truncated_title).to eq('foo') + end + end +end diff --git a/spec/lib/gitlab/github_import/representation/to_hash_spec.rb b/spec/lib/gitlab/github_import/representation/to_hash_spec.rb new file mode 100644 index 00000000000..c296aa0a45b --- /dev/null +++ b/spec/lib/gitlab/github_import/representation/to_hash_spec.rb @@ -0,0 +1,37 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Representation::ToHash do + describe '#to_hash' do + let(:user) { double(:user, attributes: { login: 'alice' }) } + + let(:issue) do + double( + :issue, + attributes: { user: user, assignees: [user], number: 42 } + ) + end + + let(:issue_hash) { issue.to_hash } + + before do + user.extend(described_class) + issue.extend(described_class) + end + + it 'converts an object to a Hash' do + expect(issue_hash).to be_an_instance_of(Hash) + end + + it 'converts nested objects to Hashes' do + expect(issue_hash[:user]).to eq({ login: 'alice' }) + end + + it 'converts Array values to Hashes' do + expect(issue_hash[:assignees]).to eq([{ login: 'alice' }]) + end + + it 'keeps values as-is if they do not respond to #to_hash' do + expect(issue_hash[:number]).to eq(42) + end + end +end diff --git a/spec/lib/gitlab/github_import/representation/user_spec.rb b/spec/lib/gitlab/github_import/representation/user_spec.rb new file mode 100644 index 00000000000..4e63e8ea568 --- /dev/null +++ b/spec/lib/gitlab/github_import/representation/user_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Representation::User do + shared_examples 'a User' do + it 'returns an instance of User' do + expect(user).to be_an_instance_of(described_class) + end + + context 'the returned User' do + it 'includes the user ID' do + expect(user.id).to eq(42) + end + + it 'includes the username' do + expect(user.login).to eq('alice') + end + end + end + + describe '.from_api_response' do + it_behaves_like 'a User' do + let(:response) { double(:response, id: 42, login: 'alice') } + let(:user) { described_class.from_api_response(response) } + end + end + + describe '.from_json_hash' do + it_behaves_like 'a User' do + let(:hash) { { 'id' => 42, 'login' => 'alice' } } + let(:user) { described_class.from_json_hash(hash) } + end + end +end diff --git a/spec/lib/gitlab/github_import/representation_spec.rb b/spec/lib/gitlab/github_import/representation_spec.rb new file mode 100644 index 00000000000..0b0610817b0 --- /dev/null +++ b/spec/lib/gitlab/github_import/representation_spec.rb @@ -0,0 +1,17 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Representation do + describe '.symbolize_hash' do + it 'returns a Hash with the keys as Symbols' do + hash = described_class.symbolize_hash('number' => 10) + + expect(hash).to eq({ number: 10 }) + end + + it 'parses timestamp fields into Time instances' do + hash = described_class.symbolize_hash('created_at' => '2017-01-01 12:00') + + expect(hash[:created_at]).to be_an_instance_of(Time) + end + end +end diff --git a/spec/lib/gitlab/github_import/sequential_importer_spec.rb b/spec/lib/gitlab/github_import/sequential_importer_spec.rb new file mode 100644 index 00000000000..6089b0b751f --- /dev/null +++ b/spec/lib/gitlab/github_import/sequential_importer_spec.rb @@ -0,0 +1,37 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::SequentialImporter do + describe '#execute' do + it 'imports a project in sequence' do + repository = double(:repository) + project = double(:project, id: 1, repository: repository) + importer = described_class.new(project, token: 'foo') + + expect_any_instance_of(Gitlab::GithubImport::Importer::RepositoryImporter) + .to receive(:execute) + + described_class::SEQUENTIAL_IMPORTERS.each do |klass| + instance = double(:instance) + + expect(klass).to receive(:new) + .with(project, importer.client) + .and_return(instance) + + expect(instance).to receive(:execute) + end + + described_class::PARALLEL_IMPORTERS.each do |klass| + instance = double(:instance) + + expect(klass).to receive(:new) + .with(project, importer.client, parallel: false) + .and_return(instance) + + expect(instance).to receive(:execute) + end + + expect(repository).to receive(:after_import) + expect(importer.execute).to eq(true) + end + end +end diff --git a/spec/lib/gitlab/github_import/user_finder_spec.rb b/spec/lib/gitlab/github_import/user_finder_spec.rb new file mode 100644 index 00000000000..29f4c00d9c7 --- /dev/null +++ b/spec/lib/gitlab/github_import/user_finder_spec.rb @@ -0,0 +1,333 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::UserFinder, :clean_gitlab_redis_cache do + let(:project) { create(:project) } + let(:client) { double(:client) } + let(:finder) { described_class.new(project, client) } + + describe '#author_id_for' do + it 'returns the user ID for the author of an object' do + user = double(:user, id: 4, login: 'kittens') + note = double(:note, author: user) + + expect(finder).to receive(:user_id_for).with(user).and_return(42) + + expect(finder.author_id_for(note)).to eq([42, true]) + end + + it 'returns the ID of the project creator if no user ID could be found' do + user = double(:user, id: 4, login: 'kittens') + note = double(:note, author: user) + + expect(finder).to receive(:user_id_for).with(user).and_return(nil) + + expect(finder.author_id_for(note)).to eq([project.creator_id, false]) + end + + it 'returns the ID of the ghost user when the object has no user' do + note = double(:note, author: nil) + + expect(finder.author_id_for(note)).to eq([User.ghost.id, true]) + end + + it 'returns the ID of the ghost user when the given object is nil' do + expect(finder.author_id_for(nil)).to eq([User.ghost.id, true]) + end + end + + describe '#assignee_id_for' do + it 'returns the user ID for the assignee of an issuable' do + user = double(:user, id: 4, login: 'kittens') + issue = double(:issue, assignee: user) + + expect(finder).to receive(:user_id_for).with(user).and_return(42) + expect(finder.assignee_id_for(issue)).to eq(42) + end + + it 'returns nil if the issuable does not have an assignee' do + issue = double(:issue, assignee: nil) + + expect(finder).not_to receive(:user_id_for) + expect(finder.assignee_id_for(issue)).to be_nil + end + end + + describe '#user_id_for' do + it 'returns the user ID for the given user' do + user = double(:user, id: 4, login: 'kittens') + + expect(finder).to receive(:find).with(user.id, user.login).and_return(42) + expect(finder.user_id_for(user)).to eq(42) + end + end + + describe '#find' do + let(:user) { create(:user) } + + before do + allow(finder).to receive(:email_for_github_username) + .and_return(user.email) + end + + context 'without a cache' do + before do + allow(finder).to receive(:find_from_cache).and_return([false, nil]) + expect(finder).to receive(:find_id_from_database).and_call_original + end + + it 'finds a GitLab user for a GitHub user ID' do + user.identities.create!(provider: :github, extern_uid: 42) + + expect(finder.find(42, user.username)).to eq(user.id) + end + + it 'finds a GitLab user for a GitHub Email address' do + expect(finder.find(42, user.username)).to eq(user.id) + end + end + + context 'with a cache' do + it 'returns the cached user ID' do + expect(finder).to receive(:find_from_cache).and_return([true, user.id]) + expect(finder).not_to receive(:find_id_from_database) + + expect(finder.find(42, user.username)).to eq(user.id) + end + + it 'does not query the database if the cache key exists but is empty' do + expect(finder).to receive(:find_from_cache).and_return([true, nil]) + expect(finder).not_to receive(:find_id_from_database) + + expect(finder.find(42, user.username)).to be_nil + end + end + end + + describe '#find_from_cache' do + it 'retrieves a GitLab user ID for a GitHub user ID' do + expect(finder) + .to receive(:cached_id_for_github_id) + .with(42) + .and_return([true, 4]) + + expect(finder.find_from_cache(42)).to eq([true, 4]) + end + + it 'retrieves a GitLab user ID for a GitHub Email address' do + email = 'kittens@example.com' + + expect(finder) + .to receive(:cached_id_for_github_id) + .with(42) + .and_return([false, nil]) + + expect(finder) + .to receive(:cached_id_for_github_email) + .with(email) + .and_return([true, 4]) + + expect(finder.find_from_cache(42, email)).to eq([true, 4]) + end + + it 'does not query the cache for an Email address when none is given' do + expect(finder) + .to receive(:cached_id_for_github_id) + .with(42) + .and_return([false, nil]) + + expect(finder).not_to receive(:cached_id_for_github_id) + + expect(finder.find_from_cache(42)).to eq([false]) + end + end + + describe '#find_id_from_database' do + let(:user) { create(:user) } + + it 'returns the GitLab user ID for a GitHub user ID' do + user.identities.create!(provider: :github, extern_uid: 42) + + expect(finder.find_id_from_database(42, user.email)).to eq(user.id) + end + + it 'returns the GitLab user ID for a GitHub Email address' do + expect(finder.find_id_from_database(42, user.email)).to eq(user.id) + end + end + + describe '#email_for_github_username' do + let(:email) { 'kittens@example.com' } + + context 'when an Email address is cached' do + it 'reads the Email address from the cache' do + expect(Gitlab::GithubImport::Caching) + .to receive(:read) + .and_return(email) + + expect(client).not_to receive(:user) + expect(finder.email_for_github_username('kittens')).to eq(email) + end + end + + context 'when an Email address is not cached' do + let(:user) { double(:user, email: email) } + + it 'retrieves the Email address from the GitHub API' do + expect(client).to receive(:user).with('kittens').and_return(user) + expect(finder.email_for_github_username('kittens')).to eq(email) + end + + it 'caches the Email address when an Email address is available' do + expect(client).to receive(:user).with('kittens').and_return(user) + + expect(Gitlab::GithubImport::Caching) + .to receive(:write) + .with(an_instance_of(String), email) + + finder.email_for_github_username('kittens') + end + + it 'returns nil if the user does not exist' do + expect(client) + .to receive(:user) + .with('kittens') + .and_return(nil) + + expect(Gitlab::GithubImport::Caching) + .not_to receive(:write) + + expect(finder.email_for_github_username('kittens')).to be_nil + end + end + end + + describe '#cached_id_for_github_id' do + let(:id) { 4 } + + it 'reads a user ID from the cache' do + Gitlab::GithubImport::Caching + .write(described_class::ID_CACHE_KEY % id, 4) + + expect(finder.cached_id_for_github_id(id)).to eq([true, 4]) + end + + it 'reads a non existing cache key' do + expect(finder.cached_id_for_github_id(id)).to eq([false, nil]) + end + end + + describe '#cached_id_for_github_email' do + let(:email) { 'kittens@example.com' } + + it 'reads a user ID from the cache' do + Gitlab::GithubImport::Caching + .write(described_class::ID_FOR_EMAIL_CACHE_KEY % email, 4) + + expect(finder.cached_id_for_github_email(email)).to eq([true, 4]) + end + + it 'reads a non existing cache key' do + expect(finder.cached_id_for_github_email(email)).to eq([false, nil]) + end + end + + describe '#id_for_github_id' do + let(:id) { 4 } + + it 'queries and caches the user ID for a given GitHub ID' do + expect(finder).to receive(:query_id_for_github_id) + .with(id) + .and_return(42) + + expect(Gitlab::GithubImport::Caching) + .to receive(:write) + .with(described_class::ID_CACHE_KEY % id, 42) + + finder.id_for_github_id(id) + end + + it 'caches a nil value if no ID could be found' do + expect(finder).to receive(:query_id_for_github_id) + .with(id) + .and_return(nil) + + expect(Gitlab::GithubImport::Caching) + .to receive(:write) + .with(described_class::ID_CACHE_KEY % id, nil) + + finder.id_for_github_id(id) + end + end + + describe '#id_for_github_email' do + let(:email) { 'kittens@example.com' } + + it 'queries and caches the user ID for a given Email address' do + expect(finder).to receive(:query_id_for_github_email) + .with(email) + .and_return(42) + + expect(Gitlab::GithubImport::Caching) + .to receive(:write) + .with(described_class::ID_FOR_EMAIL_CACHE_KEY % email, 42) + + finder.id_for_github_email(email) + end + + it 'caches a nil value if no ID could be found' do + expect(finder).to receive(:query_id_for_github_email) + .with(email) + .and_return(nil) + + expect(Gitlab::GithubImport::Caching) + .to receive(:write) + .with(described_class::ID_FOR_EMAIL_CACHE_KEY % email, nil) + + finder.id_for_github_email(email) + end + end + + describe '#query_id_for_github_id' do + it 'returns the ID of the user for the given GitHub user ID' do + user = create(:user) + + user.identities.create!(provider: :github, extern_uid: '42') + + expect(finder.query_id_for_github_id(42)).to eq(user.id) + end + + it 'returns nil when no user ID could be found' do + expect(finder.query_id_for_github_id(42)).to be_nil + end + end + + describe '#query_id_for_github_email' do + it 'returns the ID of the user for the given Email address' do + user = create(:user, email: 'kittens@example.com') + + expect(finder.query_id_for_github_email(user.email)).to eq(user.id) + end + + it 'returns nil if no user ID could be found' do + expect(finder.query_id_for_github_email('kittens@example.com')).to be_nil + end + end + + describe '#read_id_from_cache' do + it 'reads an ID from the cache' do + Gitlab::GithubImport::Caching.write('foo', 10) + + expect(finder.read_id_from_cache('foo')).to eq([true, 10]) + end + + it 'reads a cache key with an empty value' do + Gitlab::GithubImport::Caching.write('foo', nil) + + expect(finder.read_id_from_cache('foo')).to eq([true, nil]) + end + + it 'reads a cache key that does not exist' do + expect(finder.read_id_from_cache('foo')).to eq([false, nil]) + end + end +end diff --git a/spec/lib/gitlab/github_import_spec.rb b/spec/lib/gitlab/github_import_spec.rb new file mode 100644 index 00000000000..51414800e8c --- /dev/null +++ b/spec/lib/gitlab/github_import_spec.rb @@ -0,0 +1,79 @@ +require 'spec_helper' + +describe Gitlab::GithubImport do + let(:project) { double(:project) } + + describe '.new_client_for' do + it 'returns a new Client with a custom token' do + expect(described_class::Client) + .to receive(:new) + .with('123', parallel: true) + + described_class.new_client_for(project, token: '123') + end + + it 'returns a new Client with a token stored in the import data' do + import_data = double(:import_data, credentials: { user: '123' }) + + expect(project) + .to receive(:import_data) + .and_return(import_data) + + expect(described_class::Client) + .to receive(:new) + .with('123', parallel: true) + + described_class.new_client_for(project) + end + end + + describe '.insert_and_return_id' do + let(:attributes) { { iid: 1, title: 'foo' } } + let(:project) { create(:project) } + + context 'on PostgreSQL' do + it 'returns the ID returned by the query' do + expect(Gitlab::Database) + .to receive(:bulk_insert) + .with(Issue.table_name, [attributes], return_ids: true) + .and_return([10]) + + id = described_class.insert_and_return_id(attributes, project.issues) + + expect(id).to eq(10) + end + end + + context 'on MySQL' do + it 'uses a separate query to retrieve the ID' do + issue = create(:issue, project: project, iid: attributes[:iid]) + + expect(Gitlab::Database) + .to receive(:bulk_insert) + .with(Issue.table_name, [attributes], return_ids: true) + .and_return([]) + + id = described_class.insert_and_return_id(attributes, project.issues) + + expect(id).to eq(issue.id) + end + end + end + + describe '.ghost_user_id', :clean_gitlab_redis_cache do + it 'returns the ID of the ghost user' do + expect(described_class.ghost_user_id).to eq(User.ghost.id) + end + + it 'caches the ghost user ID' do + expect(Gitlab::GithubImport::Caching) + .to receive(:write) + .once + .and_call_original + + 2.times do + described_class.ghost_user_id + end + end + end +end diff --git a/spec/lib/gitlab/import_sources_spec.rb b/spec/lib/gitlab/import_sources_spec.rb index c5725f47453..f2fa315e3ec 100644 --- a/spec/lib/gitlab/import_sources_spec.rb +++ b/spec/lib/gitlab/import_sources_spec.rb @@ -56,14 +56,14 @@ describe Gitlab::ImportSources do describe '.importer' do import_sources = { - 'github' => Github::Import, + 'github' => Gitlab::GithubImport::ParallelImporter, 'bitbucket' => Gitlab::BitbucketImport::Importer, 'gitlab' => Gitlab::GitlabImport::Importer, 'google_code' => Gitlab::GoogleCodeImport::Importer, 'fogbugz' => Gitlab::FogbugzImport::Importer, 'git' => nil, 'gitlab_project' => Gitlab::ImportExport::Importer, - 'gitea' => Gitlab::GithubImport::Importer + 'gitea' => Gitlab::LegacyGithubImport::Importer } import_sources.each do |name, klass| diff --git a/spec/lib/gitlab/github_import/branch_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/branch_formatter_spec.rb index 426b43f8b51..48655851140 100644 --- a/spec/lib/gitlab/github_import/branch_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/branch_formatter_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::GithubImport::BranchFormatter do +describe Gitlab::LegacyGithubImport::BranchFormatter do let(:project) { create(:project, :repository) } let(:commit) { create(:commit, project: project) } let(:repo) { double } diff --git a/spec/lib/gitlab/legacy_github_import/client_spec.rb b/spec/lib/gitlab/legacy_github_import/client_spec.rb new file mode 100644 index 00000000000..80b767abce0 --- /dev/null +++ b/spec/lib/gitlab/legacy_github_import/client_spec.rb @@ -0,0 +1,97 @@ +require 'spec_helper' + +describe Gitlab::LegacyGithubImport::Client do + let(:token) { '123456' } + let(:github_provider) { Settingslogic.new('app_id' => 'asd123', 'app_secret' => 'asd123', 'name' => 'github', 'args' => { 'client_options' => {} }) } + + subject(:client) { described_class.new(token) } + + before do + allow(Gitlab.config.omniauth).to receive(:providers).and_return([github_provider]) + end + + it 'convert OAuth2 client options to symbols' do + client.client.options.keys.each do |key| + expect(key).to be_kind_of(Symbol) + end + end + + it 'does not crash (e.g. Settingslogic::MissingSetting) when verify_ssl config is not present' do + expect { client.api }.not_to raise_error + end + + context 'when config is missing' do + before do + allow(Gitlab.config.omniauth).to receive(:providers).and_return([]) + end + + it 'is still possible to get an Octokit client' do + expect { client.api }.not_to raise_error + end + + it 'is not be possible to get an OAuth2 client' do + expect { client.client }.to raise_error(Projects::ImportService::Error) + end + end + + context 'allow SSL verification to be configurable on API' do + before do + github_provider['verify_ssl'] = false + end + + it 'uses supplied value' do + expect(client.client.options[:connection_opts][:ssl]).to eq({ verify: false }) + expect(client.api.connection_options[:ssl]).to eq({ verify: false }) + end + end + + describe '#api_endpoint' do + context 'when provider does not specity an API endpoint' do + it 'uses GitHub root API endpoint' do + expect(client.api.api_endpoint).to eq 'https://api.github.com/' + end + end + + context 'when provider specify a custom API endpoint' do + before do + github_provider['args']['client_options']['site'] = 'https://github.company.com/' + end + + it 'uses the custom API endpoint' do + expect(OmniAuth::Strategies::GitHub).not_to receive(:default_options) + expect(client.api.api_endpoint).to eq 'https://github.company.com/' + end + end + + context 'when given a host' do + subject(:client) { described_class.new(token, host: 'https://try.gitea.io/') } + + it 'builds a endpoint with the given host and the default API version' do + expect(client.api.api_endpoint).to eq 'https://try.gitea.io/api/v3/' + end + end + + context 'when given an API version' do + subject(:client) { described_class.new(token, api_version: 'v3') } + + it 'does not use the API version without a host' do + expect(client.api.api_endpoint).to eq 'https://api.github.com/' + end + end + + context 'when given a host and version' do + subject(:client) { described_class.new(token, host: 'https://try.gitea.io/', api_version: 'v3') } + + it 'builds a endpoint with the given options' do + expect(client.api.api_endpoint).to eq 'https://try.gitea.io/api/v3/' + end + end + end + + it 'does not raise error when rate limit is disabled' do + stub_request(:get, /api.github.com/) + allow(client.api).to receive(:rate_limit!).and_raise(Octokit::NotFound) + + expect { client.issues {} }.not_to raise_error + end +end diff --git a/spec/lib/gitlab/github_import/comment_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/comment_formatter_spec.rb index 035ac8c7c1f..413654e108c 100644 --- a/spec/lib/gitlab/github_import/comment_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/comment_formatter_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::GithubImport::CommentFormatter do +describe Gitlab::LegacyGithubImport::CommentFormatter do let(:client) { double } let(:project) { create(:project) } let(:octocat) { double(id: 123456, login: 'octocat', email: 'octocat@example.com') } diff --git a/spec/lib/gitlab/github_import/importer_spec.rb b/spec/lib/gitlab/legacy_github_import/importer_spec.rb index d570f34985b..20514486727 100644 --- a/spec/lib/gitlab/github_import/importer_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/importer_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' -describe Gitlab::GithubImport::Importer do - shared_examples 'Gitlab::GithubImport::Importer#execute' do +describe Gitlab::LegacyGithubImport::Importer do + shared_examples 'Gitlab::LegacyGithubImport::Importer#execute' do let(:expected_not_called) { [] } before do @@ -35,7 +35,7 @@ describe Gitlab::GithubImport::Importer do end end - shared_examples 'Gitlab::GithubImport::Importer#execute an error occurs' do + shared_examples 'Gitlab::LegacyGithubImport::Importer#execute an error occurs' do before do allow(project).to receive(:import_data).and_return(double.as_null_object) @@ -178,7 +178,7 @@ describe Gitlab::GithubImport::Importer do end end - shared_examples 'Gitlab::GithubImport unit-testing' do + shared_examples 'Gitlab::LegacyGithubImport unit-testing' do describe '#clean_up_restored_branches' do subject { described_class.new(project) } @@ -188,7 +188,7 @@ describe Gitlab::GithubImport::Importer do end context 'when pull request stills open' do - let(:gh_pull_request) { Gitlab::GithubImport::PullRequestFormatter.new(project, pull_request) } + let(:gh_pull_request) { Gitlab::LegacyGithubImport::PullRequestFormatter.new(project, pull_request) } it 'does not remove branches' do expect(subject).not_to receive(:remove_branch) @@ -197,7 +197,7 @@ describe Gitlab::GithubImport::Importer do end context 'when pull request is closed' do - let(:gh_pull_request) { Gitlab::GithubImport::PullRequestFormatter.new(project, closed_pull_request) } + let(:gh_pull_request) { Gitlab::LegacyGithubImport::PullRequestFormatter.new(project, closed_pull_request) } it 'does remove branches' do expect(subject).to receive(:remove_branch).at_least(2).times @@ -262,14 +262,14 @@ describe Gitlab::GithubImport::Importer do let(:repo_root) { 'https://github.com' } subject { described_class.new(project) } - it_behaves_like 'Gitlab::GithubImport::Importer#execute' - it_behaves_like 'Gitlab::GithubImport::Importer#execute an error occurs' - it_behaves_like 'Gitlab::GithubImport unit-testing' + it_behaves_like 'Gitlab::LegacyGithubImport::Importer#execute' + it_behaves_like 'Gitlab::LegacyGithubImport::Importer#execute an error occurs' + it_behaves_like 'Gitlab::LegacyGithubImport unit-testing' describe '#client' do it 'instantiates a Client' do allow(project).to receive(:import_data).and_return(double(credentials: credentials)) - expect(Gitlab::GithubImport::Client).to receive(:new).with( + expect(Gitlab::LegacyGithubImport::Client).to receive(:new).with( credentials[:user], {} ) @@ -288,16 +288,16 @@ describe Gitlab::GithubImport::Importer do project.update(import_type: 'gitea', import_url: "#{repo_root}/foo/group/project.git") end - it_behaves_like 'Gitlab::GithubImport::Importer#execute' do + it_behaves_like 'Gitlab::LegacyGithubImport::Importer#execute' do let(:expected_not_called) { [:import_releases] } end - it_behaves_like 'Gitlab::GithubImport::Importer#execute an error occurs' - it_behaves_like 'Gitlab::GithubImport unit-testing' + it_behaves_like 'Gitlab::LegacyGithubImport::Importer#execute an error occurs' + it_behaves_like 'Gitlab::LegacyGithubImport unit-testing' describe '#client' do it 'instantiates a Client' do allow(project).to receive(:import_data).and_return(double(credentials: credentials)) - expect(Gitlab::GithubImport::Client).to receive(:new).with( + expect(Gitlab::LegacyGithubImport::Client).to receive(:new).with( credentials[:user], { host: "#{repo_root}:443/foo", api_version: 'v1' } ) diff --git a/spec/lib/gitlab/github_import/issuable_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/issuable_formatter_spec.rb index 05294d227bd..3b5d8945344 100644 --- a/spec/lib/gitlab/github_import/issuable_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/issuable_formatter_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::GithubImport::IssuableFormatter do +describe Gitlab::LegacyGithubImport::IssuableFormatter do let(:raw_data) do double(number: 42) end diff --git a/spec/lib/gitlab/github_import/issue_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/issue_formatter_spec.rb index 0fc56d92aa6..1a4d5dbfb70 100644 --- a/spec/lib/gitlab/github_import/issue_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/issue_formatter_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::GithubImport::IssueFormatter do +describe Gitlab::LegacyGithubImport::IssueFormatter do let(:client) { double } let!(:project) { create(:project, namespace: create(:namespace, path: 'octocat')) } let(:octocat) { double(id: 123456, login: 'octocat', email: 'octocat@example.com') } @@ -30,7 +30,7 @@ describe Gitlab::GithubImport::IssueFormatter do allow(client).to receive(:user).and_return(octocat) end - shared_examples 'Gitlab::GithubImport::IssueFormatter#attributes' do + shared_examples 'Gitlab::LegacyGithubImport::IssueFormatter#attributes' do context 'when issue is open' do let(:raw_data) { double(base_data.merge(state: 'open')) } @@ -135,7 +135,7 @@ describe Gitlab::GithubImport::IssueFormatter do end end - shared_examples 'Gitlab::GithubImport::IssueFormatter#number' do + shared_examples 'Gitlab::LegacyGithubImport::IssueFormatter#number' do let(:raw_data) { double(base_data.merge(number: 1347)) } it 'returns issue number' do @@ -144,8 +144,8 @@ describe Gitlab::GithubImport::IssueFormatter do end context 'when importing a GitHub project' do - it_behaves_like 'Gitlab::GithubImport::IssueFormatter#attributes' - it_behaves_like 'Gitlab::GithubImport::IssueFormatter#number' + it_behaves_like 'Gitlab::LegacyGithubImport::IssueFormatter#attributes' + it_behaves_like 'Gitlab::LegacyGithubImport::IssueFormatter#number' end context 'when importing a Gitea project' do @@ -153,8 +153,8 @@ describe Gitlab::GithubImport::IssueFormatter do project.update(import_type: 'gitea') end - it_behaves_like 'Gitlab::GithubImport::IssueFormatter#attributes' - it_behaves_like 'Gitlab::GithubImport::IssueFormatter#number' + it_behaves_like 'Gitlab::LegacyGithubImport::IssueFormatter#attributes' + it_behaves_like 'Gitlab::LegacyGithubImport::IssueFormatter#number' end describe '#has_comments?' do diff --git a/spec/lib/gitlab/github_import/label_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/label_formatter_spec.rb index 83fdd2cc415..0d1d04f1bf6 100644 --- a/spec/lib/gitlab/github_import/label_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/label_formatter_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::GithubImport::LabelFormatter do +describe Gitlab::LegacyGithubImport::LabelFormatter do let(:project) { create(:project) } let(:raw) { double(name: 'improvements', color: 'e6e6e6') } diff --git a/spec/lib/gitlab/github_import/milestone_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/milestone_formatter_spec.rb index 683fa51b78e..1db4bbb568c 100644 --- a/spec/lib/gitlab/github_import/milestone_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/milestone_formatter_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::GithubImport::MilestoneFormatter do +describe Gitlab::LegacyGithubImport::MilestoneFormatter do let(:project) { create(:project) } let(:created_at) { DateTime.strptime('2011-01-26T19:01:12Z') } let(:updated_at) { DateTime.strptime('2011-01-27T19:01:12Z') } @@ -19,7 +19,7 @@ describe Gitlab::GithubImport::MilestoneFormatter do subject(:formatter) { described_class.new(project, raw_data) } - shared_examples 'Gitlab::GithubImport::MilestoneFormatter#attributes' do + shared_examples 'Gitlab::LegacyGithubImport::MilestoneFormatter#attributes' do let(:data) { base_data.merge(iid_attr => 1347) } context 'when milestone is open' do @@ -82,7 +82,7 @@ describe Gitlab::GithubImport::MilestoneFormatter do end context 'when importing a GitHub project' do - it_behaves_like 'Gitlab::GithubImport::MilestoneFormatter#attributes' + it_behaves_like 'Gitlab::LegacyGithubImport::MilestoneFormatter#attributes' end context 'when importing a Gitea project' do @@ -91,6 +91,6 @@ describe Gitlab::GithubImport::MilestoneFormatter do project.update(import_type: 'gitea') end - it_behaves_like 'Gitlab::GithubImport::MilestoneFormatter#attributes' + it_behaves_like 'Gitlab::LegacyGithubImport::MilestoneFormatter#attributes' end end diff --git a/spec/lib/gitlab/github_import/project_creator_spec.rb b/spec/lib/gitlab/legacy_github_import/project_creator_spec.rb index 948e7469a18..737c9a624e0 100644 --- a/spec/lib/gitlab/github_import/project_creator_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/project_creator_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::GithubImport::ProjectCreator do +describe Gitlab::LegacyGithubImport::ProjectCreator do let(:user) { create(:user) } let(:namespace) { create(:group, owner: user) } diff --git a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/pull_request_formatter_spec.rb index 2e42f6239b7..267a41e3f32 100644 --- a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/pull_request_formatter_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::GithubImport::PullRequestFormatter do +describe Gitlab::LegacyGithubImport::PullRequestFormatter do let(:client) { double } let(:project) { create(:project, :repository) } let(:source_sha) { create(:commit, project: project).id } @@ -44,7 +44,7 @@ describe Gitlab::GithubImport::PullRequestFormatter do allow(client).to receive(:user).and_return(octocat) end - shared_examples 'Gitlab::GithubImport::PullRequestFormatter#attributes' do + shared_examples 'Gitlab::LegacyGithubImport::PullRequestFormatter#attributes' do context 'when pull request is open' do let(:raw_data) { double(base_data.merge(state: 'open')) } @@ -189,7 +189,7 @@ describe Gitlab::GithubImport::PullRequestFormatter do end end - shared_examples 'Gitlab::GithubImport::PullRequestFormatter#number' do + shared_examples 'Gitlab::LegacyGithubImport::PullRequestFormatter#number' do let(:raw_data) { double(base_data) } it 'returns pull request number' do @@ -197,7 +197,7 @@ describe Gitlab::GithubImport::PullRequestFormatter do end end - shared_examples 'Gitlab::GithubImport::PullRequestFormatter#source_branch_name' do + shared_examples 'Gitlab::LegacyGithubImport::PullRequestFormatter#source_branch_name' do context 'when source branch exists' do let(:raw_data) { double(base_data) } @@ -231,7 +231,7 @@ describe Gitlab::GithubImport::PullRequestFormatter do end end - shared_examples 'Gitlab::GithubImport::PullRequestFormatter#target_branch_name' do + shared_examples 'Gitlab::LegacyGithubImport::PullRequestFormatter#target_branch_name' do context 'when target branch exists' do let(:raw_data) { double(base_data) } @@ -250,10 +250,10 @@ describe Gitlab::GithubImport::PullRequestFormatter do end context 'when importing a GitHub project' do - it_behaves_like 'Gitlab::GithubImport::PullRequestFormatter#attributes' - it_behaves_like 'Gitlab::GithubImport::PullRequestFormatter#number' - it_behaves_like 'Gitlab::GithubImport::PullRequestFormatter#source_branch_name' - it_behaves_like 'Gitlab::GithubImport::PullRequestFormatter#target_branch_name' + it_behaves_like 'Gitlab::LegacyGithubImport::PullRequestFormatter#attributes' + it_behaves_like 'Gitlab::LegacyGithubImport::PullRequestFormatter#number' + it_behaves_like 'Gitlab::LegacyGithubImport::PullRequestFormatter#source_branch_name' + it_behaves_like 'Gitlab::LegacyGithubImport::PullRequestFormatter#target_branch_name' end context 'when importing a Gitea project' do @@ -261,10 +261,10 @@ describe Gitlab::GithubImport::PullRequestFormatter do project.update(import_type: 'gitea') end - it_behaves_like 'Gitlab::GithubImport::PullRequestFormatter#attributes' - it_behaves_like 'Gitlab::GithubImport::PullRequestFormatter#number' - it_behaves_like 'Gitlab::GithubImport::PullRequestFormatter#source_branch_name' - it_behaves_like 'Gitlab::GithubImport::PullRequestFormatter#target_branch_name' + it_behaves_like 'Gitlab::LegacyGithubImport::PullRequestFormatter#attributes' + it_behaves_like 'Gitlab::LegacyGithubImport::PullRequestFormatter#number' + it_behaves_like 'Gitlab::LegacyGithubImport::PullRequestFormatter#source_branch_name' + it_behaves_like 'Gitlab::LegacyGithubImport::PullRequestFormatter#target_branch_name' end describe '#valid?' do diff --git a/spec/lib/gitlab/github_import/release_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/release_formatter_spec.rb index 926bf725d6a..082e3b36dd0 100644 --- a/spec/lib/gitlab/github_import/release_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/release_formatter_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::GithubImport::ReleaseFormatter do +describe Gitlab::LegacyGithubImport::ReleaseFormatter do let!(:project) { create(:project, namespace: create(:namespace, path: 'octocat')) } let(:octocat) { double(id: 123456, login: 'octocat') } let(:created_at) { DateTime.strptime('2011-01-26T19:01:12Z') } diff --git a/spec/lib/gitlab/github_import/user_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/user_formatter_spec.rb index 98e3a7c28b9..3cd096eb0ad 100644 --- a/spec/lib/gitlab/github_import/user_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/user_formatter_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::GithubImport::UserFormatter do +describe Gitlab::LegacyGithubImport::UserFormatter do let(:client) { double } let(:octocat) { double(id: 123456, login: 'octocat', email: 'octocat@example.com') } diff --git a/spec/lib/gitlab/github_import/wiki_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/wiki_formatter_spec.rb index 2662cc20b32..7723533aee2 100644 --- a/spec/lib/gitlab/github_import/wiki_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/wiki_formatter_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::GithubImport::WikiFormatter do +describe Gitlab::LegacyGithubImport::WikiFormatter do let(:project) do create(:project, namespace: create(:namespace, path: 'gitlabhq'), diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 6185f55c1dc..e684389af5d 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -3016,4 +3016,77 @@ describe Project do end end end + + describe '#after_import' do + let(:project) { build(:project) } + + it 'runs the correct hooks' do + expect(project.repository).to receive(:after_import) + expect(project).to receive(:import_finish) + expect(project).to receive(:update_project_counter_caches) + expect(project).to receive(:remove_import_jid) + + project.after_import + end + end + + describe '#update_project_counter_caches' do + let(:project) { create(:project) } + + it 'updates all project counter caches' do + expect_any_instance_of(Projects::OpenIssuesCountService) + .to receive(:refresh_cache) + .and_call_original + + expect_any_instance_of(Projects::OpenMergeRequestsCountService) + .to receive(:refresh_cache) + .and_call_original + + project.update_project_counter_caches + end + end + + describe '#remove_import_jid', :clean_gitlab_redis_cache do + let(:project) { } + + context 'without an import JID' do + it 'does nothing' do + project = create(:project) + + expect(Gitlab::SidekiqStatus) + .not_to receive(:unset) + + project.remove_import_jid + end + end + + context 'with an import JID' do + it 'unsets the import JID' do + project = create(:project, import_jid: '123') + + expect(Gitlab::SidekiqStatus) + .to receive(:unset) + .with('123') + .and_call_original + + project.remove_import_jid + + expect(project.import_jid).to be_nil + end + end + end + + describe '#wiki_repository_exists?' do + it 'returns true when the wiki repository exists' do + project = create(:project, :wiki_repo) + + expect(project.wiki_repository_exists?).to eq(true) + end + + it 'returns false when the wiki repository does not exist' do + project = create(:project) + + expect(project.wiki_repository_exists?).to eq(false) + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index d2f97009ad9..a3ebf649aa8 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -841,6 +841,19 @@ describe User do end end + describe '.by_any_email' do + it 'returns an ActiveRecord::Relation' do + expect(described_class.by_any_email('foo@example.com')) + .to be_a_kind_of(ActiveRecord::Relation) + end + + it 'returns a relation of users' do + user = create(:user) + + expect(described_class.by_any_email(user.email)).to eq([user]) + end + end + describe '.search' do let!(:user) { create(:user, name: 'user', username: 'usern', email: 'email@gmail.com') } let!(:user2) { create(:user, name: 'user name', username: 'username', email: 'someemail@gmail.com') } diff --git a/spec/services/projects/import_service_spec.rb b/spec/services/projects/import_service_spec.rb index 034065aab00..bf7facaec99 100644 --- a/spec/services/projects/import_service_spec.rb +++ b/spec/services/projects/import_service_spec.rb @@ -6,6 +6,41 @@ describe Projects::ImportService do subject { described_class.new(project, user) } + describe '#async?' do + it 'returns true for an asynchronous importer' do + importer_class = double(:importer, async?: true) + + allow(subject).to receive(:has_importer?).and_return(true) + allow(subject).to receive(:importer_class).and_return(importer_class) + + expect(subject).to be_async + end + + it 'returns false for a regular importer' do + importer_class = double(:importer, async?: false) + + allow(subject).to receive(:has_importer?).and_return(true) + allow(subject).to receive(:importer_class).and_return(importer_class) + + expect(subject).not_to be_async + end + + it 'returns false when the importer does not define #async?' do + importer_class = double(:importer) + + allow(subject).to receive(:has_importer?).and_return(true) + allow(subject).to receive(:importer_class).and_return(importer_class) + + expect(subject).not_to be_async + end + + it 'returns false when the importer does not exist' do + allow(subject).to receive(:has_importer?).and_return(false) + + expect(subject).not_to be_async + end + end + describe '#execute' do context 'with unknown url' do before do @@ -37,21 +72,24 @@ describe Projects::ImportService do end context 'with a Github repository' do - it 'succeeds if repository import is successfully' do - expect_any_instance_of(Github::Import).to receive(:execute).and_return(true) + it 'succeeds if repository import was scheduled' do + expect_any_instance_of(Gitlab::GithubImport::ParallelImporter) + .to receive(:execute) + .and_return(true) result = subject.execute expect(result[:status]).to eq :success end - it 'fails if repository import fails' do - expect_any_instance_of(Repository).to receive(:fetch_remote).and_raise(Gitlab::Shell::Error.new('Failed to import the repository')) + it 'fails if repository import was not scheduled' do + expect_any_instance_of(Gitlab::GithubImport::ParallelImporter) + .to receive(:execute) + .and_return(false) result = subject.execute expect(result[:status]).to eq :error - expect(result[:message]).to eq "Error importing repository #{project.import_url} into #{project.path_with_namespace} - The remote data could not be imported." end end @@ -92,47 +130,22 @@ describe Projects::ImportService do end it 'succeeds if importer succeeds' do - allow_any_instance_of(Github::Import).to receive(:execute).and_return(true) + allow_any_instance_of(Gitlab::GithubImport::ParallelImporter) + .to receive(:execute).and_return(true) result = subject.execute expect(result[:status]).to eq :success end - it 'flushes various caches' do - allow_any_instance_of(Github::Import).to receive(:execute) - .and_return(true) - - expect_any_instance_of(Repository).to receive(:expire_content_cache) - - subject.execute - end - it 'fails if importer fails' do - allow_any_instance_of(Github::Import).to receive(:execute).and_return(false) - - result = subject.execute - - expect(result[:status]).to eq :error - expect(result[:message]).to eq "Error importing repository #{project.import_url} into #{project.full_path} - The remote data could not be imported." - end - - it 'fails if importer raise an error' do - allow_any_instance_of(Github::Import).to receive(:execute).and_raise(Projects::ImportService::Error.new('Github: failed to connect API')) + allow_any_instance_of(Gitlab::GithubImport::ParallelImporter) + .to receive(:execute) + .and_return(false) result = subject.execute expect(result[:status]).to eq :error - expect(result[:message]).to eq "Error importing repository #{project.import_url} into #{project.full_path} - Github: failed to connect API" - end - - it 'expires content cache after error' do - allow_any_instance_of(Project).to receive(:repository_exists?).and_return(false) - - expect_any_instance_of(Repository).to receive(:fetch_remote).and_raise(Gitlab::Shell::Error.new) - expect_any_instance_of(Repository).to receive(:expire_content_cache) - - subject.execute end end diff --git a/spec/support/controllers/githubish_import_controller_shared_examples.rb b/spec/support/controllers/githubish_import_controller_shared_examples.rb index b23d81a226a..a0839eefe6c 100644 --- a/spec/support/controllers/githubish_import_controller_shared_examples.rb +++ b/spec/support/controllers/githubish_import_controller_shared_examples.rb @@ -14,7 +14,7 @@ shared_examples 'a GitHub-ish import controller: POST personal_access_token' do it "updates access token" do token = 'asdfasdf9876' - allow_any_instance_of(Gitlab::GithubImport::Client) + allow_any_instance_of(Gitlab::LegacyGithubImport::Client) .to receive(:user).and_return(true) post :personal_access_token, personal_access_token: token @@ -79,7 +79,7 @@ shared_examples 'a GitHub-ish import controller: GET status' do end it "handles an invalid access token" do - allow_any_instance_of(Gitlab::GithubImport::Client) + allow_any_instance_of(Gitlab::LegacyGithubImport::Client) .to receive(:repos).and_raise(Octokit::Unauthorized) get :status @@ -110,7 +110,7 @@ shared_examples 'a GitHub-ish import controller: POST create' do context "when the repository owner is the provider user" do context "when the provider user and GitLab user's usernames match" do it "takes the current user's namespace" do - expect(Gitlab::GithubImport::ProjectCreator) + expect(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, provider_repo.name, user.namespace, user, access_params, type: provider) .and_return(double(execute: true)) @@ -122,7 +122,7 @@ shared_examples 'a GitHub-ish import controller: POST create' do let(:provider_username) { "someone_else" } it "takes the current user's namespace" do - expect(Gitlab::GithubImport::ProjectCreator) + expect(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, provider_repo.name, user.namespace, user, access_params, type: provider) .and_return(double(execute: true)) @@ -149,7 +149,7 @@ shared_examples 'a GitHub-ish import controller: POST create' do end it "takes the existing namespace" do - expect(Gitlab::GithubImport::ProjectCreator) + expect(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, provider_repo.name, existing_namespace, user, access_params, type: provider) .and_return(double(execute: true)) @@ -161,7 +161,7 @@ shared_examples 'a GitHub-ish import controller: POST create' do it "creates a project using user's namespace" do create(:user, username: other_username) - expect(Gitlab::GithubImport::ProjectCreator) + expect(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, provider_repo.name, user.namespace, user, access_params, type: provider) .and_return(double(execute: true)) @@ -173,14 +173,14 @@ shared_examples 'a GitHub-ish import controller: POST create' do context "when a namespace with the provider user's username doesn't exist" do context "when current user can create namespaces" do it "creates the namespace" do - expect(Gitlab::GithubImport::ProjectCreator) + expect(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).and_return(double(execute: true)) expect { post :create, target_namespace: provider_repo.name, format: :js }.to change(Namespace, :count).by(1) end it "takes the new namespace" do - expect(Gitlab::GithubImport::ProjectCreator) + expect(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, provider_repo.name, an_instance_of(Group), user, access_params, type: provider) .and_return(double(execute: true)) @@ -194,14 +194,14 @@ shared_examples 'a GitHub-ish import controller: POST create' do end it "doesn't create the namespace" do - expect(Gitlab::GithubImport::ProjectCreator) + expect(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).and_return(double(execute: true)) expect { post :create, format: :js }.not_to change(Namespace, :count) end it "takes the current user's namespace" do - expect(Gitlab::GithubImport::ProjectCreator) + expect(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, provider_repo.name, user.namespace, user, access_params, type: provider) .and_return(double(execute: true)) @@ -219,7 +219,7 @@ shared_examples 'a GitHub-ish import controller: POST create' do end it 'takes the selected namespace and name' do - expect(Gitlab::GithubImport::ProjectCreator) + expect(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, test_name, test_namespace, user, access_params, type: provider) .and_return(double(execute: true)) @@ -227,7 +227,7 @@ shared_examples 'a GitHub-ish import controller: POST create' do end it 'takes the selected name and default namespace' do - expect(Gitlab::GithubImport::ProjectCreator) + expect(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, test_name, user.namespace, user, access_params, type: provider) .and_return(double(execute: true)) @@ -245,7 +245,7 @@ shared_examples 'a GitHub-ish import controller: POST create' do end it 'takes the selected namespace and name' do - expect(Gitlab::GithubImport::ProjectCreator) + expect(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, test_name, nested_namespace, user, access_params, type: provider) .and_return(double(execute: true)) @@ -257,7 +257,7 @@ shared_examples 'a GitHub-ish import controller: POST create' do let(:test_name) { 'test_name' } it 'takes the selected namespace and name' do - expect(Gitlab::GithubImport::ProjectCreator) + expect(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, test_name, kind_of(Namespace), user, access_params, type: provider) .and_return(double(execute: true)) @@ -265,7 +265,7 @@ shared_examples 'a GitHub-ish import controller: POST create' do end it 'creates the namespaces' do - allow(Gitlab::GithubImport::ProjectCreator) + allow(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, test_name, kind_of(Namespace), user, access_params, type: provider) .and_return(double(execute: true)) @@ -274,7 +274,7 @@ shared_examples 'a GitHub-ish import controller: POST create' do end it 'new namespace has the right parent' do - allow(Gitlab::GithubImport::ProjectCreator) + allow(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, test_name, kind_of(Namespace), user, access_params, type: provider) .and_return(double(execute: true)) @@ -289,7 +289,7 @@ shared_examples 'a GitHub-ish import controller: POST create' do let!(:parent_namespace) { create(:group, name: 'foo', owner: user) } it 'takes the selected namespace and name' do - expect(Gitlab::GithubImport::ProjectCreator) + expect(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, test_name, kind_of(Namespace), user, access_params, type: provider) .and_return(double(execute: true)) @@ -297,7 +297,7 @@ shared_examples 'a GitHub-ish import controller: POST create' do end it 'creates the namespaces' do - allow(Gitlab::GithubImport::ProjectCreator) + allow(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, test_name, kind_of(Namespace), user, access_params, type: provider) .and_return(double(execute: true)) 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 |