diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-11-09 12:07:42 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-11-09 12:07:42 +0300 |
commit | 44d4b37b52c678a0b6a3c18c8c87319553ce84a3 (patch) | |
tree | d09bcd1aad83fe5a4d596b32356bb260eb54aca2 /spec | |
parent | 7b29a4f84e25ab3eb610c1595bad38478784f5ff (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
24 files changed, 395 insertions, 248 deletions
diff --git a/spec/controllers/projects/learn_gitlab_controller_spec.rb b/spec/controllers/projects/learn_gitlab_controller_spec.rb index 2d00fcbccf3..a93da82d948 100644 --- a/spec/controllers/projects/learn_gitlab_controller_spec.rb +++ b/spec/controllers/projects/learn_gitlab_controller_spec.rb @@ -34,8 +34,15 @@ RSpec.describe Projects::LearnGitlabController do it { is_expected.to have_gitlab_http_status(:not_found) } end - it_behaves_like 'tracks assignment and records the subject', :invite_for_help_continuous_onboarding, :namespace do - subject { project.namespace } + context 'with invite_for_help_continuous_onboarding experiment' do + it 'tracks the assignment', :experiment do + stub_experiments(invite_for_help_continuous_onboarding: true) + + expect(experiment(:invite_for_help_continuous_onboarding)) + .to track(:assignment).with_context(namespace: project.namespace).on_next_instance + + action + end end end end diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index 527cba9e618..f66e4b133ca 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -299,14 +299,15 @@ RSpec.describe Projects::PipelinesController do stub_application_setting(auto_devops_enabled: false) end - def action - get :index, params: { namespace_id: project.namespace, project_id: project } - end + context 'with runners_availability_section experiment' do + it 'tracks the assignment', :experiment do + stub_experiments(runners_availability_section: true) - subject { project.namespace } + expect(experiment(:runners_availability_section)) + .to track(:assignment).with_context(namespace: project.namespace).on_next_instance - context 'runners_availability_section experiment' do - it_behaves_like 'tracks assignment and records the subject', :runners_availability_section, :namespace + get :index, params: { namespace_id: project.namespace, project_id: project } + end end end diff --git a/spec/experiments/application_experiment_spec.rb b/spec/experiments/application_experiment_spec.rb index b144e6f77d2..7aca5e492f4 100644 --- a/spec/experiments/application_experiment_spec.rb +++ b/spec/experiments/application_experiment_spec.rb @@ -43,72 +43,6 @@ RSpec.describe ApplicationExperiment, :experiment do variant: 'control' ) end - - describe '#publish_to_database' do - using RSpec::Parameterized::TableSyntax - - let(:publish_to_database) { ActiveSupport::Deprecation.silence { application_experiment.publish_to_database } } - - shared_examples 'does not record to the database' do - it 'does not create an experiment record' do - expect { publish_to_database }.not_to change(Experiment, :count) - end - - it 'does not create an experiment subject record' do - expect { publish_to_database }.not_to change(ExperimentSubject, :count) - end - end - - context 'when there is a usable subject' do - let(:context) { { context_key => context_value } } - - where(:context_key, :context_value, :object_type) do - :namespace | build(:namespace, id: non_existing_record_id) | :namespace - :group | build(:namespace, id: non_existing_record_id) | :namespace - :project | build(:project, id: non_existing_record_id) | :project - :user | build(:user, id: non_existing_record_id) | :user - :actor | build(:user, id: non_existing_record_id) | :user - end - - with_them do - it 'creates an experiment and experiment subject record' do - expect { publish_to_database }.to change(Experiment, :count).by(1) - - expect(Experiment.last.name).to eq('namespaced/stub') - expect(ExperimentSubject.last.send(object_type)).to eq(context[context_key]) - end - end - end - - context "when experiment hasn't ran" do - let(:context) { { user: create(:user) } } - - it 'sets a variant on the experiment subject' do - publish_to_database - - expect(ExperimentSubject.last.variant).to eq('control') - end - end - - context 'when there is not a usable subject' do - let(:context) { { context_key => context_value } } - - where(:context_key, :context_value) do - :namespace | nil - :foo | :bar - end - - with_them do - include_examples 'does not record to the database' - end - end - - context 'but we should not track' do - let(:should_track) { false } - - include_examples 'does not record to the database' - end - end end describe "#track", :snowplow do diff --git a/spec/experiments/require_verification_for_namespace_creation_experiment_spec.rb b/spec/experiments/require_verification_for_namespace_creation_experiment_spec.rb index 269b6222020..c91a8f1950e 100644 --- a/spec/experiments/require_verification_for_namespace_creation_experiment_spec.rb +++ b/spec/experiments/require_verification_for_namespace_creation_experiment_spec.rb @@ -30,34 +30,6 @@ RSpec.describe RequireVerificationForNamespaceCreationExperiment, :experiment do end end - describe '#record_conversion' do - let_it_be(:namespace) { create(:namespace) } - - context 'when should_track? is false' do - before do - allow(experiment).to receive(:should_track?).and_return(false) - end - - it 'does not record a conversion event' do - expect(experiment.publish_to_database).to be_nil - expect(experiment.record_conversion(namespace)).to be_nil - end - end - - context 'when should_track? is true' do - before do - allow(experiment).to receive(:should_track?).and_return(true) - end - - it 'records a conversion event' do - experiment_subject = experiment.publish_to_database - - expect { experiment.record_conversion(namespace) }.to change { experiment_subject.reload.converted_at }.from(nil) - .and change { experiment_subject.context }.to include('namespace_id' => namespace.id) - end - end - end - describe 'exclusions' do context 'when user is new' do it 'is not excluded' do diff --git a/spec/experiments/security_reports_mr_widget_prompt_experiment_spec.rb b/spec/experiments/security_reports_mr_widget_prompt_experiment_spec.rb index 4328ff12d42..ee02fa5f1f2 100644 --- a/spec/experiments/security_reports_mr_widget_prompt_experiment_spec.rb +++ b/spec/experiments/security_reports_mr_widget_prompt_experiment_spec.rb @@ -6,10 +6,4 @@ RSpec.describe SecurityReportsMrWidgetPromptExperiment do it "defines a control and candidate" do expect(subject.behaviors.keys).to match_array(%w[control candidate]) end - - it "publishes to the database" do - expect(subject).to receive(:publish_to_database) - - subject.publish - end end diff --git a/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb b/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb index a385e8a5fd0..c9efda7822d 100644 --- a/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb +++ b/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb @@ -86,7 +86,7 @@ RSpec.describe 'Resolving all open threads in a merge request from an issue', :j expect(page).to have_link 'Create issue to resolve all threads', href: new_project_issue_path(project, merge_request_to_resolve_discussions_of: merge_request.iid) end - context 'creating an issue for threads' do + context 'creating an issue for threads', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/381729' do before do page.within '.mr-state-widget' do page.click_link 'Create issue to resolve all threads', href: new_project_issue_path(project, merge_request_to_resolve_discussions_of: merge_request.iid) diff --git a/spec/frontend/boards/board_card_inner_spec.js b/spec/frontend/boards/board_card_inner_spec.js index 3b77c999a9e..d05e057095d 100644 --- a/spec/frontend/boards/board_card_inner_spec.js +++ b/spec/frontend/boards/board_card_inner_spec.js @@ -52,7 +52,7 @@ describe('Board card component', () => { const performSearchMock = jest.fn(); - const createStore = ({ isEpicBoard = false, isProjectBoard = false } = {}) => { + const createStore = ({ isProjectBoard = false } = {}) => { store = new Vuex.Store({ ...defaultStore, actions: { @@ -65,13 +65,12 @@ describe('Board card component', () => { }, getters: { isGroupBoard: () => true, - isEpicBoard: () => isEpicBoard, isProjectBoard: () => isProjectBoard, }, }); }; - const createWrapper = (props = {}) => { + const createWrapper = ({ props = {}, isEpicBoard = false } = {}) => { wrapper = mountExtended(BoardCardInner, { store, propsData: { @@ -97,6 +96,7 @@ describe('Board card component', () => { provide: { rootPath: '/', scopedLabelsAvailable: false, + isEpicBoard, }, }); }; @@ -111,7 +111,7 @@ describe('Board card component', () => { }; createStore(); - createWrapper({ item: issue, list }); + createWrapper({ props: { item: issue, list } }); }); afterEach(() => { @@ -146,7 +146,7 @@ describe('Board card component', () => { }); it('renders the work type icon when props is passed', () => { - createWrapper({ item: issue, list, showWorkItemTypeIcon: true }); + createWrapper({ props: { item: issue, list, showWorkItemTypeIcon: true } }); expect(findWorkItemIcon().exists()).toBe(true); expect(findWorkItemIcon().props('workItemType')).toBe(issue.type); }); @@ -177,9 +177,11 @@ describe('Board card component', () => { describe('blocked', () => { it('renders blocked icon if issue is blocked', async () => { createWrapper({ - item: { - ...issue, - blocked: true, + props: { + item: { + ...issue, + blocked: true, + }, }, }); @@ -188,9 +190,11 @@ describe('Board card component', () => { it('does not show blocked icon if issue is not blocked', () => { createWrapper({ - item: { - ...issue, - blocked: false, + props: { + item: { + ...issue, + blocked: false, + }, }, }); @@ -201,9 +205,11 @@ describe('Board card component', () => { describe('confidential issue', () => { beforeEach(() => { createWrapper({ - item: { - ...wrapper.props('item'), - confidential: true, + props: { + item: { + ...wrapper.props('item'), + confidential: true, + }, }, }); }); @@ -216,9 +222,11 @@ describe('Board card component', () => { describe('hidden issue', () => { beforeEach(() => { createWrapper({ - item: { - ...wrapper.props('item'), - hidden: true, + props: { + item: { + ...wrapper.props('item'), + hidden: true, + }, }, }); }); @@ -241,11 +249,13 @@ describe('Board card component', () => { describe('with avatar', () => { beforeEach(() => { createWrapper({ - item: { - ...wrapper.props('item'), - assignees: [user], - updateData(newData) { - Object.assign(this, newData); + props: { + item: { + ...wrapper.props('item'), + assignees: [user], + updateData(newData) { + Object.assign(this, newData); + }, }, }, }); @@ -294,15 +304,17 @@ describe('Board card component', () => { global.gon.default_avatar_url = 'default_avatar'; createWrapper({ - item: { - ...wrapper.props('item'), - assignees: [ - { - id: 1, - name: 'testing 123', - username: 'test', - }, - ], + props: { + item: { + ...wrapper.props('item'), + assignees: [ + { + id: 1, + name: 'testing 123', + username: 'test', + }, + ], + }, }, }); }); @@ -323,28 +335,30 @@ describe('Board card component', () => { describe('multiple assignees', () => { beforeEach(() => { createWrapper({ - item: { - ...wrapper.props('item'), - assignees: [ - { - id: 2, - name: 'user2', - username: 'user2', - avatarUrl: 'test_image', - }, - { - id: 3, - name: 'user3', - username: 'user3', - avatarUrl: 'test_image', - }, - { - id: 4, - name: 'user4', - username: 'user4', - avatarUrl: 'test_image', - }, - ], + props: { + item: { + ...wrapper.props('item'), + assignees: [ + { + id: 2, + name: 'user2', + username: 'user2', + avatarUrl: 'test_image', + }, + { + id: 3, + name: 'user3', + username: 'user3', + avatarUrl: 'test_image', + }, + { + id: 4, + name: 'user4', + username: 'user4', + avatarUrl: 'test_image', + }, + ], + }, }, }); }); @@ -364,9 +378,11 @@ describe('Board card component', () => { }); createWrapper({ - item: { - ...wrapper.props('item'), - assignees, + props: { + item: { + ...wrapper.props('item'), + assignees, + }, }, }); }); @@ -390,9 +406,11 @@ describe('Board card component', () => { })), ]; createWrapper({ - item: { - ...wrapper.props('item'), - assignees, + props: { + item: { + ...wrapper.props('item'), + assignees, + }, }, }); @@ -405,7 +423,7 @@ describe('Board card component', () => { describe('labels', () => { beforeEach(() => { - createWrapper({ item: { ...issue, labels: [list.label, label1] } }); + createWrapper({ props: { item: { ...issue, labels: [list.label, label1] } } }); }); it('does not render list label but renders all other labels', () => { @@ -417,7 +435,7 @@ describe('Board card component', () => { }); it('does not render label if label does not have an ID', async () => { - createWrapper({ item: { ...issue, labels: [label1, { title: 'closed' }] } }); + createWrapper({ props: { item: { ...issue, labels: [label1, { title: 'closed' }] } } }); await nextTick(); @@ -429,11 +447,13 @@ describe('Board card component', () => { describe('filterByLabel method', () => { beforeEach(() => { createWrapper({ - item: { - ...issue, - labels: [label1], + props: { + item: { + ...issue, + labels: [label1], + }, + updateFilters: true, }, - updateFilters: true, }); }); @@ -480,9 +500,11 @@ describe('Board card component', () => { describe('loading', () => { it('renders loading icon', async () => { createWrapper({ - item: { - ...issue, - isLoading: true, + props: { + item: { + ...issue, + isLoading: true, + }, }, }); @@ -504,17 +526,20 @@ describe('Board card component', () => { }; beforeEach(() => { - createStore({ isEpicBoard: true }); + createStore(); }); it('should render if the item has issues', () => { createWrapper({ - item: { - ...issue, - descendantCounts, - descendantWeightSum, - hasIssues: true, + props: { + item: { + ...issue, + descendantCounts, + descendantWeightSum, + hasIssues: true, + }, }, + isEpicBoard: true, }); expect(findEpicCountables().exists()).toBe(true); @@ -535,18 +560,21 @@ describe('Board card component', () => { it('shows render item countBadge, weights, and progress correctly', () => { createWrapper({ - item: { - ...issue, - descendantCounts: { - ...descendantCounts, - openedIssues: 1, - }, - descendantWeightSum: { - closedIssues: 10, - openedIssues: 5, + props: { + item: { + ...issue, + descendantCounts: { + ...descendantCounts, + openedIssues: 1, + }, + descendantWeightSum: { + closedIssues: 10, + openedIssues: 5, + }, + hasIssues: true, }, - hasIssues: true, }, + isEpicBoard: true, }); expect(findEpicCountablesBadgeIssues().text()).toBe('1'); @@ -556,15 +584,18 @@ describe('Board card component', () => { it('does not render progress when weight is zero', () => { createWrapper({ - item: { - ...issue, - descendantCounts: { - ...descendantCounts, - openedIssues: 1, + props: { + item: { + ...issue, + descendantCounts: { + ...descendantCounts, + openedIssues: 1, + }, + descendantWeightSum, + hasIssues: true, }, - descendantWeightSum, - hasIssues: true, }, + isEpicBoard: true, }); expect(findEpicBadgeProgress().exists()).toBe(false); @@ -572,15 +603,18 @@ describe('Board card component', () => { it('renders the tooltip with the correct data', () => { createWrapper({ - item: { - ...issue, - descendantCounts, - descendantWeightSum: { - closedIssues: 10, - openedIssues: 5, + props: { + item: { + ...issue, + descendantCounts, + descendantWeightSum: { + closedIssues: 10, + openedIssues: 5, + }, + hasIssues: true, }, - hasIssues: true, }, + isEpicBoard: true, }); const tooltip = findEpicCountablesTotalTooltip(); diff --git a/spec/frontend/boards/board_list_helper.js b/spec/frontend/boards/board_list_helper.js index 65a41c49e7f..c5c3faf1712 100644 --- a/spec/frontend/boards/board_list_helper.js +++ b/spec/frontend/boards/board_list_helper.js @@ -101,6 +101,8 @@ export default function createComponent({ weightFeatureAvailable: false, boardWeight: null, canAdminList: true, + isIssueBoard: true, + isEpicBoard: false, ...provide, }, stubs, diff --git a/spec/frontend/boards/components/board_add_new_column_spec.js b/spec/frontend/boards/components/board_add_new_column_spec.js index 5fae1c4359f..a3b2988ce75 100644 --- a/spec/frontend/boards/components/board_add_new_column_spec.js +++ b/spec/frontend/boards/components/board_add_new_column_spec.js @@ -53,11 +53,11 @@ describe('Board card layout', () => { state: { labels, labelsLoading: false, - isEpicBoard: false, }, }), provide: { scopedLabelsAvailable: true, + isEpicBoard: false, }, }), ); diff --git a/spec/frontend/boards/components/board_card_spec.js b/spec/frontend/boards/components/board_card_spec.js index 2feaa5dff8c..38b79e2e3f3 100644 --- a/spec/frontend/boards/components/board_card_spec.js +++ b/spec/frontend/boards/components/board_card_spec.js @@ -30,7 +30,6 @@ describe('Board card', () => { }, actions: mockActions, getters: { - isEpicBoard: () => false, isProjectBoard: () => false, }, }); @@ -61,6 +60,7 @@ describe('Board card', () => { groupId: null, rootPath: '/', scopedLabelsAvailable: false, + isEpicBoard: false, ...provide, }, }); diff --git a/spec/frontend/boards/components/board_content_spec.js b/spec/frontend/boards/components/board_content_spec.js index bf491029c41..b2138700602 100644 --- a/spec/frontend/boards/components/board_content_spec.js +++ b/spec/frontend/boards/components/board_content_spec.js @@ -47,6 +47,8 @@ describe('BoardContent', () => { canAdminList = true, isApolloBoard = false, issuableType = 'issue', + isIssueBoard = true, + isEpicBoard = false, boardListQueryHandler = jest.fn().mockResolvedValue(boardListsQueryResponse), } = {}) => { fakeApollo = createMockApollo([[boardListsQuery, boardListQueryHandler]]); @@ -67,6 +69,8 @@ describe('BoardContent', () => { boardType: 'group', fullPath: 'gitlab-org/gitlab', issuableType, + isIssueBoard, + isEpicBoard, isApolloBoard, }, store, @@ -133,7 +137,7 @@ describe('BoardContent', () => { describe('when issuableType is not issue', () => { beforeEach(() => { - createComponent({ issuableType: 'foo' }); + createComponent({ issuableType: 'foo', isIssueBoard: false }); }); it('does not render BoardContentSidebar', () => { diff --git a/spec/frontend/boards/components/board_list_header_spec.js b/spec/frontend/boards/components/board_list_header_spec.js index 50901f3fe84..4633612891c 100644 --- a/spec/frontend/boards/components/board_list_header_spec.js +++ b/spec/frontend/boards/components/board_list_header_spec.js @@ -59,7 +59,6 @@ describe('Board List Header Component', () => { store = new Vuex.Store({ state: {}, actions: { updateList: updateListSpy, toggleListCollapsed: toggleListCollapsedSpy }, - getters: { isEpicBoard: () => false }, }); fakeApollo = createMockApollo([[listQuery, listQueryHandler]]); @@ -76,6 +75,7 @@ describe('Board List Header Component', () => { boardId, weightFeatureAvailable: false, currentUserId, + isEpicBoard: false, }, }), ); diff --git a/spec/frontend/boards/components/board_settings_sidebar_spec.js b/spec/frontend/boards/components/board_settings_sidebar_spec.js index 4171a6236de..7d602042685 100644 --- a/spec/frontend/boards/components/board_settings_sidebar_spec.js +++ b/spec/frontend/boards/components/board_settings_sidebar_spec.js @@ -45,6 +45,7 @@ describe('BoardSettingsSidebar', () => { provide: { canAdminList, scopedLabelsAvailable: false, + isIssueBoard: true, }, directives: { GlModal: createMockDirective(), diff --git a/spec/frontend/boards/components/board_top_bar_spec.js b/spec/frontend/boards/components/board_top_bar_spec.js index 997768a0cc7..08b5042f70f 100644 --- a/spec/frontend/boards/components/board_top_bar_spec.js +++ b/spec/frontend/boards/components/board_top_bar_spec.js @@ -15,18 +15,14 @@ describe('BoardTopBar', () => { Vue.use(Vuex); - const createStore = ({ mockGetters = {} } = {}) => { + const createStore = () => { return new Vuex.Store({ state: {}, - getters: { - isEpicBoard: () => false, - ...mockGetters, - }, }); }; - const createComponent = ({ provide = {}, mockGetters = {} } = {}) => { - const store = createStore({ mockGetters }); + const createComponent = ({ provide = {} } = {}) => { + const store = createStore(); wrapper = shallowMount(BoardTopBar, { store, provide: { @@ -36,6 +32,7 @@ describe('BoardTopBar', () => { fullPath: 'gitlab-org', boardType: 'group', releasesFetchPath: '/releases', + isIssueBoard: true, ...provide, }, stubs: { IssueBoardFilteredSearch }, diff --git a/spec/lib/gitlab/background_migration/populate_projects_star_count_spec.rb b/spec/lib/gitlab/background_migration/populate_projects_star_count_spec.rb new file mode 100644 index 00000000000..74f674e052d --- /dev/null +++ b/spec/lib/gitlab/background_migration/populate_projects_star_count_spec.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::PopulateProjectsStarCount, schema: 20221019105041 do + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:users) { table(:users) } + let(:users_star_projects) { table(:users_star_projects) } + + let(:namespace1) { namespaces.create!(name: 'namespace 1', path: 'namespace1') } + let(:namespace2) { namespaces.create!(name: 'namespace 2', path: 'namespace2') } + let(:namespace3) { namespaces.create!(name: 'namespace 3', path: 'namespace3') } + let(:namespace4) { namespaces.create!(name: 'namespace 4', path: 'namespace4') } + let(:namespace5) { namespaces.create!(name: 'namespace 5', path: 'namespace5') } + + let(:project1) { projects.create!(namespace_id: namespace1.id, project_namespace_id: namespace1.id) } + let(:project2) { projects.create!(namespace_id: namespace2.id, project_namespace_id: namespace2.id) } + let(:project3) { projects.create!(namespace_id: namespace3.id, project_namespace_id: namespace3.id) } + let(:project4) { projects.create!(namespace_id: namespace4.id, project_namespace_id: namespace4.id) } + let(:project5) { projects.create!(namespace_id: namespace5.id, project_namespace_id: namespace5.id) } + + let(:user_active) { users.create!(state: 'active', email: 'test1@example.com', projects_limit: 5) } + let(:user_blocked) { users.create!(state: 'blocked', email: 'test2@example.com', projects_limit: 5) } + + let(:migration) do + described_class.new( + start_id: project1.id, + end_id: project4.id, + batch_table: :projects, + batch_column: :id, + sub_batch_size: 2, + pause_ms: 2, + connection: ApplicationRecord.connection + ) + end + + subject(:perform_migration) { migration.perform } + + it 'correctly populates the star counters' do + users_star_projects.create!(project_id: project1.id, user_id: user_active.id) + users_star_projects.create!(project_id: project2.id, user_id: user_blocked.id) + users_star_projects.create!(project_id: project4.id, user_id: user_active.id) + users_star_projects.create!(project_id: project4.id, user_id: user_blocked.id) + users_star_projects.create!(project_id: project5.id, user_id: user_active.id) + + perform_migration + + expect(project1.reload.star_count).to eq(1) + expect(project2.reload.star_count).to eq(0) + expect(project3.reload.star_count).to eq(0) + expect(project4.reload.star_count).to eq(1) + expect(project5.reload.star_count).to eq(0) + end + + context 'when database timeouts' do + using RSpec::Parameterized::TableSyntax + + where(error_class: [ActiveRecord::StatementTimeout, ActiveRecord::QueryCanceled]) + + with_them do + it 'retries on timeout error' do + expect(migration).to receive(:update_batch).exactly(3).times.and_raise(error_class) + expect(migration).to receive(:sleep).with(5).twice + + expect do + perform_migration + end.to raise_error(error_class) + end + end + end +end diff --git a/spec/migrations/queue_populate_projects_star_count_spec.rb b/spec/migrations/queue_populate_projects_star_count_spec.rb new file mode 100644 index 00000000000..848136d8005 --- /dev/null +++ b/spec/migrations/queue_populate_projects_star_count_spec.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe QueuePopulateProjectsStarCount do + let_it_be(:batched_migration) { described_class::MIGRATION } + + it 'schedules a new batched migration' do + reversible_migration do |migration| + migration.before -> { + expect(batched_migration).not_to have_scheduled_batched_migration + } + + migration.after -> { + expect(batched_migration).to have_scheduled_batched_migration( + table_name: :projects, + column_name: :id, + interval: described_class::DELAY_INTERVAL + ) + } + end + end +end diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index 7d068235f18..daa65f528e9 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -388,16 +388,31 @@ RSpec.describe Deployment do end context 'when deployment is behind current deployment' do + let_it_be(:commits) { project.repository.commits('master', limit: 2) } + let!(:deployment) do - create(:deployment, :success, project: project, environment: environment, finished_at: 1.year.ago) + create(:deployment, :success, project: project, environment: environment, + finished_at: 1.year.ago, sha: commits[0].sha) end let!(:last_deployment) do - create(:deployment, :success, project: project, environment: environment) + create(:deployment, :success, project: project, environment: environment, sha: commits[1].sha) end it { is_expected.to be_truthy } end + + context 'when deployment is the same sha as the current deployment' do + let!(:deployment) do + create(:deployment, :success, project: project, environment: environment, finished_at: 1.year.ago) + end + + let!(:last_deployment) do + create(:deployment, :success, project: project, environment: environment, sha: deployment.sha) + end + + it { is_expected.to be_falsey } + end end describe '#success?' do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index c596748b0a5..736e70d1efc 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1658,6 +1658,33 @@ RSpec.describe Project, factory_default: :keep do expect(project.reload.star_count).to eq(0) end + it 'does not count stars from blocked users' do + user1 = create(:user) + user2 = create(:user) + project = create(:project, :public) + + expect(project.star_count).to eq(0) + + user1.toggle_star(project) + expect(project.reload.star_count).to eq(1) + + user2.toggle_star(project) + project.reload + expect(project.reload.star_count).to eq(2) + + user1.block + project.reload + expect(project.reload.star_count).to eq(1) + + user2.block + project.reload + expect(project.reload.star_count).to eq(0) + + user1.activate + project.reload + expect(project.reload.star_count).to eq(1) + end + it 'counts stars on the right project' do user = create(:user) project1 = create(:project, :public) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index d91eaae7caf..7207ee0b172 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2482,6 +2482,30 @@ RSpec.describe User do end end + describe 'starred_projects' do + let_it_be(:project) { create(:project) } + + before do + user.toggle_star(project) + end + + context 'when blocking a user' do + let_it_be(:user) { create(:user) } + + it 'decrements star count of project' do + expect { user.block }.to change { project.reload.star_count }.by(-1) + end + end + + context 'when activating a user' do + let_it_be(:user) { create(:user, :blocked) } + + it 'increments star count of project' do + expect { user.activate }.to change { project.reload.star_count }.by(1) + end + end + end + describe '.instance_access_request_approvers_to_be_notified' do let_it_be(:admin_issue_board_list) { create_list(:user, 12, :admin, :with_sign_ins) } diff --git a/spec/models/users_star_project_spec.rb b/spec/models/users_star_project_spec.rb index 8b66fd9c187..60ec108f77d 100644 --- a/spec/models/users_star_project_spec.rb +++ b/spec/models/users_star_project_spec.rb @@ -3,14 +3,14 @@ require 'spec_helper' RSpec.describe UsersStarProject, type: :model do + let_it_be(:project1) { create(:project) } + let_it_be(:project2) { create(:project) } + let_it_be(:user_active) { create(:user, state: 'active', name: 'user2', private_profile: true) } + let_it_be(:user_blocked) { create(:user, state: 'blocked', name: 'user1') } + it { is_expected.to belong_to(:project).touch(false) } describe 'scopes' do - let_it_be(:project1) { create(:project) } - let_it_be(:project2) { create(:project) } - let_it_be(:user_active) { create(:user, state: 'active', name: 'user2', private_profile: true) } - let_it_be(:user_blocked) { create(:user, state: 'blocked', name: 'user1') } - let_it_be(:users_star_project1) { create(:users_star_project, project: project1, user: user_active) } let_it_be(:users_star_project2) { create(:users_star_project, project: project2, user: user_blocked) } @@ -50,4 +50,38 @@ RSpec.describe UsersStarProject, type: :model do end end end + + describe 'star count hooks' do + context 'on after_create' do + context 'if user is active' do + it 'increments star count of project' do + expect { user_active.toggle_star(project1) }.to change { project1.reload.star_count }.by(1) + end + end + + context 'if user is not active' do + it 'does not increment star count of project' do + expect { user_blocked.toggle_star(project1) }.not_to change { project1.reload.star_count } + end + end + end + + context 'on after_destory' do + context 'if user is active' do + let_it_be(:users_star_project) { create(:users_star_project, project: project2, user: user_active) } + + it 'decrements star count of project' do + expect { users_star_project.destroy! }.to change { project2.reload.star_count }.by(-1) + end + end + + context 'if user is not active' do + let_it_be(:users_star_project) { create(:users_star_project, project: project2, user: user_blocked) } + + it 'does not decrement star count of project' do + expect { users_star_project.destroy! }.not_to change { project2.reload.star_count } + end + end + end + end end diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index 785fb0fc352..09fed665479 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -2884,6 +2884,27 @@ RSpec.describe ProjectPolicy do end end + describe 'read_code' do + let(:current_user) { create(:user) } + + before do + allow(subject).to receive(:allowed?).and_call_original + allow(subject).to receive(:allowed?).with(:download_code).and_return(can_download_code) + end + + context 'when the current_user can download_code' do + let(:can_download_code) { true } + + it { expect_allowed(:read_code) } + end + + context 'when the current_user cannot download_code' do + let(:can_download_code) { false } + + it { expect_disallowed(:read_code) } + end + end + private def project_subject(project_type) diff --git a/spec/services/projects/move_users_star_projects_service_spec.rb b/spec/services/projects/move_users_star_projects_service_spec.rb index 0f766ebd0ec..b580d3d8772 100644 --- a/spec/services/projects/move_users_star_projects_service_spec.rb +++ b/spec/services/projects/move_users_star_projects_service_spec.rb @@ -15,6 +15,9 @@ RSpec.describe Projects::MoveUsersStarProjectsService do end it 'moves the user\'s stars from one project to another' do + project_with_stars.reload + target_project.reload + expect(project_with_stars.users_star_projects.count).to eq 2 expect(project_with_stars.star_count).to eq 2 expect(target_project.users_star_projects.count).to eq 0 @@ -34,6 +37,8 @@ RSpec.describe Projects::MoveUsersStarProjectsService do allow(subject).to receive(:success).and_raise(StandardError) expect { subject.execute(project_with_stars) }.to raise_error(StandardError) + project_with_stars.reload + target_project.reload expect(project_with_stars.users_star_projects.count).to eq 2 expect(project_with_stars.star_count).to eq 2 diff --git a/spec/support/services/migrate_to_ghost_user_service_shared_examples.rb b/spec/support/services/migrate_to_ghost_user_service_shared_examples.rb index 1e291a90163..ae98ce689e3 100644 --- a/spec/support/services/migrate_to_ghost_user_service_shared_examples.rb +++ b/spec/support/services/migrate_to_ghost_user_service_shared_examples.rb @@ -57,7 +57,7 @@ RSpec.shared_examples "migrating a deleted user's associated records to the ghos context "race conditions" do context "when #{record_class_name} migration fails and is rolled back" do before do - expect_any_instance_of(ActiveRecord::Associations::CollectionProxy) + allow_any_instance_of(ActiveRecord::Associations::CollectionProxy) .to receive(:update_all).and_raise(ActiveRecord::StatementTimeout) end @@ -68,6 +68,7 @@ RSpec.shared_examples "migrating a deleted user's associated records to the ghos end it "doesn't unblock a previously-blocked user" do + expect(user.starred_projects).to receive(:update_all).and_call_original user.block expect { service.execute }.to raise_error(ActiveRecord::StatementTimeout) diff --git a/spec/support/shared_examples/lib/gitlab/experimentation_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/experimentation_shared_examples.rb deleted file mode 100644 index fdca326dbea..00000000000 --- a/spec/support/shared_examples/lib/gitlab/experimentation_shared_examples.rb +++ /dev/null @@ -1,22 +0,0 @@ -# frozen_string_literal: true - -RSpec.shared_examples 'tracks assignment and records the subject' do |experiment, subject_type| - before do - stub_experiments(experiment => true) - end - - it 'tracks the assignment', :experiment do - expect(experiment(experiment)) - .to track(:assignment) - .with_context(subject_type => subject) - .on_next_instance - - action - end - - it 'records the subject' do - expect(Experiment).to receive(:add_subject).with(experiment.to_s, variant: anything, subject: subject) - - action - end -end |