diff options
Diffstat (limited to 'spec/lib')
53 files changed, 1953 insertions, 328 deletions
diff --git a/spec/lib/backup/uploads_spec.rb b/spec/lib/backup/uploads_spec.rb new file mode 100644 index 00000000000..544d3754c0f --- /dev/null +++ b/spec/lib/backup/uploads_spec.rb @@ -0,0 +1,18 @@ +require 'spec_helper' + +describe Backup::Uploads do + let(:progress) { StringIO.new } + subject(:backup) { described_class.new(progress) } + + describe '#initialize' do + it 'uses the correct upload dir' do + Dir.mktmpdir do |tmpdir| + FileUtils.mkdir_p("#{tmpdir}/uploads") + + allow(Gitlab.config.uploads).to receive(:storage_path) { tmpdir } + + expect(backup.app_files_dir).to eq("#{tmpdir}/uploads") + end + end + end +end diff --git a/spec/lib/banzai/filter/merge_request_reference_filter_spec.rb b/spec/lib/banzai/filter/merge_request_reference_filter_spec.rb index 55c41e55437..72dfd6ff9ea 100644 --- a/spec/lib/banzai/filter/merge_request_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/merge_request_reference_filter_spec.rb @@ -30,6 +30,23 @@ describe Banzai::Filter::MergeRequestReferenceFilter do end end + describe 'all references' do + let(:doc) { reference_filter(merge.to_reference) } + let(:tag_el) { doc.css('a').first } + + it 'adds merge request iid' do + expect(tag_el["data-iid"]).to eq(merge.iid.to_s) + end + + it 'adds project data attribute with project id' do + expect(tag_el["data-project-path"]).to eq(project.full_path) + end + + it 'does not add `has-tooltip` class' do + expect(tag_el["class"]).not_to include('has-tooltip') + end + end + context 'internal reference' do let(:reference) { merge.to_reference } @@ -57,9 +74,9 @@ describe Banzai::Filter::MergeRequestReferenceFilter do expect(reference_filter(act).to_html).to eq exp end - it 'includes a title attribute' do + it 'has no title' do doc = reference_filter("Merge #{reference}") - expect(doc.css('a').first.attr('title')).to eq merge.title + expect(doc.css('a').first.attr('title')).to eq "" end it 'escapes the title attribute' do @@ -69,9 +86,9 @@ describe Banzai::Filter::MergeRequestReferenceFilter do expect(doc.text).to eq "Merge #{reference}" end - it 'includes default classes' do + it 'includes default classes, without tooltip' do doc = reference_filter("Merge #{reference}") - expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-merge_request has-tooltip' + expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-merge_request' end it 'includes a data-project attribute' do diff --git a/spec/lib/banzai/filter/output_safety_spec.rb b/spec/lib/banzai/filter/output_safety_spec.rb new file mode 100644 index 00000000000..5ffe591c9a4 --- /dev/null +++ b/spec/lib/banzai/filter/output_safety_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Banzai::Filter::OutputSafety do + subject do + Class.new do + include Banzai::Filter::OutputSafety + end.new + end + + let(:content) { '<pre><code>foo</code></pre>' } + + context 'when given HTML is safe' do + let(:html) { content.html_safe } + + it 'returns safe HTML' do + expect(subject.escape_once(html)).to eq(html) + end + end + + context 'when given HTML is not safe' do + let(:html) { content } + + it 'returns escaped HTML' do + expect(subject.escape_once(html)).to eq(ERB::Util.html_escape_once(html)) + end + end +end diff --git a/spec/lib/banzai/filter/suggestion_filter_spec.rb b/spec/lib/banzai/filter/suggestion_filter_spec.rb index b13c90b54bd..af6f002fa30 100644 --- a/spec/lib/banzai/filter/suggestion_filter_spec.rb +++ b/spec/lib/banzai/filter/suggestion_filter_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' describe Banzai::Filter::SuggestionFilter do include FilterSpecHelper - let(:input) { "<pre class='code highlight js-syntax-highlight suggestion'><code>foo\n</code></pre>" } + let(:input) { %(<pre class="code highlight js-syntax-highlight suggestion"><code>foo\n</code></pre>) } let(:default_context) do { suggestions_filter_enabled: true } end @@ -23,4 +23,35 @@ describe Banzai::Filter::SuggestionFilter do expect(result[:class]).to be_nil end + + context 'multi-line suggestions' do + let(:data_attr) { Banzai::Filter::SyntaxHighlightFilter::LANG_PARAMS_ATTR } + let(:input) { %(<pre class="code highlight js-syntax-highlight suggestion" #{data_attr}="-3+2"><code>foo\n</code></pre>) } + + context 'feature disabled' do + before do + stub_feature_flags(multi_line_suggestions: false) + end + + it 'removes data-lang-params if it matches a multi-line suggestion param' do + doc = filter(input, default_context) + pre = doc.css('pre').first + + expect(pre[data_attr]).to be_nil + end + end + + context 'feature enabled' do + before do + stub_feature_flags(multi_line_suggestions: true) + end + + it 'keeps data-lang-params' do + doc = filter(input, default_context) + pre = doc.css('pre').first + + expect(pre[data_attr]).to eq('-3+2') + end + end + end end diff --git a/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb b/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb index ef52c572898..05057789cc1 100644 --- a/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb +++ b/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb @@ -45,7 +45,10 @@ describe Banzai::Filter::SyntaxHighlightFilter do end context "languages that should be passed through" do - %w(math mermaid plantuml).each do |lang| + let(:delimiter) { described_class::PARAMS_DELIMITER } + let(:data_attr) { described_class::LANG_PARAMS_ATTR } + + %w(math mermaid plantuml suggestion).each do |lang| context "when #{lang} is specified" do it "highlights as plaintext but with the correct language attribute and class" do result = filter(%{<pre><code lang="#{lang}">This is a test</code></pre>}) @@ -55,6 +58,33 @@ describe Banzai::Filter::SyntaxHighlightFilter do include_examples "XSS prevention", lang end + + context "when #{lang} has extra params" do + let(:lang_params) { 'foo-bar-kux' } + + it "includes data-lang-params tag with extra information" do + result = filter(%{<pre><code lang="#{lang}#{delimiter}#{lang_params}">This is a test</code></pre>}) + + expect(result.to_html).to eq(%{<pre class="code highlight js-syntax-highlight #{lang}" lang="#{lang}" #{data_attr}="#{lang_params}" v-pre="true"><code><span id="LC1" class="line" lang="#{lang}">This is a test</span></code></pre>}) + end + + include_examples "XSS prevention", lang + include_examples "XSS prevention", + "#{lang}#{described_class::PARAMS_DELIMITER}<script>alert(1)</script>" + include_examples "XSS prevention", + "#{lang}#{described_class::PARAMS_DELIMITER}<script>alert(1)</script>" + end + end + + context 'when multiple param delimiters are used' do + let(:lang) { 'suggestion' } + let(:lang_params) { '-1+10' } + + it "delimits on the first appearence" do + result = filter(%{<pre><code lang="#{lang}#{delimiter}#{lang_params}#{delimiter}more-things">This is a test</code></pre>}) + + expect(result.to_html).to eq(%{<pre class="code highlight js-syntax-highlight #{lang}" lang="#{lang}" #{data_attr}="#{lang_params}#{delimiter}more-things" v-pre="true"><code><span id="LC1" class="line" lang="#{lang}">This is a test</span></code></pre>}) + end end end diff --git a/spec/lib/gitlab/auth/o_auth/user_spec.rb b/spec/lib/gitlab/auth/o_auth/user_spec.rb index dcbd12fe190..b765c265e69 100644 --- a/spec/lib/gitlab/auth/o_auth/user_spec.rb +++ b/spec/lib/gitlab/auth/o_auth/user_spec.rb @@ -207,6 +207,7 @@ describe Gitlab::Auth::OAuth::User do before do allow(ldap_user).to receive(:uid) { uid } allow(ldap_user).to receive(:username) { uid } + allow(ldap_user).to receive(:name) { 'John Doe' } allow(ldap_user).to receive(:email) { ['johndoe@example.com', 'john2@example.com'] } allow(ldap_user).to receive(:dn) { dn } end @@ -221,6 +222,7 @@ describe Gitlab::Auth::OAuth::User do it "creates a user with dual LDAP and omniauth identities" do expect(gl_user).to be_valid expect(gl_user.username).to eql uid + expect(gl_user.name).to eql 'John Doe' expect(gl_user.email).to eql 'johndoe@example.com' expect(gl_user.identities.length).to be 2 identities_as_hash = gl_user.identities.map { |id| { provider: id.provider, extern_uid: id.extern_uid } } @@ -232,11 +234,13 @@ describe Gitlab::Auth::OAuth::User do ) end - it "has email set as synced" do + it "has name and email set as synced" do + expect(gl_user.user_synced_attributes_metadata.name_synced).to be_truthy expect(gl_user.user_synced_attributes_metadata.email_synced).to be_truthy end - it "has email set as read-only" do + it "has name and email set as read-only" do + expect(gl_user.read_only_attribute?(:name)).to be_truthy expect(gl_user.read_only_attribute?(:email)).to be_truthy end @@ -246,7 +250,7 @@ describe Gitlab::Auth::OAuth::User do end context "and LDAP user has an account already" do - let!(:existing_user) { create(:omniauth_user, email: 'john@example.com', extern_uid: dn, provider: 'ldapmain', username: 'john') } + let!(:existing_user) { create(:omniauth_user, name: 'John Doe', email: 'john@example.com', extern_uid: dn, provider: 'ldapmain', username: 'john') } it "adds the omniauth identity to the LDAP account" do allow(Gitlab::Auth::LDAP::Person).to receive(:find_by_uid).and_return(ldap_user) @@ -254,6 +258,7 @@ describe Gitlab::Auth::OAuth::User do expect(gl_user).to be_valid expect(gl_user.username).to eql 'john' + expect(gl_user.name).to eql 'John Doe' expect(gl_user.email).to eql 'john@example.com' expect(gl_user.identities.length).to be 2 identities_as_hash = gl_user.identities.map { |id| { provider: id.provider, extern_uid: id.extern_uid } } diff --git a/spec/lib/gitlab/authorized_keys_spec.rb b/spec/lib/gitlab/authorized_keys_spec.rb new file mode 100644 index 00000000000..42bc509eeef --- /dev/null +++ b/spec/lib/gitlab/authorized_keys_spec.rb @@ -0,0 +1,194 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::AuthorizedKeys do + let(:logger) { double('logger').as_null_object } + + subject { described_class.new(logger) } + + describe '#add_key' do + context 'authorized_keys file exists' do + before do + create_authorized_keys_fixture + end + + after do + delete_authorized_keys_file + end + + it "adds a line at the end of the file and strips trailing garbage" do + auth_line = "command=\"#{Gitlab.config.gitlab_shell.path}/bin/gitlab-shell key-741\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty ssh-rsa AAAAB3NzaDAxx2E" + + expect(logger).to receive(:info).with('Adding key (key-741): ssh-rsa AAAAB3NzaDAxx2E') + expect(subject.add_key('key-741', 'ssh-rsa AAAAB3NzaDAxx2E trailing garbage')) + .to be_truthy + expect(File.read(tmp_authorized_keys_path)).to eq("existing content\n#{auth_line}\n") + end + end + + context 'authorized_keys file does not exist' do + before do + delete_authorized_keys_file + end + + it 'creates the file' do + expect(subject.add_key('key-741', 'ssh-rsa AAAAB3NzaDAxx2E')).to be_truthy + expect(File.exist?(tmp_authorized_keys_path)).to be_truthy + end + end + end + + describe '#batch_add_keys' do + let(:keys) do + [ + double(shell_id: 'key-12', key: 'ssh-dsa ASDFASGADG trailing garbage'), + double(shell_id: 'key-123', key: 'ssh-rsa GFDGDFSGSDFG') + ] + end + + context 'authorized_keys file exists' do + before do + create_authorized_keys_fixture + end + + after do + delete_authorized_keys_file + end + + it "adds lines at the end of the file" do + auth_line1 = "command=\"#{Gitlab.config.gitlab_shell.path}/bin/gitlab-shell key-12\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty ssh-dsa ASDFASGADG" + auth_line2 = "command=\"#{Gitlab.config.gitlab_shell.path}/bin/gitlab-shell key-123\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty ssh-rsa GFDGDFSGSDFG" + + expect(logger).to receive(:info).with('Adding key (key-12): ssh-dsa ASDFASGADG') + expect(logger).to receive(:info).with('Adding key (key-123): ssh-rsa GFDGDFSGSDFG') + expect(subject.batch_add_keys(keys)).to be_truthy + expect(File.read(tmp_authorized_keys_path)).to eq("existing content\n#{auth_line1}\n#{auth_line2}\n") + end + + context "invalid key" do + let(:keys) { [double(shell_id: 'key-123', key: "ssh-rsa A\tSDFA\nSGADG")] } + + it "doesn't add keys" do + expect(subject.batch_add_keys(keys)).to be_falsey + expect(File.read(tmp_authorized_keys_path)).to eq("existing content\n") + end + end + end + + context 'authorized_keys file does not exist' do + before do + delete_authorized_keys_file + end + + it 'creates the file' do + expect(subject.batch_add_keys(keys)).to be_truthy + expect(File.exist?(tmp_authorized_keys_path)).to be_truthy + end + end + end + + describe '#rm_key' do + context 'authorized_keys file exists' do + before do + create_authorized_keys_fixture + end + + after do + delete_authorized_keys_file + end + + it "removes the right line" do + other_line = "command=\"#{Gitlab.config.gitlab_shell.path}/bin/gitlab-shell key-742\",options ssh-rsa AAAAB3NzaDAxx2E" + delete_line = "command=\"#{Gitlab.config.gitlab_shell.path}/bin/gitlab-shell key-741\",options ssh-rsa AAAAB3NzaDAxx2E" + erased_line = delete_line.gsub(/./, '#') + File.open(tmp_authorized_keys_path, 'a') do |auth_file| + auth_file.puts delete_line + auth_file.puts other_line + end + + expect(logger).to receive(:info).with('Removing key (key-741)') + expect(subject.rm_key('key-741')).to be_truthy + expect(File.read(tmp_authorized_keys_path)).to eq("existing content\n#{erased_line}\n#{other_line}\n") + end + end + + context 'authorized_keys file does not exist' do + before do + delete_authorized_keys_file + end + + it 'returns false' do + expect(subject.rm_key('key-741')).to be_falsey + end + end + end + + describe '#clear' do + context 'authorized_keys file exists' do + before do + create_authorized_keys_fixture + end + + after do + delete_authorized_keys_file + end + + it "returns true" do + expect(subject.clear).to be_truthy + end + end + + context 'authorized_keys file does not exist' do + before do + delete_authorized_keys_file + end + + it "still returns true" do + expect(subject.clear).to be_truthy + end + end + end + + describe '#list_key_ids' do + context 'authorized_keys file exists' do + before do + create_authorized_keys_fixture( + existing_content: + "key-1\tssh-dsa AAA\nkey-2\tssh-rsa BBB\nkey-3\tssh-rsa CCC\nkey-9000\tssh-rsa DDD\n" + ) + end + + after do + delete_authorized_keys_file + end + + it 'returns array of key IDs' do + expect(subject.list_key_ids).to eq([1, 2, 3, 9000]) + end + end + + context 'authorized_keys file does not exist' do + before do + delete_authorized_keys_file + end + + it 'returns an empty array' do + expect(subject.list_key_ids).to be_empty + end + end + end + + def create_authorized_keys_fixture(existing_content: 'existing content') + FileUtils.mkdir_p(File.dirname(tmp_authorized_keys_path)) + File.open(tmp_authorized_keys_path, 'w') { |file| file.puts(existing_content) } + end + + def delete_authorized_keys_file + File.delete(tmp_authorized_keys_path) if File.exist?(tmp_authorized_keys_path) + end + + def tmp_authorized_keys_path + Gitlab.config.gitlab_shell.authorized_keys_file + end +end diff --git a/spec/lib/gitlab/background_migration/populate_merge_request_assignees_table_spec.rb b/spec/lib/gitlab/background_migration/populate_merge_request_assignees_table_spec.rb new file mode 100644 index 00000000000..4a81a37d341 --- /dev/null +++ b/spec/lib/gitlab/background_migration/populate_merge_request_assignees_table_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Gitlab::BackgroundMigration::PopulateMergeRequestAssigneesTable, :migration, schema: 20190315191339 do + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:users) { table(:users) } + + let(:user) { users.create!(email: 'test@example.com', projects_limit: 100, username: 'test') } + let(:user_2) { users.create!(email: 'test2@example.com', projects_limit: 100, username: 'test') } + let(:user_3) { users.create!(email: 'test3@example.com', projects_limit: 100, username: 'test') } + + let(:namespace) { namespaces.create(name: 'gitlab', path: 'gitlab-org') } + let(:project) { projects.create(namespace_id: namespace.id, name: 'foo') } + let(:merge_requests) { table(:merge_requests) } + let(:merge_request_assignees) { table(:merge_request_assignees) } + + def create_merge_request(id, params = {}) + params.merge!(id: id, + target_project_id: project.id, + target_branch: 'master', + source_project_id: project.id, + source_branch: 'mr name', + title: "mr name#{id}") + + merge_requests.create(params) + end + + describe '#perform' do + it 'creates merge_request_assignees rows according to merge_requests' do + create_merge_request(2, assignee_id: user.id) + create_merge_request(3, assignee_id: user_2.id) + create_merge_request(4, assignee_id: user_3.id) + # Test filtering already migrated row + merge_request_assignees.create!(merge_request_id: 2, user_id: user_3.id) + + subject.perform(1, 4) + + rows = merge_request_assignees.order(:id).map { |row| row.attributes.slice('merge_request_id', 'user_id') } + existing_rows = [ + { 'merge_request_id' => 2, 'user_id' => user_3.id } + ] + created_rows = [ + { 'merge_request_id' => 3, 'user_id' => user_2.id }, + { 'merge_request_id' => 4, 'user_id' => user_3.id } + ] + expected_rows = existing_rows + created_rows + + expect(rows.size).to eq(expected_rows.size) + expected_rows.each do |expected_row| + expect(rows).to include(expected_row) + end + end + end +end diff --git a/spec/lib/gitlab/badge/pipeline/template_spec.rb b/spec/lib/gitlab/badge/pipeline/template_spec.rb index 20fa4f879c3..bcef0b7e120 100644 --- a/spec/lib/gitlab/badge/pipeline/template_spec.rb +++ b/spec/lib/gitlab/badge/pipeline/template_spec.rb @@ -59,6 +59,16 @@ describe Gitlab::Badge::Pipeline::Template do end end + context 'when status is preparing' do + before do + allow(badge).to receive(:status).and_return('preparing') + end + + it 'has expected color' do + expect(template.value_color).to eq '#dfb317' + end + end + context 'when status is unknown' do before do allow(badge).to receive(:status).and_return('unknown') diff --git a/spec/lib/gitlab/checks/branch_check_spec.rb b/spec/lib/gitlab/checks/branch_check_spec.rb index 12beeecd470..8d5ab27a17c 100644 --- a/spec/lib/gitlab/checks/branch_check_spec.rb +++ b/spec/lib/gitlab/checks/branch_check_spec.rb @@ -108,64 +108,86 @@ describe Gitlab::Checks::BranchCheck do end context 'protected branch creation feature is enabled' do - context 'user is not allowed to create protected branches' do + context 'user can push to branch' do before do allow(user_access) - .to receive(:can_merge_to_branch?) + .to receive(:can_push_to_branch?) .with('feature') - .and_return(false) + .and_return(true) end - it 'raises an error' do - expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to create protected branches on this project.') + it 'does not raise an error' do + expect { subject.validate! }.not_to raise_error end end - context 'user is allowed to create protected branches' do + context 'user cannot push to branch' do before do allow(user_access) - .to receive(:can_merge_to_branch?) + .to receive(:can_push_to_branch?) .with('feature') - .and_return(true) - - allow(project.repository) - .to receive(:branch_names_contains_sha) - .with(newrev) - .and_return(['branch']) + .and_return(false) end - context "newrev isn't in any protected branches" do + context 'user cannot merge to branch' do before do - allow(ProtectedBranch) - .to receive(:any_protected?) - .with(project, ['branch']) + allow(user_access) + .to receive(:can_merge_to_branch?) + .with('feature') .and_return(false) end it 'raises an error' do - expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You can only use an existing protected branch ref as the basis of a new protected branch.') + expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to create protected branches on this project.') end end - context 'newrev is included in a protected branch' do + context 'user can merge to branch' do before do - allow(ProtectedBranch) - .to receive(:any_protected?) - .with(project, ['branch']) + allow(user_access) + .to receive(:can_merge_to_branch?) + .with('feature') .and_return(true) + + allow(project.repository) + .to receive(:branch_names_contains_sha) + .with(newrev) + .and_return(['branch']) end - context 'via web interface' do - let(:protocol) { 'web' } + context "newrev isn't in any protected branches" do + before do + allow(ProtectedBranch) + .to receive(:any_protected?) + .with(project, ['branch']) + .and_return(false) + end - it 'allows branch creation' do - expect { subject.validate! }.not_to raise_error + it 'raises an error' do + expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You can only use an existing protected branch ref as the basis of a new protected branch.') end end - context 'via SSH' do - it 'raises an error' do - expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You can only create protected branches using the web interface and API.') + context 'newrev is included in a protected branch' do + before do + allow(ProtectedBranch) + .to receive(:any_protected?) + .with(project, ['branch']) + .and_return(true) + end + + context 'via web interface' do + let(:protocol) { 'web' } + + it 'allows branch creation' do + expect { subject.validate! }.not_to raise_error + end + end + + context 'via SSH' do + it 'raises an error' do + expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You can only create protected branches using the web interface and API.') + end end end end diff --git a/spec/lib/gitlab/ci/build/prerequisite/factory_spec.rb b/spec/lib/gitlab/ci/build/prerequisite/factory_spec.rb new file mode 100644 index 00000000000..5187f99a441 --- /dev/null +++ b/spec/lib/gitlab/ci/build/prerequisite/factory_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Ci::Build::Prerequisite::Factory do + let(:build) { create(:ci_build) } + + describe '.for_build' do + let(:kubernetes_namespace) do + instance_double( + Gitlab::Ci::Build::Prerequisite::KubernetesNamespace, + unmet?: unmet) + end + + subject { described_class.new(build).unmet } + + before do + expect(Gitlab::Ci::Build::Prerequisite::KubernetesNamespace) + .to receive(:new).with(build).and_return(kubernetes_namespace) + end + + context 'prerequisite is unmet' do + let(:unmet) { true } + + it { is_expected.to eq [kubernetes_namespace] } + end + + context 'prerequisite is met' do + let(:unmet) { false } + + it { is_expected.to be_empty } + end + end +end diff --git a/spec/lib/gitlab/ci/build/prerequisite/kubernetes_namespace_spec.rb b/spec/lib/gitlab/ci/build/prerequisite/kubernetes_namespace_spec.rb new file mode 100644 index 00000000000..62dcd80fad7 --- /dev/null +++ b/spec/lib/gitlab/ci/build/prerequisite/kubernetes_namespace_spec.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Ci::Build::Prerequisite::KubernetesNamespace do + let(:build) { create(:ci_build) } + + describe '#unmet?' do + subject { described_class.new(build).unmet? } + + context 'build has no deployment' do + before do + expect(build.deployment).to be_nil + end + + it { is_expected.to be_falsey } + end + + context 'build has a deployment' do + let!(:deployment) { create(:deployment, deployable: build) } + + context 'and a cluster to deploy to' do + let(:cluster) { create(:cluster, projects: [build.project]) } + + before do + allow(build.deployment).to receive(:cluster).and_return(cluster) + end + + it { is_expected.to be_truthy } + + context 'and a namespace is already created for this project' do + let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, cluster: cluster, project: build.project) } + + it { is_expected.to be_falsey } + end + end + + context 'and no cluster to deploy to' do + before do + expect(deployment.cluster).to be_nil + end + + it { is_expected.to be_falsey } + end + end + end + + describe '#complete!' do + let!(:deployment) { create(:deployment, deployable: build) } + let(:service) { double(execute: true) } + + subject { described_class.new(build).complete! } + + context 'completion is required' do + let(:cluster) { create(:cluster, projects: [build.project]) } + + before do + allow(build.deployment).to receive(:cluster).and_return(cluster) + end + + it 'creates a kubernetes namespace' do + expect(Clusters::Gcp::Kubernetes::CreateOrUpdateNamespaceService) + .to receive(:new) + .with(cluster: cluster, kubernetes_namespace: instance_of(Clusters::KubernetesNamespace)) + .and_return(service) + + expect(service).to receive(:execute).once + + subject + end + end + + context 'completion is not required' do + before do + expect(deployment.cluster).to be_nil + end + + it 'does not create a namespace' do + expect(Clusters::Gcp::Kubernetes::CreateOrUpdateNamespaceService).not_to receive(:new) + + subject + end + 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 index dab0fb51bcc..5181e9c1583 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb @@ -48,6 +48,24 @@ describe Gitlab::Ci::Pipeline::Chain::Command do end end + describe '#merge_request_ref_exists?' do + subject { command.merge_request_ref_exists? } + + let!(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + + context 'for existing merge request ref' do + let(:origin_ref) { merge_request.ref_path } + + it { is_expected.to eq(true) } + end + + context 'for branch ref' do + let(:origin_ref) { merge_request.source_branch } + + it { is_expected.to eq(false) } + end + end + describe '#ref' do subject { command.ref } 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 8ba56d73838..7d750877d09 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb @@ -10,12 +10,33 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Abilities do let(:command) do Gitlab::Ci::Pipeline::Chain::Command.new( - project: project, current_user: user, origin_ref: ref) + project: project, current_user: user, origin_ref: origin_ref, merge_request: merge_request) end let(:step) { described_class.new(pipeline, command) } let(:ref) { 'master' } + let(:origin_ref) { ref } + let(:merge_request) { nil } + + shared_context 'detached merge request pipeline' do + let(:merge_request) do + create(:merge_request, + source_project: project, + source_branch: ref, + target_project: project, + target_branch: 'feature') + end + + let(:pipeline) do + build(:ci_pipeline, + source: :merge_request_event, + merge_request: merge_request, + project: project) + end + + let(:origin_ref) { merge_request.ref_path } + end context 'when users has no ability to run a pipeline' do before do @@ -58,6 +79,12 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Abilities do it { is_expected.to be_truthy } + context 'when pipeline is a detached merge request pipeline' do + include_context 'detached merge request pipeline' + + it { is_expected.to be_truthy } + end + context 'when the branch is protected' do let!(:protected_branch) do create(:protected_branch, project: project, name: ref) @@ -65,6 +92,12 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Abilities do it { is_expected.to be_falsey } + context 'when pipeline is a detached merge request pipeline' do + include_context 'detached merge request pipeline' + + it { is_expected.to be_falsey } + end + context 'when developers are allowed to merge' do let!(:protected_branch) do create(:protected_branch, @@ -74,6 +107,12 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Abilities do end it { is_expected.to be_truthy } + + context 'when pipeline is a detached merge request pipeline' do + include_context 'detached merge request pipeline' + + it { is_expected.to be_truthy } + end end end @@ -112,6 +151,12 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Abilities do end it { is_expected.to be_truthy } + + context 'when pipeline is a detached merge request pipeline' do + include_context 'detached merge request pipeline' + + it { is_expected.to be_truthy } + end end context 'when the tag is protected' do 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 a7cad423d09..2e8c9d70098 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,25 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Repository do end end + context 'when origin ref is a merge request ref' do + let(:command) do + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, current_user: user, origin_ref: origin_ref, checkout_sha: checkout_sha) + end + + let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + let(:origin_ref) { merge_request.ref_path } + let(:checkout_sha) { project.repository.commit(merge_request.ref_path).id } + + it 'does not break the chain' do + expect(step.break?).to be false + end + + it 'does not append pipeline errors' do + expect(pipeline.errors).to be_empty + end + end + context 'when ref is ambiguous' do let(:project) do create(:project, :repository).tap do |proj| diff --git a/spec/lib/gitlab/ci/status/build/preparing_spec.rb b/spec/lib/gitlab/ci/status/build/preparing_spec.rb new file mode 100644 index 00000000000..4d8945845ba --- /dev/null +++ b/spec/lib/gitlab/ci/status/build/preparing_spec.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Ci::Status::Build::Preparing do + subject do + described_class.new(double('subject')) + end + + describe '#illustration' do + it { expect(subject.illustration).to include(:image, :size, :title, :content) } + end + + describe '.matches?' do + subject { described_class.matches?(build, nil) } + + context 'when build is preparing' do + let(:build) { create(:ci_build, :preparing) } + + it 'is a correct match' do + expect(subject).to be true + end + end + + context 'when build is not preparing' do + let(:build) { create(:ci_build, :success) } + + it 'does not match' do + expect(subject).to be false + end + end + end +end diff --git a/spec/lib/gitlab/ci/status/preparing_spec.rb b/spec/lib/gitlab/ci/status/preparing_spec.rb new file mode 100644 index 00000000000..7211c0e506d --- /dev/null +++ b/spec/lib/gitlab/ci/status/preparing_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Ci::Status::Preparing do + subject do + described_class.new(double('subject'), nil) + end + + describe '#text' do + it { expect(subject.text).to eq 'preparing' } + end + + describe '#label' do + it { expect(subject.label).to eq 'preparing' } + end + + describe '#icon' do + it { expect(subject.icon).to eq 'status_created' } + end + + describe '#favicon' do + it { expect(subject.favicon).to eq 'favicon_status_created' } + end + + describe '#group' do + it { expect(subject.group).to eq 'preparing' } + end +end diff --git a/spec/lib/gitlab/current_settings_spec.rb b/spec/lib/gitlab/current_settings_spec.rb index 17d5eae24f5..909dbffa38f 100644 --- a/spec/lib/gitlab/current_settings_spec.rb +++ b/spec/lib/gitlab/current_settings_spec.rb @@ -115,9 +115,8 @@ describe Gitlab::CurrentSettings do 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 + it 'returns a FakeApplicationSettings object' do + expect(current_settings).to be_a(Gitlab::FakeApplicationSettings) end it 'uses the default value from ApplicationSetting.defaults' do @@ -146,6 +145,16 @@ describe Gitlab::CurrentSettings do it 'uses the value from the DB attribute if present and not overridden by an accessor' do expect(current_settings.home_page_url).to eq(db_settings.home_page_url) end + + context 'when a new column is used before being migrated' do + before do + allow(ApplicationSetting).to receive(:defaults).and_return({ foo: 'bar' }) + end + + it 'uses the default value if present' do + expect(current_settings.foo).to eq('bar') + end + end end end diff --git a/spec/lib/gitlab/database_spec.rb b/spec/lib/gitlab/database_spec.rb index ae50abd0e7a..5f57cd6b825 100644 --- a/spec/lib/gitlab/database_spec.rb +++ b/spec/lib/gitlab/database_spec.rb @@ -17,6 +17,20 @@ describe Gitlab::Database do end end + describe '.human_adapter_name' do + it 'returns PostgreSQL when using PostgreSQL' do + allow(described_class).to receive(:postgresql?).and_return(true) + + expect(described_class.human_adapter_name).to eq('PostgreSQL') + end + + it 'returns MySQL when using MySQL' do + allow(described_class).to receive(:postgresql?).and_return(false) + + expect(described_class.human_adapter_name).to eq('MySQL') + end + end + # These are just simple smoke tests to check if the methods work (regardless # of what they may return). describe '.mysql?' do diff --git a/spec/lib/gitlab/diff/file_spec.rb b/spec/lib/gitlab/diff/file_spec.rb index 611c3e946ed..cc36060f864 100644 --- a/spec/lib/gitlab/diff/file_spec.rb +++ b/spec/lib/gitlab/diff/file_spec.rb @@ -72,6 +72,13 @@ describe Gitlab::Diff::File do expect(diff_file.diff_lines_for_serializer.last.type).to eq('match') end + context 'when called multiple times' do + it 'only adds bottom match line once' do + expect(diff_file.diff_lines_for_serializer.size).to eq(31) + expect(diff_file.diff_lines_for_serializer.size).to eq(31) + end + end + context 'when deleted' do let(:commit) { project.commit('d59c60028b053793cecfb4022de34602e1a9218e') } let(:diff_file) { commit.diffs.diff_file_with_old_path('files/js/commit.js.coffee') } diff --git a/spec/lib/gitlab/diff/suggestion_diff_spec.rb b/spec/lib/gitlab/diff/suggestion_diff_spec.rb new file mode 100644 index 00000000000..5a32c2bea37 --- /dev/null +++ b/spec/lib/gitlab/diff/suggestion_diff_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Diff::SuggestionDiff do + describe '#diff_lines' do + let(:from_content) do + <<-BLOB.strip_heredoc + "tags": ["devel", "development", "nightly"], + "desktop-file-name-prefix": "(Development) ", + "finish-args": "foo", + BLOB + end + + let(:to_content) do + <<-BLOB.strip_heredoc + "buildsystem": "meson", + "builddir": true, + "name": "nautilus", + "bar": "bar", + BLOB + end + + let(:suggestion) do + instance_double(Suggestion, from_line: 12, + from_content: from_content, + to_content: to_content) + end + + subject { described_class.new(suggestion).diff_lines } + + let(:expected_diff_lines) do + [ + { old_pos: 12, new_pos: 12, type: "match", text: "@@ -12 +12" }, + { old_pos: 12, new_pos: 12, type: "old", text: "-\"tags\": [\"devel\", \"development\", \"nightly\"]," }, + { old_pos: 13, new_pos: 12, type: "old", text: "-\"desktop-file-name-prefix\": \"(Development) \"," }, + { old_pos: 14, new_pos: 12, type: "old", text: "-\"finish-args\": \"foo\"," }, + { old_pos: 15, new_pos: 12, type: "new", text: "+\"buildsystem\": \"meson\"," }, + { old_pos: 15, new_pos: 13, type: "new", text: "+\"builddir\": true," }, + { old_pos: 15, new_pos: 14, type: "new", text: "+\"name\": \"nautilus\"," }, + { old_pos: 15, new_pos: 15, type: "new", text: "+\"bar\": \"bar\"," } + ] + end + + it 'returns diff lines with correct line numbers' do + diff_lines = subject + + expect(diff_lines).to all(be_a(Gitlab::Diff::Line)) + + expected_diff_lines.each_with_index do |expected_line, index| + expect(diff_lines[index].to_hash).to include(expected_line) + end + end + end +end diff --git a/spec/lib/gitlab/diff/suggestion_spec.rb b/spec/lib/gitlab/diff/suggestion_spec.rb new file mode 100644 index 00000000000..71fd25df698 --- /dev/null +++ b/spec/lib/gitlab/diff/suggestion_spec.rb @@ -0,0 +1,88 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Diff::Suggestion do + shared_examples 'correct suggestion raw content' do + it 'returns correct raw data' do + expect(suggestion.to_hash).to include(from_content: expected_lines.join, + to_content: "#{text}\n", + lines_above: above, + lines_below: below) + end + end + + let(:merge_request) { create(:merge_request) } + let(:project) { merge_request.project } + let(:position) do + Gitlab::Diff::Position.new(old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: nil, + new_line: 9, + diff_refs: merge_request.diff_refs) + end + let(:diff_file) do + position.diff_file(project.repository) + end + let(:text) { "# parsed suggestion content\n# with comments" } + + def blob_lines_data(from_line, to_line) + diff_file.new_blob_lines_between(from_line, to_line) + end + + def blob_data + blob = diff_file.new_blob + blob.load_all_data! + blob.data + end + + let(:suggestion) do + described_class.new(text, line: line, above: above, below: below, diff_file: diff_file) + end + + describe '#to_hash' do + context 'when changing content surpasses the top limit' do + let(:line) { 4 } + let(:above) { 5 } + let(:below) { 2 } + let(:expected_above) { line - 1 } + let(:expected_below) { below } + let(:expected_lines) { blob_lines_data(line - expected_above, line + expected_below) } + + it_behaves_like 'correct suggestion raw content' + end + + context 'when changing content surpasses the amount of lines in the blob (bottom)' do + let(:line) { 5 } + let(:above) { 1 } + let(:below) { blob_data.lines.size + 10 } + let(:expected_below) { below } + let(:expected_above) { above } + let(:expected_lines) { blob_lines_data(line - expected_above, line + expected_below) } + + it_behaves_like 'correct suggestion raw content' + end + + context 'when lines are within blob lines boundary' do + let(:line) { 5 } + let(:above) { 2 } + let(:below) { 3 } + let(:expected_below) { below } + let(:expected_above) { above } + let(:expected_lines) { blob_lines_data(line - expected_above, line + expected_below) } + + it_behaves_like 'correct suggestion raw content' + end + + context 'when no extra lines (single-line suggestion)' do + let(:line) { 5 } + let(:above) { 0 } + let(:below) { 0 } + let(:expected_below) { below } + let(:expected_above) { above } + let(:expected_lines) { blob_lines_data(line - expected_above, line + expected_below) } + + it_behaves_like 'correct suggestion raw content' + end + end +end diff --git a/spec/lib/gitlab/diff/suggestions_parser_spec.rb b/spec/lib/gitlab/diff/suggestions_parser_spec.rb new file mode 100644 index 00000000000..1119ea04995 --- /dev/null +++ b/spec/lib/gitlab/diff/suggestions_parser_spec.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Diff::SuggestionsParser do + describe '.parse' do + let(:merge_request) { create(:merge_request) } + let(:project) { merge_request.project } + let(:position) do + Gitlab::Diff::Position.new(old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: nil, + new_line: 9, + diff_refs: merge_request.diff_refs) + end + + let(:diff_file) do + position.diff_file(project.repository) + end + + subject do + described_class.parse(markdown, project: merge_request.project, + position: position) + end + + def blob_lines_data(from_line, to_line) + diff_file.new_blob_lines_between(from_line, to_line).join + end + + context 'single-line suggestions' do + let(:markdown) do + <<-MARKDOWN.strip_heredoc + ```suggestion + foo + bar + ``` + + ``` + nothing + ``` + + ```suggestion + xpto + baz + ``` + + ```thing + this is not a suggestion, it's a thing + ``` + MARKDOWN + end + + it 'returns a list of Gitlab::Diff::Suggestion' do + expect(subject).to all(be_a(Gitlab::Diff::Suggestion)) + expect(subject.size).to eq(2) + end + + it 'parsed suggestion has correct data' do + from_line, to_line = position.new_line, position.new_line + + expect(subject.first.to_hash).to include(from_content: blob_lines_data(from_line, to_line), + to_content: " foo\n bar\n", + lines_above: 0, + lines_below: 0) + + expect(subject.second.to_hash).to include(from_content: blob_lines_data(from_line, to_line), + to_content: " xpto\n baz\n", + lines_above: 0, + lines_below: 0) + end + end + end +end diff --git a/spec/lib/gitlab/fake_application_settings_spec.rb b/spec/lib/gitlab/fake_application_settings_spec.rb index af12e13d36d..c81cb83d9f4 100644 --- a/spec/lib/gitlab/fake_application_settings_spec.rb +++ b/spec/lib/gitlab/fake_application_settings_spec.rb @@ -1,32 +1,33 @@ require 'spec_helper' describe Gitlab::FakeApplicationSettings do - let(:defaults) { { password_authentication_enabled_for_web: false, foobar: 'asdf', signup_enabled: true, 'test?' => 123 } } + let(:defaults) do + described_class.defaults.merge( + foobar: 'asdf', + 'test?' => 123 + ) + end - subject { described_class.new(defaults) } + let(:setting) { described_class.new(defaults) } it 'wraps OpenStruct variables properly' do - expect(subject.password_authentication_enabled_for_web).to be_falsey - expect(subject.signup_enabled).to be_truthy - expect(subject.foobar).to eq('asdf') + expect(setting.password_authentication_enabled_for_web).to be_truthy + expect(setting.signup_enabled).to be_truthy + expect(setting.foobar).to eq('asdf') end it 'defines predicate methods' do - expect(subject.password_authentication_enabled_for_web?).to be_falsey - expect(subject.signup_enabled?).to be_truthy - end - - it 'predicate method changes when value is updated' do - subject.password_authentication_enabled_for_web = true - - expect(subject.password_authentication_enabled_for_web?).to be_truthy + expect(setting.password_authentication_enabled_for_web?).to be_truthy + expect(setting.signup_enabled?).to be_truthy end it 'does not define a predicate method' do - expect(subject.foobar?).to be_nil + expect(setting.foobar?).to be_nil end it 'does not override an existing predicate method' do - expect(subject.test?).to eq(123) + expect(setting.test?).to eq(123) end + + it_behaves_like 'application settings examples' end diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index 3fb41a626b2..4a4ac833e39 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -537,6 +537,18 @@ describe Gitlab::Git::Commit, :seed_helper do end end + describe '#gitaly_commit?' do + context 'when the commit data comes from gitaly' do + it { expect(commit.gitaly_commit?).to eq(true) } + end + + context 'when the commit data comes from a Hash' do + let(:commit) { described_class.new(repository, sample_commit_hash) } + + it { expect(commit.gitaly_commit?).to eq(false) } + end + end + describe '#has_zero_stats?' do it { expect(commit.has_zero_stats?).to eq(false) } end diff --git a/spec/lib/gitlab/git/pre_receive_error_spec.rb b/spec/lib/gitlab/git/pre_receive_error_spec.rb index 1b8be62dec6..cb030e38032 100644 --- a/spec/lib/gitlab/git/pre_receive_error_spec.rb +++ b/spec/lib/gitlab/git/pre_receive_error_spec.rb @@ -1,9 +1,19 @@ require 'spec_helper' describe Gitlab::Git::PreReceiveError do - it 'makes its message HTML-friendly' do - ex = described_class.new("hello\nworld\n") + Gitlab::Git::PreReceiveError::SAFE_MESSAGE_PREFIXES.each do |prefix| + context "error messages prefixed with #{prefix}" do + it 'accepts only errors lines with the prefix' do + ex = described_class.new("#{prefix} Hello,\nworld!") - expect(ex.message).to eq('hello<br>world<br>') + expect(ex.message).to eq('Hello,') + end + + it 'makes its message HTML-friendly' do + ex = described_class.new("#{prefix} Hello,\n#{prefix} world!\n") + + expect(ex.message).to eq('Hello,<br>world!') + end + end end end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 7e6dfa30e37..8ba6862392c 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1688,6 +1688,11 @@ describe Gitlab::Git::Repository, :seed_helper do expect(repository.delete_config(*%w[does.not.exist test.foo1 test.foo2])).to be_nil + # Workaround for https://github.com/libgit2/rugged/issues/785: If + # Gitaly changes .gitconfig while Rugged has the file loaded + # Rugged::Repository#each_key will report stale values unless a + # lookup is done first. + expect(repository_rugged.config['test.foo1']).to be_nil config_keys = repository_rugged.config.each_key.to_a expect(config_keys).not_to include('test.foo1') expect(config_keys).not_to include('test.foo2') diff --git a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb index d7bd757149d..6d6107ca3e7 100644 --- a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb @@ -221,6 +221,21 @@ describe Gitlab::GitalyClient::CommitService do expect(commit).to eq(commit_dbl) end end + + context 'when caching of the ref name is enabled' do + it 'returns a cached commit' do + expect_any_instance_of(Gitaly::CommitService::Stub).to receive(:find_commit).once.and_return(double(commit: commit_dbl)) + + commit = nil + 2.times do + ::Gitlab::GitalyClient.allow_ref_name_caching do + commit = described_class.new(repository).find_commit('master') + end + end + + expect(commit).to eq(commit_dbl) + end + end end end diff --git a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb index b37fe2686b6..7579a6577b9 100644 --- a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb @@ -39,7 +39,7 @@ describe Gitlab::GitalyClient::OperationService do context "when pre_receive_error is present" do let(:response) do - Gitaly::UserCreateBranchResponse.new(pre_receive_error: "something failed") + Gitaly::UserCreateBranchResponse.new(pre_receive_error: "GitLab: something failed") end it "throws a PreReceive exception" do @@ -80,7 +80,7 @@ describe Gitlab::GitalyClient::OperationService do context "when pre_receive_error is present" do let(:response) do - Gitaly::UserUpdateBranchResponse.new(pre_receive_error: "something failed") + Gitaly::UserUpdateBranchResponse.new(pre_receive_error: "GitLab: something failed") end it "throws a PreReceive exception" do @@ -117,7 +117,7 @@ describe Gitlab::GitalyClient::OperationService do context "when pre_receive_error is present" do let(:response) do - Gitaly::UserDeleteBranchResponse.new(pre_receive_error: "something failed") + Gitaly::UserDeleteBranchResponse.new(pre_receive_error: "GitLab: something failed") end it "throws a PreReceive exception" do @@ -175,7 +175,7 @@ describe Gitlab::GitalyClient::OperationService do shared_examples 'cherry pick and revert errors' do context 'when a pre_receive_error is present' do - let(:response) { response_class.new(pre_receive_error: "something failed") } + let(:response) { response_class.new(pre_receive_error: "GitLab: something failed") } it 'raises a PreReceiveError' do expect { subject }.to raise_error(Gitlab::Git::PreReceiveError, "something failed") @@ -313,7 +313,7 @@ describe Gitlab::GitalyClient::OperationService do end context 'when a pre_receive_error is present' do - let(:response) { Gitaly::UserCommitFilesResponse.new(pre_receive_error: "something failed") } + let(:response) { Gitaly::UserCommitFilesResponse.new(pre_receive_error: "GitLab: something failed") } it 'raises a PreReceiveError' do expect { subject }.to raise_error(Gitlab::Git::PreReceiveError, "something failed") diff --git a/spec/lib/gitlab/gitaly_client/ref_service_spec.rb b/spec/lib/gitlab/gitaly_client/ref_service_spec.rb index 400d426c949..0dab39575b9 100644 --- a/spec/lib/gitlab/gitaly_client/ref_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/ref_service_spec.rb @@ -89,6 +89,16 @@ describe Gitlab::GitalyClient::RefService do end end + describe '#list_new_blobs' do + it 'raises DeadlineExceeded when timeout is too small' do + newrev = '54fcc214b94e78d7a41a9a8fe6d87a5e59500e51' + + expect do + client.list_new_blobs(newrev, dynamic_timeout: 0.001) + end.to raise_error(GRPC::DeadlineExceeded) + end + end + describe '#local_branches' do it 'sends a find_local_branches message' do expect_any_instance_of(Gitaly::RefService::Stub) diff --git a/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb index 15e59718dce..2e4a7c36fb8 100644 --- a/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb @@ -11,6 +11,7 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi let(:source_commit) { project.repository.commit('feature') } let(:target_commit) { project.repository.commit('master') } let(:milestone) { create(:milestone, project: project) } + let(:state) { :closed } let(:pull_request) do alice = Gitlab::GithubImport::Representation::User.new(id: 4, login: 'alice') @@ -26,13 +27,13 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi source_repository_id: 400, target_repository_id: 200, source_repository_owner: 'alice', - state: :closed, + state: state, milestone_number: milestone.iid, author: alice, assignee: alice, created_at: created_at, updated_at: updated_at, - merged_at: merged_at + merged_at: state == :closed && merged_at ) end @@ -260,53 +261,63 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi end it 'does not create the source branch if merge request is merged' do - mr, exists = importer.create_merge_request - - importer.insert_git_data(mr, exists) + mr = insert_git_data expect(project.repository.branch_exists?(mr.source_branch)).to be_falsey expect(project.repository.branch_exists?(mr.target_branch)).to be_truthy end - it 'creates the source branch if merge request is open' do - mr, exists = importer.create_merge_request - mr.state = 'opened' - mr.save + context 'when merge request is open' do + let(:state) { :opened } - importer.insert_git_data(mr, exists) + it 'creates the source branch' do + # Ensure the project creator is creating the branches because the + # merge request author may not have access to push to this + # repository. The project owner may also be a group. + allow(project.repository).to receive(:add_branch).with(project.creator, anything, anything).and_call_original - expect(project.repository.branch_exists?(mr.source_branch)).to be_truthy - expect(project.repository.branch_exists?(mr.target_branch)).to be_truthy - end + mr = insert_git_data - it 'ignores Git errors when creating a branch' do - mr, exists = importer.create_merge_request - mr.state = 'opened' - mr.save + expect(project.repository.branch_exists?(mr.source_branch)).to be_truthy + expect(project.repository.branch_exists?(mr.target_branch)).to be_truthy + end - expect(project.repository).to receive(:add_branch).and_raise(Gitlab::Git::CommandError) - expect(Gitlab::Sentry).to receive(:track_acceptable_exception).and_call_original + it 'is able to retry on pre-receive errors' do + expect(importer).to receive(:insert_or_replace_git_data).twice.and_call_original + expect(project.repository).to receive(:add_branch).and_raise('exception') - importer.insert_git_data(mr, exists) + expect { insert_git_data }.to raise_error('exception') - expect(project.repository.branch_exists?(mr.source_branch)).to be_falsey - expect(project.repository.branch_exists?(mr.target_branch)).to be_truthy + expect(project.repository).to receive(:add_branch).with(project.creator, anything, anything).and_call_original + + mr = insert_git_data + + expect(project.repository.branch_exists?(mr.source_branch)).to be_truthy + expect(project.repository.branch_exists?(mr.target_branch)).to be_truthy + expect(mr.merge_request_diffs).to be_one + end + + it 'ignores Git command errors when creating a branch' do + expect(project.repository).to receive(:add_branch).and_raise(Gitlab::Git::CommandError) + expect(Gitlab::Sentry).to receive(:track_acceptable_exception).and_call_original + + mr = insert_git_data + + expect(project.repository.branch_exists?(mr.source_branch)).to be_falsey + expect(project.repository.branch_exists?(mr.target_branch)).to be_truthy + end end it 'creates the merge request diffs' do - mr, exists = importer.create_merge_request - - importer.insert_git_data(mr, exists) + mr = insert_git_data expect(mr.merge_request_diffs.exists?).to eq(true) end it 'creates the merge request diff commits' do - mr, exists = importer.create_merge_request - - importer.insert_git_data(mr, exists) + mr = insert_git_data - diff = mr.merge_request_diffs.take + diff = mr.merge_request_diffs.reload.first expect(diff.merge_request_diff_commits.exists?).to eq(true) end @@ -322,5 +333,11 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi expect(mr.merge_request_diffs.exists?).to eq(true) end end + + def insert_git_data + mr, exists = importer.create_merge_request + importer.insert_git_data(mr, exists) + mr + end end end diff --git a/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb b/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb index 47233ea6ee2..41810a8ec03 100644 --- a/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb @@ -179,6 +179,17 @@ describe Gitlab::GithubImport::Importer::RepositoryImporter do describe '#import_repository' do it 'imports the repository' do + repo = double(:repo, default_branch: 'develop') + + expect(client) + .to receive(:repository) + .with('foo/bar') + .and_return(repo) + + expect(project) + .to receive(:change_head) + .with('develop') + expect(project) .to receive(:ensure_repository) diff --git a/spec/lib/gitlab/gl_repository/repo_type_spec.rb b/spec/lib/gitlab/gl_repository/repo_type_spec.rb new file mode 100644 index 00000000000..f06a2448ff7 --- /dev/null +++ b/spec/lib/gitlab/gl_repository/repo_type_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Gitlab::GlRepository::RepoType do + set(:project) { create(:project) } + + shared_examples 'a repo type' do + describe "#identifier_for_subject" do + subject { described_class.identifier_for_subject(project) } + + it { is_expected.to eq(expected_identifier) } + end + + describe "#fetch_id" do + it "finds an id match in the identifier" do + expect(described_class.fetch_id(expected_identifier)).to eq(expected_id) + end + + it 'does not break on other identifiers' do + expect(described_class.fetch_id("wiki-noid")).to eq(nil) + end + end + + describe "#path_suffix" do + subject { described_class.path_suffix } + + it { is_expected.to eq(expected_suffix) } + end + + describe "#repository_for" do + it "finds the repository for the repo type" do + expect(described_class.repository_for(project)).to eq(expected_repository) + end + end + end + + describe Gitlab::GlRepository::PROJECT do + it_behaves_like 'a repo type' do + let(:expected_identifier) { "project-#{project.id}" } + let(:expected_id) { project.id.to_s } + let(:expected_suffix) { "" } + let(:expected_repository) { project.repository } + end + + it "knows its type" do + expect(described_class).not_to be_wiki + expect(described_class).to be_project + end + end + + describe Gitlab::GlRepository::WIKI do + it_behaves_like 'a repo type' do + let(:expected_identifier) { "wiki-#{project.id}" } + let(:expected_id) { project.id.to_s } + let(:expected_suffix) { ".wiki" } + let(:expected_repository) { project.wiki.repository } + end + + it "knows its type" do + expect(described_class).to be_wiki + expect(described_class).not_to be_project + end + end +end diff --git a/spec/lib/gitlab/gl_repository_spec.rb b/spec/lib/gitlab/gl_repository_spec.rb index 4e09020471b..d4b6c629659 100644 --- a/spec/lib/gitlab/gl_repository_spec.rb +++ b/spec/lib/gitlab/gl_repository_spec.rb @@ -5,11 +5,11 @@ describe ::Gitlab::GlRepository do set(:project) { create(:project, :repository) } it 'parses a project gl_repository' do - expect(described_class.parse("project-#{project.id}")).to eq([project, false]) + expect(described_class.parse("project-#{project.id}")).to eq([project, Gitlab::GlRepository::PROJECT]) end it 'parses a wiki gl_repository' do - expect(described_class.parse("wiki-#{project.id}")).to eq([project, true]) + expect(described_class.parse("wiki-#{project.id}")).to eq([project, Gitlab::GlRepository::WIKI]) end it 'throws an argument error on an invalid gl_repository' do diff --git a/spec/lib/gitlab/group_search_results_spec.rb b/spec/lib/gitlab/group_search_results_spec.rb new file mode 100644 index 00000000000..2734fcef0a0 --- /dev/null +++ b/spec/lib/gitlab/group_search_results_spec.rb @@ -0,0 +1,69 @@ +require 'spec_helper' + +describe Gitlab::GroupSearchResults do + let(:user) { create(:user) } + + describe 'user search' do + let(:group) { create(:group) } + + it 'returns the users belonging to the group matching the search query' do + user1 = create(:user, username: 'gob_bluth') + create(:group_member, :developer, user: user1, group: group) + + user2 = create(:user, username: 'michael_bluth') + create(:group_member, :developer, user: user2, group: group) + + create(:user, username: 'gob_2018') + + result = described_class.new(user, anything, group, 'gob').objects('users') + + expect(result).to eq [user1] + end + + it 'returns the user belonging to the subgroup matching the search query', :nested_groups do + user1 = create(:user, username: 'gob_bluth') + subgroup = create(:group, parent: group) + create(:group_member, :developer, user: user1, group: subgroup) + + create(:user, username: 'gob_2018') + + result = described_class.new(user, anything, group, 'gob').objects('users') + + expect(result).to eq [user1] + end + + it 'returns the user belonging to the parent group matching the search query', :nested_groups do + user1 = create(:user, username: 'gob_bluth') + parent_group = create(:group, children: [group]) + create(:group_member, :developer, user: user1, group: parent_group) + + create(:user, username: 'gob_2018') + + result = described_class.new(user, anything, group, 'gob').objects('users') + + expect(result).to eq [user1] + end + + it 'does not return the user belonging to the private subgroup', :nested_groups do + user1 = create(:user, username: 'gob_bluth') + subgroup = create(:group, :private, parent: group) + create(:group_member, :developer, user: user1, group: subgroup) + + create(:user, username: 'gob_2018') + + result = described_class.new(user, anything, group, 'gob').objects('users') + + expect(result).to eq [] + end + + it 'does not return the user belonging to an unrelated group' do + user = create(:user, username: 'gob_bluth') + unrelated_group = create(:group) + create(:group_member, :developer, user: user, group: unrelated_group) + + result = described_class.new(user, anything, group, 'gob').objects('users') + + expect(result).to eq [] + end + end +end diff --git a/spec/lib/gitlab/hashed_storage/migrator_spec.rb b/spec/lib/gitlab/hashed_storage/migrator_spec.rb index d03a74ac9eb..8e253b51597 100644 --- a/spec/lib/gitlab/hashed_storage/migrator_spec.rb +++ b/spec/lib/gitlab/hashed_storage/migrator_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::HashedStorage::Migrator, :sidekiq do +describe Gitlab::HashedStorage::Migrator, :sidekiq, :redis do describe '#bulk_schedule_migration' do it 'schedules job to HashedStorage::MigratorWorker' do Sidekiq::Testing.fake! do @@ -189,7 +189,7 @@ describe Gitlab::HashedStorage::Migrator, :sidekiq do set(:project) { create(:project, :empty_repo) } it 'returns true when there are MigratorWorker jobs scheduled' do - Sidekiq::Testing.fake! do + Sidekiq::Testing.disable! do ::HashedStorage::MigratorWorker.perform_async(1, 5) expect(subject.migration_pending?).to be_truthy @@ -197,7 +197,7 @@ describe Gitlab::HashedStorage::Migrator, :sidekiq do end it 'returns true when there are ProjectMigrateWorker jobs scheduled' do - Sidekiq::Testing.fake! do + Sidekiq::Testing.disable! do ::HashedStorage::ProjectMigrateWorker.perform_async(1) expect(subject.migration_pending?).to be_truthy @@ -213,7 +213,7 @@ describe Gitlab::HashedStorage::Migrator, :sidekiq do set(:project) { create(:project, :empty_repo) } it 'returns true when there are RollbackerWorker jobs scheduled' do - Sidekiq::Testing.fake! do + Sidekiq::Testing.disable! do ::HashedStorage::RollbackerWorker.perform_async(1, 5) expect(subject.rollback_pending?).to be_truthy @@ -221,7 +221,7 @@ describe Gitlab::HashedStorage::Migrator, :sidekiq do end it 'returns true when there are jobs scheduled' do - Sidekiq::Testing.fake! do + Sidekiq::Testing.disable! do ::HashedStorage::ProjectRollbackWorker.perform_async(1) expect(subject.rollback_pending?).to be_truthy diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 01da3ea7081..e418516569a 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -100,6 +100,8 @@ merge_requests: - head_pipeline - latest_merge_request_diff - merge_request_pipelines +- merge_request_assignees +- suggestions merge_request_diff: - merge_request - merge_request_diff_commits @@ -351,3 +353,5 @@ resource_label_events: - label error_tracking_setting: - project +suggestions: +- note diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index ee96e5c4d42..496567b0036 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -608,3 +608,14 @@ ErrorTracking::ProjectErrorTrackingSetting: - project_id - project_name - organization_name +Suggestion: +- id +- note_id +- relative_order +- applied +- commit_id +- from_content +- to_content +- outdated +- lines_above +- lines_below diff --git a/spec/lib/gitlab/json_cache_spec.rb b/spec/lib/gitlab/json_cache_spec.rb index 2cae8ec031a..b82c09af306 100644 --- a/spec/lib/gitlab/json_cache_spec.rb +++ b/spec/lib/gitlab/json_cache_spec.rb @@ -7,7 +7,7 @@ describe Gitlab::JsonCache do let(:namespace) { 'geo' } let(:key) { 'foo' } let(:expanded_key) { "#{namespace}:#{key}:#{Rails.version}" } - let(:broadcast_message) { create(:broadcast_message) } + set(:broadcast_message) { create(:broadcast_message) } subject(:cache) { described_class.new(namespace: namespace, backend: backend) } @@ -146,6 +146,18 @@ describe Gitlab::JsonCache do expect(cache.read(key, BroadcastMessage)).to be_nil end + + it 'gracefully handles excluded fields from attributes during serialization' do + allow(backend).to receive(:read) + .with(expanded_key) + .and_return(broadcast_message.attributes.except("message_html").to_json) + + result = cache.read(key, BroadcastMessage) + + BroadcastMessage.cached_markdown_fields.html_fields.each do |field| + expect(result.public_send(field)).to be_nil + end + end end context 'when the cached value is an array' do @@ -321,6 +333,46 @@ describe Gitlab::JsonCache do expect(result).to be_new_record end + + it 'gracefully handles bad cached entry' do + allow(backend).to receive(:read) + .with(expanded_key) + .and_return('{') + + result = cache.fetch(key, as: BroadcastMessage) { 'block result' } + + expect(result).to eq 'block result' + end + + it 'gracefully handles an empty hash' do + allow(backend).to receive(:read) + .with(expanded_key) + .and_return('{}') + + expect(cache.fetch(key, as: BroadcastMessage)).to be_a(BroadcastMessage) + end + + it 'gracefully handles unknown attributes' do + allow(backend).to receive(:read) + .with(expanded_key) + .and_return(broadcast_message.attributes.merge(unknown_attribute: 1).to_json) + + result = cache.fetch(key, as: BroadcastMessage) { 'block result' } + + expect(result).to eq 'block result' + end + + it 'gracefully handles excluded fields from attributes during serialization' do + allow(backend).to receive(:read) + .with(expanded_key) + .and_return(broadcast_message.attributes.except("message_html").to_json) + + result = cache.fetch(key, as: BroadcastMessage) { 'block result' } + + BroadcastMessage.cached_markdown_fields.html_fields.each do |field| + expect(result.public_send(field)).to be_nil + end + end end it "returns the result of the block when 'as' option is nil" do diff --git a/spec/lib/gitlab/kubernetes_spec.rb b/spec/lib/gitlab/kubernetes_spec.rb index f326d57e9c6..57b570a9166 100644 --- a/spec/lib/gitlab/kubernetes_spec.rb +++ b/spec/lib/gitlab/kubernetes_spec.rb @@ -40,10 +40,40 @@ describe Gitlab::Kubernetes do describe '#filter_by_label' do it 'returns matching labels' do - matching_items = [kube_pod(app: 'foo')] + matching_items = [kube_pod(track: 'foo'), kube_deployment(track: 'foo')] + items = matching_items + [kube_pod, kube_deployment] + + expect(filter_by_label(items, 'track' => 'foo')).to eq(matching_items) + end + end + + describe '#filter_by_annotation' do + it 'returns matching labels' do + matching_items = [kube_pod(environment_slug: 'foo'), kube_deployment(environment_slug: 'foo')] + items = matching_items + [kube_pod, kube_deployment] + + expect(filter_by_annotation(items, 'app.gitlab.com/env' => 'foo')).to eq(matching_items) + end + end + + describe '#filter_by_project_environment' do + let(:matching_pod) { kube_pod(environment_slug: 'production', project_slug: 'my-cool-app') } + + it 'returns matching legacy env label' do + matching_pod['metadata']['annotations'].delete('app.gitlab.com/app') + matching_pod['metadata']['annotations'].delete('app.gitlab.com/env') + matching_pod['metadata']['labels']['app'] = 'production' + matching_items = [matching_pod] + items = matching_items + [kube_pod] + + expect(filter_by_project_environment(items, 'my-cool-app', 'production')).to eq(matching_items) + end + + it 'returns matching env label' do + matching_items = [matching_pod] items = matching_items + [kube_pod] - expect(filter_by_label(items, app: 'foo')).to eq(matching_items) + expect(filter_by_project_environment(items, 'my-cool-app', 'production')).to eq(matching_items) end end diff --git a/spec/lib/gitlab/project_search_results_spec.rb b/spec/lib/gitlab/project_search_results_spec.rb index 6831274d37c..4a41d5cf51e 100644 --- a/spec/lib/gitlab/project_search_results_spec.rb +++ b/spec/lib/gitlab/project_search_results_spec.rb @@ -412,4 +412,36 @@ describe Gitlab::ProjectSearchResults do end end end + + describe 'user search' do + it 'returns the user belonging to the project matching the search query' do + project = create(:project) + + user1 = create(:user, username: 'gob_bluth') + create(:project_member, :developer, user: user1, project: project) + + user2 = create(:user, username: 'michael_bluth') + create(:project_member, :developer, user: user2, project: project) + + create(:user, username: 'gob_2018') + + result = described_class.new(user, project, 'gob').objects('users') + + expect(result).to eq [user1] + end + + it 'returns the user belonging to the group matching the search query' do + group = create(:group) + project = create(:project, namespace: group) + + user1 = create(:user, username: 'gob_bluth') + create(:group_member, :developer, user: user1, group: group) + + create(:user, username: 'gob_2018') + + result = described_class.new(user, project, 'gob').objects('users') + + expect(result).to eq [user1] + end + end end diff --git a/spec/lib/gitlab/quick_actions/command_definition_spec.rb b/spec/lib/gitlab/quick_actions/command_definition_spec.rb index 136cfb5bcc5..b6e0adbc1c2 100644 --- a/spec/lib/gitlab/quick_actions/command_definition_spec.rb +++ b/spec/lib/gitlab/quick_actions/command_definition_spec.rb @@ -69,6 +69,36 @@ describe Gitlab::QuickActions::CommandDefinition do expect(subject.available?(opts)).to be true end end + + context "when the command has types" do + before do + subject.types = [Issue, Commit] + end + + context "when the command target type is allowed" do + it "returns true" do + opts[:quick_action_target] = Issue.new + expect(subject.available?(opts)).to be true + end + end + + context "when the command target type is not allowed" do + it "returns true" do + opts[:quick_action_target] = MergeRequest.new + expect(subject.available?(opts)).to be false + end + end + end + + context "when the command has no types" do + it "any target type is allowed" do + opts[:quick_action_target] = Issue.new + expect(subject.available?(opts)).to be true + + opts[:quick_action_target] = MergeRequest.new + expect(subject.available?(opts)).to be true + end + end end describe "#execute" do diff --git a/spec/lib/gitlab/quick_actions/dsl_spec.rb b/spec/lib/gitlab/quick_actions/dsl_spec.rb index fd4df8694ba..185adab1ff6 100644 --- a/spec/lib/gitlab/quick_actions/dsl_spec.rb +++ b/spec/lib/gitlab/quick_actions/dsl_spec.rb @@ -48,13 +48,19 @@ describe Gitlab::QuickActions::Dsl do substitution :something do |text| "#{text} Some complicated thing you want in here" end + + desc 'A command with types' + types Issue, Commit + command :has_types do + "Has Issue and Commit types" + end end end describe '.command_definitions' do it 'returns an array with commands definitions' do no_args_def, explanation_with_aliases_def, dynamic_description_def, - cc_def, cond_action_def, with_params_parsing_def, substitution_def = + cc_def, cond_action_def, with_params_parsing_def, substitution_def, has_types = DummyClass.command_definitions expect(no_args_def.name).to eq(:no_args) @@ -63,6 +69,7 @@ describe Gitlab::QuickActions::Dsl do expect(no_args_def.explanation).to eq('') expect(no_args_def.params).to eq([]) expect(no_args_def.condition_block).to be_nil + expect(no_args_def.types).to eq([]) expect(no_args_def.action_block).to be_a_kind_of(Proc) expect(no_args_def.parse_params_block).to be_nil expect(no_args_def.warning).to eq('') @@ -73,6 +80,7 @@ describe Gitlab::QuickActions::Dsl do expect(explanation_with_aliases_def.explanation).to eq('Static explanation') expect(explanation_with_aliases_def.params).to eq(['The first argument']) expect(explanation_with_aliases_def.condition_block).to be_nil + expect(explanation_with_aliases_def.types).to eq([]) expect(explanation_with_aliases_def.action_block).to be_a_kind_of(Proc) expect(explanation_with_aliases_def.parse_params_block).to be_nil expect(explanation_with_aliases_def.warning).to eq('Possible problem!') @@ -83,6 +91,7 @@ describe Gitlab::QuickActions::Dsl do expect(dynamic_description_def.explanation).to eq('') expect(dynamic_description_def.params).to eq(['The first argument', 'The second argument']) expect(dynamic_description_def.condition_block).to be_nil + expect(dynamic_description_def.types).to eq([]) expect(dynamic_description_def.action_block).to be_a_kind_of(Proc) expect(dynamic_description_def.parse_params_block).to be_nil expect(dynamic_description_def.warning).to eq('') @@ -93,6 +102,7 @@ describe Gitlab::QuickActions::Dsl do expect(cc_def.explanation).to eq('') expect(cc_def.params).to eq([]) expect(cc_def.condition_block).to be_nil + expect(cc_def.types).to eq([]) expect(cc_def.action_block).to be_nil expect(cc_def.parse_params_block).to be_nil expect(cc_def.warning).to eq('') @@ -103,6 +113,7 @@ describe Gitlab::QuickActions::Dsl do expect(cond_action_def.explanation).to be_a_kind_of(Proc) expect(cond_action_def.params).to eq([]) expect(cond_action_def.condition_block).to be_a_kind_of(Proc) + expect(cond_action_def.types).to eq([]) expect(cond_action_def.action_block).to be_a_kind_of(Proc) expect(cond_action_def.parse_params_block).to be_nil expect(cond_action_def.warning).to eq('') @@ -113,6 +124,7 @@ describe Gitlab::QuickActions::Dsl do expect(with_params_parsing_def.explanation).to eq('') expect(with_params_parsing_def.params).to eq([]) expect(with_params_parsing_def.condition_block).to be_nil + expect(with_params_parsing_def.types).to eq([]) expect(with_params_parsing_def.action_block).to be_a_kind_of(Proc) expect(with_params_parsing_def.parse_params_block).to be_a_kind_of(Proc) expect(with_params_parsing_def.warning).to eq('') @@ -123,9 +135,21 @@ describe Gitlab::QuickActions::Dsl do expect(substitution_def.explanation).to eq('') expect(substitution_def.params).to eq(['<Comment>']) expect(substitution_def.condition_block).to be_nil + expect(substitution_def.types).to eq([]) expect(substitution_def.action_block.call('text')).to eq('text Some complicated thing you want in here') expect(substitution_def.parse_params_block).to be_nil expect(substitution_def.warning).to eq('') + + expect(has_types.name).to eq(:has_types) + expect(has_types.aliases).to eq([]) + expect(has_types.description).to eq('A command with types') + expect(has_types.explanation).to eq('') + expect(has_types.params).to eq([]) + expect(has_types.condition_block).to be_nil + expect(has_types.types).to eq([Issue, Commit]) + expect(has_types.action_block).to be_a_kind_of(Proc) + expect(has_types.parse_params_block).to be_nil + expect(has_types.warning).to eq('') end end end diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb index 4139d1c650c..d982053d92e 100644 --- a/spec/lib/gitlab/reference_extractor_spec.rb +++ b/spec/lib/gitlab/reference_extractor_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe Gitlab::ReferenceExtractor do diff --git a/spec/lib/gitlab/repo_path_spec.rb b/spec/lib/gitlab/repo_path_spec.rb index 13940713dfc..4c7ca4e2b57 100644 --- a/spec/lib/gitlab/repo_path_spec.rb +++ b/spec/lib/gitlab/repo_path_spec.rb @@ -6,43 +6,47 @@ describe ::Gitlab::RepoPath do context 'a repository storage path' do it 'parses a full repository path' do - expect(described_class.parse(project.repository.full_path)).to eq([project, false, nil]) + expect(described_class.parse(project.repository.full_path)).to eq([project, Gitlab::GlRepository::PROJECT, nil]) end it 'parses a full wiki path' do - expect(described_class.parse(project.wiki.repository.full_path)).to eq([project, true, nil]) + expect(described_class.parse(project.wiki.repository.full_path)).to eq([project, Gitlab::GlRepository::WIKI, nil]) end end context 'a relative path' do it 'parses a relative repository path' do - expect(described_class.parse(project.full_path + '.git')).to eq([project, false, nil]) + expect(described_class.parse(project.full_path + '.git')).to eq([project, Gitlab::GlRepository::PROJECT, nil]) end it 'parses a relative wiki path' do - expect(described_class.parse(project.full_path + '.wiki.git')).to eq([project, true, nil]) + expect(described_class.parse(project.full_path + '.wiki.git')).to eq([project, Gitlab::GlRepository::WIKI, nil]) end it 'parses a relative path starting with /' do - expect(described_class.parse('/' + project.full_path + '.git')).to eq([project, false, nil]) + expect(described_class.parse('/' + project.full_path + '.git')).to eq([project, Gitlab::GlRepository::PROJECT, nil]) end context 'of a redirected project' do let(:redirect) { project.route.create_redirect('foo/bar') } it 'parses a relative repository path' do - expect(described_class.parse(redirect.path + '.git')).to eq([project, false, 'foo/bar']) + expect(described_class.parse(redirect.path + '.git')).to eq([project, Gitlab::GlRepository::PROJECT, 'foo/bar']) end it 'parses a relative wiki path' do - expect(described_class.parse(redirect.path + '.wiki.git')).to eq([project, true, 'foo/bar.wiki']) + expect(described_class.parse(redirect.path + '.wiki.git')).to eq([project, Gitlab::GlRepository::WIKI, 'foo/bar.wiki']) end it 'parses a relative path starting with /' do - expect(described_class.parse('/' + redirect.path + '.git')).to eq([project, false, 'foo/bar']) + expect(described_class.parse('/' + redirect.path + '.git')).to eq([project, Gitlab::GlRepository::PROJECT, 'foo/bar']) end end end + + it "returns nil for non existent paths" do + expect(described_class.parse("path/non-existent.git")).to eq(nil) + end end describe '.find_project' do diff --git a/spec/lib/gitlab/search_results_spec.rb b/spec/lib/gitlab/search_results_spec.rb index 87288baedb0..4b57eecff93 100644 --- a/spec/lib/gitlab/search_results_spec.rb +++ b/spec/lib/gitlab/search_results_spec.rb @@ -121,6 +121,22 @@ describe Gitlab::SearchResults do results.objects('issues') end end + + describe '#users' do + it 'does not call the UsersFinder when the current_user is not allowed to read users list' do + allow(Ability).to receive(:allowed?).and_return(false) + + expect(UsersFinder).not_to receive(:new).with(user, search: 'foo').and_call_original + + results.objects('users') + end + + it 'calls the UsersFinder' do + expect(UsersFinder).to receive(:new).with(user, search: 'foo').and_call_original + + results.objects('users') + end + end end it 'does not list issues on private projects' do diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index d6aadf0f7de..e2f09de2808 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -8,6 +8,7 @@ describe Gitlab::Shell do let(:gitlab_shell) { described_class.new } let(:popen_vars) { { 'GIT_TERMINAL_PROMPT' => ENV['GIT_TERMINAL_PROMPT'] } } let(:timeout) { Gitlab.config.gitlab_shell.git_timeout } + let(:gitlab_authorized_keys) { double } before do allow(Project).to receive(:find).and_return(project) @@ -49,13 +50,38 @@ describe Gitlab::Shell do describe '#add_key' do context 'when authorized_keys_enabled is true' do - it 'removes trailing garbage' do - allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path) - expect(gitlab_shell).to receive(:gitlab_shell_fast_execute).with( - [:gitlab_shell_keys_path, 'add-key', 'key-123', 'ssh-rsa foobar'] - ) + context 'authorized_keys_file not set' do + before do + stub_gitlab_shell_setting(authorized_keys_file: nil) + allow(gitlab_shell) + .to receive(:gitlab_shell_keys_path) + .and_return(:gitlab_shell_keys_path) + end + + it 'calls #gitlab_shell_fast_execute with add-key command' do + expect(gitlab_shell) + .to receive(:gitlab_shell_fast_execute) + .with([ + :gitlab_shell_keys_path, + 'add-key', + 'key-123', + 'ssh-rsa foobar' + ]) + + gitlab_shell.add_key('key-123', 'ssh-rsa foobar trailing garbage') + end + end + + context 'authorized_keys_file set' do + it 'calls Gitlab::AuthorizedKeys#add_key with id and key' do + expect(Gitlab::AuthorizedKeys).to receive(:new).and_return(gitlab_authorized_keys) - gitlab_shell.add_key('key-123', 'ssh-rsa foobar trailing garbage') + expect(gitlab_authorized_keys) + .to receive(:add_key) + .with('key-123', 'ssh-rsa foobar') + + gitlab_shell.add_key('key-123', 'ssh-rsa foobar') + end end end @@ -64,10 +90,24 @@ describe Gitlab::Shell do stub_application_setting(authorized_keys_enabled: false) end - it 'does nothing' do - expect(gitlab_shell).not_to receive(:gitlab_shell_fast_execute) + context 'authorized_keys_file not set' do + before do + stub_gitlab_shell_setting(authorized_keys_file: nil) + end + + it 'does nothing' do + expect(gitlab_shell).not_to receive(:gitlab_shell_fast_execute) + + gitlab_shell.add_key('key-123', 'ssh-rsa foobar trailing garbage') + end + end + + context 'authorized_keys_file set' do + it 'does nothing' do + expect(Gitlab::AuthorizedKeys).not_to receive(:new) - gitlab_shell.add_key('key-123', 'ssh-rsa foobar trailing garbage') + gitlab_shell.add_key('key-123', 'ssh-rsa foobar trailing garbage') + end end end @@ -76,24 +116,89 @@ describe Gitlab::Shell do stub_application_setting(authorized_keys_enabled: nil) end - it 'removes trailing garbage' do - allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path) - expect(gitlab_shell).to receive(:gitlab_shell_fast_execute).with( - [:gitlab_shell_keys_path, 'add-key', 'key-123', 'ssh-rsa foobar'] - ) + context 'authorized_keys_file not set' do + before do + stub_gitlab_shell_setting(authorized_keys_file: nil) + allow(gitlab_shell) + .to receive(:gitlab_shell_keys_path) + .and_return(:gitlab_shell_keys_path) + end + + it 'calls #gitlab_shell_fast_execute with add-key command' do + expect(gitlab_shell) + .to receive(:gitlab_shell_fast_execute) + .with([ + :gitlab_shell_keys_path, + 'add-key', + 'key-123', + 'ssh-rsa foobar' + ]) + + gitlab_shell.add_key('key-123', 'ssh-rsa foobar trailing garbage') + end + end + + context 'authorized_keys_file set' do + it 'calls Gitlab::AuthorizedKeys#add_key with id and key' do + expect(Gitlab::AuthorizedKeys).to receive(:new).and_return(gitlab_authorized_keys) + + expect(gitlab_authorized_keys) + .to receive(:add_key) + .with('key-123', 'ssh-rsa foobar') - gitlab_shell.add_key('key-123', 'ssh-rsa foobar trailing garbage') + gitlab_shell.add_key('key-123', 'ssh-rsa foobar') + end end end end describe '#batch_add_keys' do + let(:keys) { [double(shell_id: 'key-123', key: 'ssh-rsa foobar')] } + context 'when authorized_keys_enabled is true' do - it 'instantiates KeyAdder' do - expect_any_instance_of(Gitlab::Shell::KeyAdder).to receive(:add_key).with('key-123', 'ssh-rsa foobar') + context 'authorized_keys_file not set' do + let(:io) { double } + + before do + stub_gitlab_shell_setting(authorized_keys_file: nil) + end + + context 'valid keys' do + before do + allow(gitlab_shell) + .to receive(:gitlab_shell_keys_path) + .and_return(:gitlab_shell_keys_path) + end + + it 'calls gitlab-keys with batch-add-keys command' do + expect(IO) + .to receive(:popen) + .with("gitlab_shell_keys_path batch-add-keys", 'w') + .and_yield(io) + + expect(io).to receive(:puts).with("key-123\tssh-rsa foobar") + expect(gitlab_shell.batch_add_keys(keys)).to be_truthy + end + end + + context 'invalid keys' do + let(:keys) { [double(shell_id: 'key-123', key: "ssh-rsa A\tSDFA\nSGADG")] } + + it 'catches failure and returns false' do + expect(gitlab_shell.batch_add_keys(keys)).to be_falsey + end + end + end - gitlab_shell.batch_add_keys do |adder| - adder.add_key('key-123', 'ssh-rsa foobar') + context 'authorized_keys_file set' do + it 'calls Gitlab::AuthorizedKeys#batch_add_keys with keys to be added' do + expect(Gitlab::AuthorizedKeys).to receive(:new).and_return(gitlab_authorized_keys) + + expect(gitlab_authorized_keys) + .to receive(:batch_add_keys) + .with(keys) + + gitlab_shell.batch_add_keys(keys) end end end @@ -103,11 +208,23 @@ describe Gitlab::Shell do stub_application_setting(authorized_keys_enabled: false) end - it 'does nothing' do - expect_any_instance_of(Gitlab::Shell::KeyAdder).not_to receive(:add_key) + context 'authorized_keys_file not set' do + before do + stub_gitlab_shell_setting(authorized_keys_file: nil) + end + + it 'does nothing' do + expect(IO).not_to receive(:popen) + + gitlab_shell.batch_add_keys(keys) + end + end + + context 'authorized_keys_file set' do + it 'does nothing' do + expect(Gitlab::AuthorizedKeys).not_to receive(:new) - gitlab_shell.batch_add_keys do |adder| - adder.add_key('key-123', 'ssh-rsa foobar') + gitlab_shell.batch_add_keys(keys) end end end @@ -117,11 +234,37 @@ describe Gitlab::Shell do stub_application_setting(authorized_keys_enabled: nil) end - it 'instantiates KeyAdder' do - expect_any_instance_of(Gitlab::Shell::KeyAdder).to receive(:add_key).with('key-123', 'ssh-rsa foobar') + context 'authorized_keys_file not set' do + let(:io) { double } - gitlab_shell.batch_add_keys do |adder| - adder.add_key('key-123', 'ssh-rsa foobar') + before do + stub_gitlab_shell_setting(authorized_keys_file: nil) + allow(gitlab_shell) + .to receive(:gitlab_shell_keys_path) + .and_return(:gitlab_shell_keys_path) + end + + it 'calls gitlab-keys with batch-add-keys command' do + expect(IO) + .to receive(:popen) + .with("gitlab_shell_keys_path batch-add-keys", 'w') + .and_yield(io) + + expect(io).to receive(:puts).with("key-123\tssh-rsa foobar") + + gitlab_shell.batch_add_keys(keys) + end + end + + context 'authorized_keys_file set' do + it 'calls Gitlab::AuthorizedKeys#batch_add_keys with keys to be added' do + expect(Gitlab::AuthorizedKeys).to receive(:new).and_return(gitlab_authorized_keys) + + expect(gitlab_authorized_keys) + .to receive(:batch_add_keys) + .with(keys) + + gitlab_shell.batch_add_keys(keys) end end end @@ -129,13 +272,34 @@ describe Gitlab::Shell do describe '#remove_key' do context 'when authorized_keys_enabled is true' do - it 'removes trailing garbage' do - allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path) - expect(gitlab_shell).to receive(:gitlab_shell_fast_execute).with( - [:gitlab_shell_keys_path, 'rm-key', 'key-123', 'ssh-rsa foobar'] - ) + context 'authorized_keys_file not set' do + before do + stub_gitlab_shell_setting(authorized_keys_file: nil) + allow(gitlab_shell) + .to receive(:gitlab_shell_keys_path) + .and_return(:gitlab_shell_keys_path) + end - gitlab_shell.remove_key('key-123', 'ssh-rsa foobar') + it 'calls #gitlab_shell_fast_execute with rm-key command' do + expect(gitlab_shell) + .to receive(:gitlab_shell_fast_execute) + .with([ + :gitlab_shell_keys_path, + 'rm-key', + 'key-123' + ]) + + gitlab_shell.remove_key('key-123') + end + end + + context 'authorized_keys_file not set' do + it 'calls Gitlab::AuthorizedKeys#rm_key with the key to be removed' do + expect(Gitlab::AuthorizedKeys).to receive(:new).and_return(gitlab_authorized_keys) + expect(gitlab_authorized_keys).to receive(:rm_key).with('key-123') + + gitlab_shell.remove_key('key-123') + end end end @@ -144,10 +308,24 @@ describe Gitlab::Shell do stub_application_setting(authorized_keys_enabled: false) end - it 'does nothing' do - expect(gitlab_shell).not_to receive(:gitlab_shell_fast_execute) + context 'authorized_keys_file not set' do + before do + stub_gitlab_shell_setting(authorized_keys_file: nil) + end + + it 'does nothing' do + expect(gitlab_shell).not_to receive(:gitlab_shell_fast_execute) - gitlab_shell.remove_key('key-123', 'ssh-rsa foobar') + gitlab_shell.remove_key('key-123') + end + end + + context 'authorized_keys_file set' do + it 'does nothing' do + expect(Gitlab::AuthorizedKeys).not_to receive(:new) + + gitlab_shell.remove_key('key-123') + end end end @@ -156,232 +334,256 @@ describe Gitlab::Shell do stub_application_setting(authorized_keys_enabled: nil) end - it 'removes trailing garbage' do - allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path) - expect(gitlab_shell).to receive(:gitlab_shell_fast_execute).with( - [:gitlab_shell_keys_path, 'rm-key', 'key-123', 'ssh-rsa foobar'] - ) + context 'authorized_keys_file not set' do + before do + stub_gitlab_shell_setting(authorized_keys_file: nil) + allow(gitlab_shell) + .to receive(:gitlab_shell_keys_path) + .and_return(:gitlab_shell_keys_path) + end + + it 'calls #gitlab_shell_fast_execute with rm-key command' do + expect(gitlab_shell) + .to receive(:gitlab_shell_fast_execute) + .with([ + :gitlab_shell_keys_path, + 'rm-key', + 'key-123' + ]) - gitlab_shell.remove_key('key-123', 'ssh-rsa foobar') + gitlab_shell.remove_key('key-123') + end end - end - context 'when key content is not given' do - it 'calls rm-key with only one argument' do - allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path) - expect(gitlab_shell).to receive(:gitlab_shell_fast_execute).with( - [:gitlab_shell_keys_path, 'rm-key', 'key-123'] - ) + context 'authorized_keys_file not set' do + it 'calls Gitlab::AuthorizedKeys#rm_key with the key to be removed' do + expect(Gitlab::AuthorizedKeys).to receive(:new).and_return(gitlab_authorized_keys) + expect(gitlab_authorized_keys).to receive(:rm_key).with('key-123') - gitlab_shell.remove_key('key-123') + gitlab_shell.remove_key('key-123') + end end end end describe '#remove_all_keys' do context 'when authorized_keys_enabled is true' do - it 'removes trailing garbage' do - allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path) - expect(gitlab_shell).to receive(:gitlab_shell_fast_execute).with([:gitlab_shell_keys_path, 'clear']) + context 'authorized_keys_file not set' do + before do + stub_gitlab_shell_setting(authorized_keys_file: nil) + allow(gitlab_shell) + .to receive(:gitlab_shell_keys_path) + .and_return(:gitlab_shell_keys_path) + end - gitlab_shell.remove_all_keys - end - end + it 'calls #gitlab_shell_fast_execute with clear command' do + expect(gitlab_shell) + .to receive(:gitlab_shell_fast_execute) + .with([:gitlab_shell_keys_path, 'clear']) - context 'when authorized_keys_enabled is false' do - before do - stub_application_setting(authorized_keys_enabled: false) + gitlab_shell.remove_all_keys + end end - it 'does nothing' do - expect(gitlab_shell).not_to receive(:gitlab_shell_fast_execute) + context 'authorized_keys_file set' do + it 'calls Gitlab::AuthorizedKeys#clear' do + expect(Gitlab::AuthorizedKeys).to receive(:new).and_return(gitlab_authorized_keys) + expect(gitlab_authorized_keys).to receive(:clear) - gitlab_shell.remove_all_keys + gitlab_shell.remove_all_keys + end end end - context 'when authorized_keys_enabled is nil' do + context 'when authorized_keys_enabled is false' do before do - stub_application_setting(authorized_keys_enabled: nil) + stub_application_setting(authorized_keys_enabled: false) end - it 'removes trailing garbage' do - allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path) - expect(gitlab_shell).to receive(:gitlab_shell_fast_execute).with( - [:gitlab_shell_keys_path, 'clear'] - ) + context 'authorized_keys_file not set' do + before do + stub_gitlab_shell_setting(authorized_keys_file: nil) + end - gitlab_shell.remove_all_keys - end - end - end + it 'does nothing' do + expect(gitlab_shell).not_to receive(:gitlab_shell_fast_execute) - describe '#remove_keys_not_found_in_db' do - context 'when keys are in the file that are not in the DB' do - before do - gitlab_shell.remove_all_keys - gitlab_shell.add_key('key-1234', 'ssh-rsa ASDFASDF') - gitlab_shell.add_key('key-9876', 'ssh-rsa ASDFASDF') - @another_key = create(:key) # this one IS in the DB + gitlab_shell.remove_all_keys + end end - it 'removes the keys' do - expect(find_in_authorized_keys_file(1234)).to be_truthy - expect(find_in_authorized_keys_file(9876)).to be_truthy - expect(find_in_authorized_keys_file(@another_key.id)).to be_truthy - gitlab_shell.remove_keys_not_found_in_db - expect(find_in_authorized_keys_file(1234)).to be_falsey - expect(find_in_authorized_keys_file(9876)).to be_falsey - expect(find_in_authorized_keys_file(@another_key.id)).to be_truthy + context 'authorized_keys_file set' do + it 'does nothing' do + expect(Gitlab::AuthorizedKeys).not_to receive(:new) + + gitlab_shell.remove_all_keys + end end end - context 'when keys there are duplicate keys in the file that are not in the DB' do + context 'when authorized_keys_enabled is nil' do before do - gitlab_shell.remove_all_keys - gitlab_shell.add_key('key-1234', 'ssh-rsa ASDFASDF') - gitlab_shell.add_key('key-1234', 'ssh-rsa ASDFASDF') + stub_application_setting(authorized_keys_enabled: nil) end - it 'removes the keys' do - expect(find_in_authorized_keys_file(1234)).to be_truthy - gitlab_shell.remove_keys_not_found_in_db - expect(find_in_authorized_keys_file(1234)).to be_falsey - end + context 'authorized_keys_file not set' do + before do + stub_gitlab_shell_setting(authorized_keys_file: nil) + allow(gitlab_shell) + .to receive(:gitlab_shell_keys_path) + .and_return(:gitlab_shell_keys_path) + end - it 'does not run remove more than once per key (in a batch)' do - expect(gitlab_shell).to receive(:remove_key).with('key-1234').once - gitlab_shell.remove_keys_not_found_in_db - end - end + it 'calls #gitlab_shell_fast_execute with clear command' do + expect(gitlab_shell) + .to receive(:gitlab_shell_fast_execute) + .with([:gitlab_shell_keys_path, 'clear']) - context 'when keys there are duplicate keys in the file that ARE in the DB' do - before do - gitlab_shell.remove_all_keys - @key = create(:key) - gitlab_shell.add_key(@key.shell_id, @key.key) + gitlab_shell.remove_all_keys + end end - it 'does not remove the key' do - gitlab_shell.remove_keys_not_found_in_db - expect(find_in_authorized_keys_file(@key.id)).to be_truthy - end + context 'authorized_keys_file set' do + it 'calls Gitlab::AuthorizedKeys#clear' do + expect(Gitlab::AuthorizedKeys).to receive(:new).and_return(gitlab_authorized_keys) + expect(gitlab_authorized_keys).to receive(:clear) - it 'does not need to run a SELECT query for that batch, on account of that key' do - expect_any_instance_of(ActiveRecord::Relation).not_to receive(:pluck) - gitlab_shell.remove_keys_not_found_in_db + gitlab_shell.remove_all_keys + end end end + end - unless ENV['CI'] # Skip in CI, it takes 1 minute - context 'when the first batch can be skipped, but the next batch has keys that are not in the DB' do + describe '#remove_keys_not_found_in_db' do + context 'when keys are in the file that are not in the DB' do + context 'authorized_keys_file not set' do before do + stub_gitlab_shell_setting(authorized_keys_file: nil) gitlab_shell.remove_all_keys - 100.times { |i| create(:key) } # first batch is all in the DB gitlab_shell.add_key('key-1234', 'ssh-rsa ASDFASDF') + gitlab_shell.add_key('key-9876', 'ssh-rsa ASDFASDF') + @another_key = create(:key) # this one IS in the DB end - it 'removes the keys not in the DB' do - expect(find_in_authorized_keys_file(1234)).to be_truthy + it 'removes the keys' do + expect(gitlab_shell).to receive(:remove_key).with('key-1234') + expect(gitlab_shell).to receive(:remove_key).with('key-9876') + expect(gitlab_shell).not_to receive(:remove_key).with("key-#{@another_key.id}") + gitlab_shell.remove_keys_not_found_in_db - expect(find_in_authorized_keys_file(1234)).to be_falsey end end - end - end - describe '#batch_read_key_ids' do - context 'when there are keys in the authorized_keys file' do - before do - gitlab_shell.remove_all_keys - (1..4).each do |i| - gitlab_shell.add_key("key-#{i}", "ssh-rsa ASDFASDF#{i}") + context 'authorized_keys_file set' do + before do + gitlab_shell.remove_all_keys + gitlab_shell.add_key('key-1234', 'ssh-rsa ASDFASDF') + gitlab_shell.add_key('key-9876', 'ssh-rsa ASDFASDF') + @another_key = create(:key) # this one IS in the DB end - end - it 'iterates over the key IDs in the file, in batches' do - loop_count = 0 - first_batch = [1, 2] - second_batch = [3, 4] + it 'removes the keys' do + expect(gitlab_shell).to receive(:remove_key).with('key-1234') + expect(gitlab_shell).to receive(:remove_key).with('key-9876') + expect(gitlab_shell).not_to receive(:remove_key).with("key-#{@another_key.id}") - gitlab_shell.batch_read_key_ids(batch_size: 2) do |batch| - expected = (loop_count == 0 ? first_batch : second_batch) - expect(batch).to eq(expected) - loop_count += 1 + gitlab_shell.remove_keys_not_found_in_db end end end - end - describe '#list_key_ids' do - context 'when there are keys in the authorized_keys file' do - before do - gitlab_shell.remove_all_keys - (1..4).each do |i| - gitlab_shell.add_key("key-#{i}", "ssh-rsa ASDFASDF#{i}") + context 'when keys there are duplicate keys in the file that are not in the DB' do + context 'authorized_keys_file not set' do + before do + stub_gitlab_shell_setting(authorized_keys_file: nil) + gitlab_shell.remove_all_keys + gitlab_shell.add_key('key-1234', 'ssh-rsa ASDFASDF') + gitlab_shell.add_key('key-1234', 'ssh-rsa ASDFASDF') + end + + it 'removes the keys' do + expect(gitlab_shell).to receive(:remove_key).with('key-1234') + + gitlab_shell.remove_keys_not_found_in_db end end - it 'outputs the key IDs in the file, separated by newlines' do - ids = [] - gitlab_shell.list_key_ids do |io| - io.each do |line| - ids << line - end + context 'authorized_keys_file set' do + before do + gitlab_shell.remove_all_keys + gitlab_shell.add_key('key-1234', 'ssh-rsa ASDFASDF') + gitlab_shell.add_key('key-1234', 'ssh-rsa ASDFASDF') end - expect(ids).to eq(%W{1\n 2\n 3\n 4\n}) - end - end + it 'removes the keys' do + expect(gitlab_shell).to receive(:remove_key).with('key-1234') - context 'when there are no keys in the authorized_keys file' do - before do - gitlab_shell.remove_all_keys + gitlab_shell.remove_keys_not_found_in_db + end end + end - it 'outputs nothing, not even an empty string' do - ids = [] - gitlab_shell.list_key_ids do |io| - io.each do |line| - ids << line - end + context 'when keys there are duplicate keys in the file that ARE in the DB' do + context 'authorized_keys_file not set' do + before do + stub_gitlab_shell_setting(authorized_keys_file: nil) + gitlab_shell.remove_all_keys + @key = create(:key) + gitlab_shell.add_key(@key.shell_id, @key.key) end - expect(ids).to eq([]) + it 'does not remove the key' do + expect(gitlab_shell).not_to receive(:remove_key).with("key-#{@key.id}") + + gitlab_shell.remove_keys_not_found_in_db + end end - end - end - describe Gitlab::Shell::KeyAdder do - describe '#add_key' do - it 'removes trailing garbage' do - io = spy(:io) - adder = described_class.new(io) + context 'authorized_keys_file set' do + before do + gitlab_shell.remove_all_keys + @key = create(:key) + gitlab_shell.add_key(@key.shell_id, @key.key) + end - adder.add_key('key-42', "ssh-rsa foo bar\tbaz") + it 'does not remove the key' do + expect(gitlab_shell).not_to receive(:remove_key).with("key-#{@key.id}") - expect(io).to have_received(:puts).with("key-42\tssh-rsa foo") + gitlab_shell.remove_keys_not_found_in_db + end end + end - it 'handles multiple spaces in the key' do - io = spy(:io) - adder = described_class.new(io) + unless ENV['CI'] # Skip in CI, it takes 1 minute + context 'when the first batch can be skipped, but the next batch has keys that are not in the DB' do + context 'authorized_keys_file not set' do + before do + stub_gitlab_shell_setting(authorized_keys_file: nil) + gitlab_shell.remove_all_keys + 100.times { |i| create(:key) } # first batch is all in the DB + gitlab_shell.add_key('key-1234', 'ssh-rsa ASDFASDF') + end - adder.add_key('key-42', "ssh-rsa foo") + it 'removes the keys not in the DB' do + expect(gitlab_shell).to receive(:remove_key).with('key-1234') - expect(io).to have_received(:puts).with("key-42\tssh-rsa foo") - end + gitlab_shell.remove_keys_not_found_in_db + end + end - it 'raises an exception if the key contains a tab' do - expect do - described_class.new(StringIO.new).add_key('key-42', "ssh-rsa\tfoobar") - end.to raise_error(Gitlab::Shell::Error) - end + context 'authorized_keys_file set' do + before do + gitlab_shell.remove_all_keys + 100.times { |i| create(:key) } # first batch is all in the DB + gitlab_shell.add_key('key-1234', 'ssh-rsa ASDFASDF') + end + + it 'removes the keys not in the DB' do + expect(gitlab_shell).to receive(:remove_key).with('key-1234') - it 'raises an exception if the key contains a newline' do - expect do - described_class.new(StringIO.new).add_key('key-42', "ssh-rsa foobar\nssh-rsa pawned") - end.to raise_error(Gitlab::Shell::Error) + gitlab_shell.remove_keys_not_found_in_db + end + end end end end @@ -566,12 +768,4 @@ describe Gitlab::Shell do end end end - - def find_in_authorized_keys_file(key_id) - gitlab_shell.batch_read_key_ids do |ids| - return true if ids.include?(key_id) # rubocop:disable Cop/AvoidReturnFromBlocks - end - - false - end end diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index cd9e4d48cd1..549cc5ac057 100644 --- a/spec/lib/gitlab/usage_data_spec.rb +++ b/spec/lib/gitlab/usage_data_spec.rb @@ -13,6 +13,8 @@ describe Gitlab::UsageData do create(:service, project: projects[0], type: 'SlackSlashCommandsService', active: true) create(:service, project: projects[1], type: 'SlackService', active: true) create(:service, project: projects[2], type: 'SlackService', active: true) + create(:project_error_tracking_setting, project: projects[0]) + create(:project_error_tracking_setting, project: projects[1], enabled: false) gcp_cluster = create(:cluster, :provided_by_gcp) create(:cluster, :provided_by_user) @@ -117,6 +119,7 @@ describe Gitlab::UsageData do projects_slack_slash_active projects_prometheus_active projects_with_repositories_enabled + projects_with_error_tracking_enabled pages_domains protected_branches releases @@ -146,6 +149,7 @@ describe Gitlab::UsageData do expect(count_data[:projects_slack_notifications_active]).to eq(2) expect(count_data[:projects_slack_slash_active]).to eq(1) expect(count_data[:projects_with_repositories_enabled]).to eq(2) + expect(count_data[:projects_with_error_tracking_enabled]).to eq(1) expect(count_data[:clusters_enabled]).to eq(7) expect(count_data[:project_clusters_enabled]).to eq(6) diff --git a/spec/lib/gitlab/user_extractor_spec.rb b/spec/lib/gitlab/user_extractor_spec.rb index 6e2bb81fbda..b86ec5445b8 100644 --- a/spec/lib/gitlab/user_extractor_spec.rb +++ b/spec/lib/gitlab/user_extractor_spec.rb @@ -38,6 +38,18 @@ describe Gitlab::UserExtractor do expect(extractor.users).to include(user) end + + context 'input as array of strings' do + it 'is treated as one string' do + extractor = described_class.new(text.lines) + + user_1 = create(:user, username: "USER-1") + user_4 = create(:user, username: "USER-4") + user_email = create(:user, email: 'user@gitlab.org') + + expect(extractor.users).to contain_exactly(user_1, user_4, user_email) + end + end end describe '#matches' do diff --git a/spec/lib/gitlab/utils_spec.rb b/spec/lib/gitlab/utils_spec.rb index 8f5029b3565..4645339f439 100644 --- a/spec/lib/gitlab/utils_spec.rb +++ b/spec/lib/gitlab/utils_spec.rb @@ -213,4 +213,22 @@ describe Gitlab::Utils do expect(subject[:variables].first[:key]).to eq('VAR1') end end + + describe '.try_megabytes_to_bytes' do + context 'when the size can be converted to megabytes' do + it 'returns the size in megabytes' do + size = described_class.try_megabytes_to_bytes(1) + + expect(size).to eq(1.megabytes) + end + end + + context 'when the size can not be converted to megabytes' do + it 'returns the input size' do + size = described_class.try_megabytes_to_bytes('foo') + + expect(size).to eq('foo') + end + end + end end diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb index 7213eee5675..d88086b01b1 100644 --- a/spec/lib/gitlab/workhorse_spec.rb +++ b/spec/lib/gitlab/workhorse_spec.rb @@ -250,11 +250,11 @@ describe Gitlab::Workhorse do } end - subject { described_class.git_http_ok(repository, false, user, action) } + subject { described_class.git_http_ok(repository, Gitlab::GlRepository::PROJECT, user, action) } it { expect(subject).to include(params) } - context 'when is_wiki' do + context 'when the repo_type is a wiki' do let(:params) do { GL_ID: "user-#{user.id}", @@ -264,7 +264,7 @@ describe Gitlab::Workhorse do } end - subject { described_class.git_http_ok(repository, true, user, action) } + subject { described_class.git_http_ok(repository, Gitlab::GlRepository::WIKI, user, action) } it { expect(subject).to include(params) } end @@ -304,7 +304,7 @@ describe Gitlab::Workhorse do end context 'show_all_refs enabled' do - subject { described_class.git_http_ok(repository, false, user, action, show_all_refs: true) } + subject { described_class.git_http_ok(repository, Gitlab::GlRepository::PROJECT, user, action, show_all_refs: true) } it { is_expected.to include(ShowAllRefs: true) } end @@ -322,7 +322,7 @@ describe Gitlab::Workhorse do it { expect(subject).to include(gitaly_params) } context 'show_all_refs enabled' do - subject { described_class.git_http_ok(repository, false, user, action, show_all_refs: true) } + subject { described_class.git_http_ok(repository, Gitlab::GlRepository::PROJECT, user, action, show_all_refs: true) } it { is_expected.to include(ShowAllRefs: true) } end diff --git a/spec/lib/gitlab_spec.rb b/spec/lib/gitlab_spec.rb index 8232715d00e..767b5779a79 100644 --- a/spec/lib/gitlab_spec.rb +++ b/spec/lib/gitlab_spec.rb @@ -10,7 +10,7 @@ describe Gitlab do end describe '.revision' do - let(:cmd) { %W[#{described_class.config.git.bin_path} log --pretty=format:%h -n 1] } + let(:cmd) { %W[#{described_class.config.git.bin_path} log --pretty=format:%h --abbrev=11 -n 1] } around do |example| described_class.instance_variable_set(:@_revision, nil) diff --git a/spec/lib/sentry/client_spec.rb b/spec/lib/sentry/client_spec.rb index 88e7e2e5ebb..3333f8307ae 100644 --- a/spec/lib/sentry/client_spec.rb +++ b/spec/lib/sentry/client_spec.rb @@ -65,7 +65,9 @@ describe Sentry::Client do let(:issue_status) { 'unresolved' } let(:limit) { 20 } - let!(:sentry_api_request) { stub_sentry_request(sentry_url + '/issues/?limit=20&query=is:unresolved', body: issues_sample_response) } + let(:sentry_api_response) { issues_sample_response } + + let!(:sentry_api_request) { stub_sentry_request(sentry_url + '/issues/?limit=20&query=is:unresolved', body: sentry_api_response) } subject { client.list_issues(issue_status: issue_status, limit: limit) } @@ -74,6 +76,14 @@ describe Sentry::Client do it_behaves_like 'has correct return type', Gitlab::ErrorTracking::Error it_behaves_like 'has correct length', 1 + shared_examples 'has correct external_url' do + context 'external_url' do + it 'is constructed correctly' do + expect(subject[0].external_url).to eq('https://sentrytest.gitlab.com/sentry-org/sentry-project/issues/11') + end + end + end + context 'error object created from sentry response' do using RSpec::Parameterized::TableSyntax @@ -96,14 +106,10 @@ describe Sentry::Client do end with_them do - it { expect(subject[0].public_send(error_object)).to eq(issues_sample_response[0].dig(*sentry_response)) } + it { expect(subject[0].public_send(error_object)).to eq(sentry_api_response[0].dig(*sentry_response)) } end - context 'external_url' do - it 'is constructed correctly' do - expect(subject[0].external_url).to eq('https://sentrytest.gitlab.com/sentry-org/sentry-project/issues/11') - end - end + it_behaves_like 'has correct external_url' end context 'redirects' do @@ -135,12 +141,42 @@ describe Sentry::Client do expect(valid_req_stub).to have_been_requested end end + + context 'Older sentry versions where keys are not present' do + let(:sentry_api_response) do + issues_sample_response[0...1].map do |issue| + issue[:project].delete(:id) + issue + end + end + + it_behaves_like 'calls sentry api' + + it_behaves_like 'has correct return type', Gitlab::ErrorTracking::Error + it_behaves_like 'has correct length', 1 + + it_behaves_like 'has correct external_url' + end + + context 'essential keys missing in API response' do + let(:sentry_api_response) do + issues_sample_response[0...1].map do |issue| + issue.except(:id) + end + end + + it 'raises exception' do + expect { subject }.to raise_error(Sentry::Client::MissingKeysError, 'Sentry API response is missing keys. key not found: "id"') + end + end end describe '#list_projects' do let(:sentry_list_projects_url) { 'https://sentrytest.gitlab.com/api/0/projects/' } - let!(:sentry_api_request) { stub_sentry_request(sentry_list_projects_url, body: projects_sample_response) } + let(:sentry_api_response) { projects_sample_response } + + let!(:sentry_api_request) { stub_sentry_request(sentry_list_projects_url, body: sentry_api_response) } subject { client.list_projects } @@ -149,14 +185,31 @@ describe Sentry::Client do it_behaves_like 'has correct return type', Gitlab::ErrorTracking::Project it_behaves_like 'has correct length', 2 - context 'keys missing in API response' do - it 'raises exception' do - projects_sample_response[0].delete(:slug) + context 'essential keys missing in API response' do + let(:sentry_api_response) do + projects_sample_response[0...1].map do |project| + project.except(:slug) + end + end - stub_sentry_request(sentry_list_projects_url, body: projects_sample_response) + it 'raises exception' do + expect { subject }.to raise_error(Sentry::Client::MissingKeysError, 'Sentry API response is missing keys. key not found: "slug"') + end + end - expect { subject }.to raise_error(Sentry::Client::SentryError, 'Sentry API response is missing keys. key not found: "slug"') + context 'optional keys missing in sentry response' do + let(:sentry_api_response) do + projects_sample_response[0...1].map do |project| + project[:organization].delete(:id) + project.delete(:id) + project.except(:status) + end end + + it_behaves_like 'calls sentry api' + + it_behaves_like 'has correct return type', Gitlab::ErrorTracking::Project + it_behaves_like 'has correct length', 1 end context 'error object created from sentry response' do @@ -173,7 +226,11 @@ describe Sentry::Client do end with_them do - it { expect(subject[0].public_send(sentry_project_object)).to eq(projects_sample_response[0].dig(*sentry_response)) } + it do + expect(subject[0].public_send(sentry_project_object)).to( + eq(sentry_api_response[0].dig(*sentry_response)) + ) + end end end |