diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-05-22 00:08:31 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-05-22 00:08:31 +0300 |
commit | e7bc93852d0ce48c490a780b6a1adc6cc36dd342 (patch) | |
tree | b07651f4700e8ec2338298b4d224b6252b505eef | |
parent | 34e72e54129090eaae6e045890fcdf8b5ad3f629 (diff) |
Add latest changes from gitlab-org/gitlab@master
79 files changed, 621 insertions, 208 deletions
diff --git a/.rubocop.yml b/.rubocop.yml index b6f7bd3da8c..c9e8e44bd7c 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -351,12 +351,8 @@ RSpec/LeakyConstantDeclaration: - 'spec/db/schema_spec.rb' - 'spec/lib/feature_spec.rb' - 'spec/lib/gitlab/config/entry/simplifiable_spec.rb' - - 'spec/lib/gitlab/database/migration_helpers_spec.rb' - - 'spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb' - - 'spec/lib/gitlab/database/with_lock_retries_spec.rb' - 'spec/lib/gitlab/git/diff_collection_spec.rb' - 'spec/lib/gitlab/import_export/import_test_coverage_spec.rb' - - 'spec/lib/gitlab/import_export/project/relation_factory_spec.rb' - 'spec/lib/gitlab/quick_actions/dsl_spec.rb' - 'spec/lib/marginalia_spec.rb' - 'spec/mailers/notify_spec.rb' diff --git a/app/assets/javascripts/commons/bootstrap.js b/app/assets/javascripts/commons/bootstrap.js index e5e1cbb1e62..df0fa1ae88b 100644 --- a/app/assets/javascripts/commons/bootstrap.js +++ b/app/assets/javascripts/commons/bootstrap.js @@ -70,7 +70,12 @@ whitelist.acronym = []; whitelist.blockquote = []; whitelist.del = []; whitelist.ins = []; -whitelist['gl-emoji'] = []; +whitelist['gl-emoji'] = [ + 'data-name', + 'data-unicode-version', + 'data-fallback-src', + 'data-fallback-sprite-class', +]; // Whitelisting SVG tags and attributes whitelist.svg = ['viewBox']; diff --git a/app/assets/javascripts/diff_notes/components/jump_to_discussion.js b/app/assets/javascripts/diff_notes/components/jump_to_discussion.js index 0c521fa29bd..0991f5282a8 100644 --- a/app/assets/javascripts/diff_notes/components/jump_to_discussion.js +++ b/app/assets/javascripts/diff_notes/components/jump_to_discussion.js @@ -1,4 +1,4 @@ -/* eslint-disable func-names, guard-for-in, no-restricted-syntax, no-lonely-if, no-continue */ +/* eslint-disable func-names, no-continue */ /* global CommentsStore */ import $ from 'jquery'; @@ -42,13 +42,13 @@ const JumpToDiscussion = Vue.extend({ }, lastResolvedId() { let lastId; - for (const discussionId in this.discussions) { + Object.keys(this.discussions).forEach(discussionId => { const discussion = this.discussions[discussionId]; if (!discussion.isResolved()) { lastId = discussion.id; } - } + }); return lastId; }, }, @@ -95,12 +95,10 @@ const JumpToDiscussion = Vue.extend({ if (unresolvedDiscussionCount === 1) { hasDiscussionsToJumpTo = false; } - } else { + } else if (unresolvedDiscussionCount === 0) { // If there are no unresolved discussions on the diffs tab at all, // there are no discussions to jump to. - if (unresolvedDiscussionCount === 0) { - hasDiscussionsToJumpTo = false; - } + hasDiscussionsToJumpTo = false; } } else if (activeTab !== 'show') { // If we are on the commits or builds tabs, diff --git a/app/assets/javascripts/monitoring/components/dashboard.vue b/app/assets/javascripts/monitoring/components/dashboard.vue index 2018c706b11..3ea4a24f421 100644 --- a/app/assets/javascripts/monitoring/components/dashboard.vue +++ b/app/assets/javascripts/monitoring/components/dashboard.vue @@ -226,7 +226,7 @@ export default { 'allDashboards', 'environmentsLoading', 'expandedPanel', - 'promVariables', + 'variables', 'isUpdatingStarredValue', ]), ...mapGetters('monitoringDashboard', [ @@ -251,7 +251,7 @@ export default { return !this.environmentsLoading && this.filteredEnvironments.length === 0; }, shouldShowVariablesSection() { - return Object.keys(this.promVariables).length > 0; + return Object.keys(this.variables).length > 0; }, }, watch: { @@ -273,7 +273,7 @@ export default { handler({ group, panel }) { const dashboardPath = this.currentDashboard || this.selectedDashboard?.path; updateHistory({ - url: panelToUrl(dashboardPath, convertVariablesForURL(this.promVariables), group, panel), + url: panelToUrl(dashboardPath, convertVariablesForURL(this.variables), group, panel), title: document.title, }); }, @@ -344,7 +344,7 @@ export default { }, generatePanelUrl(groupKey, panel) { const dashboardPath = this.currentDashboard || this.selectedDashboard?.path; - return panelToUrl(dashboardPath, convertVariablesForURL(this.promVariables), groupKey, panel); + return panelToUrl(dashboardPath, convertVariablesForURL(this.variables), groupKey, panel); }, hideAddMetricModal() { this.$refs.addMetricModal.hide(); diff --git a/app/assets/javascripts/monitoring/components/variables_section.vue b/app/assets/javascripts/monitoring/components/variables_section.vue index e054c9d8e26..1175e3bb461 100644 --- a/app/assets/javascripts/monitoring/components/variables_section.vue +++ b/app/assets/javascripts/monitoring/components/variables_section.vue @@ -2,7 +2,7 @@ import { mapState, mapActions } from 'vuex'; import CustomVariable from './variables/custom_variable.vue'; import TextVariable from './variables/text_variable.vue'; -import { setPromCustomVariablesFromUrl } from '../utils'; +import { setCustomVariablesFromUrl } from '../utils'; export default { components: { @@ -10,12 +10,12 @@ export default { TextVariable, }, computed: { - ...mapState('monitoringDashboard', ['promVariables']), + ...mapState('monitoringDashboard', ['variables']), }, methods: { ...mapActions('monitoringDashboard', ['fetchDashboardData', 'updateVariableValues']), refreshDashboard(variable, value) { - if (this.promVariables[variable].value !== value) { + if (this.variables[variable].value !== value) { const changedVariable = { key: variable, value }; // update the Vuex store this.updateVariableValues(changedVariable); @@ -24,7 +24,7 @@ export default { // mutation respond directly. // This can be further investigate in // https://gitlab.com/gitlab-org/gitlab/-/issues/217713 - setPromCustomVariablesFromUrl(this.promVariables); + setCustomVariablesFromUrl(this.variables); // fetch data this.fetchDashboardData(); } @@ -41,7 +41,7 @@ export default { </script> <template> <div ref="variablesSection" class="d-sm-flex flex-sm-wrap pt-2 pr-1 pb-0 pl-2 variables-section"> - <div v-for="(variable, key) in promVariables" :key="key" class="mb-1 pr-2 d-flex d-sm-block"> + <div v-for="(variable, key) in variables" :key="key" class="mb-1 pr-2 d-flex d-sm-block"> <component :is="variableComponent(variable.type)" class="mb-0 flex-grow-1" diff --git a/app/assets/javascripts/monitoring/stores/actions.js b/app/assets/javascripts/monitoring/stores/actions.js index b057afa2264..fec2aba4575 100644 --- a/app/assets/javascripts/monitoring/stores/actions.js +++ b/app/assets/javascripts/monitoring/stores/actions.js @@ -223,7 +223,7 @@ export const fetchPrometheusMetric = ( queryParams.step = metric.step; } - if (Object.keys(state.promVariables).length > 0) { + if (Object.keys(state.variables).length > 0) { queryParams.variables = getters.getCustomVariablesArray; } diff --git a/app/assets/javascripts/monitoring/stores/getters.js b/app/assets/javascripts/monitoring/stores/getters.js index ae3ff5596e1..f3b1e5a7dde 100644 --- a/app/assets/javascripts/monitoring/stores/getters.js +++ b/app/assets/javascripts/monitoring/stores/getters.js @@ -122,7 +122,7 @@ export const filteredEnvironments = state => */ export const getCustomVariablesArray = state => - flatMap(state.promVariables, (variable, key) => [key, variable.value]); + flatMap(state.variables, (variable, key) => [key, variable.value]); // prevent babel-plugin-rewire from generating an invalid default during karma tests export default () => {}; diff --git a/app/assets/javascripts/monitoring/stores/mutations.js b/app/assets/javascripts/monitoring/stores/mutations.js index f41cf3fc477..36e4014e766 100644 --- a/app/assets/javascripts/monitoring/stores/mutations.js +++ b/app/assets/javascripts/monitoring/stores/mutations.js @@ -189,11 +189,11 @@ export default { state.expandedPanel.panel = panel; }, [types.SET_VARIABLES](state, variables) { - state.promVariables = variables; + state.variables = variables; }, [types.UPDATE_VARIABLE_VALUES](state, updatedVariable) { - Object.assign(state.promVariables[updatedVariable.key], { - ...state.promVariables[updatedVariable.key], + Object.assign(state.variables[updatedVariable.key], { + ...state.variables[updatedVariable.key], value: updatedVariable.value, }); }, diff --git a/app/assets/javascripts/monitoring/stores/state.js b/app/assets/javascripts/monitoring/stores/state.js index 9ae1da93e5f..9d47a9a2ad7 100644 --- a/app/assets/javascripts/monitoring/stores/state.js +++ b/app/assets/javascripts/monitoring/stores/state.js @@ -34,7 +34,11 @@ export default () => ({ panel: null, }, allDashboards: [], - promVariables: {}, + /** + * User-defined custom variables are passed + * via the dashboard.yml file. + */ + variables: {}, // Other project data annotations: [], diff --git a/app/assets/javascripts/monitoring/utils.js b/app/assets/javascripts/monitoring/utils.js index 1f028ffbcad..95d544bd6d4 100644 --- a/app/assets/javascripts/monitoring/utils.js +++ b/app/assets/javascripts/monitoring/utils.js @@ -151,7 +151,7 @@ export const removePrefixFromLabel = label => /** * Convert parsed template variables to an object - * with just keys and values. Prepare the promVariables + * with just keys and values. Prepare the variables * to be added to the URL. Keys of the object will * have a prefix so that these params can be * differentiated from other URL params. @@ -183,15 +183,15 @@ export const getPromCustomVariablesFromUrl = (search = window.location.search) = }; /** - * Update the URL with promVariables. This usually get triggered when + * Update the URL with variables. This usually get triggered when * the user interacts with the dynamic input elements in the monitoring * dashboard header. * - * @param {Object} promVariables user defined variables + * @param {Object} variables user defined variables */ -export const setPromCustomVariablesFromUrl = promVariables => { +export const setCustomVariablesFromUrl = variables => { // prep the variables to append to URL - const parsedVariables = convertVariablesForURL(promVariables); + const parsedVariables = convertVariablesForURL(variables); // update the URL updateHistory({ url: mergeUrlParams(parsedVariables, window.location.href), @@ -262,7 +262,7 @@ export const expandedPanelPayloadFromUrl = (dashboard, search = window.location. * If no group/panel is set, the dashboard URL is returned. * * @param {?String} dashboard - Dashboard path, used as identifier for a dashboard - * @param {?Object} promVariables - Custom variables that came from the URL + * @param {?Object} variables - Custom variables that came from the URL * @param {?String} group - Group Identifier * @param {?Object} panel - Panel object from the dashboard * @param {?String} url - Base URL including current search params @@ -270,14 +270,14 @@ export const expandedPanelPayloadFromUrl = (dashboard, search = window.location. */ export const panelToUrl = ( dashboard = null, - promVariables, + variables, group, panel, url = window.location.href, ) => { const params = { dashboard, - ...promVariables, + ...variables, }; if (group && panel) { diff --git a/app/models/concerns/cacheable_attributes.rb b/app/models/concerns/cacheable_attributes.rb index d459af23a2f..de176ffde5c 100644 --- a/app/models/concerns/cacheable_attributes.rb +++ b/app/models/concerns/cacheable_attributes.rb @@ -55,7 +55,7 @@ module CacheableAttributes current_without_cache.tap { |current_record| current_record&.cache! } rescue => e if Rails.env.production? - Rails.logger.warn("Cached record for #{name} couldn't be loaded, falling back to uncached record: #{e}") # rubocop:disable Gitlab/RailsLogger + Gitlab::AppLogger.warn("Cached record for #{name} couldn't be loaded, falling back to uncached record: #{e}") else raise e end diff --git a/app/models/concerns/storage/legacy_namespace.rb b/app/models/concerns/storage/legacy_namespace.rb index da4f2a79895..250889fdf8b 100644 --- a/app/models/concerns/storage/legacy_namespace.rb +++ b/app/models/concerns/storage/legacy_namespace.rb @@ -67,7 +67,7 @@ module Storage unless gitlab_shell.mv_namespace(repository_storage, full_path_before_last_save, full_path) - Rails.logger.error "Exception moving path #{repository_storage} from #{full_path_before_last_save} to #{full_path}" # rubocop:disable Gitlab/RailsLogger + Gitlab::AppLogger.error("Exception moving path #{repository_storage} from #{full_path_before_last_save} to #{full_path}") # if we cannot move namespace directory we should rollback # db changes in order to prevent out of sync between db and fs diff --git a/app/models/concerns/update_highest_role.rb b/app/models/concerns/update_highest_role.rb index 7efc436c6c8..6432cc794a5 100644 --- a/app/models/concerns/update_highest_role.rb +++ b/app/models/concerns/update_highest_role.rb @@ -29,9 +29,7 @@ module UpdateHighestRole UpdateHighestRoleWorker.perform_in(HIGHEST_ROLE_JOB_DELAY, update_highest_role_attribute) else # use same logging as ExclusiveLeaseGuard - # rubocop:disable Gitlab/RailsLogger - Rails.logger.error('Cannot obtain an exclusive lease. There must be another instance already in execution.') - # rubocop:enable Gitlab/RailsLogger + Gitlab::AppLogger.error('Cannot obtain an exclusive lease. There must be another instance already in execution.') end end end diff --git a/app/models/storage/legacy_project.rb b/app/models/storage/legacy_project.rb index 345172cca76..f643d52587e 100644 --- a/app/models/storage/legacy_project.rb +++ b/app/models/storage/legacy_project.rb @@ -35,7 +35,7 @@ module Storage gitlab_shell.mv_repository(repository_storage, "#{old_full_path}.wiki", "#{new_full_path}.wiki") return true rescue => e - Rails.logger.error "Exception renaming #{old_full_path} -> #{new_full_path}: #{e}" # rubocop:disable Gitlab/RailsLogger + Gitlab::AppLogger.error("Exception renaming #{old_full_path} -> #{new_full_path}: #{e}") # Returning false does not rollback after_* transaction but gives # us information about failing some of tasks return false diff --git a/app/models/uploads/base.rb b/app/models/uploads/base.rb index 442ed733566..7555c72e101 100644 --- a/app/models/uploads/base.rb +++ b/app/models/uploads/base.rb @@ -7,7 +7,7 @@ module Uploads attr_reader :logger def initialize(logger: nil) - @logger = Rails.logger # rubocop:disable Gitlab/RailsLogger + @logger = Gitlab::AppLogger end def delete_keys_async(keys_to_delete) diff --git a/app/services/concerns/exclusive_lease_guard.rb b/app/services/concerns/exclusive_lease_guard.rb index 0c5ecca3a50..4678d051d29 100644 --- a/app/services/concerns/exclusive_lease_guard.rb +++ b/app/services/concerns/exclusive_lease_guard.rb @@ -58,6 +58,6 @@ module ExclusiveLeaseGuard end def log_error(message, extra_args = {}) - Rails.logger.error(message) # rubocop:disable Gitlab/RailsLogger + Gitlab::AppLogger.error(message) end end diff --git a/app/services/groups/destroy_service.rb b/app/services/groups/destroy_service.rb index 9437eb9eede..1bff70e6c2e 100644 --- a/app/services/groups/destroy_service.rb +++ b/app/services/groups/destroy_service.rb @@ -6,7 +6,7 @@ module Groups def async_execute job_id = GroupDestroyWorker.perform_async(group.id, current_user.id) - Rails.logger.info("User #{current_user.id} scheduled a deletion of group ID #{group.id} with job ID #{job_id}") # rubocop:disable Gitlab/RailsLogger + Gitlab::AppLogger.info("User #{current_user.id} scheduled a deletion of group ID #{group.id} with job ID #{job_id}") end # rubocop: disable CodeReuse/ActiveRecord diff --git a/app/services/labels/create_service.rb b/app/services/labels/create_service.rb index c032985be42..a5b30e29e55 100644 --- a/app/services/labels/create_service.rb +++ b/app/services/labels/create_service.rb @@ -20,7 +20,7 @@ module Labels label.save label else - Rails.logger.warn("target_params should contain :project or :group or :template, actual value: #{target_params}") # rubocop:disable Gitlab/RailsLogger + Gitlab::AppLogger.warn("target_params should contain :project or :group or :template, actual value: #{target_params}") end end end diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index 31097b9151a..8d57a76f7d0 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -121,12 +121,12 @@ module MergeRequests end def handle_merge_error(log_message:, save_message_on_model: false) - Rails.logger.error("MergeService ERROR: #{merge_request_info} - #{log_message}") # rubocop:disable Gitlab/RailsLogger + Gitlab::AppLogger.error("MergeService ERROR: #{merge_request_info} - #{log_message}") @merge_request.update(merge_error: log_message) if save_message_on_model end def log_info(message) - @logger ||= Rails.logger # rubocop:disable Gitlab/RailsLogger + @logger ||= Gitlab::AppLogger @logger.info("#{merge_request_info} - #{message}") end diff --git a/app/services/projects/after_import_service.rb b/app/services/projects/after_import_service.rb index ee2dde8aa7f..b09a8e0bece 100644 --- a/app/services/projects/after_import_service.rb +++ b/app/services/projects/after_import_service.rb @@ -22,7 +22,7 @@ module Projects # causing GC to run every time. service.increment! rescue Projects::HousekeepingService::LeaseTaken => e - Rails.logger.info( # rubocop:disable Gitlab/RailsLogger + Gitlab::AppLogger.info( "Could not perform housekeeping for project #{@project.full_path} (#{@project.id}): #{e}") end diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index 8acc83a0d27..8dc54ee4ea0 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -166,7 +166,7 @@ module Projects log_message = message.dup log_message << " Project ID: #{@project.id}" if @project&.id - Rails.logger.error(log_message) # rubocop:disable Gitlab/RailsLogger + Gitlab::AppLogger.error(log_message) if @project && @project.persisted? && @project.import_state @project.import_state.mark_as_failed(message) diff --git a/app/services/projects/import_export/export_service.rb b/app/services/projects/import_export/export_service.rb index 86cb4f35206..0c515479cfa 100644 --- a/app/services/projects/import_export/export_service.rb +++ b/app/services/projects/import_export/export_service.rb @@ -115,11 +115,11 @@ module Projects end def notify_success - Rails.logger.info("Import/Export - Project #{project.name} with ID: #{project.id} successfully exported") # rubocop:disable Gitlab/RailsLogger + Gitlab::AppLogger.info("Import/Export - Project #{project.name} with ID: #{project.id} successfully exported") end def notify_error - Rails.logger.error("Import/Export - Project #{project.name} with ID: #{project.id} export error - #{shared.errors.join(', ')}") # rubocop:disable Gitlab/RailsLogger + Gitlab::AppLogger.error("Import/Export - Project #{project.name} with ID: #{project.id} export error - #{shared.errors.join(', ')}") notification_service.project_not_exported(project, current_user, shared.errors) end diff --git a/app/services/projects/update_statistics_service.rb b/app/services/projects/update_statistics_service.rb index cc6ffa9eafc..a0793cff2df 100644 --- a/app/services/projects/update_statistics_service.rb +++ b/app/services/projects/update_statistics_service.rb @@ -5,7 +5,7 @@ module Projects def execute return unless project - Rails.logger.info("Updating statistics for project #{project.id}") # rubocop:disable Gitlab/RailsLogger + Gitlab::AppLogger.info("Updating statistics for project #{project.id}") project.statistics.refresh!(only: statistics.map(&:to_sym)) end diff --git a/app/services/spam/akismet_service.rb b/app/services/spam/akismet_service.rb index ab35fb8700f..e11a1dbdd96 100644 --- a/app/services/spam/akismet_service.rb +++ b/app/services/spam/akismet_service.rb @@ -27,7 +27,7 @@ module Spam is_spam, is_blatant = akismet_client.check(options[:ip_address], options[:user_agent], params) is_spam || is_blatant rescue => e - Rails.logger.error("Unable to connect to Akismet: #{e}, skipping check") # rubocop:disable Gitlab/RailsLogger + Gitlab::AppLogger.error("Unable to connect to Akismet: #{e}, skipping check") false end end @@ -67,7 +67,7 @@ module Spam akismet_client.public_send(type, options[:ip_address], options[:user_agent], params) # rubocop:disable GitlabSecurity/PublicSend true rescue => e - Rails.logger.error("Unable to connect to Akismet: #{e}, skipping!") # rubocop:disable Gitlab/RailsLogger + Gitlab::AppLogger.error("Unable to connect to Akismet: #{e}, skipping!") false end end diff --git a/app/services/submit_usage_ping_service.rb b/app/services/submit_usage_ping_service.rb index 3265eb106eb..4bbde3a9648 100644 --- a/app/services/submit_usage_ping_service.rb +++ b/app/services/submit_usage_ping_service.rb @@ -28,7 +28,7 @@ class SubmitUsagePingService true rescue Gitlab::HTTP::Error => e - Rails.logger.info "Unable to contact GitLab, Inc.: #{e}" # rubocop:disable Gitlab/RailsLogger + Gitlab::AppLogger.info("Unable to contact GitLab, Inc.: #{e}") false end diff --git a/app/services/web_hook_service.rb b/app/services/web_hook_service.rb index 178a321e20c..91a26ff45b1 100644 --- a/app/services/web_hook_service.rb +++ b/app/services/web_hook_service.rb @@ -63,7 +63,7 @@ class WebHookService error_message: e.to_s ) - Rails.logger.error("WebHook Error => #{e}") # rubocop:disable Gitlab/RailsLogger + Gitlab::AppLogger.error("WebHook Error => #{e}") { status: :error, diff --git a/app/uploaders/file_mover.rb b/app/uploaders/file_mover.rb index 7c7953c8a0e..887cb702acf 100644 --- a/app/uploaders/file_mover.rb +++ b/app/uploaders/file_mover.rb @@ -98,7 +98,7 @@ class FileMover end def revert - Rails.logger.warn("Markdown not updated, file move reverted for #{to_model}") # rubocop:disable Gitlab/RailsLogger + Gitlab::AppLogger.warn("Markdown not updated, file move reverted for #{to_model}") if temp_file_uploader.file_storage? FileUtils.move(file_path, temp_file_path) diff --git a/app/workers/create_commit_signature_worker.rb b/app/workers/create_commit_signature_worker.rb index a88d2bf7d15..aeb6104a35c 100644 --- a/app/workers/create_commit_signature_worker.rb +++ b/app/workers/create_commit_signature_worker.rb @@ -37,7 +37,7 @@ class CreateCommitSignatureWorker commits.each do |commit| commit&.signature rescue => e - Rails.logger.error("Failed to create signature for commit #{commit.id}. Error: #{e.message}") # rubocop:disable Gitlab/RailsLogger + Gitlab::AppLogger.error("Failed to create signature for commit #{commit.id}. Error: #{e.message}") end end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/workers/delete_user_worker.rb b/app/workers/delete_user_worker.rb index d3b87c133d3..2f2bf500730 100644 --- a/app/workers/delete_user_worker.rb +++ b/app/workers/delete_user_worker.rb @@ -11,6 +11,6 @@ class DeleteUserWorker # rubocop:disable Scalability/IdempotentWorker Users::DestroyService.new(current_user).execute(delete_user, options.symbolize_keys) rescue Gitlab::Access::AccessDeniedError => e - Rails.logger.warn("User could not be destroyed: #{e}") # rubocop:disable Gitlab/RailsLogger + Gitlab::AppLogger.warn("User could not be destroyed: #{e}") end end diff --git a/app/workers/email_receiver_worker.rb b/app/workers/email_receiver_worker.rb index fcb88982c0b..9ceab9bb878 100644 --- a/app/workers/email_receiver_worker.rb +++ b/app/workers/email_receiver_worker.rb @@ -20,7 +20,7 @@ class EmailReceiverWorker # rubocop:disable Scalability/IdempotentWorker private def handle_failure(raw, error) - Rails.logger.warn("Email can not be processed: #{error}\n\n#{raw}") # rubocop:disable Gitlab/RailsLogger + Gitlab::AppLogger.warn("Email can not be processed: #{error}\n\n#{raw}") return unless raw.present? diff --git a/app/workers/expire_build_instance_artifacts_worker.rb b/app/workers/expire_build_instance_artifacts_worker.rb index 48fd086f88f..e6cd60a3e47 100644 --- a/app/workers/expire_build_instance_artifacts_worker.rb +++ b/app/workers/expire_build_instance_artifacts_worker.rb @@ -14,7 +14,7 @@ class ExpireBuildInstanceArtifactsWorker # rubocop:disable Scalability/Idempoten return unless build&.project && !build.project.pending_delete - Rails.logger.info "Removing artifacts for build #{build.id}..." # rubocop:disable Gitlab/RailsLogger + Gitlab::AppLogger.info("Removing artifacts for build #{build.id}...") build.erase_erasable_artifacts! end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/workers/new_note_worker.rb b/app/workers/new_note_worker.rb index 8ead87a9230..ee1d2237001 100644 --- a/app/workers/new_note_worker.rb +++ b/app/workers/new_note_worker.rb @@ -16,7 +16,7 @@ class NewNoteWorker # rubocop:disable Scalability/IdempotentWorker NotificationService.new.new_note(note) unless skip_notification?(note) Notes::PostProcessService.new(note).execute else - Rails.logger.error("NewNoteWorker: couldn't find note with ID=#{note_id}, skipping job") # rubocop:disable Gitlab/RailsLogger + Gitlab::AppLogger.error("NewNoteWorker: couldn't find note with ID=#{note_id}, skipping job") end end diff --git a/changelogs/unreleased/28589-emoji-status-popover-doesn-t-show-emoji-when-it-s-in-the-message.yml b/changelogs/unreleased/28589-emoji-status-popover-doesn-t-show-emoji-when-it-s-in-the-message.yml new file mode 100644 index 00000000000..85f6aa53840 --- /dev/null +++ b/changelogs/unreleased/28589-emoji-status-popover-doesn-t-show-emoji-when-it-s-in-the-message.yml @@ -0,0 +1,5 @@ +--- +title: Fix rendering of emojis in status tooltips +merge_request: 32604 +author: +type: fixed diff --git a/changelogs/unreleased/docs-auto-build-cnb-custom-builder.yml b/changelogs/unreleased/docs-auto-build-cnb-custom-builder.yml new file mode 100644 index 00000000000..8e96cf2d53a --- /dev/null +++ b/changelogs/unreleased/docs-auto-build-cnb-custom-builder.yml @@ -0,0 +1,5 @@ +--- +title: Customize the Cloud Native Buildpack builder used with Auto Build +merge_request: 32691 +author: +type: added diff --git a/changelogs/unreleased/leaky-constant-fix-33.yml b/changelogs/unreleased/leaky-constant-fix-33.yml new file mode 100644 index 00000000000..f53da019813 --- /dev/null +++ b/changelogs/unreleased/leaky-constant-fix-33.yml @@ -0,0 +1,5 @@ +--- +title: Fix leaky constant issue in relation factory spec +merge_request: 32129 +author: Rajendra Kadam +type: fixed diff --git a/changelogs/unreleased/leaky-constant-fix-37.yml b/changelogs/unreleased/leaky-constant-fix-37.yml new file mode 100644 index 00000000000..af986b7bb13 --- /dev/null +++ b/changelogs/unreleased/leaky-constant-fix-37.yml @@ -0,0 +1,5 @@ +--- +title: Fix leaky constant issue in migration helpers, with lock retries and ignored cols spec +merge_request: 32170 +author: Rajendra Kadam +type: fixed diff --git a/changelogs/unreleased/rails-logger-cop-10.yml b/changelogs/unreleased/rails-logger-cop-10.yml new file mode 100644 index 00000000000..c81a504350c --- /dev/null +++ b/changelogs/unreleased/rails-logger-cop-10.yml @@ -0,0 +1,5 @@ +--- +title: Use applogger in exclusive_lease_guard +merge_request: 32194 +author: Rajendra Kadam +type: fixed diff --git a/changelogs/unreleased/rails-logger-cop-11.yml b/changelogs/unreleased/rails-logger-cop-11.yml new file mode 100644 index 00000000000..fa337bf4658 --- /dev/null +++ b/changelogs/unreleased/rails-logger-cop-11.yml @@ -0,0 +1,5 @@ +--- +title: Use applogger in groups destroy service and label create service +merge_request: 32195 +author: Rajendra Kadam +type: fixed diff --git a/changelogs/unreleased/rails-logger-cop-12.yml b/changelogs/unreleased/rails-logger-cop-12.yml new file mode 100644 index 00000000000..c08f8d5426a --- /dev/null +++ b/changelogs/unreleased/rails-logger-cop-12.yml @@ -0,0 +1,5 @@ +--- +title: Use applogger in merge_service.rb +merge_request: 32196 +author: Rajendra Kadam +type: fixed diff --git a/changelogs/unreleased/rails-logger-cop-13.yml b/changelogs/unreleased/rails-logger-cop-13.yml new file mode 100644 index 00000000000..ab0414f0ae3 --- /dev/null +++ b/changelogs/unreleased/rails-logger-cop-13.yml @@ -0,0 +1,5 @@ +--- +title: Use applogger in project create service and after import service +merge_request: 32198 +author: Rajendra Kadam +type: fixed diff --git a/changelogs/unreleased/rails-logger-cop-14.yml b/changelogs/unreleased/rails-logger-cop-14.yml new file mode 100644 index 00000000000..07f332d9d35 --- /dev/null +++ b/changelogs/unreleased/rails-logger-cop-14.yml @@ -0,0 +1,5 @@ +--- +title: Use applogger in update stats service +merge_request: 32200 +author: Rajendra Kadam +type: fixed diff --git a/changelogs/unreleased/rails-logger-cop-16.yml b/changelogs/unreleased/rails-logger-cop-16.yml new file mode 100644 index 00000000000..be60fdce4d0 --- /dev/null +++ b/changelogs/unreleased/rails-logger-cop-16.yml @@ -0,0 +1,5 @@ +--- +title: Use applogger in export service +merge_request: 32203 +author: Rajendra Kadam +type: fixed diff --git a/changelogs/unreleased/rails-logger-cop-17.yml b/changelogs/unreleased/rails-logger-cop-17.yml new file mode 100644 index 00000000000..013457961cf --- /dev/null +++ b/changelogs/unreleased/rails-logger-cop-17.yml @@ -0,0 +1,5 @@ +--- +title: Use applogger in akismet service +merge_request: 32205 +author: Rajendra Kadam +type: fixed diff --git a/changelogs/unreleased/rails-logger-cop-18.yml b/changelogs/unreleased/rails-logger-cop-18.yml new file mode 100644 index 00000000000..d5ebd1b4a11 --- /dev/null +++ b/changelogs/unreleased/rails-logger-cop-18.yml @@ -0,0 +1,5 @@ +--- +title: Use applogger in file mover file +merge_request: 32206 +author: Rajendra Kadam +type: fixed diff --git a/changelogs/unreleased/rails-logger-cop-19.yml b/changelogs/unreleased/rails-logger-cop-19.yml new file mode 100644 index 00000000000..b56f06caf73 --- /dev/null +++ b/changelogs/unreleased/rails-logger-cop-19.yml @@ -0,0 +1,5 @@ +--- +title: Use applogger in commit signature worker +merge_request: 32207 +author: Rajendra Kadam +type: fixed diff --git a/changelogs/unreleased/rails-logger-cop-20.yml b/changelogs/unreleased/rails-logger-cop-20.yml new file mode 100644 index 00000000000..1128b8022e3 --- /dev/null +++ b/changelogs/unreleased/rails-logger-cop-20.yml @@ -0,0 +1,5 @@ +--- +title: Use applogger in delete user worker +merge_request: 32209 +author: Rajendra Kadam +type: fixed diff --git a/changelogs/unreleased/rails-logger-cop-21.yml b/changelogs/unreleased/rails-logger-cop-21.yml new file mode 100644 index 00000000000..099142ff482 --- /dev/null +++ b/changelogs/unreleased/rails-logger-cop-21.yml @@ -0,0 +1,5 @@ +--- +title: Use applogger in email receiver worker +merge_request: 32211 +author: Rajendra Kadam +type: fixed diff --git a/changelogs/unreleased/rails-logger-cop-22.yml b/changelogs/unreleased/rails-logger-cop-22.yml new file mode 100644 index 00000000000..df22074b16a --- /dev/null +++ b/changelogs/unreleased/rails-logger-cop-22.yml @@ -0,0 +1,5 @@ +--- +title: Use applogger in artifact worker +merge_request: 32212 +author: Rajendra Kadam +type: fixed diff --git a/changelogs/unreleased/rails-logger-cop-23.yml b/changelogs/unreleased/rails-logger-cop-23.yml new file mode 100644 index 00000000000..4e7513570e9 --- /dev/null +++ b/changelogs/unreleased/rails-logger-cop-23.yml @@ -0,0 +1,5 @@ +--- +title: Use applogger in new note worker +merge_request: 32213 +author: Rajendra Kadam +type: fixed diff --git a/changelogs/unreleased/rails-logger-cop-6.yml b/changelogs/unreleased/rails-logger-cop-6.yml new file mode 100644 index 00000000000..a5fea225b08 --- /dev/null +++ b/changelogs/unreleased/rails-logger-cop-6.yml @@ -0,0 +1,5 @@ +--- +title: Use applogger in cache attrs and highest role ruby files +merge_request: 32189 +author: Rajendra Kadam +type: fixed diff --git a/changelogs/unreleased/rails-logger-cop-7.yml b/changelogs/unreleased/rails-logger-cop-7.yml new file mode 100644 index 00000000000..b8482ccef54 --- /dev/null +++ b/changelogs/unreleased/rails-logger-cop-7.yml @@ -0,0 +1,5 @@ +--- +title: Use applogger in legacy project and namespace +merge_request: 32190 +author: Rajendra Kadam +type: fixed diff --git a/changelogs/unreleased/rails-logger-cop-8.yml b/changelogs/unreleased/rails-logger-cop-8.yml new file mode 100644 index 00000000000..4765c087bb7 --- /dev/null +++ b/changelogs/unreleased/rails-logger-cop-8.yml @@ -0,0 +1,5 @@ +--- +title: Use applogger in base.rb +merge_request: 32191 +author: Rajendra Kadam +type: fixed diff --git a/changelogs/unreleased/rails-logger-cop-9.yml b/changelogs/unreleased/rails-logger-cop-9.yml new file mode 100644 index 00000000000..8e740f6f1bd --- /dev/null +++ b/changelogs/unreleased/rails-logger-cop-9.yml @@ -0,0 +1,5 @@ +--- +title: Use applogger in usage ping and webhook service +merge_request: 32192 +author: Rajendra Kadam +type: fixed diff --git a/danger/bundle_size/Dangerfile b/danger/bundle_size/Dangerfile index e2621360c7f..a7102cd0e38 100644 --- a/danger/bundle_size/Dangerfile +++ b/danger/bundle_size/Dangerfile @@ -22,7 +22,7 @@ analyze_cmd = [ # from master that was merged into for the merged pipeline. comparison_cmd = [ "webpack-compare-reports", - "--from-sha #{gitlab.mr_json["diff_refs"]["start_sha"]}", + "--job #{ENV["CI_JOB_ID"]}", "--to-file #{analysis_result}", "--html ./bundle-size-review/comparison.html", "--markdown #{markdown_result}" diff --git a/doc/development/testing_guide/end_to_end/rspec_metadata_tests.md b/doc/development/testing_guide/end_to_end/rspec_metadata_tests.md index b7c93d205a3..89adfceb7c9 100644 --- a/doc/development/testing_guide/end_to_end/rspec_metadata_tests.md +++ b/doc/development/testing_guide/end_to_end/rspec_metadata_tests.md @@ -16,3 +16,4 @@ This is a partial list of the [RSpec metadata](https://relishapp.com/rspec/rspec | `:runner` | The test depends on and will set up a GitLab Runner instance, typically to run a pipeline. | | `:gitaly_ha` | The test will run against a GitLab instance where repositories are stored on redundant Gitaly nodes behind a Praefect node. All nodes are [separate containers](../../../administration/gitaly/praefect.md#requirements-for-configuring-a-gitaly-cluster). Tests that use this tag have a longer setup time since there are three additional containers that need to be started. | | `:skip_live_env` | The test will be excluded when run against live deployed environments such as Staging, Canary, and Production. | +| `:jira` | The test requires a Jira Server. [GitLab-QA](https://gitlab.com/gitlab-org/gitlab-qa) will provision the Jira Server in a docker container when the `Test::Integration::Jira` test scenario is run. diff --git a/doc/topics/autodevops/customize.md b/doc/topics/autodevops/customize.md index 67e2a1269a1..98b858ed0a6 100644 --- a/doc/topics/autodevops/customize.md +++ b/doc/topics/autodevops/customize.md @@ -302,6 +302,7 @@ applications. | `<ENVIRONMENT>_ADDITIONAL_HOSTS` | For a specific environment, the fully qualified domain names specified as a comma-separated list that are added to the Ingress hosts. This takes precedence over `ADDITIONAL_HOSTS`. | | `AUTO_DEVOPS_ATOMIC_RELEASE` | As of GitLab 13.0, Auto DevOps uses [`--atomic`](https://v2.helm.sh/docs/helm/#options-43) for Helm deployments by default. Set this variable to `false` to disable the use of `--atomic` | | `AUTO_DEVOPS_BUILD_IMAGE_CNB_ENABLED` | When set to a non-empty value and no `Dockerfile` is present, Auto Build builds your application using Cloud Native Buildpacks instead of Herokuish. [More details](stages.md#auto-build-using-cloud-native-buildpacks-beta). | +| `AUTO_DEVOPS_BUILD_IMAGE_CNB_BUILDER` | The builder used when building with Cloud Native Buildpacks. The default builder is `heroku/buildpacks:18`. [More details](stages.md#auto-build-using-cloud-native-buildpacks-beta). | | `AUTO_DEVOPS_BUILD_IMAGE_EXTRA_ARGS` | Extra arguments to be passed to the `docker build` command. Note that using quotes won't prevent word splitting. [More details](#passing-arguments-to-docker-build). | | `AUTO_DEVOPS_BUILD_IMAGE_FORWARDED_CI_VARIABLES` | A [comma-separated list of CI variable names](#passing-secrets-to-docker-build) to be passed to the `docker build` command as secrets. | | `AUTO_DEVOPS_CHART` | Helm Chart used to deploy your apps. Defaults to the one [provided by GitLab](https://gitlab.com/gitlab-org/charts/auto-deploy-app). | diff --git a/doc/topics/autodevops/stages.md b/doc/topics/autodevops/stages.md index 8c56a87ba30..9017d0c6404 100644 --- a/doc/topics/autodevops/stages.md +++ b/doc/topics/autodevops/stages.md @@ -53,7 +53,9 @@ troubleshoot. Auto Build supports building your application using [Cloud Native Buildpacks](https://buildpacks.io) through the [`pack` command](https://github.com/buildpacks/pack). To use Cloud Native Buildpacks, -set the CI variable `AUTO_DEVOPS_BUILD_IMAGE_CNB_ENABLED` to a non-empty value. +set the CI variable `AUTO_DEVOPS_BUILD_IMAGE_CNB_ENABLED` to a non-empty +value. The default builder is `heroku/buildpacks:18` but a different builder +can be selected using the CI variable `AUTO_DEVOPS_BUILD_IMAGE_CNB_BUILDER`. Cloud Native Buildpacks (CNBs) are an evolution of Heroku buildpacks, and will eventually supersede Herokuish-based builds within Auto DevOps. For more @@ -254,6 +254,7 @@ module QA autoload :Main, 'qa/page/project/settings/main' autoload :Repository, 'qa/page/project/settings/repository' autoload :CICD, 'qa/page/project/settings/ci_cd' + autoload :Integrations, 'qa/page/project/settings/integrations' autoload :GeneralPipelines, 'qa/page/project/settings/general_pipelines' autoload :AutoDevops, 'qa/page/project/settings/auto_devops' autoload :DeployKeys, 'qa/page/project/settings/deploy_keys' @@ -265,6 +266,10 @@ module QA autoload :Members, 'qa/page/project/settings/members' autoload :MirroringRepositories, 'qa/page/project/settings/mirroring_repositories' autoload :VisibilityFeaturesPermissions, 'qa/page/project/settings/visibility_features_permissions' + + module Services + autoload :Jira, 'qa/page/project/settings/services/jira' + end autoload :Operations, 'qa/page/project/settings/operations' autoload :Incidents, 'qa/page/project/settings/incidents' autoload :Integrations, 'qa/page/project/settings/integrations' @@ -512,6 +517,10 @@ module QA autoload :ConfigureJob, 'qa/vendor/jenkins/page/configure_job' end end + + module Jira + autoload :JiraAPI, 'qa/vendor/jira/jira_api' + end end # Classes that provide support to other parts of the framework. diff --git a/qa/qa/page/project/settings/integrations.rb b/qa/qa/page/project/settings/integrations.rb index 436a42fb093..e18ff71bcb3 100644 --- a/qa/qa/page/project/settings/integrations.rb +++ b/qa/qa/page/project/settings/integrations.rb @@ -7,13 +7,20 @@ module QA class Integrations < QA::Page::Base view 'app/views/shared/integrations/_index.html.haml' do element :prometheus_link, '{ data: { qa_selector: "#{integration.to_param' # rubocop:disable QA/ElementWithPattern + element :jira_link, '{ data: { qa_selector: "#{integration.to_param' # rubocop:disable QA/ElementWithPattern end def click_on_prometheus_integration click_element :prometheus_link end + + def click_jira_link + click_element :jira_link + end end end end end end + +QA::Page::Project::Settings::Integrations.prepend_if_ee('QA::EE::Page::Project::Settings::Integrations') diff --git a/qa/qa/page/project/settings/services/jira.rb b/qa/qa/page/project/settings/services/jira.rb new file mode 100644 index 00000000000..9f9331bac94 --- /dev/null +++ b/qa/qa/page/project/settings/services/jira.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +module QA + module Page + module Project + module Settings + module Services + class Jira < QA::Page::Base + view 'app/views/shared/_field.html.haml' do + element :url_field, 'data: { qa_selector: "#{name.downcase.gsub' # rubocop:disable QA/ElementWithPattern + element :username_field, 'data: { qa_selector: "#{name.downcase.gsub' # rubocop:disable QA/ElementWithPattern + element :password_field, 'data: { qa_selector: "#{name.downcase.gsub' # rubocop:disable QA/ElementWithPattern + element :jira_issue_transition_id_field, 'data: { qa_selector: "#{name.downcase.gsub' # rubocop:disable QA/ElementWithPattern + end + + view 'app/helpers/services_helper.rb' do + element :save_changes_button + end + + def setup_service_with(url:) + QA::Runtime::Logger.info "Setting up JIRA" + + set_jira_server_url(url) + set_username(Runtime::Env.jira_admin_username) + set_password(Runtime::Env.jira_admin_password) + set_transaction_ids('11,21,31,41') + + click_save_changes_button + wait_until(reload: false) do + has_element?(:save_changes_button, wait: 1) ? !find_element(:save_changes_button).disabled? : true + end + end + + private + + def set_jira_server_url(url) + fill_element(:url_field, url) + end + + def set_username(username) + fill_element(:username_field, username) + end + + def set_password(password) + fill_element(:password_field, password) + end + + def set_transaction_ids(transaction_ids) + fill_element(:jira_issue_transition_id_field, transaction_ids) + end + + def click_save_changes_button + click_element :save_changes_button + end + end + end + end + end + end +end diff --git a/qa/qa/resource/project.rb b/qa/qa/resource/project.rb index 78e2ba8a248..76da7ad7d9c 100644 --- a/qa/qa/resource/project.rb +++ b/qa/qa/resource/project.rb @@ -109,6 +109,10 @@ module QA "#{api_get_path}/runners" end + def api_repository_branches_path + "#{api_get_path}/repository/branches" + end + def api_pipelines_path "#{api_get_path}/pipelines" end @@ -180,6 +184,11 @@ module QA parse_body(response) end + def repository_branches + response = get Runtime::API::Request.new(api_client, api_repository_branches_path).url + parse_body(response) + end + def pipelines parse_body(get(Runtime::API::Request.new(api_client, api_pipelines_path).url)) end diff --git a/qa/qa/runtime/env.rb b/qa/qa/runtime/env.rb index 5c23586fb3e..677fba7ced7 100644 --- a/qa/qa/runtime/env.rb +++ b/qa/qa/runtime/env.rb @@ -210,6 +210,18 @@ module QA ENV['GITLAB_QA_1P_GITHUB_UUID'] end + def jira_admin_username + ENV['JIRA_ADMIN_USERNAME'] + end + + def jira_admin_password + ENV['JIRA_ADMIN_PASSWORD'] + end + + def jira_hostname + ENV['JIRA_HOSTNAME'] + end + def knapsack? !!(ENV['KNAPSACK_GENERATE_REPORT'] || ENV['KNAPSACK_REPORT_PATH'] || ENV['KNAPSACK_TEST_FILE_PATTERN']) end diff --git a/qa/qa/specs/features/browser_ui/3_create/jira/jira_basic_integration_spec.rb b/qa/qa/specs/features/browser_ui/3_create/jira/jira_basic_integration_spec.rb new file mode 100644 index 00000000000..05a932fd53e --- /dev/null +++ b/qa/qa/specs/features/browser_ui/3_create/jira/jira_basic_integration_spec.rb @@ -0,0 +1,105 @@ +# frozen_string_literal: true + +module QA + context 'Create' do + include Support::Api + + describe 'Jira integration', :jira, :orchestrated, :requires_admin do + let(:jira_project_key) { 'JITP' } + + before(:all) do + page.visit Vendor::Jira::JiraAPI.perform(&:base_url) + + QA::Support::Retrier.retry_until(sleep_interval: 3, reload_page: page, max_attempts: 20, raise_on_failure: true) do + page.has_text? 'Welcome to Jira' + end + + @project = Resource::Project.fabricate_via_api! do |project| + project.name = "project_with_jira_integration" + end + + # Retry is required because allow_local_requests_from_web_hooks_and_services + # takes some time to get enabled. + # Bug issue: https://gitlab.com/gitlab-org/gitlab/-/issues/217010 + QA::Support::Retrier.retry_on_exception(max_attempts: 5, sleep_interval: 3) do + Runtime::ApplicationSettings.set_application_settings(allow_local_requests_from_web_hooks_and_services: true) + + page.visit Runtime::Scenario.gitlab_address + Flow::Login.sign_in_unless_signed_in + + @project.visit! + + Page::Project::Menu.perform(&:go_to_integrations_settings) + QA::Page::Project::Settings::Integrations.perform(&:click_jira_link) + + QA::Page::Project::Settings::Services::Jira.perform do |jira| + jira.setup_service_with(url: Vendor::Jira::JiraAPI.perform(&:base_url)) + end + + expect(page).not_to have_text("Requests to the local network are not allowed") + end + end + + it 'closes an issue via pushing a commit' do + issue_key = Vendor::Jira::JiraAPI.perform do |jira_api| + jira_api.create_issue(jira_project_key) + end + + push_commit("Closes #{issue_key}") + + expect_issue_done(issue_key) + end + + it 'closes an issue via a merge request' do + issue_key = Vendor::Jira::JiraAPI.perform do |jira_api| + jira_api.create_issue(jira_project_key) + end + + page.visit Runtime::Scenario.gitlab_address + Flow::Login.sign_in_unless_signed_in + + merge_request = create_mr_with_description("Closes #{issue_key}") + + merge_request.visit! + + Page::MergeRequest::Show.perform(&:merge!) + + expect_issue_done(issue_key) + end + + def create_mr_with_description(description) + Resource::MergeRequest.fabricate! do |merge_request| + merge_request.project = @project + merge_request.target_new_branch = !master_branch_exists? + merge_request.description = description + end + end + + def push_commit(commit_message) + Resource::Repository::ProjectPush.fabricate! do |push| + push.branch_name = 'master' + push.commit_message = commit_message + push.file_content = commit_message + push.project = @project + push.new_branch = !master_branch_exists? + end + end + + def expect_issue_done(issue_key) + expect do + Support::Waiter.wait_until(raise_on_failure: true) do + jira_issue = Vendor::Jira::JiraAPI.perform do |jira_api| + jira_api.fetch_issue(issue_key) + end + + jira_issue[:fields][:status][:name] == 'Done' + end + end.not_to raise_error + end + + def master_branch_exists? + @project.repository_branches.map { |item| item[:name] }.include?("master") + end + end + end +end diff --git a/qa/qa/support/api.rb b/qa/qa/support/api.rb index f5e4d4e294b..3c46c039eae 100644 --- a/qa/qa/support/api.rb +++ b/qa/qa/support/api.rb @@ -11,22 +11,31 @@ module QA HTTP_STATUS_ACCEPTED = 202 HTTP_STATUS_SERVER_ERROR = 500 - def post(url, payload) - RestClient::Request.execute( + def post(url, payload, args = {}) + default_args = { method: :post, url: url, payload: payload, - verify_ssl: false) + verify_ssl: false + } + + RestClient::Request.execute( + default_args.merge(args) + ) rescue RestClient::ExceptionWithResponse => e return_response_or_raise(e) end - def get(url, raw_response: false) - RestClient::Request.execute( + def get(url, args = {}) + default_args = { method: :get, url: url, - verify_ssl: false, - raw_response: raw_response) + verify_ssl: false + } + + RestClient::Request.execute( + default_args.merge(args) + ) rescue RestClient::ExceptionWithResponse => e return_response_or_raise(e) end diff --git a/qa/qa/vendor/jira/jira_api.rb b/qa/qa/vendor/jira/jira_api.rb new file mode 100644 index 00000000000..65b080df3d0 --- /dev/null +++ b/qa/qa/vendor/jira/jira_api.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +module QA + module Vendor + module Jira + class JiraAPI + include Scenario::Actable + include Support::Api + + def base_url + host = QA::Runtime::Env.jira_hostname || 'localhost' + + "http://#{host}:8080" + end + + def api_url + "#{base_url}/rest/api/2" + end + + def fetch_issue(issue_key) + response = get("#{api_url}/issue/#{issue_key}", user: Runtime::Env.jira_admin_username, password: Runtime::Env.jira_admin_password) + + parse_body(response) + end + + def create_issue(jira_project_key) + payload = { + fields: { + project: { + key: jira_project_key + }, + summary: 'REST ye merry gentlemen.', + description: 'Creating of an issue using project keys and issue type names using the REST API', + issuetype: { + name: 'Bug' + } + } + } + + response = post("#{api_url}/issue", + payload.to_json, headers: { 'Content-Type': 'application/json' }, + user: Runtime::Env.jira_admin_username, + password: Runtime::Env.jira_admin_password) + + issue_key = parse_body(response)[:key] + + QA::Runtime::Logger.debug("Created JIRA issue with key: '#{issue_key}'") + + issue_key + end + end + end + end +end diff --git a/spec/features/issues/user_views_issue_spec.rb b/spec/features/issues/user_views_issue_spec.rb index dd04ac94105..5b8effd9406 100644 --- a/spec/features/issues/user_views_issue_spec.rb +++ b/spec/features/issues/user_views_issue_spec.rb @@ -35,18 +35,38 @@ describe "User views issue" do describe 'user status' do subject { visit(project_issue_path(project, issue)) } - describe 'showing status of the author of the issue' do + context 'when showing status of the author of the issue' do it_behaves_like 'showing user status' do let(:user_with_status) { issue.author } end end - describe 'showing status of a user who commented on an issue', :js do + context 'when showing status of a user who commented on an issue', :js do let!(:note) { create(:note, noteable: issue, project: project, author: user_with_status) } it_behaves_like 'showing user status' do let(:user_with_status) { create(:user) } end end + + context 'when status message has an emoji', :js do + let(:message) { 'My status with an emoji' } + let(:message_emoji) { 'basketball' } + + let!(:note) { create(:note, noteable: issue, project: project, author: user) } + let!(:status) { create(:user_status, user: user, emoji: 'smirk', message: "#{message} :#{message_emoji}:") } + + it 'correctly renders the emoji' do + tooltip_span = page.first(".user-status-emoji[title^='#{message}']") + + tooltip_span.hover + + tooltip = page.find('.tooltip .tooltip-inner') + + page.within(tooltip) do + expect(page).to have_emoji(message_emoji) + end + end + end end end diff --git a/spec/frontend/monitoring/components/variables_section_spec.js b/spec/frontend/monitoring/components/variables_section_spec.js index 095d89c9231..8f3748d3ff9 100644 --- a/spec/frontend/monitoring/components/variables_section_spec.js +++ b/spec/frontend/monitoring/components/variables_section_spec.js @@ -67,7 +67,7 @@ describe('Metrics dashboard/variables section component', () => { namespaced: true, state: { showEmptyState: false, - promVariables: sampleVariables, + variables: sampleVariables, }, actions: { fetchDashboardData, diff --git a/spec/frontend/monitoring/store/getters_spec.js b/spec/frontend/monitoring/store/getters_spec.js index 365052e68e3..9f17dda3b9f 100644 --- a/spec/frontend/monitoring/store/getters_spec.js +++ b/spec/frontend/monitoring/store/getters_spec.js @@ -334,11 +334,11 @@ describe('Monitoring store Getters', () => { beforeEach(() => { state = { - promVariables: {}, + variables: {}, }; }); - it('transforms the promVariables object to an array in the [variable, variable_value] format for all variable types', () => { + it('transforms the variables object to an array in the [variable, variable_value] format for all variable types', () => { mutations[types.SET_VARIABLES](state, mockTemplatingDataResponses.allVariableTypes); const variablesArray = getters.getCustomVariablesArray(state); @@ -354,7 +354,7 @@ describe('Monitoring store Getters', () => { ]); }); - it('transforms the promVariables object to an empty array when no keys are present', () => { + it('transforms the variables object to an empty array when no keys are present', () => { mutations[types.SET_VARIABLES](state, {}); const variablesArray = getters.getCustomVariablesArray(state); diff --git a/spec/frontend/monitoring/store/mutations_spec.js b/spec/frontend/monitoring/store/mutations_spec.js index 4306243689a..7bd1b3f5c5b 100644 --- a/spec/frontend/monitoring/store/mutations_spec.js +++ b/spec/frontend/monitoring/store/mutations_spec.js @@ -412,13 +412,13 @@ describe('Monitoring mutations', () => { it('stores an empty variables array when no custom variables are given', () => { mutations[types.SET_VARIABLES](stateCopy, {}); - expect(stateCopy.promVariables).toEqual({}); + expect(stateCopy.variables).toEqual({}); }); it('stores variables in the key key_value format in the array', () => { mutations[types.SET_VARIABLES](stateCopy, { pod: 'POD', stage: 'main ops' }); - expect(stateCopy.promVariables).toEqual({ pod: 'POD', stage: 'main ops' }); + expect(stateCopy.variables).toEqual({ pod: 'POD', stage: 'main ops' }); }); }); @@ -427,11 +427,11 @@ describe('Monitoring mutations', () => { mutations[types.SET_VARIABLES](stateCopy, {}); }); - it('updates only the value of the variable in promVariables', () => { + it('updates only the value of the variable in variables', () => { mutations[types.SET_VARIABLES](stateCopy, { environment: { value: 'prod', type: 'text' } }); mutations[types.UPDATE_VARIABLE_VALUES](stateCopy, { key: 'environment', value: 'new prod' }); - expect(stateCopy.promVariables).toEqual({ environment: { value: 'new prod', type: 'text' } }); + expect(stateCopy.variables).toEqual({ environment: { value: 'new prod', type: 'text' } }); }); }); }); diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index 203d39be22b..2545ea2e0ea 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -1539,12 +1539,17 @@ describe Gitlab::Database::MigrationHelpers do end describe '#create_or_update_plan_limit' do - class self::Plan < ActiveRecord::Base - self.table_name = 'plans' - end + before do + stub_const('Plan', Class.new(ActiveRecord::Base)) + stub_const('PlanLimits', Class.new(ActiveRecord::Base)) - class self::PlanLimits < ActiveRecord::Base - self.table_name = 'plan_limits' + Plan.class_eval do + self.table_name = 'plans' + end + + PlanLimits.class_eval do + self.table_name = 'plan_limits' + end end it 'properly escapes names' do @@ -1560,28 +1565,28 @@ describe Gitlab::Database::MigrationHelpers do context 'when plan does not exist' do it 'does not create any plan limits' do expect { model.create_or_update_plan_limit('project_hooks', 'plan_name', 10) } - .not_to change { self.class::PlanLimits.count } + .not_to change { PlanLimits.count } end end context 'when plan does exist' do - let!(:plan) { self.class::Plan.create!(name: 'plan_name') } + let!(:plan) { Plan.create!(name: 'plan_name') } context 'when limit does not exist' do it 'inserts a new plan limits' do expect { model.create_or_update_plan_limit('project_hooks', 'plan_name', 10) } - .to change { self.class::PlanLimits.count }.by(1) + .to change { PlanLimits.count }.by(1) - expect(self.class::PlanLimits.pluck(:project_hooks)).to contain_exactly(10) + expect(PlanLimits.pluck(:project_hooks)).to contain_exactly(10) end end context 'when limit does exist' do - let!(:plan_limit) { self.class::PlanLimits.create!(plan_id: plan.id) } + let!(:plan_limit) { PlanLimits.create!(plan_id: plan.id) } it 'updates an existing plan limits' do expect { model.create_or_update_plan_limit('project_hooks', 'plan_name', 999) } - .not_to change { self.class::PlanLimits.count } + .not_to change { PlanLimits.count } expect(plan_limit.reload.project_hooks).to eq(999) end @@ -1605,19 +1610,23 @@ describe Gitlab::Database::MigrationHelpers do describe '#backfill_iids' do include MigrationsHelpers - class self::Issue < ActiveRecord::Base - include AtomicInternalId + before do + stub_const('Issue', Class.new(ActiveRecord::Base)) + + Issue.class_eval do + include AtomicInternalId - self.table_name = 'issues' - self.inheritance_column = :_type_disabled + self.table_name = 'issues' + self.inheritance_column = :_type_disabled - belongs_to :project, class_name: "::Project" + belongs_to :project, class_name: "::Project" - has_internal_id :iid, - scope: :project, - init: ->(s) { s&.project&.issues&.maximum(:iid) }, - backfill: true, - presence: false + has_internal_id :iid, + scope: :project, + init: ->(s) { s&.project&.issues&.maximum(:iid) }, + backfill: true, + presence: false + end end let(:namespaces) { table(:namespaces) } @@ -1636,7 +1645,7 @@ describe Gitlab::Database::MigrationHelpers do model.backfill_iids('issues') - issue = self.class::Issue.create!(project_id: project.id) + issue = Issue.create!(project_id: project.id) expect(issue.iid).to eq(1) end @@ -1647,7 +1656,7 @@ describe Gitlab::Database::MigrationHelpers do model.backfill_iids('issues') - issue_b = self.class::Issue.create!(project_id: project.id) + issue_b = Issue.create!(project_id: project.id) expect(issue_a.reload.iid).to eq(1) expect(issue_b.iid).to eq(2) @@ -1662,8 +1671,8 @@ describe Gitlab::Database::MigrationHelpers do model.backfill_iids('issues') - issue_a = self.class::Issue.create!(project_id: project_a.id) - issue_b = self.class::Issue.create!(project_id: project_b.id) + issue_a = Issue.create!(project_id: project_a.id) + issue_b = Issue.create!(project_id: project_b.id) expect(issue_a.iid).to eq(2) expect(issue_b.iid).to eq(3) @@ -1672,7 +1681,7 @@ describe Gitlab::Database::MigrationHelpers do context 'when the new code creates a row post deploy but before the migration runs' do it 'does not change the row iid' do project = setup - issue = self.class::Issue.create!(project_id: project.id) + issue = Issue.create!(project_id: project.id) model.backfill_iids('issues') @@ -1683,7 +1692,7 @@ describe Gitlab::Database::MigrationHelpers do project = setup issue_a = issues.create!(project_id: project.id) issue_b = issues.create!(project_id: project.id) - issue_c = self.class::Issue.create!(project_id: project.id) + issue_c = Issue.create!(project_id: project.id) model.backfill_iids('issues') @@ -1697,8 +1706,8 @@ describe Gitlab::Database::MigrationHelpers do project_b = setup issue_a = issues.create!(project_id: project_a.id) issue_b = issues.create!(project_id: project_b.id) - issue_c = self.class::Issue.create!(project_id: project_a.id) - issue_d = self.class::Issue.create!(project_id: project_b.id) + issue_c = Issue.create!(project_id: project_a.id) + issue_d = Issue.create!(project_id: project_b.id) model.backfill_iids('issues') @@ -1712,12 +1721,12 @@ describe Gitlab::Database::MigrationHelpers do project = setup issue_a = issues.create!(project_id: project.id) issue_b = issues.create!(project_id: project.id) - issue_c = self.class::Issue.create!(project_id: project.id) + issue_c = Issue.create!(project_id: project.id) model.backfill_iids('issues') - issue_d = self.class::Issue.create!(project_id: project.id) - issue_e = self.class::Issue.create!(project_id: project.id) + issue_d = Issue.create!(project_id: project.id) + issue_e = Issue.create!(project_id: project.id) expect(issue_a.reload.iid).to eq(1) expect(issue_b.reload.iid).to eq(2) @@ -1731,14 +1740,14 @@ describe Gitlab::Database::MigrationHelpers do project_b = setup issue_a = issues.create!(project_id: project_a.id) issue_b = issues.create!(project_id: project_b.id) - issue_c = self.class::Issue.create!(project_id: project_a.id) - issue_d = self.class::Issue.create!(project_id: project_b.id) + issue_c = Issue.create!(project_id: project_a.id) + issue_d = Issue.create!(project_id: project_b.id) model.backfill_iids('issues') - issue_e = self.class::Issue.create!(project_id: project_a.id) - issue_f = self.class::Issue.create!(project_id: project_b.id) - issue_g = self.class::Issue.create!(project_id: project_a.id) + issue_e = Issue.create!(project_id: project_a.id) + issue_f = Issue.create!(project_id: project_b.id) + issue_g = Issue.create!(project_id: project_a.id) expect(issue_a.reload.iid).to eq(1) expect(issue_b.reload.iid).to eq(1) @@ -1754,7 +1763,7 @@ describe Gitlab::Database::MigrationHelpers do it 'backfills iids' do project = setup issue_a = issues.create!(project_id: project.id) - issue_b = self.class::Issue.create!(project_id: project.id) + issue_b = Issue.create!(project_id: project.id) issue_c = issues.create!(project_id: project.id) model.backfill_iids('issues') @@ -1768,12 +1777,12 @@ describe Gitlab::Database::MigrationHelpers do project = setup issue_a = issues.create!(project_id: project.id) issue_b = issues.create!(project_id: project.id) - issue_c = self.class::Issue.create!(project_id: project.id) + issue_c = Issue.create!(project_id: project.id) issue_d = issues.create!(project_id: project.id) model.backfill_iids('issues') - issue_e = self.class::Issue.create!(project_id: project.id) + issue_e = Issue.create!(project_id: project.id) expect(issue_a.reload.iid).to eq(1) expect(issue_b.reload.iid).to eq(2) @@ -1787,9 +1796,9 @@ describe Gitlab::Database::MigrationHelpers do it 'backfills iids' do project = setup issue_a = issues.create!(project_id: project.id) - issue_b = self.class::Issue.create!(project_id: project.id) + issue_b = Issue.create!(project_id: project.id) issue_c = issues.create!(project_id: project.id) - issue_d = self.class::Issue.create!(project_id: project.id) + issue_d = Issue.create!(project_id: project.id) model.backfill_iids('issues') @@ -1803,13 +1812,13 @@ describe Gitlab::Database::MigrationHelpers do project = setup issue_a = issues.create!(project_id: project.id) issue_b = issues.create!(project_id: project.id) - issue_c = self.class::Issue.create!(project_id: project.id) + issue_c = Issue.create!(project_id: project.id) issue_d = issues.create!(project_id: project.id) - issue_e = self.class::Issue.create!(project_id: project.id) + issue_e = Issue.create!(project_id: project.id) model.backfill_iids('issues') - issue_f = self.class::Issue.create!(project_id: project.id) + issue_f = Issue.create!(project_id: project.id) expect(issue_a.reload.iid).to eq(1) expect(issue_b.reload.iid).to eq(2) @@ -1825,7 +1834,7 @@ describe Gitlab::Database::MigrationHelpers do project = setup issue_a = issues.create!(project_id: project.id) issue_b = issues.create!(project_id: project.id) - issue_c = self.class::Issue.create!(project_id: project.id) + issue_c = Issue.create!(project_id: project.id) issue_c.delete model.backfill_iids('issues') @@ -1838,12 +1847,12 @@ describe Gitlab::Database::MigrationHelpers do project = setup issue_a = issues.create!(project_id: project.id) issue_b = issues.create!(project_id: project.id) - issue_c = self.class::Issue.create!(project_id: project.id) + issue_c = Issue.create!(project_id: project.id) issue_c.delete model.backfill_iids('issues') - issue_d = self.class::Issue.create!(project_id: project.id) + issue_d = Issue.create!(project_id: project.id) expect(issue_a.reload.iid).to eq(1) expect(issue_b.reload.iid).to eq(2) @@ -1856,7 +1865,7 @@ describe Gitlab::Database::MigrationHelpers do project = setup issue_a = issues.create!(project_id: project.id) issue_b = issues.create!(project_id: project.id) - issue_c = self.class::Issue.create!(project_id: project.id) + issue_c = Issue.create!(project_id: project.id) issue_c.delete issue_d = issues.create!(project_id: project.id) @@ -1871,13 +1880,13 @@ describe Gitlab::Database::MigrationHelpers do project = setup issue_a = issues.create!(project_id: project.id) issue_b = issues.create!(project_id: project.id) - issue_c = self.class::Issue.create!(project_id: project.id) + issue_c = Issue.create!(project_id: project.id) issue_c.delete issue_d = issues.create!(project_id: project.id) model.backfill_iids('issues') - issue_e = self.class::Issue.create!(project_id: project.id) + issue_e = Issue.create!(project_id: project.id) expect(issue_a.reload.iid).to eq(1) expect(issue_b.reload.iid).to eq(2) @@ -1891,9 +1900,9 @@ describe Gitlab::Database::MigrationHelpers do project = setup issue_a = issues.create!(project_id: project.id) issue_b = issues.create!(project_id: project.id) - issue_c = self.class::Issue.create!(project_id: project.id) + issue_c = Issue.create!(project_id: project.id) issue_c.delete - issue_d = self.class::Issue.create!(project_id: project.id) + issue_d = Issue.create!(project_id: project.id) model.backfill_iids('issues') @@ -1906,13 +1915,13 @@ describe Gitlab::Database::MigrationHelpers do project = setup issue_a = issues.create!(project_id: project.id) issue_b = issues.create!(project_id: project.id) - issue_c = self.class::Issue.create!(project_id: project.id) + issue_c = Issue.create!(project_id: project.id) issue_c.delete - issue_d = self.class::Issue.create!(project_id: project.id) + issue_d = Issue.create!(project_id: project.id) model.backfill_iids('issues') - issue_e = self.class::Issue.create!(project_id: project.id) + issue_e = Issue.create!(project_id: project.id) expect(issue_a.reload.iid).to eq(1) expect(issue_b.reload.iid).to eq(2) @@ -1929,7 +1938,7 @@ describe Gitlab::Database::MigrationHelpers do model.backfill_iids('issues') - issue_b = self.class::Issue.create!(project_id: project_b.id) + issue_b = Issue.create!(project_id: project_b.id) expect(issue_a.reload.iid).to eq(1) expect(issue_b.reload.iid).to eq(1) diff --git a/spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb b/spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb index 0f68201a153..dee1d7df1a9 100644 --- a/spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb +++ b/spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb @@ -3,39 +3,48 @@ require 'spec_helper' describe Gitlab::Database::ObsoleteIgnoredColumns do - module Testing + before do + stub_const('Testing', Module.new) + stub_const('Testing::MyBase', Class.new(ActiveRecord::Base)) + stub_const('SomeAbstract', Class.new(Testing::MyBase)) + stub_const('Testing::B', Class.new(Testing::MyBase)) + stub_const('Testing::A', Class.new(SomeAbstract)) + stub_const('Testing::C', Class.new(Testing::MyBase)) + # Used a fixed date to prevent tests failing across date boundaries - REMOVE_DATE = Date.new(2019, 12, 16) + stub_const('REMOVE_DATE', Date.new(2019, 12, 16)) - class MyBase < ApplicationRecord - end + Testing.module_eval do + Testing::MyBase.class_eval do + end - class SomeAbstract < MyBase - include IgnorableColumns + SomeAbstract.class_eval do + include IgnorableColumns - self.abstract_class = true + self.abstract_class = true - self.table_name = 'projects' + self.table_name = 'projects' - ignore_column :unused, remove_after: '2019-01-01', remove_with: '12.0' - end + ignore_column :unused, remove_after: '2019-01-01', remove_with: '12.0' + end - class B < MyBase - include IgnorableColumns + Testing::B.class_eval do + include IgnorableColumns - self.table_name = 'issues' + self.table_name = 'issues' - ignore_column :id, :other, remove_after: '2019-01-01', remove_with: '12.0' - ignore_column :not_used_but_still_ignored, remove_after: REMOVE_DATE.to_s, remove_with: '12.1' - end + ignore_column :id, :other, remove_after: '2019-01-01', remove_with: '12.0' + ignore_column :not_used_but_still_ignored, remove_after: REMOVE_DATE.to_s, remove_with: '12.1' + end - class A < SomeAbstract - ignore_column :also_unused, remove_after: '2019-02-01', remove_with: '12.1' - ignore_column :not_used_but_still_ignored, remove_after: REMOVE_DATE.to_s, remove_with: '12.1' - end + Testing::A.class_eval do + ignore_column :also_unused, remove_after: '2019-02-01', remove_with: '12.1' + ignore_column :not_used_but_still_ignored, remove_after: REMOVE_DATE.to_s, remove_with: '12.1' + end - class C < MyBase - self.table_name = 'users' + Testing::C.class_eval do + self.table_name = 'users' + end end end @@ -43,7 +52,7 @@ describe Gitlab::Database::ObsoleteIgnoredColumns do describe '#execute' do it 'returns a list of class names and columns pairs' do - Timecop.freeze(Testing::REMOVE_DATE) do + Timecop.freeze(REMOVE_DATE) do expect(subject.execute).to eq([ ['Testing::A', { 'unused' => IgnorableColumns::ColumnIgnore.new(Date.parse('2019-01-01'), '12.0'), diff --git a/spec/lib/gitlab/database/with_lock_retries_spec.rb b/spec/lib/gitlab/database/with_lock_retries_spec.rb index 9c8c9749125..d7eee594631 100644 --- a/spec/lib/gitlab/database/with_lock_retries_spec.rb +++ b/spec/lib/gitlab/database/with_lock_retries_spec.rb @@ -35,9 +35,6 @@ describe Gitlab::Database::WithLockRetries do end context 'when lock retry is enabled' do - class ActiveRecordSecond < ActiveRecord::Base - end - let(:lock_fiber) do Fiber.new do # Initiating a second DB connection for the lock @@ -52,6 +49,8 @@ describe Gitlab::Database::WithLockRetries do end before do + stub_const('ActiveRecordSecond', Class.new(ActiveRecord::Base)) + lock_fiber.resume # start the transaction and lock the table end diff --git a/spec/lib/gitlab/import_export/project/relation_factory_spec.rb b/spec/lib/gitlab/import_export/project/relation_factory_spec.rb index 175da623c1b..3339129cb8f 100644 --- a/spec/lib/gitlab/import_export/project/relation_factory_spec.rb +++ b/spec/lib/gitlab/import_export/project/relation_factory_spec.rb @@ -18,6 +18,22 @@ describe Gitlab::ImportExport::Project::RelationFactory do excluded_keys: excluded_keys) end + before do + # Mocks an ActiveRecordish object with the dodgy columns + stub_const('FooModel', Class.new) + FooModel.class_eval do + include ActiveModel::Model + + def initialize(params = {}) + params.each { |key, value| send("#{key}=", value) } + end + + def values + instance_variables.map { |ivar| instance_variable_get(ivar) } + end + end + end + context 'hook object' do let(:relation_sym) { :hooks } let(:id) { 999 } @@ -83,19 +99,6 @@ describe Gitlab::ImportExport::Project::RelationFactory do end end - # Mocks an ActiveRecordish object with the dodgy columns - class FooModel - include ActiveModel::Model - - def initialize(params = {}) - params.each { |key, value| send("#{key}=", value) } - end - - def values - instance_variables.map { |ivar| instance_variable_get(ivar) } - end - end - context 'merge_request object' do let(:relation_sym) { :merge_requests } @@ -208,11 +211,12 @@ describe Gitlab::ImportExport::Project::RelationFactory do } end - class HazardousFooModel < FooModel - attr_accessor :service_id, :moved_to_id, :namespace_id, :ci_id, :random_project_id, :random_id, :milestone_id, :project_id - end - before do + stub_const('HazardousFooModel', Class.new(FooModel)) + HazardousFooModel.class_eval do + attr_accessor :service_id, :moved_to_id, :namespace_id, :ci_id, :random_project_id, :random_id, :milestone_id, :project_id + end + allow(HazardousFooModel).to receive(:reflect_on_association).and_return(nil) end @@ -246,11 +250,12 @@ describe Gitlab::ImportExport::Project::RelationFactory do Gitlab::ImportExport::Project::RelationFactory::PROJECT_REFERENCES.map { |ref| { ref => 99 } }.inject(:merge) end - class ProjectFooModel < FooModel - attr_accessor(*Gitlab::ImportExport::Project::RelationFactory::PROJECT_REFERENCES) - end - before do + stub_const('ProjectFooModel', Class.new(FooModel)) + ProjectFooModel.class_eval do + attr_accessor(*Gitlab::ImportExport::Project::RelationFactory::PROJECT_REFERENCES) + end + allow(ProjectFooModel).to receive(:reflect_on_association).and_return(nil) end diff --git a/spec/models/concerns/cacheable_attributes_spec.rb b/spec/models/concerns/cacheable_attributes_spec.rb index 56e0d044247..6694b2aba22 100644 --- a/spec/models/concerns/cacheable_attributes_spec.rb +++ b/spec/models/concerns/cacheable_attributes_spec.rb @@ -135,7 +135,7 @@ describe CacheableAttributes do end it 'returns an uncached record and logs a warning' do - expect(Rails.logger).to receive(:warn).with("Cached record for TestClass couldn't be loaded, falling back to uncached record: Redis::BaseError") + expect(Gitlab::AppLogger).to receive(:warn).with("Cached record for TestClass couldn't be loaded, falling back to uncached record: Redis::BaseError") expect(MinimalTestClass.current).to eq(:last) end @@ -147,7 +147,7 @@ describe CacheableAttributes do end it 'returns an uncached record and logs a warning' do - expect(Rails.logger).not_to receive(:warn) + expect(Gitlab::AppLogger).not_to receive(:warn) expect { MinimalTestClass.current }.to raise_error(Redis::BaseError) end diff --git a/spec/services/merge_requests/ff_merge_service_spec.rb b/spec/services/merge_requests/ff_merge_service_spec.rb index 87fcd70a298..efe1860d4e5 100644 --- a/spec/services/merge_requests/ff_merge_service_spec.rb +++ b/spec/services/merge_requests/ff_merge_service_spec.rb @@ -87,7 +87,7 @@ describe MergeRequests::FfMergeService do let(:service) { described_class.new(project, user, valid_merge_params.merge(commit_message: 'Awesome message')) } before do - allow(Rails.logger).to receive(:error) + allow(Gitlab::AppLogger).to receive(:error) end it 'logs and saves error if there is an exception' do @@ -99,7 +99,7 @@ describe MergeRequests::FfMergeService do service.execute(merge_request) expect(merge_request.merge_error).to include(error_message) - expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message)) + expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message)) end it 'logs and saves error if there is an PreReceiveError exception' do @@ -111,7 +111,7 @@ describe MergeRequests::FfMergeService do service.execute(merge_request) expect(merge_request.merge_error).to include(error_message) - expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message)) + expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message)) end it 'does not update squash_commit_sha if squash merge is not successful' do diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index bcad822b1dc..b98fc9785ff 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -277,7 +277,7 @@ describe MergeRequests::MergeService do context 'error handling' do before do - allow(Rails.logger).to receive(:error) + allow(Gitlab::AppLogger).to receive(:error) end context 'when source is missing' do @@ -289,7 +289,7 @@ describe MergeRequests::MergeService do service.execute(merge_request) expect(merge_request.merge_error).to eq(error_message) - expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message)) + expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message)) end end @@ -302,7 +302,7 @@ describe MergeRequests::MergeService do service.execute(merge_request) expect(merge_request.merge_error).to include('Something went wrong during merge') - expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message)) + expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message)) end it 'logs and saves error if user is not authorized' do @@ -326,7 +326,7 @@ describe MergeRequests::MergeService do service.execute(merge_request) expect(merge_request.merge_error).to include('Something went wrong during merge pre-receive hook') - expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message)) + expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message)) end it 'logs and saves error if there is a merge conflict' do @@ -340,7 +340,7 @@ describe MergeRequests::MergeService do expect(merge_request).to be_open expect(merge_request.merge_commit_sha).to be_nil expect(merge_request.merge_error).to include(error_message) - expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message)) + expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message)) end context 'when squashing' do @@ -359,7 +359,7 @@ describe MergeRequests::MergeService do expect(merge_request).to be_open expect(merge_request.merge_commit_sha).to be_nil expect(merge_request.merge_error).to include(error_message) - expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message)) + expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message)) end it 'logs and saves error if there is a squash in progress' do @@ -373,7 +373,7 @@ describe MergeRequests::MergeService do expect(merge_request).to be_open expect(merge_request.merge_commit_sha).to be_nil expect(merge_request.merge_error).to include(error_message) - expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message)) + expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message)) end context 'when fast-forward merge is not allowed' do @@ -393,7 +393,7 @@ describe MergeRequests::MergeService do expect(merge_request).to be_open expect(merge_request.merge_commit_sha).to be_nil expect(merge_request.merge_error).to include(error_message) - expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message)) + expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message)) end end end diff --git a/spec/services/projects/import_export/export_service_spec.rb b/spec/services/projects/import_export/export_service_spec.rb index 5f496cb1e56..f7e8171f1cf 100644 --- a/spec/services/projects/import_export/export_service_spec.rb +++ b/spec/services/projects/import_export/export_service_spec.rb @@ -119,9 +119,9 @@ describe Projects::ImportExport::ExportService do end it 'notifies logger' do - allow(Rails.logger).to receive(:error) + allow(Gitlab::AppLogger).to receive(:error) - expect(Rails.logger).to receive(:error) + expect(Gitlab::AppLogger).to receive(:error) end end end @@ -149,7 +149,7 @@ describe Projects::ImportExport::ExportService do end it 'notifies logger' do - expect(Rails.logger).to receive(:error) + expect(Gitlab::AppLogger).to receive(:error) end it 'does not call the export strategy' do diff --git a/spec/services/spam/akismet_service_spec.rb b/spec/services/spam/akismet_service_spec.rb index a496cd1890e..413b43d0156 100644 --- a/spec/services/spam/akismet_service_spec.rb +++ b/spec/services/spam/akismet_service_spec.rb @@ -45,9 +45,7 @@ describe Spam::AkismetService do end it 'logs an error' do - logger_spy = double(:logger) - expect(Rails).to receive(:logger).and_return(logger_spy) - expect(logger_spy).to receive(:error).with(/skipping/) + expect(Gitlab::AppLogger).to receive(:error).with(/skipping/) subject.send(method_call) end @@ -98,9 +96,7 @@ describe Spam::AkismetService do end it 'logs an error' do - logger_spy = double(:logger) - expect(Rails).to receive(:logger).and_return(logger_spy) - expect(logger_spy).to receive(:error).with(/skipping check/) + expect(Gitlab::AppLogger).to receive(:error).with(/skipping check/) subject.spam? end diff --git a/spec/workers/new_note_worker_spec.rb b/spec/workers/new_note_worker_spec.rb index 387a0958d6b..cf350fbcf2a 100644 --- a/spec/workers/new_note_worker_spec.rb +++ b/spec/workers/new_note_worker_spec.rb @@ -27,7 +27,7 @@ describe NewNoteWorker do let(:unexistent_note_id) { non_existing_record_id } it 'logs NewNoteWorker process skipping' do - expect(Rails.logger).to receive(:error) + expect(Gitlab::AppLogger).to receive(:error) .with("NewNoteWorker: couldn't find note with ID=#{unexistent_note_id}, skipping job") described_class.new.perform(unexistent_note_id) |