diff options
Diffstat (limited to 'spec')
21 files changed, 321 insertions, 594 deletions
diff --git a/spec/controllers/health_controller_spec.rb b/spec/controllers/health_controller_spec.rb index 542eddc2d16..d800ad7c187 100644 --- a/spec/controllers/health_controller_spec.rb +++ b/spec/controllers/health_controller_spec.rb @@ -69,8 +69,7 @@ describe HealthController do expect(json_response['cache_check']['status']).to eq('ok') expect(json_response['queues_check']['status']).to eq('ok') expect(json_response['shared_state_check']['status']).to eq('ok') - expect(json_response['fs_shards_check']['status']).to eq('ok') - expect(json_response['fs_shards_check']['labels']['shard']).to eq('default') + expect(json_response['gitaly_check']['status']).to eq('ok') end end @@ -122,7 +121,6 @@ describe HealthController do expect(json_response['cache_check']['status']).to eq('ok') expect(json_response['queues_check']['status']).to eq('ok') expect(json_response['shared_state_check']['status']).to eq('ok') - expect(json_response['fs_shards_check']['status']).to eq('ok') end end diff --git a/spec/controllers/metrics_controller_spec.rb b/spec/controllers/metrics_controller_spec.rb index 9e8a37171ec..7376841fac8 100644 --- a/spec/controllers/metrics_controller_spec.rb +++ b/spec/controllers/metrics_controller_spec.rb @@ -59,6 +59,13 @@ describe MetricsController do expect(response.body).to match(/^redis_shared_state_ping_latency_seconds [0-9\.]+$/) end + it 'returns Gitaly metrics' do + get :index + + expect(response.body).to match(/^gitaly_health_check_success{shard="default"} 1$/) + expect(response.body).to match(/^gitaly_health_check_latency_seconds{shard="default"} [0-9\.]+$/) + end + context 'prometheus metrics are disabled' do before do allow(Gitlab::Metrics).to receive(:prometheus_metrics_enabled?).and_return(false) diff --git a/spec/features/protected_branches_spec.rb b/spec/features/protected_branches_spec.rb index 4c0f9971425..39bd4af6cd0 100644 --- a/spec/features/protected_branches_spec.rb +++ b/spec/features/protected_branches_spec.rb @@ -60,33 +60,6 @@ feature 'Protected Branches', :js do expect(page).to have_content('No branches to show') end end - - describe "Saved defaults" do - it "keeps the allowed to merge and push dropdowns defaults based on the previous selection" do - visit project_protected_branches_path(project) - form = '.js-new-protected-branch' - - within form do - find(".js-allowed-to-merge").click - wait_for_requests - click_link 'No one' - find(".js-allowed-to-push").click - wait_for_requests - click_link 'Developers + Maintainers' - end - - visit project_protected_branches_path(project) - - within form do - page.within(".js-allowed-to-merge") do - expect(page.find(".dropdown-toggle-text")).to have_content("No one") - end - page.within(".js-allowed-to-push") do - expect(page.find(".dropdown-toggle-text")).to have_content("Developers + Maintainers") - end - end - end - end end context 'logged in as admin' do @@ -97,6 +70,7 @@ feature 'Protected Branches', :js do describe "explicit protected branches" do it "allows creating explicit protected branches" do visit project_protected_branches_path(project) + set_defaults set_protected_branch_name('some-branch') click_on "Protect" @@ -110,6 +84,7 @@ feature 'Protected Branches', :js do project.repository.add_branch(admin, 'some-branch', commit.id) visit project_protected_branches_path(project) + set_defaults set_protected_branch_name('some-branch') click_on "Protect" @@ -118,6 +93,7 @@ feature 'Protected Branches', :js do it "displays an error message if the named branch does not exist" do visit project_protected_branches_path(project) + set_defaults set_protected_branch_name('some-branch') click_on "Protect" @@ -128,6 +104,7 @@ feature 'Protected Branches', :js do describe "wildcard protected branches" do it "allows creating protected branches with a wildcard" do visit project_protected_branches_path(project) + set_defaults set_protected_branch_name('*-stable') click_on "Protect" @@ -141,6 +118,7 @@ feature 'Protected Branches', :js do project.repository.add_branch(admin, 'staging-stable', 'master') visit project_protected_branches_path(project) + set_defaults set_protected_branch_name('*-stable') click_on "Protect" @@ -157,6 +135,7 @@ feature 'Protected Branches', :js do visit project_protected_branches_path(project) set_protected_branch_name('*-stable') + set_defaults click_on "Protect" visit project_protected_branches_path(project) @@ -180,4 +159,18 @@ feature 'Protected Branches', :js do find(".dropdown-input-field").set(branch_name) click_on("Create wildcard #{branch_name}") end + + def set_defaults + find(".js-allowed-to-merge").click + within('.qa-allowed-to-merge-dropdown') do + expect(first("li")).to have_content("Roles") + find(:link, 'No one').click + end + + find(".js-allowed-to-push").click + within('.qa-allowed-to-push-dropdown') do + expect(first("li")).to have_content("Roles") + find(:link, 'No one').click + end + end end diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index 593b2ca1825..14297a1a544 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -157,7 +157,7 @@ describe ApplicationHelper do let(:noteable_type) { Issue } it 'returns paths for autocomplete_sources_controller' do sources = helper.autocomplete_data_sources(project, noteable_type) - expect(sources.keys).to match_array([:members, :issues, :merge_requests, :labels, :milestones, :commands]) + expect(sources.keys).to match_array([:members, :issues, :mergeRequests, :labels, :milestones, :commands]) sources.keys.each do |key| expect(sources[key]).not_to be_nil end diff --git a/spec/initializers/6_validations_spec.rb b/spec/initializers/6_validations_spec.rb index 8d9dc092547..f96e5a2133f 100644 --- a/spec/initializers/6_validations_spec.rb +++ b/spec/initializers/6_validations_spec.rb @@ -44,49 +44,6 @@ describe '6_validations' do end end - describe 'validate_storages_paths' do - context 'with correct settings' do - before do - mock_storages('foo' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/paths/a/b/c'), 'bar' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/paths/a/b/d')) - end - - it 'passes through' do - expect { validate_storages_paths }.not_to raise_error - end - end - - context 'with nested storage paths' do - before do - mock_storages('foo' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/paths/a/b/c'), 'bar' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/paths/a/b/c/d')) - end - - it 'throws an error' do - expect { validate_storages_paths }.to raise_error('bar is a nested path of foo. Nested paths are not supported for repository storages. Please fix this in your gitlab.yml before starting GitLab.') - end - end - - context 'with similar but un-nested storage paths' do - before do - mock_storages('foo' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/paths/a/b/c'), 'bar' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/paths/a/b/c2')) - end - - it 'passes through' do - expect { validate_storages_paths }.not_to raise_error - end - end - - describe 'inaccessible storage' do - before do - mock_storages('foo' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/a/path/that/does/not/exist')) - end - - it 'passes through with a warning' do - expect(Rails.logger).to receive(:error) - expect { validate_storages_paths }.not_to raise_error - end - end - end - def mock_storages(storages) allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) end diff --git a/spec/javascripts/boards/issue_card_spec.js b/spec/javascripts/boards/issue_card_spec.js index 05acf903933..72e961c8a10 100644 --- a/spec/javascripts/boards/issue_card_spec.js +++ b/spec/javascripts/boards/issue_card_spec.js @@ -9,7 +9,7 @@ import '~/vue_shared/models/assignee'; import '~/boards/models/issue'; import '~/boards/models/list'; import '~/boards/stores/boards_store'; -import '~/boards/components/issue_card_inner'; +import IssueCardInner from '~/boards/components/issue_card_inner.vue'; import { listObj } from './mock_data'; describe('Issue card component', () => { @@ -48,7 +48,7 @@ describe('Issue card component', () => { component = new Vue({ el: document.querySelector('.test-container'), components: { - 'issue-card': gl.issueBoards.IssueCardInner, + 'issue-card': IssueCardInner, }, data() { return { diff --git a/spec/javascripts/diffs/components/diff_line_gutter_content_spec.js b/spec/javascripts/diffs/components/diff_line_gutter_content_spec.js index cce10c4083c..2d136a63c52 100644 --- a/spec/javascripts/diffs/components/diff_line_gutter_content_spec.js +++ b/spec/javascripts/diffs/components/diff_line_gutter_content_spec.js @@ -2,12 +2,6 @@ import Vue from 'vue'; import DiffLineGutterContent from '~/diffs/components/diff_line_gutter_content.vue'; import store from '~/mr_notes/stores'; import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; -import { - MATCH_LINE_TYPE, - CONTEXT_LINE_TYPE, - OLD_NO_NEW_LINE_TYPE, - NEW_NO_NEW_LINE_TYPE, -} from '~/diffs/constants'; import discussionsMockData from '../mock_data/diff_discussions'; import diffFileMockData from '../mock_data/diff_file'; @@ -31,45 +25,6 @@ describe('DiffLineGutterContent', () => { }; describe('computed', () => { - describe('isMatchLine', () => { - it('should return true for match line type', () => { - const component = createComponent({ lineType: MATCH_LINE_TYPE }); - expect(component.isMatchLine).toEqual(true); - }); - - it('should return false for non-match line type', () => { - const component = createComponent({ lineType: CONTEXT_LINE_TYPE }); - expect(component.isMatchLine).toEqual(false); - }); - }); - - describe('isContextLine', () => { - it('should return true for context line type', () => { - const component = createComponent({ lineType: CONTEXT_LINE_TYPE }); - expect(component.isContextLine).toEqual(true); - }); - - it('should return false for non-context line type', () => { - const component = createComponent({ lineType: MATCH_LINE_TYPE }); - expect(component.isContextLine).toEqual(false); - }); - }); - - describe('isMetaLine', () => { - it('should return true for meta line type', () => { - const component = createComponent({ lineType: NEW_NO_NEW_LINE_TYPE }); - expect(component.isMetaLine).toEqual(true); - - const component2 = createComponent({ lineType: OLD_NO_NEW_LINE_TYPE }); - expect(component2.isMetaLine).toEqual(true); - }); - - it('should return false for non-meta line type', () => { - const component = createComponent({ lineType: MATCH_LINE_TYPE }); - expect(component.isMetaLine).toEqual(false); - }); - }); - describe('lineHref', () => { it('should prepend # to lineCode', () => { const lineCode = 'LC_42'; @@ -109,7 +64,7 @@ describe('DiffLineGutterContent', () => { describe('template', () => { it('should render three dots for context lines', () => { const component = createComponent({ - lineType: MATCH_LINE_TYPE, + isMatchLine: true, }); expect(component.$el.querySelector('span').classList.contains('context-cell')).toEqual(true); diff --git a/spec/javascripts/diffs/components/inline_diff_view_spec.js b/spec/javascripts/diffs/components/inline_diff_view_spec.js index 0d5a3576204..e1adf60962e 100644 --- a/spec/javascripts/diffs/components/inline_diff_view_spec.js +++ b/spec/javascripts/diffs/components/inline_diff_view_spec.js @@ -1,7 +1,6 @@ import Vue from 'vue'; import InlineDiffView from '~/diffs/components/inline_diff_view.vue'; import store from '~/mr_notes/stores'; -import * as constants from '~/diffs/constants'; import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; import diffFileMockData from '../mock_data/diff_file'; import discussionsMockData from '../mock_data/diff_discussions'; @@ -14,58 +13,13 @@ describe('InlineDiffView', () => { beforeEach(() => { const diffFile = getDiffFileMock(); + store.dispatch('setInlineDiffViewType'); component = createComponentWithStore(Vue.extend(InlineDiffView), store, { diffFile, diffLines: diffFile.highlightedDiffLines, }).$mount(); }); - describe('methods', () => { - describe('handleMouse', () => { - it('should set hoveredLineCode', () => { - expect(component.hoveredLineCode).toEqual(null); - - component.handleMouse('lineCode1', true); - expect(component.hoveredLineCode).toEqual('lineCode1'); - - component.handleMouse('lineCode1', false); - expect(component.hoveredLineCode).toEqual(null); - }); - }); - - describe('getLineClass', () => { - it('should return line class object', () => { - const { LINE_HOVER_CLASS_NAME, LINE_UNFOLD_CLASS_NAME } = constants; - const { MATCH_LINE_TYPE, NEW_LINE_TYPE } = constants; - - expect(component.getLineClass(component.diffLines[0])).toEqual({ - [NEW_LINE_TYPE]: NEW_LINE_TYPE, - [LINE_UNFOLD_CLASS_NAME]: false, - [LINE_HOVER_CLASS_NAME]: false, - }); - - component.handleMouse(component.diffLines[0].lineCode, true); - Object.defineProperty(component, 'isLoggedIn', { - get() { - return true; - }, - }); - - expect(component.getLineClass(component.diffLines[0])).toEqual({ - [NEW_LINE_TYPE]: NEW_LINE_TYPE, - [LINE_UNFOLD_CLASS_NAME]: false, - [LINE_HOVER_CLASS_NAME]: true, - }); - - expect(component.getLineClass(component.diffLines[5])).toEqual({ - [MATCH_LINE_TYPE]: MATCH_LINE_TYPE, - [LINE_UNFOLD_CLASS_NAME]: true, - [LINE_HOVER_CLASS_NAME]: false, - }); - }); - }); - }); - describe('template', () => { it('should have rendered diff lines', () => { const el = component.$el; @@ -89,23 +43,5 @@ describe('InlineDiffView', () => { done(); }); }); - - it('should render new discussion forms', done => { - const el = component.$el; - const lines = getDiffFileMock().highlightedDiffLines; - - component.handleShowCommentForm({ lineCode: lines[0].lineCode }); - component.handleShowCommentForm({ lineCode: lines[1].lineCode }); - - Vue.nextTick(() => { - expect(el.querySelectorAll('.js-vue-markdown-field').length).toEqual(2); - expect(el.querySelectorAll('tr')[1].classList.contains('notes_holder')).toEqual(true); - expect(el.querySelectorAll('tr')[3].classList.contains('notes_holder')).toEqual(true); - - store.state.diffs.diffLineCommentForms = {}; - - done(); - }); - }); }); }); diff --git a/spec/javascripts/diffs/components/parallel_diff_view_spec.js b/spec/javascripts/diffs/components/parallel_diff_view_spec.js index cab533217c0..165e4b69b6c 100644 --- a/spec/javascripts/diffs/components/parallel_diff_view_spec.js +++ b/spec/javascripts/diffs/components/parallel_diff_view_spec.js @@ -4,12 +4,10 @@ import store from '~/mr_notes/stores'; import * as constants from '~/diffs/constants'; import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; import diffFileMockData from '../mock_data/diff_file'; -import discussionsMockData from '../mock_data/diff_discussions'; describe('ParallelDiffView', () => { let component; const getDiffFileMock = () => Object.assign({}, diffFileMockData); - const getDiscussionsMockData = () => [Object.assign({}, discussionsMockData)]; beforeEach(() => { const diffFile = getDiffFileMock(); @@ -28,197 +26,4 @@ describe('ParallelDiffView', () => { }); }); }); - - describe('methods', () => { - describe('hasDiscussion', () => { - it('it should return true if there is a discussion either for left or right section', () => { - Object.defineProperty(component, 'discussionsByLineCode', { - get() { - return { line_42: true }; - }, - }); - - expect(component.hasDiscussion({ left: {}, right: {} })).toEqual(undefined); - expect(component.hasDiscussion({ left: { lineCode: 'line_42' }, right: {} })).toEqual(true); - expect(component.hasDiscussion({ left: {}, right: { lineCode: 'line_42' } })).toEqual(true); - }); - }); - - describe('getClassName', () => { - it('should return line class object', () => { - const { LINE_HOVER_CLASS_NAME, LINE_UNFOLD_CLASS_NAME } = constants; - const { MATCH_LINE_TYPE, NEW_LINE_TYPE, LINE_POSITION_RIGHT } = constants; - - expect(component.getClassName(component.diffLines[1], LINE_POSITION_RIGHT)).toEqual({ - [NEW_LINE_TYPE]: NEW_LINE_TYPE, - [LINE_UNFOLD_CLASS_NAME]: false, - [LINE_HOVER_CLASS_NAME]: false, - }); - - const eventMock = { - target: component.$refs.rightLines[1], - }; - - component.handleMouse(eventMock, component.diffLines[1], true); - Object.defineProperty(component, 'isLoggedIn', { - get() { - return true; - }, - }); - - expect(component.getClassName(component.diffLines[1], LINE_POSITION_RIGHT)).toEqual({ - [NEW_LINE_TYPE]: NEW_LINE_TYPE, - [LINE_UNFOLD_CLASS_NAME]: false, - [LINE_HOVER_CLASS_NAME]: true, - }); - - expect(component.getClassName(component.diffLines[5], LINE_POSITION_RIGHT)).toEqual({ - [MATCH_LINE_TYPE]: MATCH_LINE_TYPE, - [LINE_UNFOLD_CLASS_NAME]: true, - [LINE_HOVER_CLASS_NAME]: false, - }); - }); - }); - - describe('handleMouse', () => { - it('should set hovered line code and line section to null when isHover is false', () => { - const rightLineEventMock = { target: component.$refs.rightLines[1] }; - expect(component.hoveredLineCode).toEqual(null); - expect(component.hoveredSection).toEqual(null); - - component.handleMouse(rightLineEventMock, null, false); - expect(component.hoveredLineCode).toEqual(null); - expect(component.hoveredSection).toEqual(null); - }); - - it('should set hovered line code and line section for right section', () => { - const rightLineEventMock = { target: component.$refs.rightLines[1] }; - component.handleMouse(rightLineEventMock, component.diffLines[1], true); - expect(component.hoveredLineCode).toEqual(component.diffLines[1].right.lineCode); - expect(component.hoveredSection).toEqual(constants.LINE_POSITION_RIGHT); - }); - - it('should set hovered line code and line section for left section', () => { - const leftLineEventMock = { target: component.$refs.leftLines[2] }; - component.handleMouse(leftLineEventMock, component.diffLines[2], true); - expect(component.hoveredLineCode).toEqual(component.diffLines[2].left.lineCode); - expect(component.hoveredSection).toEqual(constants.LINE_POSITION_LEFT); - }); - }); - - describe('shouldRenderDiscussions', () => { - it('should return true if there is a discussion on left side and it is expanded', () => { - const line = { left: { lineCode: 'lineCode1' } }; - spyOn(component, 'isDiscussionExpanded').and.returnValue(true); - Object.defineProperty(component, 'discussionsByLineCode', { - get() { - return { - [line.left.lineCode]: true, - }; - }, - }); - - expect(component.shouldRenderDiscussions(line, constants.LINE_POSITION_LEFT)).toEqual(true); - expect(component.isDiscussionExpanded).toHaveBeenCalledWith(line.left.lineCode); - }); - - it('should return false if there is a discussion on left side but it is collapsed', () => { - const line = { left: { lineCode: 'lineCode1' } }; - spyOn(component, 'isDiscussionExpanded').and.returnValue(false); - Object.defineProperty(component, 'discussionsByLineCode', { - get() { - return { - [line.left.lineCode]: true, - }; - }, - }); - - expect(component.shouldRenderDiscussions(line, constants.LINE_POSITION_LEFT)).toEqual( - false, - ); - }); - - it('should return false for discussions on the right side if there is no line type', () => { - const CUSTOM_RIGHT_LINE_TYPE = 'CUSTOM_RIGHT_LINE_TYPE'; - const line = { right: { lineCode: 'lineCode1', type: CUSTOM_RIGHT_LINE_TYPE } }; - spyOn(component, 'isDiscussionExpanded').and.returnValue(true); - Object.defineProperty(component, 'discussionsByLineCode', { - get() { - return { - [line.right.lineCode]: true, - }; - }, - }); - - expect(component.shouldRenderDiscussions(line, constants.LINE_POSITION_RIGHT)).toEqual( - CUSTOM_RIGHT_LINE_TYPE, - ); - }); - }); - - describe('hasAnyExpandedDiscussion', () => { - const LINE_CODE_LEFT = 'LINE_CODE_LEFT'; - const LINE_CODE_RIGHT = 'LINE_CODE_RIGHT'; - - it('should return true if there is a discussion either on the left or the right side', () => { - const mockLineOne = { - right: { lineCode: LINE_CODE_RIGHT }, - left: {}, - }; - const mockLineTwo = { - left: { lineCode: LINE_CODE_LEFT }, - right: {}, - }; - - spyOn(component, 'isDiscussionExpanded').and.callFake(lc => lc === LINE_CODE_RIGHT); - expect(component.hasAnyExpandedDiscussion(mockLineOne)).toEqual(true); - expect(component.hasAnyExpandedDiscussion(mockLineTwo)).toEqual(false); - }); - }); - }); - - describe('template', () => { - it('should have rendered diff lines', () => { - const el = component.$el; - - expect(el.querySelectorAll('tr.line_holder.parallel').length).toEqual(6); - expect(el.querySelectorAll('td.empty-cell').length).toEqual(4); - expect(el.querySelectorAll('td.line_content.parallel.right-side').length).toEqual(6); - expect(el.querySelectorAll('td.line_content.parallel.left-side').length).toEqual(6); - expect(el.querySelectorAll('td.match').length).toEqual(4); - expect(el.textContent.indexOf('Bad dates') > -1).toEqual(true); - }); - - it('should render discussions', done => { - const el = component.$el; - component.$store.dispatch('setInitialNotes', getDiscussionsMockData()); - - Vue.nextTick(() => { - expect(el.querySelectorAll('.notes_holder').length).toEqual(1); - expect(el.querySelectorAll('.notes_holder .note-discussion li').length).toEqual(5); - expect(el.innerText.indexOf('comment 5') > -1).toEqual(true); - component.$store.dispatch('setInitialNotes', []); - - done(); - }); - }); - - it('should render new discussion forms', done => { - const el = component.$el; - const lines = getDiffFileMock().parallelDiffLines; - - component.handleShowCommentForm({ lineCode: lines[0].lineCode }); - component.handleShowCommentForm({ lineCode: lines[1].lineCode }); - - Vue.nextTick(() => { - expect(el.querySelectorAll('.js-vue-markdown-field').length).toEqual(2); - expect(el.querySelectorAll('tr')[1].classList.contains('notes_holder')).toEqual(true); - expect(el.querySelectorAll('tr')[3].classList.contains('notes_holder')).toEqual(true); - - store.state.diffs.diffLineCommentForms = {}; - - done(); - }); - }); - }); }); diff --git a/spec/lib/gitaly/server_spec.rb b/spec/lib/gitaly/server_spec.rb index ed5d56e91d4..09bf21b5946 100644 --- a/spec/lib/gitaly/server_spec.rb +++ b/spec/lib/gitaly/server_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Gitaly::Server do + let(:server) { described_class.new('default') } + describe '.all' do let(:storages) { Gitlab.config.repositories.storages } @@ -17,6 +19,38 @@ describe Gitaly::Server do it { is_expected.to respond_to(:up_to_date?) } it { is_expected.to respond_to(:address) } + describe 'readable?' do + context 'when the storage is readable' do + it 'returns true' do + expect(server).to be_readable + end + end + + context 'when the storage is not readable' do + let(:server) { described_class.new('broken') } + + it 'returns false' do + expect(server).not_to be_readable + end + end + end + + describe 'writeable?' do + context 'when the storage is writeable' do + it 'returns true' do + expect(server).to be_writeable + end + end + + context 'when the storage is not writeable' do + let(:server) { described_class.new('broken') } + + it 'returns false' do + expect(server).not_to be_writeable + end + end + end + describe 'request memoization' do context 'when requesting multiple properties', :request_store do it 'uses memoization for the info request' do diff --git a/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb b/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb deleted file mode 100644 index 9dcf272d25e..00000000000 --- a/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb +++ /dev/null @@ -1,200 +0,0 @@ -require 'spec_helper' - -describe Gitlab::HealthChecks::FsShardsCheck do - def command_exists?(command) - _, status = Gitlab::Popen.popen(%W{ #{command} 1 echo }) - status.zero? - rescue Errno::ENOENT - false - end - - def timeout_command - @timeout_command ||= - if command_exists?('timeout') - 'timeout' - elsif command_exists?('gtimeout') - 'gtimeout' - else - '' - end - end - - let(:metric_class) { Gitlab::HealthChecks::Metric } - let(:result_class) { Gitlab::HealthChecks::Result } - let(:repository_storages) { ['default'] } - let(:tmp_dir) { Dir.mktmpdir } - - let(:storages_paths) do - { - default: Gitlab::GitalyClient::StorageSettings.new('path' => tmp_dir) - }.with_indifferent_access - end - - before do - allow(described_class).to receive(:repository_storages) { repository_storages } - allow(described_class).to receive(:storages_paths) { storages_paths } - stub_const('Gitlab::HealthChecks::FsShardsCheck::TIMEOUT_EXECUTABLE', timeout_command) - end - - after do - FileUtils.remove_entry_secure(tmp_dir) if Dir.exist?(tmp_dir) - end - - shared_examples 'filesystem checks' do - describe '#readiness' do - subject { described_class.readiness } - - context 'storage has a tripped circuitbreaker', :broken_storage do - let(:repository_storages) { ['broken'] } - let(:storages_paths) do - Gitlab.config.repositories.storages - end - - it { is_expected.to include(result_class.new(false, 'circuitbreaker tripped', shard: 'broken')) } - end - - context 'storage points to not existing folder' do - let(:storages_paths) do - { - default: Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/this/path/doesnt/exist') - }.with_indifferent_access - end - - before do - allow(described_class).to receive(:storage_circuitbreaker_test) { true } - end - - it { is_expected.to include(result_class.new(false, 'cannot stat storage', shard: 'default')) } - end - - context 'storage points to directory that has both read and write rights' do - before do - FileUtils.chmod_R(0755, tmp_dir) - end - - it { is_expected.to include(result_class.new(true, nil, shard: 'default')) } - - it 'cleans up files used for testing' do - expect(described_class).to receive(:storage_write_test).with(any_args).and_call_original - - expect { subject }.not_to change(Dir.entries(tmp_dir), :count) - end - - context 'read test fails' do - before do - allow(described_class).to receive(:storage_read_test).with(any_args).and_return(false) - end - - it { is_expected.to include(result_class.new(false, 'cannot read from storage', shard: 'default')) } - end - - context 'write test fails' do - before do - allow(described_class).to receive(:storage_write_test).with(any_args).and_return(false) - end - - it { is_expected.to include(result_class.new(false, 'cannot write to storage', shard: 'default')) } - end - end - end - - describe '#metrics' do - context 'storage points to not existing folder' do - let(:storages_paths) do - { - default: Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/this/path/doesnt/exist') - }.with_indifferent_access - end - - it 'provides metrics' do - metrics = described_class.metrics - - expect(metrics).to all(have_attributes(labels: { shard: 'default' })) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_accessible, value: 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_readable, value: 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_writable, value: 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_access_latency_seconds, value: be >= 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_read_latency_seconds, value: be >= 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_write_latency_seconds, value: be >= 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_circuitbreaker_latency_seconds, value: be >= 0)) - end - end - - context 'storage points to directory that has both read and write rights' do - before do - FileUtils.chmod_R(0755, tmp_dir) - end - - it 'provides metrics' do - metrics = described_class.metrics - - expect(metrics).to all(have_attributes(labels: { shard: 'default' })) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_accessible, value: 1)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_readable, value: 1)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_writable, value: 1)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_access_latency_seconds, value: be >= 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_read_latency_seconds, value: be >= 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_write_latency_seconds, value: be >= 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_circuitbreaker_latency_seconds, value: be >= 0)) - end - - it 'cleans up files used for metrics' do - expect { described_class.metrics }.not_to change(Dir.entries(tmp_dir), :count) - end - end - end - end - - context 'when timeout kills fs checks' do - before do - stub_const('Gitlab::HealthChecks::FsShardsCheck::COMMAND_TIMEOUT', '1') - - allow(described_class).to receive(:exec_with_timeout).and_wrap_original { |m| m.call(%w(sleep 60)) } - FileUtils.chmod_R(0755, tmp_dir) - end - - describe '#readiness' do - subject { described_class.readiness } - - it { is_expected.to include(result_class.new(false, 'cannot stat storage', shard: 'default')) } - end - - describe '#metrics' do - it 'provides metrics' do - metrics = described_class.metrics - - expect(metrics).to all(have_attributes(labels: { shard: 'default' })) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_accessible, value: 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_readable, value: 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_writable, value: 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_access_latency_seconds, value: be >= 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_read_latency_seconds, value: be >= 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_write_latency_seconds, value: be >= 0)) - end - end - end - - context 'when popen always finds required binaries' do - before do - allow(described_class).to receive(:exec_with_timeout).and_wrap_original do |method, *args, &block| - begin - method.call(*args, &block) - rescue RuntimeError, Errno::ENOENT - raise 'expected not to happen' - end - end - - stub_const('Gitlab::HealthChecks::FsShardsCheck::COMMAND_TIMEOUT', '10') - end - - it_behaves_like 'filesystem checks' - end - - context 'when popen never finds required binaries' do - before do - allow(Gitlab::Popen).to receive(:popen).and_raise(Errno::ENOENT) - end - - it_behaves_like 'filesystem checks' - end -end diff --git a/spec/lib/gitlab/health_checks/gitaly_check_spec.rb b/spec/lib/gitlab/health_checks/gitaly_check_spec.rb index 724beefff69..4912cd48761 100644 --- a/spec/lib/gitlab/health_checks/gitaly_check_spec.rb +++ b/spec/lib/gitlab/health_checks/gitaly_check_spec.rb @@ -30,13 +30,14 @@ describe Gitlab::HealthChecks::GitalyCheck do describe '#metrics' do subject { described_class.metrics } + let(:server) { double(storage: 'default', read_writeable?: up) } before do - expect(Gitlab::GitalyClient::HealthCheckService).to receive(:new).and_return(gitaly_check) + allow(Gitaly::Server).to receive(:new).and_return(server) end context 'Gitaly server is up' do - let(:gitaly_check) { double(check: { success: true }) } + let(:up) { true } it 'provides metrics' do expect(subject).to all(have_attributes(labels: { shard: 'default' })) @@ -46,7 +47,7 @@ describe Gitlab::HealthChecks::GitalyCheck do end context 'Gitaly server is down' do - let(:gitaly_check) { double(check: { success: false, message: 'Connection refused' }) } + let(:up) { false } it 'provides metrics' do expect(subject).to include(an_object_having_attributes(name: 'gitaly_health_check_success', value: 0)) diff --git a/spec/lib/gitlab/shard_health_cache_spec.rb b/spec/lib/gitlab/shard_health_cache_spec.rb new file mode 100644 index 00000000000..e1a69261939 --- /dev/null +++ b/spec/lib/gitlab/shard_health_cache_spec.rb @@ -0,0 +1,52 @@ +require 'spec_helper' + +describe Gitlab::ShardHealthCache, :clean_gitlab_redis_cache do + let(:shards) { %w(foo bar) } + + before do + described_class.update(shards) + end + + describe '.clear' do + it 'leaves no shards around' do + described_class.clear + + expect(described_class.healthy_shard_count).to eq(0) + end + end + + describe '.update' do + it 'returns the healthy shards' do + expect(described_class.cached_healthy_shards).to match_array(shards) + end + + it 'replaces the existing set' do + new_set = %w(test me more) + described_class.update(new_set) + + expect(described_class.cached_healthy_shards).to match_array(new_set) + end + end + + describe '.healthy_shard_count' do + it 'returns the healthy shard count' do + expect(described_class.healthy_shard_count).to eq(2) + end + + it 'returns 0 if no shards are available' do + described_class.update([]) + + expect(described_class.healthy_shard_count).to eq(0) + end + end + + describe '.healthy_shard?' do + it 'returns true for a healthy shard' do + expect(described_class.healthy_shard?('foo')).to be_truthy + end + + it 'returns false for an unknown shard' do + expect(described_class.healthy_shard?('unknown')).to be_falsey + end + end +end diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index d8fdfd6dee1..4bc5d3ee899 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -21,6 +21,89 @@ describe API::Files do "/projects/#{project.id}/repository/files/#{file_path}" end + describe "HEAD /projects/:id/repository/files/:file_path" do + shared_examples_for 'repository files' do + it 'returns file attributes in headers' do + head api(route(file_path), current_user), params + + expect(response).to have_gitlab_http_status(200) + expect(response.headers['X-Gitlab-File-Path']).to eq(CGI.unescape(file_path)) + expect(response.headers['X-Gitlab-File-Name']).to eq('popen.rb') + expect(response.headers['X-Gitlab-Last-Commit-Id']).to eq('570e7b2abdd848b95f2f578043fc23bd6f6fd24d') + expect(response.headers['X-Gitlab-Content-Sha256']).to eq('c440cd09bae50c4632cc58638ad33c6aa375b6109d811e76a9cc3a613c1e8887') + end + + it 'returns file by commit sha' do + # This file is deleted on HEAD + file_path = "files%2Fjs%2Fcommit%2Ejs%2Ecoffee" + params[:ref] = "6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9" + + head api(route(file_path), current_user), params + + expect(response).to have_gitlab_http_status(200) + expect(response.headers['X-Gitlab-File-Name']).to eq('commit.js.coffee') + expect(response.headers['X-Gitlab-Content-Sha256']).to eq('08785f04375b47f81f46e68cc125d5ef368aa20576ddb53f91f4d83f1d04b929') + end + + context 'when mandatory params are not given' do + it "responds with a 400 status" do + head api(route("any%2Ffile"), current_user) + + expect(response).to have_gitlab_http_status(400) + end + end + + context 'when file_path does not exist' do + it "responds with a 404 status" do + params[:ref] = 'master' + + head api(route('app%2Fmodels%2Fapplication%2Erb'), current_user), params + + expect(response).to have_gitlab_http_status(404) + end + end + + context 'when file_path does not exist' do + include_context 'disabled repository' + + it "responds with a 403 status" do + head api(route(file_path), current_user), params + + expect(response).to have_gitlab_http_status(403) + end + end + end + + context 'when unauthenticated', 'and project is public' do + it_behaves_like 'repository files' do + let(:project) { create(:project, :public, :repository) } + let(:current_user) { nil } + end + end + + context 'when unauthenticated', 'and project is private' do + it "responds with a 404 status" do + current_user = nil + + head api(route(file_path), current_user), params + + expect(response).to have_gitlab_http_status(404) + end + end + + context 'when authenticated', 'as a developer' do + it_behaves_like 'repository files' do + let(:current_user) { user } + end + end + + context 'when authenticated', 'as a guest' do + it_behaves_like '403 response' do + let(:request) { head api(route(file_path), guest), params } + end + end + end + describe "GET /projects/:id/repository/files/:file_path" do shared_examples_for 'repository files' do it 'returns file attributes as json' do @@ -30,6 +113,7 @@ describe API::Files do expect(json_response['file_path']).to eq(CGI.unescape(file_path)) expect(json_response['file_name']).to eq('popen.rb') expect(json_response['last_commit_id']).to eq('570e7b2abdd848b95f2f578043fc23bd6f6fd24d') + expect(json_response['content_sha256']).to eq('c440cd09bae50c4632cc58638ad33c6aa375b6109d811e76a9cc3a613c1e8887') expect(Base64.decode64(json_response['content']).lines.first).to eq("require 'fileutils'\n") end @@ -51,6 +135,7 @@ describe API::Files do expect(response).to have_gitlab_http_status(200) expect(json_response['file_name']).to eq('commit.js.coffee') + expect(json_response['content_sha256']).to eq('08785f04375b47f81f46e68cc125d5ef368aa20576ddb53f91f4d83f1d04b929') expect(Base64.decode64(json_response['content']).lines.first).to eq("class Commit\n") end diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index a15d60aafe0..95eff029f98 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -1679,7 +1679,7 @@ describe API::Issues do let!(:user_agent_detail) { create(:user_agent_detail, subject: issue) } context 'when unauthenticated' do - it "returns unautorized" do + it "returns unauthorized" do get api("/projects/#{project.id}/issues/#{issue.iid}/user_agent_detail") expect(response).to have_gitlab_http_status(401) @@ -1695,7 +1695,7 @@ describe API::Issues do expect(json_response['akismet_submitted']).to eq(user_agent_detail.submitted) end - it "returns unautorized for non-admin users" do + it "returns unauthorized for non-admin users" do get api("/projects/#{project.id}/issues/#{issue.iid}/user_agent_detail", user) expect(response).to have_gitlab_http_status(403) diff --git a/spec/requests/api/project_snippets_spec.rb b/spec/requests/api/project_snippets_spec.rb index 4a2289ca137..a3b5e8c6223 100644 --- a/spec/requests/api/project_snippets_spec.rb +++ b/spec/requests/api/project_snippets_spec.rb @@ -25,7 +25,7 @@ describe API::ProjectSnippets do expect(response).to have_gitlab_http_status(404) end - it "returns unautorized for non-admin users" do + it "returns unauthorized for non-admin users" do get api("/projects/#{snippet.project.id}/snippets/#{snippet.id}/user_agent_detail", user) expect(response).to have_gitlab_http_status(403) diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb index c5456977b60..6da769cb3ed 100644 --- a/spec/requests/api/snippets_spec.rb +++ b/spec/requests/api/snippets_spec.rb @@ -314,7 +314,7 @@ describe API::Snippets do expect(json_response['akismet_submitted']).to eq(user_agent_detail.submitted) end - it "returns unautorized for non-admin users" do + it "returns unauthorized for non-admin users" do get api("/snippets/#{snippet.id}/user_agent_detail", user) expect(response).to have_gitlab_http_status(403) diff --git a/spec/services/merge_requests/post_merge_service_spec.rb b/spec/services/merge_requests/post_merge_service_spec.rb index 790ecce8ded..46e4e3559dc 100644 --- a/spec/services/merge_requests/post_merge_service_spec.rb +++ b/spec/services/merge_requests/post_merge_service_spec.rb @@ -47,5 +47,18 @@ describe MergeRequests::PostMergeService do expect(diff_removal_service).to have_received(:execute) end + + it 'marks MR as merged regardless of errors when closing issues' do + merge_request.update(target_branch: 'foo') + allow(project).to receive(:default_branch).and_return('foo') + + issue = create(:issue, project: project) + allow(merge_request).to receive(:closes_issues).and_return([issue]) + allow_any_instance_of(Issues::CloseService).to receive(:execute).with(issue, commit: merge_request).and_raise + + expect { described_class.new(project, user, {}).execute(merge_request) }.to raise_error + + expect(merge_request.reload).to be_merged + end end end diff --git a/spec/support/shared_examples/features/protected_branches_access_control_ce.rb b/spec/support/shared_examples/features/protected_branches_access_control_ce.rb index 5241c0fa6f1..a8f2c2e7a5a 100644 --- a/spec/support/shared_examples/features/protected_branches_access_control_ce.rb +++ b/spec/support/shared_examples/features/protected_branches_access_control_ce.rb @@ -5,6 +5,12 @@ shared_examples "protected branches > access control > CE" do set_protected_branch_name('master') + find(".js-allowed-to-merge").click + within('.qa-allowed-to-merge-dropdown') do + expect(first("li")).to have_content("Roles") + find(:link, 'No one').click + end + within('.js-new-protected-branch') do allowed_to_push_button = find(".js-allowed-to-push") @@ -25,6 +31,18 @@ shared_examples "protected branches > access control > CE" do set_protected_branch_name('master') + find(".js-allowed-to-merge").click + within('.qa-allowed-to-merge-dropdown') do + expect(first("li")).to have_content("Roles") + find(:link, 'No one').click + end + + find(".js-allowed-to-push").click + within('.qa-allowed-to-push-dropdown') do + expect(first("li")).to have_content("Roles") + find(:link, 'No one').click + end + click_on "Protect" expect(ProtectedBranch.count).to eq(1) @@ -59,6 +77,12 @@ shared_examples "protected branches > access control > CE" do end end + find(".js-allowed-to-push").click + within('.qa-allowed-to-push-dropdown') do + expect(first("li")).to have_content("Roles") + find(:link, 'No one').click + end + click_on "Protect" expect(ProtectedBranch.count).to eq(1) @@ -70,6 +94,18 @@ shared_examples "protected branches > access control > CE" do set_protected_branch_name('master') + find(".js-allowed-to-merge").click + within('.qa-allowed-to-merge-dropdown') do + expect(first("li")).to have_content("Roles") + find(:link, 'No one').click + end + + find(".js-allowed-to-push").click + within('.qa-allowed-to-push-dropdown') do + expect(first("li")).to have_content("Roles") + find(:link, 'No one').click + end + click_on "Protect" expect(ProtectedBranch.count).to eq(1) diff --git a/spec/workers/repository_check/batch_worker_spec.rb b/spec/workers/repository_check/batch_worker_spec.rb index 6cd27d2fafb..6bc551be9ad 100644 --- a/spec/workers/repository_check/batch_worker_spec.rb +++ b/spec/workers/repository_check/batch_worker_spec.rb @@ -1,14 +1,19 @@ require 'spec_helper' describe RepositoryCheck::BatchWorker do + let(:shard_name) { 'default' } subject { described_class.new } + before do + Gitlab::ShardHealthCache.update([shard_name]) + end + it 'prefers projects that have never been checked' do projects = create_list(:project, 3, created_at: 1.week.ago) projects[0].update_column(:last_repository_check_at, 4.months.ago) projects[2].update_column(:last_repository_check_at, 3.months.ago) - expect(subject.perform).to eq(projects.values_at(1, 0, 2).map(&:id)) + expect(subject.perform(shard_name)).to eq(projects.values_at(1, 0, 2).map(&:id)) end it 'sorts projects by last_repository_check_at' do @@ -17,7 +22,7 @@ describe RepositoryCheck::BatchWorker do projects[1].update_column(:last_repository_check_at, 4.months.ago) projects[2].update_column(:last_repository_check_at, 3.months.ago) - expect(subject.perform).to eq(projects.values_at(1, 2, 0).map(&:id)) + expect(subject.perform(shard_name)).to eq(projects.values_at(1, 2, 0).map(&:id)) end it 'excludes projects that were checked recently' do @@ -26,7 +31,14 @@ describe RepositoryCheck::BatchWorker do projects[1].update_column(:last_repository_check_at, 2.months.ago) projects[2].update_column(:last_repository_check_at, 3.days.ago) - expect(subject.perform).to eq([projects[1].id]) + expect(subject.perform(shard_name)).to eq([projects[1].id]) + end + + it 'excludes projects on another shard' do + projects = create_list(:project, 2, created_at: 1.week.ago) + projects[0].update_column(:repository_storage, 'other') + + expect(subject.perform(shard_name)).to eq([projects[1].id]) end it 'does nothing when repository checks are disabled' do @@ -34,13 +46,20 @@ describe RepositoryCheck::BatchWorker do stub_application_setting(repository_checks_enabled: false) - expect(subject.perform).to eq(nil) + expect(subject.perform(shard_name)).to eq(nil) + end + + it 'does nothing when shard is unhealthy' do + shard_name = 'broken' + create(:project, created_at: 1.week.ago, repository_storage: shard_name) + + expect(subject.perform(shard_name)).to eq(nil) end it 'skips projects created less than 24 hours ago' do project = create(:project) project.update_column(:created_at, 23.hours.ago) - expect(subject.perform).to eq([]) + expect(subject.perform(shard_name)).to eq([]) end end diff --git a/spec/workers/repository_check/dispatch_worker_spec.rb b/spec/workers/repository_check/dispatch_worker_spec.rb new file mode 100644 index 00000000000..20a4f1f5344 --- /dev/null +++ b/spec/workers/repository_check/dispatch_worker_spec.rb @@ -0,0 +1,36 @@ +require 'spec_helper' + +describe RepositoryCheck::DispatchWorker do + subject { described_class.new } + + it 'does nothing when repository checks are disabled' do + stub_application_setting(repository_checks_enabled: false) + + expect(RepositoryCheck::BatchWorker).not_to receive(:perform_async) + + subject.perform + end + + it 'dispatches work to RepositoryCheck::BatchWorker' do + expect(RepositoryCheck::BatchWorker).to receive(:perform_async).at_least(:once) + + subject.perform + end + + context 'with unhealthy shard' do + let(:default_shard_name) { 'default' } + let(:unhealthy_shard_name) { 'unhealthy' } + let(:default_shard) { Gitlab::HealthChecks::Result.new(true, nil, shard: default_shard_name) } + let(:unhealthy_shard) { Gitlab::HealthChecks::Result.new(false, '14:Connect Failed', shard: unhealthy_shard_name) } + + before do + allow(Gitlab::HealthChecks::GitalyCheck).to receive(:readiness).and_return([default_shard, unhealthy_shard]) + end + + it 'only triggers RepositoryCheck::BatchWorker for healthy shards' do + expect(RepositoryCheck::BatchWorker).to receive(:perform_async).with('default') + + subject.perform + end + end +end |