diff options
140 files changed, 853 insertions, 362 deletions
diff --git a/.gitlab/ci/review.gitlab-ci.yml b/.gitlab/ci/review.gitlab-ci.yml index d10b2b59c35..0e93c1d3bee 100644 --- a/.gitlab/ci/review.gitlab-ci.yml +++ b/.gitlab/ci/review.gitlab-ci.yml @@ -85,7 +85,7 @@ review-build-cng: variables: HOST_SUFFIX: "${CI_ENVIRONMENT_SLUG}" DOMAIN: "-${CI_ENVIRONMENT_SLUG}.${REVIEW_APPS_DOMAIN}" - GITLAB_HELM_CHART_REF: "v2.6.8" + GITLAB_HELM_CHART_REF: "v3.2.2" GITLAB_EDITION: "ce" environment: name: review/${CI_COMMIT_REF_NAME} diff --git a/.gitlab/issue_templates/Productivity Improvement.md b/.gitlab/issue_templates/Productivity Improvement.md index 89505cd85b4..79e1461392e 100644 --- a/.gitlab/issue_templates/Productivity Improvement.md +++ b/.gitlab/issue_templates/Productivity Improvement.md @@ -25,13 +25,6 @@ Please provide pros/cons and a weight estimate for each solution. - [ ] All potential solutions are listed. - [ ] A solution has been chosen for the first iteration: `PUT THE CHOSEN SOLUTION HERE` -## Who and when will the solution be implemented? - -<!-- -For history reason, please list the person that will implement the solution and -the planned milestone/date. ---> - ## Verify that the solution has improved the situation <!-- diff --git a/app/assets/javascripts/blob/pipeline_tour_success_modal.vue b/app/assets/javascripts/blob/pipeline_tour_success_modal.vue index 179a515e5ca..3ccd84037a7 100644 --- a/app/assets/javascripts/blob/pipeline_tour_success_modal.vue +++ b/app/assets/javascripts/blob/pipeline_tour_success_modal.vue @@ -21,6 +21,8 @@ export default { }, false, ), + goToTrackValue: 10, + trackEvent: 'click_button', components: { GlModal, GlSprintf, @@ -43,12 +45,17 @@ export default { }, data() { return { - tracking: { - label: 'congratulate_first_pipeline', - property: this.humanAccess, - }, + trackLabel: 'congratulate_first_pipeline', }; }, + computed: { + tracking() { + return { + label: this.trackLabel, + property: this.humanAccess, + }; + }, + }, mounted() { this.track(); this.disableModalFromRenderingAgain(); @@ -89,7 +96,17 @@ export default { </template> </gl-sprintf> <template #modal-footer> - <a :href="goToPipelinesPath" class="btn btn-success">{{ __('Go to Pipelines') }}</a> + <a + ref="goto" + :href="goToPipelinesPath" + class="btn btn-success" + :data-track-property="humanAccess" + :data-track-value="$options.goToTrackValue" + :data-track-event="$options.trackEvent" + :data-track-label="trackLabel" + > + {{ __('Go to Pipelines') }} + </a> </template> </gl-modal> </template> diff --git a/app/assets/javascripts/lib/utils/icon_utils.js b/app/assets/javascripts/lib/utils/icon_utils.js index 97ee773358d..043043f2eb5 100644 --- a/app/assets/javascripts/lib/utils/icon_utils.js +++ b/app/assets/javascripts/lib/utils/icon_utils.js @@ -9,8 +9,10 @@ const getSvgDom = memoize(() => axios .get(gon.sprite_icons) .then(({ data: svgs }) => new DOMParser().parseFromString(svgs, 'text/xml')) - .catch(() => { + .catch(e => { getSvgDom.cache.clear(); + + throw e; }), ); diff --git a/app/assets/javascripts/pages/static_site_editor/index.js b/app/assets/javascripts/pages/static_site_editor/index.js new file mode 100644 index 00000000000..8f808dae56c --- /dev/null +++ b/app/assets/javascripts/pages/static_site_editor/index.js @@ -0,0 +1,5 @@ +import initStaticSiteEditor from '~/static_site_editor'; + +window.addEventListener('DOMContentLoaded', () => { + initStaticSiteEditor(document.querySelector('#static-site-editor')); +}); diff --git a/app/assets/javascripts/static_site_editor/components/static_site_editor.vue b/app/assets/javascripts/static_site_editor/components/static_site_editor.vue new file mode 100644 index 00000000000..7b8b46cb048 --- /dev/null +++ b/app/assets/javascripts/static_site_editor/components/static_site_editor.vue @@ -0,0 +1,3 @@ +<template> + <div></div> +</template> diff --git a/app/assets/javascripts/static_site_editor/index.js b/app/assets/javascripts/static_site_editor/index.js new file mode 100644 index 00000000000..4290cb9a9ba --- /dev/null +++ b/app/assets/javascripts/static_site_editor/index.js @@ -0,0 +1,20 @@ +import Vue from 'vue'; +import StaticSiteEditor from './components/static_site_editor.vue'; +import createStore from './store'; + +const initStaticSiteEditor = el => { + const store = createStore(); + + return new Vue({ + el, + store, + components: { + StaticSiteEditor, + }, + render(createElement) { + return createElement('static-site-editor', StaticSiteEditor); + }, + }); +}; + +export default initStaticSiteEditor; diff --git a/app/assets/javascripts/static_site_editor/store/index.js b/app/assets/javascripts/static_site_editor/store/index.js new file mode 100644 index 00000000000..653c2532ee6 --- /dev/null +++ b/app/assets/javascripts/static_site_editor/store/index.js @@ -0,0 +1,13 @@ +import Vuex from 'vuex'; +import Vue from 'vue'; +import createState from './state'; + +Vue.use(Vuex); + +const createStore = ({ initialState } = {}) => { + return new Vuex.Store({ + state: createState(initialState), + }); +}; + +export default createStore; diff --git a/app/assets/javascripts/static_site_editor/store/state.js b/app/assets/javascripts/static_site_editor/store/state.js new file mode 100644 index 00000000000..512967cc3d9 --- /dev/null +++ b/app/assets/javascripts/static_site_editor/store/state.js @@ -0,0 +1,6 @@ +const createState = (initialState = {}) => ({ + content: '', + ...initialState, +}); + +export default createState; diff --git a/app/models/member.rb b/app/models/member.rb index dc2c8a37c0e..d7c515b650b 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -470,7 +470,6 @@ class Member < ApplicationRecord # for a Member to be commited before attempting to update the highest role. # rubocop: disable CodeReuse/ServiceClass def update_highest_role - return unless Feature.enabled?(:highest_role_callback) return unless user_id.present? return unless previous_changes[:access_level].present? diff --git a/app/models/user.rb b/app/models/user.rb index b147c7d4d48..768d4c17235 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -237,6 +237,7 @@ class User < ApplicationRecord end end end + after_commit :update_highest_role, on: [:create, :update] after_initialize :set_projects_limit @@ -1844,6 +1845,21 @@ class User < ApplicationRecord def no_recent_activity? last_active_at.to_i <= MINIMUM_INACTIVE_DAYS.days.ago.to_i end + + # Triggers the service to schedule a Sidekiq job to update the highest role + # for a User + # + # The job will be called outside of a transaction in order to ensure the changes + # for a Member to be commited before attempting to update the highest role. + # rubocop: disable CodeReuse/ServiceClass + def update_highest_role + return unless (previous_changes.keys & %w(state user_type ghost)).any? + + run_after_commit_or_now do + Members::UpdateHighestRoleService.new(id).execute + end + end + # rubocop: enable CodeReuse/ServiceClass end User.prepend_if_ee('EE::User') diff --git a/app/services/members/update_highest_role_service.rb b/app/services/members/update_highest_role_service.rb index ce1707c13ec..5ebd2e03df1 100644 --- a/app/services/members/update_highest_role_service.rb +++ b/app/services/members/update_highest_role_service.rb @@ -4,7 +4,8 @@ module Members class UpdateHighestRoleService < ::BaseService include ExclusiveLeaseGuard - LEASE_TIMEOUT = 30.minutes.to_i + LEASE_TIMEOUT = 10.minutes.to_i + DELAY = 10.minutes attr_reader :user_id @@ -14,7 +15,7 @@ module Members def execute try_obtain_lease do - UpdateHighestRoleWorker.perform_async(user_id) + UpdateHighestRoleWorker.perform_in(DELAY, user_id) end end diff --git a/app/services/snippets/update_service.rb b/app/services/snippets/update_service.rb index 76ce8231410..5794332a45b 100644 --- a/app/services/snippets/update_service.rb +++ b/app/services/snippets/update_service.rb @@ -52,7 +52,7 @@ module Snippets create_commit(snippet) if snippet.repository_exists? end rescue => e - snippet.errors.add(:repository, 'Error updating the snippet') + snippet.errors.add(:repository, e.message) log_error(e.message) false diff --git a/app/services/users/update_highest_member_role_service.rb b/app/services/users/update_highest_member_role_service.rb index 00f0af89e8d..90a5966265d 100644 --- a/app/services/users/update_highest_member_role_service.rb +++ b/app/services/users/update_highest_member_role_service.rb @@ -2,7 +2,7 @@ module Users class UpdateHighestMemberRoleService < BaseService - attr_reader :user, :identity_params + attr_reader :user def initialize(user) @user = user diff --git a/app/workers/update_highest_role_worker.rb b/app/workers/update_highest_role_worker.rb index e62131a77d0..1e2c974b6e5 100644 --- a/app/workers/update_highest_role_worker.rb +++ b/app/workers/update_highest_role_worker.rb @@ -11,9 +11,15 @@ class UpdateHighestRoleWorker # rubocop: disable CodeReuse/ActiveRecord def perform(user_id) - user = User.active.find_by(id: user_id) + user = User.find_by(id: user_id) - Users::UpdateHighestMemberRoleService.new(user).execute if user.present? + return unless user.present? + + if user.active? && user.user_type.nil? && !user.internal? + Users::UpdateHighestMemberRoleService.new(user).execute + else + UserHighestRole.where(user_id: user_id).delete_all + end end # rubocop: enable CodeReuse/ActiveRecord end diff --git a/changelogs/unreleased/fj-show-error-message-snippet-update.yml b/changelogs/unreleased/fj-show-error-message-snippet-update.yml new file mode 100644 index 00000000000..9d895af9683 --- /dev/null +++ b/changelogs/unreleased/fj-show-error-message-snippet-update.yml @@ -0,0 +1,5 @@ +--- +title: Show snippet error update to the user +merge_request: 28516 +author: +type: changed diff --git a/changelogs/unreleased/populate_user_highest_roles_table.yml b/changelogs/unreleased/populate_user_highest_roles_table.yml new file mode 100644 index 00000000000..d55851fde33 --- /dev/null +++ b/changelogs/unreleased/populate_user_highest_roles_table.yml @@ -0,0 +1,5 @@ +--- +title: Populate user_highest_roles table +merge_request: 27127 +author: +type: added diff --git a/changelogs/unreleased/update_highest_role_with_user_callback.yml b/changelogs/unreleased/update_highest_role_with_user_callback.yml new file mode 100644 index 00000000000..6564dd61d38 --- /dev/null +++ b/changelogs/unreleased/update_highest_role_with_user_callback.yml @@ -0,0 +1,5 @@ +--- +title: Update user's highest role to keep the users statistics up to date +merge_request: 28087 +author: +type: added diff --git a/db/migrate/20200317110602_add_migrating_user_highest_roles_table_index_to_users.rb b/db/migrate/20200317110602_add_migrating_user_highest_roles_table_index_to_users.rb new file mode 100644 index 00000000000..e67dc323c39 --- /dev/null +++ b/db/migrate/20200317110602_add_migrating_user_highest_roles_table_index_to_users.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddMigratingUserHighestRolesTableIndexToUsers < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + INDEX_NAME = 'index_for_migrating_user_highest_roles_table' + + disable_ddl_transaction! + + def up + add_concurrent_index :users, + :id, + where: "state = 'active' AND user_type IS NULL AND bot_type IS NULL AND ghost IS NOT TRUE", + name: INDEX_NAME + end + + def down + remove_concurrent_index :users, :id, name: INDEX_NAME + end +end diff --git a/db/post_migrate/20200311130802_schedule_populate_user_highest_roles_table.rb b/db/post_migrate/20200311130802_schedule_populate_user_highest_roles_table.rb new file mode 100644 index 00000000000..36f0d42a855 --- /dev/null +++ b/db/post_migrate/20200311130802_schedule_populate_user_highest_roles_table.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +class SchedulePopulateUserHighestRolesTable < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + BATCH_SIZE = 10_000 + DELAY = 5.minutes.to_i + DOWNTIME = false + MIGRATION = 'PopulateUserHighestRolesTable' + + disable_ddl_transaction! + + class User < ActiveRecord::Base + include EachBatch + + scope :active, -> { + where(state: 'active', user_type: nil, bot_type: nil) + .where('ghost IS NOT TRUE') + } + end + + def up + # We currently have ~5_300_000 users with the state active on GitLab.com. + # This means it'll schedule ~530 jobs (10k Users each) with a 5 minutes gap, + # so this should take ~44 hours for all background migrations to complete. + User.active.each_batch(of: BATCH_SIZE) do |batch, index| + range = batch.pluck(Arel.sql('MIN(id)'), Arel.sql('MAX(id)')).first + delay = index * DELAY + + migrate_in(delay.seconds, MIGRATION, [*range]) + end + end + + def down + # nothing + end +end diff --git a/db/structure.sql b/db/structure.sql index aa43d49b6dc..178cf4c63da 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -9066,6 +9066,8 @@ CREATE UNIQUE INDEX index_feature_gates_on_feature_key_and_key_and_value ON publ CREATE UNIQUE INDEX index_features_on_key ON public.features USING btree (key); +CREATE INDEX index_for_migrating_user_highest_roles_table ON public.users USING btree (id) WHERE (((state)::text = 'active'::text) AND (user_type IS NULL) AND (bot_type IS NULL) AND (ghost IS NOT TRUE)); + CREATE INDEX index_for_resource_group ON public.ci_builds USING btree (resource_group_id, id) WHERE (resource_group_id IS NOT NULL); CREATE INDEX index_for_status_per_branch_per_project ON public.merge_trains USING btree (target_project_id, target_branch, status); @@ -12865,6 +12867,7 @@ COPY "schema_migrations" (version) FROM STDIN; 20200311084025 20200311093210 20200311094020 +20200311130802 20200311141053 20200311141943 20200311154110 @@ -12880,6 +12883,7 @@ COPY "schema_migrations" (version) FROM STDIN; 20200316111759 20200316162648 20200316173312 +20200317110602 20200317142110 20200318140400 20200318152134 diff --git a/doc/administration/instance_limits.md b/doc/administration/instance_limits.md index e44fc8679ce..034d80be90b 100644 --- a/doc/administration/instance_limits.md +++ b/doc/administration/instance_limits.md @@ -70,6 +70,13 @@ NOTE: **Note:** Set the limit to `0` to disable it. GitLab ignores all incoming emails sent from auto-responders by looking for the `X-Autoreply` header. Such emails don't create comments on issues or merge requests. +## Amount of data sent from Sentry via Error Tracking + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/14926) in GitLab 12.6. + +Sentry payloads sent to GitLab have a 1 MB maximum limit, both for security reasons +and to limit memory consumption. + ## CI/CD limits ### Number of jobs in active pipelines diff --git a/doc/administration/monitoring/prometheus/gitlab_metrics.md b/doc/administration/monitoring/prometheus/gitlab_metrics.md index 11fda995955..889c2209a54 100644 --- a/doc/administration/monitoring/prometheus/gitlab_metrics.md +++ b/doc/administration/monitoring/prometheus/gitlab_metrics.md @@ -92,15 +92,6 @@ The following metrics are available: | `failed_login_captcha_total` | Gauge | 11.0 | Counter of failed CAPTCHA attempts during login | | | `successful_login_captcha_total` | Gauge | 11.0 | Counter of successful CAPTCHA attempts during login | | | `auto_devops_pipelines_completed_total` | Counter | 12.7 | Counter of completed Auto DevOps pipelines, labeled by status | | -| `sidekiq_jobs_cpu_seconds` | Histogram | 12.4 | Seconds of cpu time to run Sidekiq job | queue, boundary, external_dependencies, feature_category, job_status, urgency | -| `sidekiq_jobs_completion_seconds` | Histogram | 12.2 | Seconds to complete Sidekiq job | queue, boundary, external_dependencies, feature_category, job_status, urgency | -| `sidekiq_jobs_db_seconds` | Histogram | 12.9 | Seconds of DB time to run Sidekiq job | queue, boundary, external_dependencies, feature_category, job_status, urgency | -| `sidekiq_jobs_gitaly_seconds` | Histogram | 12.9 | Seconds of Gitaly time to run Sidekiq job | queue, boundary, external_dependencies, feature_category, job_status, urgency | -| `sidekiq_jobs_queue_duration_seconds` | Histogram | 12.5 | Duration in seconds that a Sidekiq job was queued before being executed | queue, boundary, external_dependencies, feature_category, urgency | -| `sidekiq_jobs_failed_total` | Counter | 12.2 | Sidekiq jobs failed | queue, boundary, external_dependencies, feature_category, urgency | -| `sidekiq_jobs_retried_total` | Counter | 12.2 | Sidekiq jobs retried | queue, boundary, external_dependencies, feature_category, urgency | -| `sidekiq_running_jobs` | Gauge | 12.2 | Number of Sidekiq jobs running | queue, boundary, external_dependencies, feature_category, urgency | -| `sidekiq_concurrency` | Gauge | 12.5 | Maximum number of Sidekiq jobs | | ## Metrics controlled by a feature flag @@ -120,6 +111,15 @@ configuration option in `gitlab.yml`. These metrics are served from the | Metric | Type | Since | Description | Labels | |:---------------------------------------------- |:------- |:----- |:----------- |:------ | +| `sidekiq_jobs_cpu_seconds` | Histogram | 12.4 | Seconds of cpu time to run Sidekiq job | queue, boundary, external_dependencies, feature_category, job_status, urgency | +| `sidekiq_jobs_completion_seconds` | Histogram | 12.2 | Seconds to complete Sidekiq job | queue, boundary, external_dependencies, feature_category, job_status, urgency | +| `sidekiq_jobs_db_seconds` | Histogram | 12.9 | Seconds of DB time to run Sidekiq job | queue, boundary, external_dependencies, feature_category, job_status, urgency | +| `sidekiq_jobs_gitaly_seconds` | Histogram | 12.9 | Seconds of Gitaly time to run Sidekiq job | queue, boundary, external_dependencies, feature_category, job_status, urgency | +| `sidekiq_jobs_queue_duration_seconds` | Histogram | 12.5 | Duration in seconds that a Sidekiq job was queued before being executed | queue, boundary, external_dependencies, feature_category, urgency | +| `sidekiq_jobs_failed_total` | Counter | 12.2 | Sidekiq jobs failed | queue, boundary, external_dependencies, feature_category, urgency | +| `sidekiq_jobs_retried_total` | Counter | 12.2 | Sidekiq jobs retried | queue, boundary, external_dependencies, feature_category, urgency | +| `sidekiq_running_jobs` | Gauge | 12.2 | Number of Sidekiq jobs running | queue, boundary, external_dependencies, feature_category, urgency | +| `sidekiq_concurrency` | Gauge | 12.5 | Maximum number of Sidekiq jobs | | | `geo_db_replication_lag_seconds` | Gauge | 10.2 | Database replication lag (seconds) | url | | `geo_repositories` | Gauge | 10.2 | Total number of repositories available on primary | url | | `geo_repositories_synced` | Gauge | 10.2 | Number of repositories synced on secondary | url | diff --git a/doc/development/testing_guide/best_practices.md b/doc/development/testing_guide/best_practices.md index 31a449dbbb2..62767180077 100644 --- a/doc/development/testing_guide/best_practices.md +++ b/doc/development/testing_guide/best_practices.md @@ -78,6 +78,10 @@ FDOC=1 bin/rspec spec/[path]/[to]/[spec].rb - Use `focus: true` to isolate parts of the specs you want to run. - Use [`:aggregate_failures`](https://relishapp.com/rspec/rspec-core/docs/expectation-framework-integration/aggregating-failures) when there is more than one expectation in a test. - For [empty test description blocks](https://github.com/rubocop-hq/rspec-style-guide#it-and-specify), use `specify` rather than `it do` if the test is self-explanatory. +- Use `non_existing_record_id`/`non_existing_record_iid`/`non_existing_record_access_level` + when you need an ID/IID/access level that doesn't actually exists. Using 123, 1234, + or even 999 is brittle as these IDs could actually exist in the database in the + context of a CI run. ### Coverage @@ -244,7 +248,11 @@ so we need to set some guidelines for their use going forward: In some cases, there is no need to recreate the same object for tests again for each example. For example, a project and a guest of that project is needed to test issues on the same project, one project and user will do for the entire file. -This can be achieved by using + +As much as possible, do not implement this using `before(:all)` or `before(:context)`. If you do, +you would need to manually clean up the data as those hooks run outside a database transaction. + +Instead, this can be achieved by using [`let_it_be`](https://test-prof.evilmartians.io/#/let_it_be) variables and the [`before_all`](https://test-prof.evilmartians.io/#/before_all) hook from the [`test-prof` gem](https://rubygems.org/gems/test-prof). diff --git a/lib/gitlab/background_migration/populate_user_highest_roles_table.rb b/lib/gitlab/background_migration/populate_user_highest_roles_table.rb new file mode 100644 index 00000000000..0c9e15b5a80 --- /dev/null +++ b/lib/gitlab/background_migration/populate_user_highest_roles_table.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + # This background migration creates records on user_highest_roles according to + # the given user IDs range. IDs will load users with a left outer joins to + # have a record for users without a Group or Project. One INSERT per ID is + # issued. + class PopulateUserHighestRolesTable + BATCH_SIZE = 100 + + # rubocop:disable Style/Documentation + class User < ActiveRecord::Base + self.table_name = 'users' + + scope :active, -> { + where(state: 'active', user_type: nil, bot_type: nil) + .where('ghost IS NOT TRUE') + } + end + + def perform(from_id, to_id) + (from_id..to_id).each_slice(BATCH_SIZE) do |ids| + execute( + <<-EOF + INSERT INTO user_highest_roles (updated_at, user_id, highest_access_level) + #{select_sql(from_id, to_id)} + ON CONFLICT (user_id) DO + UPDATE SET highest_access_level = EXCLUDED.highest_access_level + EOF + ) + end + end + + private + + def select_sql(from_id, to_id) + User + .select('NOW() as updated_at, users.id, MAX(access_level) AS highest_access_level') + .joins('LEFT OUTER JOIN members ON members.user_id = users.id AND members.requested_at IS NULL') + .where(users: { id: active_user_ids(from_id, to_id) }) + .group('users.id') + .to_sql + end + + def active_user_ids(from_id, to_id) + User.active.where(users: { id: from_id..to_id }).pluck(:id) + end + + def execute(sql) + @connection ||= ActiveRecord::Base.connection + @connection.execute(sql) + end + end + end +end diff --git a/scripts/review_apps/base-config.yaml b/scripts/review_apps/base-config.yaml index 83dc4de879a..9b9fcdb85c4 100644 --- a/scripts/review_apps/base-config.yaml +++ b/scripts/review_apps/base-config.yaml @@ -155,8 +155,6 @@ redis: limits: cpu: 200m memory: 130M -redis-ha: - enabled: false registry: hpa: minReplicas: 1 diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb index f3d4234a558..7b1e9748268 100644 --- a/spec/controllers/autocomplete_controller_spec.rb +++ b/spec/controllers/autocomplete_controller_spec.rb @@ -192,9 +192,9 @@ describe AutocompleteController do end it 'rejects non existent user ids' do - get(:users, params: { author_id: 99999 }) + get(:users, params: { author_id: non_existing_record_id }) - expect(json_response.collect { |u| u['id'] }).not_to include(99999) + expect(json_response.collect { |u| u['id'] }).not_to include(non_existing_record_id) end end diff --git a/spec/controllers/boards/issues_controller_spec.rb b/spec/controllers/boards/issues_controller_spec.rb index c57ad937712..8639b76ef0f 100644 --- a/spec/controllers/boards/issues_controller_spec.rb +++ b/spec/controllers/boards/issues_controller_spec.rb @@ -26,7 +26,7 @@ describe Boards::IssuesController do context 'with invalid board id' do it 'returns a not found 404 response' do - list_issues user: user, board: 999, list: list2 + list_issues user: user, board: non_existing_record_id, list: list2 expect(response).to have_gitlab_http_status(:not_found) end @@ -123,7 +123,7 @@ describe Boards::IssuesController do context 'with invalid list id' do it 'returns a not found 404 response' do - list_issues user: user, board: board, list: 999 + list_issues user: user, board: board, list: non_existing_record_id expect(response).to have_gitlab_http_status(:not_found) end @@ -441,7 +441,7 @@ describe Boards::IssuesController do context 'with invalid board id' do it 'returns a not found 404 response' do - create_issue user: user, board: 999, list: list1, title: 'New issue' + create_issue user: user, board: non_existing_record_id, list: list1, title: 'New issue' expect(response).to have_gitlab_http_status(:not_found) end @@ -449,7 +449,7 @@ describe Boards::IssuesController do context 'with invalid list id' do it 'returns a not found 404 response' do - create_issue user: user, board: board, list: 999, title: 'New issue' + create_issue user: user, board: board, list: non_existing_record_id, title: 'New issue' expect(response).to have_gitlab_http_status(:not_found) end @@ -512,13 +512,13 @@ describe Boards::IssuesController do end it 'returns a not found 404 response for invalid board id' do - move user: user, board: 999, issue: issue, from_list_id: list1.id, to_list_id: list2.id + move user: user, board: non_existing_record_id, issue: issue, from_list_id: list1.id, to_list_id: list2.id expect(response).to have_gitlab_http_status(:not_found) end it 'returns a not found 404 response for invalid issue id' do - move user: user, board: board, issue: double(id: 999), from_list_id: list1.id, to_list_id: list2.id + move user: user, board: board, issue: double(id: non_existing_record_id), from_list_id: list1.id, to_list_id: list2.id expect(response).to have_gitlab_http_status(:not_found) end diff --git a/spec/controllers/boards/lists_controller_spec.rb b/spec/controllers/boards/lists_controller_spec.rb index 3886388bcf4..d0b34b55c36 100644 --- a/spec/controllers/boards/lists_controller_spec.rb +++ b/spec/controllers/boards/lists_controller_spec.rb @@ -154,7 +154,7 @@ describe Boards::ListsController do context 'with invalid list id' do it 'returns a not found 404 response' do - move user: user, board: board, list: 999, position: 1 + move user: user, board: board, list: non_existing_record_id, position: 1 expect(response).to have_gitlab_http_status(:not_found) end @@ -246,7 +246,7 @@ describe Boards::ListsController do context 'with invalid list id' do it 'returns a not found 404 response' do - remove_board_list user: user, board: board, list: 999 + remove_board_list user: user, board: board, list: non_existing_record_id expect(response).to have_gitlab_http_status(:not_found) end diff --git a/spec/controllers/dashboard/todos_controller_spec.rb b/spec/controllers/dashboard/todos_controller_spec.rb index cea0ea75e5d..004eef1873e 100644 --- a/spec/controllers/dashboard/todos_controller_spec.rb +++ b/spec/controllers/dashboard/todos_controller_spec.rb @@ -24,7 +24,7 @@ describe Dashboard::TodosController do end it 'renders 404 when given project does not exists' do - get :index, params: { project_id: 999 } + get :index, params: { project_id: non_existing_record_id } expect(response).to have_gitlab_http_status(:not_found) end diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index 6c63b220322..3b035eea7d5 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -176,7 +176,7 @@ describe Projects::EnvironmentsController do context 'with invalid id' do it 'responds with a status code 404' do params = environment_params - params[:id] = 12345 + params[:id] = non_existing_record_id get :show, params: params expect(response).to have_gitlab_http_status(:not_found) diff --git a/spec/controllers/projects/error_tracking/stack_traces_controller_spec.rb b/spec/controllers/projects/error_tracking/stack_traces_controller_spec.rb index 75e1c817baa..27d49147e99 100644 --- a/spec/controllers/projects/error_tracking/stack_traces_controller_spec.rb +++ b/spec/controllers/projects/error_tracking/stack_traces_controller_spec.rb @@ -12,7 +12,7 @@ describe Projects::ErrorTracking::StackTracesController do end describe 'GET #index' do - let(:issue_id) { 1234 } + let(:issue_id) { non_existing_record_id } let(:issue_stack_trace_service) { spy(:issue_stack_trace_service) } subject(:get_stack_trace) do diff --git a/spec/controllers/projects/error_tracking_controller_spec.rb b/spec/controllers/projects/error_tracking_controller_spec.rb index 22826938de2..6be979418ad 100644 --- a/spec/controllers/projects/error_tracking_controller_spec.rb +++ b/spec/controllers/projects/error_tracking_controller_spec.rb @@ -182,7 +182,7 @@ describe Projects::ErrorTrackingController do end describe 'GET #issue_details' do - let_it_be(:issue_id) { 1234 } + let_it_be(:issue_id) { non_existing_record_id } let(:issue_details_service) { spy(:issue_details_service) } @@ -279,7 +279,7 @@ describe Projects::ErrorTrackingController do end describe 'PUT #update' do - let(:issue_id) { 1234 } + let(:issue_id) { non_existing_record_id } let(:issue_update_service) { spy(:issue_update_service) } let(:permitted_params) do ActionController::Parameters.new( @@ -301,7 +301,7 @@ describe Projects::ErrorTrackingController do context 'update result is successful' do before do expect(issue_update_service).to receive(:execute) - .and_return(status: :success, updated: true, closed_issue_iid: 1234) + .and_return(status: :success, updated: true, closed_issue_iid: non_existing_record_iid) update_issue end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 094b50322d1..1fda9d63bbe 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -338,13 +338,13 @@ describe Projects::IssuesController do context 'with invalid params' do it 'returns a unprocessable entity 422 response for invalid move ids' do - reorder_issue(issue1, move_after_id: 99, move_before_id: 999) + reorder_issue(issue1, move_after_id: 99, move_before_id: non_existing_record_id) expect(response).to have_gitlab_http_status(:unprocessable_entity) end it 'returns a not found 404 response for invalid issue id' do - reorder_issue(object_double(issue1, iid: 999), + reorder_issue(object_double(issue1, iid: non_existing_record_iid), move_after_id: issue2.id, move_before_id: issue3.id) diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index 65f57deff1b..0071e6c8a19 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -132,7 +132,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do context 'when job does not exist' do before do - get_show(id: 1234) + get_show(id: non_existing_record_id) end it 'renders not_found' do @@ -1146,7 +1146,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do context 'when job does not exist' do it 'renders not_found' do - get_terminal(id: 1234) + get_terminal(id: non_existing_record_id) expect(response).to have_gitlab_http_status(:not_found) end @@ -1191,7 +1191,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do context 'and invalid id' do it 'returns 404' do - get_terminal_websocket(id: 1234) + get_terminal_websocket(id: non_existing_record_id) expect(response).to have_gitlab_http_status(:not_found) end diff --git a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb index a220c1bff95..3d9193e3e33 100644 --- a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb @@ -8,11 +8,9 @@ describe Projects::MergeRequests::DiffsController do shared_examples '404 for unexistent diffable' do context 'when diffable does not exists' do it 'returns 404' do - unexistent_diff_id = 9999 + go(diff_id: non_existing_record_id) - go(diff_id: unexistent_diff_id) - - expect(MergeRequestDiff.find_by(id: unexistent_diff_id)).to be_nil + expect(MergeRequestDiff.find_by(id: non_existing_record_id)).to be_nil expect(response).to have_gitlab_http_status(:not_found) end end diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index 7c00af7b945..39594ff287d 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -585,7 +585,7 @@ describe Projects::NotesController do context 'when a noteable is not found' do it 'returns 404 status' do - request_params[:target_id] = 9999 + request_params[:target_id] = non_existing_record_id post :create, params: request_params.merge(format: :json) expect(response).to have_gitlab_http_status(:not_found) diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb index f42d0560e80..309a8226226 100644 --- a/spec/controllers/uploads_controller_spec.rb +++ b/spec/controllers/uploads_controller_spec.rb @@ -76,7 +76,7 @@ describe UploadsController do end it "returns 404 status when object not found" do - post :create, params: { model: model, id: 9999 }, format: :json + post :create, params: { model: model, id: non_existing_record_id }, format: :json expect(response).to have_gitlab_http_status(:not_found) end diff --git a/spec/features/issuables/markdown_references/jira_spec.rb b/spec/features/issuables/markdown_references/jira_spec.rb index c5818691b3c..779606effdc 100644 --- a/spec/features/issuables/markdown_references/jira_spec.rb +++ b/spec/features/issuables/markdown_references/jira_spec.rb @@ -163,7 +163,7 @@ describe "Jira", :js do markdown = <<~HEREDOC Referencing internal issue #{issue_actual_project.to_reference}, cross-project #{issue_other_project.to_reference(actual_project)} external JIRA-5 - and non existing #999 + and non existing ##{non_existing_record_iid} HEREDOC page.within("#diff-notes-app") do @@ -186,6 +186,6 @@ describe "Jira", :js do expect(page).not_to have_link("JIRA-5", href: "https://jira.example.com/browse/JIRA-5") end - expect(page).not_to have_link("#999") + expect(page).not_to have_link("##{non_existing_record_iid}") end end diff --git a/spec/features/projects/snippets/user_updates_snippet_spec.rb b/spec/features/projects/snippets/user_updates_snippet_spec.rb index bad3fde8a4a..58ca922b9cb 100644 --- a/spec/features/projects/snippets/user_updates_snippet_spec.rb +++ b/spec/features/projects/snippets/user_updates_snippet_spec.rb @@ -54,9 +54,11 @@ describe 'Projects > Snippets > User updates a snippet', :js do end context 'when the git operation fails' do + let(:error_message) { 'foobar' } + before do allow_next_instance_of(Snippets::UpdateService) do |instance| - allow(instance).to receive(:create_commit).and_raise(StandardError) + allow(instance).to receive(:create_commit).and_raise(StandardError, error_message) end fill_in('project_snippet_title', with: 'Snippet new title') @@ -65,7 +67,7 @@ describe 'Projects > Snippets > User updates a snippet', :js do end it 'renders edit page and displays the error' do - expect(page.find('.flash-container span').text).to eq('Error updating the snippet') + expect(page.find('.flash-container span').text).to eq(error_message) expect(page).to have_content('Edit Snippet') end end diff --git a/spec/features/snippets/user_edits_snippet_spec.rb b/spec/features/snippets/user_edits_snippet_spec.rb index 0bbb92b1f3f..1ec18e3e0e3 100644 --- a/spec/features/snippets/user_edits_snippet_spec.rb +++ b/spec/features/snippets/user_edits_snippet_spec.rb @@ -85,9 +85,11 @@ describe 'User edits snippet', :js do end context 'when the git operation fails' do + let(:error_message) { 'foobar' } + before do allow_next_instance_of(Snippets::UpdateService) do |instance| - allow(instance).to receive(:create_commit).and_raise(StandardError) + allow(instance).to receive(:create_commit).and_raise(StandardError, error_message) end fill_in 'personal_snippet_title', with: 'New Snippet Title' @@ -96,7 +98,7 @@ describe 'User edits snippet', :js do end it 'renders edit page and displays the error' do - expect(page.find('.flash-container span').text).to eq('Error updating the snippet') + expect(page.find('.flash-container span').text).to eq(error_message) expect(page).to have_content('Edit Snippet') end end diff --git a/spec/finders/concerns/finder_with_cross_project_access_spec.rb b/spec/finders/concerns/finder_with_cross_project_access_spec.rb index f3365309b05..d11d4da25a8 100644 --- a/spec/finders/concerns/finder_with_cross_project_access_spec.rb +++ b/spec/finders/concerns/finder_with_cross_project_access_spec.rb @@ -97,7 +97,7 @@ describe FinderWithCrossProjectAccess do end it 're-enables the check after the find failed' do - finder.find_by!(id: 9999) rescue ActiveRecord::RecordNotFound + finder.find_by!(id: non_existing_record_id) rescue ActiveRecord::RecordNotFound expect(finder.instance_variable_get(:@should_skip_cross_project_check)) .to eq(false) diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb index b6bedab724a..baf40861a6e 100644 --- a/spec/finders/issues_finder_spec.rb +++ b/spec/finders/issues_finder_spec.rb @@ -774,14 +774,16 @@ describe IssuesFinder do end describe '#row_count', :request_store do + let_it_be(:admin) { create(:admin) } + it 'returns the number of rows for the default state' do - finder = described_class.new(user) + finder = described_class.new(admin) expect(finder.row_count).to eq(4) end it 'returns the number of rows for a given state' do - finder = described_class.new(user, state: 'closed') + finder = described_class.new(admin, state: 'closed') expect(finder.row_count).to be_zero end diff --git a/spec/finders/sentry_issue_finder_spec.rb b/spec/finders/sentry_issue_finder_spec.rb index 5535eb8c214..520f690a134 100644 --- a/spec/finders/sentry_issue_finder_spec.rb +++ b/spec/finders/sentry_issue_finder_spec.rb @@ -27,7 +27,7 @@ describe SentryIssueFinder do it { is_expected.to eq(sentry_issue) } context 'when identifier is incorrect' do - let(:identifier) { 1234 } + let(:identifier) { non_existing_record_id } it { is_expected.to be_nil } end diff --git a/spec/finders/snippets_finder_spec.rb b/spec/finders/snippets_finder_spec.rb index 69e03c4c473..fdcc73f6e92 100644 --- a/spec/finders/snippets_finder_spec.rb +++ b/spec/finders/snippets_finder_spec.rb @@ -114,7 +114,7 @@ describe SnippetsFinder do context 'when author is not valid' do it 'returns quickly' do - finder = described_class.new(admin, author: 1234) + finder = described_class.new(admin, author: non_existing_record_id) expect(finder).not_to receive(:init_collection) expect(Snippet).to receive(:none).and_call_original @@ -208,7 +208,7 @@ describe SnippetsFinder do context 'when project is not valid' do it 'returns quickly' do - finder = described_class.new(admin, project: 1234) + finder = described_class.new(admin, project: non_existing_record_id) expect(finder).not_to receive(:init_collection) expect(Snippet).to receive(:none).and_call_original diff --git a/spec/fixtures/lib/gitlab/import_export/complex/project.json b/spec/fixtures/lib/gitlab/import_export/complex/project.json index 0fbe33a3f8a..97baf5cdd39 100644 --- a/spec/fixtures/lib/gitlab/import_export/complex/project.json +++ b/spec/fixtures/lib/gitlab/import_export/complex/project.json @@ -2468,7 +2468,7 @@ "id": 27, "target_branch": "feature", "source_branch": "feature_conflict", - "source_project_id": 999, + "source_project_id": 2147483547, "author_id": 1, "assignee_id": null, "title": "MR1", @@ -6334,13 +6334,13 @@ "status": "failed", "started_at": null, "finished_at": null, - "user_id": 9999, + "user_id": 2147483547, "duration": null, "source": "push", "merge_request_id": null, "notes": [ { - "id": 999, + "id": 2147483547, "note": "Natus rerum qui dolorem dolorum voluptas.", "noteable_type": "Commit", "author_id": 1, @@ -6544,7 +6544,7 @@ "id": 27, "target_branch": "feature", "source_branch": "feature_conflict", - "source_project_id": 999, + "source_project_id": 2147483547, "author_id": 1, "assignee_id": null, "title": "MR1", diff --git a/spec/fixtures/lib/gitlab/import_export/group_exports/complex/group.json b/spec/fixtures/lib/gitlab/import_export/group_exports/complex/group.json index 66512019f12..001bf7b43c1 100644 --- a/spec/fixtures/lib/gitlab/import_export/group_exports/complex/group.json +++ b/spec/fixtures/lib/gitlab/import_export/group_exports/complex/group.json @@ -1,7 +1,7 @@ { "name": "ymg09t5704clnxnqfgaj2h098gz4r7gyx4wc3fzmlqj1en24zf", "path": "ymg09t5704clnxnqfgaj2h098gz4r7gyx4wc3fzmlqj1en24zf", - "owner_id": 123, + "owner_id": 2147483547, "created_at": "2019-11-20 17:01:53 UTC", "updated_at": "2019-11-20 17:05:44 UTC", "description": "Group Description", diff --git a/spec/fixtures/lib/gitlab/import_export/light/project.json b/spec/fixtures/lib/gitlab/import_export/light/project.json index c0ef3690d27..ad0fc59326b 100644 --- a/spec/fixtures/lib/gitlab/import_export/light/project.json +++ b/spec/fixtures/lib/gitlab/import_export/light/project.json @@ -1,7 +1,7 @@ { "description": "Nisi et repellendus ut enim quo accusamus vel magnam.", "import_type": "gitlab_project", - "creator_id": 123, + "creator_id": 2147483547, "visibility_level": 10, "archived": false, "milestones": [ diff --git a/spec/fixtures/lib/gitlab/import_export/milestone-iid/project.json b/spec/fixtures/lib/gitlab/import_export/milestone-iid/project.json index b028147b5eb..24bfb8836d7 100644 --- a/spec/fixtures/lib/gitlab/import_export/milestone-iid/project.json +++ b/spec/fixtures/lib/gitlab/import_export/milestone-iid/project.json @@ -1,7 +1,7 @@ { "description": "Nisi et repellendus ut enim quo accusamus vel magnam.", "import_type": "gitlab_project", - "creator_id": 123, + "creator_id": 2147483547, "visibility_level": 10, "archived": false, "issues": [ diff --git a/spec/fixtures/lib/gitlab/import_export/with_invalid_records/project.json b/spec/fixtures/lib/gitlab/import_export/with_invalid_records/project.json index 40d08f78f28..a6e6ba43bdc 100644 --- a/spec/fixtures/lib/gitlab/import_export/with_invalid_records/project.json +++ b/spec/fixtures/lib/gitlab/import_export/with_invalid_records/project.json @@ -1,7 +1,7 @@ { "description": "Nisi et repellendus ut enim quo accusamus vel magnam.", "import_type": "gitlab_project", - "creator_id": 999, + "creator_id": 2147483547, "visibility_level": 10, "archived": false, "milestones": [ diff --git a/spec/frontend/blob/pipeline_tour_success_modal_spec.js b/spec/frontend/blob/pipeline_tour_success_modal_spec.js index 613a7ce8303..99940225652 100644 --- a/spec/frontend/blob/pipeline_tour_success_modal_spec.js +++ b/spec/frontend/blob/pipeline_tour_success_modal_spec.js @@ -2,7 +2,7 @@ import pipelineTourSuccess from '~/blob/pipeline_tour_success_modal.vue'; import { shallowMount } from '@vue/test-utils'; import Cookies from 'js-cookie'; import { GlSprintf, GlModal } from '@gitlab/ui'; -import { mockTracking, unmockTracking } from 'helpers/tracking_helper'; +import { mockTracking, triggerEvent, unmockTracking } from 'helpers/tracking_helper'; import modalProps from './pipeline_tour_success_mock_data'; describe('PipelineTourSuccessModal', () => { @@ -16,6 +16,9 @@ describe('PipelineTourSuccessModal', () => { trackingSpy = mockTracking('_category_', undefined, jest.spyOn); wrapper = shallowMount(pipelineTourSuccess, { propsData: modalProps, + stubs: { + GlModal, + }, }); cookieSpy = jest.spyOn(Cookies, 'remove'); @@ -41,11 +44,23 @@ describe('PipelineTourSuccessModal', () => { }); describe('tracking', () => { - it('send event for basic view of popover', () => { + it('send event for basic view of modal', () => { expect(trackingSpy).toHaveBeenCalledWith(undefined, undefined, { label: 'congratulate_first_pipeline', property: modalProps.humanAccess, }); }); + + it('send an event when go to pipelines is clicked', () => { + trackingSpy = mockTracking('_category_', wrapper.element, jest.spyOn); + const goToBtn = wrapper.find({ ref: 'goto' }); + triggerEvent(goToBtn.element); + + expect(trackingSpy).toHaveBeenCalledWith('_category_', 'click_button', { + label: 'congratulate_first_pipeline', + property: modalProps.humanAccess, + value: '10', + }); + }); }); }); diff --git a/spec/graphql/mutations/issues/update_spec.rb b/spec/graphql/mutations/issues/update_spec.rb index 83bd3041cbf..da286bb4092 100644 --- a/spec/graphql/mutations/issues/update_spec.rb +++ b/spec/graphql/mutations/issues/update_spec.rb @@ -43,7 +43,7 @@ describe Mutations::Issues::Update do context 'when iid does not exist' do it 'raises resource not available error' do - mutation_params[:iid] = 99999 + mutation_params[:iid] = non_existing_record_iid expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) end diff --git a/spec/helpers/issuables_helper_spec.rb b/spec/helpers/issuables_helper_spec.rb index 7971eb8849d..38ad11846d2 100644 --- a/spec/helpers/issuables_helper_spec.rb +++ b/spec/helpers/issuables_helper_spec.rb @@ -40,7 +40,7 @@ describe IssuablesHelper do end it 'returns default label when a group was not found for the provided id' do - expect(group_dropdown_label(9999, default)).to eq('default label') + expect(group_dropdown_label(non_existing_record_id, default)).to eq('default label') end end diff --git a/spec/lib/api/helpers_spec.rb b/spec/lib/api/helpers_spec.rb index 9980f4d8e23..3595d06a184 100644 --- a/spec/lib/api/helpers_spec.rb +++ b/spec/lib/api/helpers_spec.rb @@ -22,7 +22,7 @@ describe API::Helpers do context 'when ID is used as an argument' do let(:existing_id) { project.id } - let(:non_existing_id) { (Project.maximum(:id) || 0) + 1 } + let(:non_existing_id) { non_existing_record_id } it_behaves_like 'project finder' end @@ -66,7 +66,7 @@ describe API::Helpers do context 'when ID is used as an argument' do let(:existing_id) { namespace.id } - let(:non_existing_id) { 9999 } + let(:non_existing_id) { non_existing_record_id } it_behaves_like 'namespace finder' end diff --git a/spec/lib/banzai/filter/reference_redactor_filter_spec.rb b/spec/lib/banzai/filter/reference_redactor_filter_spec.rb index a68581b3000..956bc85e53f 100644 --- a/spec/lib/banzai/filter/reference_redactor_filter_spec.rb +++ b/spec/lib/banzai/filter/reference_redactor_filter_spec.rb @@ -77,7 +77,7 @@ describe Banzai::Filter::ReferenceRedactorFilter do end it 'handles invalid references' do - link = reference_link(project: 12345, reference_type: 'test') + link = reference_link(project: non_existing_record_id, reference_type: 'test') expect { filter(link) }.not_to raise_error end diff --git a/spec/lib/gitlab/application_context_spec.rb b/spec/lib/gitlab/application_context_spec.rb index c6bc3c945a8..0903ca6f9e8 100644 --- a/spec/lib/gitlab/application_context_spec.rb +++ b/spec/lib/gitlab/application_context_spec.rb @@ -9,14 +9,16 @@ describe Gitlab::ApplicationContext do end it 'passes the expected context on to labkit' do + user = build(:user) + project = build(:project) fake_proc = duck_type(:call) expected_context = hash_including(user: fake_proc, project: fake_proc, root_namespace: fake_proc) expect(Labkit::Context).to receive(:with_context).with(expected_context) described_class.with_context( - user: build(:user), - project: build(:project), + user: user, + project: project, namespace: build(:namespace)) {} end diff --git a/spec/lib/gitlab/background_migration/backfill_project_fullpath_in_repo_config_spec.rb b/spec/lib/gitlab/background_migration/backfill_project_fullpath_in_repo_config_spec.rb index 91ede05f395..44f5c3380a1 100644 --- a/spec/lib/gitlab/background_migration/backfill_project_fullpath_in_repo_config_spec.rb +++ b/spec/lib/gitlab/background_migration/backfill_project_fullpath_in_repo_config_spec.rb @@ -39,7 +39,7 @@ describe Gitlab::BackgroundMigration::BackfillProjectFullpathInRepoConfig, schem end it 'raises OrphanedNamespaceError when any parent namespace does not exist' do - subgroup.update_attribute(:parent_id, namespaces.maximum(:id).succ) + subgroup.update_attribute(:parent_id, non_existing_record_id) expect { project.full_path }.to raise_error(Gitlab::BackgroundMigration::BackfillProjectFullpathInRepoConfig::OrphanedNamespaceError) end diff --git a/spec/lib/gitlab/background_migration/backfill_project_repositories_spec.rb b/spec/lib/gitlab/background_migration/backfill_project_repositories_spec.rb index 510a0074554..cfaef1578a9 100644 --- a/spec/lib/gitlab/background_migration/backfill_project_repositories_spec.rb +++ b/spec/lib/gitlab/background_migration/backfill_project_repositories_spec.rb @@ -90,7 +90,7 @@ describe Gitlab::BackgroundMigration::BackfillProjectRepositories do it 'raises OrphanedNamespaceError when any parent namespace does not exist' do subgroup = create(:group, parent: group) project_orphaned_namespace = create(:project, name: 'baz', path: 'baz', namespace: subgroup, storage_version: nil) - subgroup.update_column(:parent_id, Namespace.maximum(:id).succ) + subgroup.update_column(:parent_id, non_existing_record_id) project = described_class.find(project_orphaned_namespace.id) project.route.destroy diff --git a/spec/lib/gitlab/background_migration/populate_user_highest_roles_table_spec.rb b/spec/lib/gitlab/background_migration/populate_user_highest_roles_table_spec.rb new file mode 100644 index 00000000000..be661d5b83e --- /dev/null +++ b/spec/lib/gitlab/background_migration/populate_user_highest_roles_table_spec.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::BackgroundMigration::PopulateUserHighestRolesTable, schema: 20200311130802 do + let(:members) { table(:members) } + let(:users) { table(:users) } + let(:user_highest_roles) { table(:user_highest_roles) } + + def create_user(id, params = {}) + user_params = { + id: id, + state: 'active', + user_type: nil, + bot_type: nil, + ghost: nil, + email: "user#{id}@example.com", + projects_limit: 0 + }.merge(params) + + users.create(user_params) + end + + def create_member(id, access_level, params = {}) + params = { + user_id: id, + access_level: access_level, + source_id: 1, + source_type: 'Group', + notification_level: 0 + }.merge(params) + + members.create(params) + end + + before do + create_user(1) + create_user(2, state: 'blocked') + create_user(3, user_type: 2) + create_user(4) + create_user(5, bot_type: 1) + create_user(6, ghost: true) + create_user(7, ghost: false) + create_user(8) + + create_member(1, 40) + create_member(7, 30) + create_member(8, 20, requested_at: Time.current) + + user_highest_roles.create(user_id: 1, highest_access_level: 50) + end + + describe '#perform' do + it 'creates user_highest_roles rows according to users', :aggregate_failures do + expect { subject.perform(1, 8) }.to change(UserHighestRole, :count).from(1).to(4) + + created_or_updated_rows = [ + { 'user_id' => 1, 'highest_access_level' => 40 }, + { 'user_id' => 4, 'highest_access_level' => nil }, + { 'user_id' => 7, 'highest_access_level' => 30 }, + { 'user_id' => 8, 'highest_access_level' => nil } + ] + + rows = user_highest_roles.order(:user_id).map do |row| + row.attributes.slice('user_id', 'highest_access_level') + end + + expect(rows).to match_array(created_or_updated_rows) + end + end +end diff --git a/spec/lib/gitlab/background_migration/recalculate_project_authorizations_spec.rb b/spec/lib/gitlab/background_migration/recalculate_project_authorizations_spec.rb index 4699cc42b38..ba87312e2bf 100644 --- a/spec/lib/gitlab/background_migration/recalculate_project_authorizations_spec.rb +++ b/spec/lib/gitlab/background_migration/recalculate_project_authorizations_spec.rb @@ -234,10 +234,8 @@ describe Gitlab::BackgroundMigration::RecalculateProjectAuthorizations, schema: end context 'deleted user' do - let(:nonexistent_user_id) { User.maximum(:id).to_i + 999 } - it 'does not fail' do - expect { described_class.new.perform([nonexistent_user_id]) }.not_to raise_error + expect { described_class.new.perform([non_existing_record_id]) }.not_to raise_error end end end diff --git a/spec/lib/gitlab/ci/status/composite_spec.rb b/spec/lib/gitlab/ci/status/composite_spec.rb index b9d4c39e0c2..8a226b382b0 100644 --- a/spec/lib/gitlab/ci/status/composite_spec.rb +++ b/spec/lib/gitlab/ci/status/composite_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' describe Gitlab::Ci::Status::Composite do let_it_be(:pipeline) { create(:ci_pipeline) } - before(:all) do + before_all do @statuses = HasStatus::STATUSES_ENUM.map do |status, idx| [status, create(:ci_build, pipeline: pipeline, status: status, importing: true)] end.to_h diff --git a/spec/lib/gitlab/import_export/group/tree_restorer_spec.rb b/spec/lib/gitlab/import_export/group/tree_restorer_spec.rb index 5584f1503f7..fc7e4737d13 100644 --- a/spec/lib/gitlab/import_export/group/tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/group/tree_restorer_spec.rb @@ -8,7 +8,7 @@ describe Gitlab::ImportExport::Group::TreeRestorer do let(:shared) { Gitlab::ImportExport::Shared.new(group) } describe 'restore group tree' do - before(:context) do + before_all do # Using an admin for import, so we can check assignment of existing members user = create(:admin, email: 'root@gitlabexample.com') create(:user, email: 'adriene.mcclure@gitlabexample.com') diff --git a/spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb b/spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb new file mode 100644 index 00000000000..076f454895f --- /dev/null +++ b/spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb @@ -0,0 +1,98 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::ImportExport::JSON::StreamingSerializer do + let_it_be(:user) { create(:user) } + let_it_be(:release) { create(:release) } + let_it_be(:group) { create(:group) } + + let_it_be(:exportable) do + create(:project, + :public, + :repository, + :issues_disabled, + :wiki_enabled, + :builds_private, + description: 'description', + releases: [release], + group: group, + approvals_before_merge: 1) + end + let_it_be(:issue) do + create(:issue, + assignees: [user], + project: exportable) + end + + let(:exportable_path) { 'project' } + let(:json_writer) { instance_double('Gitlab::ImportExport::JSON::LegacyWriter') } + let(:hash) { { name: exportable.name, description: exportable.description }.stringify_keys } + let(:include) { [] } + + let(:relations_schema) do + { + only: [:name, :description], + include: include, + preload: { issues: nil } + } + end + + subject do + described_class.new(exportable, relations_schema, json_writer, exportable_path: exportable_path) + end + + describe '#execute' do + before do + allow(json_writer).to receive(:write_attributes).with(exportable_path, hash) + end + + it 'calls json_writer.write_attributes with proper params' do + subject.execute + end + + context 'with many relations' do + let(:include) do + [{ issues: { include: [] } }] + end + + it 'calls json_writer.write_relation_array with proper params' do + expect(json_writer).to receive(:write_relation_array).with(exportable_path, :issues, array_including(issue.to_json)) + + subject.execute + end + end + + context 'with single relation' do + let(:group_options) do + { include: [], only: [:name, :path, :description] } + end + let(:include) do + [{ group: group_options }] + end + + it 'calls json_writer.write_relation with proper params' do + expect(json_writer).to receive(:write_relation).with(exportable_path, :group, group.to_json(group_options)) + + subject.execute + end + end + + context 'with array relation' do + let(:project_member) { create(:project_member, user: user) } + let(:include) do + [{ project_members: { include: [] } }] + end + + before do + allow(exportable).to receive(:project_members).and_return([project_member]) + end + + it 'calls json_writer.write_relation_array with proper params' do + expect(json_writer).to receive(:write_relation_array).with(exportable_path, :project_members, array_including(project_member.to_json)) + + subject.execute + end + end + end +end diff --git a/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb index 229f45e9543..e7edbc7690d 100644 --- a/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb @@ -8,7 +8,7 @@ describe Gitlab::ImportExport::Project::TreeRestorer do let(:shared) { project.import_export_shared } describe 'restore project tree' do - before(:context) do + before_all do # Using an admin for import, so we can check assignment of existing members @user = create(:admin) @existing_members = [ diff --git a/spec/lib/gitlab/object_hierarchy_spec.rb b/spec/lib/gitlab/object_hierarchy_spec.rb index b16eccbcb2c..b72aeb6d601 100644 --- a/spec/lib/gitlab/object_hierarchy_spec.rb +++ b/spec/lib/gitlab/object_hierarchy_spec.rb @@ -165,13 +165,13 @@ describe Gitlab::ObjectHierarchy do end it 'uses ancestors_base #initialize argument for ancestors' do - relation = described_class.new(Group.where(id: child1.id), Group.where(id: Group.maximum(:id).succ)).all_objects + relation = described_class.new(Group.where(id: child1.id), Group.where(id: non_existing_record_id)).all_objects expect(relation).to include(parent) end it 'uses descendants_base #initialize argument for descendants' do - relation = described_class.new(Group.where(id: Group.maximum(:id).succ), Group.where(id: child1.id)).all_objects + relation = described_class.new(Group.where(id: non_existing_record_id), Group.where(id: child1.id)).all_objects expect(relation).to include(child2) end diff --git a/spec/lib/gitlab/project_authorizations_spec.rb b/spec/lib/gitlab/project_authorizations_spec.rb index d1c441c8605..261e44bc5fa 100644 --- a/spec/lib/gitlab/project_authorizations_spec.rb +++ b/spec/lib/gitlab/project_authorizations_spec.rb @@ -234,7 +234,7 @@ describe Gitlab::ProjectAuthorizations do end context 'unrelated project owner' do - let(:common_id) { [Project.maximum(:id).to_i, Namespace.maximum(:id).to_i].max + 999 } + let(:common_id) { non_existing_record_id } let!(:group) { create(:group, id: common_id) } let!(:unrelated_project) { create(:project, id: common_id) } let(:user) { unrelated_project.owner } diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb index a7961190ff1..8ea591c6f74 100644 --- a/spec/lib/gitlab/reference_extractor_spec.rb +++ b/spec/lib/gitlab/reference_extractor_spec.rb @@ -130,7 +130,7 @@ describe Gitlab::ReferenceExtractor do @i0 = create(:issue, project: project) @i1 = create(:issue, project: project) - subject.analyze("#{@i0.to_reference}, #{@i1.to_reference}, and #{Issue.reference_prefix}999.") + subject.analyze("#{@i0.to_reference}, #{@i1.to_reference}, and #{Issue.reference_prefix}#{non_existing_record_iid}.") expect(subject.issues).to match_array([@i0, @i1]) end @@ -139,7 +139,7 @@ describe Gitlab::ReferenceExtractor do @m0 = create(:merge_request, source_project: project, target_project: project, source_branch: 'markdown') @m1 = create(:merge_request, source_project: project, target_project: project, source_branch: 'feature_conflict') - subject.analyze("!999, !#{@m1.iid}, and !#{@m0.iid}.") + subject.analyze("!#{non_existing_record_iid}, !#{@m1.iid}, and !#{@m0.iid}.") expect(subject.merge_requests).to match_array([@m1, @m0]) end @@ -149,7 +149,7 @@ describe Gitlab::ReferenceExtractor do @l1 = create(:label, title: 'two', project: project) @l2 = create(:label) - subject.analyze("~#{@l0.id}, ~999, ~#{@l2.id}, ~#{@l1.id}") + subject.analyze("~#{@l0.id}, ~#{non_existing_record_id}, ~#{@l2.id}, ~#{@l1.id}") expect(subject.labels).to match_array([@l0, @l1]) end @@ -159,7 +159,7 @@ describe Gitlab::ReferenceExtractor do @s1 = create(:project_snippet, project: project) @s2 = create(:project_snippet) - subject.analyze("$#{@s0.id}, $999, $#{@s2.id}, $#{@s1.id}") + subject.analyze("$#{@s0.id}, $#{non_existing_record_id}, $#{@s2.id}, $#{@s1.id}") expect(subject.snippets).to match_array([@s0, @s1]) end @@ -205,7 +205,7 @@ describe Gitlab::ReferenceExtractor do end it 'returns only Jira issues if the internal one does not exists' do - subject.analyze("JIRA-123 and FOOBAR-4567 and #999") + subject.analyze("JIRA-123 and FOOBAR-4567 and ##{non_existing_record_iid}") expect(subject.issues).to eq [ExternalIssue.new('JIRA-123', project), ExternalIssue.new('FOOBAR-4567', project)] end diff --git a/spec/migrations/schedule_populate_user_highest_roles_table_spec.rb b/spec/migrations/schedule_populate_user_highest_roles_table_spec.rb new file mode 100644 index 00000000000..67e0b994265 --- /dev/null +++ b/spec/migrations/schedule_populate_user_highest_roles_table_spec.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20200311130802_schedule_populate_user_highest_roles_table.rb') + +describe SchedulePopulateUserHighestRolesTable do + let(:users) { table(:users) } + + def create_user(id, params = {}) + user_params = { + id: id, + state: 'active', + user_type: nil, + bot_type: nil, + ghost: nil, + email: "user#{id}@example.com", + projects_limit: 0 + }.merge(params) + + users.create!(user_params) + end + + it 'correctly schedules background migrations' do + create_user(1) + create_user(2, state: 'blocked') + create_user(3, user_type: 2) + create_user(4) + create_user(5, bot_type: 1) + create_user(6, ghost: true) + create_user(7, ghost: false) + + stub_const("#{described_class.name}::BATCH_SIZE", 2) + + Sidekiq::Testing.fake! do + Timecop.freeze do + migrate! + + expect(described_class::MIGRATION).to be_scheduled_delayed_migration(5.minutes, 1, 4) + + expect(described_class::MIGRATION).to be_scheduled_delayed_migration(10.minutes, 7, 7) + + expect(BackgroundMigrationWorker.jobs.size).to eq(2) + end + end + end +end diff --git a/spec/models/active_session_spec.rb b/spec/models/active_session_spec.rb index bff3ac313c4..27a80f93566 100644 --- a/spec/models/active_session_spec.rb +++ b/spec/models/active_session_spec.rb @@ -165,10 +165,10 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do ActiveSession.set(user, request) Gitlab::Redis::SharedState.with do |redis| - expect(redis.scan_each.to_a).to match_array [ + expect(redis.scan_each.to_a).to include( "session:user:gitlab:#{user.id}:6919a6f1bb119dd7396fadc38fd18d0d", "session:lookup:user:gitlab:#{user.id}" - ] + ) end end diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 0e8ae320405..cd9d7c16f88 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -597,97 +597,49 @@ describe Member do end context 'when after_commit :update_highest_role' do - context 'with feature flag enabled' do - where(:member_type, :source_type) do - :project_member | :project - :group_member | :group - end + where(:member_type, :source_type) do + :project_member | :project + :group_member | :group + end - with_them do - describe 'create member' do - it 'initializes a new Members::UpdateHighestRoleService object' do - source = create(source_type) # source owner initializes a new service object too - user = create(:user) + with_them do + describe 'create member' do + it 'initializes a new Members::UpdateHighestRoleService object' do + source = create(source_type) # source owner initializes a new service object too + user = create(:user) - expect(Members::UpdateHighestRoleService).to receive(:new).with(user.id).and_call_original + expect(Members::UpdateHighestRoleService).to receive(:new).with(user.id).and_call_original - create(member_type, :guest, user: user, source_type => source) - end + create(member_type, :guest, user: user, source_type => source) end + end - context 'when member exists' do - let!(:member) { create(member_type) } - - describe 'update member' do - context 'when access level was changed' do - it 'initializes a new Members::UpdateHighestRoleService object' do - expect(Members::UpdateHighestRoleService).to receive(:new).with(member.user_id).and_call_original - - member.update(access_level: Gitlab::Access::GUEST) - end - end - - context 'when access level was not changed' do - it 'does not initialize a new Members::UpdateHighestRoleService object' do - expect(Members::UpdateHighestRoleService).not_to receive(:new).with(member.user_id) - - member.update(notification_level: NotificationSetting.levels[:disabled]) - end - end - end + context 'when member exists' do + let!(:member) { create(member_type) } - describe 'destroy member' do + describe 'update member' do + context 'when access level was changed' do it 'initializes a new Members::UpdateHighestRoleService object' do expect(Members::UpdateHighestRoleService).to receive(:new).with(member.user_id).and_call_original - member.destroy + member.update(access_level: Gitlab::Access::GUEST) end end - end - end - end - - context 'with feature flag disabled' do - before do - stub_feature_flags(highest_role_callback: false) - end - - where(:member_type, :source_type) do - :project_member | :project - :group_member | :group - end - - with_them do - describe 'create member' do - it 'does not initialize a new Members::UpdateHighestRoleService object' do - source = create(source_type) - user = create(:user) - - expect(Members::UpdateHighestRoleService).not_to receive(:new).with(user.id) - - create(member_type, :guest, user: user, source_type => source) - end - end - context 'when member exists' do - let!(:member) { create(member_type) } - - describe 'update member' do - context 'when access level was changed' do - it 'does not initialize a new Members::UpdateHighestRoleService object' do - expect(Members::UpdateHighestRoleService).not_to receive(:new).with(member.user_id) + context 'when access level was not changed' do + it 'does not initialize a new Members::UpdateHighestRoleService object' do + expect(Members::UpdateHighestRoleService).not_to receive(:new).with(member.user_id) - member.update(access_level: Gitlab::Access::GUEST) - end + member.update(notification_level: NotificationSetting.levels[:disabled]) end end + end - describe 'destroy member' do - it 'does not initialize a new Members::UpdateHighestRoleService object' do - expect(Members::UpdateHighestRoleService).not_to receive(:new).with(member.user_id) + describe 'destroy member' do + it 'initializes a new Members::UpdateHighestRoleService object' do + expect(Members::UpdateHighestRoleService).to receive(:new).with(member.user_id).and_call_original - member.destroy - end + member.destroy end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 694c6935c1d..508098bfc39 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -827,7 +827,7 @@ describe Project do end it 'returns nil when no issue found' do - expect(project.get_issue(999, user)).to be_nil + expect(project.get_issue(non_existing_record_id, user)).to be_nil end it "returns nil when user doesn't have access" do diff --git a/spec/models/user_preference_spec.rb b/spec/models/user_preference_spec.rb index 7884b87cc26..cf32d4eeca7 100644 --- a/spec/models/user_preference_spec.rb +++ b/spec/models/user_preference_spec.rb @@ -53,7 +53,7 @@ describe UserPreference do it 'returns the current notes filter' do user_preference.set_notes_filter(only_comments, issuable) - expect(user_preference.set_notes_filter(9999, issuable)).to eq(only_comments) + expect(user_preference.set_notes_filter(non_existing_record_id, issuable)).to eq(only_comments) end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index f16b1fb136d..b21841ad034 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1221,6 +1221,10 @@ describe User, :do_not_mock_admin_mode do end it 'uses SecureRandom to generate the incoming email token' do + allow_next_instance_of(User) do |user| + allow(user).to receive(:update_highest_role) + end + expect(SecureRandom).to receive(:hex).and_return('3b8ca303') user = create(:user) @@ -4441,4 +4445,52 @@ describe User, :do_not_mock_admin_mode do end end end + + context 'when after_commit :update_highest_role' do + describe 'create user' do + it 'initializes a new Members::UpdateHighestRoleService object' do + expect_next_instance_of(Members::UpdateHighestRoleService) do |service| + expect(service).to receive(:execute) + end + + create(:user) + end + end + + context 'when user already exists' do + let!(:user) { create(:user) } + + describe 'update user' do + using RSpec::Parameterized::TableSyntax + + where(:attributes) do + [ + { state: 'blocked' }, + { ghost: true }, + { user_type: :alert_bot } + ] + end + + with_them do + context 'when state was changed' do + it 'initializes a new Members::UpdateHighestRoleService object' do + expect_next_instance_of(Members::UpdateHighestRoleService) do |service| + expect(service).to receive(:execute) + end + + user.update(attributes) + end + end + end + + context 'when state was not changed' do + it 'does not initialize a new Members::UpdateHighestRoleService object' do + expect(Members::UpdateHighestRoleService).not_to receive(:new) + + user.update(email: 'newmail@example.com') + end + end + end + end + end end diff --git a/spec/rake_helper.rb b/spec/rake_helper.rb index 77fece5842d..b47240658e3 100644 --- a/spec/rake_helper.rb +++ b/spec/rake_helper.rb @@ -17,5 +17,7 @@ RSpec.configure do |config| # Reset stdout config.after(:all) do $stdout = STDOUT + + delete_from_all_tables! end end diff --git a/spec/requests/api/award_emoji_spec.rb b/spec/requests/api/award_emoji_spec.rb index fea1e3ac836..543fe970abd 100644 --- a/spec/requests/api/award_emoji_spec.rb +++ b/spec/requests/api/award_emoji_spec.rb @@ -26,7 +26,7 @@ describe API::AwardEmoji do end it "returns a 404 error when issue id not found" do - get api("/projects/#{project.id}/issues/12345/award_emoji", user) + get api("/projects/#{project.id}/issues/#{non_existing_record_iid}/award_emoji", user) expect(response).to have_gitlab_http_status(:not_found) end @@ -91,7 +91,7 @@ describe API::AwardEmoji do end it "returns a 404 error if the award is not found" do - get api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji/12345", user) + get api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) end @@ -255,7 +255,7 @@ describe API::AwardEmoji do end it 'returns a 404 error when the award emoji can not be found' do - delete api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji/12345", user) + delete api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) end @@ -275,7 +275,7 @@ describe API::AwardEmoji do end it 'returns a 404 error when note id not found' do - delete api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/notes/12345", user) + delete api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/notes/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) end diff --git a/spec/requests/api/environments_spec.rb b/spec/requests/api/environments_spec.rb index 4e2dfe7725e..91b3dd93433 100644 --- a/spec/requests/api/environments_spec.rb +++ b/spec/requests/api/environments_spec.rb @@ -127,7 +127,7 @@ describe API::Environments do end it 'returns a 400 when the required params are missing' do - post api("/projects/12345/environments", non_member), params: { external_url: 'http://env.git.com' } + post api("/projects/#{non_existing_record_id}/environments", non_member), params: { external_url: 'http://env.git.com' } end end end @@ -163,7 +163,7 @@ describe API::Environments do end it 'returns a 404 if the environment does not exist' do - put api("/projects/#{project.id}/environments/12345", user) + put api("/projects/#{project.id}/environments/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) end @@ -186,7 +186,7 @@ describe API::Environments do end it 'returns a 404 for non existing id' do - delete api("/projects/#{project.id}/environments/12345", user) + delete api("/projects/#{project.id}/environments/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Not found') @@ -229,7 +229,7 @@ describe API::Environments do end it 'returns a 404 for non existing id' do - post api("/projects/#{project.id}/environments/12345/stop", user) + post api("/projects/#{project.id}/environments/#{non_existing_record_id}/stop", user) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Not found') diff --git a/spec/requests/api/group_container_repositories_spec.rb b/spec/requests/api/group_container_repositories_spec.rb index 785006253d8..9f439bb2167 100644 --- a/spec/requests/api/group_container_repositories_spec.rb +++ b/spec/requests/api/group_container_repositories_spec.rb @@ -47,7 +47,7 @@ describe API::GroupContainerRepositories do it_behaves_like 'a gitlab tracking event', described_class.name, 'list_repositories' context 'with invalid group id' do - let(:url) { '/groups/123412341234/registry/repositories' } + let(:url) { "/groups/#{non_existing_record_id}/registry/repositories" } it 'returns not found' do subject diff --git a/spec/requests/api/group_import_spec.rb b/spec/requests/api/group_import_spec.rb index 3f85428aac2..58bff08dcbb 100644 --- a/spec/requests/api/group_import_spec.rb +++ b/spec/requests/api/group_import_spec.rb @@ -97,7 +97,7 @@ describe API::GroupImport do context 'when parent group is invalid' do it 'returns 404 and does not create new group' do - params[:parent_id] = 99999 + params[:parent_id] = non_existing_record_id expect { subject }.not_to change { Group.count } diff --git a/spec/requests/api/group_labels_spec.rb b/spec/requests/api/group_labels_spec.rb index 231a055f73c..715c1255cb3 100644 --- a/spec/requests/api/group_labels_spec.rb +++ b/spec/requests/api/group_labels_spec.rb @@ -334,7 +334,7 @@ describe API::GroupLabels do context 'when label ID is not found' do it 'returns 404 error' do - post api("/groups/#{group.id}/labels/1234/subscribe", user) + post api("/groups/#{group.id}/labels/#{non_existing_record_id}/subscribe", user) expect(response).to have_gitlab_http_status(:not_found) end @@ -380,7 +380,7 @@ describe API::GroupLabels do context 'when label ID is not found' do it 'returns 404 error' do - post api("/groups/#{group.id}/labels/1234/unsubscribe", user) + post api("/groups/#{group.id}/labels/#{non_existing_record_id}/unsubscribe", user) expect(response).to have_gitlab_http_status(:not_found) end diff --git a/spec/requests/api/import_github_spec.rb b/spec/requests/api/import_github_spec.rb index 1a7d3b18d11..ecdb02beeb0 100644 --- a/spec/requests/api/import_github_spec.rb +++ b/spec/requests/api/import_github_spec.rb @@ -34,7 +34,7 @@ describe API::ImportGithub do post api("/import/github", user), params: { target_namespace: user.namespace_path, personal_access_token: token, - repo_id: 1234 + repo_id: non_existing_record_id } expect(response).to have_gitlab_http_status(:created) expect(json_response).to be_a Hash @@ -47,7 +47,7 @@ describe API::ImportGithub do post api("/import/github", user), params: { target_namespace: other_namespace.name, personal_access_token: token, - repo_id: 1234 + repo_id: non_existing_record_id } expect(response).to have_gitlab_http_status(:unprocessable_entity) diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb index 06ba0bd30ff..575d695ef54 100644 --- a/spec/requests/api/internal/base_spec.rb +++ b/spec/requests/api/internal/base_spec.rb @@ -53,7 +53,7 @@ describe API::Internal::Base do post api('/internal/two_factor_recovery_codes'), params: { secret_token: secret_token, - key_id: 12345 + key_id: non_existing_record_id } expect(json_response['success']).to be_falsey @@ -152,13 +152,13 @@ describe API::Internal::Base do end it 'returns a 404 when the wrong key is provided' do - lfs_auth_key(key.id + 12345, project) + lfs_auth_key(non_existing_record_id, project) expect(response).to have_gitlab_http_status(:not_found) end it 'returns a 404 when the wrong user is provided' do - lfs_auth_user(user.id + 12345, project) + lfs_auth_user(non_existing_record_id, project) expect(response).to have_gitlab_http_status(:not_found) end diff --git a/spec/requests/api/issues/get_project_issues_spec.rb b/spec/requests/api/issues/get_project_issues_spec.rb index 1e6a1d2ed20..4a728c81215 100644 --- a/spec/requests/api/issues/get_project_issues_spec.rb +++ b/spec/requests/api/issues/get_project_issues_spec.rb @@ -78,7 +78,7 @@ describe API::Issues do description: "closes #{issue.to_reference(private_mrs_project)}") end - before(:all) do + before_all do project.add_reporter(user) project.add_guest(guest) private_mrs_project.add_reporter(user) diff --git a/spec/requests/api/issues/issues_spec.rb b/spec/requests/api/issues/issues_spec.rb index d9496d6646f..00169c1529f 100644 --- a/spec/requests/api/issues/issues_spec.rb +++ b/spec/requests/api/issues/issues_spec.rb @@ -61,7 +61,7 @@ describe API::Issues do let(:no_milestone_title) { 'None' } let(:any_milestone_title) { 'Any' } - before(:all) do + before_all do project.add_reporter(user) project.add_guest(guest) private_mrs_project.add_reporter(user) diff --git a/spec/requests/api/issues/post_projects_issues_spec.rb b/spec/requests/api/issues/post_projects_issues_spec.rb index 5b8d45d9465..be48113c215 100644 --- a/spec/requests/api/issues/post_projects_issues_spec.rb +++ b/spec/requests/api/issues/post_projects_issues_spec.rb @@ -60,7 +60,7 @@ describe API::Issues do let(:no_milestone_title) { 'None' } let(:any_milestone_title) { 'Any' } - before(:all) do + before_all do project.add_reporter(user) project.add_guest(guest) end diff --git a/spec/requests/api/issues/put_projects_issues_spec.rb b/spec/requests/api/issues/put_projects_issues_spec.rb index 51037f701e1..ffc5e2b1db8 100644 --- a/spec/requests/api/issues/put_projects_issues_spec.rb +++ b/spec/requests/api/issues/put_projects_issues_spec.rb @@ -61,7 +61,7 @@ describe API::Issues do let(:no_milestone_title) { 'None' } let(:any_milestone_title) { 'Any' } - before(:all) do + before_all do project.add_reporter(user) project.add_guest(guest) end diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb index 19d4e8cef20..1f17359a473 100644 --- a/spec/requests/api/labels_spec.rb +++ b/spec/requests/api/labels_spec.rb @@ -596,7 +596,7 @@ describe API::Labels do context "when label ID is not found" do it "returns 404 error" do - post api("/projects/#{project.id}/labels/1234/subscribe", user) + post api("/projects/#{project.id}/labels/#{non_existing_record_id}/subscribe", user) expect(response).to have_gitlab_http_status(:not_found) end @@ -642,7 +642,7 @@ describe API::Labels do context "when label ID is not found" do it "returns 404 error" do - post api("/projects/#{project.id}/labels/1234/unsubscribe", user) + post api("/projects/#{project.id}/labels/#{non_existing_record_id}/unsubscribe", user) expect(response).to have_gitlab_http_status(:not_found) end diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index 0b504df1f51..0ecef26c27a 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -314,9 +314,9 @@ describe API::Members do expect(response).to have_gitlab_http_status(:bad_request) end - it 'returns 400 when access_level is not valid' do + it 'returns 400 when access_level is not valid' do post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), - params: { user_id: stranger.id, access_level: 1234 } + params: { user_id: stranger.id, access_level: non_existing_record_access_level } expect(response).to have_gitlab_http_status(:bad_request) end @@ -371,9 +371,9 @@ describe API::Members do expect(response).to have_gitlab_http_status(:bad_request) end - it 'returns 400 when access level is not valid' do + it 'returns 400 when access level is not valid' do put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", maintainer), - params: { access_level: 1234 } + params: { access_level: non_existing_record_access_level } expect(response).to have_gitlab_http_status(:bad_request) end diff --git a/spec/requests/api/merge_request_diffs_spec.rb b/spec/requests/api/merge_request_diffs_spec.rb index 90b6f39cc90..d00bc4a6dde 100644 --- a/spec/requests/api/merge_request_diffs_spec.rb +++ b/spec/requests/api/merge_request_diffs_spec.rb @@ -60,7 +60,7 @@ describe API::MergeRequestDiffs, 'MergeRequestDiffs' do end it 'returns a 404 when merge_request_iid is not found' do - get api("/projects/#{project.id}/merge_requests/12345/versions/#{merge_request_diff.id}", user) + get api("/projects/#{project.id}/merge_requests/#{non_existing_record_iid}/versions/#{merge_request_diff.id}", user) expect(response).to have_gitlab_http_status(:not_found) end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 403e182ad99..a8543c8e282 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -1720,7 +1720,7 @@ describe API::MergeRequests do end it "returns 404 for an invalid merge request IID" do - delete api("/projects/#{project.id}/merge_requests/12345", user) + delete api("/projects/#{project.id}/merge_requests/#{non_existing_record_iid}", user) expect(response).to have_gitlab_http_status(:not_found) end @@ -1931,7 +1931,7 @@ describe API::MergeRequests do end it "returns 404 for an invalid merge request IID" do - put api("/projects/#{project.id}/merge_requests/12345/merge", user) + put api("/projects/#{project.id}/merge_requests/#{non_existing_record_iid}/merge", user) expect(response).to have_gitlab_http_status(:not_found) end @@ -2053,7 +2053,7 @@ describe API::MergeRequests do end context 'when invalid merge request IID' do - let(:merge_request_iid) { '12345' } + let(:merge_request_iid) { non_existing_record_iid } it 'returns 404' do get api(url, user) @@ -2301,7 +2301,7 @@ describe API::MergeRequests do end it "returns 404 for an invalid merge request IID" do - put api("/projects/#{project.id}/merge_requests/12345", user), params: { state_event: "close" } + put api("/projects/#{project.id}/merge_requests/#{non_existing_record_iid}", user), params: { state_event: "close" } expect(response).to have_gitlab_http_status(:not_found) end @@ -2366,7 +2366,7 @@ describe API::MergeRequests do end it "returns 404 for an invalid merge request IID" do - get api("/projects/#{project.id}/merge_requests/12345/closes_issues", user) + get api("/projects/#{project.id}/merge_requests/#{non_existing_record_iid}/closes_issues", user) expect(response).to have_gitlab_http_status(:not_found) end diff --git a/spec/requests/api/pipelines_spec.rb b/spec/requests/api/pipelines_spec.rb index 6b824690e6c..a8f21f64a72 100644 --- a/spec/requests/api/pipelines_spec.rb +++ b/spec/requests/api/pipelines_spec.rb @@ -442,7 +442,7 @@ describe API::Pipelines do end it 'returns 404 when it does not exist' do - get api("/projects/#{project.id}/pipelines/123456", user) + get api("/projects/#{project.id}/pipelines/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq '404 Not found' @@ -599,7 +599,7 @@ describe API::Pipelines do end it 'returns 404 when it does not exist' do - delete api("/projects/#{project.id}/pipelines/123456", owner) + delete api("/projects/#{project.id}/pipelines/#{non_existing_record_id}", owner) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq '404 Not found' diff --git a/spec/requests/api/project_events_spec.rb b/spec/requests/api/project_events_spec.rb index 3fa3d4fa899..489b83de434 100644 --- a/spec/requests/api/project_events_spec.rb +++ b/spec/requests/api/project_events_spec.rb @@ -107,7 +107,7 @@ describe API::ProjectEvents do end it 'returns 404 if project does not exist' do - get api("/projects/1234/events", user) + get api("/projects/#{non_existing_record_id}/events", user) expect(response).to have_gitlab_http_status(:not_found) end diff --git a/spec/requests/api/project_hooks_spec.rb b/spec/requests/api/project_hooks_spec.rb index 540b30e2969..4474f2f0577 100644 --- a/spec/requests/api/project_hooks_spec.rb +++ b/spec/requests/api/project_hooks_spec.rb @@ -75,7 +75,7 @@ describe API::ProjectHooks, 'ProjectHooks' do end it "returns a 404 error if hook id is not available" do - get api("/projects/#{project.id}/hooks/1234", user) + get api("/projects/#{project.id}/hooks/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) end @@ -180,7 +180,7 @@ describe API::ProjectHooks, 'ProjectHooks' do end it "returns 404 error if hook id not found" do - put api("/projects/#{project.id}/hooks/1234", user), params: { url: 'http://example.org' } + put api("/projects/#{project.id}/hooks/#{non_existing_record_id}", user), params: { url: 'http://example.org' } expect(response).to have_gitlab_http_status(:not_found) end diff --git a/spec/requests/api/project_snippets_spec.rb b/spec/requests/api/project_snippets_spec.rb index 1af5d553bf0..89ade15c1f6 100644 --- a/spec/requests/api/project_snippets_spec.rb +++ b/spec/requests/api/project_snippets_spec.rb @@ -110,7 +110,7 @@ describe API::ProjectSnippets do end it 'returns 404 for invalid snippet id' do - get api("/projects/#{project.id}/snippets/1234", user) + get api("/projects/#{project.id}/snippets/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Not found') @@ -349,7 +349,7 @@ describe API::ProjectSnippets do end it 'returns 404 for invalid snippet id' do - update_snippet(snippet_id: '1234', params: { title: 'foo' }) + update_snippet(snippet_id: non_existing_record_id, params: { title: 'foo' }) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Snippet Not Found') @@ -442,7 +442,7 @@ describe API::ProjectSnippets do end it 'returns 404 for invalid snippet id' do - delete api("/projects/#{snippet.project.id}/snippets/1234", admin) + delete api("/projects/#{snippet.project.id}/snippets/#{non_existing_record_id}", admin) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Snippet Not Found') @@ -471,7 +471,7 @@ describe API::ProjectSnippets do end it 'returns 404 for invalid snippet id' do - get api("/projects/#{snippet.project.id}/snippets/1234/raw", admin) + get api("/projects/#{snippet.project.id}/snippets/#{non_existing_record_id}/raw", admin) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Snippet Not Found') diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index a50b2b7aca8..190afb9cda5 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -1101,7 +1101,7 @@ describe API::Projects do end it 'returns error when user not found' do - get api('/users/9999/starred_projects/') + get api("/users/#{non_existing_record_id}/starred_projects/") expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 User Not Found') @@ -2088,13 +2088,13 @@ describe API::Projects do end it 'returns a 404 error when group does not exist' do - post api("/projects/#{project.id}/share", user), params: { group_id: 1234, group_access: Gitlab::Access::DEVELOPER } + post api("/projects/#{project.id}/share", user), params: { group_id: non_existing_record_id, group_access: Gitlab::Access::DEVELOPER } expect(response).to have_gitlab_http_status(:not_found) end it "returns a 400 error when wrong params passed" do - post api("/projects/#{project.id}/share", user), params: { group_id: group.id, group_access: 1234 } + post api("/projects/#{project.id}/share", user), params: { group_id: group.id, group_access: non_existing_record_access_level } expect(response).to have_gitlab_http_status(:bad_request) expect(json_response['error']).to eq 'group_access does not have a valid value' @@ -2137,13 +2137,13 @@ describe API::Projects do end it 'returns a 404 error when group link does not exist' do - delete api("/projects/#{project.id}/share/1234", user) + delete api("/projects/#{project.id}/share/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) end it 'returns a 404 error when project does not exist' do - delete api("/projects/123/share/1234", user) + delete api("/projects/123/share/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) end @@ -2634,7 +2634,7 @@ describe API::Projects do end it 'returns not_found(404) for not existing project' do - get api("/projects/9999999999/starrers", user) + get api("/projects/#{non_existing_record_id}/starrers", user) expect(response).to have_gitlab_http_status(:not_found) end diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb index a96bc68ace9..0c66bfd6c4d 100644 --- a/spec/requests/api/repositories_spec.rb +++ b/spec/requests/api/repositories_spec.rb @@ -117,7 +117,7 @@ describe API::Repositories do context 'when sha does not exist' do it_behaves_like '404 response' do - let(:request) { get api(route.sub(sample_blob.oid, '123456'), current_user) } + let(:request) { get api(route.sub(sample_blob.oid, 'abcd9876'), current_user) } let(:message) { '404 Blob Not Found' } end end @@ -179,7 +179,7 @@ describe API::Repositories do context 'when sha does not exist' do it_behaves_like '404 response' do - let(:request) { get api(route.sub(sample_blob.oid, '123456'), current_user) } + let(:request) { get api(route.sub(sample_blob.oid, 'abcd9876'), current_user) } let(:message) { '404 Blob Not Found' } end end diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb index caa9d9251d8..3e30dc537e4 100644 --- a/spec/requests/api/snippets_spec.rb +++ b/spec/requests/api/snippets_spec.rb @@ -354,7 +354,7 @@ describe API::Snippets do it_behaves_like 'snippet updates' it 'returns 404 for invalid snippet id' do - update_snippet(snippet_id: '1234', params: { title: 'Foo' }) + update_snippet(snippet_id: non_existing_record_id, params: { title: 'Foo' }) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Snippet Not Found') @@ -441,7 +441,7 @@ describe API::Snippets do end it 'returns 404 for invalid snippet id' do - delete api("/snippets/1234", user) + delete api("/snippets/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Snippet Not Found') diff --git a/spec/requests/api/system_hooks_spec.rb b/spec/requests/api/system_hooks_spec.rb index 50015d2e2c3..609aa615d33 100644 --- a/spec/requests/api/system_hooks_spec.rb +++ b/spec/requests/api/system_hooks_spec.rb @@ -126,7 +126,7 @@ describe API::SystemHooks do end it 'returns 404 if the system hook does not exist' do - delete api('/hooks/12345', admin) + delete api("/hooks/#{non_existing_record_id}", admin) expect(response).to have_gitlab_http_status(:not_found) end diff --git a/spec/services/error_tracking/issue_update_service_spec.rb b/spec/services/error_tracking/issue_update_service_spec.rb index ffb15901f80..a13d42ec141 100644 --- a/spec/services/error_tracking/issue_update_service_spec.rb +++ b/spec/services/error_tracking/issue_update_service_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' describe ErrorTracking::IssueUpdateService do include_context 'sentry error tracking context' - let(:arguments) { { issue_id: 1234, status: 'resolved' } } + let(:arguments) { { issue_id: non_existing_record_id, status: 'resolved' } } subject(:update_service) { described_class.new(project, user, arguments) } diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 69e47d890a5..c32bef5a1a5 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -824,14 +824,14 @@ describe Issues::UpdateService, :mailer do context 'when moving an issue ' do it 'raises an error for invalid move ids within a project' do - opts = { move_between_ids: [9000, 9999] } + opts = { move_between_ids: [9000, non_existing_record_id] } expect { described_class.new(issue.project, user, opts).execute(issue) } .to raise_error(ActiveRecord::RecordNotFound) end it 'raises an error for invalid move ids within a group' do - opts = { move_between_ids: [9000, 9999], board_group_id: create(:group).id } + opts = { move_between_ids: [9000, non_existing_record_id], board_group_id: create(:group).id } expect { described_class.new(issue.project, user, opts).execute(issue) } .to raise_error(ActiveRecord::RecordNotFound) diff --git a/spec/services/labels/available_labels_service_spec.rb b/spec/services/labels/available_labels_service_spec.rb index ce120344f16..3ba1add121d 100644 --- a/spec/services/labels/available_labels_service_spec.rb +++ b/spec/services/labels/available_labels_service_spec.rb @@ -65,7 +65,7 @@ describe Labels::AvailableLabelsService do end describe '#filter_labels_ids_in_param' do - let(:label_ids) { labels.map(&:id).push(99999) } + let(:label_ids) { labels.map(&:id).push(non_existing_record_id) } context 'when parent is a project' do it 'returns only relevant label ids' do diff --git a/spec/services/members/update_highest_role_service_spec.rb b/spec/services/members/update_highest_role_service_spec.rb index b56a51f83f9..6fcb939203d 100644 --- a/spec/services/members/update_highest_role_service_spec.rb +++ b/spec/services/members/update_highest_role_service_spec.rb @@ -22,7 +22,9 @@ describe Members::UpdateHighestRoleService, :clean_gitlab_redis_shared_state do expect(service.exclusive_lease.exists?).to be_truthy end - it 'schedules a job' do + it 'schedules a job in the future', :aggregate_failures do + expect(UpdateHighestRoleWorker).to receive(:perform_in).with(described_class::DELAY, user.id).and_call_original + Sidekiq::Testing.fake! do expect { subject }.to change(UpdateHighestRoleWorker.jobs, :size).by(1) end diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb index 7ba069d1e39..a03b78a9a7a 100644 --- a/spec/services/notes/create_service_spec.rb +++ b/spec/services/notes/create_service_spec.rb @@ -50,7 +50,7 @@ describe Notes::CreateService do end it 'enqueues NewNoteWorker' do - note = build(:note, id: 999, project: project) + note = build(:note, id: non_existing_record_id, project: project) allow(Note).to receive(:new).with(opts) { note } expect(NewNoteWorker).to receive(:perform_async).with(note.id) diff --git a/spec/services/projects/participants_service_spec.rb b/spec/services/projects/participants_service_spec.rb index 6eaf7a71b23..f4a04159db4 100644 --- a/spec/services/projects/participants_service_spec.rb +++ b/spec/services/projects/participants_service_spec.rb @@ -92,7 +92,7 @@ describe Projects::ParticipantsService do let_it_be(:group_ancestor_owner) { create(:user) } - before(:context) do + before_all do public_group.add_owner public_group_owner private_group.add_developer private_group_member public_project.add_maintainer public_project_maintainer @@ -102,7 +102,7 @@ describe Projects::ParticipantsService do end context 'when the private group is invited to the public project' do - before(:context) do + before_all do create(:project_group_link, group: private_group, project: public_project) end diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb index 7db94d4a4ac..6cc2e2b6abe 100644 --- a/spec/services/quick_actions/interpret_service_spec.rb +++ b/spec/services/quick_actions/interpret_service_spec.rb @@ -1218,7 +1218,7 @@ describe QuickActions::InterpretService do end it_behaves_like 'empty command' do - let(:content) { "/copy_metadata imaginary#1234" } + let(:content) { "/copy_metadata imaginary##{non_existing_record_iid}" } let(:issuable) { issue } end @@ -1253,7 +1253,7 @@ describe QuickActions::InterpretService do end it_behaves_like 'empty command', _('Failed to mark this issue as a duplicate because referenced issue was not found.') do - let(:content) { "/duplicate imaginary#1234" } + let(:content) { "/duplicate imaginary##{non_existing_record_iid}" } let(:issuable) { issue } end diff --git a/spec/services/repository_archive_clean_up_service_spec.rb b/spec/services/repository_archive_clean_up_service_spec.rb index 60a14d7a107..c0b286ac675 100644 --- a/spec/services/repository_archive_clean_up_service_spec.rb +++ b/spec/services/repository_archive_clean_up_service_spec.rb @@ -9,7 +9,7 @@ describe RepositoryArchiveCleanUpService do let(:sha) { "0" * 40 } it 'removes outdated archives and directories in a new-style path' do - in_directory_with_files("project-999/#{sha}", %w[tar tar.bz2 tar.gz zip], 3.hours) do |dirname, files| + in_directory_with_files("project-#{non_existing_record_id}/#{sha}", %w[tar tar.bz2 tar.gz zip], 3.hours) do |dirname, files| service.execute files.each { |filename| expect(File.exist?(filename)).to be_falsy } @@ -19,7 +19,7 @@ describe RepositoryArchiveCleanUpService do end it 'does not remove directories when they contain outdated non-archives' do - in_directory_with_files("project-999/#{sha}", %w[tar conf rb], 2.hours) do |dirname, files| + in_directory_with_files("project-#{non_existing_record_id}/#{sha}", %w[tar conf rb], 2.hours) do |dirname, files| service.execute expect(File.directory?(dirname)).to be_truthy @@ -27,7 +27,7 @@ describe RepositoryArchiveCleanUpService do end it 'does not remove in-date archives in a new-style path' do - in_directory_with_files("project-999/#{sha}", %w[tar tar.bz2 tar.gz zip], 1.hour) do |dirname, files| + in_directory_with_files("project-#{non_existing_record_id}/#{sha}", %w[tar tar.bz2 tar.gz zip], 1.hour) do |dirname, files| service.execute files.each { |filename| expect(File.exist?(filename)).to be_truthy } diff --git a/spec/services/snippets/update_service_spec.rb b/spec/services/snippets/update_service_spec.rb index 03dc857c666..9c88e741d51 100644 --- a/spec/services/snippets/update_service_spec.rb +++ b/spec/services/snippets/update_service_spec.rb @@ -141,14 +141,16 @@ describe Snippets::UpdateService do end it 'returns error when the commit action fails' do + error_message = 'foobar' + allow_next_instance_of(SnippetRepository) do |instance| - allow(instance).to receive(:multi_files_action).and_raise(SnippetRepository::CommitError) + allow(instance).to receive(:multi_files_action).and_raise(SnippetRepository::CommitError, error_message) end response = subject expect(response).to be_error - expect(response.payload[:snippet].errors.full_messages).to eq ['Repository Error updating the snippet'] + expect(response.payload[:snippet].errors[:repository].to_sentence).to eq error_message end end @@ -168,12 +170,14 @@ describe Snippets::UpdateService do end context 'when an error is raised' do + let(:error_message) { 'foobar' } + before do - allow(snippet.snippet_repository).to receive(:multi_files_action).and_raise(SnippetRepository::CommitError, 'foobar') + allow(snippet.snippet_repository).to receive(:multi_files_action).and_raise(SnippetRepository::CommitError, error_message) end it 'logs the error' do - expect(Gitlab::AppLogger).to receive(:error).with('foobar') + expect(Gitlab::AppLogger).to receive(:error).with(error_message) subject end @@ -182,7 +186,7 @@ describe Snippets::UpdateService do response = subject expect(response).to be_error - expect(response.payload[:snippet].errors.full_messages).to eq ['Repository Error updating the snippet'] + expect(response.payload[:snippet].errors[:repository].to_sentence).to eq error_message end end diff --git a/spec/services/todos/destroy/entity_leave_service_spec.rb b/spec/services/todos/destroy/entity_leave_service_spec.rb index ce809bbf6c5..45e3bf381fb 100644 --- a/spec/services/todos/destroy/entity_leave_service_spec.rb +++ b/spec/services/todos/destroy/entity_leave_service_spec.rb @@ -298,7 +298,7 @@ describe Todos::Destroy::EntityLeaveService do context 'when entity was not found' do it 'does not remove any todos' do - expect { described_class.new(user.id, 999999, 'Group').execute } + expect { described_class.new(user.id, non_existing_record_id, 'Group').execute } .not_to change { Todo.count } end end diff --git a/spec/services/users/activity_service_spec.rb b/spec/services/users/activity_service_spec.rb index f477eee1dd6..5f068a2033c 100644 --- a/spec/services/users/activity_service_spec.rb +++ b/spec/services/users/activity_service_spec.rb @@ -30,7 +30,7 @@ describe Users::ActivityService do end it 'tries to obtain ExclusiveLease' do - expect(Gitlab::ExclusiveLease).to receive(:new).and_call_original + expect(Gitlab::ExclusiveLease).to receive(:new).with("activity_service:#{user.id}", anything).and_call_original subject.execute end @@ -56,7 +56,7 @@ describe Users::ActivityService do end it 'does not try to obtain ExclusiveLease' do - expect(Gitlab::ExclusiveLease).not_to receive(:new) + expect(Gitlab::ExclusiveLease).not_to receive(:new).with("activity_service:#{user.id}", anything) subject.execute end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 08320934ba1..0e6078cc444 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -100,6 +100,7 @@ RSpec.configure do |config| config.include ExpectOffense config.include FactoryBot::Syntax::Methods config.include FixtureHelpers + config.include NonExistingRecordsHelpers config.include GitlabRoutingHelper config.include StubFeatureFlags config.include StubExperiments diff --git a/spec/support/database_cleaner.rb b/spec/support/database_cleaner.rb index aaf408f6143..f6339d7343c 100644 --- a/spec/support/database_cleaner.rb +++ b/spec/support/database_cleaner.rb @@ -11,10 +11,6 @@ RSpec.configure do |config| DatabaseCleaner.clean_with(:truncation) end - config.append_after(:context) do - delete_from_all_tables! - end - config.append_after(:context, :migration) do delete_from_all_tables! diff --git a/spec/support/non_existing_records_helper.rb b/spec/support/non_existing_records_helper.rb new file mode 100644 index 00000000000..1d4368c8ddf --- /dev/null +++ b/spec/support/non_existing_records_helper.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +module NonExistingRecordsHelpers + # Max value for PG `serial` type: https://www.postgresql.org/docs/12/datatype-numeric.html + ACTIVE_MODEL_INTEGER_MAX = 2147483647 + + def non_existing_record_id + ACTIVE_MODEL_INTEGER_MAX + end + alias_method :non_existing_record_iid, :non_existing_record_id + alias_method :non_existing_record_access_level, :non_existing_record_id +end diff --git a/spec/support/shared_contexts/finders/issues_finder_shared_contexts.rb b/spec/support/shared_contexts/finders/issues_finder_shared_contexts.rb index fc62ae5a13f..9ffde54c84a 100644 --- a/spec/support/shared_contexts/finders/issues_finder_shared_contexts.rb +++ b/spec/support/shared_contexts/finders/issues_finder_shared_contexts.rb @@ -28,19 +28,10 @@ RSpec.shared_context 'IssuesFinder#execute context' do let(:params) { {} } let(:issues) { described_class.new(search_user, params.reverse_merge(scope: scope, state: 'opened')).execute } - before(:context) do + before_all do project1.add_maintainer(user) project2.add_developer(user) project2.add_developer(user2) project3.add_developer(user) - - issue1 - issue2 - issue3 - issue4 - - award_emoji1 - award_emoji2 - award_emoji3 end end diff --git a/spec/support/shared_examples/finders/snippet_visibility_shared_examples.rb b/spec/support/shared_examples/finders/snippet_visibility_shared_examples.rb index 5bd2da03f3f..c802038c9da 100644 --- a/spec/support/shared_examples/finders/snippet_visibility_shared_examples.rb +++ b/spec/support/shared_examples/finders/snippet_visibility_shared_examples.rb @@ -3,11 +3,6 @@ RSpec.shared_examples 'snippet visibility' do using RSpec::Parameterized::TableSyntax - # Make sure no snippets exist prior to running the test matrix - before(:context) do - DatabaseCleaner.clean_with(:truncation) - end - let_it_be(:author) { create(:user) } let_it_be(:member) { create(:user) } let_it_be(:external) { create(:user, :external) } diff --git a/spec/support/shared_examples/lib/gitlab/import_export/project_tree_restorer_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/import_export/project_tree_restorer_shared_examples.rb index 0e43f686327..5f9316c1cde 100644 --- a/spec/support/shared_examples/lib/gitlab/import_export/project_tree_restorer_shared_examples.rb +++ b/spec/support/shared_examples/lib/gitlab/import_export/project_tree_restorer_shared_examples.rb @@ -38,10 +38,9 @@ RSpec.shared_examples 'restores project successfully' do |**results| expect(project.external_pull_requests.size).to eq(results.fetch(:external_pull_requests, 0)) end - # This test is quarantined because the use of magic number 999 causes failure on CI - it 'does not set params that are excluded from import_export settings', quarantine: 'https://gitlab.com/gitlab-org/gitlab/issues/207932#note_293724442' do + it 'does not set params that are excluded from import_export settings' do expect(project.import_type).to be_nil - expect(project.creator_id).not_to eq 999 + expect(project.creator_id).not_to eq non_existing_record_id end it 'records exact number of import failures' do diff --git a/spec/support/shared_examples/models/mentionable_shared_examples.rb b/spec/support/shared_examples/models/mentionable_shared_examples.rb index b6b131338cc..ba6aa4e1d89 100644 --- a/spec/support/shared_examples/models/mentionable_shared_examples.rb +++ b/spec/support/shared_examples/models/mentionable_shared_examples.rb @@ -287,9 +287,9 @@ RSpec.shared_examples 'load mentions from DB' do |mentionable_type| before do user_mention = note.send(:model_user_mention) mention_ids = { - mentioned_users_ids: user_mention.mentioned_users_ids.to_a << User.maximum(:id).to_i.succ, - mentioned_projects_ids: user_mention.mentioned_projects_ids.to_a << Project.maximum(:id).to_i.succ, - mentioned_groups_ids: user_mention.mentioned_groups_ids.to_a << Group.maximum(:id).to_i.succ + mentioned_users_ids: user_mention.mentioned_users_ids.to_a << non_existing_record_id, + mentioned_projects_ids: user_mention.mentioned_projects_ids.to_a << non_existing_record_id, + mentioned_groups_ids: user_mention.mentioned_groups_ids.to_a << non_existing_record_id } user_mention.update(mention_ids) end diff --git a/spec/support/shared_examples/requests/api/discussions_shared_examples.rb b/spec/support/shared_examples/requests/api/discussions_shared_examples.rb index 939ea405724..6315c10b0c4 100644 --- a/spec/support/shared_examples/requests/api/discussions_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/discussions_shared_examples.rb @@ -66,7 +66,7 @@ RSpec.shared_examples 'discussions API' do |parent_type, noteable_type, id_name, end it "returns a 404 error when noteable id not found" do - get api("/#{parent_type}/#{parent.id}/#{noteable_type}/12345/discussions", user) + get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{non_existing_record_iid}/discussions", user) expect(response).to have_gitlab_http_status(:not_found) end @@ -90,7 +90,7 @@ RSpec.shared_examples 'discussions API' do |parent_type, noteable_type, id_name, end it "returns a 404 error if discussion not found" do - get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions/12345", user) + get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) end @@ -248,7 +248,7 @@ RSpec.shared_examples 'discussions API' do |parent_type, noteable_type, id_name, it 'returns a 404 error when note id not found' do put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\ - "discussions/#{note.discussion_id}/notes/12345", user), + "discussions/#{note.discussion_id}/notes/#{non_existing_record_id}", user), params: { body: 'Hello!' } expect(response).to have_gitlab_http_status(:not_found) @@ -276,7 +276,7 @@ RSpec.shared_examples 'discussions API' do |parent_type, noteable_type, id_name, it 'returns a 404 error when note id not found' do delete api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\ - "discussions/#{note.discussion_id}/notes/12345", user) + "discussions/#{note.discussion_id}/notes/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) end diff --git a/spec/support/shared_examples/requests/api/issuable_participants_examples.rb b/spec/support/shared_examples/requests/api/issuable_participants_examples.rb index e442b988349..673d7741017 100644 --- a/spec/support/shared_examples/requests/api/issuable_participants_examples.rb +++ b/spec/support/shared_examples/requests/api/issuable_participants_examples.rb @@ -18,7 +18,7 @@ RSpec.shared_examples 'issuable participants endpoint' do end it 'returns a 404 when iid does not exist' do - get api("/projects/#{project.id}/#{area}/999/participants", user) + get api("/projects/#{project.id}/#{area}/#{non_existing_record_iid}/participants", user) expect(response).to have_gitlab_http_status(:not_found) end diff --git a/spec/support/shared_examples/requests/api/milestones_shared_examples.rb b/spec/support/shared_examples/requests/api/milestones_shared_examples.rb index 3afb26bc869..77b49b7caef 100644 --- a/spec/support/shared_examples/requests/api/milestones_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/milestones_shared_examples.rb @@ -56,7 +56,7 @@ RSpec.shared_examples 'group and project milestones' do |route_definition| end it 'does not return any milestone if none found' do - get api(route, user), params: { iids: [Milestone.maximum(:iid).succ] } + get api(route, user), params: { iids: [non_existing_record_iid] } expect(response).to have_gitlab_http_status(:ok) expect(json_response).to be_an Array @@ -120,7 +120,7 @@ RSpec.shared_examples 'group and project milestones' do |route_definition| end it 'returns a 404 error if milestone id not found' do - get api("#{route}/1234", user) + get api("#{route}/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) end @@ -183,7 +183,7 @@ RSpec.shared_examples 'group and project milestones' do |route_definition| end it 'returns a 404 error if milestone id not found' do - put api("#{route}/1234", user), params: { title: 'updated title' } + put api("#{route}/#{non_existing_record_id}", user), params: { title: 'updated title' } expect(response).to have_gitlab_http_status(:not_found) end @@ -367,7 +367,7 @@ RSpec.shared_examples 'group and project milestones' do |route_definition| end it 'returns a 404 error if milestone id not found' do - not_found_route = "#{route}/1234/merge_requests" + not_found_route = "#{route}/#{non_existing_record_id}/merge_requests" get api(not_found_route, user) diff --git a/spec/support/shared_examples/requests/api/notes_shared_examples.rb b/spec/support/shared_examples/requests/api/notes_shared_examples.rb index 61ac9acb154..60ed61269df 100644 --- a/spec/support/shared_examples/requests/api/notes_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/notes_shared_examples.rb @@ -97,7 +97,7 @@ RSpec.shared_examples 'noteable API' do |parent_type, noteable_type, id_name| end it "returns a 404 error when noteable id not found" do - get api("/#{parent_type}/#{parent.id}/#{noteable_type}/12345/notes", user) + get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{non_existing_record_id}/notes", user) expect(response).to have_gitlab_http_status(:not_found) end @@ -120,7 +120,7 @@ RSpec.shared_examples 'noteable API' do |parent_type, noteable_type, id_name| end it "returns a 404 error if note not found" do - get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes/12345", user) + get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) end @@ -274,7 +274,7 @@ RSpec.shared_examples 'noteable API' do |parent_type, noteable_type, id_name| end it 'returns a 404 error when note id not found' do - put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes/12345", user), + put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes/#{non_existing_record_id}", user), params: { body: 'Hello!' } expect(response).to have_gitlab_http_status(:not_found) @@ -301,7 +301,7 @@ RSpec.shared_examples 'noteable API' do |parent_type, noteable_type, id_name| end it 'returns a 404 error when note id not found' do - delete api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes/12345", user) + delete api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) end diff --git a/spec/support/shared_examples/requests/api/resolvable_discussions_shared_examples.rb b/spec/support/shared_examples/requests/api/resolvable_discussions_shared_examples.rb index 8d2a3f13d8e..5748e873fd4 100644 --- a/spec/support/shared_examples/requests/api/resolvable_discussions_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/resolvable_discussions_shared_examples.rb @@ -66,7 +66,7 @@ RSpec.shared_examples 'resolvable discussions API' do |parent_type, noteable_typ it 'returns a 404 error when note id not found' do put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\ - "discussions/#{note.discussion_id}/notes/12345", user), + "discussions/#{note.discussion_id}/notes/#{non_existing_record_id}", user), params: { body: 'Hello!' } expect(response).to have_gitlab_http_status(:not_found) diff --git a/spec/support/shared_examples/requests/api/resource_label_events_api_shared_examples.rb b/spec/support/shared_examples/requests/api/resource_label_events_api_shared_examples.rb index 520c3ea8e47..f49f944f38d 100644 --- a/spec/support/shared_examples/requests/api/resource_label_events_api_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/resource_label_events_api_shared_examples.rb @@ -15,7 +15,7 @@ RSpec.shared_examples 'resource_label_events API' do |parent_type, eventable_typ end it "returns a 404 error when eventable id not found" do - get api("/#{parent_type}/#{parent.id}/#{eventable_type}/12345/resource_label_events", user) + get api("/#{parent_type}/#{parent.id}/#{eventable_type}/#{non_existing_record_id}/resource_label_events", user) expect(response).to have_gitlab_http_status(:not_found) end @@ -74,7 +74,7 @@ RSpec.shared_examples 'resource_label_events API' do |parent_type, eventable_typ end it "returns a 404 error if resource label event not found" do - get api("/#{parent_type}/#{parent.id}/#{eventable_type}/#{eventable[id_name]}/resource_label_events/12345", user) + get api("/#{parent_type}/#{parent.id}/#{eventable_type}/#{eventable[id_name]}/resource_label_events/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) end diff --git a/spec/tasks/gitlab/backup_rake_spec.rb b/spec/tasks/gitlab/backup_rake_spec.rb index e042fc58f82..febc18e497a 100644 --- a/spec/tasks/gitlab/backup_rake_spec.rb +++ b/spec/tasks/gitlab/backup_rake_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' require 'rake' -describe 'gitlab:app namespace rake task' do +describe 'gitlab:app namespace rake task', :delete do let(:enable_registry) { true } def tars_glob diff --git a/spec/tasks/gitlab/storage_rake_spec.rb b/spec/tasks/gitlab/storage_rake_spec.rb index 54a84c73a57..226d3aaab4e 100644 --- a/spec/tasks/gitlab/storage_rake_spec.rb +++ b/spec/tasks/gitlab/storage_rake_spec.rb @@ -128,10 +128,10 @@ describe 'rake gitlab:storage:*' do context 'with same id in range' do it 'displays message when project cant be found' do - stub_env('ID_FROM', 99999) - stub_env('ID_TO', 99999) + stub_env('ID_FROM', non_existing_record_id) + stub_env('ID_TO', non_existing_record_id) - expect { run_rake_task(task) }.to abort_execution.with_message(/There are no projects requiring storage migration with ID=99999/) + expect { run_rake_task(task) }.to abort_execution.with_message(/There are no projects requiring storage migration with ID=#{non_existing_record_id}/) end it 'displays a message when project exists but its already migrated' do diff --git a/spec/workers/ci/create_cross_project_pipeline_worker_spec.rb b/spec/workers/ci/create_cross_project_pipeline_worker_spec.rb index 22eab1d20f7..492f5e812ee 100644 --- a/spec/workers/ci/create_cross_project_pipeline_worker_spec.rb +++ b/spec/workers/ci/create_cross_project_pipeline_worker_spec.rb @@ -29,7 +29,7 @@ describe Ci::CreateCrossProjectPipelineWorker do expect(Ci::CreateCrossProjectPipelineService) .not_to receive(:new) - described_class.new.perform(1234) + described_class.new.perform(non_existing_record_id) end end end diff --git a/spec/workers/ci/pipeline_bridge_status_worker_spec.rb b/spec/workers/ci/pipeline_bridge_status_worker_spec.rb index d5f95a035fd..a6129b2cf93 100644 --- a/spec/workers/ci/pipeline_bridge_status_worker_spec.rb +++ b/spec/workers/ci/pipeline_bridge_status_worker_spec.rb @@ -25,7 +25,7 @@ describe Ci::PipelineBridgeStatusWorker do end context 'when pipeline does not exist' do - let(:pipeline_id) { 1234 } + let(:pipeline_id) { non_existing_record_id } it 'does not call the service' do expect(Ci::PipelineBridgeStatusService) diff --git a/spec/workers/cluster_update_app_worker_spec.rb b/spec/workers/cluster_update_app_worker_spec.rb index d91104334e5..5391c194679 100644 --- a/spec/workers/cluster_update_app_worker_spec.rb +++ b/spec/workers/cluster_update_app_worker_spec.rb @@ -52,6 +52,8 @@ describe ClusterUpdateAppWorker do let(:lease_key) { "#{described_class.name.underscore}-#{application.id}" } before do + # update_highest_role uses exclusive key too: + allow(Gitlab::ExclusiveLease).to receive(:new).and_call_original stub_exclusive_lease_taken(lease_key) end @@ -62,8 +64,6 @@ describe ClusterUpdateAppWorker do end it 'does not allow same app to be updated concurrently by different project', :aggregate_failures do - stub_exclusive_lease("refresh_authorized_projects:#{user.id}") - stub_exclusive_lease("update_highest_role:#{user.id}") project1 = create(:project, namespace: create(:namespace, owner: user)) expect(Clusters::Applications::PrometheusUpdateService).not_to receive(:new) @@ -87,8 +87,6 @@ describe ClusterUpdateAppWorker do application2 = create(:clusters_applications_prometheus, :installed) lease_key2 = "#{described_class.name.underscore}-#{application2.id}" - stub_exclusive_lease("refresh_authorized_projects:#{user.id}") - stub_exclusive_lease("update_highest_role:#{user.id}") project2 = create(:project, namespace: create(:namespace, owner: user)) stub_exclusive_lease(lease_key2) diff --git a/spec/workers/expire_job_cache_worker_spec.rb b/spec/workers/expire_job_cache_worker_spec.rb index 2776624f14c..804a50e89fe 100644 --- a/spec/workers/expire_job_cache_worker_spec.rb +++ b/spec/workers/expire_job_cache_worker_spec.rb @@ -39,7 +39,7 @@ describe ExpireJobCacheWorker do it 'does not change the etag store' do expect(Gitlab::EtagCaching::Store).not_to receive(:new) - perform_multiple(9999) + perform_multiple(non_existing_record_id) end end end diff --git a/spec/workers/group_export_worker_spec.rb b/spec/workers/group_export_worker_spec.rb index 4aa85d2b381..350777df912 100644 --- a/spec/workers/group_export_worker_spec.rb +++ b/spec/workers/group_export_worker_spec.rb @@ -21,8 +21,8 @@ describe GroupExportWorker do it 'raises an exception when params are invalid' do expect_any_instance_of(::Groups::ImportExport::ExportService).not_to receive(:execute) - expect { subject.perform(1234, group.id, {}) }.to raise_exception(ActiveRecord::RecordNotFound) - expect { subject.perform(user.id, 1234, {}) }.to raise_exception(ActiveRecord::RecordNotFound) + expect { subject.perform(non_existing_record_id, group.id, {}) }.to raise_exception(ActiveRecord::RecordNotFound) + expect { subject.perform(user.id, non_existing_record_id, {}) }.to raise_exception(ActiveRecord::RecordNotFound) end end end diff --git a/spec/workers/group_import_worker_spec.rb b/spec/workers/group_import_worker_spec.rb index 0783ac4df4e..641aa45c9b0 100644 --- a/spec/workers/group_import_worker_spec.rb +++ b/spec/workers/group_import_worker_spec.rb @@ -21,8 +21,8 @@ describe GroupImportWorker do it 'raises an exception when params are invalid' do expect_any_instance_of(::Groups::ImportExport::ImportService).not_to receive(:execute) - expect { subject.perform(1234, group.id) }.to raise_exception(ActiveRecord::RecordNotFound) - expect { subject.perform(user.id, 1234) }.to raise_exception(ActiveRecord::RecordNotFound) + expect { subject.perform(non_existing_record_id, group.id) }.to raise_exception(ActiveRecord::RecordNotFound) + expect { subject.perform(user.id, non_existing_record_id) }.to raise_exception(ActiveRecord::RecordNotFound) end end end diff --git a/spec/workers/incident_management/process_prometheus_alert_worker_spec.rb b/spec/workers/incident_management/process_prometheus_alert_worker_spec.rb index 19ef2635882..5fbc39cad4e 100644 --- a/spec/workers/incident_management/process_prometheus_alert_worker_spec.rb +++ b/spec/workers/incident_management/process_prometheus_alert_worker_spec.rb @@ -60,7 +60,7 @@ describe IncidentManagement::ProcessPrometheusAlertWorker do end context 'when project could not be found' do - let(:non_existing_project_id) { (Project.maximum(:id) || 0) + 1 } + let(:non_existing_project_id) { non_existing_record_id } it 'does not create an issue' do expect { subject.perform(non_existing_project_id, alert_params) } @@ -75,7 +75,7 @@ describe IncidentManagement::ProcessPrometheusAlertWorker do context 'when event could not be found' do before do - alert_params[:labels][:gitlab_alert_id] = (PrometheusAlertEvent.maximum(:id) || 0) + 1 + alert_params[:labels][:gitlab_alert_id] = non_existing_record_id end it 'does not create an issue' do diff --git a/spec/workers/namespaceless_project_destroy_worker_spec.rb b/spec/workers/namespaceless_project_destroy_worker_spec.rb index e6686328291..b97a44c714d 100644 --- a/spec/workers/namespaceless_project_destroy_worker_spec.rb +++ b/spec/workers/namespaceless_project_destroy_worker_spec.rb @@ -82,7 +82,7 @@ describe NamespacelessProjectDestroyWorker do context 'project has non-existing namespace' do let!(:project) do - project = build(:project, namespace_id: Namespace.maximum(:id).to_i.succ) + project = build(:project, namespace_id: non_existing_record_id) project.save(validate: false) project end diff --git a/spec/workers/namespaces/schedule_aggregation_worker_spec.rb b/spec/workers/namespaces/schedule_aggregation_worker_spec.rb index e7da346df46..2f4c7f8bc07 100644 --- a/spec/workers/namespaces/schedule_aggregation_worker_spec.rb +++ b/spec/workers/namespaces/schedule_aggregation_worker_spec.rb @@ -50,7 +50,7 @@ describe Namespaces::ScheduleAggregationWorker, '#perform', :clean_gitlab_redis_ it 'logs the error' do expect(Gitlab::ErrorTracking).to receive(:track_exception).once - worker.perform(12345) + worker.perform(non_existing_record_id) end end diff --git a/spec/workers/new_issue_worker_spec.rb b/spec/workers/new_issue_worker_spec.rb index 88a75ce5b70..1584e9d5302 100644 --- a/spec/workers/new_issue_worker_spec.rb +++ b/spec/workers/new_issue_worker_spec.rb @@ -30,9 +30,11 @@ describe NewIssueWorker do end it 'logs an error' do + issue = create(:issue) + expect(Rails.logger).to receive(:error).with('NewIssueWorker: couldn\'t find User with ID=99, skipping job') - worker.perform(create(:issue).id, 99) + worker.perform(issue.id, 99) end end diff --git a/spec/workers/new_merge_request_worker_spec.rb b/spec/workers/new_merge_request_worker_spec.rb index d078ddd07d9..fe22226903f 100644 --- a/spec/workers/new_merge_request_worker_spec.rb +++ b/spec/workers/new_merge_request_worker_spec.rb @@ -15,9 +15,11 @@ describe NewMergeRequestWorker do end it 'logs an error' do + user = create(:user) + expect(Rails.logger).to receive(:error).with('NewMergeRequestWorker: couldn\'t find MergeRequest with ID=99, skipping job') - worker.perform(99, create(:user).id) + worker.perform(99, user.id) end end @@ -30,9 +32,11 @@ describe NewMergeRequestWorker do end it 'logs an error' do + merge_request = create(:merge_request) + expect(Rails.logger).to receive(:error).with('NewMergeRequestWorker: couldn\'t find User with ID=99, skipping job') - worker.perform(create(:merge_request).id, 99) + worker.perform(merge_request.id, 99) end end diff --git a/spec/workers/new_note_worker_spec.rb b/spec/workers/new_note_worker_spec.rb index ae62237960a..387a0958d6b 100644 --- a/spec/workers/new_note_worker_spec.rb +++ b/spec/workers/new_note_worker_spec.rb @@ -24,11 +24,11 @@ describe NewNoteWorker do end context 'when Note not found' do - let(:unexistent_note_id) { 999 } + let(:unexistent_note_id) { non_existing_record_id } it 'logs NewNoteWorker process skipping' do expect(Rails.logger).to receive(:error) - .with("NewNoteWorker: couldn't find note with ID=999, skipping job") + .with("NewNoteWorker: couldn't find note with ID=#{unexistent_note_id}, skipping job") described_class.new.perform(unexistent_note_id) end diff --git a/spec/workers/pipeline_notification_worker_spec.rb b/spec/workers/pipeline_notification_worker_spec.rb index 5defd3d5bd7..f2cc2b56236 100644 --- a/spec/workers/pipeline_notification_worker_spec.rb +++ b/spec/workers/pipeline_notification_worker_spec.rb @@ -18,7 +18,7 @@ describe PipelineNotificationWorker, :mailer do it 'does nothing when the pipeline does not exist' do expect(NotificationService).not_to receive(:new) - subject.perform(Ci::Pipeline.maximum(:id).to_i.succ) + subject.perform(non_existing_record_id) end end end diff --git a/spec/workers/project_export_worker_spec.rb b/spec/workers/project_export_worker_spec.rb index d0d52e0df2d..373e7f32530 100644 --- a/spec/workers/project_export_worker_spec.rb +++ b/spec/workers/project_export_worker_spec.rb @@ -57,11 +57,11 @@ describe ProjectExportWorker do end it 'does not raise error when project cannot be found' do - expect { subject.perform(user.id, -234, {}) }.not_to raise_error + expect { subject.perform(user.id, non_existing_record_id, {}) }.not_to raise_error end it 'does not raise error when user cannot be found' do - expect { subject.perform(-863, project.id, {}) }.not_to raise_error + expect { subject.perform(non_existing_record_id, project.id, {}) }.not_to raise_error end end end diff --git a/spec/workers/remote_mirror_notification_worker_spec.rb b/spec/workers/remote_mirror_notification_worker_spec.rb index c9321fd7c56..f0fb46f84d9 100644 --- a/spec/workers/remote_mirror_notification_worker_spec.rb +++ b/spec/workers/remote_mirror_notification_worker_spec.rb @@ -26,7 +26,7 @@ describe RemoteMirrorNotificationWorker, :mailer do it 'does nothing when the mirror does not exist' do expect(NotificationService).not_to receive(:new) - subject.perform(RemoteMirror.maximum(:id).to_i.succ) + subject.perform(non_existing_record_id) end it 'does nothing when a notification has already been sent' do diff --git a/spec/workers/update_highest_role_worker_spec.rb b/spec/workers/update_highest_role_worker_spec.rb index cb112ebe07e..1e378a5a61e 100644 --- a/spec/workers/update_highest_role_worker_spec.rb +++ b/spec/workers/update_highest_role_worker_spec.rb @@ -8,56 +8,73 @@ describe UpdateHighestRoleWorker, :clean_gitlab_redis_shared_state do let(:worker) { described_class.new } describe '#perform' do - let(:active_scope_attributes) do - { - state: 'active', - ghost: false, - user_type: nil - } + context 'when user is not found' do + it 'does not update or deletes any highest role', :aggregate_failures do + expect { worker.perform(-1) }.not_to change(UserHighestRole, :count) + end end - let(:user) { create(:user, attributes) } - - subject { worker.perform(user.id) } context 'when user is found' do - let(:attributes) { active_scope_attributes } + let(:active_attributes) do + { + state: 'active', + ghost: false, + user_type: nil + } + end + let(:user) { create(:user, active_attributes) } - it 'updates the highest role for the user' do - user_highest_role = create(:user_highest_role, user: user) - create(:group_member, :developer, user: user) + subject { worker.perform(user.id) } - expect { subject } - .to change { user_highest_role.reload.highest_access_level } - .from(nil) - .to(Gitlab::Access::DEVELOPER) - end - end + context 'when user is active and not internal' do + context 'when user highest role exists' do + it 'updates the highest role for the user' do + user_highest_role = create(:user_highest_role, user: user) + create(:group_member, :developer, user: user) - context 'when user is not found' do - shared_examples 'no update' do - it 'does not update any highest role' do - expect(Users::UpdateHighestMemberRoleService).not_to receive(:new) + expect { subject } + .to change { user_highest_role.reload.highest_access_level } + .from(nil) + .to(Gitlab::Access::DEVELOPER) + end + end + + context 'when user highest role does not exist' do + it 'creates the highest role for the user' do + create(:group_member, :developer, user: user) - worker.perform(user.id) + expect { subject }.to change { UserHighestRole.count }.by(1) + end end end - context 'when user is blocked' do - let(:attributes) { active_scope_attributes.merge(state: 'blocked') } + context 'when user is either inactive or internal' do + using RSpec::Parameterized::TableSyntax - it_behaves_like 'no update' - end + where(:additional_attributes) do + [ + { state: 'blocked' }, + { ghost: true }, + { user_type: :alert_bot } + ] + end - context 'when user is a ghost' do - let(:attributes) { active_scope_attributes.merge(ghost: true) } + with_them do + it 'deletes highest role' do + user = create(:user, active_attributes.merge(additional_attributes)) + create(:user_highest_role, user: user) - it_behaves_like 'no update' - end + expect { worker.perform(user.id) }.to change { UserHighestRole.count }.from(1).to(0) + end + end - context 'when user has a user type' do - let(:attributes) { active_scope_attributes.merge(user_type: :alert_bot) } + context 'when user highest role does not exist' do + it 'does not delete a highest role' do + user = create(:user, state: 'blocked') - it_behaves_like 'no update' + expect { worker.perform(user.id) }.not_to change(UserHighestRole, :count) + end + end end end end diff --git a/spec/workers/upload_checksum_worker_spec.rb b/spec/workers/upload_checksum_worker_spec.rb index ad054f0ff72..44e9e4f048d 100644 --- a/spec/workers/upload_checksum_worker_spec.rb +++ b/spec/workers/upload_checksum_worker_spec.rb @@ -8,7 +8,7 @@ describe UploadChecksumWorker do context 'without a valid record' do it 'rescues ActiveRecord::RecordNotFound' do - expect { subject.perform(999_999) }.not_to raise_error + expect { subject.perform(non_existing_record_id) }.not_to raise_error end end |