diff options
author | Filipa Lacerda <filipa@gitlab.com> | 2017-12-11 10:01:13 +0000 |
---|---|---|
committer | Filipa Lacerda <filipa@gitlab.com> | 2017-12-11 10:01:13 +0000 |
commit | 7ad8704c24e54ba08db204131843cd9eaeaed7df (patch) | |
tree | ae0546e271bb65389cab8e64c081a891e1029fe9 /spec/lib | |
parent | 37450e5178ce35e35bef0bfa7ec564c18f626177 (diff) | |
parent | fb47f2a7459f4c413f3fe496bcdb1b40d81d73a4 (diff) | |
download | gitlab-ce-7ad8704c24e54ba08db204131843cd9eaeaed7df.tar.gz |
Merge branch 'master' into 40468-empty-states
* master: (76 commits)
Fix image view mode
Remove the need for destroy and add a comment in the spec
Use build instead of create in importer spec
Simplify normalizing of paths
Fix failing importer test case on MySQL due to missing trailing slash in root path
Fix gitlab:import:repos Rake task moving repositories into the wrong location
Allow git pull/push on project redirects
Resolve "Include asset_sync gem"
Use prefix for TableOfContents filter hrefs
Resolve "No feedback when checking on checklist if potential spam was detected"
Fix Rubocop
Fix N+1 query when displaying events
Add Chain::Command specs
Fix a bug of before_sha being inproperly evaluated to `checkout_sha`
Implement and use Gitlab::Ci::Pipeline::Chain::Command
Fix invalid pipeline build chain tag evaluation
Add docs explaining why you get signed out with "Remember me"
Move the circuitbreaker check out in a separate process
Migrate Git::Repository#fsck to Gitaly
Fix new personal access token showing up in a flash message
...
Diffstat (limited to 'spec/lib')
33 files changed, 919 insertions, 327 deletions
diff --git a/spec/lib/gitlab/backup/manager_spec.rb b/spec/lib/backup/manager_spec.rb index b68301a066a..b68301a066a 100644 --- a/spec/lib/gitlab/backup/manager_spec.rb +++ b/spec/lib/backup/manager_spec.rb diff --git a/spec/lib/backup/repository_spec.rb b/spec/lib/backup/repository_spec.rb new file mode 100644 index 00000000000..6ee3d531d6e --- /dev/null +++ b/spec/lib/backup/repository_spec.rb @@ -0,0 +1,69 @@ +require 'spec_helper' + +describe Backup::Repository do + let(:progress) { StringIO.new } + let!(:project) { create(:project) } + + before do + allow(progress).to receive(:puts) + allow(progress).to receive(:print) + + allow_any_instance_of(String).to receive(:color) do |string, _color| + string + end + + allow_any_instance_of(described_class).to receive(:progress).and_return(progress) + end + + describe '#dump' do + describe 'repo failure' do + before do + allow(Gitlab::Popen).to receive(:popen).and_return(['normal output', 0]) + end + + it 'does not raise error' do + expect { described_class.new.dump }.not_to raise_error + end + end + end + + describe '#restore' do + describe 'command failure' do + before do + allow(Gitlab::Popen).to receive(:popen).and_return(['error', 1]) + end + + it 'shows the appropriate error' do + described_class.new.restore + + expect(progress).to have_received(:puts).with("Ignoring error on #{project.full_path} - error") + end + end + end + + describe '#empty_repo?' do + context 'for a wiki' do + let(:wiki) { create(:project_wiki) } + + it 'invalidates the emptiness cache' do + expect(wiki.repository).to receive(:expire_emptiness_caches).once + + wiki.empty? + end + + context 'wiki repo has content' do + let!(:wiki_page) { create(:wiki_page, wiki: wiki) } + + it 'returns true, regardless of bad cache value' do + expect(described_class.new.send(:empty_repo?, wiki)).to be(false) + end + end + + context 'wiki repo does not have content' do + it 'returns true, regardless of bad cache value' do + expect(described_class.new.send(:empty_repo?, wiki)).to be_truthy + end + end + end + end +end diff --git a/spec/lib/banzai/filter/commit_reference_filter_spec.rb b/spec/lib/banzai/filter/commit_reference_filter_spec.rb index 702fcac0c6f..080a5f57da9 100644 --- a/spec/lib/banzai/filter/commit_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/commit_reference_filter_spec.rb @@ -92,6 +92,18 @@ describe Banzai::Filter::CommitReferenceFilter do expect(link).not_to match %r(https?://) expect(link).to eq urls.project_commit_url(project, reference, only_path: true) end + + context "in merge request context" do + let(:noteable) { create(:merge_request, target_project: project, source_project: project) } + let(:commit) { noteable.commits.first } + + it 'handles merge request contextual commit references' do + url = urls.diffs_project_merge_request_url(project, noteable, commit_id: commit.id) + doc = reference_filter("See #{reference}", noteable: noteable) + + expect(doc.css('a').first[:href]).to eq(url) + end + end end context 'cross-project / cross-namespace complete reference' do diff --git a/spec/lib/banzai/filter/table_of_contents_filter_spec.rb b/spec/lib/banzai/filter/table_of_contents_filter_spec.rb index 85eddde732e..0cfef4ff5bf 100644 --- a/spec/lib/banzai/filter/table_of_contents_filter_spec.rb +++ b/spec/lib/banzai/filter/table_of_contents_filter_spec.rb @@ -65,6 +65,13 @@ describe Banzai::Filter::TableOfContentsFilter do expect(doc.css('h2 a').first.attr('href')).to eq '#one-1' end + it 'prepends a prefix to digits-only ids' do + doc = filter(header(1, "123") + header(2, "1.0")) + + expect(doc.css('h1 a').first.attr('href')).to eq '#anchor-123' + expect(doc.css('h2 a').first.attr('href')).to eq '#anchor-10' + end + it 'supports Unicode' do doc = filter(header(1, '한글')) expect(doc.css('h1 a').first.attr('id')).to eq 'user-content-한글' diff --git a/spec/lib/gitlab/backup/repository_spec.rb b/spec/lib/gitlab/backup/repository_spec.rb deleted file mode 100644 index 535cce12780..00000000000 --- a/spec/lib/gitlab/backup/repository_spec.rb +++ /dev/null @@ -1,117 +0,0 @@ -require 'spec_helper' - -describe Backup::Repository do - let(:progress) { StringIO.new } - let!(:project) { create(:project) } - - before do - allow(progress).to receive(:puts) - allow(progress).to receive(:print) - - allow_any_instance_of(String).to receive(:color) do |string, _color| - string - end - - allow_any_instance_of(described_class).to receive(:progress).and_return(progress) - end - - describe '#dump' do - describe 'repo failure' do - before do - allow_any_instance_of(Repository).to receive(:empty_repo?).and_raise(Rugged::OdbError) - allow(Gitlab::Popen).to receive(:popen).and_return(['normal output', 0]) - end - - it 'does not raise error' do - expect { described_class.new.dump }.not_to raise_error - end - - it 'shows the appropriate error' do - described_class.new.dump - - expect(progress).to have_received(:puts).with("Ignoring repository error and continuing backing up project: #{project.full_path} - Rugged::OdbError") - end - end - - describe 'command failure' do - before do - allow_any_instance_of(Repository).to receive(:empty_repo?).and_return(false) - allow(Gitlab::Popen).to receive(:popen).and_return(['error', 1]) - end - - it 'shows the appropriate error' do - described_class.new.dump - - expect(progress).to have_received(:puts).with("Ignoring error on #{project.full_path} - error") - end - end - end - - describe '#restore' do - describe 'command failure' do - before do - allow(Gitlab::Popen).to receive(:popen).and_return(['error', 1]) - end - - it 'shows the appropriate error' do - described_class.new.restore - - expect(progress).to have_received(:puts).with("Ignoring error on #{project.full_path} - error") - end - end - end - - describe '#empty_repo?' do - context 'for a wiki' do - let(:wiki) { create(:project_wiki) } - - context 'wiki repo has content' do - let!(:wiki_page) { create(:wiki_page, wiki: wiki) } - - before do - wiki.repository.exists? # initial cache - end - - context '`repository.exists?` is incorrectly cached as false' do - before do - repo = wiki.repository - repo.send(:cache).expire(:exists?) - repo.send(:cache).fetch(:exists?) { false } - repo.send(:instance_variable_set, :@exists, false) - end - - it 'returns false, regardless of bad cache value' do - expect(described_class.new.send(:empty_repo?, wiki)).to be_falsey - end - end - - context '`repository.exists?` is correctly cached as true' do - it 'returns false' do - expect(described_class.new.send(:empty_repo?, wiki)).to be_falsey - end - end - end - - context 'wiki repo does not have content' do - context '`repository.exists?` is incorrectly cached as true' do - before do - repo = wiki.repository - repo.send(:cache).expire(:exists?) - repo.send(:cache).fetch(:exists?) { true } - repo.send(:instance_variable_set, :@exists, true) - end - - it 'returns true, regardless of bad cache value' do - expect(described_class.new.send(:empty_repo?, wiki)).to be_truthy - end - end - - context '`repository.exists?` is correctly cached as false' do - it 'returns true' do - expect(described_class.new.send(:empty_repo?, wiki)).to be_truthy - end - end - end - end - end -end diff --git a/spec/lib/gitlab/bare_repository_import/importer_spec.rb b/spec/lib/gitlab/bare_repository_import/importer_spec.rb index 7f3bf5fc41c..8a83e446935 100644 --- a/spec/lib/gitlab/bare_repository_import/importer_spec.rb +++ b/spec/lib/gitlab/bare_repository_import/importer_spec.rb @@ -132,6 +132,23 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do expect(File).to exist(File.join(project.repository_storage_path, project.disk_path + '.git')) end + + it 'moves an existing project to the correct path' do + # This is a quick way to get a valid repository instead of copying an + # existing one. Since it's not persisted, the importer will try to + # create the project. + project = build(:project, :repository) + original_commit_count = project.repository.commit_count + + bare_repo = Gitlab::BareRepositoryImport::Repository.new(project.repository_storage_path, project.repository.path) + gitlab_importer = described_class.new(admin, bare_repo) + + expect(gitlab_importer).to receive(:create_project).and_call_original + + new_project = gitlab_importer.create_project_if_needed + + expect(new_project.repository.commit_count).to eq(original_commit_count) + end end context 'with Wiki' do diff --git a/spec/lib/gitlab/bare_repository_import/repository_spec.rb b/spec/lib/gitlab/bare_repository_import/repository_spec.rb index 2db737f5fb6..61b73abcba4 100644 --- a/spec/lib/gitlab/bare_repository_import/repository_spec.rb +++ b/spec/lib/gitlab/bare_repository_import/repository_spec.rb @@ -46,6 +46,13 @@ describe ::Gitlab::BareRepositoryImport::Repository do describe '#project_full_path' do it 'returns the project full path' do expect(project_repo_path.repo_path).to eq('/full/path/to/repo.git') + expect(project_repo_path.project_full_path).to eq('to/repo') + end + + it 'with no trailing slash in the root path' do + repo_path = described_class.new('/full/path', '/full/path/to/repo.git') + + expect(repo_path.project_full_path).to eq('to/repo') end end end diff --git a/spec/lib/gitlab/checks/project_moved_spec.rb b/spec/lib/gitlab/checks/project_moved_spec.rb new file mode 100644 index 00000000000..fa1575e2177 --- /dev/null +++ b/spec/lib/gitlab/checks/project_moved_spec.rb @@ -0,0 +1,81 @@ +require 'rails_helper' + +describe Gitlab::Checks::ProjectMoved, :clean_gitlab_redis_shared_state do + let(:user) { create(:user) } + let(:project) { create(:project) } + + describe '.fetch_redirct_message' do + context 'with a redirect message queue' do + it 'should return the redirect message' do + project_moved = described_class.new(project, user, 'foo/bar', 'http') + project_moved.add_redirect_message + + expect(described_class.fetch_redirect_message(user.id, project.id)).to eq(project_moved.redirect_message) + end + + it 'should delete the redirect message from redis' do + project_moved = described_class.new(project, user, 'foo/bar', 'http') + project_moved.add_redirect_message + + expect(Gitlab::Redis::SharedState.with { |redis| redis.get("redirect_namespace:#{user.id}:#{project.id}") }).not_to be_nil + described_class.fetch_redirect_message(user.id, project.id) + expect(Gitlab::Redis::SharedState.with { |redis| redis.get("redirect_namespace:#{user.id}:#{project.id}") }).to be_nil + end + end + + context 'with no redirect message queue' do + it 'should return nil' do + expect(described_class.fetch_redirect_message(1, 2)).to be_nil + end + end + end + + describe '#add_redirect_message' do + it 'should queue a redirect message' do + project_moved = described_class.new(project, user, 'foo/bar', 'http') + expect(project_moved.add_redirect_message).to eq("OK") + end + end + + describe '#redirect_message' do + context 'when the push is rejected' do + it 'should return a redirect message telling the user to try again' do + project_moved = described_class.new(project, user, 'foo/bar', 'http') + message = "Project 'foo/bar' was moved to '#{project.full_path}'." + + "\n\nPlease update your Git remote:" + + "\n\n git remote set-url origin #{project.http_url_to_repo} and try again.\n" + + expect(project_moved.redirect_message(rejected: true)).to eq(message) + end + end + + context 'when the push is not rejected' do + it 'should return a redirect message' do + project_moved = described_class.new(project, user, 'foo/bar', 'http') + message = "Project 'foo/bar' was moved to '#{project.full_path}'." + + "\n\nPlease update your Git remote:" + + "\n\n git remote set-url origin #{project.http_url_to_repo}\n" + + expect(project_moved.redirect_message).to eq(message) + end + end + end + + describe '#permanent_redirect?' do + context 'with a permanent RedirectRoute' do + it 'should return true' do + project.route.create_redirect('foo/bar', permanent: true) + project_moved = described_class.new(project, user, 'foo/bar', 'http') + expect(project_moved.permanent_redirect?).to be_truthy + end + end + + context 'without a permanent RedirectRoute' do + it 'should return false' do + project.route.create_redirect('foo/bar') + project_moved = described_class.new(project, user, 'foo/bar', 'http') + expect(project_moved.permanent_redirect?).to be_falsy + end + end + end +end diff --git a/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb index 0f1d72080c5..3ae7053a995 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb @@ -6,46 +6,81 @@ describe Gitlab::Ci::Pipeline::Chain::Build do let(:pipeline) { Ci::Pipeline.new } let(:command) do - double('command', source: :push, - origin_ref: 'master', - checkout_sha: project.commit.id, - after_sha: nil, - before_sha: nil, - trigger_request: nil, - schedule: nil, - project: project, - current_user: user) + Gitlab::Ci::Pipeline::Chain::Command.new( + source: :push, + origin_ref: 'master', + checkout_sha: project.commit.id, + after_sha: nil, + before_sha: nil, + trigger_request: nil, + schedule: nil, + project: project, + current_user: user) end let(:step) { described_class.new(pipeline, command) } before do stub_repository_ci_yaml_file(sha: anything) - - step.perform! end it 'never breaks the chain' do + step.perform! + expect(step.break?).to be false end it 'fills pipeline object with data' do + step.perform! + expect(pipeline.sha).not_to be_empty expect(pipeline.sha).to eq project.commit.id expect(pipeline.ref).to eq 'master' + expect(pipeline.tag).to be false expect(pipeline.user).to eq user expect(pipeline.project).to eq project end it 'sets a valid config source' do + step.perform! + expect(pipeline.repository_source?).to be true end it 'returns a valid pipeline' do + step.perform! + expect(pipeline).to be_valid end it 'does not persist a pipeline' do + step.perform! + expect(pipeline).not_to be_persisted end + + context 'when pipeline is running for a tag' do + let(:command) do + Gitlab::Ci::Pipeline::Chain::Command.new( + source: :push, + origin_ref: 'mytag', + checkout_sha: project.commit.id, + after_sha: nil, + before_sha: nil, + trigger_request: nil, + schedule: nil, + project: project, + current_user: user) + end + + before do + allow_any_instance_of(Repository).to receive(:tag_exists?).with('mytag').and_return(true) + + step.perform! + end + + it 'correctly indicated that this is a tagged pipeline' do + expect(pipeline).to be_tag + end + end end diff --git a/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb new file mode 100644 index 00000000000..75a177d2d1f --- /dev/null +++ b/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb @@ -0,0 +1,185 @@ +require 'spec_helper' + +describe Gitlab::Ci::Pipeline::Chain::Command do + set(:project) { create(:project, :repository) } + + describe '#initialize' do + subject do + described_class.new(origin_ref: 'master') + end + + it 'properly initialises object from hash' do + expect(subject.origin_ref).to eq('master') + end + end + + context 'handling of origin_ref' do + let(:command) { described_class.new(project: project, origin_ref: origin_ref) } + + describe '#branch_exists?' do + subject { command.branch_exists? } + + context 'for existing branch' do + let(:origin_ref) { 'master' } + + it { is_expected.to eq(true) } + end + + context 'for invalid branch' do + let(:origin_ref) { 'something' } + + it { is_expected.to eq(false) } + end + end + + describe '#tag_exists?' do + subject { command.tag_exists? } + + context 'for existing ref' do + let(:origin_ref) { 'v1.0.0' } + + it { is_expected.to eq(true) } + end + + context 'for invalid ref' do + let(:origin_ref) { 'something' } + + it { is_expected.to eq(false) } + end + end + + describe '#ref' do + subject { command.ref } + + context 'for regular ref' do + let(:origin_ref) { 'master' } + + it { is_expected.to eq('master') } + end + + context 'for branch ref' do + let(:origin_ref) { 'refs/heads/master' } + + it { is_expected.to eq('master') } + end + + context 'for tag ref' do + let(:origin_ref) { 'refs/tags/1.0.0' } + + it { is_expected.to eq('1.0.0') } + end + + context 'for other refs' do + let(:origin_ref) { 'refs/merge-requests/11/head' } + + it { is_expected.to eq('refs/merge-requests/11/head') } + end + end + end + + describe '#sha' do + subject { command.sha } + + context 'when invalid checkout_sha is specified' do + let(:command) { described_class.new(project: project, checkout_sha: 'aaa') } + + it 'returns empty value' do + is_expected.to be_nil + end + end + + context 'when a valid checkout_sha is specified' do + let(:command) { described_class.new(project: project, checkout_sha: project.commit.id) } + + it 'returns checkout_sha' do + is_expected.to eq(project.commit.id) + end + end + + context 'when a valid after_sha is specified' do + let(:command) { described_class.new(project: project, after_sha: project.commit.id) } + + it 'returns after_sha' do + is_expected.to eq(project.commit.id) + end + end + + context 'when a valid origin_ref is specified' do + let(:command) { described_class.new(project: project, origin_ref: 'HEAD') } + + it 'returns SHA for given ref' do + is_expected.to eq(project.commit.id) + end + end + end + + describe '#origin_sha' do + subject { command.origin_sha } + + context 'when using checkout_sha and after_sha' do + let(:command) { described_class.new(project: project, checkout_sha: 'aaa', after_sha: 'bbb') } + + it 'uses checkout_sha' do + is_expected.to eq('aaa') + end + end + + context 'when using after_sha only' do + let(:command) { described_class.new(project: project, after_sha: 'bbb') } + + it 'uses after_sha' do + is_expected.to eq('bbb') + end + end + end + + describe '#before_sha' do + subject { command.before_sha } + + context 'when using checkout_sha and before_sha' do + let(:command) { described_class.new(project: project, checkout_sha: 'aaa', before_sha: 'bbb') } + + it 'uses before_sha' do + is_expected.to eq('bbb') + end + end + + context 'when using checkout_sha only' do + let(:command) { described_class.new(project: project, checkout_sha: 'aaa') } + + it 'uses checkout_sha' do + is_expected.to eq('aaa') + end + end + + context 'when checkout_sha and before_sha are empty' do + let(:command) { described_class.new(project: project) } + + it 'uses BLANK_SHA' do + is_expected.to eq(Gitlab::Git::BLANK_SHA) + end + end + end + + describe '#protected_ref?' do + let(:command) { described_class.new(project: project, origin_ref: 'my-branch') } + + subject { command.protected_ref? } + + context 'when a ref is protected' do + before do + expect_any_instance_of(Project).to receive(:protected_for?).with('my-branch').and_return(true) + end + + it { is_expected.to eq(true) } + end + + context 'when a ref is unprotected' do + before do + expect_any_instance_of(Project).to receive(:protected_for?).with('my-branch').and_return(false) + end + + it { is_expected.to eq(false) } + end + end +end diff --git a/spec/lib/gitlab/ci/pipeline/chain/create_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/create_spec.rb index f54e2326b06..1b03227d67b 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/create_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/create_spec.rb @@ -10,9 +10,9 @@ describe Gitlab::Ci::Pipeline::Chain::Create do end let(:command) do - double('command', project: project, - current_user: user, - seeds_block: nil) + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, + current_user: user, seeds_block: nil) end let(:step) { described_class.new(pipeline, command) } diff --git a/spec/lib/gitlab/ci/pipeline/chain/sequence_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/sequence_spec.rb index e165e0fac2a..eca23694a2b 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/sequence_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/sequence_spec.rb @@ -5,7 +5,7 @@ describe Gitlab::Ci::Pipeline::Chain::Sequence do set(:user) { create(:user) } let(:pipeline) { build_stubbed(:ci_pipeline) } - let(:command) { double('command' ) } + let(:command) { Gitlab::Ci::Pipeline::Chain::Command.new } let(:first_step) { spy('first step') } let(:second_step) { spy('second step') } let(:sequence) { [first_step, second_step] } diff --git a/spec/lib/gitlab/ci/pipeline/chain/skip_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/skip_spec.rb index 32bd5de829b..dc13cae961c 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/skip_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/skip_spec.rb @@ -6,10 +6,11 @@ describe Gitlab::Ci::Pipeline::Chain::Skip do set(:pipeline) { create(:ci_pipeline, project: project) } let(:command) do - double('command', project: project, - current_user: user, - ignore_skip_ci: false, - save_incompleted: true) + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, + current_user: user, + ignore_skip_ci: false, + save_incompleted: true) end let(:step) { described_class.new(pipeline, command) } diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb index 0bbdd23f4d6..a973ccda8de 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb @@ -5,11 +5,12 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Abilities do set(:user) { create(:user) } let(:pipeline) do - build_stubbed(:ci_pipeline, ref: ref, project: project) + build_stubbed(:ci_pipeline, project: project) end let(:command) do - double('command', project: project, current_user: user) + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, current_user: user, origin_ref: ref) end let(:step) { described_class.new(pipeline, command) } diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb index 8357af38f92..5c12c6e6392 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb @@ -5,9 +5,10 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Config do set(:user) { create(:user) } let(:command) do - double('command', project: project, - current_user: user, - save_incompleted: true) + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, + current_user: user, + save_incompleted: true) end let!(:step) { described_class.new(pipeline, command) } diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb index bb356efe9ad..fb1b53fc55c 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb @@ -3,10 +3,7 @@ require 'spec_helper' describe Gitlab::Ci::Pipeline::Chain::Validate::Repository do set(:project) { create(:project, :repository) } set(:user) { create(:user) } - - let(:command) do - double('command', project: project, current_user: user) - end + let(:pipeline) { build_stubbed(:ci_pipeline) } let!(:step) { described_class.new(pipeline, command) } @@ -14,9 +11,10 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Repository do step.perform! end - context 'when pipeline ref and sha exists' do - let(:pipeline) do - build_stubbed(:ci_pipeline, ref: 'master', sha: '123', project: project) + context 'when ref and sha exists' do + let(:command) do + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, current_user: user, origin_ref: 'master', checkout_sha: project.commit.id) end it 'does not break the chain' do @@ -28,9 +26,10 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Repository do end end - context 'when pipeline ref does not exist' do - let(:pipeline) do - build_stubbed(:ci_pipeline, ref: 'something', project: project) + context 'when ref does not exist' do + let(:command) do + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, current_user: user, origin_ref: 'something') end it 'breaks the chain' do @@ -43,9 +42,10 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Repository do end end - context 'when pipeline does not have SHA set' do - let(:pipeline) do - build_stubbed(:ci_pipeline, ref: 'master', sha: nil, project: project) + context 'when does not have existing SHA set' do + let(:command) do + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, current_user: user, origin_ref: 'master', checkout_sha: 'something') end it 'breaks the chain' do diff --git a/spec/lib/gitlab/diff/inline_diff_spec.rb b/spec/lib/gitlab/diff/inline_diff_spec.rb index 15451c2cf99..0a41362f606 100644 --- a/spec/lib/gitlab/diff/inline_diff_spec.rb +++ b/spec/lib/gitlab/diff/inline_diff_spec.rb @@ -31,6 +31,10 @@ describe Gitlab::Diff::InlineDiff do expect(subject[7]).to eq([17..17]) expect(subject[8]).to be_nil end + + it 'can handle unchanged empty lines' do + expect { described_class.for_lines(['- bar', '+ baz', '']) }.not_to raise_error + end end describe "#inline_diffs" do diff --git a/spec/lib/gitlab/email/handler/create_merge_request_handler_spec.rb b/spec/lib/gitlab/email/handler/create_merge_request_handler_spec.rb index e361d1a7393..51ce3116880 100644 --- a/spec/lib/gitlab/email/handler/create_merge_request_handler_spec.rb +++ b/spec/lib/gitlab/email/handler/create_merge_request_handler_spec.rb @@ -10,14 +10,14 @@ describe Gitlab::Email::Handler::CreateMergeRequestHandler do stub_config_setting(host: 'localhost') end + after do + TestEnv.clean_test_path + end + let(:email_raw) { fixture_file('emails/valid_new_merge_request.eml') } let(:namespace) { create(:namespace, path: 'gitlabhq') } - # project's git repository is not deleted when project is deleted - # between tests. Then tests fail because re-creation of the project with - # the same name fails on existing git repository -> skip_disk_validation - # ignores repository existence on disk - let!(:project) { create(:project, :public, :repository, skip_disk_validation: true, namespace: namespace, path: 'gitlabhq') } + let!(:project) { create(:project, :public, :repository, namespace: namespace, path: 'gitlabhq') } let!(:user) do create( :user, diff --git a/spec/lib/gitlab/git/remote_repository_spec.rb b/spec/lib/gitlab/git/remote_repository_spec.rb index 0506210887c..eb148cc3804 100644 --- a/spec/lib/gitlab/git/remote_repository_spec.rb +++ b/spec/lib/gitlab/git/remote_repository_spec.rb @@ -4,7 +4,7 @@ describe Gitlab::Git::RemoteRepository, seed_helper: true do let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } subject { described_class.new(repository) } - describe '#empty_repo?' do + describe '#empty?' do using RSpec::Parameterized::TableSyntax where(:repository, :result) do @@ -13,7 +13,7 @@ describe Gitlab::Git::RemoteRepository, seed_helper: true do end with_them do - it { expect(subject.empty_repo?).to eq(result) } + it { expect(subject.empty?).to eq(result) } end end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 08dd6ea80ff..f19b65a5f71 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -257,7 +257,7 @@ describe Gitlab::Git::Repository, seed_helper: true do end describe '#empty?' do - it { expect(repository.empty?).to be_falsey } + it { expect(repository).not_to be_empty } end describe '#ref_names' do diff --git a/spec/lib/gitlab/git/storage/checker_spec.rb b/spec/lib/gitlab/git/storage/checker_spec.rb new file mode 100644 index 00000000000..d74c3bcb04c --- /dev/null +++ b/spec/lib/gitlab/git/storage/checker_spec.rb @@ -0,0 +1,132 @@ +require 'spec_helper' + +describe Gitlab::Git::Storage::Checker, :clean_gitlab_redis_shared_state do + let(:storage_name) { 'default' } + let(:hostname) { Gitlab::Environment.hostname } + let(:cache_key) { "storage_accessible:#{storage_name}:#{hostname}" } + + subject(:checker) { described_class.new(storage_name) } + + def value_from_redis(name) + Gitlab::Git::Storage.redis.with do |redis| + redis.hmget(cache_key, name) + end.first + end + + def set_in_redis(name, value) + Gitlab::Git::Storage.redis.with do |redis| + redis.hmset(cache_key, name, value) + end.first + end + + describe '.check_all' do + it 'calls a check for each storage' do + fake_checker_default = double + fake_checker_broken = double + fake_logger = fake_logger + + expect(described_class).to receive(:new).with('default', fake_logger) { fake_checker_default } + expect(described_class).to receive(:new).with('broken', fake_logger) { fake_checker_broken } + expect(fake_checker_default).to receive(:check_with_lease) + expect(fake_checker_broken).to receive(:check_with_lease) + + described_class.check_all(fake_logger) + end + + context 'with broken storage', :broken_storage do + it 'returns the results' do + expected_result = [ + { storage: 'default', success: true }, + { storage: 'broken', success: false } + ] + + expect(described_class.check_all).to eq(expected_result) + end + end + end + + describe '#initialize' do + it 'assigns the settings' do + expect(checker.hostname).to eq(hostname) + expect(checker.storage).to eq('default') + expect(checker.storage_path).to eq(TestEnv.repos_path) + end + end + + describe '#check_with_lease' do + it 'only allows one check at a time' do + expect(checker).to receive(:check).once { sleep 1 } + + thread = Thread.new { checker.check_with_lease } + checker.check_with_lease + thread.join + end + + it 'returns a result hash' do + expect(checker.check_with_lease).to eq(storage: 'default', success: true) + end + end + + describe '#check' do + it 'tracks that the storage was accessible' do + set_in_redis(:failure_count, 10) + set_in_redis(:last_failure, Time.now.to_f) + + checker.check + + expect(value_from_redis(:failure_count).to_i).to eq(0) + expect(value_from_redis(:last_failure)).to be_empty + expect(value_from_redis(:first_failure)).to be_empty + end + + it 'calls the check with the correct arguments' do + stub_application_setting(circuitbreaker_storage_timeout: 30, + circuitbreaker_access_retries: 3) + + expect(Gitlab::Git::Storage::ForkedStorageCheck) + .to receive(:storage_available?).with(TestEnv.repos_path, 30, 3) + .and_call_original + + checker.check + end + + it 'returns `true`' do + expect(checker.check).to eq(true) + end + + it 'maintains known storage keys' do + Timecop.freeze do + # Insert an old key to expire + old_entry = Time.now.to_i - 3.days.to_i + Gitlab::Git::Storage.redis.with do |redis| + redis.zadd(Gitlab::Git::Storage::REDIS_KNOWN_KEYS, old_entry, 'to_be_removed') + end + + checker.check + + known_keys = Gitlab::Git::Storage.redis.with do |redis| + redis.zrange(Gitlab::Git::Storage::REDIS_KNOWN_KEYS, 0, -1) + end + + expect(known_keys).to contain_exactly(cache_key) + end + end + + context 'the storage is not available', :broken_storage do + let(:storage_name) { 'broken' } + + it 'tracks that the storage was inaccessible' do + Timecop.freeze do + expect { checker.check }.to change { value_from_redis(:failure_count).to_i }.by(1) + + expect(value_from_redis(:last_failure)).not_to be_empty + expect(value_from_redis(:first_failure)).not_to be_empty + end + end + + it 'returns `false`' do + expect(checker.check).to eq(false) + end + end + end +end diff --git a/spec/lib/gitlab/git/storage/circuit_breaker_spec.rb b/spec/lib/gitlab/git/storage/circuit_breaker_spec.rb index f34c9f09057..210b90bfba9 100644 --- a/spec/lib/gitlab/git/storage/circuit_breaker_spec.rb +++ b/spec/lib/gitlab/git/storage/circuit_breaker_spec.rb @@ -1,11 +1,18 @@ require 'spec_helper' -describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state: true, broken_storage: true do +describe Gitlab::Git::Storage::CircuitBreaker, :broken_storage do let(:storage_name) { 'default' } let(:circuit_breaker) { described_class.new(storage_name, hostname) } let(:hostname) { Gitlab::Environment.hostname } let(:cache_key) { "storage_accessible:#{storage_name}:#{hostname}" } + def set_in_redis(name, value) + Gitlab::Git::Storage.redis.with do |redis| + redis.zadd(Gitlab::Git::Storage::REDIS_KNOWN_KEYS, 0, cache_key) + redis.hmset(cache_key, name, value) + end.first + end + before do # Override test-settings for the circuitbreaker with something more realistic # for these specs. @@ -19,36 +26,7 @@ describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state: ) end - def value_from_redis(name) - Gitlab::Git::Storage.redis.with do |redis| - redis.hmget(cache_key, name) - end.first - end - - def set_in_redis(name, value) - Gitlab::Git::Storage.redis.with do |redis| - redis.zadd(Gitlab::Git::Storage::REDIS_KNOWN_KEYS, 0, cache_key) - redis.hmset(cache_key, name, value) - end.first - end - - describe '.reset_all!' do - it 'clears all entries form redis' do - set_in_redis(:failure_count, 10) - - described_class.reset_all! - - key_exists = Gitlab::Git::Storage.redis.with { |redis| redis.exists(cache_key) } - - expect(key_exists).to be_falsey - end - - it 'does not break when there are no keys in redis' do - expect { described_class.reset_all! }.not_to raise_error - end - end - - describe '.for_storage' do + describe '.for_storage', :request_store do it 'only builds a single circuitbreaker per storage' do expect(described_class).to receive(:new).once.and_call_original @@ -71,7 +49,6 @@ describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state: it 'assigns the settings' do expect(circuit_breaker.hostname).to eq(hostname) expect(circuit_breaker.storage).to eq('default') - expect(circuit_breaker.storage_path).to eq(TestEnv.repos_path) end end @@ -91,9 +68,9 @@ describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state: end end - describe '#failure_wait_time' do + describe '#check_interval' do it 'reads the value from settings' do - expect(circuit_breaker.failure_wait_time).to eq(1) + expect(circuit_breaker.check_interval).to eq(1) end end @@ -114,12 +91,6 @@ describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state: expect(circuit_breaker.access_retries).to eq(4) end end - - describe '#backoff_threshold' do - it 'reads the value from settings' do - expect(circuit_breaker.backoff_threshold).to eq(5) - end - end end describe '#perform' do @@ -134,19 +105,6 @@ describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state: end end - it 'raises the correct exception when backing off' do - Timecop.freeze do - set_in_redis(:last_failure, 1.second.ago.to_f) - set_in_redis(:failure_count, 90) - - expect { |b| circuit_breaker.perform(&b) } - .to raise_error do |exception| - expect(exception).to be_kind_of(Gitlab::Git::Storage::Failing) - expect(exception.retry_after).to eq(30) - end - end - end - it 'yields the block' do expect { |b| circuit_breaker.perform(&b) } .to yield_control @@ -170,54 +128,6 @@ describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state: .to raise_error(Rugged::OSError) end - it 'tracks that the storage was accessible' do - set_in_redis(:failure_count, 10) - set_in_redis(:last_failure, Time.now.to_f) - - circuit_breaker.perform { '' } - - expect(value_from_redis(:failure_count).to_i).to eq(0) - expect(value_from_redis(:last_failure)).to be_empty - expect(circuit_breaker.failure_count).to eq(0) - expect(circuit_breaker.last_failure).to be_nil - end - - it 'maintains known storage keys' do - Timecop.freeze do - # Insert an old key to expire - old_entry = Time.now.to_i - 3.days.to_i - Gitlab::Git::Storage.redis.with do |redis| - redis.zadd(Gitlab::Git::Storage::REDIS_KNOWN_KEYS, old_entry, 'to_be_removed') - end - - circuit_breaker.perform { '' } - - known_keys = Gitlab::Git::Storage.redis.with do |redis| - redis.zrange(Gitlab::Git::Storage::REDIS_KNOWN_KEYS, 0, -1) - end - - expect(known_keys).to contain_exactly(cache_key) - end - end - - it 'only performs the accessibility check once' do - expect(Gitlab::Git::Storage::ForkedStorageCheck) - .to receive(:storage_available?).once.and_call_original - - 2.times { circuit_breaker.perform { '' } } - end - - it 'calls the check with the correct arguments' do - stub_application_setting(circuitbreaker_storage_timeout: 30, - circuitbreaker_access_retries: 3) - - expect(Gitlab::Git::Storage::ForkedStorageCheck) - .to receive(:storage_available?).with(TestEnv.repos_path, 30, 3) - .and_call_original - - circuit_breaker.perform { '' } - end - context 'with the feature disabled' do before do stub_feature_flags(git_storage_circuit_breaker: false) @@ -240,31 +150,6 @@ describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state: expect(result).to eq('hello') end end - - context 'the storage is not available' do - let(:storage_name) { 'broken' } - - it 'raises the correct exception' do - expect(circuit_breaker).to receive(:track_storage_inaccessible) - - expect { circuit_breaker.perform { '' } } - .to raise_error do |exception| - expect(exception).to be_kind_of(Gitlab::Git::Storage::Inaccessible) - expect(exception.retry_after).to eq(30) - end - end - - it 'tracks that the storage was inaccessible' do - Timecop.freeze do - expect { circuit_breaker.perform { '' } }.to raise_error(Gitlab::Git::Storage::Inaccessible) - - expect(value_from_redis(:failure_count).to_i).to eq(1) - expect(value_from_redis(:last_failure)).not_to be_empty - expect(circuit_breaker.failure_count).to eq(1) - expect(circuit_breaker.last_failure).to be_within(1.second).of(Time.now) - end - end - end end describe '#circuit_broken?' do @@ -283,32 +168,6 @@ describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state: end end - describe '#backing_off?' do - it 'is true when there was a recent failure' do - Timecop.freeze do - set_in_redis(:last_failure, 1.second.ago.to_f) - set_in_redis(:failure_count, 90) - - expect(circuit_breaker.backing_off?).to be_truthy - end - end - - context 'the `failure_wait_time` is set to 0' do - before do - stub_application_setting(circuitbreaker_failure_wait_time: 0) - end - - it 'is working even when there are failures' do - Timecop.freeze do - set_in_redis(:last_failure, 0.seconds.ago.to_f) - set_in_redis(:failure_count, 90) - - expect(circuit_breaker.backing_off?).to be_falsey - end - end - end - end - describe '#last_failure' do it 'returns the last failure time' do time = Time.parse("2017-05-26 17:52:30") diff --git a/spec/lib/gitlab/git/storage/failure_info_spec.rb b/spec/lib/gitlab/git/storage/failure_info_spec.rb new file mode 100644 index 00000000000..bae88fdda86 --- /dev/null +++ b/spec/lib/gitlab/git/storage/failure_info_spec.rb @@ -0,0 +1,70 @@ +require 'spec_helper' + +describe Gitlab::Git::Storage::FailureInfo, :broken_storage do + let(:storage_name) { 'default' } + let(:hostname) { Gitlab::Environment.hostname } + let(:cache_key) { "storage_accessible:#{storage_name}:#{hostname}" } + + def value_from_redis(name) + Gitlab::Git::Storage.redis.with do |redis| + redis.hmget(cache_key, name) + end.first + end + + def set_in_redis(name, value) + Gitlab::Git::Storage.redis.with do |redis| + redis.zadd(Gitlab::Git::Storage::REDIS_KNOWN_KEYS, 0, cache_key) + redis.hmset(cache_key, name, value) + end.first + end + + describe '.reset_all!' do + it 'clears all entries form redis' do + set_in_redis(:failure_count, 10) + + described_class.reset_all! + + key_exists = Gitlab::Git::Storage.redis.with { |redis| redis.exists(cache_key) } + + expect(key_exists).to be_falsey + end + + it 'does not break when there are no keys in redis' do + expect { described_class.reset_all! }.not_to raise_error + end + end + + describe '.load' do + it 'loads failure information for a storage on a host' do + first_failure = Time.parse("2017-11-14 17:52:30") + last_failure = Time.parse("2017-11-14 18:54:37") + failure_count = 11 + + set_in_redis(:first_failure, first_failure.to_i) + set_in_redis(:last_failure, last_failure.to_i) + set_in_redis(:failure_count, failure_count.to_i) + + info = described_class.load(cache_key) + + expect(info.first_failure).to eq(first_failure) + expect(info.last_failure).to eq(last_failure) + expect(info.failure_count).to eq(failure_count) + end + end + + describe '#no_failures?' do + it 'is true when there are no failures' do + info = described_class.new(nil, nil, 0) + + expect(info.no_failures?).to be_truthy + end + + it 'is false when there are failures' do + info = described_class.new(Time.parse("2017-11-14 17:52:30"), + Time.parse("2017-11-14 18:54:37"), + 20) + + expect(info.no_failures?).to be_falsy + end + end +end diff --git a/spec/lib/gitlab/git/storage/health_spec.rb b/spec/lib/gitlab/git/storage/health_spec.rb index d7a52a04fbb..bb670fc5d94 100644 --- a/spec/lib/gitlab/git/storage/health_spec.rb +++ b/spec/lib/gitlab/git/storage/health_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Git::Storage::Health, clean_gitlab_redis_shared_state: true, broken_storage: true do +describe Gitlab::Git::Storage::Health, broken_storage: true do let(:host1_key) { 'storage_accessible:broken:web01' } let(:host2_key) { 'storage_accessible:default:kiq01' } diff --git a/spec/lib/gitlab/git/storage/null_circuit_breaker_spec.rb b/spec/lib/gitlab/git/storage/null_circuit_breaker_spec.rb index 5db37f55e03..93ad20011de 100644 --- a/spec/lib/gitlab/git/storage/null_circuit_breaker_spec.rb +++ b/spec/lib/gitlab/git/storage/null_circuit_breaker_spec.rb @@ -27,7 +27,7 @@ describe Gitlab::Git::Storage::NullCircuitBreaker do end describe '#failure_info' do - it { Timecop.freeze { expect(breaker.failure_info).to eq(Gitlab::Git::Storage::CircuitBreaker::FailureInfo.new(Time.now, breaker.failure_count_threshold)) } } + it { expect(breaker.failure_info.no_failures?).to be_falsy } end end @@ -49,7 +49,7 @@ describe Gitlab::Git::Storage::NullCircuitBreaker do end describe '#failure_info' do - it { expect(breaker.failure_info).to eq(Gitlab::Git::Storage::CircuitBreaker::FailureInfo.new(nil, 0)) } + it { expect(breaker.failure_info.no_failures?).to be_truthy } end end diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index c9643c5da47..2db560c2cec 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -193,7 +193,15 @@ describe Gitlab::GitAccess do let(:actor) { build(:rsa_deploy_key_2048, user: user) } end - describe '#check_project_moved!' do + shared_examples 'check_project_moved' do + it 'enqueues a redirected message' do + push_access_check + + expect(Gitlab::Checks::ProjectMoved.fetch_redirect_message(user.id, project.id)).not_to be_nil + end + end + + describe '#check_project_moved!', :clean_gitlab_redis_shared_state do before do project.add_master(user) end @@ -207,7 +215,40 @@ describe Gitlab::GitAccess do end end - context 'when a redirect was followed to find the project' do + context 'when a permanent redirect and ssh protocol' do + let(:redirected_path) { 'some/other-path' } + + before do + allow_any_instance_of(Gitlab::Checks::ProjectMoved).to receive(:permanent_redirect?).and_return(true) + end + + it 'allows push and pull access' do + aggregate_failures do + expect { push_access_check }.not_to raise_error + end + end + + it_behaves_like 'check_project_moved' + end + + context 'with a permanent redirect and http protocol' do + let(:redirected_path) { 'some/other-path' } + let(:protocol) { 'http' } + + before do + allow_any_instance_of(Gitlab::Checks::ProjectMoved).to receive(:permanent_redirect?).and_return(true) + end + + it 'allows_push and pull access' do + aggregate_failures do + expect { push_access_check }.not_to raise_error + end + end + + it_behaves_like 'check_project_moved' + end + + context 'with a temporal redirect and ssh protocol' do let(:redirected_path) { 'some/other-path' } it 'blocks push and pull access' do @@ -219,16 +260,15 @@ describe Gitlab::GitAccess do expect { pull_access_check }.to raise_error(described_class::ProjectMovedError, /git remote set-url origin #{project.ssh_url_to_repo}/) end end + end - context 'http protocol' do - let(:protocol) { 'http' } + context 'with a temporal redirect and http protocol' do + let(:redirected_path) { 'some/other-path' } + let(:protocol) { 'http' } - it 'includes the path to the project using HTTP' do - aggregate_failures do - expect { push_access_check }.to raise_error(described_class::ProjectMovedError, /git remote set-url origin #{project.http_url_to_repo}/) - expect { pull_access_check }.to raise_error(described_class::ProjectMovedError, /git remote set-url origin #{project.http_url_to_repo}/) - end - end + it 'does not allow to push and pull access' do + expect { push_access_check }.to raise_error(described_class::ProjectMovedError, /git remote set-url origin #{project.http_url_to_repo}/) + expect { pull_access_check }.to raise_error(described_class::ProjectMovedError, /git remote set-url origin #{project.http_url_to_repo}/) end end end diff --git a/spec/lib/gitlab/git_spec.rb b/spec/lib/gitlab/git_spec.rb index 494dfe0e595..ce15057dd7d 100644 --- a/spec/lib/gitlab/git_spec.rb +++ b/spec/lib/gitlab/git_spec.rb @@ -38,4 +38,29 @@ describe Gitlab::Git do expect(described_class.ref_name(utf8_invalid_ref)).to eq("an_invalid_ref_å") end end + + describe '.shas_eql?' do + using RSpec::Parameterized::TableSyntax + + where(:sha1, :sha2, :result) do + sha = RepoHelpers.sample_commit.id + short_sha = sha[0, Gitlab::Git::Commit::MIN_SHA_LENGTH] + too_short_sha = sha[0, Gitlab::Git::Commit::MIN_SHA_LENGTH - 1] + + [ + [sha, sha, true], + [sha, short_sha, true], + [sha, sha.reverse, false], + [sha, too_short_sha, false], + [sha, nil, false] + ] + end + + with_them do + it { expect(described_class.shas_eql?(sha1, sha2)).to eq(result) } + it 'is commutative' do + expect(described_class.shas_eql?(sha2, sha1)).to eq(result) + end + end + end end diff --git a/spec/lib/gitlab/identifier_spec.rb b/spec/lib/gitlab/identifier_spec.rb index cfaeb1f0d4f..0385dd762c2 100644 --- a/spec/lib/gitlab/identifier_spec.rb +++ b/spec/lib/gitlab/identifier_spec.rb @@ -70,6 +70,10 @@ describe Gitlab::Identifier do expect(identifier.identify_using_commit(project, '123')).to eq(user) end end + + it 'returns nil if the project & ref are not present' do + expect(identifier.identify_using_commit(nil, nil)).to be_nil + end end describe '#identify_using_user' do diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb index ef874368077..8ec3f55e6de 100644 --- a/spec/lib/gitlab/reference_extractor_spec.rb +++ b/spec/lib/gitlab/reference_extractor_spec.rb @@ -115,6 +115,15 @@ describe Gitlab::ReferenceExtractor do end end + it 'does not include anchors from table of contents in issue references' do + issue1 = create(:issue, project: project) + issue2 = create(:issue, project: project) + + subject.analyze("not real issue <h4>#{issue1.iid}</h4>, real issue #{issue2.to_reference}") + + expect(subject.issues).to match_array([issue2]) + end + it 'accesses valid issue objects' do @i0 = create(:issue, project: project) @i1 = create(:issue, project: project) diff --git a/spec/lib/gitlab/storage_check/cli_spec.rb b/spec/lib/gitlab/storage_check/cli_spec.rb new file mode 100644 index 00000000000..6db0925899c --- /dev/null +++ b/spec/lib/gitlab/storage_check/cli_spec.rb @@ -0,0 +1,19 @@ +require 'spec_helper' + +describe Gitlab::StorageCheck::CLI do + let(:options) { Gitlab::StorageCheck::Options.new('unix://tmp/socket.sock', nil, 1, false) } + subject(:runner) { described_class.new(options) } + + describe '#update_settings' do + it 'updates the interval when changed in a valid response and logs the change' do + fake_response = double + expect(fake_response).to receive(:valid?).and_return(true) + expect(fake_response).to receive(:check_interval).and_return(42) + expect(runner.logger).to receive(:info) + + runner.update_settings(fake_response) + + expect(options.interval).to eq(42) + end + end +end diff --git a/spec/lib/gitlab/storage_check/gitlab_caller_spec.rb b/spec/lib/gitlab/storage_check/gitlab_caller_spec.rb new file mode 100644 index 00000000000..d869022fd31 --- /dev/null +++ b/spec/lib/gitlab/storage_check/gitlab_caller_spec.rb @@ -0,0 +1,46 @@ +require 'spec_helper' + +describe Gitlab::StorageCheck::GitlabCaller do + let(:options) { Gitlab::StorageCheck::Options.new('unix://tmp/socket.sock', nil, nil, false) } + subject(:gitlab_caller) { described_class.new(options) } + + describe '#call!' do + context 'when a socket is given' do + it 'calls a socket' do + fake_connection = double + expect(fake_connection).to receive(:post) + expect(Excon).to receive(:new).with('unix://tmp/socket.sock', socket: "tmp/socket.sock") { fake_connection } + + gitlab_caller.call! + end + end + + context 'when a host is given' do + let(:options) { Gitlab::StorageCheck::Options.new('http://localhost:8080', nil, nil, false) } + + it 'it calls a http response' do + fake_connection = double + expect(Excon).to receive(:new).with('http://localhost:8080', socket: nil) { fake_connection } + expect(fake_connection).to receive(:post) + + gitlab_caller.call! + end + end + end + + describe '#headers' do + it 'Adds the JSON header' do + headers = gitlab_caller.headers + + expect(headers['Content-Type']).to eq('application/json') + end + + context 'when a token was provided' do + let(:options) { Gitlab::StorageCheck::Options.new('unix://tmp/socket.sock', 'atoken', nil, false) } + + it 'adds it to the headers' do + expect(gitlab_caller.headers['TOKEN']).to eq('atoken') + end + end + end +end diff --git a/spec/lib/gitlab/storage_check/option_parser_spec.rb b/spec/lib/gitlab/storage_check/option_parser_spec.rb new file mode 100644 index 00000000000..cad4dfbefcf --- /dev/null +++ b/spec/lib/gitlab/storage_check/option_parser_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe Gitlab::StorageCheck::OptionParser do + describe '.parse!' do + it 'assigns all options' do + args = %w(--target unix://tmp/hello/world.sock --token thetoken --interval 42) + + options = described_class.parse!(args) + + expect(options.token).to eq('thetoken') + expect(options.interval).to eq(42) + expect(options.target).to eq('unix://tmp/hello/world.sock') + end + + it 'requires the interval to be a number' do + args = %w(--target unix://tmp/hello/world.sock --interval fortytwo) + + expect { described_class.parse!(args) }.to raise_error(OptionParser::InvalidArgument) + end + + it 'raises an error if the scheme is not included' do + args = %w(--target tmp/hello/world.sock) + + expect { described_class.parse!(args) }.to raise_error(OptionParser::InvalidArgument) + end + + it 'raises an error if both socket and host are missing' do + expect { described_class.parse!([]) }.to raise_error(OptionParser::InvalidArgument) + end + end +end diff --git a/spec/lib/gitlab/storage_check/response_spec.rb b/spec/lib/gitlab/storage_check/response_spec.rb new file mode 100644 index 00000000000..0ff2963e443 --- /dev/null +++ b/spec/lib/gitlab/storage_check/response_spec.rb @@ -0,0 +1,54 @@ +require 'spec_helper' + +describe Gitlab::StorageCheck::Response do + let(:fake_json) do + { + check_interval: 42, + results: [ + { storage: 'working', success: true }, + { storage: 'skipped', success: nil }, + { storage: 'failing', success: false } + ] + }.to_json + end + + let(:fake_http_response) do + fake_response = instance_double("Excon::Response - Status check") + allow(fake_response).to receive(:status).and_return(200) + allow(fake_response).to receive(:body).and_return(fake_json) + allow(fake_response).to receive(:headers).and_return('Content-Type' => 'application/json') + + fake_response + end + let(:response) { described_class.new(fake_http_response) } + + describe '#valid?' do + it 'is valid for a success response with parseable JSON' do + expect(response).to be_valid + end + end + + describe '#check_interval' do + it 'returns the result from the JSON' do + expect(response.check_interval).to eq(42) + end + end + + describe '#responsive_shards' do + it 'contains the names of working shards' do + expect(response.responsive_shards).to contain_exactly('working') + end + end + + describe '#skipped_shards' do + it 'contains the names of skipped shards' do + expect(response.skipped_shards).to contain_exactly('skipped') + end + end + + describe '#failing_shards' do + it 'contains the name of failing shards' do + expect(response.failing_shards).to contain_exactly('failing') + end + end +end |