diff options
47 files changed, 695 insertions, 370 deletions
diff --git a/.rubocop_todo/layout/argument_alignment.yml b/.rubocop_todo/layout/argument_alignment.yml index 365128bf6c5..d859adfc540 100644 --- a/.rubocop_todo/layout/argument_alignment.yml +++ b/.rubocop_todo/layout/argument_alignment.yml @@ -605,14 +605,6 @@ Layout/ArgumentAlignment: - 'app/services/markdown_content_rewriter_service.rb' - 'app/services/members/base_service.rb' - 'app/services/members/create_service.rb' - - 'app/services/merge_requests/build_service.rb' - - 'app/services/merge_requests/ff_merge_service.rb' - - 'app/services/merge_requests/merge_service.rb' - - 'app/services/merge_requests/merge_to_ref_service.rb' - - 'app/services/merge_requests/push_options_handler_service.rb' - - 'app/services/merge_requests/refresh_service.rb' - - 'app/services/merge_requests/reload_diffs_service.rb' - - 'app/services/merge_requests/retarget_chain_service.rb' - 'app/services/metrics/dashboard/annotations/create_service.rb' - 'app/services/metrics/dashboard/annotations/delete_service.rb' - 'app/services/metrics/dashboard/clone_dashboard_service.rb' @@ -1538,7 +1530,6 @@ Layout/ArgumentAlignment: - 'ee/spec/services/groups/restore_service_spec.rb' - 'ee/spec/services/issue_feature_flags/list_service_spec.rb' - 'ee/spec/services/merge_request_approval_settings/update_service_spec.rb' - - 'ee/spec/services/merge_requests/build_service_spec.rb' - 'ee/spec/services/protected_environments/create_service_spec.rb' - 'ee/spec/services/protected_environments/update_service_spec.rb' - 'ee/spec/services/quick_actions/interpret_service_spec.rb' @@ -2574,23 +2565,6 @@ Layout/ArgumentAlignment: - 'spec/services/issues/resolve_discussions_spec.rb' - 'spec/services/issues/update_service_spec.rb' - 'spec/services/loose_foreign_keys/batch_cleaner_service_spec.rb' - - 'spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb' - - 'spec/services/merge_requests/assign_issues_service_spec.rb' - - 'spec/services/merge_requests/conflicts/resolve_service_spec.rb' - - 'spec/services/merge_requests/create_service_spec.rb' - - 'spec/services/merge_requests/ff_merge_service_spec.rb' - - 'spec/services/merge_requests/merge_orchestration_service_spec.rb' - - 'spec/services/merge_requests/merge_service_spec.rb' - - 'spec/services/merge_requests/merge_to_ref_service_spec.rb' - - 'spec/services/merge_requests/mergeability/detailed_merge_status_service_spec.rb' - - 'spec/services/merge_requests/mergeability_check_service_spec.rb' - - 'spec/services/merge_requests/rebase_service_spec.rb' - - 'spec/services/merge_requests/refresh_service_spec.rb' - - 'spec/services/merge_requests/reload_diffs_service_spec.rb' - - 'spec/services/merge_requests/squash_service_spec.rb' - - 'spec/services/merge_requests/update_assignees_service_spec.rb' - - 'spec/services/merge_requests/update_reviewers_service_spec.rb' - - 'spec/services/merge_requests/update_service_spec.rb' - 'spec/services/metrics/dashboard/clone_dashboard_service_spec.rb' - 'spec/services/note_summary_spec.rb' - 'spec/services/notes/build_service_spec.rb' @@ -17,9 +17,7 @@ gem 'rails', '~> 6.1.7.2' gem 'bootsnap', '~> 1.16.0', require: false -# Pin openssl to match the version bundled with our supported Rubies. -# See https://stdgems.org/openssl/#gem-version. -gem 'openssl', '2.2.2' +gem 'openssl', '~> 3.0' gem 'ipaddr', '~> 1.2.5' # Responders respond_to and respond_with diff --git a/Gemfile.checksum b/Gemfile.checksum index aa1586e1d5b..73fb708a38c 100644 --- a/Gemfile.checksum +++ b/Gemfile.checksum @@ -422,7 +422,7 @@ {"name":"omniauth_openid_connect","version":"0.6.1","platform":"ruby","checksum":"5f1318f5b19b05e339ff494def060b57a503b1e3ea83c3a0ced6cc014407d423"}, {"name":"open4","version":"1.3.4","platform":"ruby","checksum":"a1df037310624ecc1ea1d81264b11c83e96d0c3c1c6043108d37d396dcd0f4b1"}, {"name":"openid_connect","version":"1.3.0","platform":"ruby","checksum":"a796855096850cc01140e37ea6ae9fd14f2be818b9b5bc698418063dfe228770"}, -{"name":"openssl","version":"2.2.2","platform":"ruby","checksum":"53f72382bac046c36c37049c7ec9d5597d42628d140b5cfbcd61e0226c0ca077"}, +{"name":"openssl","version":"3.1.0","platform":"ruby","checksum":"e3a01279e918a7a5cf741db69b124864878b1a9783b1f2d34854bc1d444ac430"}, {"name":"openssl-signature_algorithm","version":"1.3.0","platform":"ruby","checksum":"a3b40b5e8276162d4a6e50c7c97cdaf1446f9b2c3946a6fa2c14628e0c957e80"}, {"name":"opentracing","version":"0.5.0","platform":"ruby","checksum":"deb5d7abe6b0e7631d866d8cb5ee7bb9352650a504a32f61591302bc510b9286"}, {"name":"optimist","version":"3.0.1","platform":"ruby","checksum":"336b753676d6117cad9301fac7e91dab4228f747d4e7179891ad3a163c64e2ed"}, diff --git a/Gemfile.lock b/Gemfile.lock index 2940d614572..87bae618f10 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1093,8 +1093,7 @@ GEM validate_email validate_url webfinger (>= 1.0.1) - openssl (2.2.2) - ipaddr + openssl (3.1.0) openssl-signature_algorithm (1.3.0) openssl (> 2.0) opentracing (0.5.0) @@ -1855,7 +1854,7 @@ DEPENDENCIES omniauth_crowd (~> 2.4.0)! omniauth_openid_connect (~> 0.6.1) openid_connect (= 1.3.0) - openssl (= 2.2.2) + openssl (~> 3.0) org-ruby (~> 0.9.12) pact (~> 1.63) parallel (~> 1.19) diff --git a/app/assets/javascripts/behaviors/shortcuts/keybindings.js b/app/assets/javascripts/behaviors/shortcuts/keybindings.js index 38ee02938cc..d173703f172 100644 --- a/app/assets/javascripts/behaviors/shortcuts/keybindings.js +++ b/app/assets/javascripts/behaviors/shortcuts/keybindings.js @@ -117,6 +117,12 @@ export const HIDE_APPEARING_CONTENT = { defaultKeys: ['esc'], }; +export const TOGGLE_SUPER_SIDEBAR = { + id: 'globalShortcuts.toggleSuperSidebar', + description: __('Toggle the navigation sidebar'), + defaultKeys: ['mod+\\'], // eslint-disable-line @gitlab/require-i18n-strings +}; + export const TOGGLE_CANARY = { id: 'globalShortcuts.toggleCanary', description: __('Toggle GitLab Next'), @@ -536,6 +542,10 @@ export const GLOBAL_SHORTCUTS_GROUP = { ], }; +if (gon.use_new_navigation) { + GLOBAL_SHORTCUTS_GROUP.keybindings.push(TOGGLE_SUPER_SIDEBAR); +} + export const EDITING_SHORTCUTS_GROUP = { id: 'editing', name: __('Editing'), diff --git a/app/assets/javascripts/repository/components/blob_content_viewer.vue b/app/assets/javascripts/repository/components/blob_content_viewer.vue index 334e7964bc2..2eeff48dcf9 100644 --- a/app/assets/javascripts/repository/components/blob_content_viewer.vue +++ b/app/assets/javascripts/repository/components/blob_content_viewer.vue @@ -145,7 +145,7 @@ export default { }, computed: { shouldRenderGenie() { - return this.explainCodeAvailable; + return this.explainCodeAvailable && this.viewer?.fileType === TEXT_FILE_TYPE; }, isLoggedIn() { return isLoggedIn(); diff --git a/app/assets/javascripts/super_sidebar/components/super_sidebar.vue b/app/assets/javascripts/super_sidebar/components/super_sidebar.vue index 64460fdcb68..2cd37875f57 100644 --- a/app/assets/javascripts/super_sidebar/components/super_sidebar.vue +++ b/app/assets/javascripts/super_sidebar/components/super_sidebar.vue @@ -1,5 +1,7 @@ <script> import { GlButton, GlCollapse } from '@gitlab/ui'; +import Mousetrap from 'mousetrap'; +import { keysFor, TOGGLE_SUPER_SIDEBAR } from '~/behaviors/shortcuts/keybindings'; import { __ } from '~/locale'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { @@ -7,7 +9,7 @@ import { SUPER_SIDEBAR_PEEK_OPEN_DELAY, SUPER_SIDEBAR_PEEK_CLOSE_DELAY, } from '../constants'; -import { toggleSuperSidebarCollapsed } from '../super_sidebar_collapsed_state_manager'; +import { isCollapsed, toggleSuperSidebarCollapsed } from '../super_sidebar_collapsed_state_manager'; import UserBar from './user_bar.vue'; import SidebarPortalTarget from './sidebar_portal_target.vue'; import ContextSwitcherToggle from './context_switcher_toggle.vue'; @@ -44,7 +46,16 @@ export default { return this.sidebarData.current_menu_items || []; }, }, + mounted() { + Mousetrap.bind(keysFor(TOGGLE_SUPER_SIDEBAR), this.toggleSidebar); + }, + beforeDestroy() { + Mousetrap.unbind(keysFor(TOGGLE_SUPER_SIDEBAR)); + }, methods: { + toggleSidebar() { + toggleSuperSidebarCollapsed(!isCollapsed(), true); + }, collapseSidebar() { toggleSuperSidebarCollapsed(true, false); }, diff --git a/app/services/concerns/exclusive_lease_guard.rb b/app/services/concerns/exclusive_lease_guard.rb index 76d59cf2159..74acaa0522a 100644 --- a/app/services/concerns/exclusive_lease_guard.rb +++ b/app/services/concerns/exclusive_lease_guard.rb @@ -21,7 +21,7 @@ module ExclusiveLeaseGuard lease = exclusive_lease.try_obtain unless lease - log_error("Cannot obtain an exclusive lease for #{lease_key}. There must be another instance already in execution.") + log_lease_taken return end @@ -57,7 +57,23 @@ module ExclusiveLeaseGuard exclusive_lease.renew end - def log_error(message, extra_args = {}) - Gitlab::AppLogger.error(message) + def log_lease_taken + logger = Gitlab::AppJsonLogger + args = { message: lease_taken_message, lease_key: lease_key, class_name: self.class.name, lease_timeout: lease_timeout } + + case lease_taken_log_level + when :debug then logger.debug(args) + when :info then logger.info(args) + when :warn then logger.warn(args) + else logger.error(args) + end + end + + def lease_taken_message + "Cannot obtain an exclusive lease. There must be another instance already in execution." + end + + def lease_taken_log_level + :error end end diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index d5b109a764d..3a7b577d59a 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -42,17 +42,17 @@ module MergeRequests attr_accessor :merge_request delegate :target_branch, - :target_branch_ref, - :target_project, - :source_branch, - :source_branch_ref, - :source_project, - :compare_commits, - :draft_title, - :description, - :first_multiline_commit, - :errors, - to: :merge_request + :target_branch_ref, + :target_project, + :source_branch, + :source_branch_ref, + :source_project, + :compare_commits, + :draft_title, + :description, + :first_multiline_commit, + :errors, + to: :merge_request def force_remove_source_branch if params.key?(:force_remove_source_branch) diff --git a/app/services/merge_requests/ff_merge_service.rb b/app/services/merge_requests/ff_merge_service.rb index 6e1d1b6ad23..1a83bbf9de6 100644 --- a/app/services/merge_requests/ff_merge_service.rb +++ b/app/services/merge_requests/ff_merge_service.rb @@ -14,10 +14,12 @@ module MergeRequests override :execute_git_merge def execute_git_merge - repository.ff_merge(current_user, - source, - merge_request.target_branch, - merge_request: merge_request) + repository.ff_merge( + current_user, + source, + merge_request.target_branch, + merge_request: merge_request + ) end override :merge_success_data diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index e6b0ffbf716..10301774f96 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -115,8 +115,7 @@ module MergeRequests def try_merge execute_git_merge rescue Gitlab::Git::PreReceiveError => e - raise MergeError, - "Something went wrong during merge pre-receive hook. #{e.message}".strip + raise MergeError, "Something went wrong during merge pre-receive hook. #{e.message}".strip rescue StandardError => e handle_merge_error(log_message: e.message) raise_error(GENERIC_ERROR_MESSAGE) @@ -180,9 +179,7 @@ module MergeRequests end def log_payload(message) - Gitlab::ApplicationContext.current - .merge(merge_request_info: merge_request_info, - message: message) + Gitlab::ApplicationContext.current.merge(merge_request_info: merge_request_info, message: message) end def merge_request_info diff --git a/app/services/merge_requests/merge_to_ref_service.rb b/app/services/merge_requests/merge_to_ref_service.rb index 8519cbac3cb..1bd26f06e41 100644 --- a/app/services/merge_requests/merge_to_ref_service.rb +++ b/app/services/merge_requests/merge_to_ref_service.rb @@ -25,9 +25,7 @@ module MergeRequests commit = project.commit(commit_id) target_id, source_id = commit.parent_ids - success(commit_id: commit.id, - target_id: target_id, - source_id: source_id) + success(commit_id: commit.id, target_id: target_id, source_id: source_id) rescue MergeError, ArgumentError => error error(error.message) end diff --git a/app/services/merge_requests/push_options_handler_service.rb b/app/services/merge_requests/push_options_handler_service.rb index e9abafceb13..1890addf692 100644 --- a/app/services/merge_requests/push_options_handler_service.rb +++ b/app/services/merge_requests/push_options_handler_service.rb @@ -4,8 +4,7 @@ module MergeRequests class PushOptionsHandlerService < ::BaseProjectService LIMIT = 10 - attr_reader :errors, :changes, - :push_options, :target_project + attr_reader :errors, :changes, :push_options, :target_project def initialize(project:, current_user:, changes:, push_options:, params: {}) super(project: project, current_user: current_user, params: params) @@ -112,8 +111,10 @@ module MergeRequests merge_request = ::MergeRequests::CreateService.new( project: project, current_user: current_user, - params: merge_request.attributes.merge(assignee_ids: merge_request.assignee_ids, - label_ids: merge_request.label_ids) + params: merge_request.attributes.merge( + assignee_ids: merge_request.assignee_ids, + label_ids: merge_request.label_ids + ) ).execute end diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index 21e0d9a6e6b..d6740cdf1ac 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -247,9 +247,11 @@ module MergeRequests mr_commit_ids.include?(commit.id) end - SystemNoteService.add_commits(merge_request, merge_request.project, - @current_user, new_commits, - existing_commits, @push.oldrev) + SystemNoteService.add_commits( + merge_request, merge_request.project, + @current_user, new_commits, + existing_commits, @push.oldrev + ) notification_service.push_to_merge_request(merge_request, @current_user, new_commits: new_commits, existing_commits: existing_commits) end diff --git a/app/services/merge_requests/reload_diffs_service.rb b/app/services/merge_requests/reload_diffs_service.rb index c64b2e99b52..2c6ec9333b2 100644 --- a/app/services/merge_requests/reload_diffs_service.rb +++ b/app/services/merge_requests/reload_diffs_service.rb @@ -22,9 +22,11 @@ module MergeRequests def update_diff_discussion_positions(old_diff_refs) new_diff_refs = merge_request.diff_refs - merge_request.update_diff_discussion_positions(old_diff_refs: old_diff_refs, - new_diff_refs: new_diff_refs, - current_user: current_user) + merge_request.update_diff_discussion_positions( + old_diff_refs: old_diff_refs, + new_diff_refs: new_diff_refs, + current_user: current_user + ) end # rubocop: disable CodeReuse/ActiveRecord diff --git a/app/services/merge_requests/retarget_chain_service.rb b/app/services/merge_requests/retarget_chain_service.rb index 33aae4184ae..b4b05ffb08c 100644 --- a/app/services/merge_requests/retarget_chain_service.rb +++ b/app/services/merge_requests/retarget_chain_service.rb @@ -21,13 +21,14 @@ module MergeRequests # Update only MRs on projects that we have access to next unless can?(current_user, :update_merge_request, other_merge_request.source_project) - ::MergeRequests::UpdateService - .new(project: other_merge_request.source_project, current_user: current_user, - params: { - target_branch: merge_request.target_branch, - target_branch_was_deleted: true - }) - .execute(other_merge_request) + ::MergeRequests::UpdateService.new( + project: other_merge_request.source_project, + current_user: current_user, + params: { + target_branch: merge_request.target_branch, + target_branch_was_deleted: true + } + ).execute(other_merge_request) end end end diff --git a/app/views/notify/_note_email.text.erb b/app/views/notify/_note_email.text.erb index 8853519fb8d..b23ba464c06 100644 --- a/app/views/notify/_note_email.text.erb +++ b/app/views/notify/_note_email.text.erb @@ -6,7 +6,7 @@ <%= sanitize_name(author.name) -%> <% if discussion.nil? -%> - <%= 'commented' -%>: + <%= 'commented' -%> <% else -%> <% if discussion.first_note == note -%> <%= 'started a new discussion' -%> @@ -16,9 +16,9 @@ <% if discussion.diff_discussion? -%> <%= "on #{discussion.file_path}" -%> <% end -%> -<%= ':' -%> -<%= " #{target_url}" -%> <% end -%> +<%= ':' -%> +<%= " #{target_url}" -%> <% if discussion&.diff_discussion? && discussion.on_text? -%> diff --git a/app/views/shared/labels/_form.html.haml b/app/views/shared/labels/_form.html.haml index 9148cb615d4..cc806044374 100644 --- a/app/views/shared/labels/_form.html.haml +++ b/app/views/shared/labels/_form.html.haml @@ -10,7 +10,7 @@ .form-group.row .col-12 = f.label :description - = f.text_field :description, class: "gl-form-input form-control js-quick-submit", data: { qa_selector: 'label_description_field' } + = f.text_area :description, class: "gl-form-input form-control js-quick-submit", rows: 4, data: { qa_selector: 'label_description_field' } .form-group.row .col-12 = f.label :color, _("Background color") diff --git a/app/workers/deployments/drop_older_deployments_worker.rb b/app/workers/deployments/drop_older_deployments_worker.rb index c464febd119..5f252122232 100644 --- a/app/workers/deployments/drop_older_deployments_worker.rb +++ b/app/workers/deployments/drop_older_deployments_worker.rb @@ -11,8 +11,6 @@ module Deployments queue_namespace :deployment feature_category :continuous_delivery - def perform(deployment_id) - Deployments::OlderDeploymentsDropService.new(deployment_id).execute - end + def perform(deployment_id); end end end diff --git a/doc/user/clusters/agent/gitops/flux_tutorial.md b/doc/user/clusters/agent/gitops/flux_tutorial.md index 88cc824b6b2..bdb772ac14e 100644 --- a/doc/user/clusters/agent/gitops/flux_tutorial.md +++ b/doc/user/clusters/agent/gitops/flux_tutorial.md @@ -59,7 +59,7 @@ Create a Git repository, install Flux, and authenticate Flux with your repo: --owner=gitlab-org/configure/examples/flux \ --repository=flux-config \ --branch=main \ - --path=clusters/my-cluster + --path=clusters/my-cluster \ --deploy-token-auth ``` diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 67d06ae80ec..5a87d2a1890 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -38975,6 +38975,9 @@ msgstr "" msgid "ScanResultPolicy|Choose criteria type" msgstr "" +msgid "ScanResultPolicy|License scanning allows only one criteria: Status" +msgstr "" + msgid "ScanResultPolicy|Maximum number of severity-criteria is one" msgstr "" @@ -38999,6 +39002,9 @@ msgstr "" msgid "ScanResultPolicy|Status is:" msgstr "" +msgid "ScanResultPolicy|When %{scanType} find any license %{matchType} %{licenseType} in an open merge request targeting the %{branches} and the licences match all of the following criteria" +msgstr "" + msgid "ScanResultPolicy|When %{scanners} runs against the %{branches} and find(s) more than %{vulnerabilitiesAllowed} %{boldDescription} of the following criteria:" msgstr "" @@ -39008,10 +39014,7 @@ msgstr "" msgid "ScanResultPolicy|except" msgstr "" -msgid "ScanResultPolicy|finds any license %{matchType} %{licenseType} and is %{licenseStates} in an open merge request targeting %{branches}" -msgstr "" - -msgid "ScanResultPolicy|license states" +msgid "ScanResultPolicy|license status" msgstr "" msgid "ScanResultPolicy|license type" @@ -46433,6 +46436,9 @@ msgstr "" msgid "Toggle the Performance Bar" msgstr "" +msgid "Toggle the navigation sidebar" +msgstr "" + msgid "Toggled :%{name}: emoji award." msgstr "" diff --git a/spec/frontend/super_sidebar/components/super_sidebar_spec.js b/spec/frontend/super_sidebar/components/super_sidebar_spec.js index 85f2a63943d..cd2f3c89ed9 100644 --- a/spec/frontend/super_sidebar/components/super_sidebar_spec.js +++ b/spec/frontend/super_sidebar/components/super_sidebar_spec.js @@ -1,5 +1,6 @@ import { nextTick } from 'vue'; import { GlCollapse } from '@gitlab/ui'; +import Mousetrap from 'mousetrap'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import SuperSidebar from '~/super_sidebar/components/super_sidebar.vue'; import HelpCenter from '~/super_sidebar/components/help_center.vue'; @@ -10,9 +11,14 @@ import { SUPER_SIDEBAR_PEEK_OPEN_DELAY, SUPER_SIDEBAR_PEEK_CLOSE_DELAY, } from '~/super_sidebar/constants'; +import { + toggleSuperSidebarCollapsed, + isCollapsed, +} from '~/super_sidebar/super_sidebar_collapsed_state_manager'; import { stubComponent } from 'helpers/stub_component'; import { sidebarData } from '../mock_data'; +jest.mock('~/super_sidebar/super_sidebar_collapsed_state_manager'); const focusInputMock = jest.fn(); describe('SuperSidebar component', () => { @@ -85,6 +91,28 @@ describe('SuperSidebar component', () => { expect(link.attributes('href')).toBe(linkAttrs.href); expect(link.attributes('class')).toContain('gl-display-none'); }); + + it('sets up the sidebar toggle shortcut', () => { + createWrapper(); + + isCollapsed.mockReturnValue(false); + Mousetrap.trigger('mod+\\'); + + expect(toggleSuperSidebarCollapsed).toHaveBeenCalledTimes(1); + expect(toggleSuperSidebarCollapsed).toHaveBeenCalledWith(true, true); + + isCollapsed.mockReturnValue(true); + Mousetrap.trigger('mod+\\'); + + expect(toggleSuperSidebarCollapsed).toHaveBeenCalledTimes(2); + expect(toggleSuperSidebarCollapsed).toHaveBeenCalledWith(false, true); + + jest.spyOn(Mousetrap, 'unbind'); + + wrapper.destroy(); + + expect(Mousetrap.unbind).toHaveBeenCalledWith(['mod+\\']); + }); }); describe('when peeking on hover', () => { diff --git a/spec/lib/gitlab/ci/secure_files/cer_spec.rb b/spec/lib/gitlab/ci/secure_files/cer_spec.rb index 6b9cd0e3bfc..1393e8d2efd 100644 --- a/spec/lib/gitlab/ci/secure_files/cer_spec.rb +++ b/spec/lib/gitlab/ci/secure_files/cer_spec.rb @@ -9,7 +9,7 @@ RSpec.describe Gitlab::Ci::SecureFiles::Cer do describe '#certificate_data' do it 'assigns the error message and returns nil' do expect(invalid_certificate.certificate_data).to be nil - expect(invalid_certificate.error).to eq('not enough data') + expect(invalid_certificate.error).to eq('PEM_read_bio_X509: no start line') end end diff --git a/spec/lib/gitlab/ci/secure_files/mobile_provision_spec.rb b/spec/lib/gitlab/ci/secure_files/mobile_provision_spec.rb index fb382174c64..1812b90df8b 100644 --- a/spec/lib/gitlab/ci/secure_files/mobile_provision_spec.rb +++ b/spec/lib/gitlab/ci/secure_files/mobile_provision_spec.rb @@ -10,7 +10,7 @@ RSpec.describe Gitlab::Ci::SecureFiles::MobileProvision do describe '#decoded_plist' do it 'assigns the error message and returns nil' do expect(invalid_profile.decoded_plist).to be nil - expect(invalid_profile.error).to eq('Could not parse the PKCS7: not enough data') + expect(invalid_profile.error).to eq('Could not parse the PKCS7: no start line') end end diff --git a/spec/models/ci/secure_file_spec.rb b/spec/models/ci/secure_file_spec.rb index e526e90e250..478af41266d 100644 --- a/spec/models/ci/secure_file_spec.rb +++ b/spec/models/ci/secure_file_spec.rb @@ -194,7 +194,7 @@ RSpec.describe Ci::SecureFile do it 'logs an error when something goes wrong with the file parsing' do corrupt_file = create(:ci_secure_file, name: 'file1.cer', file: CarrierWaveStringFile.new('11111111')) - message = 'Validation failed: Metadata must be a valid json schema - not enough data.' + message = 'Validation failed: Metadata must be a valid json schema - PEM_read_bio_X509: no start line.' expect(Gitlab::AppLogger).to receive(:error).with("Secure File Parser Failure (#{corrupt_file.id}): #{message}") corrupt_file.update_metadata! end diff --git a/spec/services/concerns/exclusive_lease_guard_spec.rb b/spec/services/concerns/exclusive_lease_guard_spec.rb index b081d2642c0..ca8bff4ecc4 100644 --- a/spec/services/concerns/exclusive_lease_guard_spec.rb +++ b/spec/services/concerns/exclusive_lease_guard_spec.rb @@ -49,11 +49,49 @@ RSpec.describe ExclusiveLeaseGuard, :clean_gitlab_redis_shared_state, feature_ca subject.exclusive_lease.cancel end - it 'does not call internal_method but logs error', :aggregate_failures do - expect(subject).not_to receive(:internal_method) - expect(Gitlab::AppLogger).to receive(:error).with("Cannot obtain an exclusive lease for #{subject.lease_key}. There must be another instance already in execution.") + context 'when the class does not override lease_taken_log_level' do + it 'does not call internal_method but logs error', :aggregate_failures do + expect(subject).not_to receive(:internal_method) + expect(Gitlab::AppJsonLogger).to receive(:error).with({ message: "Cannot obtain an exclusive lease. There must be another instance already in execution.", lease_key: 'exclusive_lease_guard_test_class', class_name: 'ExclusiveLeaseGuardTestClass', lease_timeout: 1.second }) - subject.call + subject.call + end + end + + context 'when the class overrides lease_taken_log_level to return :info' do + subject :overwritten_subject_class do + Class.new(subject_class) do + def lease_taken_log_level + :info + end + end + end + + let(:subject) { overwritten_subject_class.new } + + it 'logs info', :aggregate_failures do + expect(Gitlab::AppJsonLogger).to receive(:info).with({ message: "Cannot obtain an exclusive lease. There must be another instance already in execution.", lease_key: 'exclusive_lease_guard_test_class', class_name: 'ExclusiveLeaseGuardTestClass', lease_timeout: 1.second }) + + subject.call + end + end + + context 'when the class overrides lease_taken_log_level to return :debug' do + subject :overwritten_subject_class do + Class.new(subject_class) do + def lease_taken_log_level + :debug + end + end + end + + let(:subject) { overwritten_subject_class.new } + + it 'logs debug', :aggregate_failures do + expect(Gitlab::AppJsonLogger).to receive(:debug).with({ message: "Cannot obtain an exclusive lease. There must be another instance already in execution.", lease_key: 'exclusive_lease_guard_test_class', class_name: 'ExclusiveLeaseGuardTestClass', lease_timeout: 1.second }) + + subject.call + end end end diff --git a/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb b/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb index 3c37792b576..1307e2be3be 100644 --- a/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb +++ b/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb @@ -74,10 +74,13 @@ RSpec.describe ::MergeRequests::AddTodoWhenBuildFailsService, feature_category: context 'when build belongs to a merge request pipeline' do let(:pipeline) do - create(:ci_pipeline, source: :merge_request_event, - ref: merge_request.merge_ref_path, - merge_request: merge_request, - merge_requests_as_head_pipeline: [merge_request]) + create( + :ci_pipeline, + source: :merge_request_event, + ref: merge_request.merge_ref_path, + merge_request: merge_request, + merge_requests_as_head_pipeline: [merge_request] + ) end let(:commit_status) { create(:ci_build, ref: merge_request.merge_ref_path, pipeline: pipeline) } @@ -119,10 +122,13 @@ RSpec.describe ::MergeRequests::AddTodoWhenBuildFailsService, feature_category: context 'when build belongs to a merge request pipeline' do let(:pipeline) do - create(:ci_pipeline, source: :merge_request_event, - ref: merge_request.merge_ref_path, - merge_request: merge_request, - merge_requests_as_head_pipeline: [merge_request]) + create( + :ci_pipeline, + source: :merge_request_event, + ref: merge_request.merge_ref_path, + merge_request: merge_request, + merge_requests_as_head_pipeline: [merge_request] + ) end let(:commit_status) { create(:ci_build, ref: merge_request.merge_ref_path, pipeline: pipeline) } diff --git a/spec/services/merge_requests/assign_issues_service_spec.rb b/spec/services/merge_requests/assign_issues_service_spec.rb index f5fb88b6161..9f82207086b 100644 --- a/spec/services/merge_requests/assign_issues_service_spec.rb +++ b/spec/services/merge_requests/assign_issues_service_spec.rb @@ -37,12 +37,14 @@ RSpec.describe MergeRequests::AssignIssuesService, feature_category: :code_revie it 'accepts precomputed data for closes_issues' do issue2 = create(:issue, project: project) - service2 = described_class.new(project: project, - current_user: user, - params: { - merge_request: merge_request, - closes_issues: [issue, issue2] - }) + service2 = described_class.new( + project: project, + current_user: user, + params: { + merge_request: merge_request, + closes_issues: [issue, issue2] + } + ) expect(service2.assignable_issues.count).to eq 2 end diff --git a/spec/services/merge_requests/conflicts/resolve_service_spec.rb b/spec/services/merge_requests/conflicts/resolve_service_spec.rb index e11cccaa54a..002a07ff14e 100644 --- a/spec/services/merge_requests/conflicts/resolve_service_spec.rb +++ b/spec/services/merge_requests/conflicts/resolve_service_spec.rb @@ -12,15 +12,22 @@ RSpec.describe MergeRequests::Conflicts::ResolveService, feature_category: :code end let(:merge_request) do - create(:merge_request, - source_branch: 'conflict-resolvable', source_project: project, - target_branch: 'conflict-start') + create( + :merge_request, + source_branch: 'conflict-resolvable', + source_project: project, + target_branch: 'conflict-start' + ) end let(:merge_request_from_fork) do - create(:merge_request, - source_branch: 'conflict-resolvable-fork', source_project: forked_project, - target_branch: 'conflict-start', target_project: project) + create( + :merge_request, + source_branch: 'conflict-resolvable-fork', + source_project: forked_project, + target_branch: 'conflict-start', + target_project: project + ) end describe '#execute' do diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb index efbd693fc82..c4de56f39dd 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -249,10 +249,13 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state, f context "when branch pipeline was created before a merge request pipline has been created" do before do - create(:ci_pipeline, project: merge_request.source_project, - sha: merge_request.diff_head_sha, - ref: merge_request.source_branch, - tag: false) + create( + :ci_pipeline, + project: merge_request.source_project, + sha: merge_request.diff_head_sha, + ref: merge_request.source_branch, + tag: false + ) merge_request end @@ -464,8 +467,10 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state, f context "when issuable feature is private" do before do - project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE, - merge_requests_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!( + issues_access_level: ProjectFeature::PRIVATE, + merge_requests_access_level: ProjectFeature::PRIVATE + ) end levels = [Gitlab::VisibilityLevel::INTERNAL, Gitlab::VisibilityLevel::PUBLIC] diff --git a/spec/services/merge_requests/ff_merge_service_spec.rb b/spec/services/merge_requests/ff_merge_service_spec.rb index 993b7cfa85b..f2dbc02f12c 100644 --- a/spec/services/merge_requests/ff_merge_service_spec.rb +++ b/spec/services/merge_requests/ff_merge_service_spec.rb @@ -6,11 +6,13 @@ RSpec.describe MergeRequests::FfMergeService, feature_category: :code_review_wor let(:user) { create(:user) } let(:user2) { create(:user) } let(:merge_request) do - create(:merge_request, - source_branch: 'flatten-dir', - target_branch: 'improve/awesome', - assignees: [user2], - author: create(:user)) + create( + :merge_request, + source_branch: 'flatten-dir', + target_branch: 'improve/awesome', + assignees: [user2], + author: create(:user) + ) end let(:project) { merge_request.project } diff --git a/spec/services/merge_requests/merge_orchestration_service_spec.rb b/spec/services/merge_requests/merge_orchestration_service_spec.rb index 41da7abfeab..389956bf258 100644 --- a/spec/services/merge_requests/merge_orchestration_service_spec.rb +++ b/spec/services/merge_requests/merge_orchestration_service_spec.rb @@ -10,8 +10,11 @@ RSpec.describe MergeRequests::MergeOrchestrationService, feature_category: :code let(:service) { described_class.new(project, user, merge_params) } let!(:merge_request) do - create(:merge_request, source_project: project, source_branch: 'feature', - target_project: project, target_branch: 'master') + create( + :merge_request, + source_project: project, source_branch: 'feature', + target_project: project, target_branch: 'master' + ) end shared_context 'fresh repository' do diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index f380357a659..c77cf288f56 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -69,12 +69,15 @@ RSpec.describe MergeRequests::MergeService, feature_category: :code_review_workf let(:merge_request) do # A merge request with 5 commits - create(:merge_request, :simple, - author: user2, - assignees: [user2], - squash: true, - source_branch: 'improve/awesome', - target_branch: 'fix') + create( + :merge_request, + :simple, + author: user2, + assignees: [user2], + squash: true, + source_branch: 'improve/awesome', + target_branch: 'fix' + ) end it 'merges the merge request with squashed commits' do @@ -147,12 +150,15 @@ RSpec.describe MergeRequests::MergeService, feature_category: :code_review_workf context 'when an invalid sha is passed' do let(:merge_request) do - create(:merge_request, :simple, - author: user2, - assignees: [user2], - squash: true, - source_branch: 'improve/awesome', - target_branch: 'fix') + create( + :merge_request, + :simple, + author: user2, + assignees: [user2], + squash: true, + source_branch: 'improve/awesome', + target_branch: 'fix' + ) end let(:merge_params) do @@ -351,9 +357,12 @@ RSpec.describe MergeRequests::MergeService, feature_category: :code_review_workf service.execute(merge_request) expect(merge_request.merge_error).to eq(error_message) - expect(Gitlab::AppLogger).to have_received(:error) - .with(hash_including(merge_request_info: merge_request.to_reference(full: true), - message: a_string_matching(error_message))) + expect(Gitlab::AppLogger).to have_received(:error).with( + hash_including( + merge_request_info: merge_request.to_reference(full: true), + message: a_string_matching(error_message) + ) + ) end end @@ -366,9 +375,12 @@ RSpec.describe MergeRequests::MergeService, feature_category: :code_review_workf service.execute(merge_request) expect(merge_request.merge_error).to eq(described_class::GENERIC_ERROR_MESSAGE) - expect(Gitlab::AppLogger).to have_received(:error) - .with(hash_including(merge_request_info: merge_request.to_reference(full: true), - message: a_string_matching(error_message))) + expect(Gitlab::AppLogger).to have_received(:error).with( + hash_including( + merge_request_info: merge_request.to_reference(full: true), + message: a_string_matching(error_message) + ) + ) end it 'logs and saves error if user is not authorized' do @@ -394,9 +406,12 @@ RSpec.describe MergeRequests::MergeService, feature_category: :code_review_workf service.execute(merge_request) expect(merge_request.merge_error).to include('Something went wrong during merge pre-receive hook') - expect(Gitlab::AppLogger).to have_received(:error) - .with(hash_including(merge_request_info: merge_request.to_reference(full: true), - message: a_string_matching(error_message))) + expect(Gitlab::AppLogger).to have_received(:error).with( + hash_including( + merge_request_info: merge_request.to_reference(full: true), + message: a_string_matching(error_message) + ) + ) end it 'logs and saves error if commit is not created' do @@ -408,9 +423,12 @@ RSpec.describe MergeRequests::MergeService, feature_category: :code_review_workf expect(merge_request).to be_open expect(merge_request.merge_commit_sha).to be_nil expect(merge_request.merge_error).to include(described_class::GENERIC_ERROR_MESSAGE) - expect(Gitlab::AppLogger).to have_received(:error) - .with(hash_including(merge_request_info: merge_request.to_reference(full: true), - message: a_string_matching(described_class::GENERIC_ERROR_MESSAGE))) + expect(Gitlab::AppLogger).to have_received(:error).with( + hash_including( + merge_request_info: merge_request.to_reference(full: true), + message: a_string_matching(described_class::GENERIC_ERROR_MESSAGE) + ) + ) end context 'when squashing is required' do @@ -429,9 +447,12 @@ RSpec.describe MergeRequests::MergeService, feature_category: :code_review_workf expect(merge_request.merge_commit_sha).to be_nil expect(merge_request.squash_commit_sha).to be_nil expect(merge_request.merge_error).to include(error_message) - expect(Gitlab::AppLogger).to have_received(:error) - .with(hash_including(merge_request_info: merge_request.to_reference(full: true), - message: a_string_matching(error_message))) + expect(Gitlab::AppLogger).to have_received(:error).with( + hash_including( + merge_request_info: merge_request.to_reference(full: true), + message: a_string_matching(error_message) + ) + ) end end @@ -452,9 +473,12 @@ RSpec.describe MergeRequests::MergeService, feature_category: :code_review_workf expect(merge_request.merge_commit_sha).to be_nil expect(merge_request.squash_commit_sha).to be_nil expect(merge_request.merge_error).to include(error_message) - expect(Gitlab::AppLogger).to have_received(:error) - .with(hash_including(merge_request_info: merge_request.to_reference(full: true), - message: a_string_matching(error_message))) + expect(Gitlab::AppLogger).to have_received(:error).with( + hash_including( + merge_request_info: merge_request.to_reference(full: true), + message: a_string_matching(error_message) + ) + ) end it 'logs and saves error if there is an PreReceiveError exception' do @@ -470,9 +494,12 @@ RSpec.describe MergeRequests::MergeService, feature_category: :code_review_workf expect(merge_request.merge_commit_sha).to be_nil expect(merge_request.squash_commit_sha).to be_nil expect(merge_request.merge_error).to include('Something went wrong during merge pre-receive hook') - expect(Gitlab::AppLogger).to have_received(:error) - .with(hash_including(merge_request_info: merge_request.to_reference(full: true), - message: a_string_matching(error_message))) + expect(Gitlab::AppLogger).to have_received(:error).with( + hash_including( + merge_request_info: merge_request.to_reference(full: true), + message: a_string_matching(error_message) + ) + ) end context 'when fast-forward merge is not allowed' do @@ -494,9 +521,12 @@ RSpec.describe MergeRequests::MergeService, feature_category: :code_review_workf expect(merge_request.merge_commit_sha).to be_nil expect(merge_request.squash_commit_sha).to be_nil expect(merge_request.merge_error).to include(error_message) - expect(Gitlab::AppLogger).to have_received(:error) - .with(hash_including(merge_request_info: merge_request.to_reference(full: true), - message: a_string_matching(error_message))) + expect(Gitlab::AppLogger).to have_received(:error).with( + hash_including( + merge_request_info: merge_request.to_reference(full: true), + message: a_string_matching(error_message) + ) + ) end end end @@ -513,9 +543,12 @@ RSpec.describe MergeRequests::MergeService, feature_category: :code_review_workf it 'logs and saves error' do service.execute(merge_request) - expect(Gitlab::AppLogger).to have_received(:error) - .with(hash_including(merge_request_info: merge_request.to_reference(full: true), - message: a_string_matching(error_message))) + expect(Gitlab::AppLogger).to have_received(:error).with( + hash_including( + merge_request_info: merge_request.to_reference(full: true), + message: a_string_matching(error_message) + ) + ) end end @@ -527,9 +560,12 @@ RSpec.describe MergeRequests::MergeService, feature_category: :code_review_workf it 'logs and saves error' do service.execute(merge_request) - expect(Gitlab::AppLogger).to have_received(:error) - .with(hash_including(merge_request_info: merge_request.to_reference(full: true), - message: a_string_matching(error_message))) + expect(Gitlab::AppLogger).to have_received(:error).with( + hash_including( + merge_request_info: merge_request.to_reference(full: true), + message: a_string_matching(error_message) + ) + ) end context 'when passing `skip_discussions_check: true` as `options` parameter' do diff --git a/spec/services/merge_requests/merge_to_ref_service_spec.rb b/spec/services/merge_requests/merge_to_ref_service_spec.rb index 8428848c3c8..8200f60b072 100644 --- a/spec/services/merge_requests/merge_to_ref_service_spec.rb +++ b/spec/services/merge_requests/merge_to_ref_service_spec.rb @@ -210,11 +210,14 @@ RSpec.describe MergeRequests::MergeToRefService, feature_category: :code_review_ let(:merge_request) { create(:merge_request, assignees: [user], author: user) } let(:project) { merge_request.project } let!(:todo) do - create(:todo, :assigned, - project: project, - author: user, - user: user, - target: merge_request) + create( + :todo, + :assigned, + project: project, + author: user, + user: user, + target: merge_request + ) end before do @@ -258,8 +261,10 @@ RSpec.describe MergeRequests::MergeToRefService, feature_category: :code_review_ context 'when first merge happens' do let(:merge_request) do - create(:merge_request, source_project: project, source_branch: 'feature', - target_project: project, target_branch: 'master') + create( + :merge_request, source_project: project, source_branch: 'feature', + target_project: project, target_branch: 'master' + ) end it_behaves_like 'successfully merges to ref with merge method' do @@ -269,8 +274,11 @@ RSpec.describe MergeRequests::MergeToRefService, feature_category: :code_review_ context 'when second merge happens' do let(:merge_request) do - create(:merge_request, source_project: project, source_branch: 'improve/awesome', - target_project: project, target_branch: 'master') + create( + :merge_request, + source_project: project, source_branch: 'improve/awesome', + target_project: project, target_branch: 'master' + ) end it_behaves_like 'successfully merges to ref with merge method' do diff --git a/spec/services/merge_requests/mergeability/detailed_merge_status_service_spec.rb b/spec/services/merge_requests/mergeability/detailed_merge_status_service_spec.rb index 876b1e7f23a..a037d21bd94 100644 --- a/spec/services/merge_requests/mergeability/detailed_merge_status_service_spec.rb +++ b/spec/services/merge_requests/mergeability/detailed_merge_status_service_spec.rb @@ -72,9 +72,14 @@ RSpec.describe ::MergeRequests::Mergeability::DetailedMergeStatusService, featur context 'when pipeline exists' do before do - create(:ci_pipeline, ci_status, merge_request: merge_request, - project: merge_request.project, sha: merge_request.source_branch_sha, - head_pipeline_of: merge_request) + create( + :ci_pipeline, + ci_status, + merge_request: merge_request, + project: merge_request.project, + sha: merge_request.source_branch_sha, + head_pipeline_of: merge_request + ) end context 'when the pipeline is running' do diff --git a/spec/services/merge_requests/mergeability_check_service_spec.rb b/spec/services/merge_requests/mergeability_check_service_spec.rb index 881157e7f11..82ef8440380 100644 --- a/spec/services/merge_requests/mergeability_check_service_spec.rb +++ b/spec/services/merge_requests/mergeability_check_service_spec.rb @@ -158,9 +158,11 @@ RSpec.describe MergeRequests::MergeabilityCheckService, :clean_gitlab_redis_shar threads = execute_within_threads(amount: 3, retry_lease: false) results = threads.map { |t| [t.value.status, t.value.message] } - expect(results).to contain_exactly([:error, 'Failed to obtain a lock'], - [:error, 'Failed to obtain a lock'], - [:success, nil]) + expect(results).to contain_exactly( + [:error, 'Failed to obtain a lock'], + [:error, 'Failed to obtain a lock'], + [:success, nil] + ) end end end @@ -183,11 +185,13 @@ RSpec.describe MergeRequests::MergeabilityCheckService, :clean_gitlab_redis_shar context 'when it cannot be merged on git' do let(:merge_request) do - create(:merge_request, - merge_status: :unchecked, - source_branch: 'conflict-resolvable', - source_project: project, - target_branch: 'conflict-start') + create( + :merge_request, + merge_status: :unchecked, + source_branch: 'conflict-resolvable', + source_project: project, + target_branch: 'conflict-start' + ) end it 'returns ServiceResponse.error and keeps merge status as cannot_be_merged' do diff --git a/spec/services/merge_requests/rebase_service_spec.rb b/spec/services/merge_requests/rebase_service_spec.rb index 704dc1f9000..c8b9ab5a34e 100644 --- a/spec/services/merge_requests/rebase_service_spec.rb +++ b/spec/services/merge_requests/rebase_service_spec.rb @@ -8,10 +8,12 @@ RSpec.describe MergeRequests::RebaseService, feature_category: :source_code_mana let(:user) { create(:user) } let(:rebase_jid) { 'fake-rebase-jid' } let(:merge_request) do - create :merge_request, - source_branch: 'feature_conflict', - target_branch: 'master', - rebase_jid: rebase_jid + create( + :merge_request, + source_branch: 'feature_conflict', + target_branch: 'master', + rebase_jid: rebase_jid + ) end let(:project) { merge_request.project } @@ -102,8 +104,9 @@ RSpec.describe MergeRequests::RebaseService, feature_category: :source_code_mana end it 'returns an error' do - expect(service.execute(merge_request)).to match(status: :error, - message: described_class::REBASE_ERROR) + expect(service.execute(merge_request)).to match( + status: :error, message: described_class::REBASE_ERROR + ) end it 'logs the error' do @@ -154,8 +157,9 @@ RSpec.describe MergeRequests::RebaseService, feature_category: :source_code_mana end it 'returns an error' do - expect(service.execute(merge_request)).to match(status: :error, - message: described_class::REBASE_ERROR) + expect(service.execute(merge_request)).to match( + status: :error, message: described_class::REBASE_ERROR + ) end end @@ -215,9 +219,11 @@ RSpec.describe MergeRequests::RebaseService, feature_category: :source_code_mana message: 'Add new file to target', branch_name: 'master') - create(:merge_request, - source_branch: 'master', source_project: forked_project, - target_branch: 'master', target_project: project) + create( + :merge_request, + source_branch: 'master', source_project: forked_project, + target_branch: 'master', target_project: project + ) end it 'rebases source branch', :sidekiq_might_not_need_inline do diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index d8e5eb3bcde..4d533b67690 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -19,43 +19,53 @@ RSpec.describe MergeRequests::RefreshService, feature_category: :code_review_wor @project = create(:project, :repository, namespace: group) @fork_project = fork_project(@project, @user, repository: true) - @merge_request = create(:merge_request, - source_project: @project, - source_branch: 'master', - target_branch: 'feature', - target_project: @project, - auto_merge_enabled: true, - auto_merge_strategy: AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS, - merge_user: @user) - - @another_merge_request = create(:merge_request, - source_project: @project, - source_branch: 'master', - target_branch: 'test', - target_project: @project, - auto_merge_enabled: true, - auto_merge_strategy: AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS, - merge_user: @user) - - @fork_merge_request = create(:merge_request, - source_project: @fork_project, - source_branch: 'master', - target_branch: 'feature', - target_project: @project) - - @build_failed_todo = create(:todo, - :build_failed, - user: @user, - project: @project, - target: @merge_request, - author: @user) - - @fork_build_failed_todo = create(:todo, - :build_failed, - user: @user, - project: @project, - target: @merge_request, - author: @user) + @merge_request = create( + :merge_request, + source_project: @project, + source_branch: 'master', + target_branch: 'feature', + target_project: @project, + auto_merge_enabled: true, + auto_merge_strategy: AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS, + merge_user: @user + ) + + @another_merge_request = create( + :merge_request, + source_project: @project, + source_branch: 'master', + target_branch: 'test', + target_project: @project, + auto_merge_enabled: true, + auto_merge_strategy: AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS, + merge_user: @user + ) + + @fork_merge_request = create( + :merge_request, + source_project: @fork_project, + source_branch: 'master', + target_branch: 'feature', + target_project: @project + ) + + @build_failed_todo = create( + :todo, + :build_failed, + user: @user, + project: @project, + target: @merge_request, + author: @user + ) + + @fork_build_failed_todo = create( + :todo, + :build_failed, + user: @user, + project: @project, + target: @merge_request, + author: @user + ) @commits = @merge_request.commits @@ -306,10 +316,13 @@ RSpec.describe MergeRequests::RefreshService, feature_category: :code_review_wor context "when branch pipeline was created before a detaced merge request pipeline has been created" do before do - create(:ci_pipeline, project: @merge_request.source_project, - sha: @merge_request.diff_head_sha, - ref: @merge_request.source_branch, - tag: false) + create( + :ci_pipeline, + project: @merge_request.source_project, + sha: @merge_request.diff_head_sha, + ref: @merge_request.source_branch, + tag: false + ) subject end @@ -340,9 +353,11 @@ RSpec.describe MergeRequests::RefreshService, feature_category: :code_review_wor context 'when the pipeline should be skipped' do it 'saves a skipped detached merge request pipeline' do - project.repository.create_file(@user, 'new-file.txt', 'A new file', - message: '[skip ci] This is a test', - branch_name: 'master') + project.repository.create_file( + @user, 'new-file.txt', 'A new file', + message: '[skip ci] This is a test', + branch_name: 'master' + ) expect { subject } .to change { @merge_request.pipelines_for_merge_request.count }.by(1) @@ -452,11 +467,13 @@ RSpec.describe MergeRequests::RefreshService, feature_category: :code_review_wor context 'when an MR to be closed was empty already' do let!(:empty_fork_merge_request) do - create(:merge_request, - source_project: @fork_project, - source_branch: 'master', - target_branch: 'master', - target_project: @project) + create( + :merge_request, + source_project: @fork_project, + source_branch: 'master', + target_branch: 'master', + target_project: @project + ) end before do @@ -597,23 +614,33 @@ RSpec.describe MergeRequests::RefreshService, feature_category: :code_review_wor context 'forked projects with the same source branch name as target branch' do let!(:first_commit) do - @fork_project.repository.create_file(@user, 'test1.txt', 'Test data', - message: 'Test commit', - branch_name: 'master') + @fork_project.repository.create_file( + @user, + 'test1.txt', + 'Test data', + message: 'Test commit', + branch_name: 'master' + ) end let!(:second_commit) do - @fork_project.repository.create_file(@user, 'test2.txt', 'More test data', - message: 'Second test commit', - branch_name: 'master') + @fork_project.repository.create_file( + @user, + 'test2.txt', + 'More test data', + message: 'Second test commit', + branch_name: 'master' + ) end let!(:forked_master_mr) do - create(:merge_request, - source_project: @fork_project, - source_branch: 'master', - target_branch: 'master', - target_project: @project) + create( + :merge_request, + source_project: @fork_project, + source_branch: 'master', + target_branch: 'master', + target_project: @project + ) end let(:force_push_commit) { @project.commit('feature').id } @@ -647,9 +674,13 @@ RSpec.describe MergeRequests::RefreshService, feature_category: :code_review_wor end it 'does not increase the diff count for a new push to target branch' do - new_commit = @project.repository.create_file(@user, 'new-file.txt', 'A new file', - message: 'This is a test', - branch_name: 'master') + new_commit = @project.repository.create_file( + @user, + 'new-file.txt', + 'A new file', + message: 'This is a test', + branch_name: 'master' + ) expect do service.new(project: @project, current_user: @user).execute(@newrev, new_commit, 'refs/heads/master') @@ -721,10 +752,12 @@ RSpec.describe MergeRequests::RefreshService, feature_category: :code_review_wor CommitCollection.new(project, [commit], 'close-by-commit') ) - merge_request = create(:merge_request, - target_branch: 'master', - source_branch: 'close-by-commit', - source_project: project) + merge_request = create( + :merge_request, + target_branch: 'master', + source_branch: 'close-by-commit', + source_project: project + ) refresh_service = service.new(project: project, current_user: user) allow(refresh_service).to receive(:execute_hooks) @@ -743,11 +776,13 @@ RSpec.describe MergeRequests::RefreshService, feature_category: :code_review_wor CommitCollection.new(forked_project, [commit], 'close-by-commit') ) - merge_request = create(:merge_request, - target_branch: 'master', - target_project: project, - source_branch: 'close-by-commit', - source_project: forked_project) + merge_request = create( + :merge_request, + target_branch: 'master', + target_project: project, + source_branch: 'close-by-commit', + source_project: forked_project + ) refresh_service = service.new(project: forked_project, current_user: user) allow(refresh_service).to receive(:execute_hooks) @@ -767,11 +802,13 @@ RSpec.describe MergeRequests::RefreshService, feature_category: :code_review_wor end it 'marks the merge request as draft from fixup commits' do - fixup_merge_request = create(:merge_request, - source_project: @project, - source_branch: 'wip', - target_branch: 'master', - target_project: @project) + fixup_merge_request = create( + :merge_request, + source_project: @project, + source_branch: 'wip', + target_branch: 'master', + target_project: @project + ) commits = fixup_merge_request.commits oldrev = commits.last.id newrev = commits.first.id @@ -786,11 +823,13 @@ RSpec.describe MergeRequests::RefreshService, feature_category: :code_review_wor end it 'references the commit that caused the draft status' do - draft_merge_request = create(:merge_request, - source_project: @project, - source_branch: 'wip', - target_branch: 'master', - target_project: @project) + draft_merge_request = create( + :merge_request, + source_project: @project, + source_branch: 'wip', + target_branch: 'master', + target_project: @project + ) commits = draft_merge_request.commits oldrev = commits.last.id @@ -904,22 +943,23 @@ RSpec.describe MergeRequests::RefreshService, feature_category: :code_review_wor end let_it_be(:merge_request, refind: true) do - create(:merge_request, - author: author, - source_project: source_project, - source_branch: 'feature', - target_branch: 'master', - target_project: target_project, - auto_merge_enabled: true, - auto_merge_strategy: AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS, - merge_user: user) + create( + :merge_request, + author: author, + source_project: source_project, + source_branch: 'feature', + target_branch: 'master', + target_project: target_project, + auto_merge_enabled: true, + auto_merge_strategy: AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS, + merge_user: user + ) end let_it_be(:newrev) do - target_project - .repository - .create_file(user, 'test1.txt', 'Test data', - message: 'Test commit', branch_name: 'master') + target_project.repository.create_file( + user, 'test1.txt', 'Test data', message: 'Test commit', branch_name: 'master' + ) end let_it_be(:oldrev) do diff --git a/spec/services/merge_requests/reload_diffs_service_spec.rb b/spec/services/merge_requests/reload_diffs_service_spec.rb index 4dd05f75a3e..675b458c435 100644 --- a/spec/services/merge_requests/reload_diffs_service_spec.rb +++ b/spec/services/merge_requests/reload_diffs_service_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe MergeRequests::ReloadDiffsService, :use_clean_rails_memory_store_caching, -feature_category: :code_review_workflow do + feature_category: :code_review_workflow do let(:current_user) { create(:user) } let(:merge_request) { create(:merge_request) } let(:subject) { described_class.new(merge_request, current_user) } @@ -19,10 +19,9 @@ feature_category: :code_review_workflow do new_diff_refs = merge_request.diff_refs expect(merge_request).to receive(:create_merge_request_diff).and_return(new_diff) - expect(merge_request).to receive(:update_diff_discussion_positions) - .with(old_diff_refs: old_diff_refs, - new_diff_refs: new_diff_refs, - current_user: current_user) + expect(merge_request).to receive(:update_diff_discussion_positions).with( + old_diff_refs: old_diff_refs, new_diff_refs: new_diff_refs, current_user: current_user + ) subject.execute end diff --git a/spec/services/merge_requests/squash_service_spec.rb b/spec/services/merge_requests/squash_service_spec.rb index 26e225db9fc..1afca466fb5 100644 --- a/spec/services/merge_requests/squash_service_spec.rb +++ b/spec/services/merge_requests/squash_service_spec.rb @@ -13,21 +13,27 @@ RSpec.describe MergeRequests::SquashService, feature_category: :source_code_mana end let(:merge_request_with_one_commit) do - create(:merge_request, - source_branch: 'feature', source_project: project, - target_branch: 'master', target_project: project) + create( + :merge_request, + source_branch: 'feature', source_project: project, + target_branch: 'master', target_project: project + ) end let(:merge_request_with_only_new_files) do - create(:merge_request, - source_branch: 'video', source_project: project, - target_branch: 'master', target_project: project) + create( + :merge_request, + source_branch: 'video', source_project: project, + target_branch: 'master', target_project: project + ) end let(:merge_request_with_large_files) do - create(:merge_request, - source_branch: 'squash-large-files', source_project: project, - target_branch: 'master', target_project: project) + create( + :merge_request, + source_branch: 'squash-large-files', source_project: project, + target_branch: 'master', target_project: project + ) end shared_examples 'the squash succeeds' do diff --git a/spec/services/merge_requests/update_assignees_service_spec.rb b/spec/services/merge_requests/update_assignees_service_spec.rb index 7d08eb86441..85d749de83c 100644 --- a/spec/services/merge_requests/update_assignees_service_spec.rb +++ b/spec/services/merge_requests/update_assignees_service_spec.rb @@ -12,13 +12,17 @@ RSpec.describe MergeRequests::UpdateAssigneesService, feature_category: :code_re let_it_be(:user3) { create(:user) } let_it_be_with_reload(:merge_request) do - create(:merge_request, :simple, :unique_branches, - title: 'Old title', - description: "FYI #{user2.to_reference}", - assignee_ids: [user3.id], - source_project: project, - target_project: project, - author: create(:user)) + create( + :merge_request, + :simple, + :unique_branches, + title: 'Old title', + description: "FYI #{user2.to_reference}", + assignee_ids: [user3.id], + source_project: project, + target_project: project, + author: create(:user) + ) end before do @@ -107,12 +111,16 @@ RSpec.describe MergeRequests::UpdateAssigneesService, feature_category: :code_re .with(merge_request, [user3], execute_hooks: true) end - other_mr = create(:merge_request, :simple, :unique_branches, - title: merge_request.title, - description: merge_request.description, - assignee_ids: merge_request.assignee_ids, - source_project: merge_request.project, - author: merge_request.author) + other_mr = create( + :merge_request, + :simple, + :unique_branches, + title: merge_request.title, + description: merge_request.description, + assignee_ids: merge_request.assignee_ids, + source_project: merge_request.project, + author: merge_request.author + ) update_service = ::MergeRequests::UpdateService.new(project: project, current_user: user, params: opts) diff --git a/spec/services/merge_requests/update_reviewers_service_spec.rb b/spec/services/merge_requests/update_reviewers_service_spec.rb index ed2d448b523..1ae20e8c29c 100644 --- a/spec/services/merge_requests/update_reviewers_service_spec.rb +++ b/spec/services/merge_requests/update_reviewers_service_spec.rb @@ -12,13 +12,17 @@ RSpec.describe MergeRequests::UpdateReviewersService, feature_category: :code_re let_it_be(:user3) { create(:user) } let_it_be_with_reload(:merge_request) do - create(:merge_request, :simple, :unique_branches, - title: 'Old title', - description: "FYI #{user2.to_reference}", - reviewer_ids: [user3.id], - source_project: project, - target_project: project, - author: create(:user)) + create( + :merge_request, + :simple, + :unique_branches, + title: 'Old title', + description: "FYI #{user2.to_reference}", + reviewer_ids: [user3.id], + source_project: project, + target_project: project, + author: create(:user) + ) end before do diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 000c85fd1f8..012eb5f6fca 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -15,11 +15,15 @@ RSpec.describe MergeRequests::UpdateService, :mailer, feature_category: :code_re let(:milestone) { create(:milestone, project: project) } let(:merge_request) do - create(:merge_request, :simple, title: 'Old title', - description: "FYI #{user2.to_reference}", - assignee_ids: [user3.id], - source_project: project, - author: create(:user)) + create( + :merge_request, + :simple, + title: 'Old title', + description: "FYI #{user2.to_reference}", + assignee_ids: [user3.id], + source_project: project, + author: create(:user) + ) end before do @@ -1187,10 +1191,12 @@ RSpec.describe MergeRequests::UpdateService, :mailer, feature_category: :code_re let(:source_project) { fork_project(target_project, nil, repository: true) } let(:user) { create(:user) } let(:merge_request) do - create(:merge_request, - source_project: source_project, - source_branch: 'fixes', - target_project: target_project) + create( + :merge_request, + source_project: source_project, + source_branch: 'fixes', + target_project: target_project + ) end before do @@ -1222,10 +1228,12 @@ RSpec.describe MergeRequests::UpdateService, :mailer, feature_category: :code_re let(:source_project) { fork_project(target_project, nil, repository: true) } let(:user) { target_project.first_owner } let(:merge_request) do - create(:merge_request, - source_project: source_project, - source_branch: 'fixes', - target_project: target_project) + create( + :merge_request, + source_project: source_project, + source_branch: 'fixes', + target_project: target_project + ) end it "cannot be done by members of the target project when they don't have access" do @@ -1243,10 +1251,12 @@ RSpec.describe MergeRequests::UpdateService, :mailer, feature_category: :code_re context 'updating `target_branch`' do let(:merge_request) do - create(:merge_request, - source_project: project, - source_branch: 'mr-b', - target_branch: 'mr-a') + create( + :merge_request, + source_project: project, + source_branch: 'mr-b', + target_branch: 'mr-a' + ) end it 'updates to master' do diff --git a/spec/services/projects/git_deduplication_service_spec.rb b/spec/services/projects/git_deduplication_service_spec.rb index 314c9d160dd..2b9f0974ae2 100644 --- a/spec/services/projects/git_deduplication_service_spec.rb +++ b/spec/services/projects/git_deduplication_service_spec.rb @@ -139,7 +139,7 @@ RSpec.describe Projects::GitDeduplicationService, feature_category: :source_code end it 'fails when a lease is already out' do - expect(service).to receive(:log_error).with("Cannot obtain an exclusive lease for #{lease_key}. There must be another instance already in execution.") + expect(Gitlab::AppJsonLogger).to receive(:error).with({ message: "Cannot obtain an exclusive lease. There must be another instance already in execution.", lease_key: lease_key, class_name: described_class.name, lease_timeout: lease_timeout }) service.execute end diff --git a/spec/support/helpers/usage_data_helpers.rb b/spec/support/helpers/usage_data_helpers.rb index 9abfc39e31f..beee663fbc6 100644 --- a/spec/support/helpers/usage_data_helpers.rb +++ b/spec/support/helpers/usage_data_helpers.rb @@ -120,14 +120,14 @@ module UsageDataHelpers end def stub_prometheus_queries - stub_request(:get, %r{^https?://::1:9090/-/ready}) + stub_request(:get, %r{^https?://.*:9090/-/ready}) .to_return( status: 200, body: [{}].to_json, headers: { 'Content-Type' => 'application/json' } ) - stub_request(:get, %r{^https?://::1:9090/api/v1/query\?query=.*}) + stub_request(:get, %r{^https?://.*:9090/api/v1/query\?query=.*}) .to_return( status: 200, body: [{}].to_json, diff --git a/spec/support/shared_examples/db/seeds/awesome_co_shared_examples.rb b/spec/support/shared_examples/db/seeds/awesome_co_shared_examples.rb new file mode 100644 index 00000000000..ae0fe19f6b9 --- /dev/null +++ b/spec/support/shared_examples/db/seeds/awesome_co_shared_examples.rb @@ -0,0 +1,111 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'raises an error when specifying an invalid factory' do + it 'raises an error' do + expect { parser.parse }.to raise_error(RuntimeError, /invalids.*to a valid registered Factory/) + end +end + +RSpec.shared_examples 'specifying invalid traits to a factory' do + it 'raises an error', :aggregate_failures do + expect { parser.parse }.to raise_error do |error| + expect(error).to be_a(RuntimeError) + expect(error.message).to include('Trait not registered: \\"invalid\\"') + expect(error.message).to include('for Factory \\"issue\\"') + end + end +end + +RSpec.shared_examples 'specifying invalid attributes to a factory' do + it 'raises an error' do + expect { parser.parse }.to raise_error(RuntimeError, /is not a valid attribute/) + end + + it 'contains possible alternatives' do + expect { parser.parse }.to raise_error(RuntimeError, /Did you mean/) + end +end + +RSpec.shared_examples 'an id already exists' do + it 'raises a validation error' do + expect { parser.parse }.to raise_error(/id `my_label` must be unique/) + end +end + +RSpec.shared_examples 'name is not specified' do + it 'raises an error when name is not specified' do + expect { parser.parse }.to raise_error(/Seed file must specify a name/) + end +end + +RSpec.shared_examples 'factory definitions' do + it 'has exactly two definitions' do + parser.parse + + expect(parser.definitions.size).to eq(2) + end + + it 'creates the group label' do + expect { parser.parse }.to change { GroupLabel.count }.by(1) + end + + it 'creates the project' do + expect { parser.parse }.to change { Project.count }.by(1) + end +end + +RSpec.shared_examples 'passes traits' do + it 'passes traits' do + expect_next_instance_of(AwesomeCo::FactoryDefinitions::FactoryDefinition) do |instance| + # `described` trait will automaticaly generate a description + expect(instance.build(binding).description).to eq('Description of Test Label') + end + + parser.parse + end +end + +RSpec.shared_examples 'has a name' do + it 'has a name' do + parser.parse + + expect(parser.name).to eq('Test') + end +end + +RSpec.shared_examples 'definition has an id' do + it 'binds the object', :aggregate_failures do + parser.parse + + expect(group_labels).to be_a(OpenStruct) # rubocop:disable Style/OpenStructUse + expect(group_labels.my_label).to be_a(GroupLabel) + expect(group_labels.my_label.title).to eq('My Label') + end +end + +RSpec.shared_examples 'id has spaces' do + it 'binds to an underscored variable', :aggregate_failures do + parser.parse + + expect(group_labels).to respond_to(:id_with_spaces) + expect(group_labels.id_with_spaces.title).to eq('With Spaces') + end + + it 'renders a warning' do + expect { parser.parse }.to output(%(parsing id "id with spaces" as "id_with_spaces"\n)).to_stderr + end +end + +RSpec.shared_examples 'definition does not have an id' do + it 'does not bind the object' do + parser.parse + + expect(group_labels.to_h).to be_empty + end +end + +RSpec.shared_examples 'invalid id' do |message| + it 'raises an error' do + expect { parser.parse }.to raise_error(message) + end +end diff --git a/spec/workers/deployments/drop_older_deployments_worker_spec.rb b/spec/workers/deployments/drop_older_deployments_worker_spec.rb deleted file mode 100644 index 8637a7788cc..00000000000 --- a/spec/workers/deployments/drop_older_deployments_worker_spec.rb +++ /dev/null @@ -1,18 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Deployments::DropOlderDeploymentsWorker, feature_category: :continuous_delivery do - subject { described_class.new.perform(deployment&.id) } - - describe '#perform' do - let(:deployment) { create(:deployment, :success) } - - it 'executes Deployments::OlderDeploymentsDropService' do - expect(Deployments::OlderDeploymentsDropService) - .to receive(:new).with(deployment.id).and_call_original - - subject - end - end -end |