diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-11-26 15:09:48 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-11-26 15:09:48 +0300 |
commit | 142890d5bbefa7b1e1b30f263f9ad67d9d496d29 (patch) | |
tree | 2c4de4059aba586f3204f983f17757dbf1643b6c /spec | |
parent | bbede1e22c4dd615042a3e9c0d7e24afc83ca377 (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
44 files changed, 871 insertions, 500 deletions
diff --git a/spec/controllers/import/google_code_controller_spec.rb b/spec/controllers/import/google_code_controller_spec.rb deleted file mode 100644 index 0fda111c029..00000000000 --- a/spec/controllers/import/google_code_controller_spec.rb +++ /dev/null @@ -1,65 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Import::GoogleCodeController do - include ImportSpecHelper - - let(:user) { create(:user) } - let(:dump_file) { fixture_file_upload('spec/fixtures/GoogleCodeProjectHosting.json', 'application/json') } - - before do - sign_in(user) - end - - describe "POST callback" do - it "stores Google Takeout dump list in session" do - post :callback, params: { dump_file: dump_file } - - expect(session[:google_code_dump]).to be_a(Hash) - expect(session[:google_code_dump]["kind"]).to eq("projecthosting#user") - expect(session[:google_code_dump]).to have_key("projects") - end - end - - describe "GET status" do - before do - @repo = OpenStruct.new(name: 'vim') - stub_client(valid?: true) - end - - it "assigns variables" do - @project = create(:project, import_type: 'google_code', creator_id: user.id) - stub_client(repos: [@repo], incompatible_repos: []) - - get :status - - expect(assigns(:already_added_projects)).to eq([@project]) - expect(assigns(:repos)).to eq([@repo]) - expect(assigns(:incompatible_repos)).to eq([]) - end - - it "does not show already added project" do - @project = create(:project, import_type: 'google_code', creator_id: user.id, import_source: 'vim') - stub_client(repos: [@repo], incompatible_repos: []) - - get :status - - expect(assigns(:already_added_projects)).to eq([@project]) - expect(assigns(:repos)).to eq([]) - end - - it "does not show any invalid projects" do - stub_client(repos: [], incompatible_repos: [@repo]) - - get :status - - expect(assigns(:repos)).to be_empty - expect(assigns(:incompatible_repos)).to eq([@repo]) - end - end - - describe "POST create" do - it_behaves_like 'project import rate limiter' - end -end diff --git a/spec/factories/sequences.rb b/spec/factories/sequences.rb index ca0804965df..4e66d7c125c 100644 --- a/spec/factories/sequences.rb +++ b/spec/factories/sequences.rb @@ -13,4 +13,5 @@ FactoryBot.define do sequence(:past_time) { |n| 4.hours.ago + (2 * n).seconds } sequence(:iid) sequence(:sha) { |n| Digest::SHA1.hexdigest("commit-like-#{n}") } + sequence(:variable) { |n| "var#{n}" } end diff --git a/spec/features/projects/active_tabs_spec.rb b/spec/features/projects/active_tabs_spec.rb index 349e5f5e177..8001ce0f454 100644 --- a/spec/features/projects/active_tabs_spec.rb +++ b/spec/features/projects/active_tabs_spec.rb @@ -124,15 +124,15 @@ RSpec.describe 'Project active tab' do context 'on project Analytics' do before do - visit charts_project_graph_path(project, 'master') + visit project_cycle_analytics_path(project) end - context 'on project Analytics/Repository Analytics' do + context 'on project Analytics/Value Stream Analytics' do it_behaves_like 'page has active tab', _('Analytics') - it_behaves_like 'page has active sub tab', _('Repository') + it_behaves_like 'page has active sub tab', _('Value Stream') end - context 'on project Analytics/Cycle Analytics' do + context 'on project Analytics/"CI / CD"' do before do click_tab(_('CI / CD')) end diff --git a/spec/features/projects/new_project_spec.rb b/spec/features/projects/new_project_spec.rb index 796fd76cfdf..4aabf040655 100644 --- a/spec/features/projects/new_project_spec.rb +++ b/spec/features/projects/new_project_spec.rb @@ -56,7 +56,6 @@ RSpec.describe 'New project', :js do expect(page).to have_link('GitHub') expect(page).to have_link('Bitbucket') expect(page).to have_link('GitLab.com') - expect(page).to have_link('Google Code') expect(page).to have_button('Repo by URL') expect(page).to have_link('GitLab export') end @@ -292,17 +291,6 @@ RSpec.describe 'New project', :js do end end - context 'from Google Code' do - before do - first('.import_google_code').click - end - - it 'shows import instructions' do - expect(page).to have_content('Import projects from Google Code') - expect(current_path).to eq new_import_google_code_path - end - end - context 'from manifest file' do before do first('.import_manifest').click diff --git a/spec/features/users/show_spec.rb b/spec/features/users/show_spec.rb index aebe2cc602d..be2d71aef90 100644 --- a/spec/features/users/show_spec.rb +++ b/spec/features/users/show_spec.rb @@ -7,7 +7,7 @@ RSpec.describe 'User page' do let_it_be(:user) { create(:user, bio: '**Lorem** _ipsum_ dolor sit [amet](https://example.com)') } - subject { visit(user_path(user)) } + subject(:visit_profile) { visit(user_path(user)) } context 'with public profile' do it 'shows all the tabs' do @@ -123,6 +123,32 @@ RSpec.describe 'User page' do end end + context 'with unconfirmed user' do + let_it_be(:user) { create(:user, :unconfirmed) } + + before do + visit_profile + end + + it 'shows user name as unconfirmed' do + expect(page).to have_css(".cover-title", text: 'Unconfirmed user') + end + + it 'shows no tab' do + expect(page).to have_css("div.profile-header") + expect(page).not_to have_css("ul.nav-links") + end + + it 'shows no additional fields' do + expect(page).not_to have_css(".profile-user-bio") + expect(page).not_to have_css(".profile-link-holder") + end + + it 'shows private profile message' do + expect(page).to have_content("This user has a private profile") + end + end + it 'shows the status if there was one' do create(:user_status, user: user, message: "Working hard!") diff --git a/spec/frontend/cycle_analytics/banner_spec.js b/spec/frontend/cycle_analytics/banner_spec.js index f0b8cb18a90..0cae0298cee 100644 --- a/spec/frontend/cycle_analytics/banner_spec.js +++ b/spec/frontend/cycle_analytics/banner_spec.js @@ -2,7 +2,7 @@ import Vue from 'vue'; import mountComponent from 'helpers/vue_mount_component_helper'; import banner from '~/cycle_analytics/components/banner.vue'; -describe('Cycle analytics banner', () => { +describe('Value Stream Analytics banner', () => { let vm; beforeEach(() => { diff --git a/spec/frontend/diffs/components/diff_content_spec.js b/spec/frontend/diffs/components/diff_content_spec.js index c0532e392d1..43d295ff1b3 100644 --- a/spec/frontend/diffs/components/diff_content_spec.js +++ b/spec/frontend/diffs/components/diff_content_spec.js @@ -6,7 +6,6 @@ import InlineDiffView from '~/diffs/components/inline_diff_view.vue'; import NotDiffableViewer from '~/vue_shared/components/diff_viewer/viewers/not_diffable.vue'; import NoPreviewViewer from '~/vue_shared/components/diff_viewer/viewers/no_preview.vue'; import ParallelDiffView from '~/diffs/components/parallel_diff_view.vue'; -import ImageDiffOverlay from '~/diffs/components/image_diff_overlay.vue'; import NoteForm from '~/notes/components/note_form.vue'; import DiffDiscussions from '~/diffs/components/diff_discussions.vue'; import { IMAGE_DIFF_POSITION_TYPE } from '~/diffs/constants'; @@ -166,14 +165,6 @@ describe('DiffContent', () => { describe('with image files', () => { const imageDiffFile = { ...defaultProps.diffFile, viewer: { name: diffViewerModes.image } }; - it('should have image diff view in place', () => { - getCommentFormForDiffFileGetterMock.mockReturnValue(() => true); - createComponent({ props: { diffFile: imageDiffFile } }); - - expect(wrapper.find(InlineDiffView).exists()).toBe(false); - expect(wrapper.find(ImageDiffOverlay).exists()).toBe(true); - }); - it('renders diff file discussions', () => { getCommentFormForDiffFileGetterMock.mockReturnValue(() => true); createComponent({ diff --git a/spec/frontend/diffs/components/image_diff_overlay_spec.js b/spec/frontend/diffs/components/image_diff_overlay_spec.js index 5a88a3cabd1..087715111b4 100644 --- a/spec/frontend/diffs/components/image_diff_overlay_spec.js +++ b/spec/frontend/diffs/components/image_diff_overlay_spec.js @@ -24,6 +24,8 @@ describe('Diffs image diff overlay component', () => { propsData: { discussions: [...imageDiffDiscussions], fileHash: 'ABC', + renderedWidth: 200, + renderedHeight: 200, ...props, }, methods: { @@ -71,8 +73,8 @@ describe('Diffs image diff overlay component', () => { createComponent(); const imageBadges = getAllImageBadges(); - expect(imageBadges.at(0).attributes('style')).toBe('left: 10px; top: 10px;'); - expect(imageBadges.at(1).attributes('style')).toBe('left: 5px; top: 5px;'); + expect(imageBadges.at(0).attributes('style')).toBe('left: 10%; top: 5%;'); + expect(imageBadges.at(1).attributes('style')).toBe('left: 5%; top: 2.5%;'); }); it('renders single badge for discussion object', () => { @@ -95,6 +97,8 @@ describe('Diffs image diff overlay component', () => { y: 0, width: 100, height: 200, + xPercent: 0, + yPercent: 0, }); }); @@ -120,11 +124,13 @@ describe('Diffs image diff overlay component', () => { describe('comment form', () => { const getCommentIndicator = () => wrapper.find('.comment-indicator'); beforeEach(() => { - createComponent({}, store => { + createComponent({ canComment: true }, store => { store.state.diffs.commentForms.push({ fileHash: 'ABC', x: 20, y: 10, + xPercent: 10, + yPercent: 10, }); }); }); @@ -134,7 +140,7 @@ describe('Diffs image diff overlay component', () => { }); it('sets comment form badge position', () => { - expect(getCommentIndicator().attributes('style')).toBe('left: 20px; top: 10px;'); + expect(getCommentIndicator().attributes('style')).toBe('left: 10%; top: 10%;'); }); }); }); diff --git a/spec/frontend/importer_status_spec.js b/spec/frontend/importer_status_spec.js deleted file mode 100644 index 4ef74a2fe84..00000000000 --- a/spec/frontend/importer_status_spec.js +++ /dev/null @@ -1,141 +0,0 @@ -import MockAdapter from 'axios-mock-adapter'; -import { ImporterStatus } from '~/importer_status'; -import axios from '~/lib/utils/axios_utils'; - -describe('Importer Status', () => { - let instance; - let mock; - - beforeEach(() => { - mock = new MockAdapter(axios); - }); - - afterEach(() => { - mock.restore(); - }); - - describe('addToImport', () => { - const importUrl = '/import_url'; - const fixtures = ` - <table> - <tr id="repo_123"> - <td class="import-target"></td> - <td class="import-actions job-status"> - <button name="button" type="submit" class="btn btn-import js-add-to-import"> - </button> - </td> - </tr> - </table> - `; - - beforeEach(() => { - setFixtures(fixtures); - jest.spyOn(ImporterStatus.prototype, 'initStatusPage').mockImplementation(() => {}); - jest.spyOn(ImporterStatus.prototype, 'setAutoUpdate').mockImplementation(() => {}); - instance = new ImporterStatus({ - jobsUrl: '', - importUrl, - }); - }); - - it('sets table row to active after post request', done => { - mock.onPost(importUrl).reply(200, { - id: 1, - full_path: '/full_path', - }); - - instance - .addToImport({ - currentTarget: document.querySelector('.js-add-to-import'), - }) - .then(() => { - expect(document.querySelector('tr').classList.contains('table-active')).toEqual(true); - done(); - }) - .catch(done.fail); - }); - - it('shows error message after failed POST request', done => { - setFixtures(`${fixtures}<div class="flash-container"></div>`); - - mock.onPost(importUrl).reply(422, { - errors: 'You forgot your lunch', - }); - - instance - .addToImport({ - currentTarget: document.querySelector('.js-add-to-import'), - }) - .then(() => { - const flashMessage = document.querySelector('.flash-text'); - - expect(flashMessage.textContent.trim()).toEqual( - 'An error occurred while importing project: You forgot your lunch', - ); - done(); - }) - .catch(done.fail); - }); - }); - - describe('autoUpdate', () => { - const jobsUrl = '/jobs_url'; - - beforeEach(() => { - const div = document.createElement('div'); - div.innerHTML = ` - <div id="project_1"> - <div class="job-status"> - </div> - </div> - `; - - document.body.appendChild(div); - - jest.spyOn(ImporterStatus.prototype, 'initStatusPage').mockImplementation(() => {}); - jest.spyOn(ImporterStatus.prototype, 'setAutoUpdate').mockImplementation(() => {}); - instance = new ImporterStatus({ - jobsUrl, - }); - }); - - function setupMock(importStatus) { - mock.onGet(jobsUrl).reply(200, [ - { - id: 1, - import_status: importStatus, - }, - ]); - } - - function expectJobStatus(done, status) { - instance - .autoUpdate() - .then(() => { - expect(document.querySelector('#project_1').innerText.trim()).toEqual(status); - done(); - }) - .catch(done.fail); - } - - it('sets the job status to done', done => { - setupMock('finished'); - expectJobStatus(done, 'Done'); - }); - - it('sets the job status to scheduled', done => { - setupMock('scheduled'); - expectJobStatus(done, 'Scheduled'); - }); - - it('sets the job status to started', done => { - setupMock('started'); - expectJobStatus(done, 'Started'); - }); - - it('sets the job status to custom status', done => { - setupMock('custom status'); - expectJobStatus(done, 'custom status'); - }); - }); -}); diff --git a/spec/frontend/vue_mr_widget/components/states/mr_widget_missing_branch_spec.js b/spec/frontend/vue_mr_widget/components/states/mr_widget_missing_branch_spec.js index 3f03ebdb047..f45368bf443 100644 --- a/spec/frontend/vue_mr_widget/components/states/mr_widget_missing_branch_spec.js +++ b/spec/frontend/vue_mr_widget/components/states/mr_widget_missing_branch_spec.js @@ -1,40 +1,46 @@ -import Vue from 'vue'; -import mountComponent from 'helpers/vue_mount_component_helper'; -import missingBranchComponent from '~/vue_merge_request_widget/components/states/mr_widget_missing_branch.vue'; - -describe('MRWidgetMissingBranch', () => { - let vm; - - beforeEach(() => { - const Component = Vue.extend(missingBranchComponent); - vm = mountComponent(Component, { mr: { sourceBranchRemoved: true } }); - }); - - afterEach(() => { - vm.$destroy(); +import { shallowMount } from '@vue/test-utils'; +import MissingBranchComponent from '~/vue_merge_request_widget/components/states/mr_widget_missing_branch.vue'; + +let wrapper; + +function factory(sourceBranchRemoved, mergeRequestWidgetGraphql) { + wrapper = shallowMount(MissingBranchComponent, { + propsData: { + mr: { sourceBranchRemoved }, + }, + provide: { + glFeatures: { mergeRequestWidgetGraphql }, + }, }); - describe('computed', () => { - describe('missingBranchName', () => { - it('should return proper branch name', () => { - expect(vm.missingBranchName).toEqual('source'); + if (mergeRequestWidgetGraphql) { + wrapper.setData({ state: { sourceBranchExists: !sourceBranchRemoved } }); + } - vm.mr.sourceBranchRemoved = false; + return wrapper.vm.$nextTick(); +} - expect(vm.missingBranchName).toEqual('target'); - }); - }); +describe('MRWidgetMissingBranch', () => { + afterEach(() => { + wrapper.destroy(); }); - describe('template', () => { - it('should have correct elements', () => { - const el = vm.$el; - const content = el.textContent.replace(/\n(\s)+/g, ' ').trim(); - - expect(el.classList.contains('mr-widget-body')).toBeTruthy(); - expect(el.querySelector('button').getAttribute('disabled')).toBeTruthy(); - expect(content.replace(/\s\s+/g, ' ')).toContain('source branch does not exist.'); - expect(content).toContain('Please restore it or use a different source branch'); + [true, false].forEach(mergeRequestWidgetGraphql => { + describe(`widget GraphQL feature flag is ${ + mergeRequestWidgetGraphql ? 'enabled' : 'disabled' + }`, () => { + it.each` + sourceBranchRemoved | branchName + ${true} | ${'source'} + ${false} | ${'target'} + `( + 'should set missing branch name as $branchName when sourceBranchRemoved is $sourceBranchRemoved', + async ({ sourceBranchRemoved, branchName }) => { + await factory(sourceBranchRemoved, mergeRequestWidgetGraphql); + + expect(wrapper.find('[data-testid="missingBranchName"]').text()).toContain(branchName); + }, + ); }); }); }); diff --git a/spec/helpers/users_helper_spec.rb b/spec/helpers/users_helper_spec.rb index 9ebbf975903..6d869c50041 100644 --- a/spec/helpers/users_helper_spec.rb +++ b/spec/helpers/users_helper_spec.rb @@ -272,4 +272,65 @@ RSpec.describe UsersHelper do end end end + + describe '#user_display_name' do + subject { helper.user_display_name(user) } + + before do + stub_current_user(nil) + end + + context 'for a confirmed user' do + let(:user) { create(:user) } + + before do + stub_profile_permission_allowed(true) + end + + it { is_expected.to eq(user.name) } + end + + context 'for an unconfirmed user' do + let(:user) { create(:user, :unconfirmed) } + + before do + stub_profile_permission_allowed(false) + end + + it { is_expected.to eq('Unconfirmed user') } + + context 'when current user is an admin' do + before do + admin_user = create(:admin) + stub_current_user(admin_user) + stub_profile_permission_allowed(true, admin_user) + end + + it { is_expected.to eq(user.name) } + end + + context 'when the current user is self' do + before do + stub_current_user(user) + stub_profile_permission_allowed(true, user) + end + + it { is_expected.to eq(user.name) } + end + end + + context 'for a blocked user' do + let(:user) { create(:user, :blocked) } + + it { is_expected.to eq('Blocked user') } + end + + def stub_current_user(user) + allow(helper).to receive(:current_user).and_return(user) + end + + def stub_profile_permission_allowed(allowed, current_user = nil) + allow(helper).to receive(:can?).with(user, :read_user_profile, current_user).and_return(allowed) + end + end end diff --git a/spec/lib/gitlab/ci/ansi2json/result_spec.rb b/spec/lib/gitlab/ci/ansi2json/result_spec.rb index 31c0da95f0a..b7b4d6de8b9 100644 --- a/spec/lib/gitlab/ci/ansi2json/result_spec.rb +++ b/spec/lib/gitlab/ci/ansi2json/result_spec.rb @@ -10,7 +10,7 @@ RSpec.describe Gitlab::Ci::Ansi2json::Result do { lines: [], state: state, append: false, truncated: false, offset: offset, stream: stream } end - subject { described_class.new(params) } + subject { described_class.new(**params) } describe '#size' do before do diff --git a/spec/lib/gitlab/ci/ansi2json/style_spec.rb b/spec/lib/gitlab/ci/ansi2json/style_spec.rb index d27a642ecf3..ff70ff69aaa 100644 --- a/spec/lib/gitlab/ci/ansi2json/style_spec.rb +++ b/spec/lib/gitlab/ci/ansi2json/style_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe Gitlab::Ci::Ansi2json::Style do describe '#set?' do - subject { described_class.new(params).set? } + subject { described_class.new(**params).set? } context 'when fg color is set' do let(:params) { { fg: 'term-fg-black' } } @@ -44,7 +44,7 @@ RSpec.describe Gitlab::Ci::Ansi2json::Style do end describe 'update formats to mimic terminals' do - subject { described_class.new(params) } + subject { described_class.new(**params) } context 'when fg color present' do let(:params) { { fg: 'term-fg-black', mask: mask } } diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb index 77b8aa1d591..efe99cd276c 100644 --- a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb +++ b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb @@ -142,7 +142,7 @@ RSpec.describe Gitlab::Ci::Build::Artifacts::Metadata do it 'reads expected number of entries' do stream = File.open(tmpfile.path) - metadata = described_class.new(stream, 'public', { recursive: true }) + metadata = described_class.new(stream, 'public', recursive: true) expect(metadata.find_entries!.count).to eq entry_count end diff --git a/spec/lib/gitlab/ci/config/entry/image_spec.rb b/spec/lib/gitlab/ci/config/entry/image_spec.rb index c3d91057328..e810d65d560 100644 --- a/spec/lib/gitlab/ci/config/entry/image_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/image_spec.rb @@ -81,7 +81,7 @@ RSpec.describe Gitlab::Ci::Config::Entry::Image do context 'when configuration has ports' do let(:ports) { [{ number: 80, protocol: 'http', name: 'foobar' }] } let(:config) { { name: 'ruby:2.7', entrypoint: %w(/bin/sh run), ports: ports } } - let(:entry) { described_class.new(config, { with_image_ports: image_ports }) } + let(:entry) { described_class.new(config, with_image_ports: image_ports) } let(:image_ports) { false } context 'when with_image_ports metadata is not enabled' do diff --git a/spec/lib/gitlab/ci/config/entry/need_spec.rb b/spec/lib/gitlab/ci/config/entry/need_spec.rb index 5a826bf8282..983e95fae42 100644 --- a/spec/lib/gitlab/ci/config/entry/need_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/need_spec.rb @@ -165,6 +165,45 @@ RSpec.describe ::Gitlab::Ci::Config::Entry::Need do end end + context 'with cross pipeline artifacts needs' do + context 'when pipeline is provided' do + context 'when job is provided' do + let(:config) { { job: 'job_name', pipeline: '$THE_PIPELINE_ID' } } + + it { is_expected.to be_valid } + + it 'sets artifacts:true by default' do + expect(need.value).to eq(job: 'job_name', pipeline: '$THE_PIPELINE_ID', artifacts: true) + end + + it 'sets the type as cross_dependency' do + expect(need.type).to eq(:cross_dependency) + end + end + + context 'when artifacts is provided' do + let(:config) { { job: 'job_name', pipeline: '$THE_PIPELINE_ID', artifacts: false } } + + it { is_expected.to be_valid } + + it 'returns the correct value' do + expect(need.value).to eq(job: 'job_name', pipeline: '$THE_PIPELINE_ID', artifacts: false) + end + end + end + + context 'when config contains not allowed keys' do + let(:config) { { job: 'job_name', pipeline: '$THE_PIPELINE_ID', something: 'else' } } + + it { is_expected.not_to be_valid } + + it 'returns an error' do + expect(need.errors) + .to contain_exactly('cross pipeline dependency config contains unknown keys: something') + end + end + end + context 'when need config is not a string or a hash' do let(:config) { :job_name } diff --git a/spec/lib/gitlab/ci/config/entry/needs_spec.rb b/spec/lib/gitlab/ci/config/entry/needs_spec.rb index f3b9d0c3c84..f11f2a56f5f 100644 --- a/spec/lib/gitlab/ci/config/entry/needs_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/needs_spec.rb @@ -6,7 +6,7 @@ RSpec.describe ::Gitlab::Ci::Config::Entry::Needs do subject(:needs) { described_class.new(config) } before do - needs.metadata[:allowed_needs] = %i[job] + needs.metadata[:allowed_needs] = %i[job cross_dependency] end describe 'validations' do @@ -66,6 +66,27 @@ RSpec.describe ::Gitlab::Ci::Config::Entry::Needs do end end end + + context 'with too many cross pipeline dependencies' do + let(:limit) { described_class::NEEDS_CROSS_PIPELINE_DEPENDENCIES_LIMIT } + + let(:config) do + Array.new(limit.next) do |index| + { pipeline: "$UPSTREAM_PIPELINE_#{index}", job: 'job-1' } + end + end + + describe '#valid?' do + it { is_expected.not_to be_valid } + end + + describe '#errors' do + it 'returns error about incorrect type' do + expect(needs.errors).to contain_exactly( + "needs config must be less than or equal to #{limit}") + end + end + end end describe '.compose!' do diff --git a/spec/lib/gitlab/ci/config/entry/service_spec.rb b/spec/lib/gitlab/ci/config/entry/service_spec.rb index ec137ef2ae4..2795cc9dddf 100644 --- a/spec/lib/gitlab/ci/config/entry/service_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/service_spec.rb @@ -96,7 +96,7 @@ RSpec.describe Gitlab::Ci::Config::Entry::Service do { name: 'postgresql:9.5', alias: 'db', command: %w(cmd run), entrypoint: %w(/bin/sh run), ports: ports } end - let(:entry) { described_class.new(config, { with_image_ports: image_ports }) } + let(:entry) { described_class.new(config, with_image_ports: image_ports) } let(:image_ports) { false } context 'when with_image_ports metadata is not enabled' do diff --git a/spec/lib/gitlab/ci/config/entry/services_spec.rb b/spec/lib/gitlab/ci/config/entry/services_spec.rb index e4f8a348d21..85e7f297b03 100644 --- a/spec/lib/gitlab/ci/config/entry/services_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/services_spec.rb @@ -38,7 +38,7 @@ RSpec.describe Gitlab::Ci::Config::Entry::Services do context 'when configuration has ports' do let(:ports) { [{ number: 80, protocol: 'http', name: 'foobar' }] } let(:config) { ['postgresql:9.5', { name: 'postgresql:9.1', alias: 'postgres_old', ports: ports }] } - let(:entry) { described_class.new(config, { with_image_ports: image_ports }) } + let(:entry) { described_class.new(config, with_image_ports: image_ports) } let(:image_ports) { false } context 'when with_image_ports metadata is not enabled' do diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index fb6395e888a..411337fc32f 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -2111,6 +2111,71 @@ module Gitlab end end + describe 'cross pipeline needs' do + context 'when configuration is valid' do + let(:config) do + <<~YAML + rspec: + stage: test + script: rspec + needs: + - pipeline: $THE_PIPELINE_ID + job: dependency-job + YAML + end + + it 'returns a valid configuration and sets artifacts: true by default' do + expect(subject).to be_valid + + rspec = subject.build_attributes(:rspec) + expect(rspec.dig(:options, :cross_dependencies)).to eq( + [{ pipeline: '$THE_PIPELINE_ID', job: 'dependency-job', artifacts: true }] + ) + end + + context 'when pipeline ID is hard-coded' do + let(:config) do + <<~YAML + rspec: + stage: test + script: rspec + needs: + - pipeline: "123" + job: dependency-job + YAML + end + + it 'returns a valid configuration and sets artifacts: true by default' do + expect(subject).to be_valid + + rspec = subject.build_attributes(:rspec) + expect(rspec.dig(:options, :cross_dependencies)).to eq( + [{ pipeline: '123', job: 'dependency-job', artifacts: true }] + ) + end + end + end + + context 'when configuration is not valid' do + let(:config) do + <<~YAML + rspec: + stage: test + script: rspec + needs: + - pipeline: $THE_PIPELINE_ID + job: dependency-job + something: else + YAML + end + + it 'returns an error' do + expect(subject).not_to be_valid + expect(subject.errors).to include(/:need config contains unknown keys: something/) + end + end + end + describe "Hidden jobs" do let(:config_processor) { Gitlab::Ci::YamlProcessor.new(config).execute } diff --git a/spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb b/spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb index 719d4a69985..21503dc1501 100644 --- a/spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb @@ -11,7 +11,7 @@ RSpec.describe Gitlab::CycleAnalytics::StageSummary do project.add_maintainer(user) end - let(:stage_summary) { described_class.new(project, options).data } + let(:stage_summary) { described_class.new(project, **options).data } describe "#new_issues" do subject { stage_summary.first } @@ -121,7 +121,7 @@ RSpec.describe Gitlab::CycleAnalytics::StageSummary do end it 'does not include commit stats' do - data = described_class.new(project, options).data + data = described_class.new(project, **options).data expect(includes_commits?(data)).to be_falsy end diff --git a/spec/lib/gitlab/email/reply_parser_spec.rb b/spec/lib/gitlab/email/reply_parser_spec.rb index 575ff7f357b..bc4c6cf007d 100644 --- a/spec/lib/gitlab/email/reply_parser_spec.rb +++ b/spec/lib/gitlab/email/reply_parser_spec.rb @@ -6,7 +6,7 @@ require "spec_helper" RSpec.describe Gitlab::Email::ReplyParser do describe '#execute' do def test_parse_body(mail_string, params = {}) - described_class.new(Mail::Message.new(mail_string), params).execute + described_class.new(Mail::Message.new(mail_string), **params).execute end it "returns an empty string if the message is blank" do diff --git a/spec/lib/gitlab/google_code_import/client_spec.rb b/spec/lib/gitlab/google_code_import/client_spec.rb deleted file mode 100644 index 402d2169432..00000000000 --- a/spec/lib/gitlab/google_code_import/client_spec.rb +++ /dev/null @@ -1,38 +0,0 @@ -# frozen_string_literal: true - -require "spec_helper" - -RSpec.describe Gitlab::GoogleCodeImport::Client do - let(:raw_data) { Gitlab::Json.parse(fixture_file("GoogleCodeProjectHosting.json")) } - - subject { described_class.new(raw_data) } - - describe "#valid?" do - context "when the data is valid" do - it "returns true" do - expect(subject).to be_valid - end - end - - context "when the data is invalid" do - let(:raw_data) { "No clue" } - - it "returns true" do - expect(subject).not_to be_valid - end - end - end - - describe "#repos" do - it "returns only Git repositories" do - expect(subject.repos.length).to eq(1) - expect(subject.incompatible_repos.length).to eq(1) - end - end - - describe "#repo" do - it "returns the referenced repository" do - expect(subject.repo("tint2").name).to eq("tint2") - end - end -end diff --git a/spec/lib/gitlab/google_code_import/importer_spec.rb b/spec/lib/gitlab/google_code_import/importer_spec.rb deleted file mode 100644 index a22e80ae1c0..00000000000 --- a/spec/lib/gitlab/google_code_import/importer_spec.rb +++ /dev/null @@ -1,88 +0,0 @@ -# frozen_string_literal: true - -require "spec_helper" - -RSpec.describe Gitlab::GoogleCodeImport::Importer do - let(:mapped_user) { create(:user, username: "thilo123") } - let(:raw_data) { Gitlab::Json.parse(fixture_file("GoogleCodeProjectHosting.json")) } - let(:client) { Gitlab::GoogleCodeImport::Client.new(raw_data) } - let(:import_data) do - { - 'repo' => client.repo('tint2').raw_data, - 'user_map' => { 'thilo...' => "@#{mapped_user.username}" } - } - end - - let(:project) { create(:project) } - - subject { described_class.new(project) } - - before do - project.add_maintainer(project.creator) - project.create_import_data(data: import_data) - end - - describe "#execute" do - it "imports status labels" do - subject.execute - - %w(New NeedInfo Accepted Wishlist Started Fixed Invalid Duplicate WontFix Incomplete).each do |status| - expect(project.labels.find_by(name: "Status: #{status}")).not_to be_nil - end - end - - it "imports labels" do - subject.execute - - %w( - Type-Defect Type-Enhancement Type-Task Type-Review Type-Other Milestone-0.12 Priority-Critical - Priority-High Priority-Medium Priority-Low OpSys-All OpSys-Windows OpSys-Linux OpSys-OSX Security - Performance Usability Maintainability Component-Panel Component-Taskbar Component-Battery - Component-Systray Component-Clock Component-Launcher Component-Tint2conf Component-Docs Component-New - ).each do |label| - label = label.sub("-", ": ") - expect(project.labels.find_by(name: label)).not_to be_nil - end - end - - it "imports issues" do - subject.execute - - issue = project.issues.first - expect(issue).not_to be_nil - expect(issue.iid).to eq(169) - expect(issue.author).to eq(project.creator) - expect(issue.assignees).to eq([mapped_user]) - expect(issue.state).to eq("closed") - expect(issue.label_names).to include("Priority: Medium") - expect(issue.label_names).to include("Status: Fixed") - expect(issue.label_names).to include("Type: Enhancement") - expect(issue.title).to eq("Scrolling through tasks") - expect(issue.state).to eq("closed") - expect(issue.description).to include("schattenpr\\.\\.\\.") - expect(issue.description).to include("November 18, 2009 00:20") - expect(issue.description).to include("Google Code") - expect(issue.description).to include('I like to scroll through the tasks with my scrollwheel (like in fluxbox).') - expect(issue.description).to include('Patch is attached that adds two new mouse-actions (next_task+prev_task)') - expect(issue.description).to include('that can be used for exactly that purpose.') - expect(issue.description).to include('all the best!') - expect(issue.description).to include('[tint2_task_scrolling.diff](https://storage.googleapis.com/google-code-attachments/tint2/issue-169/comment-0/tint2_task_scrolling.diff)') - expect(issue.description).to include('![screenshot.png](https://storage.googleapis.com/google-code-attachments/tint2/issue-169/comment-0/screenshot.png)') - expect(issue.description).to include('![screenshot1.PNG](https://storage.googleapis.com/google-code-attachments/tint2/issue-169/comment-0/screenshot1.PNG)') - end - - it "imports issue comments" do - subject.execute - - note = project.issues.first.notes.first - expect(note).not_to be_nil - expect(note.note).to include("Comment 1") - expect(note.note).to include("@#{mapped_user.username}") - expect(note.note).to include("November 18, 2009 05:14") - expect(note.note).to include("applied, thanks.") - expect(note.note).to include("Status: Fixed") - expect(note.note).to include("~~Type: Defect~~") - expect(note.note).to include("Type: Enhancement") - end - end -end diff --git a/spec/lib/gitlab/google_code_import/project_creator_spec.rb b/spec/lib/gitlab/google_code_import/project_creator_spec.rb deleted file mode 100644 index cfebe57aed3..00000000000 --- a/spec/lib/gitlab/google_code_import/project_creator_spec.rb +++ /dev/null @@ -1,32 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::GoogleCodeImport::ProjectCreator do - let(:user) { create(:user) } - let(:repo) do - Gitlab::GoogleCodeImport::Repository.new( - "name" => 'vim', - "summary" => 'VI Improved', - "repositoryUrls" => ["https://vim.googlecode.com/git/"] - ) - end - - let(:namespace) { create(:group) } - - before do - namespace.add_owner(user) - end - - it 'creates project' do - expect_next_instance_of(Project) do |project| - expect(project).to receive(:add_import_job) - end - - project_creator = described_class.new(repo, namespace, user) - project = project_creator.execute - - expect(project.import_url).to eq("https://vim.googlecode.com/git/") - expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::PUBLIC) - end -end diff --git a/spec/lib/gitlab/graphql/pagination/externally_paginated_array_connection_spec.rb b/spec/lib/gitlab/graphql/pagination/externally_paginated_array_connection_spec.rb index 84e8f8b95e8..d2475d1edb9 100644 --- a/spec/lib/gitlab/graphql/pagination/externally_paginated_array_connection_spec.rb +++ b/spec/lib/gitlab/graphql/pagination/externally_paginated_array_connection_spec.rb @@ -10,7 +10,7 @@ RSpec.describe Gitlab::Graphql::Pagination::ExternallyPaginatedArrayConnection d let(:arguments) { {} } subject(:connection) do - described_class.new(all_nodes, { max_page_size: values.size }.merge(arguments)) + described_class.new(all_nodes, **{ max_page_size: values.size }.merge(arguments)) end it_behaves_like 'a connection with collection methods' diff --git a/spec/lib/gitlab/graphql/pagination/keyset/connection_spec.rb b/spec/lib/gitlab/graphql/pagination/keyset/connection_spec.rb index 1fee24bdc1f..0ac54a20fcc 100644 --- a/spec/lib/gitlab/graphql/pagination/keyset/connection_spec.rb +++ b/spec/lib/gitlab/graphql/pagination/keyset/connection_spec.rb @@ -10,11 +10,11 @@ RSpec.describe Gitlab::Graphql::Pagination::Keyset::Connection do let(:context) { GraphQL::Query::Context.new(query: OpenStruct.new(schema: schema), values: nil, object: nil) } subject(:connection) do - described_class.new(nodes, { context: context, max_page_size: 3 }.merge(arguments)) + described_class.new(nodes, **{ context: context, max_page_size: 3 }.merge(arguments)) end def encoded_cursor(node) - described_class.new(nodes, { context: context }).cursor_for(node) + described_class.new(nodes, context: context).cursor_for(node) end def decoded_cursor(cursor) diff --git a/spec/lib/gitlab/import_sources_spec.rb b/spec/lib/gitlab/import_sources_spec.rb index 0dfd8a2ee50..b0bc1029a53 100644 --- a/spec/lib/gitlab/import_sources_spec.rb +++ b/spec/lib/gitlab/import_sources_spec.rb @@ -11,7 +11,6 @@ RSpec.describe Gitlab::ImportSources do 'Bitbucket Cloud' => 'bitbucket', 'Bitbucket Server' => 'bitbucket_server', 'GitLab.com' => 'gitlab', - 'Google Code' => 'google_code', 'FogBugz' => 'fogbugz', 'Repo by URL' => 'git', 'GitLab export' => 'gitlab_project', @@ -32,7 +31,6 @@ RSpec.describe Gitlab::ImportSources do bitbucket bitbucket_server gitlab - google_code fogbugz git gitlab_project @@ -53,7 +51,6 @@ RSpec.describe Gitlab::ImportSources do bitbucket bitbucket_server gitlab - google_code fogbugz gitlab_project gitea @@ -70,7 +67,6 @@ RSpec.describe Gitlab::ImportSources do 'bitbucket' => Gitlab::BitbucketImport::Importer, 'bitbucket_server' => Gitlab::BitbucketServerImport::Importer, 'gitlab' => Gitlab::GitlabImport::Importer, - 'google_code' => Gitlab::GoogleCodeImport::Importer, 'fogbugz' => Gitlab::FogbugzImport::Importer, 'git' => nil, 'gitlab_project' => Gitlab::ImportExport::Importer, @@ -92,7 +88,6 @@ RSpec.describe Gitlab::ImportSources do 'bitbucket' => 'Bitbucket Cloud', 'bitbucket_server' => 'Bitbucket Server', 'gitlab' => 'GitLab.com', - 'google_code' => 'Google Code', 'fogbugz' => 'FogBugz', 'git' => 'Repo by URL', 'gitlab_project' => 'GitLab export', diff --git a/spec/models/ci/build_dependencies_spec.rb b/spec/models/ci/build_dependencies_spec.rb index 4fa1b3eb5a5..623320370b1 100644 --- a/spec/models/ci/build_dependencies_spec.rb +++ b/spec/models/ci/build_dependencies_spec.rb @@ -155,9 +155,9 @@ RSpec.describe Ci::BuildDependencies do subject { dependencies.all } - it 'returns the union of all local dependencies and any cross pipeline dependencies' do + it 'returns the union of all local dependencies and any cross project dependencies' do expect(dependencies).to receive(:local).and_return([1, 2, 3]) - expect(dependencies).to receive(:cross_pipeline).and_return([3, 4]) + expect(dependencies).to receive(:cross_project).and_return([3, 4]) expect(subject).to contain_exactly(1, 2, 3, 4) end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index b4da57ed89b..26f8271a180 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -3024,6 +3024,17 @@ RSpec.describe Project, factory_default: :keep do expect { project.set_repository_read_only! }.to raise_error(described_class::RepositoryReadOnlyError, /in progress/) end + + context 'skip_git_transfer_check is true' do + it 'makes the project read-only when git transfers are in progress' do + allow(project).to receive(:git_transfer_in_progress?) { true } + + expect { project.set_repository_read_only!(skip_git_transfer_check: true) } + .to change(project, :repository_read_only?) + .from(false) + .to(true) + end + end end describe '#set_repository_writable!' do diff --git a/spec/policies/user_policy_spec.rb b/spec/policies/user_policy_spec.rb index 17ac7d0e44d..78212f06526 100644 --- a/spec/policies/user_policy_spec.rb +++ b/spec/policies/user_policy_spec.rb @@ -160,4 +160,16 @@ RSpec.describe UserPolicy do it { is_expected.not_to be_allowed(:read_group_count) } end end + + describe ':read_user_profile' do + context 'when the user is unconfirmed' do + let(:user) { create(:user, :unconfirmed) } + + it { is_expected.not_to be_allowed(:read_user_profile) } + end + + context 'when the user is confirmed' do + it { is_expected.to be_allowed(:read_user_profile) } + end + end end diff --git a/spec/requests/api/project_repository_storage_moves_spec.rb b/spec/requests/api/project_repository_storage_moves_spec.rb index ecf4c75b52f..923cd158c98 100644 --- a/spec/requests/api/project_repository_storage_moves_spec.rb +++ b/spec/requests/api/project_repository_storage_moves_spec.rb @@ -6,7 +6,7 @@ RSpec.describe API::ProjectRepositoryStorageMoves do include AccessMatchersForRequest let_it_be(:user) { create(:admin) } - let_it_be(:project) { create(:project) } + let_it_be(:project) { create(:project, :repository).tap { |project| project.track_project_repository } } let_it_be(:storage_move) { create(:project_repository_storage_move, :scheduled, project: project) } shared_examples 'get single project repository storage move' do @@ -159,4 +159,64 @@ RSpec.describe API::ProjectRepositoryStorageMoves do end end end + + describe 'POST /project_repository_storage_moves' do + let(:source_storage_name) { 'default' } + let(:destination_storage_name) { 'test_second_storage' } + + def create_project_repository_storage_moves + post api('/project_repository_storage_moves', user), params: { + source_storage_name: source_storage_name, + destination_storage_name: destination_storage_name + } + end + + before do + stub_storage_settings('test_second_storage' => { 'path' => 'tmp/tests/extra_storage' }) + end + + it 'schedules the worker' do + expect(ProjectScheduleBulkRepositoryShardMovesWorker).to receive(:perform_async).with(source_storage_name, destination_storage_name) + + create_project_repository_storage_moves + + expect(response).to have_gitlab_http_status(:accepted) + end + + context 'source_storage_name is invalid' do + let(:destination_storage_name) { 'not-a-real-storage' } + + it 'gives an error' do + create_project_repository_storage_moves + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + + context 'destination_storage_name is missing' do + let(:destination_storage_name) { nil } + + it 'schedules the worker' do + expect(ProjectScheduleBulkRepositoryShardMovesWorker).to receive(:perform_async).with(source_storage_name, destination_storage_name) + + create_project_repository_storage_moves + + expect(response).to have_gitlab_http_status(:accepted) + end + end + + context 'destination_storage_name is invalid' do + let(:destination_storage_name) { 'not-a-real-storage' } + + it 'gives an error' do + create_project_repository_storage_moves + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + + describe 'normal user' do + it { expect { create_project_repository_storage_moves }.to be_denied_for(:user) } + end + end end diff --git a/spec/routing/import_routing_spec.rb b/spec/routing/import_routing_spec.rb index 15d2f32de78..b1da2eaa33b 100644 --- a/spec/routing/import_routing_spec.rb +++ b/spec/routing/import_routing_spec.rb @@ -126,32 +126,6 @@ RSpec.describe Import::BitbucketServerController, 'routing' do end end -# status_import_google_code GET /import/google_code/status(.:format) import/google_code#status -# callback_import_google_code POST /import/google_code/callback(.:format) import/google_code#callback -# jobs_import_google_code GET /import/google_code/jobs(.:format) import/google_code#jobs -# new_user_map_import_google_code GET /import/google_code/user_map(.:format) import/google_code#new_user_map -# create_user_map_import_google_code POST /import/google_code/user_map(.:format) import/google_code#create_user_map -# import_google_code POST /import/google_code(.:format) import/google_code#create -# new_import_google_code GET /import/google_code/new(.:format) import/google_code#new -RSpec.describe Import::GoogleCodeController, 'routing' do - it_behaves_like 'importer routing' do - let(:except_actions) { [:callback] } - let(:provider) { 'google_code' } - end - - it 'to #callback' do - expect(post("/import/google_code/callback")).to route_to("import/google_code#callback") - end - - it 'to #new_user_map' do - expect(get('/import/google_code/user_map')).to route_to('import/google_code#new_user_map') - end - - it 'to #create_user_map' do - expect(post('/import/google_code/user_map')).to route_to('import/google_code#create_user_map') - end -end - # status_import_fogbugz GET /import/fogbugz/status(.:format) import/fogbugz#status # callback_import_fogbugz POST /import/fogbugz/callback(.:format) import/fogbugz#callback # realtime_changes_import_fogbugz GET /import/fogbugz/realtime_changes(.:format) import/fogbugz#realtime_changes diff --git a/spec/rubocop/cop/lint/last_keyword_argument_spec.rb b/spec/rubocop/cop/lint/last_keyword_argument_spec.rb index f942390569b..5822bf74e8d 100644 --- a/spec/rubocop/cop/lint/last_keyword_argument_spec.rb +++ b/spec/rubocop/cop/lint/last_keyword_argument_spec.rb @@ -46,6 +46,9 @@ RSpec.describe RuboCop::Cop::Lint::LastKeywordArgument, type: :rubocop do --- test_api/projects_get_/projects_when_unauthenticated_behaves_like_projects_response_returns_an_array_of_projects: - | + DEPRECATION WARNING: /Users/tkuah/code/ee-gdk/gitlab/projects_spec.rb:1: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call + /Users/tkuah/code/ee-gdk/gitlab/lib/gitlab/project.rb:15: warning: The called method `initialize' is defined here + - | DEPRECATION WARNING: /Users/tkuah/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/state_machines-activerecord-0.6.0/lib/state_machines/integrations/active_record.rb:511: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call /Users/tkuah/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/activerecord-6.0.3.3/lib/active_record/suppressor.rb:43: warning: The called method `save' is defined here - | @@ -70,6 +73,17 @@ RSpec.describe RuboCop::Cop::Lint::LastKeywordArgument, type: :rubocop do SOURCE end + it 'registers an offense for the new method call' do + expect_offense(<<~SOURCE, 'projects_spec.rb') + Project.new(params) + ^^^^^^ Using the last argument as keyword parameters is deprecated + SOURCE + + expect_correction(<<~SOURCE) + Project.new(**params) + SOURCE + end + it 'registers an offense and corrects by converting hash to kwarg' do expect_offense(<<~SOURCE, 'create_service.rb') users.call(id, { a: :b, c: :d }) diff --git a/spec/services/ci/create_job_artifacts_service_spec.rb b/spec/services/ci/create_job_artifacts_service_spec.rb index 72b0d220b11..29e51a23dea 100644 --- a/spec/services/ci/create_job_artifacts_service_spec.rb +++ b/spec/services/ci/create_job_artifacts_service_spec.rb @@ -24,7 +24,7 @@ RSpec.describe Ci::CreateJobArtifactsService do upload = Tempfile.new('upload') FileUtils.copy(path, upload.path) - UploadedFile.new(upload.path, params) + UploadedFile.new(upload.path, **params) end describe '#execute' do diff --git a/spec/services/projects/schedule_bulk_repository_shard_moves_service_spec.rb b/spec/services/projects/schedule_bulk_repository_shard_moves_service_spec.rb new file mode 100644 index 00000000000..5b76386bfab --- /dev/null +++ b/spec/services/projects/schedule_bulk_repository_shard_moves_service_spec.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::ScheduleBulkRepositoryShardMovesService do + before do + stub_storage_settings('test_second_storage' => { 'path' => 'tmp/tests/extra_storage' }) + end + + let!(:project) { create(:project, :repository).tap { |project| project.track_project_repository } } + let(:source_storage_name) { 'default' } + let(:destination_storage_name) { 'test_second_storage' } + + describe '#execute' do + it 'schedules project repository storage moves' do + expect { subject.execute(source_storage_name, destination_storage_name) } + .to change(ProjectRepositoryStorageMove, :count).by(1) + + storage_move = project.repository_storage_moves.last! + + expect(storage_move).to have_attributes( + source_storage_name: source_storage_name, + destination_storage_name: destination_storage_name, + state_name: :scheduled + ) + end + + context 'read-only repository' do + let!(:project) { create(:project, :repository, :read_only).tap { |project| project.track_project_repository } } + + it 'does not get scheduled' do + expect(subject).to receive(:log_info) + .with("Project #{project.full_path} (#{project.id}) was skipped: Project is read only") + expect { subject.execute(source_storage_name, destination_storage_name) } + .to change(ProjectRepositoryStorageMove, :count).by(0) + end + end + end + + describe '.enqueue' do + it 'defers to the worker' do + expect(::ProjectScheduleBulkRepositoryShardMovesWorker).to receive(:perform_async).with(source_storage_name, destination_storage_name) + + described_class.enqueue(source_storage_name, destination_storage_name) + end + end +end diff --git a/spec/support/caching.rb b/spec/support/caching.rb index 883d531550a..11e4f534971 100644 --- a/spec/support/caching.rb +++ b/spec/support/caching.rb @@ -25,7 +25,7 @@ RSpec.configure do |config| original_null_store = Rails.cache caching_config_hash = Gitlab::Redis::Cache.params caching_config_hash[:namespace] = Gitlab::Redis::Cache::CACHE_NAMESPACE - Rails.cache = ActiveSupport::Cache::RedisCacheStore.new(caching_config_hash) + Rails.cache = ActiveSupport::Cache::RedisCacheStore.new(**caching_config_hash) redis_cache_cleanup! diff --git a/spec/support/graphql/var.rb b/spec/support/graphql/var.rb new file mode 100644 index 00000000000..4f2c774e898 --- /dev/null +++ b/spec/support/graphql/var.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +module Graphql + # Helper to pass variables around generated queries. + # + # e.g.: + # first = var('Int') + # after = var('String') + # + # query = with_signature( + # [first, after], + # query_graphql_path([ + # [:project, { full_path: project.full_path }], + # [:issues, { after: after, first: first }] + # :nodes + # ], all_graphql_fields_for('Issue')) + # ) + # + # post_graphql(query, variables: [first.with(2), after.with(some_cursor)]) + # + class Var + attr_reader :name, :type + attr_accessor :value + + def initialize(name, type) + @name = name + @type = type + end + + def sig + "#{to_graphql_value}: #{type}" + end + + def to_graphql_value + "$#{name}" + end + + # We return a new object so that running the same query twice with + # different values does not risk re-using the value + # + # e.g. + # + # x = var('Int') + # expect { post_graphql(query, variables: x) } + # .to issue_same_number_of_queries_as { post_graphql(query, variables: x.with(1)) } + # + # Here we post the `x` variable once with the value set to 1, and once with + # the value set to `nil`. + def with(value) + copy = Var.new(name, type) + copy.value = value + copy + end + + def to_h + { name => value } + end + end +end diff --git a/spec/support/helpers/gitlab_verify_helpers.rb b/spec/support/helpers/gitlab_verify_helpers.rb index 9901ce374ed..2a6ba8aaff4 100644 --- a/spec/support/helpers/gitlab_verify_helpers.rb +++ b/spec/support/helpers/gitlab_verify_helpers.rb @@ -2,7 +2,7 @@ module GitlabVerifyHelpers def collect_ranges(args = {}) - verifier = described_class.new(args.merge(batch_size: 1)) + verifier = described_class.new(**args.merge(batch_size: 1)) collect_results(verifier).map { |range, _| range } end diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb index 9b6505cc649..87c1e63feb6 100644 --- a/spec/support/helpers/graphql_helpers.rb +++ b/spec/support/helpers/graphql_helpers.rb @@ -4,6 +4,7 @@ module GraphqlHelpers MutationDefinition = Struct.new(:query, :variables) NoData = Class.new(StandardError) + UnauthorizedObject = Class.new(StandardError) # makes an underscored string look like a fieldname # "merge_request" => "mergeRequest" @@ -17,7 +18,10 @@ module GraphqlHelpers # ready, then the early return is returned instead. # # Then the resolve method is called. - def resolve(resolver_class, args: {}, **resolver_args) + def resolve(resolver_class, args: {}, lookahead: :not_given, parent: :not_given, **resolver_args) + args = aliased_args(resolver_class, args) + args[:parent] = parent unless parent == :not_given + args[:lookahead] = lookahead unless lookahead == :not_given resolver = resolver_instance(resolver_class, **resolver_args) ready, early_return = sync_all { resolver.ready?(**args) } @@ -26,6 +30,15 @@ module GraphqlHelpers resolver.resolve(**args) end + # TODO: Remove this method entirely when GraphqlHelpers uses real resolve_field + def aliased_args(resolver, args) + definitions = resolver.arguments + + args.transform_keys do |k| + definitions[GraphqlHelpers.fieldnamerize(k)]&.keyword || k + end + end + def resolver_instance(resolver_class, obj: nil, ctx: {}, field: nil, schema: GitlabSchema) if ctx.is_a?(Hash) q = double('Query', schema: schema) @@ -111,24 +124,25 @@ module GraphqlHelpers def variables_for_mutation(name, input) graphql_input = prepare_input_for_mutation(input) - result = { input_variable_name_for_mutation(name) => graphql_input } + { input_variable_name_for_mutation(name) => graphql_input } + end - # Avoid trying to serialize multipart data into JSON - if graphql_input.values.none? { |value| io_value?(value) } - result.to_json - else - result - end + def serialize_variables(variables) + return unless variables + return variables if variables.is_a?(String) + + ::Gitlab::Utils::MergeHash.merge(Array.wrap(variables).map(&:to_h)).to_json end - def resolve_field(name, object, args = {}) - context = double("Context", - schema: GitlabSchema, - query: GraphQL::Query.new(GitlabSchema), - parent: nil) - field = described_class.fields[::GraphqlHelpers.fieldnamerize(name)] + def resolve_field(name, object, args = {}, current_user: nil) + q = GraphQL::Query.new(GitlabSchema) + context = GraphQL::Query::Context.new(query: q, object: object, values: { current_user: current_user }) + allow(context).to receive(:parent).and_return(nil) + field = described_class.fields.fetch(GraphqlHelpers.fieldnamerize(name)) instance = described_class.authorized_new(object, context) - field.resolve_field(instance, {}, context) + raise UnauthorizedObject unless instance + + field.resolve_field(instance, args, context) end # Recursively convert a Hash with Ruby-style keys to GraphQL fieldname-style keys @@ -165,10 +179,32 @@ module GraphqlHelpers end def query_graphql_field(name, attributes = {}, fields = nil) - <<~QUERY - #{field_with_params(name, attributes)} - #{wrap_fields(fields || all_graphql_fields_for(name.to_s.classify))} - QUERY + attributes, fields = [nil, attributes] if fields.nil? && !attributes.is_a?(Hash) + + field = field_with_params(name, attributes) + + field + wrap_fields(fields || all_graphql_fields_for(name.to_s.classify)).to_s + end + + def page_info_selection + "pageInfo { hasNextPage hasPreviousPage endCursor startCursor }" + end + + def query_nodes(name, fields = nil, args: nil, of: name, include_pagination_info: false, max_depth: 1) + fields ||= all_graphql_fields_for(of.to_s.classify, max_depth: max_depth) + node_selection = include_pagination_info ? "#{page_info_selection} nodes" : :nodes + query_graphql_path([[name, args], node_selection], fields) + end + + # e.g: + # query_graphql_path(%i[foo bar baz], all_graphql_fields_for('Baz')) + # => foo { bar { baz { x y z } } } + def query_graphql_path(segments, fields = nil) + # we really want foldr here... + segments.reverse.reduce(fields) do |tail, segment| + name, args = Array.wrap(segment) + query_graphql_field(name, args, tail) + end end def wrap_fields(fields) @@ -233,6 +269,14 @@ module GraphqlHelpers end.join(", ") end + def with_signature(variables, query) + %Q[query(#{variables.map(&:sig).join(', ')}) #{query}] + end + + def var(type) + ::Graphql::Var.new(generate(:variable), type) + end + # Fairly dumb Ruby => GraphQL rendering function. Only suitable for testing. # Use symbol for Enum values def as_graphql_literal(value) @@ -245,7 +289,12 @@ module GraphqlHelpers when nil then 'null' when true then 'true' when false then 'false' - else raise ArgumentError, "Cannot represent #{value} as GraphQL literal" + else + if value.respond_to?(:to_graphql_value) + value.to_graphql_value + else + raise ArgumentError, "Cannot represent #{value} as GraphQL literal" + end end end @@ -254,7 +303,7 @@ module GraphqlHelpers end def post_graphql(query, current_user: nil, variables: nil, headers: {}) - params = { query: query, variables: variables&.to_json } + params = { query: query, variables: serialize_variables(variables) } post api('/', current_user, version: 'graphql'), params: params, headers: headers end @@ -332,13 +381,19 @@ module GraphqlHelpers graphql_dig_at(graphql_data, *path) end + # Slightly more powerful than just `dig`: + # - also supports implicit flat-mapping (.e.g. :foo :nodes :bar :nodes) def graphql_dig_at(data, *path) keys = path.map { |segment| segment.is_a?(Integer) ? segment : GraphqlHelpers.fieldnamerize(segment) } # Allows for array indexing, like this # ['project', 'boards', 'edges', 0, 'node', 'lists'] keys.reduce(data) do |memo, key| - memo.is_a?(Array) ? memo[key] : memo&.dig(key) + if memo.is_a?(Array) + key.is_a?(Integer) ? memo[key] : memo.flat_map { |e| Array.wrap(e[key]) } + else + memo&.dig(key) + end end end @@ -498,6 +553,20 @@ module GraphqlHelpers variables: {} ) end + + # A lookahead that selects everything + def positive_lookahead + double(selects?: true).tap do |selection| + allow(selection).to receive(:selection).and_return(selection) + end + end + + # A lookahead that selects nothing + def negative_lookahead + double(selects?: false).tap do |selection| + allow(selection).to receive(:selection).and_return(selection) + end + end end # This warms our schema, doing this as part of loading the helpers to avoid diff --git a/spec/support/shared_examples/models/concerns/shardable_shared_examples.rb b/spec/support/shared_examples/models/concerns/shardable_shared_examples.rb index fa929d5b791..fd0639b628e 100644 --- a/spec/support/shared_examples/models/concerns/shardable_shared_examples.rb +++ b/spec/support/shared_examples/models/concerns/shardable_shared_examples.rb @@ -18,4 +18,10 @@ RSpec.shared_examples 'shardable scopes' do expect(described_class.excluding_repository_storage('default')).to eq([record_2]) end end + + describe '.for_shard' do + it 'returns the objects for a given shard' do + expect(described_class.for_shard(record_1.shard)).to eq([record_1]) + end + end end diff --git a/spec/support_specs/graphql/var_spec.rb b/spec/support_specs/graphql/var_spec.rb new file mode 100644 index 00000000000..f708f01a11e --- /dev/null +++ b/spec/support_specs/graphql/var_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::Graphql::Var do + subject(:var) { described_class.new('foo', 'Int') } + + it 'associates a name with a type and an initially empty value' do + expect(var).to have_attributes( + name: 'foo', + type: 'Int', + value: be_nil + ) + end + + it 'has a correct signature' do + expect(var).to have_attributes(sig: '$foo: Int') + end + + it 'implements to_graphql_value as $name' do + expect(var.to_graphql_value).to eq('$foo') + end + + it 'can set a value using with, returning a new object' do + with_value = var.with(42) + + expect(with_value).to have_attributes(name: 'foo', type: 'Int', value: 42) + expect(var).to have_attributes(value: be_nil) + end + + it 'returns an object suitable for passing to post_graphql(variables:)' do + expect(var.with(17).to_h).to eq('foo' => 17) + end +end diff --git a/spec/support_specs/helpers/graphql_helpers_spec.rb b/spec/support_specs/helpers/graphql_helpers_spec.rb index bc777621674..47e5192cdc9 100644 --- a/spec/support_specs/helpers/graphql_helpers_spec.rb +++ b/spec/support_specs/helpers/graphql_helpers_spec.rb @@ -5,6 +5,223 @@ require 'spec_helper' RSpec.describe GraphqlHelpers do include GraphqlHelpers + # Normalize irrelevant whitespace to make comparison easier + def norm(query) + query.tr("\n", ' ').gsub(/\s+/, ' ').strip + end + + describe 'graphql_dig_at' do + it 'transforms symbol keys to graphql field names' do + data = { 'camelCased' => 'names' } + + expect(graphql_dig_at(data, :camel_cased)).to eq('names') + end + + it 'supports integer indexing' do + data = { 'array' => [:boom, { 'id' => :hooray! }, :boom] } + + expect(graphql_dig_at(data, :array, 1, :id)).to eq(:hooray!) + end + + it 'gracefully degrades to nil' do + data = { 'project' => { 'mergeRequest' => nil } } + + expect(graphql_dig_at(data, :project, :merge_request, :id)).to be_nil + end + + it 'supports implicitly flat-mapping traversals' do + data = { + 'foo' => { + 'nodes' => [ + { 'bar' => { 'nodes' => [{ 'id' => 1 }, { 'id' => 2 }] } }, + { 'bar' => { 'nodes' => [{ 'id' => 3 }, { 'id' => 4 }] } }, + { 'bar' => nil } + ] + }, + 'irrelevant' => 'the field is a red-herring' + } + + expect(graphql_dig_at(data, :foo, :nodes, :bar, :nodes, :id)).to eq([1, 2, 3, 4]) + end + end + + describe 'var' do + it 'allocates a fresh name for each var' do + a = var('Int') + b = var('Int') + + expect(a.name).not_to eq(b.name) + end + + it 'can be used to construct correct signatures' do + a = var('Int') + b = var('String!') + + q = with_signature([a, b], '{ foo bar }') + + expect(q).to eq("query(#{a.to_graphql_value}: Int, #{b.to_graphql_value}: String!) { foo bar }") + end + + it 'can be used to pass arguments to fields' do + a = var('ID!') + + q = graphql_query_for(:project, { full_path: a }, :id) + + expect(norm(q)).to eq("{ project(fullPath: #{a.to_graphql_value}){ id } }") + end + + it 'can associate values with variables' do + a = var('Int') + + expect(a.with(3).to_h).to eq(a.name => 3) + end + + it 'does not mutate the variable when providing a value' do + a = var('Int') + three = a.with(3) + + expect(three.value).to eq(3) + expect(a.value).to be_nil + end + + it 'can associate many values with variables' do + a = var('Int').with(3) + b = var('String').with('foo') + + expect(serialize_variables([a, b])).to eq({ a.name => 3, b.name => 'foo' }.to_json) + end + end + + describe '.query_nodes' do + it 'can produce a basic connection selection' do + selection = query_nodes(:users) + + expected = query_graphql_path([:users, :nodes], all_graphql_fields_for('User', max_depth: 1)) + + expect(selection).to eq(expected) + end + + it 'allows greater depth' do + selection = query_nodes(:users, max_depth: 2) + + expected = query_graphql_path([:users, :nodes], all_graphql_fields_for('User', max_depth: 2)) + + expect(selection).to eq(expected) + end + + it 'accepts fields' do + selection = query_nodes(:users, :id) + + expected = query_graphql_path([:users, :nodes], :id) + + expect(selection).to eq(expected) + end + + it 'accepts arguments' do + args = { username: 'foo' } + selection = query_nodes(:users, args: args) + + expected = query_graphql_path([[:users, args], :nodes], all_graphql_fields_for('User', max_depth: 1)) + + expect(selection).to eq(expected) + end + + it 'accepts arguments and fields' do + selection = query_nodes(:users, :id, args: { username: 'foo' }) + + expected = query_graphql_path([[:users, { username: 'foo' }], :nodes], :id) + + expect(selection).to eq(expected) + end + + it 'accepts explicit type name' do + selection = query_nodes(:members, of: 'User') + + expected = query_graphql_path([:members, :nodes], all_graphql_fields_for('User', max_depth: 1)) + + expect(selection).to eq(expected) + end + + it 'can optionally provide pagination info' do + selection = query_nodes(:users, include_pagination_info: true) + + expected = query_graphql_path([:users, "#{page_info_selection} nodes"], all_graphql_fields_for('User', max_depth: 1)) + + expect(selection).to eq(expected) + end + end + + describe '.query_graphql_path' do + it 'can build nested paths' do + selection = query_graphql_path(%i[foo bar wibble_wobble], :id) + + expected = norm(<<-GQL) + foo{ + bar{ + wibbleWobble{ + id + } + } + } + GQL + + expect(norm(selection)).to eq(expected) + end + + it 'can insert arguments at any point' do + selection = query_graphql_path( + [:foo, [:bar, { quux: true }], [:wibble_wobble, { eccentricity: :HIGH }]], + :id + ) + + expected = norm(<<-GQL) + foo{ + bar(quux: true){ + wibbleWobble(eccentricity: HIGH){ + id + } + } + } + GQL + + expect(norm(selection)).to eq(expected) + end + end + + describe '.attributes_to_graphql' do + it 'can serialize hashes to literal arguments' do + x = var('Int') + args = { + an_array: [1, nil, "foo", true, [:foo, :bar]], + a_hash: { + nested: true, + value: "bar" + }, + an_int: 42, + a_float: 0.1, + a_string: "wibble", + an_enum: :LOW, + null: nil, + a_bool: false, + a_var: x + } + + literal = attributes_to_graphql(args) + + expect(norm(literal)).to eq(norm(<<~EXP)) + anArray: [1,null,"foo",true,[foo,bar]], + aHash: {nested: true, value: "bar"}, + anInt: 42, + aFloat: 0.1, + aString: "wibble", + anEnum: LOW, + null: null, + aBool: false, + aVar: #{x.to_graphql_value} + EXP + end + end + describe '.graphql_mutation' do shared_examples 'correct mutation definition' do it 'returns correct mutation definition' do @@ -15,7 +232,7 @@ RSpec.describe GraphqlHelpers do } } MUTATION - variables = %q({"updateAlertStatusInput":{"projectPath":"test/project"}}) + variables = { "updateAlertStatusInput" => { "projectPath" => "test/project" } } is_expected.to eq(GraphqlHelpers::MutationDefinition.new(query, variables)) end diff --git a/spec/workers/project_schedule_bulk_repository_shard_moves_worker_spec.rb b/spec/workers/project_schedule_bulk_repository_shard_moves_worker_spec.rb new file mode 100644 index 00000000000..aadfae51906 --- /dev/null +++ b/spec/workers/project_schedule_bulk_repository_shard_moves_worker_spec.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ProjectScheduleBulkRepositoryShardMovesWorker do + describe "#perform" do + before do + stub_storage_settings('test_second_storage' => { 'path' => 'tmp/tests/extra_storage' }) + + allow(ProjectUpdateRepositoryStorageWorker).to receive(:perform_async) + end + + let!(:project) { create(:project, :repository).tap { |project| project.track_project_repository } } + let(:source_storage_name) { 'default' } + let(:destination_storage_name) { 'test_second_storage' } + + include_examples 'an idempotent worker' do + let(:job_args) { [source_storage_name, destination_storage_name] } + + it 'schedules project repository storage moves' do + expect { subject }.to change(ProjectRepositoryStorageMove, :count).by(1) + + storage_move = project.repository_storage_moves.last! + + expect(storage_move).to have_attributes( + source_storage_name: source_storage_name, + destination_storage_name: destination_storage_name, + state_name: :scheduled + ) + end + end + end +end |