diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-03-16 15:09:12 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-03-16 15:09:12 +0300 |
commit | cbfe03ae04a52d9825ff7cbeccdfe5d313adf6a2 (patch) | |
tree | e4879b35d019d3bbba1689f3ac4c48b81bf7b451 /spec | |
parent | 3fd97b4bba24ca412112aad025a38a32c7a6cf8c (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
20 files changed, 432 insertions, 236 deletions
diff --git a/spec/controllers/projects/tags/releases_controller_spec.rb b/spec/controllers/projects/tags/releases_controller_spec.rb index da87756d782..cb12e074732 100644 --- a/spec/controllers/projects/tags/releases_controller_spec.rb +++ b/spec/controllers/projects/tags/releases_controller_spec.rb @@ -67,13 +67,13 @@ describe Projects::Tags::ReleasesController do expect(response).to have_gitlab_http_status(:found) end - it 'deletes release when description is empty' do - initial_releases_count = project.releases.count + it 'does not delete release when description is empty' do + expect do + update_release(tag, "") + end.not_to change { project.releases.count } - response = update_release(release.tag, "") + expect(release.reload.description).to eq("") - expect(initial_releases_count).to eq(1) - expect(project.releases.count).to eq(0) expect(response).to have_gitlab_http_status(:found) end diff --git a/spec/features/markdown/mermaid_spec.rb b/spec/features/markdown/mermaid_spec.rb index 1cd5760c30e..4bf7edf98ca 100644 --- a/spec/features/markdown/mermaid_spec.rb +++ b/spec/features/markdown/mermaid_spec.rb @@ -38,7 +38,9 @@ describe 'Mermaid rendering', :js do visit project_issue_path(project, issue) - expected = '<text><tspan xml:space="preserve" dy="1em" x="1">Line 1</tspan><tspan xml:space="preserve" dy="1em" x="1">Line 2</tspan></text>' + wait_for_requests + + expected = '<text style=""><tspan xml:space="preserve" dy="1em" x="1">Line 1</tspan><tspan xml:space="preserve" dy="1em" x="1">Line 2</tspan></text>' expect(page.html.scan(expected).count).to be(4) end @@ -121,4 +123,40 @@ describe 'Mermaid rendering', :js do expect(svg[:width].to_i).to eq(100) expect(svg[:height].to_i).to eq(0) end + + it 'display button when diagram exceeds length', :js do + graph_edges = "A-->B;B-->A;" * 420 + + description = <<~MERMAID + ```mermaid + graph LR + #{graph_edges} + ``` + MERMAID + + project = create(:project, :public) + issue = create(:issue, project: project, description: description) + + visit project_issue_path(project, issue) + + page.within('.description') do + expect(page).not_to have_selector('svg') + + expect(page).to have_selector('pre.mermaid') + + expect(page).to have_selector('.lazy-alert-shown') + + expect(page).to have_selector('.js-lazy-render-mermaid-container') + end + + wait_for_requests + + find('.js-lazy-render-mermaid').click + + page.within('.description') do + expect(page).to have_selector('svg') + + expect(page).not_to have_selector('.js-lazy-render-mermaid-container') + end + end end diff --git a/spec/features/profiles/user_edit_profile_spec.rb b/spec/features/profiles/user_edit_profile_spec.rb index 9839b3d6c80..171dfb353f0 100644 --- a/spec/features/profiles/user_edit_profile_spec.rb +++ b/spec/features/profiles/user_edit_profile_spec.rb @@ -15,6 +15,11 @@ describe 'User edit profile' do wait_for_requests if respond_to?(:wait_for_requests) end + def visit_user + visit user_path(user) + wait_for_requests + end + it 'changes user profile' do fill_in 'user_skype', with: 'testskype' fill_in 'user_linkedin', with: 'testlinkedin' @@ -22,8 +27,8 @@ describe 'User edit profile' do fill_in 'user_website_url', with: 'testurl' fill_in 'user_location', with: 'Ukraine' fill_in 'user_bio', with: 'I <3 GitLab' + fill_in 'user_job_title', with: 'Frontend Engineer' fill_in 'user_organization', with: 'GitLab' - select 'Data Analyst', from: 'user_role' submit_settings expect(user.reload).to have_attributes( @@ -32,8 +37,8 @@ describe 'User edit profile' do twitter: 'testtwitter', website_url: 'testurl', bio: 'I <3 GitLab', - organization: 'GitLab', - role: 'data_analyst' + job_title: 'Frontend Engineer', + organization: 'GitLab' ) expect(find('#user_location').value).to eq 'Ukraine' @@ -94,11 +99,6 @@ describe 'User edit profile' do end context 'user status', :js do - def visit_user - visit user_path(user) - wait_for_requests - end - def select_emoji(emoji_name, is_modal = false) emoji_menu_class = is_modal ? '.js-modal-status-emoji-menu' : '.js-status-emoji-menu' toggle_button = find('.js-toggle-emoji-menu') @@ -381,4 +381,40 @@ describe 'User edit profile' do end end end + + context 'work information', :js do + context 'when job title and organziation are entered' do + it "shows job title and organzation on user's profile" do + fill_in 'user_job_title', with: 'Frontend Engineer' + fill_in 'user_organization', with: 'GitLab - work info test' + submit_settings + + visit_user + + expect(page).to have_content('Frontend Engineer at GitLab - work info test') + end + end + + context 'when only job title is entered' do + it "shows only job title on user's profile" do + fill_in 'user_job_title', with: 'Frontend Engineer - work info test' + submit_settings + + visit_user + + expect(page).to have_content('Frontend Engineer - work info test') + end + end + + context 'when only organization is entered' do + it "shows only organization on user's profile" do + fill_in 'user_organization', with: 'GitLab - work info test' + submit_settings + + visit_user + + expect(page).to have_content('GitLab - work info test') + end + end + end end diff --git a/spec/features/users/show_spec.rb b/spec/features/users/show_spec.rb index 8c2b555305a..a45389a7ed5 100644 --- a/spec/features/users/show_spec.rb +++ b/spec/features/users/show_spec.rb @@ -26,6 +26,34 @@ describe 'User page' do expect(page).not_to have_content("This user has a private profile") end + + context 'work information' do + subject { visit(user_path(user)) } + + it 'shows job title and organization details' do + user.update(organization: 'GitLab - work info test', job_title: 'Frontend Engineer') + + subject + + expect(page).to have_content('Frontend Engineer at GitLab - work info test') + end + + it 'shows job title' do + user.update(organization: nil, job_title: 'Frontend Engineer - work info test') + + subject + + expect(page).to have_content('Frontend Engineer - work info test') + end + + it 'shows organization details' do + user.update(organization: 'GitLab - work info test', job_title: '') + + subject + + expect(page).to have_content('GitLab - work info test') + end + end end context 'with private profile' do diff --git a/spec/frontend/vue_shared/components/user_popover/user_popover_spec.js b/spec/frontend/vue_shared/components/user_popover/user_popover_spec.js index a8bbc80d2df..a2e2d2447d5 100644 --- a/spec/frontend/vue_shared/components/user_popover/user_popover_spec.js +++ b/spec/frontend/vue_shared/components/user_popover/user_popover_spec.js @@ -1,4 +1,4 @@ -import { GlSkeletonLoading } from '@gitlab/ui'; +import { GlSkeletonLoading, GlSprintf } from '@gitlab/ui'; import { shallowMount } from '@vue/test-utils'; import UserPopover from '~/vue_shared/components/user_popover/user_popover.vue'; import Icon from '~/vue_shared/components/icon.vue'; @@ -11,6 +11,7 @@ const DEFAULT_PROPS = { location: 'Vienna', bio: null, organization: null, + jobTitle: null, status: null, }, }; @@ -39,6 +40,9 @@ describe('User Popover Component', () => { target: findTarget(), ...props, }, + stubs: { + 'gl-sprintf': GlSprintf, + }, ...options, }); }; @@ -56,6 +60,7 @@ describe('User Popover Component', () => { location: null, bio: null, organization: null, + jobTitle: null, status: null, }, }, @@ -85,51 +90,125 @@ describe('User Popover Component', () => { }); describe('job data', () => { - it('should show only bio if no organization is available', () => { - const user = { ...DEFAULT_PROPS.user, bio: 'Engineer' }; + const findWorkInformation = () => wrapper.find({ ref: 'workInformation' }); + const findBio = () => wrapper.find({ ref: 'bio' }); + + it('should show only bio if organization and job title are not available', () => { + const user = { ...DEFAULT_PROPS.user, bio: 'My super interesting bio' }; createWrapper({ user }); - expect(wrapper.text()).toContain('Engineer'); + expect(findBio().text()).toBe('My super interesting bio'); + expect(findWorkInformation().exists()).toBe(false); }); - it('should show only organization if no bio is available', () => { + it('should show only organization if job title is not available', () => { const user = { ...DEFAULT_PROPS.user, organization: 'GitLab' }; createWrapper({ user }); - expect(wrapper.text()).toContain('GitLab'); + expect(findWorkInformation().text()).toBe('GitLab'); + }); + + it('should show only job title if organization is not available', () => { + const user = { ...DEFAULT_PROPS.user, jobTitle: 'Frontend Engineer' }; + + createWrapper({ user }); + + expect(findWorkInformation().text()).toBe('Frontend Engineer'); + }); + + it('should show organization and job title if they are both available', () => { + const user = { + ...DEFAULT_PROPS.user, + organization: 'GitLab', + jobTitle: 'Frontend Engineer', + }; + + createWrapper({ user }); + + expect(findWorkInformation().text()).toBe('Frontend Engineer at GitLab'); + }); + + it('should display bio and job info in separate lines', () => { + const user = { + ...DEFAULT_PROPS.user, + bio: 'My super interesting bio', + organization: 'GitLab', + }; + + createWrapper({ user }); + + expect(findBio().text()).toBe('My super interesting bio'); + expect(findWorkInformation().text()).toBe('GitLab'); }); - it('should display bio and organization in separate lines', () => { - const user = { ...DEFAULT_PROPS.user, bio: 'Engineer', organization: 'GitLab' }; + it('should not encode special characters in bio', () => { + const user = { + ...DEFAULT_PROPS.user, + bio: 'I like <html> & CSS', + }; createWrapper({ user }); - expect(wrapper.find('.js-bio').text()).toContain('Engineer'); - expect(wrapper.find('.js-organization').text()).toContain('GitLab'); + expect(findBio().text()).toBe('I like <html> & CSS'); }); - it('should not encode special characters in bio and organization', () => { + it('should not encode special characters in organization', () => { const user = { ...DEFAULT_PROPS.user, - bio: 'Manager & Team Lead', organization: 'Me & my <funky> Company', }; createWrapper({ user }); - expect(wrapper.find('.js-bio').text()).toContain('Manager & Team Lead'); - expect(wrapper.find('.js-organization').text()).toContain('Me & my <funky> Company'); + expect(findWorkInformation().text()).toBe('Me & my <funky> Company'); + }); + + it('should not encode special characters in job title', () => { + const user = { + ...DEFAULT_PROPS.user, + jobTitle: 'Manager & Team Lead', + }; + + createWrapper({ user }); + + expect(findWorkInformation().text()).toBe('Manager & Team Lead'); + }); + + it('should not encode special characters when both job title and organization are set', () => { + const user = { + ...DEFAULT_PROPS.user, + jobTitle: 'Manager & Team Lead', + organization: 'Me & my <funky> Company', + }; + + createWrapper({ user }); + + expect(findWorkInformation().text()).toBe('Manager & Team Lead at Me & my <funky> Company'); }); it('shows icon for bio', () => { + const user = { + ...DEFAULT_PROPS.user, + bio: 'My super interesting bio', + }; + + createWrapper({ user }); + expect(wrapper.findAll(Icon).filter(icon => icon.props('name') === 'profile').length).toEqual( 1, ); }); it('shows icon for organization', () => { + const user = { + ...DEFAULT_PROPS.user, + organization: 'GitLab', + }; + + createWrapper({ user }); + expect(wrapper.findAll(Icon).filter(icon => icon.props('name') === 'work').length).toEqual(1); }); }); diff --git a/spec/helpers/users_helper_spec.rb b/spec/helpers/users_helper_spec.rb index 8479f8509f5..893d5cde24a 100644 --- a/spec/helpers/users_helper_spec.rb +++ b/spec/helpers/users_helper_spec.rb @@ -178,4 +178,42 @@ describe UsersHelper do end end end + + describe '#work_information' do + subject { helper.work_information(user) } + + context 'when both job_title and organization are present' do + let(:user) { build(:user, organization: 'GitLab', job_title: 'Frontend Engineer') } + + it 'returns job title concatenated with organization' do + is_expected.to eq('Frontend Engineer at GitLab') + end + end + + context 'when only organization is present' do + let(:user) { build(:user, organization: 'GitLab') } + + it "returns organization" do + is_expected.to eq('GitLab') + end + end + + context 'when only job_title is present' do + let(:user) { build(:user, job_title: 'Frontend Engineer') } + + it 'returns job title' do + is_expected.to eq('Frontend Engineer') + end + end + + context 'when neither organization nor job_title are present' do + it { is_expected.to be_nil } + end + + context 'when user parameter is nil' do + let(:user) { nil } + + it { is_expected.to be_nil } + end + end end diff --git a/spec/lib/backup/repository_spec.rb b/spec/lib/backup/repository_spec.rb index 2ac1b0d2583..e0afa256581 100644 --- a/spec/lib/backup/repository_spec.rb +++ b/spec/lib/backup/repository_spec.rb @@ -50,9 +50,9 @@ describe Backup::Repository do describe 'command failure' do before do - allow_next_instance_of(Gitlab::Shell) do |instance| - allow(instance).to receive(:create_repository).and_return(false) - end + # Allow us to set expectations on the project directly + expect(Project).to receive(:find_each).and_yield(project) + expect(project.repository).to receive(:create_repository) { raise 'Fail in tests' } end context 'hashed storage' do diff --git a/spec/lib/gitlab/legacy_github_import/importer_spec.rb b/spec/lib/gitlab/legacy_github_import/importer_spec.rb index 7fef763f64d..23df970957a 100644 --- a/spec/lib/gitlab/legacy_github_import/importer_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/importer_spec.rb @@ -173,10 +173,6 @@ describe Gitlab::LegacyGithubImport::Importer do ] } - unless project.gitea_import? - error[:errors] << { type: :release, url: "#{api_root}/repos/octocat/Hello-World/releases/2", errors: "Validation failed: Description can't be blank" } - end - described_class.new(project).execute expect(project.import_state.last_error).to eq error.to_json diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index 280d4fba5a2..3ea9f71d3a6 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -7,18 +7,17 @@ describe Gitlab::Shell do let_it_be(:project) { create(:project, :repository) } let(:repository) { project.repository } let(:gitlab_shell) { described_class.new } - let(:popen_vars) { { 'GIT_TERMINAL_PROMPT' => ENV['GIT_TERMINAL_PROMPT'] } } - let(:timeout) { Gitlab.config.gitlab_shell.git_timeout } - before do - allow(Project).to receive(:find).and_return(project) - end - - it { is_expected.to respond_to :create_repository } it { is_expected.to respond_to :remove_repository } it { is_expected.to respond_to :fork_repository } - it { expect(gitlab_shell.url_to_repo('diaspora')).to eq(Gitlab.config.gitlab_shell.ssh_path_prefix + "diaspora.git") } + describe '.url_to_repo' do + let(:full_path) { 'diaspora/disaspora-rails' } + + subject { described_class.url_to_repo(full_path) } + + it { is_expected.to eq(Gitlab.config.gitlab_shell.ssh_path_prefix + full_path + '.git') } + end describe 'memoized secret_token' do let(:secret_file) { 'tmp/tests/.secret_shell_test' } @@ -49,37 +48,12 @@ describe Gitlab::Shell do describe 'projects commands' do let(:gitlab_shell_path) { File.expand_path('tmp/tests/gitlab-shell') } let(:projects_path) { File.join(gitlab_shell_path, 'bin/gitlab-projects') } - let(:gitlab_shell_hooks_path) { File.join(gitlab_shell_path, 'hooks') } before do allow(Gitlab.config.gitlab_shell).to receive(:path).and_return(gitlab_shell_path) allow(Gitlab.config.gitlab_shell).to receive(:git_timeout).and_return(800) end - describe '#create_repository' do - let(:repository_storage) { 'default' } - let(:repository_storage_path) do - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - Gitlab.config.repositories.storages[repository_storage].legacy_disk_path - end - end - let(:repo_name) { 'project/path' } - let(:created_path) { File.join(repository_storage_path, repo_name + '.git') } - - after do - FileUtils.rm_rf(created_path) - end - - it 'returns false when the command fails' do - FileUtils.mkdir_p(File.dirname(created_path)) - # This file will block the creation of the repo's .git directory. That - # should cause #create_repository to fail. - FileUtils.touch(created_path) - - expect(gitlab_shell.create_repository(repository_storage, repo_name, repo_name)).to be_falsy - end - end - describe '#remove_repository' do let!(:project) { create(:project, :repository, :legacy_storage) } let(:disk_path) { "#{project.disk_path}.git" } diff --git a/spec/models/concerns/bulk_insert_safe_spec.rb b/spec/models/concerns/bulk_insert_safe_spec.rb index 0cc355ea694..a8e56cb8bdd 100644 --- a/spec/models/concerns/bulk_insert_safe_spec.rb +++ b/spec/models/concerns/bulk_insert_safe_spec.rb @@ -22,6 +22,18 @@ describe BulkInsertSafe do algorithm: 'aes-256-gcm', key: Settings.attr_encrypted_db_key_base_32, insecure_mode: false + + default_value_for :enum_value, 'case_1' + default_value_for :secret_value, 'my-secret' + default_value_for :sha_value, '2fd4e1c67a2d28fced849ee1bb76e7391b93eb12' + + def self.valid_list(count) + Array.new(count) { |n| new(name: "item-#{n}") } + end + + def self.invalid_list(count) + Array.new(count) { new } + end end module InheritedUnsafeMethods @@ -48,6 +60,8 @@ describe BulkInsertSafe do t.text :encrypted_secret_value, null: false t.string :encrypted_secret_value_iv, null: false t.binary :sha_value, null: false, limit: 20 + + t.index :name, unique: true end end @@ -60,87 +74,95 @@ describe BulkInsertSafe do end end - def build_valid_items_for_bulk_insertion - Array.new(10) do |n| - BulkInsertItem.new( - name: "item-#{n}", - enum_value: 'case_1', - secret_value: 'my-secret', - sha_value: '2fd4e1c67a2d28fced849ee1bb76e7391b93eb12' - ) + describe BulkInsertItem do + it_behaves_like 'a BulkInsertSafe model', described_class do + let(:valid_items_for_bulk_insertion) { described_class.valid_list(10) } + let(:invalid_items_for_bulk_insertion) { described_class.invalid_list(10) } end - end - def build_invalid_items_for_bulk_insertion - Array.new(10) do - BulkInsertItem.new( - name: nil, # requires `name` to be set - enum_value: 'case_1', - secret_value: 'my-secret', - sha_value: '2fd4e1c67a2d28fced849ee1bb76e7391b93eb12' - ) + context 'when inheriting class methods' do + it 'raises an error when method is not bulk-insert safe' do + expect { described_class.include(InheritedUnsafeMethods) } + .to raise_error(described_class::MethodNotAllowedError) + end + + it 'does not raise an error when method is bulk-insert safe' do + expect { described_class.include(InheritedSafeMethods) }.not_to raise_error + end end - end - it_behaves_like 'a BulkInsertSafe model', BulkInsertItem do - let(:valid_items_for_bulk_insertion) { build_valid_items_for_bulk_insertion } - let(:invalid_items_for_bulk_insertion) { build_invalid_items_for_bulk_insertion } - end + context 'primary keys' do + it 'raises error if primary keys are set prior to insertion' do + item = described_class.new(name: 'valid', id: 10) - context 'when inheriting class methods' do - it 'raises an error when method is not bulk-insert safe' do - expect { BulkInsertItem.include(InheritedUnsafeMethods) }.to( - raise_error(subject::MethodNotAllowedError)) + expect { described_class.bulk_insert!([item]) } + .to raise_error(described_class::PrimaryKeySetError) + end end - it 'does not raise an error when method is bulk-insert safe' do - expect { BulkInsertItem.include(InheritedSafeMethods) }.not_to raise_error - end - end + describe '.bulk_insert!' do + it 'inserts items in the given number of batches' do + items = described_class.valid_list(10) + + expect(ActiveRecord::InsertAll).to receive(:new).twice.and_call_original - context 'primary keys' do - it 'raises error if primary keys are set prior to insertion' do - items = build_valid_items_for_bulk_insertion - items.each_with_index do |item, n| - item.id = n + described_class.bulk_insert!(items, batch_size: 5) end - expect { BulkInsertItem.bulk_insert!(items) }.to raise_error(subject::PrimaryKeySetError) - end - end + it 'items can be properly fetched from database' do + items = described_class.valid_list(10) - describe '.bulk_insert!' do - it 'inserts items in the given number of batches' do - items = build_valid_items_for_bulk_insertion - expect(items.size).to eq(10) - expect(BulkInsertItem).to receive(:insert_all!).twice + described_class.bulk_insert!(items) - BulkInsertItem.bulk_insert!(items, batch_size: 5) - end + attribute_names = described_class.attribute_names - %w[id] + expect(described_class.last(items.size).pluck(*attribute_names)).to eq( + items.pluck(*attribute_names)) + end - it 'items can be properly fetched from database' do - items = build_valid_items_for_bulk_insertion + it 'rolls back the transaction when any item is invalid' do + # second batch is bad + all_items = described_class.valid_list(10) + + described_class.invalid_list(10) - BulkInsertItem.bulk_insert!(items) + expect do + described_class.bulk_insert!(all_items, batch_size: 2) rescue nil + end.not_to change { described_class.count } + end - attribute_names = BulkInsertItem.attribute_names - %w[id] - expect(BulkInsertItem.last(items.size).pluck(*attribute_names)).to eq( - items.pluck(*attribute_names)) + it 'does nothing and returns true when items are empty' do + expect(described_class.bulk_insert!([])).to be(true) + expect(described_class.count).to eq(0) + end end - it 'rolls back the transaction when any item is invalid' do - # second batch is bad - all_items = build_valid_items_for_bulk_insertion + build_invalid_items_for_bulk_insertion - batch_size = all_items.size / 2 + context 'when duplicate items are to be inserted' do + let!(:existing_object) { described_class.create!(name: 'duplicate', secret_value: 'old value') } + let(:new_object) { described_class.new(name: 'duplicate', secret_value: 'new value') } + + describe '.bulk_insert!' do + context 'when skip_duplicates is set to false' do + it 'raises an exception' do + expect { described_class.bulk_insert!([new_object], skip_duplicates: false) } + .to raise_error(ActiveRecord::RecordNotUnique) + end + end + + context 'when skip_duplicates is set to true' do + it 'does not update existing object' do + described_class.bulk_insert!([new_object], skip_duplicates: true) + + expect(existing_object.reload.secret_value).to eq('old value') + end + end + end - expect do - BulkInsertItem.bulk_insert!(all_items, batch_size: batch_size) rescue nil - end.not_to change { BulkInsertItem.count } - end + describe '.bulk_upsert!' do + it 'updates existing object' do + described_class.bulk_upsert!([new_object], unique_by: %w[name]) - it 'does nothing and returns true when items are empty' do - expect(BulkInsertItem.bulk_insert!([])).to be(true) - expect(BulkInsertItem.count).to eq(0) + expect(existing_object.reload.secret_value).to eq('new value') + end + end end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 6d9b46c9941..2c6cfce7247 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1921,30 +1921,15 @@ describe Project do describe '#create_repository' do let(:project) { create(:project, :repository) } - let(:shell) { Gitlab::Shell.new } - - before do - allow(project).to receive(:gitlab_shell).and_return(shell) - end context 'using a regular repository' do it 'creates the repository' do - expect(shell).to receive(:create_repository) - .with(project.repository_storage, project.disk_path, project.full_path) - .and_return(true) - - expect(project.repository).to receive(:after_create) - + expect(project.repository).to receive(:create_repository) expect(project.create_repository).to eq(true) end it 'adds an error if the repository could not be created' do - expect(shell).to receive(:create_repository) - .with(project.repository_storage, project.disk_path, project.full_path) - .and_return(false) - - expect(project.repository).not_to receive(:after_create) - + expect(project.repository).to receive(:create_repository) { raise 'Fail in test' } expect(project.create_repository).to eq(false) expect(project.errors).not_to be_empty end @@ -1953,7 +1938,7 @@ describe Project do context 'using a forked repository' do it 'does nothing' do expect(project).to receive(:forked?).and_return(true) - expect(shell).not_to receive(:create_repository) + expect(project.repository).not_to receive(:create_repository) project.create_repository end @@ -1962,28 +1947,16 @@ describe Project do describe '#ensure_repository' do let(:project) { create(:project, :repository) } - let(:shell) { Gitlab::Shell.new } - - before do - allow(project).to receive(:gitlab_shell).and_return(shell) - end it 'creates the repository if it not exist' do - allow(project).to receive(:repository_exists?) - .and_return(false) - - allow(shell).to receive(:create_repository) - .with(project.repository_storage, project.disk_path, project.full_path) - .and_return(true) - + allow(project).to receive(:repository_exists?).and_return(false) expect(project).to receive(:create_repository).with(force: true) project.ensure_repository end it 'does not create the repository if it exists' do - allow(project).to receive(:repository_exists?) - .and_return(true) + allow(project).to receive(:repository_exists?).and_return(true) expect(project).not_to receive(:create_repository) @@ -1992,13 +1965,8 @@ describe Project do it 'creates the repository if it is a fork' do expect(project).to receive(:forked?).and_return(true) - - allow(project).to receive(:repository_exists?) - .and_return(false) - - expect(shell).to receive(:create_repository) - .with(project.repository_storage, project.disk_path, project.full_path) - .and_return(true) + expect(project).to receive(:repository_exists?).and_return(false) + expect(project.repository).to receive(:create_repository) { true } project.ensure_repository end diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb index af23f121bdc..2d660d1deab 100644 --- a/spec/models/project_wiki_spec.rb +++ b/spec/models/project_wiki_spec.rb @@ -34,7 +34,7 @@ describe ProjectWiki do describe "#url_to_repo" do it "returns the correct ssh url to the repo" do - expect(subject.url_to_repo).to eq(gitlab_shell.url_to_repo(subject.full_path)) + expect(subject.url_to_repo).to eq(Gitlab::Shell.url_to_repo(subject.full_path)) end end @@ -97,9 +97,7 @@ describe ProjectWiki do it "raises CouldNotCreateWikiError if it can't create the wiki repository" do # Create a fresh project which will not have a wiki project_wiki = described_class.new(create(:project), user) - gitlab_shell = double(:gitlab_shell) - allow(gitlab_shell).to receive(:create_wiki_repository) - allow(project_wiki).to receive(:gitlab_shell).and_return(gitlab_shell) + expect(project_wiki.repository).to receive(:create_if_not_exists) { false } expect { project_wiki.send(:wiki) }.to raise_exception(ProjectWiki::CouldNotCreateWikiError) end @@ -416,26 +414,12 @@ describe ProjectWiki do end end - describe '#create_repo!' do - let(:project) { create(:project) } - - it 'creates a repository' do - expect(raw_repository.exists?).to eq(false) - expect(subject.repository).to receive(:after_create) - - subject.send(:create_repo!, raw_repository) - - expect(raw_repository.exists?).to eq(true) - end - end - describe '#ensure_repository' do let(:project) { create(:project) } it 'creates the repository if it not exist' do expect(raw_repository.exists?).to eq(false) - expect(subject).to receive(:create_repo!).and_call_original subject.ensure_repository expect(raw_repository.exists?).to eq(true) diff --git a/spec/models/release_spec.rb b/spec/models/release_spec.rb index 85e398c7d5f..3884b8138be 100644 --- a/spec/models/release_spec.rb +++ b/spec/models/release_spec.rb @@ -20,7 +20,6 @@ RSpec.describe Release do describe 'validation' do it { is_expected.to validate_presence_of(:project) } - it { is_expected.to validate_presence_of(:description) } it { is_expected.to validate_presence_of(:tag) } context 'when a release exists in the database without a name' do diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb index be592ac6a5c..c3e52975d6d 100644 --- a/spec/requests/api/internal/base_spec.rb +++ b/spec/requests/api/internal/base_spec.rb @@ -262,6 +262,8 @@ describe API::Internal::Base do describe "POST /internal/allowed", :clean_gitlab_redis_shared_state do context "access granted" do + let(:env) { {} } + around do |example| Timecop.freeze { example.run } end @@ -270,30 +272,32 @@ describe API::Internal::Base do project.add_developer(user) end - context 'with env passed as a JSON' do - let(:gl_repository) { Gitlab::GlRepository::WIKI.identifier_for_container(project) } - - it 'sets env in RequestStore' do - obj_dir_relative = './objects' - alt_obj_dirs_relative = ['./alt-objects-1', './alt-objects-2'] + shared_examples 'sets hook env' do + context 'with env passed as a JSON' do + let(:obj_dir_relative) { './objects' } + let(:alt_obj_dirs_relative) { ['./alt-objects-1', './alt-objects-2'] } + let(:env) do + { + GIT_OBJECT_DIRECTORY_RELATIVE: obj_dir_relative, + GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE: alt_obj_dirs_relative + } + end - expect(Gitlab::Git::HookEnv).to receive(:set).with(gl_repository, { - 'GIT_OBJECT_DIRECTORY_RELATIVE' => obj_dir_relative, - 'GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE' => alt_obj_dirs_relative - }) + it 'sets env in RequestStore' do + expect(Gitlab::Git::HookEnv).to receive(:set).with(gl_repository, env.stringify_keys) - push(key, project.wiki, env: { - GIT_OBJECT_DIRECTORY_RELATIVE: obj_dir_relative, - GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE: alt_obj_dirs_relative - }.to_json) + subject - expect(response).to have_gitlab_http_status(:ok) + expect(response).to have_gitlab_http_status(:ok) + end end end context "git push with project.wiki" do + subject { push(key, project.wiki, env: env.to_json) } + it 'responds with success' do - push(key, project.wiki) + subject expect(response).to have_gitlab_http_status(:ok) expect(json_response["status"]).to be_truthy @@ -301,6 +305,10 @@ describe API::Internal::Base do expect(json_response["gl_repository"]).to eq("wiki-#{project.id}") expect(user.reload.last_activity_on).to be_nil end + + it_behaves_like 'sets hook env' do + let(:gl_repository) { Gitlab::GlRepository::WIKI.identifier_for_container(project) } + end end context "git pull with project.wiki" do @@ -328,8 +336,10 @@ describe API::Internal::Base do end context 'git push with personal snippet' do + subject { push(key, personal_snippet, env: env.to_json) } + it 'responds with success' do - push(key, personal_snippet) + subject expect(response).to have_gitlab_http_status(:ok) expect(json_response["status"]).to be_truthy @@ -338,8 +348,9 @@ describe API::Internal::Base do expect(user.reload.last_activity_on).to be_nil end - it_behaves_like 'snippets with disabled feature flag' do - subject { push(key, personal_snippet) } + it_behaves_like 'snippets with disabled feature flag' + it_behaves_like 'sets hook env' do + let(:gl_repository) { Gitlab::GlRepository::SNIPPET.identifier_for_container(personal_snippet) } end end @@ -360,8 +371,10 @@ describe API::Internal::Base do end context 'git push with project snippet' do + subject { push(key, project_snippet, env: env.to_json) } + it 'responds with success' do - push(key, project_snippet) + subject expect(response).to have_gitlab_http_status(:ok) expect(json_response["status"]).to be_truthy @@ -370,8 +383,9 @@ describe API::Internal::Base do expect(user.reload.last_activity_on).to be_nil end - it_behaves_like 'snippets with disabled feature flag' do - subject { push(key, project_snippet) } + it_behaves_like 'snippets with disabled feature flag' + it_behaves_like 'sets hook env' do + let(:gl_repository) { Gitlab::GlRepository::SNIPPET.identifier_for_container(project_snippet) } end end diff --git a/spec/requests/api/releases_spec.rb b/spec/requests/api/releases_spec.rb index 4eb6e87c254..41999ca6e60 100644 --- a/spec/requests/api/releases_spec.rb +++ b/spec/requests/api/releases_spec.rb @@ -406,6 +406,22 @@ describe API::Releases do expect(project.releases.last.description).to eq('Super nice release') end + it 'creates a new release without description' do + params = { + name: 'New release without description', + tag_name: 'v0.1', + released_at: '2019-03-25 10:00:00' + } + + expect do + post api("/projects/#{project.id}/releases", maintainer), params: params + end.to change { Release.count }.by(1) + + expect(project.releases.last.name).to eq('New release without description') + expect(project.releases.last.tag).to eq('v0.1') + expect(project.releases.last.description).to eq(nil) + end + it 'sets the released_at to the current time if the released_at parameter is not provided' do now = Time.zone.parse('2015-08-25 06:00:00Z') Timecop.freeze(now) do @@ -451,26 +467,6 @@ describe API::Releases do expect(project.releases.last.released_at).to eq('2019-03-25T01:00:00Z') end - context 'when description is empty' do - let(:params) do - { - name: 'New release', - tag_name: 'v0.1', - description: '' - } - end - - it 'returns an error as validation failure' do - expect do - post api("/projects/#{project.id}/releases", maintainer), params: params - end.not_to change { Release.count } - - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']) - .to eq("Validation failed: Description can't be blank") - end - end - it 'matches response schema' do post api("/projects/#{project.id}/releases", maintainer), params: params diff --git a/spec/services/ci/update_ci_ref_status_service_spec.rb b/spec/services/ci/update_ci_ref_status_service_spec.rb index 2b069452a55..8b60586318d 100644 --- a/spec/services/ci/update_ci_ref_status_service_spec.rb +++ b/spec/services/ci/update_ci_ref_status_service_spec.rb @@ -119,6 +119,14 @@ describe Ci::UpdateCiRefStatusService do it_behaves_like 'does a noop' end + context 'pipeline is retried' do + before do + ci_ref.update!(last_updated_by_pipeline: pipeline) + end + + it_behaves_like 'updates ci_ref' + end + context 'ref is stale' do let(:pipeline1) { create(:ci_pipeline, :success, project: ci_ref.project, ref: ci_ref.ref, tag: ci_ref.tag) } let(:pipeline2) { create(:ci_pipeline, :success, project: ci_ref.project, ref: ci_ref.ref, tag: ci_ref.tag) } diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index 731febe75b3..55987c6fa0f 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -312,6 +312,8 @@ describe Projects::ForkService do # Stub everything required to move a project to a Gitaly shard that does not exist stub_storage_settings('test_second_storage' => { 'path' => TestEnv::SECOND_STORAGE_PATH }) + allow_any_instance_of(Gitlab::Git::Repository).to receive(:create_repository) + .and_return(true) allow_any_instance_of(Gitlab::Git::Repository).to receive(:replicate) allow_any_instance_of(Gitlab::Git::Repository).to receive(:checksum) .and_return(::Gitlab::Git::BLANK_SHA) diff --git a/spec/services/projects/update_repository_storage_service_spec.rb b/spec/services/projects/update_repository_storage_service_spec.rb index 106a639ba28..23ce6f9165d 100644 --- a/spec/services/projects/update_repository_storage_service_spec.rb +++ b/spec/services/projects/update_repository_storage_service_spec.rb @@ -32,6 +32,8 @@ describe Projects::UpdateRepositoryStorageService do project.repository.path_to_repo end + expect(project_repository_double).to receive(:create_repository) + .and_return(true) expect(project_repository_double).to receive(:replicate) .with(project.repository.raw) expect(project_repository_double).to receive(:checksum) @@ -58,6 +60,8 @@ describe Projects::UpdateRepositoryStorageService do context 'when the move fails' do it 'unmarks the repository as read-only without updating the repository storage' do + expect(project_repository_double).to receive(:create_repository) + .and_return(true) expect(project_repository_double).to receive(:replicate) .with(project.repository.raw) .and_raise(Gitlab::Git::CommandError) @@ -73,6 +77,8 @@ describe Projects::UpdateRepositoryStorageService do context 'when the checksum does not match' do it 'unmarks the repository as read-only without updating the repository storage' do + expect(project_repository_double).to receive(:create_repository) + .and_return(true) expect(project_repository_double).to receive(:replicate) .with(project.repository.raw) expect(project_repository_double).to receive(:checksum) @@ -91,6 +97,8 @@ describe Projects::UpdateRepositoryStorageService do let!(:pool) { create(:pool_repository, :ready, source_project: project) } it 'leaves the pool' do + expect(project_repository_double).to receive(:create_repository) + .and_return(true) expect(project_repository_double).to receive(:replicate) .with(project.repository.raw) expect(project_repository_double).to receive(:checksum) diff --git a/spec/services/releases/update_service_spec.rb b/spec/services/releases/update_service_spec.rb index f6c70873540..7f1849e39a4 100644 --- a/spec/services/releases/update_service_spec.rb +++ b/spec/services/releases/update_service_spec.rb @@ -44,12 +44,6 @@ describe Releases::UpdateService do it_behaves_like 'a failed update' end - context 'with an invalid update' do - let(:new_description) { '' } - - it_behaves_like 'a failed update' - end - context 'when a milestone is passed in' do let(:milestone) { create(:milestone, project: project, title: 'v1.0') } let(:params_with_milestone) { params.merge!({ milestones: [new_title] }) } diff --git a/spec/support/shared_examples/services/projects/update_repository_storage_service_shared_examples.rb b/spec/support/shared_examples/services/projects/update_repository_storage_service_shared_examples.rb index e30c620c4b1..b22379b8b68 100644 --- a/spec/support/shared_examples/services/projects/update_repository_storage_service_shared_examples.rb +++ b/spec/support/shared_examples/services/projects/update_repository_storage_service_shared_examples.rb @@ -22,11 +22,15 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type| context 'when the move succeeds', :clean_gitlab_redis_shared_state do before do + allow(project_repository_double).to receive(:create_repository) + .and_return(true) allow(project_repository_double).to receive(:replicate) .with(project.repository.raw) allow(project_repository_double).to receive(:checksum) .and_return(project_repository_checksum) + allow(repository_double).to receive(:create_repository) + .and_return(true) allow(repository_double).to receive(:replicate) .with(repository.raw) allow(repository_double).to receive(:checksum) @@ -90,11 +94,15 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type| context "when the move of the #{repository_type} repository fails" do it 'unmarks the repository as read-only without updating the repository storage' do + allow(project_repository_double).to receive(:create_repository) + .and_return(true) allow(project_repository_double).to receive(:replicate) .with(project.repository.raw) allow(project_repository_double).to receive(:checksum) .and_return(project_repository_checksum) + allow(repository_double).to receive(:create_repository) + .and_return(true) allow(repository_double).to receive(:replicate) .with(repository.raw) .and_raise(Gitlab::Git::CommandError) @@ -111,11 +119,15 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type| context "when the checksum of the #{repository_type} repository does not match" do it 'unmarks the repository as read-only without updating the repository storage' do + allow(project_repository_double).to receive(:create_repository) + .and_return(true) allow(project_repository_double).to receive(:replicate) .with(project.repository.raw) allow(project_repository_double).to receive(:checksum) .and_return(project_repository_checksum) + allow(repository_double).to receive(:create_repository) + .and_return(true) allow(repository_double).to receive(:replicate) .with(repository.raw) allow(repository_double).to receive(:checksum) |