diff options
Diffstat (limited to 'spec/lib')
42 files changed, 927 insertions, 190 deletions
diff --git a/spec/lib/api/api_spec.rb b/spec/lib/api/api_spec.rb new file mode 100644 index 00000000000..ceef0b41e59 --- /dev/null +++ b/spec/lib/api/api_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +describe API::API do + describe '.prefix' do + it 'has a prefix defined' do + expect(described_class.prefix).to eq :api + end + end + + describe '.version' do + it 'uses most recent version of the API' do + expect(described_class.version).to eq 'v4' + end + end + + describe '.versions' do + it 'returns all available versions' do + expect(described_class.versions).to eq %w[v3 v4] + end + end +end diff --git a/spec/lib/api/helpers/version_spec.rb b/spec/lib/api/helpers/version_spec.rb new file mode 100644 index 00000000000..34006e0930b --- /dev/null +++ b/spec/lib/api/helpers/version_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' + +describe API::Helpers::Version do + describe '.new' do + it 'is possible to initialize it with existing API version' do + expect(described_class.new('v4').to_s).to eq 'v4' + end + + it 'raises an error when unsupported API version is provided' do + expect { described_class.new('v111') }.to raise_error ArgumentError + end + end + + describe '#root_path' do + it 'returns a root path of the API version' do + expect(described_class.new('v4').root_path).to eq '/api/v4' + end + end + + describe '#root_url' do + it 'returns an URL for a root path for the API version' do + expect(described_class.new('v4').root_url) + .to eq 'http://localhost/api/v4' + end + end +end diff --git a/spec/lib/api/helpers_spec.rb b/spec/lib/api/helpers_spec.rb index 58a49124ce6..1c73a936e17 100644 --- a/spec/lib/api/helpers_spec.rb +++ b/spec/lib/api/helpers_spec.rb @@ -148,4 +148,36 @@ describe API::Helpers do it_behaves_like 'user namespace finder' end + + describe '#send_git_blob' do + context 'content disposition' do + let(:repository) { double } + let(:blob) { double(name: 'foobar') } + + let(:send_git_blob) do + subject.send(:send_git_blob, repository, blob) + end + + before do + allow(subject).to receive(:env).and_return({}) + allow(subject).to receive(:content_type) + allow(subject).to receive(:header).and_return({}) + allow(Gitlab::Workhorse).to receive(:send_git_blob) + end + + context 'when blob name is null' do + let(:blob) { double(name: nil) } + + it 'returns only the disposition' do + expect(send_git_blob['Content-Disposition']).to eq 'attachment' + end + end + + context 'when blob name is not null' do + it 'returns disposition with the blob name' do + expect(send_git_blob['Content-Disposition']).to eq 'attachment; filename="foobar"' + end + end + end + end end diff --git a/spec/lib/banzai/filter/external_link_filter_spec.rb b/spec/lib/banzai/filter/external_link_filter_spec.rb index 2a3c0cd78b8..e6dae8d5382 100644 --- a/spec/lib/banzai/filter/external_link_filter_spec.rb +++ b/spec/lib/banzai/filter/external_link_filter_spec.rb @@ -49,16 +49,16 @@ describe Banzai::Filter::ExternalLinkFilter do end context 'for invalid urls' do - it 'skips broken hrefs' do + it 'adds rel and target attributes to broken hrefs' do doc = filter %q(<p><a href="don't crash on broken urls">Google</a></p>) - expected = %q(<p><a href="don't%20crash%20on%20broken%20urls">Google</a></p>) + expected = %q(<p><a href="don't%20crash%20on%20broken%20urls" rel="nofollow noreferrer noopener" target="_blank">Google</a></p>) expect(doc.to_html).to eq(expected) end - it 'skips improperly formatted mailtos' do + it 'adds rel and target to improperly formatted mailtos' do doc = filter %q(<p><a href="mailto://jblogs@example.com">Email</a></p>) - expected = %q(<p><a href="mailto://jblogs@example.com">Email</a></p>) + expected = %q(<p><a href="mailto://jblogs@example.com" rel="nofollow noreferrer noopener" target="_blank">Email</a></p>) expect(doc.to_html).to eq(expected) end diff --git a/spec/lib/banzai/filter/label_reference_filter_spec.rb b/spec/lib/banzai/filter/label_reference_filter_spec.rb index 00257ed7904..9cfdb9e53a2 100644 --- a/spec/lib/banzai/filter/label_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/label_reference_filter_spec.rb @@ -236,6 +236,24 @@ describe Banzai::Filter::LabelReferenceFilter do end end + context 'References with html entities' do + let!(:label) { create(:label, name: '<html>', project: project) } + + it 'links to a valid reference' do + doc = reference_filter('See ~"<html>"') + + expect(doc.css('a').first.attr('href')).to eq urls + .project_issues_url(project, label_name: label.name) + expect(doc.text).to eq 'See <html>' + end + + it 'ignores invalid label names and escapes entities' do + act = %(Label #{Label.reference_prefix}"<non valid>") + + expect(reference_filter(act).to_html).to eq act + end + end + describe 'consecutive references' do let(:bug) { create(:label, name: 'bug', project: project) } let(:feature_proposal) { create(:label, name: 'feature proposal', project: project) } diff --git a/spec/lib/banzai/filter/milestone_reference_filter_spec.rb b/spec/lib/banzai/filter/milestone_reference_filter_spec.rb index 1a87cfa5b45..4c94e4fdae0 100644 --- a/spec/lib/banzai/filter/milestone_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/milestone_reference_filter_spec.rb @@ -59,7 +59,7 @@ describe Banzai::Filter::MilestoneReferenceFilter do it 'links with adjacent text' do doc = reference_filter("Milestone (#{reference}.)") - expect(doc.to_html).to match(%r(\(<a.+>#{milestone.name}</a>\.\))) + expect(doc.to_html).to match(%r(\(<a.+>#{milestone.reference_link_text}</a>\.\))) end it 'ignores invalid milestone IIDs' do @@ -80,12 +80,12 @@ describe Banzai::Filter::MilestoneReferenceFilter do doc = reference_filter("See #{reference}") expect(doc.css('a').first.attr('href')).to eq urls.milestone_url(milestone) - expect(doc.text).to eq 'See gfm' + expect(doc.text).to eq "See #{milestone.reference_link_text}" end it 'links with adjacent text' do doc = reference_filter("Milestone (#{reference}.)") - expect(doc.to_html).to match(%r(\(<a.+>#{milestone.name}</a>\.\))) + expect(doc.to_html).to match(%r(\(<a.+>#{milestone.reference_link_text}</a>\.\))) end it 'ignores invalid milestone names' do @@ -106,12 +106,12 @@ describe Banzai::Filter::MilestoneReferenceFilter do doc = reference_filter("See #{reference}") expect(doc.css('a').first.attr('href')).to eq urls.milestone_url(milestone) - expect(doc.text).to eq 'See gfm references' + expect(doc.text).to eq "See #{milestone.reference_link_text}" end it 'links with adjacent text' do doc = reference_filter("Milestone (#{reference}.)") - expect(doc.to_html).to match(%r(\(<a.+>#{milestone.name}</a>\.\))) + expect(doc.to_html).to match(%r(\(<a.+>#{milestone.reference_link_text}</a>\.\))) end it 'ignores invalid milestone names' do @@ -201,14 +201,14 @@ describe Banzai::Filter::MilestoneReferenceFilter do doc = reference_filter("See (#{reference}.)") expect(doc.css('a').first.text) - .to eq("#{milestone.name} in #{another_project.full_path}") + .to eq("#{milestone.reference_link_text} in #{another_project.full_path}") end it 'has valid text' do doc = reference_filter("See (#{reference}.)") expect(doc.text) - .to eq("See (#{milestone.name} in #{another_project.full_path}.)") + .to eq("See (#{milestone.reference_link_text} in #{another_project.full_path}.)") end it 'escapes the name attribute' do @@ -217,7 +217,7 @@ describe Banzai::Filter::MilestoneReferenceFilter do doc = reference_filter("See #{reference}") expect(doc.css('a').first.text) - .to eq "#{milestone.name} in #{another_project.full_path}" + .to eq "#{milestone.reference_link_text} in #{another_project.full_path}" end end @@ -238,14 +238,14 @@ describe Banzai::Filter::MilestoneReferenceFilter do doc = reference_filter("See (#{reference}.)") expect(doc.css('a').first.text) - .to eq("#{milestone.name} in #{another_project.path}") + .to eq("#{milestone.reference_link_text} in #{another_project.path}") end it 'has valid text' do doc = reference_filter("See (#{reference}.)") expect(doc.text) - .to eq("See (#{milestone.name} in #{another_project.path}.)") + .to eq("See (#{milestone.reference_link_text} in #{another_project.path}.)") end it 'escapes the name attribute' do @@ -254,7 +254,7 @@ describe Banzai::Filter::MilestoneReferenceFilter do doc = reference_filter("See #{reference}") expect(doc.css('a').first.text) - .to eq "#{milestone.name} in #{another_project.path}" + .to eq "#{milestone.reference_link_text} in #{another_project.path}" end end @@ -275,14 +275,14 @@ describe Banzai::Filter::MilestoneReferenceFilter do doc = reference_filter("See (#{reference}.)") expect(doc.css('a').first.text) - .to eq("#{milestone.name} in #{another_project.path}") + .to eq("#{milestone.reference_link_text} in #{another_project.path}") end it 'has valid text' do doc = reference_filter("See (#{reference}.)") expect(doc.text) - .to eq("See (#{milestone.name} in #{another_project.path}.)") + .to eq("See (#{milestone.reference_link_text} in #{another_project.path}.)") end it 'escapes the name attribute' do @@ -291,7 +291,7 @@ describe Banzai::Filter::MilestoneReferenceFilter do doc = reference_filter("See #{reference}") expect(doc.css('a').first.text) - .to eq "#{milestone.name} in #{another_project.path}" + .to eq "#{milestone.reference_link_text} in #{another_project.path}" end end @@ -346,7 +346,7 @@ describe Banzai::Filter::MilestoneReferenceFilter do milestone.update!(group: parent_group) doc = reference_filter("See #{reference}") - expect(doc.css('a').first.text).to eq(milestone.name) + expect(doc.css('a').first.text).to eq(milestone.reference_link_text) end end diff --git a/spec/lib/gitlab/blob_helper_spec.rb b/spec/lib/gitlab/blob_helper_spec.rb index 0b56f8687c3..e057385b35f 100644 --- a/spec/lib/gitlab/blob_helper_spec.rb +++ b/spec/lib/gitlab/blob_helper_spec.rb @@ -53,11 +53,11 @@ describe Gitlab::BlobHelper do describe '#text?' do it 'returns true' do - expect(blob.text?).to be_truthy + expect(blob.text_in_repo?).to be_truthy end it 'returns false' do - expect(large_blob.text?).to be_falsey + expect(large_blob.text_in_repo?).to be_falsey end end diff --git a/spec/lib/gitlab/checks/diff_check_spec.rb b/spec/lib/gitlab/checks/diff_check_spec.rb index eeec1e83179..a341dfa5636 100644 --- a/spec/lib/gitlab/checks/diff_check_spec.rb +++ b/spec/lib/gitlab/checks/diff_check_spec.rb @@ -47,5 +47,43 @@ describe Gitlab::Checks::DiffCheck do end end end + + context 'commit diff validations' do + before do + allow(subject).to receive(:validations_for_diff).and_return([lambda { |diff| return }]) + + expect_any_instance_of(Commit).to receive(:raw_deltas).and_call_original + + subject.validate! + end + + context 'when request store is inactive' do + it 'are run for every commit' do + expect_any_instance_of(Commit).to receive(:raw_deltas).and_call_original + + subject.validate! + end + end + + context 'when request store is active', :request_store do + it 'are cached for every commit' do + expect_any_instance_of(Commit).not_to receive(:raw_deltas) + + subject.validate! + end + + it 'are run for not cached commits' do + allow(project.repository).to receive(:new_commits).and_return( + project.repository.commits_between('be93687618e4b132087f430a4d8fc3a609c9b77c', 'a5391128b0ef5d21df5dd23d98557f4ef12fae20') + ) + change_access.instance_variable_set(:@commits, project.repository.new_commits) + + expect(project.repository.new_commits.first).not_to receive(:raw_deltas).and_call_original + expect(project.repository.new_commits.last).to receive(:raw_deltas).and_call_original + + subject.validate! + end + end + end end end diff --git a/spec/lib/gitlab/checks/push_check_spec.rb b/spec/lib/gitlab/checks/push_check_spec.rb index 25f0d428cb9..e1bd52d6c0b 100644 --- a/spec/lib/gitlab/checks/push_check_spec.rb +++ b/spec/lib/gitlab/checks/push_check_spec.rb @@ -13,7 +13,7 @@ describe Gitlab::Checks::PushCheck do context 'when the user is not allowed to push to the repo' do it 'raises an error' do expect(user_access).to receive(:can_do_action?).with(:push_code).and_return(false) - expect(user_access).to receive(:can_push_to_branch?).with('master').and_return(false) + expect(project).to receive(:branch_allows_collaboration?).with(user_access.user, 'master').and_return(false) expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to push code to this project.') end diff --git a/spec/lib/gitlab/ci/config/external/file/base_spec.rb b/spec/lib/gitlab/ci/config/external/file/base_spec.rb index 2e92d5204d6..ada8775c489 100644 --- a/spec/lib/gitlab/ci/config/external/file/base_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/base_spec.rb @@ -3,13 +3,43 @@ require 'fast_spec_helper' describe Gitlab::Ci::Config::External::File::Base do - subject { described_class.new(location) } + let(:context) { described_class::Context.new(nil, 'HEAD') } + + let(:test_class) do + Class.new(described_class) do + def initialize(params, context = {}) + @location = params + + super + end + end + end + + subject { test_class.new(location, context) } before do - allow_any_instance_of(described_class) + allow_any_instance_of(test_class) .to receive(:content).and_return('key: value') end + describe '#matching?' do + context 'when a location is present' do + let(:location) { 'some-location' } + + it 'should return true' do + expect(subject).to be_matching + end + end + + context 'with a location is missing' do + let(:location) { nil } + + it 'should return false' do + expect(subject).not_to be_matching + end + end + end + describe '#valid?' do context 'when location is not a YAML file' do let(:location) { 'some/file.txt' } @@ -39,7 +69,7 @@ describe Gitlab::Ci::Config::External::File::Base do let(:location) { 'some/file/config.yml' } before do - allow_any_instance_of(described_class) + allow_any_instance_of(test_class) .to receive(:content).and_return('invalid_syntax') end diff --git a/spec/lib/gitlab/ci/config/external/file/local_spec.rb b/spec/lib/gitlab/ci/config/external/file/local_spec.rb index 541deb13b97..83be43e240b 100644 --- a/spec/lib/gitlab/ci/config/external/file/local_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/local_spec.rb @@ -3,8 +3,37 @@ require 'spec_helper' describe Gitlab::Ci::Config::External::File::Local do - let(:project) { create(:project, :repository) } - let(:local_file) { described_class.new(location, { project: project, sha: '12345' }) } + set(:project) { create(:project, :repository) } + + let(:context) { described_class::Context.new(project, '12345') } + let(:params) { { local: location } } + let(:local_file) { described_class.new(params, context) } + + describe '#matching?' do + context 'when a local is specified' do + let(:params) { { local: 'file' } } + + it 'should return true' do + expect(local_file).to be_matching + end + end + + context 'with a missing local' do + let(:params) { { local: nil } } + + it 'should return false' do + expect(local_file).not_to be_matching + end + end + + context 'with a missing local key' do + let(:params) { {} } + + it 'should return false' do + expect(local_file).not_to be_matching + end + end + end describe '#valid?' do context 'when is a valid local path' do @@ -44,7 +73,6 @@ describe Gitlab::Ci::Config::External::File::Local do - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs - ruby -v - which ruby - - gem install bundler --no-ri --no-rdoc - bundle install --jobs $(nproc) "${FLAGS[@]}" HEREDOC end diff --git a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb index 7c1a1c38736..319e7137f9f 100644 --- a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb @@ -3,7 +3,9 @@ require 'spec_helper' describe Gitlab::Ci::Config::External::File::Remote do - let(:remote_file) { described_class.new(location) } + let(:context) { described_class::Context.new(nil, '12345') } + let(:params) { { remote: location } } + let(:remote_file) { described_class.new(params, context) } let(:location) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } let(:remote_file_content) do <<~HEREDOC @@ -11,11 +13,36 @@ describe Gitlab::Ci::Config::External::File::Remote do - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs - ruby -v - which ruby - - gem install bundler --no-ri --no-rdoc - bundle install --jobs $(nproc) "${FLAGS[@]}" HEREDOC end + describe '#matching?' do + context 'when a remote is specified' do + let(:params) { { remote: 'http://remote' } } + + it 'should return true' do + expect(remote_file).to be_matching + end + end + + context 'with a missing remote' do + let(:params) { { remote: nil } } + + it 'should return false' do + expect(remote_file).not_to be_matching + end + end + + context 'with a missing remote key' do + let(:params) { {} } + + it 'should return false' do + expect(remote_file).not_to be_matching + end + end + end + describe "#valid?" do context 'when is a valid remote url' do before do diff --git a/spec/lib/gitlab/ci/config/external/file/template_spec.rb b/spec/lib/gitlab/ci/config/external/file/template_spec.rb new file mode 100644 index 00000000000..1fb5655309a --- /dev/null +++ b/spec/lib/gitlab/ci/config/external/file/template_spec.rb @@ -0,0 +1,93 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Ci::Config::External::File::Template do + let(:context) { described_class::Context.new(nil, '12345') } + let(:template) { 'Auto-DevOps.gitlab-ci.yml' } + let(:params) { { template: template } } + + subject { described_class.new(params, context) } + + describe '#matching?' do + context 'when a template is specified' do + let(:params) { { template: 'some-template' } } + + it 'should return true' do + expect(subject).to be_matching + end + end + + context 'with a missing template' do + let(:params) { { template: nil } } + + it 'should return false' do + expect(subject).not_to be_matching + end + end + + context 'with a missing template key' do + let(:params) { {} } + + it 'should return false' do + expect(subject).not_to be_matching + end + end + end + + describe "#valid?" do + context 'when is a valid template name' do + let(:template) { 'Auto-DevOps.gitlab-ci.yml' } + + it 'should return true' do + expect(subject).to be_valid + end + end + + context 'with invalid template name' do + let(:template) { 'Template.yml' } + + it 'should return false' do + expect(subject).not_to be_valid + expect(subject.error_message).to include('Template file `Template.yml` is not a valid location!') + end + end + + context 'with a non-existing template' do + let(:template) { 'I-Do-Not-Have-This-Template.gitlab-ci.yml' } + + it 'should return false' do + expect(subject).not_to be_valid + expect(subject.error_message).to include('Included file `I-Do-Not-Have-This-Template.gitlab-ci.yml` is empty or does not exist!') + end + end + end + + describe '#template_name' do + let(:template_name) { subject.send(:template_name) } + + context 'when template does end with .gitlab-ci.yml' do + let(:template) { 'my-template.gitlab-ci.yml' } + + it 'returns template name' do + expect(template_name).to eq('my-template') + end + end + + context 'when template is nil' do + let(:template) { nil } + + it 'returns nil' do + expect(template_name).to be_nil + end + end + + context 'when template does not end with .gitlab-ci.yml' do + let(:template) { 'my-template' } + + it 'returns nil' do + expect(template_name).to be_nil + end + end + end +end diff --git a/spec/lib/gitlab/ci/config/external/mapper_spec.rb b/spec/lib/gitlab/ci/config/external/mapper_spec.rb index 5b236fe99f1..e27d2cd9422 100644 --- a/spec/lib/gitlab/ci/config/external/mapper_spec.rb +++ b/spec/lib/gitlab/ci/config/external/mapper_spec.rb @@ -3,84 +3,130 @@ require 'spec_helper' describe Gitlab::Ci::Config::External::Mapper do - let(:project) { create(:project, :repository) } + set(:project) { create(:project, :repository) } + + let(:local_file) { '/lib/gitlab/ci/templates/non-existent-file.yml' } + let(:remote_url) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } + let(:template_file) { 'Auto-DevOps.gitlab-ci.yml' } + let(:file_content) do <<~HEREDOC image: 'ruby:2.2' HEREDOC end + before do + WebMock.stub_request(:get, remote_url).to_return(body: file_content) + end + describe '#process' do - subject { described_class.new(values, project, '123456').process } + subject { described_class.new(values, project: project, sha: '123456').process } - context "when 'include' keyword is defined as string" do + context "when single 'include' keyword is defined" do context 'when the string is a local file' do let(:values) do - { - include: '/lib/gitlab/ci/templates/non-existent-file.yml', - image: 'ruby:2.2' - } + { include: local_file, + image: 'ruby:2.2' } + end + + it 'returns File instances' do + expect(subject).to contain_exactly( + an_instance_of(Gitlab::Ci::Config::External::File::Local)) end + end - it 'returns an array' do - expect(subject).to be_an(Array) + context 'when the key is a local file hash' do + let(:values) do + { include: { 'local' => local_file }, + image: 'ruby:2.2' } end it 'returns File instances' do - expect(subject.first) - .to be_an_instance_of(Gitlab::Ci::Config::External::File::Local) + expect(subject).to contain_exactly( + an_instance_of(Gitlab::Ci::Config::External::File::Local)) end end context 'when the string is a remote file' do - let(:remote_url) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } let(:values) do - { - include: remote_url, - image: 'ruby:2.2' - } + { include: remote_url, image: 'ruby:2.2' } + end + + it 'returns File instances' do + expect(subject).to contain_exactly( + an_instance_of(Gitlab::Ci::Config::External::File::Remote)) + end + end + + context 'when the key is a remote file hash' do + let(:values) do + { include: { 'remote' => remote_url }, + image: 'ruby:2.2' } end - before do - WebMock.stub_request(:get, remote_url).to_return(body: file_content) + it 'returns File instances' do + expect(subject).to contain_exactly( + an_instance_of(Gitlab::Ci::Config::External::File::Remote)) end + end - it 'returns an array' do - expect(subject).to be_an(Array) + context 'when the key is a template file hash' do + let(:values) do + { include: { 'template' => template_file }, + image: 'ruby:2.2' } end it 'returns File instances' do - expect(subject.first) - .to be_an_instance_of(Gitlab::Ci::Config::External::File::Remote) + expect(subject).to contain_exactly( + an_instance_of(Gitlab::Ci::Config::External::File::Template)) + end + end + + context 'when the key is a hash of file and remote' do + let(:values) do + { include: { 'local' => local_file, 'remote' => remote_url }, + image: 'ruby:2.2' } + end + + it 'returns ambigious specification error' do + expect { subject }.to raise_error(described_class::AmbigiousSpecificationError) end end end context "when 'include' is defined as an array" do - let(:remote_url) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } let(:values) do - { - include: - [ - remote_url, - '/lib/gitlab/ci/templates/template.yml' - ], - image: 'ruby:2.2' - } + { include: [remote_url, local_file], + image: 'ruby:2.2' } end - before do - WebMock.stub_request(:get, remote_url).to_return(body: file_content) + it 'returns Files instances' do + expect(subject).to all(respond_to(:valid?)) + expect(subject).to all(respond_to(:content)) end + end - it 'returns an array' do - expect(subject).to be_an(Array) + context "when 'include' is defined as an array of hashes" do + let(:values) do + { include: [{ remote: remote_url }, { local: local_file }], + image: 'ruby:2.2' } end it 'returns Files instances' do expect(subject).to all(respond_to(:valid?)) expect(subject).to all(respond_to(:content)) end + + context 'when it has ambigious match' do + let(:values) do + { include: [{ remote: remote_url, local: local_file }], + image: 'ruby:2.2' } + end + + it 'returns ambigious specification error' do + expect { subject }.to raise_error(described_class::AmbigiousSpecificationError) + end + end end context "when 'include' is not defined" do diff --git a/spec/lib/gitlab/ci/config/external/processor_spec.rb b/spec/lib/gitlab/ci/config/external/processor_spec.rb index 1a05f716247..d2d4fbc5115 100644 --- a/spec/lib/gitlab/ci/config/external/processor_spec.rb +++ b/spec/lib/gitlab/ci/config/external/processor_spec.rb @@ -3,8 +3,9 @@ require 'spec_helper' describe Gitlab::Ci::Config::External::Processor do - let(:project) { create(:project, :repository) } - let(:processor) { described_class.new(values, project, '12345') } + set(:project) { create(:project, :repository) } + + let(:processor) { described_class.new(values, project: project, sha: '12345') } describe "#perform" do context 'when no external files defined' do @@ -51,7 +52,6 @@ describe Gitlab::Ci::Config::External::Processor do - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs - ruby -v - which ruby - - gem install bundler --no-ri --no-rdoc - bundle install --jobs $(nproc) "${FLAGS[@]}" rspec: @@ -86,7 +86,6 @@ describe Gitlab::Ci::Config::External::Processor do - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs - ruby -v - which ruby - - gem install bundler --no-ri --no-rdoc - bundle install --jobs $(nproc) "${FLAGS[@]}" HEREDOC end diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb index 975e11e8cc1..49988468d1a 100644 --- a/spec/lib/gitlab/ci/config_spec.rb +++ b/spec/lib/gitlab/ci/config_spec.rb @@ -170,7 +170,6 @@ describe Gitlab::Ci::Config do before_script_values = [ "apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs", "ruby -v", "which ruby", - "gem install bundler --no-ri --no-rdoc", "bundle install --jobs $(nproc) \"${FLAGS[@]}\"" ] variables = { @@ -206,6 +205,23 @@ describe Gitlab::Ci::Config do end end + context "when gitlab_ci.yml has ambigious 'include' defined" do + let(:gitlab_ci_yml) do + <<~HEREDOC + include: + remote: http://url + local: /local/file.yml + HEREDOC + end + + it 'raises error YamlProcessor validationError' do + expect { config }.to raise_error( + described_class::ConfigError, + 'Include `{"remote":"http://url","local":"/local/file.yml"}` needs to match exactly one accessor!' + ) + end + end + describe 'external file version' do context 'when external local file SHA is defined' do it 'is using a defined value' do diff --git a/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb index 75a177d2d1f..6aa802ce6fd 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb @@ -182,4 +182,24 @@ describe Gitlab::Ci::Pipeline::Chain::Command do it { is_expected.to eq(false) } end end + + describe '#ambiguous_ref' do + let(:project) { create(:project, :repository) } + let(:command) { described_class.new(project: project, origin_ref: 'ref') } + + subject { command.ambiguous_ref? } + + context 'when ref is not ambiguous' do + it { is_expected. to eq(false) } + end + + context 'when ref is ambiguous' do + before do + project.repository.add_tag(project.creator, 'ref', 'master') + project.repository.add_branch(project.creator, 'ref', 'master') + end + + it { is_expected. to eq(true) } + end + end end diff --git a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb index 284aed91e29..1b014ecfaa4 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb @@ -14,6 +14,7 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do Gitlab::Ci::Pipeline::Chain::Command.new( project: project, current_user: user, + origin_ref: 'master', seeds_block: nil) end @@ -106,6 +107,7 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do Gitlab::Ci::Pipeline::Chain::Command.new( project: project, current_user: user, + origin_ref: 'master', seeds_block: seeds_block) end 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 fb1b53fc55c..a7cad423d09 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb @@ -42,6 +42,27 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Repository do end end + context 'when ref is ambiguous' do + let(:project) do + create(:project, :repository).tap do |proj| + proj.repository.add_tag(user, 'master', 'master') + end + end + let(:command) do + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, current_user: user, origin_ref: 'master') + end + + it 'breaks the chain' do + expect(step.break?).to be true + end + + it 'adds an error about missing ref' do + expect(pipeline.errors.to_a) + .to include 'Ref is ambiguous' + end + end + context 'when does not have existing SHA set' do let(:command) do Gitlab::Ci::Pipeline::Chain::Command.new( diff --git a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb index fffa727c2ed..2cf812b26dc 100644 --- a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb @@ -1,7 +1,8 @@ require 'spec_helper' describe Gitlab::Ci::Pipeline::Seed::Build do - let(:pipeline) { create(:ci_empty_pipeline) } + let(:project) { create(:project, :repository) } + let(:pipeline) { create(:ci_empty_pipeline, project: project) } let(:attributes) do { name: 'rspec', diff --git a/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb index 05ce3412fd8..82f741845db 100644 --- a/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb @@ -1,7 +1,8 @@ require 'spec_helper' describe Gitlab::Ci::Pipeline::Seed::Stage do - let(:pipeline) { create(:ci_empty_pipeline) } + let(:project) { create(:project, :repository) } + let(:pipeline) { create(:ci_empty_pipeline, project: project) } let(:attributes) do { name: 'test', diff --git a/spec/lib/gitlab/cleanup/remote_uploads_spec.rb b/spec/lib/gitlab/cleanup/remote_uploads_spec.rb index 8d03baeb07b..35642cd6e50 100644 --- a/spec/lib/gitlab/cleanup/remote_uploads_spec.rb +++ b/spec/lib/gitlab/cleanup/remote_uploads_spec.rb @@ -25,7 +25,7 @@ describe Gitlab::Cleanup::RemoteUploads do expect(::Fog::Storage).to receive(:new).and_return(connection) - expect(connection).to receive(:directories).and_return(double(get: directory)) + expect(connection).to receive(:directories).and_return(double(new: directory)) expect(directory).to receive(:files).and_return(remote_files) end diff --git a/spec/lib/gitlab/current_settings_spec.rb b/spec/lib/gitlab/current_settings_spec.rb index 55490f37ac7..caf9fc5442c 100644 --- a/spec/lib/gitlab/current_settings_spec.rb +++ b/spec/lib/gitlab/current_settings_spec.rb @@ -54,7 +54,7 @@ describe Gitlab::CurrentSettings do expect(ApplicationSetting).not_to receive(:current) end - it 'returns an in-memory ApplicationSetting object' do + it 'returns a FakeApplicationSettings object' do expect(described_class.current_application_settings).to be_a(Gitlab::FakeApplicationSettings) end @@ -91,6 +91,14 @@ describe Gitlab::CurrentSettings do allow(ActiveRecord::Base.connection).to receive(:cached_table_exists?).with('application_settings').and_return(true) end + context 'with RequestStore enabled', :request_store do + it 'fetches the settings from DB only once' do + described_class.current_application_settings # warm the cache + + expect(ActiveRecord::QueryRecorder.new { described_class.current_application_settings }.count).to eq(0) + end + end + it 'creates default ApplicationSettings if none are present' do settings = described_class.current_application_settings @@ -99,34 +107,45 @@ describe Gitlab::CurrentSettings do expect(settings).to have_attributes(settings_from_defaults) end - context 'with migrations pending' do + context 'with pending migrations' do before do expect(ActiveRecord::Migrator).to receive(:needs_migration?).and_return(true) end - it 'returns an in-memory ApplicationSetting object' do - settings = described_class.current_application_settings + shared_examples 'a non-persisted ApplicationSetting object' do + let(:current_settings) { described_class.current_application_settings } + + it 'returns a non-persisted ApplicationSetting object' do + expect(current_settings).to be_a(ApplicationSetting) + expect(current_settings).not_to be_persisted + end + + it 'uses the default value from ApplicationSetting.defaults' do + expect(current_settings.signup_enabled).to eq(ApplicationSetting.defaults[:signup_enabled]) + end + + it 'uses the default value from custom ApplicationSetting accessors' do + expect(current_settings.commit_email_hostname).to eq(ApplicationSetting.default_commit_email_hostname) + end + + it 'responds to predicate methods' do + expect(current_settings.signup_enabled?).to eq(current_settings.signup_enabled) + end + end - expect(settings).to be_a(Gitlab::FakeApplicationSettings) - expect(settings.sign_in_enabled?).to eq(settings.sign_in_enabled) - expect(settings.sign_up_enabled?).to eq(settings.sign_up_enabled) + context 'with no ApplicationSetting DB record' do + it_behaves_like 'a non-persisted ApplicationSetting object' end - it 'uses the existing database settings and falls back to defaults' do - db_settings = create(:application_setting, - home_page_url: 'http://mydomain.com', - signup_enabled: false) - settings = described_class.current_application_settings - app_defaults = ApplicationSetting.last - - expect(settings).to be_a(Gitlab::FakeApplicationSettings) - expect(settings.home_page_url).to eq(db_settings.home_page_url) - expect(settings.signup_enabled?).to be_falsey - expect(settings.signup_enabled).to be_falsey - - # Check that unspecified values use the defaults - settings.reject! { |key, _| [:home_page_url, :signup_enabled].include? key } - settings.each { |key, _| expect(settings[key]).to eq(app_defaults[key]) } + context 'with an existing ApplicationSetting DB record' do + let!(:db_settings) { ApplicationSetting.build_from_defaults(home_page_url: 'http://mydomain.com').save! && ApplicationSetting.last } + let(:current_settings) { described_class.current_application_settings } + + it_behaves_like 'a non-persisted ApplicationSetting object' + + it 'uses the value from the DB attribute if present and not overriden by an accessor' do + expect(current_settings.home_page_url).to eq(db_settings.home_page_url) + end end end @@ -138,17 +157,12 @@ describe Gitlab::CurrentSettings do end end - context 'when the application_settings table does not exists' do - it 'returns an in-memory ApplicationSetting object' do - expect(ApplicationSetting).to receive(:create_from_defaults).and_raise(ActiveRecord::StatementInvalid) - - expect(described_class.current_application_settings).to be_a(Gitlab::FakeApplicationSettings) - end - end - - context 'when the application_settings table is not fully migrated' do - it 'returns an in-memory ApplicationSetting object' do - expect(ApplicationSetting).to receive(:create_from_defaults).and_raise(ActiveRecord::UnknownAttributeError) + context 'when the application_settings table does not exist' do + it 'returns a FakeApplicationSettings object' do + expect(Gitlab::Database) + .to receive(:cached_table_exists?) + .with('application_settings') + .and_return(false) expect(described_class.current_application_settings).to be_a(Gitlab::FakeApplicationSettings) end diff --git a/spec/lib/gitlab/diff/file_spec.rb b/spec/lib/gitlab/diff/file_spec.rb index b15d22c634a..862590268ca 100644 --- a/spec/lib/gitlab/diff/file_spec.rb +++ b/spec/lib/gitlab/diff/file_spec.rb @@ -310,7 +310,7 @@ describe Gitlab::Diff::File do context 'when the content changed' do context 'when the file represented by the diff file is binary' do before do - allow(diff_file).to receive(:raw_binary?).and_return(true) + allow(diff_file).to receive(:binary?).and_return(true) end it 'returns a No Preview viewer' do @@ -345,7 +345,7 @@ describe Gitlab::Diff::File do context 'when the file represented by the diff file is binary' do before do - allow(diff_file).to receive(:raw_binary?).and_return(true) + allow(diff_file).to receive(:binary?).and_return(true) end it 'returns an Added viewer' do @@ -380,7 +380,7 @@ describe Gitlab::Diff::File do context 'when the file represented by the diff file is binary' do before do - allow(diff_file).to receive(:raw_binary?).and_return(true) + allow(diff_file).to receive(:binary?).and_return(true) end it 'returns a Deleted viewer' do @@ -436,7 +436,7 @@ describe Gitlab::Diff::File do allow(diff_file).to receive(:deleted_file?).and_return(false) allow(diff_file).to receive(:renamed_file?).and_return(false) allow(diff_file).to receive(:mode_changed?).and_return(false) - allow(diff_file).to receive(:raw_text?).and_return(false) + allow(diff_file).to receive(:text?).and_return(false) end it 'returns a No Preview viewer' do diff --git a/spec/lib/gitlab/diff/lines_unfolder_spec.rb b/spec/lib/gitlab/diff/lines_unfolder_spec.rb index 8e00c8e0e30..f22c2c90334 100644 --- a/spec/lib/gitlab/diff/lines_unfolder_spec.rb +++ b/spec/lib/gitlab/diff/lines_unfolder_spec.rb @@ -185,7 +185,7 @@ describe Gitlab::Diff::LinesUnfolder do let(:project) { create(:project) } - let(:old_blob) { Gitlab::Git::Blob.new(data: raw_old_blob) } + let(:old_blob) { Blob.decorate(Gitlab::Git::Blob.new(data: raw_old_blob, size: 10)) } let(:diff) do Gitlab::Git::Diff.new(diff: raw_diff, diff --git a/spec/lib/gitlab/discussions_diff/file_collection_spec.rb b/spec/lib/gitlab/discussions_diff/file_collection_spec.rb new file mode 100644 index 00000000000..0489206458b --- /dev/null +++ b/spec/lib/gitlab/discussions_diff/file_collection_spec.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::DiscussionsDiff::FileCollection do + let(:merge_request) { create(:merge_request) } + let!(:diff_note_a) { create(:diff_note_on_merge_request, project: merge_request.project, noteable: merge_request) } + let!(:diff_note_b) { create(:diff_note_on_merge_request, project: merge_request.project, noteable: merge_request) } + let(:note_diff_file_a) { diff_note_a.note_diff_file } + let(:note_diff_file_b) { diff_note_b.note_diff_file } + + subject { described_class.new([note_diff_file_a, note_diff_file_b]) } + + describe '#load_highlight', :clean_gitlab_redis_shared_state do + it 'writes uncached diffs highlight' do + file_a_caching_content = diff_note_a.diff_file.highlighted_diff_lines.map(&:to_hash) + file_b_caching_content = diff_note_b.diff_file.highlighted_diff_lines.map(&:to_hash) + + expect(Gitlab::DiscussionsDiff::HighlightCache) + .to receive(:write_multiple) + .with({ note_diff_file_a.id => file_a_caching_content, + note_diff_file_b.id => file_b_caching_content }) + .and_call_original + + subject.load_highlight([note_diff_file_a.id, note_diff_file_b.id]) + end + + it 'does not write cache for already cached file' do + subject.load_highlight([note_diff_file_a.id]) + + file_b_caching_content = diff_note_b.diff_file.highlighted_diff_lines.map(&:to_hash) + + expect(Gitlab::DiscussionsDiff::HighlightCache) + .to receive(:write_multiple) + .with({ note_diff_file_b.id => file_b_caching_content }) + .and_call_original + + subject.load_highlight([note_diff_file_a.id, note_diff_file_b.id]) + end + + it 'does not err when given ID does not exist in @collection' do + expect { subject.load_highlight([999]) }.not_to raise_error + end + + it 'loaded diff files have highlighted lines loaded' do + subject.load_highlight([note_diff_file_a.id]) + + diff_file = subject.find_by_id(note_diff_file_a.id) + + expect(diff_file.highlight_loaded?).to be(true) + end + + it 'not loaded diff files does not have highlighted lines loaded' do + subject.load_highlight([note_diff_file_a.id]) + + diff_file = subject.find_by_id(note_diff_file_b.id) + + expect(diff_file.highlight_loaded?).to be(false) + end + end +end diff --git a/spec/lib/gitlab/discussions_diff/highlight_cache_spec.rb b/spec/lib/gitlab/discussions_diff/highlight_cache_spec.rb new file mode 100644 index 00000000000..fe26ebb8796 --- /dev/null +++ b/spec/lib/gitlab/discussions_diff/highlight_cache_spec.rb @@ -0,0 +1,102 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::DiscussionsDiff::HighlightCache, :clean_gitlab_redis_cache do + describe '#write_multiple' do + it 'sets multiple keys serializing content as JSON' do + mapping = { + 3 => [ + { + text: 'foo', + type: 'new', + index: 2, + old_pos: 10, + new_pos: 11, + line_code: 'xpto', + rich_text: '<blips>blops</blips>' + }, + { + text: 'foo', + type: 'new', + index: 3, + old_pos: 11, + new_pos: 12, + line_code: 'xpto', + rich_text: '<blops>blips</blops>' + } + ] + } + + 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_json) + end + end + end + + describe '#read_multiple' do + it 'reads multiple keys and serializes content into Gitlab::Diff::Line objects' do + mapping = { + 3 => [ + { + text: 'foo', + type: 'new', + index: 2, + old_pos: 11, + new_pos: 12, + line_code: 'xpto', + rich_text: '<blips>blops</blips>' + }, + { + text: 'foo', + type: 'new', + index: 3, + old_pos: 10, + new_pos: 11, + line_code: 'xpto', + rich_text: '<blips>blops</blips>' + } + ] + } + + described_class.write_multiple(mapping) + + found = described_class.read_multiple(mapping.keys) + + expect(found.size).to eq(1) + expect(found.first.size).to eq(2) + expect(found.first).to all(be_a(Gitlab::Diff::Line)) + end + + it 'returns nil when cached key is not found' do + mapping = { + 3 => [ + { + text: 'foo', + type: 'new', + index: 2, + old_pos: 11, + new_pos: 12, + line_code: 'xpto', + rich_text: '<blips>blops</blips>' + } + ] + } + + described_class.write_multiple(mapping) + + found = described_class.read_multiple([2, 3]) + + expect(found.size).to eq(2) + + expect(found.first).to eq(nil) + expect(found.second.size).to eq(1) + expect(found.second).to all(be_a(Gitlab::Diff::Line)) + end + end +end diff --git a/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb b/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb index 1d75e8cb5da..48139c2f9dc 100644 --- a/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb +++ b/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb @@ -11,7 +11,7 @@ describe Gitlab::Email::Handler::CreateIssueHandler do stub_config_setting(host: 'localhost') end - let(:email_raw) { fixture_file('emails/valid_new_issue.eml') } + let(:email_raw) { email_fixture('emails/valid_new_issue.eml') } let(:namespace) { create(:namespace, path: 'gitlabhq') } let!(:project) { create(:project, :public, namespace: namespace, path: 'gitlabhq') } @@ -23,21 +23,58 @@ describe Gitlab::Email::Handler::CreateIssueHandler do ) end + context "when email key" do + let(:mail) { Mail::Message.new(email_raw) } + + it "matches the new format" do + handler = described_class.new(mail, "gitlabhq-gitlabhq-#{project.project_id}-#{user.incoming_email_token}-issue") + + expect(handler.instance_variable_get(:@project_id)).to eq project.project_id + expect(handler.instance_variable_get(:@project_slug)).to eq project.full_path_slug + expect(handler.instance_variable_get(:@incoming_email_token)).to eq user.incoming_email_token + expect(handler.can_handle?).to be_truthy + end + + it "matches the legacy format" do + handler = described_class.new(mail, "h5bp/html5-boilerplate+#{user.incoming_email_token}") + + expect(handler.instance_variable_get(:@project_path)).to eq 'h5bp/html5-boilerplate' + expect(handler.instance_variable_get(:@incoming_email_token)).to eq user.incoming_email_token + expect(handler.can_handle?).to be_truthy + end + + it "doesn't match either format" do + handler = described_class.new(mail, "h5bp-html5-boilerplate+something+invalid") + + expect(handler.can_handle?).to be_falsey + end + end + context "when everything is fine" do - it "creates a new issue" do - setup_attachment + shared_examples "a new issue" do + it "creates a new issue" do + setup_attachment - expect { receiver.execute }.to change { project.issues.count }.by(1) - issue = project.issues.last + expect { receiver.execute }.to change { project.issues.count }.by(1) + issue = project.issues.last + + expect(issue.author).to eq(user) + expect(issue.title).to eq('New Issue by email') + expect(issue.description).to include('reply by email') + expect(issue.description).to include(markdown) + end + end + + it_behaves_like "a new issue" - expect(issue.author).to eq(user) - expect(issue.title).to eq('New Issue by email') - expect(issue.description).to include('reply by email') - expect(issue.description).to include(markdown) + context "creates a new issue with legacy email address" do + let(:email_raw) { fixture_file('emails/valid_new_issue_legacy.eml') } + + it_behaves_like "a new issue" end context "when the reply is blank" do - let(:email_raw) { fixture_file("emails/valid_new_issue_empty.eml") } + let(:email_raw) { email_fixture("emails/valid_new_issue_empty.eml") } it "creates a new issue" do expect { receiver.execute }.to change { project.issues.count }.by(1) @@ -50,7 +87,7 @@ describe Gitlab::Email::Handler::CreateIssueHandler do end context "when there are quotes in email" do - let(:email_raw) { fixture_file("emails/valid_new_issue_with_quote.eml") } + let(:email_raw) { email_fixture("emails/valid_new_issue_with_quote.eml") } it "creates a new issue" do expect { receiver.execute }.to change { project.issues.count }.by(1) @@ -76,7 +113,7 @@ describe Gitlab::Email::Handler::CreateIssueHandler do end context "when we can't find the incoming_email_token" do - let(:email_raw) { fixture_file("emails/wrong_incoming_email_token.eml") } + let(:email_raw) { email_fixture("emails/wrong_issue_incoming_email_token.eml") } it "raises an UserNotFoundError" do expect { receiver.execute }.to raise_error(Gitlab::Email::UserNotFoundError) @@ -91,4 +128,8 @@ describe Gitlab::Email::Handler::CreateIssueHandler do end end end + + def email_fixture(path) + fixture_file(path).gsub('project_id', project.project_id.to_s) + end end 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 f276f1a8ddf..2fa86b2b46f 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 @@ -15,7 +15,7 @@ describe Gitlab::Email::Handler::CreateMergeRequestHandler do TestEnv.clean_test_path end - let(:email_raw) { fixture_file('emails/valid_new_merge_request.eml') } + let(:email_raw) { email_fixture('emails/valid_new_merge_request.eml') } let(:namespace) { create(:namespace, path: 'gitlabhq') } let!(:project) { create(:project, :public, :repository, namespace: namespace, path: 'gitlabhq') } @@ -27,6 +27,33 @@ describe Gitlab::Email::Handler::CreateMergeRequestHandler do ) end + context "when email key" do + let(:mail) { Mail::Message.new(email_raw) } + + it "matches the new format" do + handler = described_class.new(mail, "gitlabhq-gitlabhq-#{project.project_id}-#{user.incoming_email_token}-merge-request") + + expect(handler.instance_variable_get(:@project_id)).to eq project.project_id + expect(handler.instance_variable_get(:@project_slug)).to eq project.full_path_slug + expect(handler.instance_variable_get(:@incoming_email_token)).to eq user.incoming_email_token + expect(handler.can_handle?).to be_truthy + end + + it "matches the legacy format" do + handler = described_class.new(mail, "h5bp/html5-boilerplate+merge-request+#{user.incoming_email_token}") + + expect(handler.instance_variable_get(:@project_path)).to eq 'h5bp/html5-boilerplate' + expect(handler.instance_variable_get(:@incoming_email_token)).to eq user.incoming_email_token + expect(handler.can_handle?).to be_truthy + end + + it "doesn't match either format" do + handler = described_class.new(mail, "h5bp-html5-boilerplate+merge-request") + + expect(handler.can_handle?).to be_falsey + end + end + context "as a non-developer" do before do project.add_guest(user) @@ -43,15 +70,25 @@ describe Gitlab::Email::Handler::CreateMergeRequestHandler do end context "when everything is fine" do - it "creates a new merge request" do - expect { receiver.execute }.to change { project.merge_requests.count }.by(1) - merge_request = project.merge_requests.last - - expect(merge_request.author).to eq(user) - expect(merge_request.source_branch).to eq('feature') - expect(merge_request.title).to eq('Feature added') - expect(merge_request.description).to eq('Merge request description') - expect(merge_request.target_branch).to eq(project.default_branch) + shared_examples "a new merge request" do + it "creates a new merge request" do + expect { receiver.execute }.to change { project.merge_requests.count }.by(1) + merge_request = project.merge_requests.last + + expect(merge_request.author).to eq(user) + expect(merge_request.source_branch).to eq('feature') + expect(merge_request.title).to eq('Feature added') + expect(merge_request.description).to eq('Merge request description') + expect(merge_request.target_branch).to eq(project.default_branch) + end + end + + it_behaves_like "a new merge request" + + context "creates a new merge request with legacy email address" do + let(:email_raw) { fixture_file('emails/valid_new_merge_request_legacy.eml') } + + it_behaves_like "a new merge request" end end @@ -67,7 +104,7 @@ describe Gitlab::Email::Handler::CreateMergeRequestHandler do end context "when we can't find the incoming_email_token" do - let(:email_raw) { fixture_file("emails/wrong_incoming_email_token.eml") } + let(:email_raw) { email_fixture("emails/wrong_merge_request_incoming_email_token.eml") } it "raises an UserNotFoundError" do expect { receiver.execute }.to raise_error(Gitlab::Email::UserNotFoundError) @@ -75,7 +112,7 @@ describe Gitlab::Email::Handler::CreateMergeRequestHandler do end context "when the subject is blank" do - let(:email_raw) { fixture_file("emails/valid_new_merge_request_no_subject.eml") } + let(:email_raw) { email_fixture("emails/valid_new_merge_request_no_subject.eml") } it "raises an InvalidMergeRequestError" do expect { receiver.execute }.to raise_error(Gitlab::Email::InvalidMergeRequestError) @@ -83,7 +120,7 @@ describe Gitlab::Email::Handler::CreateMergeRequestHandler do end context "when the message body is blank" do - let(:email_raw) { fixture_file("emails/valid_new_merge_request_no_description.eml") } + let(:email_raw) { email_fixture("emails/valid_new_merge_request_no_description.eml") } it "creates a new merge request with description set from the last commit" do expect { receiver.execute }.to change { project.merge_requests.count }.by(1) @@ -95,7 +132,7 @@ describe Gitlab::Email::Handler::CreateMergeRequestHandler do end context 'when the email contains patch attachments' do - let(:email_raw) { fixture_file("emails/valid_merge_request_with_patch.eml") } + let(:email_raw) { email_fixture("emails/valid_merge_request_with_patch.eml") } it 'creates the source branch and applies the patches' do receiver.execute @@ -120,7 +157,7 @@ describe Gitlab::Email::Handler::CreateMergeRequestHandler do end context 'when the patch could not be applied' do - let(:email_raw) { fixture_file("emails/merge_request_with_conflicting_patch.eml") } + let(:email_raw) { email_fixture("emails/merge_request_with_conflicting_patch.eml") } it 'raises an error' do expect { receiver.execute }.to raise_error(Gitlab::Email::InvalidAttachment) @@ -128,7 +165,7 @@ describe Gitlab::Email::Handler::CreateMergeRequestHandler do end context 'when specifying the target branch using quick actions' do - let(:email_raw) { fixture_file('emails/merge_request_with_patch_and_target_branch.eml') } + let(:email_raw) { email_fixture('emails/merge_request_with_patch_and_target_branch.eml') } it 'creates the merge request with the correct target branch' do receiver.execute @@ -150,7 +187,7 @@ describe Gitlab::Email::Handler::CreateMergeRequestHandler do end describe '#patch_attachments' do - let(:email_raw) { fixture_file('emails/merge_request_multiple_patches.eml') } + let(:email_raw) { email_fixture('emails/merge_request_multiple_patches.eml') } let(:mail) { Mail::Message.new(email_raw) } subject(:handler) { described_class.new(mail, mail_key) } @@ -163,4 +200,8 @@ describe Gitlab::Email::Handler::CreateMergeRequestHandler do expect(attachments).to eq(expected_filenames) end end + + def email_fixture(path) + fixture_file(path).gsub('project_id', project.project_id.to_s) + end end diff --git a/spec/lib/gitlab/email/handler/unsubscribe_handler_spec.rb b/spec/lib/gitlab/email/handler/unsubscribe_handler_spec.rb index b8660b133ec..dcddd00df59 100644 --- a/spec/lib/gitlab/email/handler/unsubscribe_handler_spec.rb +++ b/spec/lib/gitlab/email/handler/unsubscribe_handler_spec.rb @@ -10,13 +10,35 @@ describe Gitlab::Email::Handler::UnsubscribeHandler do stub_config_setting(host: 'localhost') end - let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(mail_key, "#{mail_key}+unsubscribe") } - let(:project) { create(:project, :public) } - let(:user) { create(:user) } - let(:noteable) { create(:issue, project: project) } + let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(mail_key, "#{mail_key}#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX}") } + let(:project) { create(:project, :public) } + let(:user) { create(:user) } + let(:noteable) { create(:issue, project: project) } let!(:sent_notification) { SentNotification.record(noteable, user.id, mail_key) } + context "when email key" do + let(:mail) { Mail::Message.new(email_raw) } + + it "matches the new format" do + handler = described_class.new(mail, "#{mail_key}#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX}") + + expect(handler.can_handle?).to be_truthy + end + + it "matches the legacy format" do + handler = described_class.new(mail, "#{mail_key}#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX_LEGACY}") + + expect(handler.can_handle?).to be_truthy + end + + it "doesn't match either format" do + handler = described_class.new(mail, "+#{mail_key}#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX}") + + expect(handler.can_handle?).to be_falsey + end + end + context 'when notification concerns a commit' do let(:commit) { create(:commit, project: project) } let!(:sent_notification) { SentNotification.record(commit, user.id, mail_key) } @@ -40,6 +62,14 @@ describe Gitlab::Email::Handler::UnsubscribeHandler do it 'unsubscribes user from notable' do expect { receiver.execute }.to change { noteable.subscribed?(user) }.from(true).to(false) end + + context 'when using old style unsubscribe link' do + let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(mail_key, "#{mail_key}#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX_LEGACY}") } + + it 'unsubscribes user from notable' do + expect { receiver.execute }.to change { noteable.subscribed?(user) }.from(true).to(false) + end + end end context 'when the noteable could not be found' do diff --git a/spec/lib/gitlab/email/handler_spec.rb b/spec/lib/gitlab/email/handler_spec.rb index c651765dc0f..d2920b08956 100644 --- a/spec/lib/gitlab/email/handler_spec.rb +++ b/spec/lib/gitlab/email/handler_spec.rb @@ -19,7 +19,8 @@ describe Gitlab::Email::Handler do describe 'regexps are set properly' do let(:addresses) do - %W(sent_notification_key#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX} sent_notification_key path/to/project+merge-request+user_email_token path/to/project+user_email_token) + %W(sent_notification_key#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX} sent_notification_key path-to-project-123-user_email_token-merge-request path-to-project-123-user_email_token-issue) + + %W(sent_notification_key#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX_LEGACY} sent_notification_key path/to/project+merge-request+user_email_token path/to/project+user_email_token) end it 'picks each handler at least once' do diff --git a/spec/lib/gitlab/git/blob_spec.rb b/spec/lib/gitlab/git/blob_spec.rb index 80dd3dcc58e..1bcec04d28f 100644 --- a/spec/lib/gitlab/git/blob_spec.rb +++ b/spec/lib/gitlab/git/blob_spec.rb @@ -59,7 +59,7 @@ describe Gitlab::Git::Blob, :seed_helper do it { expect(blob.data[0..10]).to eq("*.rbc\n*.sas") } it { expect(blob.size).to eq(241) } it { expect(blob.mode).to eq("100644") } - it { expect(blob).not_to be_binary } + it { expect(blob).not_to be_binary_in_repo } end context 'file in root with leading slash' do @@ -92,7 +92,7 @@ describe Gitlab::Git::Blob, :seed_helper do end it 'does not mark the blob as binary' do - expect(blob).not_to be_binary + expect(blob).not_to be_binary_in_repo end end @@ -123,7 +123,7 @@ describe Gitlab::Git::Blob, :seed_helper do .with(hash_including(binary: true)) .and_call_original - expect(blob).to be_binary + expect(blob).to be_binary_in_repo end end end @@ -196,7 +196,7 @@ describe Gitlab::Git::Blob, :seed_helper do it { expect(blob.id).to eq('409f37c4f05865e4fb208c771485f211a22c4c2d') } it { expect(blob.data).to eq('') } it 'does not mark the blob as binary' do - expect(blob).not_to be_binary + expect(blob).not_to be_binary_in_repo end end diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index a417ef77c9e..3e34dd592f2 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -14,7 +14,7 @@ describe Gitlab::GitAccess do let(:authentication_abilities) { %i[read_project download_code push_code] } let(:redirected_path) { nil } let(:auth_result_type) { nil } - let(:changes) { '_any' } + let(:changes) { Gitlab::GitAccess::ANY } let(:push_access_check) { access.check('git-receive-pack', changes) } let(:pull_access_check) { access.check('git-upload-pack', changes) } @@ -437,7 +437,7 @@ describe Gitlab::GitAccess do let(:project) { nil } context 'when changes is _any' do - let(:changes) { '_any' } + let(:changes) { Gitlab::GitAccess::ANY } context 'when authentication abilities include push code' do let(:authentication_abilities) { [:push_code] } @@ -483,7 +483,7 @@ describe Gitlab::GitAccess do end context 'when project exists' do - let(:changes) { '_any' } + let(:changes) { Gitlab::GitAccess::ANY } let!(:project) { create(:project) } it 'does not create a new project' do @@ -497,7 +497,7 @@ describe Gitlab::GitAccess do let(:project_path) { "nonexistent" } let(:project) { nil } let(:namespace_path) { user.namespace.path } - let(:changes) { '_any' } + let(:changes) { Gitlab::GitAccess::ANY } it 'does not create a new project' do expect { access.send(:ensure_project_on_push!, cmd, changes) }.not_to change { Project.count } @@ -507,7 +507,7 @@ describe Gitlab::GitAccess do context 'when pull' do let(:cmd) { 'git-upload-pack' } - let(:changes) { '_any' } + let(:changes) { Gitlab::GitAccess::ANY } context 'when project does not exist' do let(:project_path) { "new-project" } @@ -709,10 +709,22 @@ describe Gitlab::GitAccess do project.add_developer(user) end - it 'checks LFS integrity only for first change' do - expect_any_instance_of(Gitlab::Checks::LfsIntegrity).to receive(:objects_missing?).exactly(1).times + context 'when LFS is not enabled' do + it 'does not run LFSIntegrity check' do + expect(Gitlab::Checks::LfsIntegrity).not_to receive(:new) - push_access_check + push_access_check + end + end + + context 'when LFS is enabled' do + it 'checks LFS integrity only for first change' do + allow(project).to receive(:lfs_enabled?).and_return(true) + + expect_any_instance_of(Gitlab::Checks::LfsIntegrity).to receive(:objects_missing?).exactly(1).times + + push_access_check + end end end @@ -724,7 +736,8 @@ describe Gitlab::GitAccess do end let(:changes) do - { push_new_branch: "#{Gitlab::Git::BLANK_SHA} 570e7b2ab refs/heads/wow", + { any: Gitlab::GitAccess::ANY, + push_new_branch: "#{Gitlab::Git::BLANK_SHA} 570e7b2ab refs/heads/wow", push_master: '6f6d7e7ed 570e7b2ab refs/heads/master', push_protected_branch: '6f6d7e7ed 570e7b2ab refs/heads/feature', push_remove_protected_branch: "570e7b2ab #{Gitlab::Git::BLANK_SHA} "\ @@ -786,6 +799,7 @@ describe Gitlab::GitAccess do permissions_matrix = { admin: { + any: true, push_new_branch: true, push_master: true, push_protected_branch: true, @@ -797,6 +811,7 @@ describe Gitlab::GitAccess do }, maintainer: { + any: true, push_new_branch: true, push_master: true, push_protected_branch: true, @@ -808,6 +823,7 @@ describe Gitlab::GitAccess do }, developer: { + any: true, push_new_branch: true, push_master: true, push_protected_branch: false, @@ -819,6 +835,7 @@ describe Gitlab::GitAccess do }, reporter: { + any: false, push_new_branch: false, push_master: false, push_protected_branch: false, @@ -830,6 +847,7 @@ describe Gitlab::GitAccess do }, guest: { + any: false, push_new_branch: false, push_master: false, push_protected_branch: false, diff --git a/spec/lib/gitlab/git_access_wiki_spec.rb b/spec/lib/gitlab/git_access_wiki_spec.rb index 9c6c9fe13bf..6ba65b56618 100644 --- a/spec/lib/gitlab/git_access_wiki_spec.rb +++ b/spec/lib/gitlab/git_access_wiki_spec.rb @@ -38,7 +38,7 @@ describe Gitlab::GitAccessWiki do end describe '#access_check_download!' do - subject { access.check('git-upload-pack', '_any') } + subject { access.check('git-upload-pack', Gitlab::GitAccess::ANY) } before do project.add_developer(user) diff --git a/spec/lib/gitlab/gitaly_client/blobs_stitcher_spec.rb b/spec/lib/gitlab/gitaly_client/blobs_stitcher_spec.rb index 9db710e759e..742b2872c40 100644 --- a/spec/lib/gitlab/gitaly_client/blobs_stitcher_spec.rb +++ b/spec/lib/gitlab/gitaly_client/blobs_stitcher_spec.rb @@ -21,7 +21,7 @@ describe Gitlab::GitalyClient::BlobsStitcher do expect(blobs[0].size).to eq(1642) expect(blobs[0].commit_id).to eq('f00ba7') expect(blobs[0].data).to eq("first-line\nsecond-line") - expect(blobs[0].binary?).to be false + expect(blobs[0].binary_in_repo?).to be false expect(blobs[1].id).to eq('abcdef2') expect(blobs[1].mode).to eq('100644') @@ -30,7 +30,7 @@ describe Gitlab::GitalyClient::BlobsStitcher do expect(blobs[1].size).to eq(2461) expect(blobs[1].commit_id).to eq('f00ba8') expect(blobs[1].data).to eq("GIF87a\x90\x01".b) - expect(blobs[1].binary?).to be true + expect(blobs[1].binary_in_repo?).to be true end end end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index c8c74883640..d3cae137c3c 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -66,6 +66,9 @@ snippets: releases: - author - project +- links +links: +- release project_members: - created_by - user diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 24b1f2d995b..2422868474e 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -120,6 +120,13 @@ Release: - project_id - created_at - updated_at +Releases::Link: +- id +- release_id +- url +- name +- created_at +- updated_at ProjectMember: - id - access_level diff --git a/spec/lib/gitlab/incoming_email_spec.rb b/spec/lib/gitlab/incoming_email_spec.rb index 4c0c3fcbcc7..2db62ab983a 100644 --- a/spec/lib/gitlab/incoming_email_spec.rb +++ b/spec/lib/gitlab/incoming_email_spec.rb @@ -61,7 +61,7 @@ describe Gitlab::IncomingEmail do end it 'returns the address with interpolated reply key and unsubscribe suffix' do - expect(described_class.unsubscribe_address('key')).to eq('replies+key+unsubscribe@example.com') + expect(described_class.unsubscribe_address('key')).to eq("replies+key#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX}@example.com") end end diff --git a/spec/lib/gitlab/legacy_github_import/importer_spec.rb b/spec/lib/gitlab/legacy_github_import/importer_spec.rb index d2df21d7bb5..6bc3792eb22 100644 --- a/spec/lib/gitlab/legacy_github_import/importer_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/importer_spec.rb @@ -138,7 +138,7 @@ describe Gitlab::LegacyGithubImport::Importer do let(:release2) do double( - tag_name: 'v2.0.0', + tag_name: 'v1.1.0', name: 'Second release', body: nil, draft: false, diff --git a/spec/lib/gitlab/middleware/go_spec.rb b/spec/lib/gitlab/middleware/go_spec.rb index 7a3a9ab875b..f52095bf633 100644 --- a/spec/lib/gitlab/middleware/go_spec.rb +++ b/spec/lib/gitlab/middleware/go_spec.rb @@ -96,43 +96,36 @@ describe Gitlab::Middleware::Go do it_behaves_like 'unauthorized' end - end - - context 'using warden' do - before do - env['warden'] = double(authenticate: current_user) - end - context 'when active' do - it_behaves_like 'authenticated' - end - - context 'when blocked' do + context 'with user is blocked' do before do - current_user.block! + current_user.block end it_behaves_like 'unauthorized' end end - context 'using a personal access token' do - let(:personal_access_token) { create(:personal_access_token, user: current_user) } - - before do - env['HTTP_PRIVATE_TOKEN'] = personal_access_token.token - end - - context 'with api scope' do - it_behaves_like 'authenticated' - end + context 'using basic auth' do + context 'using a personal access token' do + let(:personal_access_token) { create(:personal_access_token, user: current_user) } - context 'with read_user scope' do before do - personal_access_token.update_attribute(:scopes, [:read_user]) + env['REMOTE_ADDR'] = "192.168.0.1" + env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials(current_user.username, personal_access_token.token) end - it_behaves_like 'unauthorized' + context 'with api scope' do + it_behaves_like 'authenticated' + end + + context 'with read_user scope' do + before do + personal_access_token.update_attribute(:scopes, [:read_user]) + end + + it_behaves_like 'unauthorized' + end end end end diff --git a/spec/lib/gitlab/prometheus/metric_group_spec.rb b/spec/lib/gitlab/prometheus/metric_group_spec.rb index e7d16e73663..5cc6827488b 100644 --- a/spec/lib/gitlab/prometheus/metric_group_spec.rb +++ b/spec/lib/gitlab/prometheus/metric_group_spec.rb @@ -21,6 +21,13 @@ describe Gitlab::Prometheus::MetricGroup do common_metric_group_a.id, common_metric_group_b_q1.id, common_metric_group_b_q2.id) end + + it 'orders by priority' do + priorities = subject.map(&:priority) + names = subject.map(&:name) + expect(priorities).to eq([10, 5]) + expect(names).to eq(['Response metrics (AWS ELB)', 'System metrics (Kubernetes)']) + end end describe '.for_project' do diff --git a/spec/lib/json_web_token/rsa_token_spec.rb b/spec/lib/json_web_token/rsa_token_spec.rb index d6edc964844..a3c54651e80 100644 --- a/spec/lib/json_web_token/rsa_token_spec.rb +++ b/spec/lib/json_web_token/rsa_token_spec.rb @@ -25,7 +25,7 @@ describe JSONWebToken::RSAToken do rsa_token['key'] = 'value' end - subject { JWT.decode(rsa_encoded, rsa_key) } + subject { JWT.decode(rsa_encoded, rsa_key, true, { algorithm: 'RS256' }) } it { expect {subject}.not_to raise_error } it { expect(subject.first).to include('key' => 'value') } @@ -39,7 +39,7 @@ describe JSONWebToken::RSAToken do context 'for invalid key to raise an exception' do let(:new_key) { OpenSSL::PKey::RSA.generate(512) } - subject { JWT.decode(rsa_encoded, new_key) } + subject { JWT.decode(rsa_encoded, new_key, true, { algorithm: 'RS256' }) } it { expect {subject}.to raise_error(JWT::DecodeError) } end |