From f35a7a3b8e97d7af2ec1505d3fbcd6ffdd869fd2 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 7 May 2020 15:09:29 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .../components/alert_management_list.vue | 34 +- .../javascripts/alert_management/constants.js | 32 ++ .../code_navigation/store/actions.js | 4 +- .../design_notes/design_discussion.vue | 8 +- .../components/design_notes/design_note.vue | 120 +++++-- .../design_notes/design_reply_form.vue | 44 ++- .../mutations/update_note.mutation.graphql | 10 + .../design_management/pages/design/index.vue | 7 +- .../design_management/utils/cache_update.js | 2 +- .../design_management/utils/error_messages.js | 2 + app/assets/javascripts/editor/editor_lite.js | 4 + app/assets/javascripts/ide/lib/editor.js | 3 + .../javascripts/ide/lib/languages/index.js | 5 + .../javascripts/ide/lib/languages/vue.js | 306 ++++++++++++++++++ app/assets/javascripts/ide/utils.js | 10 + .../monitoring/components/dashboard.vue | 43 ++- .../javascripts/monitoring/stores/actions.js | 7 + app/assets/javascripts/monitoring/utils.js | 66 +++- .../components/snippet_description_view.vue | 21 ++ .../snippets/components/snippet_title.vue | 14 +- .../components/mr_widget_pipeline.vue | 15 +- .../mr_widget_pipeline_container.vue | 1 + .../components/states/ready_to_merge.vue | 25 +- .../mixins/ready_to_merge.js | 6 + .../mr_widget_options.vue | 9 +- .../stores/mr_widget_store.js | 1 + .../components/blob_viewers/rich_viewer.vue | 6 +- .../components/markdown/field_view.vue | 19 ++ .../page_bundles/themes/_dark.scss | 2 +- app/assets/stylesheets/pages/alerts_list.scss | 10 + .../projects/alert_management_controller.rb | 3 + .../types/alert_management/status_enum.rb | 4 +- app/models/alert_management/alert.rb | 69 +++- app/models/merge_request_diff.rb | 26 -- app/models/namespace.rb | 4 + .../process_prometheus_alert_service.rb | 86 +++++ .../update_alert_status_service.rb | 4 +- app/services/concerns/git/logger.rb | 10 - app/services/merge_requests/base_service.rb | 26 ++ app/services/merge_requests/rebase_service.rb | 8 +- app/services/merge_requests/squash_service.rb | 18 +- .../prometheus/alerts/notify_service.rb | 11 + .../projects/merge_requests/_widget.html.haml | 1 + app/workers/all_queues.yml | 10 +- app/workers/concerns/chaos_queue.rb | 2 +- ...18609-design-comment-edit-comment-text.yml | 5 + .../197344-use-cookies-with-metadata.yml | 5 + .../unreleased/212279-webide-vue-files.yml | 5 + .../215472-single-chart-from-url.yml | 5 + ...when-pipelines-disabled-mr-must-succee.yml | 7 + ...ser_highest_roles_background_migration.yml | 5 + .../sh-fix-squash-error-handling.yml | 5 + config/feature_categories.yml | 1 - config/initializers/cookies_serializer.rb | 2 +- ...1_cleanup_user_highest_roles_population.rb | 23 ++ db/structure.sql | 3 +- doc/development/documentation/styleguide.md | 57 ++-- lib/gitlab/alert_management/alert_params.rb | 17 + lib/gitlab/alerting/alert.rb | 12 + lib/gitlab/metrics/samplers/ruby_sampler.rb | 29 +- lib/gitlab/metrics/system.rb | 77 +++-- locale/gitlab.pot | 41 ++- qa/qa/page/dashboard/snippet/show.rb | 4 +- spec/factories/alert_management/alerts.rb | 18 +- .../alert_management/alerts_finder_spec.rb | 12 +- .../components/alert_management_list_spec.js | 70 +++- .../code_navigation/store/actions_spec.js | 10 + .../__snapshots__/design_note_spec.js.snap | 85 ++--- .../design_reply_form_spec.js.snap | 15 + .../design_notes/design_note_spec.js | 106 ++++-- .../design_notes/design_reply_form_spec.js | 50 ++- .../editor/editor_lite_spec.js | 37 ++- spec/frontend/ide/lib/editor_spec.js | 14 +- spec/frontend/ide/lib/languages/vue_spec.js | 92 ++++++ spec/frontend/ide/utils_spec.js | 77 ++++- .../components/dashboard_panel_spec.js | 16 +- .../monitoring/components/dashboard_spec.js | 86 ++++- .../frontend/monitoring/store/actions_spec.js | 36 +++ spec/frontend/monitoring/utils_spec.js | 144 +++++++-- .../snippet_description_view_spec.js.snap | 16 + .../snippet_description_view_spec.js | 27 ++ .../snippets/components/snippet_title_spec.js | 4 +- .../blob_viewers/rich_viewer_spec.js | 5 + .../components/markdown/field_view_spec.js | 26 ++ .../update_alert_status_spec.rb | 2 +- .../alert_management_alert_resolver_spec.rb | 4 +- .../alert_management/status_enum_spec.rb | 17 +- .../components/mr_widget_pipeline_spec.js | 13 + .../states/mr_widget_ready_to_merge_spec.js | 1 + .../alert_management/alert_params_spec.rb | 39 +++ spec/lib/gitlab/alerting/alert_spec.rb | 24 ++ .../metrics/samplers/ruby_sampler_spec.rb | 20 +- spec/lib/gitlab/metrics/system_spec.rb | 113 ++++++- spec/models/alert_management/alert_spec.rb | 195 ++++++++--- .../project/alert_management/alerts_spec.rb | 15 +- .../process_prometheus_alert_service_spec.rb | 136 ++++++++ .../update_alert_status_service_spec.rb | 2 +- .../merge_requests/rebase_service_spec.rb | 19 +- .../merge_requests/squash_service_spec.rb | 40 ++- .../projects/alerting/notify_service_spec.rb | 2 +- .../prometheus/alerts/notify_service_spec.rb | 52 +++ 101 files changed, 2642 insertions(+), 419 deletions(-) create mode 100644 app/assets/javascripts/alert_management/constants.js create mode 100644 app/assets/javascripts/design_management/graphql/mutations/update_note.mutation.graphql create mode 100644 app/assets/javascripts/ide/lib/languages/index.js create mode 100644 app/assets/javascripts/ide/lib/languages/vue.js create mode 100644 app/assets/javascripts/snippets/components/snippet_description_view.vue create mode 100644 app/assets/javascripts/vue_shared/components/markdown/field_view.vue create mode 100644 app/services/alert_management/process_prometheus_alert_service.rb delete mode 100644 app/services/concerns/git/logger.rb create mode 100644 changelogs/unreleased/118609-design-comment-edit-comment-text.yml create mode 100644 changelogs/unreleased/197344-use-cookies-with-metadata.yml create mode 100644 changelogs/unreleased/212279-webide-vue-files.yml create mode 100644 changelogs/unreleased/215472-single-chart-from-url.yml create mode 100644 changelogs/unreleased/216031-show-accurate-error-message-when-pipelines-disabled-mr-must-succee.yml create mode 100644 changelogs/unreleased/cleanup_user_highest_roles_background_migration.yml create mode 100644 changelogs/unreleased/sh-fix-squash-error-handling.yml create mode 100644 db/post_migrate/20200506125731_cleanup_user_highest_roles_population.rb create mode 100644 spec/frontend/design_management/components/design_notes/__snapshots__/design_reply_form_spec.js.snap rename spec/{javascripts => frontend}/editor/editor_lite_spec.js (78%) create mode 100644 spec/frontend/ide/lib/languages/vue_spec.js create mode 100644 spec/frontend/snippets/components/__snapshots__/snippet_description_view_spec.js.snap create mode 100644 spec/frontend/snippets/components/snippet_description_view_spec.js create mode 100644 spec/frontend/vue_shared/components/markdown/field_view_spec.js create mode 100644 spec/services/alert_management/process_prometheus_alert_service_spec.rb diff --git a/app/assets/javascripts/alert_management/components/alert_management_list.vue b/app/assets/javascripts/alert_management/components/alert_management_list.vue index e29835da117..f1716182e5f 100644 --- a/app/assets/javascripts/alert_management/components/alert_management_list.vue +++ b/app/assets/javascripts/alert_management/components/alert_management_list.vue @@ -8,10 +8,15 @@ import { GlIcon, GlNewDropdown, GlNewDropdownItem, + GlTabs, + GlTab, + GlBadge, } from '@gitlab/ui'; import { s__ } from '~/locale'; import TimeAgo from '~/vue_shared/components/time_ago_tooltip.vue'; import getAlerts from '../graphql/queries/getAlerts.query.graphql'; +import { ALERTS_STATUS, ALERTS_STATUS_TABS } from '../constants'; +import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; const tdClass = 'table-col d-flex d-md-table-cell align-items-center'; @@ -59,10 +64,11 @@ export default { }, ], statuses: { - triggered: s__('AlertManagement|Triggered'), - acknowledged: s__('AlertManagement|Acknowledged'), - resolved: s__('AlertManagement|Resolved'), + [ALERTS_STATUS.TRIGGERED]: s__('AlertManagement|Triggered'), + [ALERTS_STATUS.ACKNOWLEDGED]: s__('AlertManagement|Acknowledged'), + [ALERTS_STATUS.RESOLVED]: s__('AlertManagement|Resolved'), }, + statusTabs: ALERTS_STATUS_TABS, components: { GlEmptyState, GlLoadingIcon, @@ -73,7 +79,11 @@ export default { GlNewDropdown, GlNewDropdownItem, GlIcon, + GlTabs, + GlTab, + GlBadge, }, + mixins: [glFeatureFlagsMixin()], props: { projectPath: { type: String, @@ -102,6 +112,7 @@ export default { variables() { return { projectPath: this.projectPath, + status: this.statusFilter, }; }, update(data) { @@ -118,6 +129,7 @@ export default { errored: false, isAlertDismissed: false, isErrorAlertDismissed: false, + statusFilter: this.$options.statusTabs[0].status, }; }, computed: { @@ -131,6 +143,11 @@ export default { return this.$apollo.queries.alerts.loading; }, }, + methods: { + filterALertsByStatus(tabIndex) { + this.statusFilter = this.$options.statusTabs[tabIndex].status; + }, + }, }; @@ -144,6 +161,17 @@ export default { {{ $options.i18n.errorMsg }} + + + + + +

{{ s__('AlertManagement|Alerts') }}

diff --git a/app/assets/javascripts/alert_management/constants.js b/app/assets/javascripts/alert_management/constants.js new file mode 100644 index 00000000000..c95a3c29f04 --- /dev/null +++ b/app/assets/javascripts/alert_management/constants.js @@ -0,0 +1,32 @@ +import { s__ } from '~/locale'; + +export const ALERTS_STATUS = { + OPEN: 'open', + TRIGGERED: 'triggered', + ACKNOWLEDGED: 'acknowledged', + RESOLVED: 'resolved', + ALL: 'all', +}; + +export const ALERTS_STATUS_TABS = [ + { + title: s__('AlertManagement|Open'), + status: ALERTS_STATUS.OPEN, + }, + { + title: s__('AlertManagement|Triggered'), + status: ALERTS_STATUS.TRIGGERED, + }, + { + title: s__('AlertManagement|Acknowledged'), + status: ALERTS_STATUS.ACKNOWLEDGED, + }, + { + title: s__('AlertManagement|Resolved'), + status: ALERTS_STATUS.RESOLVED, + }, + { + title: s__('AlertManagement|All alerts'), + status: ALERTS_STATUS.ALL, + }, +]; diff --git a/app/assets/javascripts/code_navigation/store/actions.js b/app/assets/javascripts/code_navigation/store/actions.js index f5b96145314..7b2669691bd 100644 --- a/app/assets/javascripts/code_navigation/store/actions.js +++ b/app/assets/javascripts/code_navigation/store/actions.js @@ -30,7 +30,9 @@ export default { }); }, showBlobInteractionZones({ state }, path) { - Object.values(state.data[path]).forEach(d => addInteractionClass(path, d)); + if (state.data && state.data[path]) { + Object.values(state.data[path]).forEach(d => addInteractionClass(path, d)); + } }, showDefinition({ commit, state }, { target: el }) { let definition; diff --git a/app/assets/javascripts/design_management/components/design_notes/design_discussion.vue b/app/assets/javascripts/design_management/components/design_notes/design_discussion.vue index 9ba69e34494..024957abe46 100644 --- a/app/assets/javascripts/design_management/components/design_notes/design_discussion.vue +++ b/app/assets/javascripts/design_management/components/design_notes/design_discussion.vue @@ -103,7 +103,13 @@ export default { class="design-discussion bordered-box position-relative" data-qa-selector="design_discussion_content" > - +
+import { ApolloMutation } from 'vue-apollo'; +import { GlTooltipDirective, GlIcon } from '@gitlab/ui'; +import updateNoteMutation from '../../graphql/mutations/update_note.mutation.graphql'; import UserAvatarLink from '~/vue_shared/components/user_avatar/user_avatar_link.vue'; import TimelineEntryItem from '~/vue_shared/components/notes/timeline_entry_item.vue'; import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; +import DesignReplyForm from './design_reply_form.vue'; import { findNoteId } from '../../utils/design_management_utils'; +import { hasErrors } from '../../utils/cache_update'; export default { components: { UserAvatarLink, TimelineEntryItem, TimeAgoTooltip, + DesignReplyForm, + ApolloMutation, + GlIcon, + }, + directives: { + GlTooltip: GlTooltipDirective, }, props: { note: { type: Object, required: true, }, + markdownPreviewPath: { + type: String, + required: false, + default: '', + }, + }, + data() { + return { + noteText: this.note.body, + isEditing: false, + }; }, computed: { author() { @@ -26,12 +48,31 @@ export default { isNoteLinked() { return this.$route.hash === `#note_${this.noteAnchorId}`; }, + mutationPayload() { + return { + id: this.note.id, + body: this.noteText, + }; + }, }, mounted() { if (this.isNoteLinked) { this.$refs.anchor.$el.scrollIntoView({ behavior: 'smooth', inline: 'start' }); } }, + methods: { + hideForm() { + this.isEditing = false; + this.noteText = this.note.body; + }, + onDone({ data }) { + this.hideForm(); + if (hasErrors(data.updateNote)) { + this.$emit('error', data.errors[0]); + } + }, + }, + updateNoteMutation, }; @@ -43,26 +84,65 @@ export default { :img-alt="author.username" :img-size="40" /> - - {{ author.name }} - - @{{ author.username }} - - - - - -
+ + + + +
+ + +
+ + + diff --git a/app/assets/javascripts/design_management/components/design_notes/design_reply_form.vue b/app/assets/javascripts/design_management/components/design_notes/design_reply_form.vue index 3736b0204e3..40be9867fee 100644 --- a/app/assets/javascripts/design_management/components/design_notes/design_reply_form.vue +++ b/app/assets/javascripts/design_management/components/design_notes/design_reply_form.vue @@ -1,6 +1,7 @@ + diff --git a/app/assets/javascripts/snippets/components/snippet_title.vue b/app/assets/javascripts/snippets/components/snippet_title.vue index 06484ad5110..5267c3748ca 100644 --- a/app/assets/javascripts/snippets/components/snippet_title.vue +++ b/app/assets/javascripts/snippets/components/snippet_title.vue @@ -1,11 +1,14 @@ diff --git a/app/assets/javascripts/vue_shared/components/markdown/field_view.vue b/app/assets/javascripts/vue_shared/components/markdown/field_view.vue new file mode 100644 index 00000000000..d77123371f2 --- /dev/null +++ b/app/assets/javascripts/vue_shared/components/markdown/field_view.vue @@ -0,0 +1,19 @@ + + + diff --git a/app/assets/stylesheets/page_bundles/themes/_dark.scss b/app/assets/stylesheets/page_bundles/themes/_dark.scss index 634f18ee1bd..30822eb7de4 100644 --- a/app/assets/stylesheets/page_bundles/themes/_dark.scss +++ b/app/assets/stylesheets/page_bundles/themes/_dark.scss @@ -94,7 +94,7 @@ } .ide-pipeline svg { - --svg-status-bg: transparent; + --svg-status-bg: $background; } .multi-file-tab-close:hover { diff --git a/app/assets/stylesheets/pages/alerts_list.scss b/app/assets/stylesheets/pages/alerts_list.scss index 0d0db0ea6fe..5974f97b728 100644 --- a/app/assets/stylesheets/pages/alerts_list.scss +++ b/app/assets/stylesheets/pages/alerts_list.scss @@ -74,4 +74,14 @@ } } } + + .gl-tab-nav-item { + color: $gl-gray-600; + + > .gl-tab-counter-badge { + color: inherit; + @include gl-font-sm; + background-color: $white-normal; + } + } } diff --git a/app/controllers/projects/alert_management_controller.rb b/app/controllers/projects/alert_management_controller.rb index fd85b4346c0..9be8a89fc02 100644 --- a/app/controllers/projects/alert_management_controller.rb +++ b/app/controllers/projects/alert_management_controller.rb @@ -3,6 +3,9 @@ class Projects::AlertManagementController < Projects::ApplicationController before_action :ensure_list_feature_enabled, only: :index before_action :ensure_detail_feature_enabled, only: :details + before_action do + push_frontend_feature_flag(:alert_list_status_filtering_enabled) + end def index end diff --git a/app/graphql/types/alert_management/status_enum.rb b/app/graphql/types/alert_management/status_enum.rb index f55e954d5d4..4ff6c4a9505 100644 --- a/app/graphql/types/alert_management/status_enum.rb +++ b/app/graphql/types/alert_management/status_enum.rb @@ -6,8 +6,8 @@ module Types graphql_name 'AlertManagementStatus' description 'Alert status values' - ::AlertManagement::Alert.statuses.keys.each do |status| - value status.upcase, value: status, description: "#{status.titleize} status" + ::AlertManagement::Alert::STATUSES.each do |name, value| + value name.upcase, value: value, description: "#{name.to_s.titleize} status" end end end diff --git a/app/models/alert_management/alert.rb b/app/models/alert_management/alert.rb index 6522d52690d..6bbdc1645ca 100644 --- a/app/models/alert_management/alert.rb +++ b/app/models/alert_management/alert.rb @@ -6,6 +6,20 @@ module AlertManagement include ShaAttribute include Sortable + STATUSES = { + triggered: 0, + acknowledged: 1, + resolved: 2, + ignored: 3 + }.freeze + + STATUS_EVENTS = { + triggered: :trigger, + acknowledged: :acknowledge, + resolved: :resolve, + ignored: :ignore + }.freeze + belongs_to :project belongs_to :issue, optional: true has_internal_id :iid, scope: :project, init: ->(s) { s.project.alert_management_alerts.maximum(:iid) } @@ -37,14 +51,49 @@ module AlertManagement unknown: 5 } - enum status: { - triggered: 0, - acknowledged: 1, - resolved: 2, - ignored: 3 - } + state_machine :status, initial: :triggered do + state :triggered, value: STATUSES[:triggered] + + state :acknowledged, value: STATUSES[:acknowledged] + + state :resolved, value: STATUSES[:resolved] do + validates :ended_at, presence: true + end + + state :ignored, value: STATUSES[:ignored] + + state :triggered, :acknowledged, :ignored do + validates :ended_at, absence: true + end + + event :trigger do + transition any => :triggered + end + + event :acknowledge do + transition any => :acknowledged + end + + event :resolve do + transition any => :resolved + end + + event :ignore do + transition any => :ignored + end + + before_transition to: [:triggered, :acknowledged, :ignored] do |alert, _transition| + alert.ended_at = nil + end + + before_transition to: :resolved do |alert, transition| + ended_at = transition.args.first + alert.ended_at = ended_at || Time.current + end + end scope :for_iid, -> (iid) { where(iid: iid) } + scope :for_fingerprint, -> (project, fingerprint) { where(project: project, fingerprint: fingerprint) } scope :order_start_time, -> (sort_order) { order(started_at: sort_order) } scope :order_end_time, -> (sort_order) { order(ended_at: sort_order) } @@ -69,14 +118,6 @@ module AlertManagement end end - def fingerprint=(value) - if value.blank? - super(nil) - else - super(Digest::SHA1.hexdigest(value.to_s)) - end - end - def details details_payload = payload.except(*attributes.keys) diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index ea3e00d9cd3..f793bd3d76f 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -385,37 +385,12 @@ class MergeRequestDiff < ApplicationRecord end end - # Carrierwave defines `write_uploader` dynamically on this class, so `super` - # does not work. Alias the carrierwave method so we can call it when needed - alias_method :carrierwave_write_uploader, :write_uploader - - # The `external_diff`, `external_diff_store`, and `stored_externally` - # columns were introduced in GitLab 11.8, but some background migration specs - # use factories that rely on current code with an old schema. Without these - # `has_attribute?` guards, they fail with a `MissingAttributeError`. - # - # For more details, see: https://gitlab.com/gitlab-org/gitlab-foss/issues/44990 - - def write_uploader(column, identifier) - carrierwave_write_uploader(column, identifier) if has_attribute?(column) - end - def update_external_diff_store - return unless has_attribute?(:external_diff_store) return unless saved_change_to_external_diff? || saved_change_to_stored_externally? update_column(:external_diff_store, external_diff.object_store) end - def saved_change_to_external_diff? - super if has_attribute?(:external_diff) - end - - def stored_externally - super if has_attribute?(:stored_externally) - end - alias_method :stored_externally?, :stored_externally - # If enabled, yields the external file containing the diff. Otherwise, yields # nil. This method is not thread-safe, but it *is* re-entrant, which allows # multiple merge_request_diff_files to load their data efficiently @@ -577,7 +552,6 @@ class MergeRequestDiff < ApplicationRecord end def use_external_diff? - return false unless has_attribute?(:external_diff) return false unless Gitlab.config.external_diffs.enabled case Gitlab.config.external_diffs.when diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 98715a8c67c..e2d1a433935 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -175,6 +175,10 @@ class Namespace < ApplicationRecord kind == 'user' end + def group? + type == 'Group' + end + def find_fork_of(project) return unless project.fork_network diff --git a/app/services/alert_management/process_prometheus_alert_service.rb b/app/services/alert_management/process_prometheus_alert_service.rb new file mode 100644 index 00000000000..af28f1354b3 --- /dev/null +++ b/app/services/alert_management/process_prometheus_alert_service.rb @@ -0,0 +1,86 @@ +# frozen_string_literal: true + +module AlertManagement + class ProcessPrometheusAlertService < BaseService + include Gitlab::Utils::StrongMemoize + + def execute + return bad_request unless parsed_alert.valid? + + process_alert_management_alert + + ServiceResponse.success + end + + private + + delegate :firing?, :resolved?, :gitlab_fingerprint, :ends_at, to: :parsed_alert + + def parsed_alert + strong_memoize(:parsed_alert) do + Gitlab::Alerting::Alert.new(project: project, payload: params) + end + end + + def process_alert_management_alert + process_firing_alert_management_alert if firing? + process_resolved_alert_management_alert if resolved? + end + + def process_firing_alert_management_alert + if am_alert.present? + reset_alert_management_alert_status + else + create_alert_management_alert + end + end + + def reset_alert_management_alert_status + return if am_alert.trigger + + logger.warn( + message: 'Unable to update AlertManagement::Alert status to triggered', + project_id: project.id, + alert_id: am_alert.id + ) + end + + def create_alert_management_alert + am_alert = AlertManagement::Alert.new(am_alert_params.merge(ended_at: nil)) + return if am_alert.save + + logger.warn( + message: 'Unable to create AlertManagement::Alert', + project_id: project.id, + alert_errors: am_alert.errors.messages + ) + end + + def am_alert_params + Gitlab::AlertManagement::AlertParams.from_prometheus_alert(project: project, parsed_alert: parsed_alert) + end + + def process_resolved_alert_management_alert + return if am_alert.blank? + return if am_alert.resolve(ends_at) + + logger.warn( + message: 'Unable to update AlertManagement::Alert status to resolved', + project_id: project.id, + alert_id: am_alert.id + ) + end + + def logger + @logger ||= Gitlab::AppLogger + end + + def am_alert + @am_alert ||= AlertManagement::Alert.for_fingerprint(project, gitlab_fingerprint).first + end + + def bad_request + ServiceResponse.error(message: 'Bad Request', http_status: :bad_request) + end + end +end diff --git a/app/services/alert_management/update_alert_status_service.rb b/app/services/alert_management/update_alert_status_service.rb index 73ee654874f..8e0c3e38b8b 100644 --- a/app/services/alert_management/update_alert_status_service.rb +++ b/app/services/alert_management/update_alert_status_service.rb @@ -8,9 +8,9 @@ module AlertManagement end def execute - return error('Invalid status') unless AlertManagement::Alert.statuses.key?(status.to_s) + return error('Invalid status') unless AlertManagement::Alert::STATUSES.key?(status.to_sym) - alert.status = status + alert.status_event = AlertManagement::Alert::STATUS_EVENTS[status.to_sym] if alert.save success diff --git a/app/services/concerns/git/logger.rb b/app/services/concerns/git/logger.rb deleted file mode 100644 index 7c036212e66..00000000000 --- a/app/services/concerns/git/logger.rb +++ /dev/null @@ -1,10 +0,0 @@ -# frozen_string_literal: true - -module Git - module Logger - def log_error(message, save_message_on_model: false) - Gitlab::GitLogger.error("#{self.class.name} error (#{merge_request.to_reference(full: true)}): #{message}") - merge_request.update(merge_error: message) if save_message_on_model - end - end -end diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 5365c09eb25..b250c2d7d21 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -114,6 +114,32 @@ module MergeRequests yield merge_request end end + + def log_error(exception:, message:, save_message_on_model: false) + reference = merge_request.to_reference(full: true) + data = { + class: self.class.name, + message: message, + merge_request_id: merge_request.id, + merge_request: reference, + save_message_on_model: save_message_on_model + } + + if exception + Gitlab::ErrorTracking.with_context(current_user) do + Gitlab::ErrorTracking.track_exception(exception, data) + end + + data[:"exception.message"] = exception.message + end + + # TODO: Deprecate Gitlab::GitLogger since ErrorTracking should suffice: + # https://gitlab.com/gitlab-org/gitlab/-/issues/216379 + data[:message] = "#{self.class.name} error (#{reference}): #{message}" + Gitlab::GitLogger.error(data) + + merge_request.update(merge_error: message) if save_message_on_model + end end end diff --git a/app/services/merge_requests/rebase_service.rb b/app/services/merge_requests/rebase_service.rb index bc1e97088af..87808a21a15 100644 --- a/app/services/merge_requests/rebase_service.rb +++ b/app/services/merge_requests/rebase_service.rb @@ -2,8 +2,6 @@ module MergeRequests class RebaseService < MergeRequests::BaseService - include Git::Logger - REBASE_ERROR = 'Rebase failed. Please rebase locally' attr_reader :merge_request @@ -22,7 +20,7 @@ module MergeRequests def rebase # Ensure Gitaly isn't already running a rebase if source_project.repository.rebase_in_progress?(merge_request.id) - log_error('Rebase task canceled: Another rebase is already in progress', save_message_on_model: true) + log_error(exception: nil, message: 'Rebase task canceled: Another rebase is already in progress', save_message_on_model: true) return false end @@ -30,8 +28,8 @@ module MergeRequests true rescue => e - log_error(REBASE_ERROR, save_message_on_model: true) - log_error(e.message) + log_error(exception: e, message: REBASE_ERROR, save_message_on_model: true) + false ensure merge_request.update_column(:rebase_jid, nil) diff --git a/app/services/merge_requests/squash_service.rb b/app/services/merge_requests/squash_service.rb index d25997c925e..4b04d42b48e 100644 --- a/app/services/merge_requests/squash_service.rb +++ b/app/services/merge_requests/squash_service.rb @@ -2,7 +2,7 @@ module MergeRequests class SquashService < MergeRequests::BaseService - include Git::Logger + SquashInProgressError = Class.new(RuntimeError) def execute # If performing a squash would result in no change, then @@ -11,11 +11,13 @@ module MergeRequests return success(squash_sha: merge_request.diff_head_sha) end - if merge_request.squash_in_progress? + if squash_in_progress? return error(s_('MergeRequests|Squash task canceled: another squash is already in progress.')) end squash! || error(s_('MergeRequests|Failed to squash. Should be done manually.')) + rescue SquashInProgressError + error(s_('MergeRequests|An error occurred while checking whether another squash is in progress.')) end private @@ -25,11 +27,19 @@ module MergeRequests success(squash_sha: squash_sha) rescue => e - log_error("Failed to squash merge request #{merge_request.to_reference(full: true)}:") - log_error(e.message) + log_error(exception: e, message: 'Failed to squash merge request') + false end + def squash_in_progress? + merge_request.squash_in_progress? + rescue => e + log_error(exception: e, message: 'Failed to check squash in progress') + + raise SquashInProgressError, e.message + end + def repository target_project.repository end diff --git a/app/services/projects/prometheus/alerts/notify_service.rb b/app/services/projects/prometheus/alerts/notify_service.rb index 6ebc061c2e3..c1793d2a2d6 100644 --- a/app/services/projects/prometheus/alerts/notify_service.rb +++ b/app/services/projects/prometheus/alerts/notify_service.rb @@ -12,6 +12,7 @@ module Projects return unprocessable_entity unless valid_version? return unauthorized unless valid_alert_manager_token?(token) + process_prometheus_alerts persist_events send_alert_email if send_email? process_incident_issues if process_issues? @@ -115,6 +116,16 @@ module Projects end end + def process_prometheus_alerts + return unless Feature.enabled?(:alert_management_minimal, project) + + alerts.each do |alert| + AlertManagement::ProcessPrometheusAlertService + .new(project, nil, alert.to_h) + .execute + end + end + def persist_events CreateEventsService.new(project, nil, params).execute end diff --git a/app/views/projects/merge_requests/_widget.html.haml b/app/views/projects/merge_requests/_widget.html.haml index 1853d40c2e4..6aba5c98d52 100644 --- a/app/views/projects/merge_requests/_widget.html.haml +++ b/app/views/projects/merge_requests/_widget.html.haml @@ -8,6 +8,7 @@ window.gl.mrWidgetData.squash_before_merge_help_path = '#{help_page_path("user/project/merge_requests/squash_and_merge")}'; window.gl.mrWidgetData.troubleshooting_docs_path = '#{help_page_path('user/project/merge_requests/reviewing_and_managing_merge_requests.md', anchor: 'troubleshooting')}'; + window.gl.mrWidgetData.pipeline_must_succeed_docs_path = '#{help_page_path('user/project/merge_requests/merge_when_pipeline_succeeds.md', anchor: 'only-allow-merge-requests-to-be-merged-if-the-pipeline-succeeds')}'; window.gl.mrWidgetData.security_approvals_help_page_path = '#{help_page_path('user/application_security/index.html', anchor: 'security-approvals-in-merge-requests-ultimate')}'; window.gl.mrWidgetData.eligible_approvers_docs_path = '#{help_page_path('user/project/merge_requests/merge_request_approvals', anchor: 'eligible-approvers')}'; window.gl.mrWidgetData.pipelines_empty_svg_path = '#{image_path('illustrations/pipelines_empty.svg')}'; diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 6809d818717..7155107b586 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -18,35 +18,35 @@ :weight: 3 :idempotent: - :name: chaos:chaos_cpu_spin - :feature_category: :chaos_engineering + :feature_category: :not_owned :has_external_dependencies: :urgency: :low :resource_boundary: :unknown :weight: 2 :idempotent: - :name: chaos:chaos_db_spin - :feature_category: :chaos_engineering + :feature_category: :not_owned :has_external_dependencies: :urgency: :low :resource_boundary: :unknown :weight: 2 :idempotent: - :name: chaos:chaos_kill - :feature_category: :chaos_engineering + :feature_category: :not_owned :has_external_dependencies: :urgency: :low :resource_boundary: :unknown :weight: 2 :idempotent: - :name: chaos:chaos_leak_mem - :feature_category: :chaos_engineering + :feature_category: :not_owned :has_external_dependencies: :urgency: :low :resource_boundary: :unknown :weight: 2 :idempotent: - :name: chaos:chaos_sleep - :feature_category: :chaos_engineering + :feature_category: :not_owned :has_external_dependencies: :urgency: :low :resource_boundary: :unknown diff --git a/app/workers/concerns/chaos_queue.rb b/app/workers/concerns/chaos_queue.rb index c5db10491f2..a9c557f0175 100644 --- a/app/workers/concerns/chaos_queue.rb +++ b/app/workers/concerns/chaos_queue.rb @@ -5,6 +5,6 @@ module ChaosQueue included do queue_namespace :chaos - feature_category :chaos_engineering + feature_category_not_owned! end end diff --git a/changelogs/unreleased/118609-design-comment-edit-comment-text.yml b/changelogs/unreleased/118609-design-comment-edit-comment-text.yml new file mode 100644 index 00000000000..d5fdc2ca274 --- /dev/null +++ b/changelogs/unreleased/118609-design-comment-edit-comment-text.yml @@ -0,0 +1,5 @@ +--- +title: 'Resolve Design Comment: Edit Comment text' +merge_request: 30479 +author: +type: added diff --git a/changelogs/unreleased/197344-use-cookies-with-metadata.yml b/changelogs/unreleased/197344-use-cookies-with-metadata.yml new file mode 100644 index 00000000000..c07927ee6ad --- /dev/null +++ b/changelogs/unreleased/197344-use-cookies-with-metadata.yml @@ -0,0 +1,5 @@ +--- +title: Use cookies with metadata to prevent reuse as another cookie +merge_request: 31311 +author: +type: other diff --git a/changelogs/unreleased/212279-webide-vue-files.yml b/changelogs/unreleased/212279-webide-vue-files.yml new file mode 100644 index 00000000000..957c1642c0c --- /dev/null +++ b/changelogs/unreleased/212279-webide-vue-files.yml @@ -0,0 +1,5 @@ +--- +title: 'Web IDE: Introduce syntax highlighting for .vue files.' +merge_request: 30986 +author: +type: added diff --git a/changelogs/unreleased/215472-single-chart-from-url.yml b/changelogs/unreleased/215472-single-chart-from-url.yml new file mode 100644 index 00000000000..f5fef9e76c3 --- /dev/null +++ b/changelogs/unreleased/215472-single-chart-from-url.yml @@ -0,0 +1,5 @@ +--- +title: Display expanded dashboard from a panel's "Link to chart" URL +merge_request: 30476 +author: +type: added diff --git a/changelogs/unreleased/216031-show-accurate-error-message-when-pipelines-disabled-mr-must-succee.yml b/changelogs/unreleased/216031-show-accurate-error-message-when-pipelines-disabled-mr-must-succee.yml new file mode 100644 index 00000000000..c7868e90106 --- /dev/null +++ b/changelogs/unreleased/216031-show-accurate-error-message-when-pipelines-disabled-mr-must-succee.yml @@ -0,0 +1,7 @@ +--- +title: + Add clear explanation to the MR widget when no CI is available and Pipeline + must succeed option is activated +merge_request: 31112 +author: +type: changed diff --git a/changelogs/unreleased/cleanup_user_highest_roles_background_migration.yml b/changelogs/unreleased/cleanup_user_highest_roles_background_migration.yml new file mode 100644 index 00000000000..fa7af7c7f34 --- /dev/null +++ b/changelogs/unreleased/cleanup_user_highest_roles_background_migration.yml @@ -0,0 +1,5 @@ +--- +title: Cleanup background migration for populating user_highest_roles table +merge_request: 31218 +author: +type: other diff --git a/changelogs/unreleased/sh-fix-squash-error-handling.yml b/changelogs/unreleased/sh-fix-squash-error-handling.yml new file mode 100644 index 00000000000..d011d424a27 --- /dev/null +++ b/changelogs/unreleased/sh-fix-squash-error-handling.yml @@ -0,0 +1,5 @@ +--- +title: Improve error handling of squash and rebase +merge_request: 23740 +author: +type: other diff --git a/config/feature_categories.yml b/config/feature_categories.yml index 6033a128284..010d3d14fcb 100644 --- a/config/feature_categories.yml +++ b/config/feature_categories.yml @@ -19,7 +19,6 @@ - backup_restore - behavior_analytics - billing -- chaos_engineering - chatops - cloud_native_installation - cluster_cost_optimization diff --git a/config/initializers/cookies_serializer.rb b/config/initializers/cookies_serializer.rb index a04d5044f4e..fa1736dfea6 100644 --- a/config/initializers/cookies_serializer.rb +++ b/config/initializers/cookies_serializer.rb @@ -1,4 +1,4 @@ # Be sure to restart your server when you modify this file. -Rails.application.config.action_dispatch.use_cookies_with_metadata = false +Rails.application.config.action_dispatch.use_cookies_with_metadata = true Rails.application.config.action_dispatch.cookies_serializer = :hybrid diff --git a/db/post_migrate/20200506125731_cleanup_user_highest_roles_population.rb b/db/post_migrate/20200506125731_cleanup_user_highest_roles_population.rb new file mode 100644 index 00000000000..5e613228c56 --- /dev/null +++ b/db/post_migrate/20200506125731_cleanup_user_highest_roles_population.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class CleanupUserHighestRolesPopulation < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + INDEX_NAME = 'index_for_migrating_user_highest_roles_table' + + disable_ddl_transaction! + + def up + Gitlab::BackgroundMigration.steal('PopulateUserHighestRolesTable') + + remove_concurrent_index(:users, :id, name: INDEX_NAME) + end + + def down + add_concurrent_index(:users, + :id, + where: "state = 'active' AND user_type IS NULL AND bot_type IS NULL AND ghost IS NOT TRUE", + name: INDEX_NAME) + end +end diff --git a/db/structure.sql b/db/structure.sql index e450a99eace..df1eac0303a 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -9556,8 +9556,6 @@ CREATE UNIQUE INDEX index_feature_gates_on_feature_key_and_key_and_value ON publ CREATE UNIQUE INDEX index_features_on_key ON public.features USING btree (key); -CREATE INDEX index_for_migrating_user_highest_roles_table ON public.users USING btree (id) WHERE (((state)::text = 'active'::text) AND (user_type IS NULL) AND (bot_type IS NULL) AND (ghost IS NOT TRUE)); - CREATE INDEX index_for_resource_group ON public.ci_builds USING btree (resource_group_id, id) WHERE (resource_group_id IS NOT NULL); CREATE INDEX index_for_status_per_branch_per_project ON public.merge_trains USING btree (target_project_id, target_branch, status); @@ -13709,5 +13707,6 @@ COPY "schema_migrations" (version) FROM STDIN; 20200429181335 20200429181955 20200429182245 +20200506125731 \. diff --git a/doc/development/documentation/styleguide.md b/doc/development/documentation/styleguide.md index 630a1845002..ab57ff686b5 100644 --- a/doc/development/documentation/styleguide.md +++ b/doc/development/documentation/styleguide.md @@ -248,12 +248,6 @@ GitLab documentation should be clear and easy to understand. - Be clear, concise, and stick to the goal of the documentation. - Write in US English with US grammar. - Use inclusive language. -- Avoid jargon. -- Avoid uncommon words. -- Don't write in the first person singular. - - Instead of "I" or "me," use "we," "you," "us," or "one." - - When possible, stay user focused by writing in the second person ("you" or the imperative). -- Don't overuse "that". In many cases, you can remove "that" from a sentence and improve readability. ### Point of view @@ -287,25 +281,52 @@ because it’s friendly and easy to understand. Some features are also objects. For example, "GitLab's Merge Requests support X" and "Create a new merge request for Z." +### Language to avoid + +When creating documentation, limit or avoid the use of the following verb +tenses, words, and phrases: + +- Avoid jargon. +- Avoid uncommon words. +- Don't write in the first person singular. + - Instead of "I" or "me," use "we," "you," "us," or "one." + - When possible, stay user focused by writing in the second person ("you" or + the imperative). +- Don't overuse "that". In many cases, you can remove "that" from a sentence + and improve readability. - Avoid use of the future tense: - - Instead of "after you execute this command, GitLab will display the result", use "after you execute this command, GitLab displays the result". - - Only use the future tense to convey when the action or result will actually occur at a future time. -- Do not use slashes to clump different words together or as a replacement for the word "or": - - Instead of "and/or," consider using "or," or use another sensible construction. - - Other examples include "clone/fetch," author/assignee," and "namespace/repository name." Break apart any such instances in an appropriate way. - - Exceptions to this rule include commonly accepted technical terms such as CI/CD, TCP/IP, and so on. -- Do not use "may" and "might" interchangeably: - - Use "might" to indicate the probability of something occurring. "If you skip this step, the import process might fail." - - Use "may" to indicate giving permission for someone to do something, or consider using "can" instead. "You may select either option on this screen." Or, "you can select either option on this screen." + - Instead of "after you execute this command, GitLab will display the + result", use "after you execute this command, GitLab displays the result". + - Only use the future tense to convey when the action or result will actually + occur at a future time. +- Don't use slashes to clump different words together or as a replacement for + the word "or": + - Instead of "and/or," consider using "or," or use another sensible + construction. + - Other examples include "clone/fetch," author/assignee," and + "namespace/repository name." Break apart any such instances in an + appropriate way. + - Exceptions to this rule include commonly accepted technical terms, such as + CI/CD and TCP/IP. - We discourage use of Latin abbreviations, such as "e.g.," "i.e.," or "etc.," -as even native users of English might misunderstand them. + as even native users of English might misunderstand them. - Instead of "i.e.," use "that is." - Instead of "e.g.," use "for example," "such as," "for instance," or "like." - - Instead of "etc.," either use "and so on" or consider editing it out, since it can be vague. -- Avoid using the word *Currently* when talking about the product or its + - Instead of "etc.," either use "and so on" or consider editing it out, since + it can be vague. +- Avoid using the word *currently* when talking about the product or its features. The documentation describes the product as it is, and not as it will be at some indeterminate point in the future. +### Word usage clarifications + +- Don't use "may" and "might" interchangeably: + - Use "might" to indicate the probability of something occurring. "If you + skip this step, the import process might fail." + - Use "may" to indicate giving permission for someone to do something, or + consider using "can" instead. "You may select either option on this + screen." Or, "You can select either option on this screen." + ### Contractions - Use common contractions when it helps create a friendly and informal tone, especially in tutorials, instructional documentation, and [UIs](https://design.gitlab.com/content/punctuation/#contractions). diff --git a/lib/gitlab/alert_management/alert_params.rb b/lib/gitlab/alert_management/alert_params.rb index 014eba6326d..876eff6d91e 100644 --- a/lib/gitlab/alert_management/alert_params.rb +++ b/lib/gitlab/alert_management/alert_params.rb @@ -3,6 +3,10 @@ module Gitlab module AlertManagement class AlertParams + MONITORING_TOOLS = { + prometheus: 'Prometheus' + }.freeze + def self.from_generic_alert(project:, payload:) parsed_payload = Gitlab::Alerting::NotificationPayloadParser.call(payload).with_indifferent_access annotations = parsed_payload[:annotations] @@ -18,6 +22,19 @@ module Gitlab started_at: parsed_payload['startsAt'] } end + + def self.from_prometheus_alert(project:, parsed_alert:) + { + project_id: project.id, + title: parsed_alert.title, + description: parsed_alert.description, + monitoring_tool: MONITORING_TOOLS[:prometheus], + payload: parsed_alert.payload, + started_at: parsed_alert.starts_at, + ended_at: parsed_alert.ends_at, + fingerprint: parsed_alert.gitlab_fingerprint + } + end end end end diff --git a/lib/gitlab/alerting/alert.rb b/lib/gitlab/alerting/alert.rb index 7d97bd1bb52..d859ca89418 100644 --- a/lib/gitlab/alerting/alert.rb +++ b/lib/gitlab/alerting/alert.rb @@ -105,6 +105,10 @@ module Gitlab metric_id.present? end + def gitlab_fingerprint + Digest::SHA1.hexdigest(plain_gitlab_fingerprint) + end + def valid? payload.respond_to?(:dig) && project && title && starts_at end @@ -115,6 +119,14 @@ module Gitlab private + def plain_gitlab_fingerprint + if gitlab_managed? + [metric_id, starts_at].join('/') + else # self managed + [starts_at, title, full_query].join('/') + end + end + def parse_environment_from_payload environment_name = payload&.dig('labels', 'gitlab_environment_name') diff --git a/lib/gitlab/metrics/samplers/ruby_sampler.rb b/lib/gitlab/metrics/samplers/ruby_sampler.rb index c38769f39a9..0ca266f2e0c 100644 --- a/lib/gitlab/metrics/samplers/ruby_sampler.rb +++ b/lib/gitlab/metrics/samplers/ruby_sampler.rb @@ -34,14 +34,16 @@ module Gitlab def init_metrics metrics = { - file_descriptors: ::Gitlab::Metrics.gauge(with_prefix(:file, :descriptors), 'File descriptors used', labels), - memory_bytes: ::Gitlab::Metrics.gauge(with_prefix(:memory, :bytes), 'Memory used', labels), - process_cpu_seconds_total: ::Gitlab::Metrics.gauge(with_prefix(:process, :cpu_seconds_total), 'Process CPU seconds total'), - process_max_fds: ::Gitlab::Metrics.gauge(with_prefix(:process, :max_fds), 'Process max fds'), - process_resident_memory_bytes: ::Gitlab::Metrics.gauge(with_prefix(:process, :resident_memory_bytes), 'Memory used', labels), - process_start_time_seconds: ::Gitlab::Metrics.gauge(with_prefix(:process, :start_time_seconds), 'Process start time seconds'), - sampler_duration: ::Gitlab::Metrics.counter(with_prefix(:sampler, :duration_seconds_total), 'Sampler time', labels), - gc_duration_seconds: ::Gitlab::Metrics.histogram(with_prefix(:gc, :duration_seconds), 'GC time', labels, GC_REPORT_BUCKETS) + file_descriptors: ::Gitlab::Metrics.gauge(with_prefix(:file, :descriptors), 'File descriptors used', labels), + memory_bytes: ::Gitlab::Metrics.gauge(with_prefix(:memory, :bytes), 'Memory used (RSS)', labels), + process_cpu_seconds_total: ::Gitlab::Metrics.gauge(with_prefix(:process, :cpu_seconds_total), 'Process CPU seconds total'), + process_max_fds: ::Gitlab::Metrics.gauge(with_prefix(:process, :max_fds), 'Process max fds'), + process_resident_memory_bytes: ::Gitlab::Metrics.gauge(with_prefix(:process, :resident_memory_bytes), 'Memory used (RSS)', labels), + process_unique_memory_bytes: ::Gitlab::Metrics.gauge(with_prefix(:process, :unique_memory_bytes), 'Memory used (USS)', labels), + process_proportional_memory_bytes: ::Gitlab::Metrics.gauge(with_prefix(:process, :proportional_memory_bytes), 'Memory used (PSS)', labels), + process_start_time_seconds: ::Gitlab::Metrics.gauge(with_prefix(:process, :start_time_seconds), 'Process start time seconds'), + sampler_duration: ::Gitlab::Metrics.counter(with_prefix(:sampler, :duration_seconds_total), 'Sampler time', labels), + gc_duration_seconds: ::Gitlab::Metrics.histogram(with_prefix(:gc, :duration_seconds), 'GC time', labels, GC_REPORT_BUCKETS) } GC.stat.keys.each do |key| @@ -85,10 +87,15 @@ module Gitlab end def set_memory_usage_metrics - memory_usage = System.memory_usage + memory_rss = System.memory_usage + metrics[:memory_bytes].set(labels, memory_rss) + metrics[:process_resident_memory_bytes].set(labels, memory_rss) - metrics[:memory_bytes].set(labels, memory_usage) - metrics[:process_resident_memory_bytes].set(labels, memory_usage) + if Gitlab::Utils.to_boolean(ENV['enable_memory_uss_pss']) + memory_uss_pss = System.memory_usage_uss_pss + metrics[:process_unique_memory_bytes].set(labels, memory_uss_pss[:uss]) + metrics[:process_proportional_memory_bytes].set(labels, memory_uss_pss[:pss]) + end end end end diff --git a/lib/gitlab/metrics/system.rb b/lib/gitlab/metrics/system.rb index 2a61b3de405..d01b6bc5b50 100644 --- a/lib/gitlab/metrics/system.rb +++ b/lib/gitlab/metrics/system.rb @@ -7,47 +7,37 @@ module Gitlab # This module relies on the /proc filesystem being available. If /proc is # not available the methods of this module will be stubbed. module System - if File.exist?('/proc') - # Returns the current process' memory usage in bytes. - def self.memory_usage - mem = 0 - match = File.read('/proc/self/status').match(/VmRSS:\s+(\d+)/) + PROC_STATUS_PATH = '/proc/self/status' + PROC_SMAPS_ROLLUP_PATH = '/proc/self/smaps_rollup' + PROC_LIMITS_PATH = '/proc/self/limits' + PROC_FD_GLOB = '/proc/self/fd/*' - if match && match[1] - mem = match[1].to_f * 1024 - end + PRIVATE_PAGES_PATTERN = /^(Private_Clean|Private_Dirty|Private_Hugetlb):\s+(?\d+)/.freeze + PSS_PATTERN = /^Pss:\s+(?\d+)/.freeze + RSS_PATTERN = /VmRSS:\s+(?\d+)/.freeze + MAX_OPEN_FILES_PATTERN = /Max open files\s*(?\d+)/.freeze - mem - end + # Returns the current process' RSS (resident set size) in bytes. + def self.memory_usage + sum_matches(PROC_STATUS_PATH, rss: RSS_PATTERN)[:rss].kilobytes + end - def self.file_descriptor_count - Dir.glob('/proc/self/fd/*').length - end + # Returns the current process' USS/PSS (unique/proportional set size) in bytes. + def self.memory_usage_uss_pss + sum_matches(PROC_SMAPS_ROLLUP_PATH, uss: PRIVATE_PAGES_PATTERN, pss: PSS_PATTERN) + .transform_values(&:kilobytes) + end - def self.max_open_file_descriptors - match = File.read('/proc/self/limits').match(/Max open files\s*(\d+)/) + def self.file_descriptor_count + Dir.glob(PROC_FD_GLOB).length + end - return unless match && match[1] - - match[1].to_i - end - else - def self.memory_usage - 0.0 - end - - def self.file_descriptor_count - 0 - end - - def self.max_open_file_descriptors - 0 - end + def self.max_open_file_descriptors + sum_matches(PROC_LIMITS_PATH, max_fds: MAX_OPEN_FILES_PATTERN)[:max_fds] end def self.cpu_time - Process - .clock_gettime(Process::CLOCK_PROCESS_CPUTIME_ID, :float_second) + Process.clock_gettime(Process::CLOCK_PROCESS_CPUTIME_ID, :float_second) end # Returns the current real time in a given precision. @@ -78,6 +68,27 @@ module Gitlab end_time - start_time end + + # Given a path to a file in /proc and a hash of (metric, pattern) pairs, + # sums up all values found for those patterns under the respective metric. + def self.sum_matches(proc_file, **patterns) + results = patterns.transform_values { 0 } + + begin + File.foreach(proc_file) do |line| + patterns.each do |metric, pattern| + match = line.match(pattern) + value = match&.named_captures&.fetch('value', 0) + results[metric] += value.to_i + end + end + rescue Errno::ENOENT + # This means the procfile we're reading from did not exist; + # this is safe to ignore, since we initialize each metric to 0 + end + + results + end end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index b504a5ce6bc..abb751000b1 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1712,6 +1712,9 @@ msgstr "" msgid "AlertManagement|Alerts" msgstr "" +msgid "AlertManagement|All alerts" +msgstr "" + msgid "AlertManagement|Authorize external service" msgstr "" @@ -1742,6 +1745,9 @@ msgstr "" msgid "AlertManagement|No alerts to display." msgstr "" +msgid "AlertManagement|Open" +msgstr "" + msgid "AlertManagement|Overview" msgstr "" @@ -7214,15 +7220,27 @@ msgstr "" msgid "DesignManagement|Adding a design with the same filename replaces the file in a new version." msgstr "" +msgid "DesignManagement|Are you sure you want to cancel changes to this comment?" +msgstr "" + msgid "DesignManagement|Are you sure you want to cancel creating this comment?" msgstr "" msgid "DesignManagement|Are you sure you want to delete the selected designs?" msgstr "" +msgid "DesignManagement|Cancel changes" +msgstr "" + msgid "DesignManagement|Cancel comment confirmation" msgstr "" +msgid "DesignManagement|Cancel comment update confirmation" +msgstr "" + +msgid "DesignManagement|Comment" +msgstr "" + msgid "DesignManagement|Could not add a new comment. Please try again." msgstr "" @@ -7232,6 +7250,9 @@ msgstr "" msgid "DesignManagement|Could not update discussion. Please try again." msgstr "" +msgid "DesignManagement|Could not update note. Please try again." +msgstr "" + msgid "DesignManagement|Delete" msgstr "" @@ -7259,12 +7280,18 @@ msgstr "" msgid "DesignManagement|Go to previous design" msgstr "" +msgid "DesignManagement|Keep changes" +msgstr "" + msgid "DesignManagement|Keep comment" msgstr "" msgid "DesignManagement|Requested design version does not exist. Showing latest version instead" msgstr "" +msgid "DesignManagement|Save comment" +msgstr "" + msgid "DesignManagement|Select all" msgstr "" @@ -13081,6 +13108,9 @@ msgstr "" msgid "MergeRequests|Add a reply" msgstr "" +msgid "MergeRequests|An error occurred while checking whether another squash is in progress." +msgstr "" + msgid "MergeRequests|An error occurred while saving the draft comment." msgstr "" @@ -13296,6 +13326,9 @@ msgstr "" msgid "Metrics|Link contains an invalid time window, please verify the link to see the requested time range." msgstr "" +msgid "Metrics|Link contains invalid chart information, please verify the link to see the expanded panel." +msgstr "" + msgid "Metrics|Max" msgstr "" @@ -15078,6 +15111,9 @@ msgstr "" msgid "Pipelines for merge requests are configured. A detached pipeline runs in the context of the merge request, and not against the merged result. Learn more in the documentation for Pipelines for Merged Results." msgstr "" +msgid "Pipelines must succeed for merge requests to be eligible to merge. Please enable pipelines for this project to continue. For more information, see the %{linkStart}documentation.%{linkEnd}" +msgstr "" + msgid "Pipelines settings for '%{project_name}' were successfully updated." msgstr "" @@ -15141,7 +15177,7 @@ msgstr "" msgid "Pipeline|Commit" msgstr "" -msgid "Pipeline|Could not retrieve the pipeline status. For troubleshooting steps, read the %{linkStart}documentation.%{linkEnd}" +msgid "Pipeline|Could not retrieve the pipeline status. For troubleshooting steps, read the %{linkStart}documentation%{linkEnd}." msgstr "" msgid "Pipeline|Coverage" @@ -15168,6 +15204,9 @@ msgstr "" msgid "Pipeline|Merged result pipeline" msgstr "" +msgid "Pipeline|No pipeline has been run for this commit." +msgstr "" + msgid "Pipeline|Pipeline" msgstr "" diff --git a/qa/qa/page/dashboard/snippet/show.rb b/qa/qa/page/dashboard/snippet/show.rb index fc40b072032..2dcb11018ef 100644 --- a/qa/qa/page/dashboard/snippet/show.rb +++ b/qa/qa/page/dashboard/snippet/show.rb @@ -5,8 +5,8 @@ module QA module Dashboard module Snippet class Show < Page::Base - view 'app/assets/javascripts/snippets/components/snippet_description_edit.vue' do - element :snippet_description_field, required: true + view 'app/assets/javascripts/snippets/components/snippet_description_view.vue' do + element :snippet_description_field end view 'app/assets/javascripts/snippets/components/snippet_title.vue' do diff --git a/spec/factories/alert_management/alerts.rb b/spec/factories/alert_management/alerts.rb index 20bbfffc6c6..28cfe5d6b29 100644 --- a/spec/factories/alert_management/alerts.rb +++ b/spec/factories/alert_management/alerts.rb @@ -31,8 +31,23 @@ FactoryBot.define do ended_at { Time.current } end + trait :without_ended_at do + ended_at { nil } + end + + trait :acknowledged do + status { AlertManagement::Alert::STATUSES[:acknowledged] } + without_ended_at + end + trait :resolved do - status { :resolved } + status { AlertManagement::Alert::STATUSES[:resolved] } + with_ended_at + end + + trait :ignored do + status { AlertManagement::Alert::STATUSES[:ignored] } + without_ended_at end trait :all_fields do @@ -41,7 +56,6 @@ FactoryBot.define do with_service with_monitoring_tool with_host - with_ended_at end end end diff --git a/spec/finders/alert_management/alerts_finder_spec.rb b/spec/finders/alert_management/alerts_finder_spec.rb index c25585a5da5..8ce5216ba9a 100644 --- a/spec/finders/alert_management/alerts_finder_spec.rb +++ b/spec/finders/alert_management/alerts_finder_spec.rb @@ -5,8 +5,8 @@ require 'spec_helper' describe AlertManagement::AlertsFinder, '#execute' do let_it_be(:current_user) { create(:user) } let_it_be(:project) { create(:project) } - let_it_be(:alert_1) { create(:alert_management_alert, project: project, ended_at: 1.year.ago, events: 2, severity: :high, status: :resolved) } - let_it_be(:alert_2) { create(:alert_management_alert, project: project, events: 1, severity: :critical, status: :ignored) } + let_it_be(:alert_1) { create(:alert_management_alert, :resolved, project: project, ended_at: 1.year.ago, events: 2, severity: :high) } + let_it_be(:alert_2) { create(:alert_management_alert, :ignored, project: project, events: 1, severity: :critical) } let_it_be(:alert_3) { create(:alert_management_alert) } let(:params) { {} } @@ -155,10 +155,10 @@ describe AlertManagement::AlertsFinder, '#execute' do end context 'when sorting by status' do - let_it_be(:alert_triggered) { create(:alert_management_alert, project: project, status: :triggered) } - let_it_be(:alert_acknowledged) { create(:alert_management_alert, project: project, status: :acknowledged) } - let_it_be(:alert_resolved) { create(:alert_management_alert, project: project, status: :resolved) } - let_it_be(:alert_ignored) { create(:alert_management_alert, project: project, status: :ignored) } + let_it_be(:alert_triggered) { create(:alert_management_alert, project: project) } + let_it_be(:alert_acknowledged) { create(:alert_management_alert, :acknowledged, project: project) } + let_it_be(:alert_resolved) { create(:alert_management_alert, :resolved, project: project) } + let_it_be(:alert_ignored) { create(:alert_management_alert, :ignored, project: project) } context 'sorts alerts ascending' do let(:params) { { sort: 'status_asc' } } diff --git a/spec/frontend/alert_management/components/alert_management_list_spec.js b/spec/frontend/alert_management/components/alert_management_list_spec.js index c0494d736a6..cb2531663bd 100644 --- a/spec/frontend/alert_management/components/alert_management_list_spec.js +++ b/spec/frontend/alert_management/components/alert_management_list_spec.js @@ -1,6 +1,16 @@ import { mount } from '@vue/test-utils'; -import { GlEmptyState, GlTable, GlAlert, GlLoadingIcon, GlNewDropdown, GlIcon } from '@gitlab/ui'; +import { + GlEmptyState, + GlTable, + GlAlert, + GlLoadingIcon, + GlNewDropdown, + GlBadge, + GlIcon, + GlTab, +} from '@gitlab/ui'; import AlertManagementList from '~/alert_management/components/alert_management_list.vue'; +import { ALERTS_STATUS_TABS } from '../../../../app/assets/javascripts/alert_management/constants'; import mockAlerts from '../mocks/alerts.json'; @@ -12,6 +22,8 @@ describe('AlertManagementList', () => { const findAlert = () => wrapper.find(GlAlert); const findLoader = () => wrapper.find(GlLoadingIcon); const findStatusDropdown = () => wrapper.find(GlNewDropdown); + const findStatusFilterTabs = () => wrapper.findAll(GlTab); + const findNumberOfAlertsBadge = () => wrapper.findAll(GlBadge); function mountComponent({ props = { @@ -20,6 +32,8 @@ describe('AlertManagementList', () => { }, data = {}, loading = false, + alertListStatusFilteringEnabled = false, + stubs = {}, } = {}) { wrapper = mount(AlertManagementList, { propsData: { @@ -28,6 +42,9 @@ describe('AlertManagementList', () => { emptyAlertSvgPath: 'illustration/path', ...props, }, + provide: { + glFeatures: { alertListStatusFilteringEnabled }, + }, data() { return data; }, @@ -40,6 +57,7 @@ describe('AlertManagementList', () => { }, }, }, + stubs, }); } @@ -59,6 +77,56 @@ describe('AlertManagementList', () => { }); }); + describe('Status Filter Tabs', () => { + describe('alertListStatusFilteringEnabled feature flag enabled', () => { + beforeEach(() => { + mountComponent({ + props: { alertManagementEnabled: true, userCanEnableAlertManagement: true }, + data: { alerts: mockAlerts }, + loading: false, + alertListStatusFilteringEnabled: true, + stubs: { + GlTab: true, + }, + }); + }); + + it('should display filter tabs for all statuses', () => { + const tabs = findStatusFilterTabs().wrappers; + tabs.forEach((tab, i) => { + expect(tab.text()).toContain(ALERTS_STATUS_TABS[i].title); + }); + }); + + it('should have number of items badge along with status tab', () => { + expect(findNumberOfAlertsBadge().length).toEqual(ALERTS_STATUS_TABS.length); + expect( + findNumberOfAlertsBadge() + .at(0) + .text(), + ).toEqual(`${mockAlerts.length}`); + }); + }); + + describe('alertListStatusFilteringEnabled feature flag disabled', () => { + beforeEach(() => { + mountComponent({ + props: { alertManagementEnabled: true, userCanEnableAlertManagement: true }, + data: { alerts: mockAlerts }, + loading: false, + alertListStatusFilteringEnabled: false, + stubs: { + GlTab: true, + }, + }); + }); + + it('should NOT display tabs', () => { + expect(findStatusFilterTabs()).not.toExist(); + }); + }); + }); + describe('Alerts table', () => { it('loading state', () => { mountComponent({ diff --git a/spec/frontend/code_navigation/store/actions_spec.js b/spec/frontend/code_navigation/store/actions_spec.js index 3996e8ea1f5..4cf77ed1be5 100644 --- a/spec/frontend/code_navigation/store/actions_spec.js +++ b/spec/frontend/code_navigation/store/actions_spec.js @@ -143,6 +143,16 @@ describe('Code navigation actions', () => { expect(addInteractionClass.mock.calls[0]).toEqual(['index.js', 'test']); expect(addInteractionClass.mock.calls[1]).toEqual(['index.js', 'console.log']); }); + + it('does not call addInteractionClass when no data exists', () => { + const state = { + data: null, + }; + + actions.showBlobInteractionZones({ state }, 'index.js'); + + expect(addInteractionClass).not.toHaveBeenCalled(); + }); }); describe('showDefinition', () => { diff --git a/spec/frontend/design_management/components/design_notes/__snapshots__/design_note_spec.js.snap b/spec/frontend/design_management/components/design_notes/__snapshots__/design_note_spec.js.snap index c6d8f9fe174..84e52e1099a 100644 --- a/spec/frontend/design_management/components/design_notes/__snapshots__/design_note_spec.js.snap +++ b/spec/frontend/design_management/components/design_notes/__snapshots__/design_note_spec.js.snap @@ -3,7 +3,7 @@ exports[`Design note component should match the snapshot 1`] = ` - - + + + + + + + + + @ + + + + + + + + + + + +
diff --git a/spec/frontend/design_management/components/design_notes/__snapshots__/design_reply_form_spec.js.snap b/spec/frontend/design_management/components/design_notes/__snapshots__/design_reply_form_spec.js.snap new file mode 100644 index 00000000000..e01c79e3520 --- /dev/null +++ b/spec/frontend/design_management/components/design_notes/__snapshots__/design_reply_form_spec.js.snap @@ -0,0 +1,15 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Design reply form component renders button text as "Comment" when creating a comment 1`] = ` +"" +`; + +exports[`Design reply form component renders button text as "Save comment" when creating a comment 1`] = ` +"" +`; diff --git a/spec/frontend/design_management/components/design_notes/design_note_spec.js b/spec/frontend/design_management/components/design_notes/design_note_spec.js index 4e5b7a66611..228ee9df00e 100644 --- a/spec/frontend/design_management/components/design_notes/design_note_spec.js +++ b/spec/frontend/design_management/components/design_notes/design_note_spec.js @@ -1,29 +1,54 @@ import { shallowMount } from '@vue/test-utils'; +import { ApolloMutation } from 'vue-apollo'; import DesignNote from '~/design_management/components/design_notes/design_note.vue'; import UserAvatarLink from '~/vue_shared/components/user_avatar/user_avatar_link.vue'; import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; +import DesignReplyForm from '~/design_management/components/design_notes/design_reply_form.vue'; const scrollIntoViewMock = jest.fn(); +const note = { + id: 'gid://gitlab/DiffNote/123', + author: { + id: 'author-id', + }, + body: 'test', +}; HTMLElement.prototype.scrollIntoView = scrollIntoViewMock; const $route = { hash: '#note_123', }; +const mutate = jest.fn().mockResolvedValue({ data: { updateNote: {} } }); + describe('Design note component', () => { let wrapper; const findUserAvatar = () => wrapper.find(UserAvatarLink); const findUserLink = () => wrapper.find('.js-user-link'); + const findReplyForm = () => wrapper.find(DesignReplyForm); + const findEditButton = () => wrapper.find('.js-note-edit'); + const findNoteContent = () => wrapper.find('.js-note-text'); - function createComponent(props = {}) { + function createComponent(props = {}, data = { isEditing: false }) { wrapper = shallowMount(DesignNote, { propsData: { note: {}, ...props, }, + data() { + return { + ...data, + }; + }, mocks: { $route, + $apollo: { + mutate, + }, + }, + stubs: { + ApolloMutation, }, }); } @@ -34,13 +59,7 @@ describe('Design note component', () => { it('should match the snapshot', () => { createComponent({ - note: { - id: '1', - createdAt: '2019-07-26T15:02:20Z', - author: { - id: 'author-id', - }, - }, + note, }); expect(wrapper.element).toMatchSnapshot(); @@ -48,12 +67,7 @@ describe('Design note component', () => { it('should render an author', () => { createComponent({ - note: { - id: '1', - author: { - id: 'author-id', - }, - }, + note, }); expect(findUserAvatar().exists()).toBe(true); @@ -63,11 +77,8 @@ describe('Design note component', () => { it('should render a time ago tooltip if note has createdAt property', () => { createComponent({ note: { - id: '1', + ...note, createdAt: '2019-07-26T15:02:20Z', - author: { - id: 'author-id', - }, }, }); @@ -76,14 +87,61 @@ describe('Design note component', () => { it('should trigger a scrollIntoView method', () => { createComponent({ - note: { - id: 'gid://gitlab/DiffNote/123', - author: { - id: 'author-id', - }, - }, + note, }); expect(scrollIntoViewMock).toHaveBeenCalled(); }); + + it('should open an edit form on edit button click', () => { + createComponent({ + note, + }); + + findEditButton().trigger('click'); + + return wrapper.vm.$nextTick().then(() => { + expect(findReplyForm().exists()).toBe(true); + expect(findNoteContent().exists()).toBe(false); + }); + }); + + describe('when edit form is rendered', () => { + beforeEach(() => { + createComponent( + { + note, + }, + { isEditing: true }, + ); + }); + + it('should not render note content and should render reply form', () => { + expect(findNoteContent().exists()).toBe(false); + expect(findReplyForm().exists()).toBe(true); + }); + + it('hides the form on hideForm event', () => { + findReplyForm().vm.$emit('cancelForm'); + + return wrapper.vm.$nextTick().then(() => { + expect(findReplyForm().exists()).toBe(false); + expect(findNoteContent().exists()).toBe(true); + }); + }); + + it('calls a mutation on submitForm event and hides a form', () => { + findReplyForm().vm.$emit('submitForm'); + expect(mutate).toHaveBeenCalled(); + + return mutate() + .then(() => { + return wrapper.vm.$nextTick(); + }) + .then(() => { + expect(findReplyForm().exists()).toBe(false); + expect(findNoteContent().exists()).toBe(true); + }); + }); + }); }); diff --git a/spec/frontend/design_management/components/design_notes/design_reply_form_spec.js b/spec/frontend/design_management/components/design_notes/design_reply_form_spec.js index 0780a9017f4..34b8f1f9fa8 100644 --- a/spec/frontend/design_management/components/design_notes/design_reply_form_spec.js +++ b/spec/frontend/design_management/components/design_notes/design_reply_form_spec.js @@ -1,6 +1,15 @@ import { mount } from '@vue/test-utils'; import DesignReplyForm from '~/design_management/components/design_notes/design_reply_form.vue'; +const showModal = jest.fn(); + +const GlModal = { + template: '
', + methods: { + show: showModal, + }, +}; + describe('Design reply form component', () => { let wrapper; @@ -16,6 +25,7 @@ describe('Design reply form component', () => { isSaving: false, ...props, }, + stubs: { GlModal }, }); } @@ -29,6 +39,18 @@ describe('Design reply form component', () => { expect(findTextarea().element).toEqual(document.activeElement); }); + it('renders button text as "Comment" when creating a comment', () => { + createComponent(); + + expect(findSubmitButton().html()).toMatchSnapshot(); + }); + + it('renders button text as "Save comment" when creating a comment', () => { + createComponent({ isNewComment: false }); + + expect(findSubmitButton().html()).toMatchSnapshot(); + }); + describe('when form has no text', () => { beforeEach(() => { createComponent({ @@ -120,16 +142,34 @@ describe('Design reply form component', () => { }); }); - it('opens confirmation modal on pressing Escape button', () => { + it('emits cancelForm event on Escape key if text was not changed', () => { findTextarea().trigger('keyup.esc'); - expect(findModal().exists()).toBe(true); + expect(wrapper.emitted('cancelForm')).toBeTruthy(); }); - it('opens confirmation modal on Cancel button click', () => { - findCancelButton().vm.$emit('click'); + it('opens confirmation modal on Escape key when text has changed', () => { + wrapper.setProps({ value: 'test2' }); - expect(findModal().exists()).toBe(true); + return wrapper.vm.$nextTick().then(() => { + findTextarea().trigger('keyup.esc'); + expect(showModal).toHaveBeenCalled(); + }); + }); + + it('emits cancelForm event on Cancel button click if text was not changed', () => { + findCancelButton().trigger('click'); + + expect(wrapper.emitted('cancelForm')).toBeTruthy(); + }); + + it('opens confirmation modal on Cancel button click when text has changed', () => { + wrapper.setProps({ value: 'test2' }); + + return wrapper.vm.$nextTick().then(() => { + findCancelButton().trigger('click'); + expect(showModal).toHaveBeenCalled(); + }); }); it('emits cancelForm event on modal Ok button click', () => { diff --git a/spec/javascripts/editor/editor_lite_spec.js b/spec/frontend/editor/editor_lite_spec.js similarity index 78% rename from spec/javascripts/editor/editor_lite_spec.js rename to spec/frontend/editor/editor_lite_spec.js index 106264aa13f..cb07bcf8f28 100644 --- a/spec/javascripts/editor/editor_lite_spec.js +++ b/spec/frontend/editor/editor_lite_spec.js @@ -1,4 +1,4 @@ -import { editor as monacoEditor, Uri } from 'monaco-editor'; +import { editor as monacoEditor, languages as monacoLanguages, Uri } from 'monaco-editor'; import Editor from '~/editor/editor_lite'; import { DEFAULT_THEME, themes } from '~/ide/lib/themes'; @@ -41,13 +41,13 @@ describe('Base editor', () => { let dispose; beforeEach(() => { - setModel = jasmine.createSpy(); - dispose = jasmine.createSpy(); - modelSpy = spyOn(monacoEditor, 'createModel').and.returnValue(fakeModel); - instanceSpy = spyOn(monacoEditor, 'create').and.returnValue({ + setModel = jest.fn(); + dispose = jest.fn(); + modelSpy = jest.spyOn(monacoEditor, 'createModel').mockImplementation(() => fakeModel); + instanceSpy = jest.spyOn(monacoEditor, 'create').mockImplementation(() => ({ setModel, dispose, - }); + })); }); it('does nothing if no dom element is supplied', () => { @@ -73,7 +73,7 @@ describe('Base editor', () => { editor.createInstance({ el: editorEl }); expect(editor.editorEl).not.toBe(null); - expect(instanceSpy).toHaveBeenCalledWith(editorEl, jasmine.anything()); + expect(instanceSpy).toHaveBeenCalledWith(editorEl, expect.anything()); }); }); @@ -91,6 +91,11 @@ describe('Base editor', () => { }); it('is capable of changing the language of the model', () => { + // ignore warnings and errors Monaco posts during setup + // (due to being called from Jest/Node.js environment) + jest.spyOn(console, 'warn').mockImplementation(() => {}); + jest.spyOn(console, 'error').mockImplementation(() => {}); + const blobRenamedPath = 'test.js'; expect(editor.model.getLanguageIdentifier().language).toEqual('markdown'); @@ -101,7 +106,7 @@ describe('Base editor', () => { it('falls back to plaintext if there is no language associated with an extension', () => { const blobRenamedPath = 'test.myext'; - const spy = spyOn(console, 'error'); + const spy = jest.spyOn(console, 'error').mockImplementation(() => {}); editor.updateModelLanguage(blobRenamedPath); @@ -110,14 +115,26 @@ describe('Base editor', () => { }); }); + describe('languages', () => { + it('registers custom languages defined with Monaco', () => { + expect(monacoLanguages.getLanguages()).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + id: 'vue', + }), + ]), + ); + }); + }); + describe('syntax highlighting theme', () => { let themeDefineSpy; let themeSetSpy; let defaultScheme; beforeEach(() => { - themeDefineSpy = spyOn(monacoEditor, 'defineTheme'); - themeSetSpy = spyOn(monacoEditor, 'setTheme'); + themeDefineSpy = jest.spyOn(monacoEditor, 'defineTheme').mockImplementation(() => {}); + themeSetSpy = jest.spyOn(monacoEditor, 'setTheme').mockImplementation(() => {}); defaultScheme = window.gon.user_color_scheme; }); diff --git a/spec/frontend/ide/lib/editor_spec.js b/spec/frontend/ide/lib/editor_spec.js index 78e7bf5b58a..36d4c3c26ee 100644 --- a/spec/frontend/ide/lib/editor_spec.js +++ b/spec/frontend/ide/lib/editor_spec.js @@ -1,4 +1,4 @@ -import { editor as monacoEditor } from 'monaco-editor'; +import { editor as monacoEditor, languages as monacoLanguages } from 'monaco-editor'; import Editor from '~/ide/lib/editor'; import { defaultEditorOptions } from '~/ide/lib/editor_options'; import { file } from '../helpers'; @@ -181,6 +181,18 @@ describe('Multi-file editor library', () => { }); }); + describe('languages', () => { + it('registers custom languages defined with Monaco', () => { + expect(monacoLanguages.getLanguages()).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + id: 'vue', + }), + ]), + ); + }); + }); + describe('dispose', () => { it('calls disposble dispose method', () => { jest.spyOn(instance.disposable, 'dispose'); diff --git a/spec/frontend/ide/lib/languages/vue_spec.js b/spec/frontend/ide/lib/languages/vue_spec.js new file mode 100644 index 00000000000..3d8784c1436 --- /dev/null +++ b/spec/frontend/ide/lib/languages/vue_spec.js @@ -0,0 +1,92 @@ +import { editor } from 'monaco-editor'; +import { registerLanguages } from '~/ide/utils'; +import vue from '~/ide/lib/languages/vue'; + +// This file only tests syntax specific to vue. This does not test existing syntaxes +// of html, javascript, css and handlebars, which vue files extend. +describe('tokenization for .vue files', () => { + beforeEach(() => { + registerLanguages(vue); + }); + + test.each([ + [ + '
content
', + [ + [ + { language: 'vue', offset: 0, type: 'delimiter.html' }, + { language: 'vue', offset: 1, type: 'tag.html' }, + { language: 'vue', offset: 4, type: '' }, + { language: 'vue', offset: 5, type: 'variable' }, + { language: 'vue', offset: 21, type: 'delimiter.html' }, + { language: 'vue', offset: 22, type: '' }, + { language: 'vue', offset: 29, type: 'delimiter.html' }, + { language: 'vue', offset: 31, type: 'tag.html' }, + { language: 'vue', offset: 34, type: 'delimiter.html' }, + ], + ], + ], + [ + '', + [ + [ + { language: 'vue', offset: 0, type: 'delimiter.html' }, + { language: 'vue', offset: 1, type: 'tag.html' }, + { language: 'vue', offset: 6, type: '' }, + { language: 'vue', offset: 7, type: 'variable' }, + { language: 'vue', offset: 33, type: 'delimiter.html' }, + ], + ], + ], + [ + '', + [ + [ + { language: 'vue', offset: 0, type: 'delimiter.html' }, + { language: 'vue', offset: 1, type: 'tag.html' }, + { language: 'vue', offset: 3, type: 'attribute.name' }, + { language: 'vue', offset: 9, type: '' }, + { language: 'vue', offset: 10, type: 'variable' }, + { language: 'vue', offset: 28, type: 'delimiter.html' }, + { language: 'vue', offset: 31, type: 'tag.html' }, + { language: 'vue', offset: 33, type: 'attribute.name' }, + { language: 'vue', offset: 39, type: 'delimiter.html' }, + ], + ], + ], + [ + '...', + [ + [ + { language: 'vue', offset: 0, type: 'delimiter.html' }, + { language: 'vue', offset: 1, type: 'tag.html' }, + { language: 'vue', offset: 2, type: '' }, + { language: 'vue', offset: 3, type: 'variable' }, + { language: 'vue', offset: 32, type: 'delimiter.html' }, + { language: 'vue', offset: 33, type: '' }, + { language: 'vue', offset: 36, type: 'delimiter.html' }, + { language: 'vue', offset: 38, type: 'tag.html' }, + { language: 'vue', offset: 39, type: 'delimiter.html' }, + ], + ], + ], + [ + '...', + [ + [ + { language: 'vue', offset: 0, type: 'delimiter.html' }, + { language: 'vue', offset: 1, type: 'tag.html' }, + { language: 'vue', offset: 2, type: '' }, + { language: 'vue', offset: 3, type: 'variable' }, + { language: 'vue', offset: 25, type: 'delimiter.html' }, + { language: 'vue', offset: 26, type: '' }, + { language: 'vue', offset: 29, type: 'delimiter.html' }, + { language: 'vue', offset: 31, type: 'tag.html' }, + { language: 'vue', offset: 32, type: 'delimiter.html' }, + ], + ], + ], + ])('%s', (string, tokens) => { + expect(editor.tokenize(string, 'vue')).toEqual(tokens); + }); +}); diff --git a/spec/frontend/ide/utils_spec.js b/spec/frontend/ide/utils_spec.js index 44eae7eacbe..10a31842dd4 100644 --- a/spec/frontend/ide/utils_spec.js +++ b/spec/frontend/ide/utils_spec.js @@ -1,6 +1,7 @@ import { commitItemIconMap } from '~/ide/constants'; -import { getCommitIconMap, isTextFile } from '~/ide/utils'; +import { getCommitIconMap, isTextFile, registerLanguages } from '~/ide/utils'; import { decorateData } from '~/ide/stores/utils'; +import { languages } from 'monaco-editor'; describe('WebIDE utils', () => { describe('isTextFile', () => { @@ -102,4 +103,78 @@ describe('WebIDE utils', () => { expect(getCommitIconMap(entry)).toEqual(commitItemIconMap.modified); }); }); + + describe('registerLanguages', () => { + let langs; + + beforeEach(() => { + langs = [ + { + id: 'html', + extensions: ['.html'], + conf: { comments: { blockComment: [''] } }, + language: { tokenizer: {} }, + }, + { + id: 'css', + extensions: ['.css'], + conf: { comments: { blockComment: ['/*', '*/'] } }, + language: { tokenizer: {} }, + }, + { + id: 'js', + extensions: ['.js'], + conf: { comments: { blockComment: ['/*', '*/'] } }, + language: { tokenizer: {} }, + }, + ]; + + jest.spyOn(languages, 'register').mockImplementation(() => {}); + jest.spyOn(languages, 'setMonarchTokensProvider').mockImplementation(() => {}); + jest.spyOn(languages, 'setLanguageConfiguration').mockImplementation(() => {}); + }); + + it('registers all the passed languages with Monaco', () => { + registerLanguages(...langs); + + expect(languages.register.mock.calls).toEqual([ + [ + { + conf: { comments: { blockComment: ['/*', '*/'] } }, + extensions: ['.css'], + id: 'css', + language: { tokenizer: {} }, + }, + ], + [ + { + conf: { comments: { blockComment: ['/*', '*/'] } }, + extensions: ['.js'], + id: 'js', + language: { tokenizer: {} }, + }, + ], + [ + { + conf: { comments: { blockComment: [''] } }, + extensions: ['.html'], + id: 'html', + language: { tokenizer: {} }, + }, + ], + ]); + + expect(languages.setMonarchTokensProvider.mock.calls).toEqual([ + ['css', { tokenizer: {} }], + ['js', { tokenizer: {} }], + ['html', { tokenizer: {} }], + ]); + + expect(languages.setLanguageConfiguration.mock.calls).toEqual([ + ['css', { comments: { blockComment: ['/*', '*/'] } }], + ['js', { comments: { blockComment: ['/*', '*/'] } }], + ['html', { comments: { blockComment: [''] } }], + ]); + }); + }); }); diff --git a/spec/frontend/monitoring/components/dashboard_panel_spec.js b/spec/frontend/monitoring/components/dashboard_panel_spec.js index ccc29623c31..fb7fb24a341 100644 --- a/spec/frontend/monitoring/components/dashboard_panel_spec.js +++ b/spec/frontend/monitoring/components/dashboard_panel_spec.js @@ -376,10 +376,6 @@ describe('Dashboard Panel', () => { }); }); - afterEach(() => { - wrapper.destroy(); - }); - it('sets clipboard text on the dropdown', () => { expect(findCopyLink().exists()).toBe(true); expect(findCopyLink().element.dataset.clipboardText).toBe(clipboardText); @@ -396,6 +392,18 @@ describe('Dashboard Panel', () => { }); }); + describe('when cliboard data is not available', () => { + it('there is no "copy to clipboard" link for a null value', () => { + createWrapper({ clipboardText: null }); + expect(findCopyLink().exists()).toBe(false); + }); + + it('there is no "copy to clipboard" link for an empty value', () => { + createWrapper({ clipboardText: '' }); + expect(findCopyLink().exists()).toBe(false); + }); + }); + describe('when downloading metrics data as CSV', () => { beforeEach(() => { wrapper = shallowMount(DashboardPanel, { diff --git a/spec/frontend/monitoring/components/dashboard_spec.js b/spec/frontend/monitoring/components/dashboard_spec.js index 24883e9055e..12cb2c588dd 100644 --- a/spec/frontend/monitoring/components/dashboard_spec.js +++ b/spec/frontend/monitoring/components/dashboard_spec.js @@ -2,6 +2,7 @@ import { shallowMount, mount } from '@vue/test-utils'; import Tracking from '~/tracking'; import { ESC_KEY, ESC_KEY_IE11 } from '~/lib/utils/keys'; import { GlModal, GlDropdownItem, GlDeprecatedButton } from '@gitlab/ui'; +import { objectToQuery } from '~/lib/utils/url_utility'; import VueDraggable from 'vuedraggable'; import MockAdapter from 'axios-mock-adapter'; import axios from '~/lib/utils/axios_utils'; @@ -20,6 +21,9 @@ import * as types from '~/monitoring/stores/mutation_types'; import { setupStoreWithDashboard, setMetricResult, setupStoreWithData } from '../store_utils'; import { environmentData, dashboardGitResponse, propsData } from '../mock_data'; import { metricsDashboardViewModel, metricsDashboardPanelCount } from '../fixture_data'; +import createFlash from '~/flash'; + +jest.mock('~/flash'); describe('Dashboard', () => { let store; @@ -64,7 +68,6 @@ describe('Dashboard', () => { describe('no metrics are available yet', () => { beforeEach(() => { - jest.spyOn(store, 'dispatch'); createShallowWrapper(); }); @@ -150,6 +153,87 @@ describe('Dashboard', () => { }); }); + describe('when the URL contains a reference to a panel', () => { + let location; + + const setSearch = search => { + window.location = { ...location, search }; + }; + + beforeEach(() => { + location = window.location; + delete window.location; + }); + + afterEach(() => { + window.location = location; + }); + + it('when the URL points to a panel it expands', () => { + const panelGroup = metricsDashboardViewModel.panelGroups[0]; + const panel = panelGroup.panels[0]; + + setSearch( + objectToQuery({ + group: panelGroup.group, + title: panel.title, + y_label: panel.y_label, + }), + ); + + createMountedWrapper({ hasMetrics: true }); + setupStoreWithData(wrapper.vm.$store); + + return wrapper.vm.$nextTick().then(() => { + expect(store.dispatch).toHaveBeenCalledWith('monitoringDashboard/setExpandedPanel', { + group: panelGroup.group, + panel: expect.objectContaining({ + title: panel.title, + y_label: panel.y_label, + }), + }); + }); + }); + + it('when the URL does not link to any panel, no panel is expanded', () => { + setSearch(''); + + createMountedWrapper({ hasMetrics: true }); + setupStoreWithData(wrapper.vm.$store); + + return wrapper.vm.$nextTick().then(() => { + expect(store.dispatch).not.toHaveBeenCalledWith( + 'monitoringDashboard/setExpandedPanel', + expect.anything(), + ); + }); + }); + + it('when the URL points to an incorrect panel it shows an error', () => { + const panelGroup = metricsDashboardViewModel.panelGroups[0]; + const panel = panelGroup.panels[0]; + + setSearch( + objectToQuery({ + group: panelGroup.group, + title: 'incorrect', + y_label: panel.y_label, + }), + ); + + createMountedWrapper({ hasMetrics: true }); + setupStoreWithData(wrapper.vm.$store); + + return wrapper.vm.$nextTick().then(() => { + expect(createFlash).toHaveBeenCalled(); + expect(store.dispatch).not.toHaveBeenCalledWith( + 'monitoringDashboard/setExpandedPanel', + expect.anything(), + ); + }); + }); + }); + describe('when all requests have been commited by the store', () => { beforeEach(() => { createMountedWrapper({ hasMetrics: true }); diff --git a/spec/frontend/monitoring/store/actions_spec.js b/spec/frontend/monitoring/store/actions_spec.js index 9039dd3d6db..fd8fb4c6418 100644 --- a/spec/frontend/monitoring/store/actions_spec.js +++ b/spec/frontend/monitoring/store/actions_spec.js @@ -11,6 +11,7 @@ import { ENVIRONMENT_AVAILABLE_STATE } from '~/monitoring/constants'; import store from '~/monitoring/stores'; import * as types from '~/monitoring/stores/mutation_types'; import { + fetchData, fetchDashboard, receiveMetricsDashboardSuccess, fetchDeploymentsData, @@ -86,6 +87,41 @@ describe('Monitoring store actions', () => { createFlash.mockReset(); }); + describe('fetchData', () => { + it('dispatches fetchEnvironmentsData and fetchEnvironmentsData', () => { + const { state } = store; + + return testAction( + fetchData, + null, + state, + [], + [{ type: 'fetchEnvironmentsData' }, { type: 'fetchDashboard' }], + ); + }); + + it('dispatches when feature metricsDashboardAnnotations is on', () => { + const origGon = window.gon; + window.gon = { features: { metricsDashboardAnnotations: true } }; + + const { state } = store; + + return testAction( + fetchData, + null, + state, + [], + [ + { type: 'fetchEnvironmentsData' }, + { type: 'fetchDashboard' }, + { type: 'fetchAnnotations' }, + ], + ).then(() => { + window.gon = origGon; + }); + }); + }); + describe('fetchDeploymentsData', () => { it('dispatches receiveDeploymentsDataSuccess on success', () => { const { state } = store; diff --git a/spec/frontend/monitoring/utils_spec.js b/spec/frontend/monitoring/utils_spec.js index 964c462988c..639330446be 100644 --- a/spec/frontend/monitoring/utils_spec.js +++ b/spec/frontend/monitoring/utils_spec.js @@ -1,5 +1,5 @@ import * as monitoringUtils from '~/monitoring/utils'; -import { queryToObject, mergeUrlParams, removeParams } from '~/lib/utils/url_utility'; +import * as urlUtils from '~/lib/utils/url_utility'; import { TEST_HOST } from 'jest/helpers/test_constants'; import { mockProjectDir, @@ -7,9 +7,7 @@ import { anomalyMockGraphData, barMockData, } from './mock_data'; -import { graphData } from './fixture_data'; - -jest.mock('~/lib/utils/url_utility'); +import { metricsDashboardViewModel, graphData } from './fixture_data'; const mockPath = `${TEST_HOST}${mockProjectDir}/-/environments/29/metrics`; @@ -27,11 +25,6 @@ const rollingRange = { }; describe('monitoring/utils', () => { - afterEach(() => { - mergeUrlParams.mockReset(); - queryToObject.mockReset(); - }); - describe('trackGenerateLinkToChartEventOptions', () => { it('should return Cluster Monitoring options if located on Cluster Health Dashboard', () => { document.body.dataset.page = 'groups:clusters:show'; @@ -139,18 +132,25 @@ describe('monitoring/utils', () => { }); describe('timeRangeFromUrl', () => { + beforeEach(() => { + jest.spyOn(urlUtils, 'queryToObject'); + }); + + afterEach(() => { + urlUtils.queryToObject.mockRestore(); + }); + const { timeRangeFromUrl } = monitoringUtils; - it('returns a fixed range when query contains `start` and `end` paramters are given', () => { - queryToObject.mockReturnValueOnce(range); - + it('returns a fixed range when query contains `start` and `end` parameters are given', () => { + urlUtils.queryToObject.mockReturnValueOnce(range); expect(timeRangeFromUrl()).toEqual(range); }); - it('returns a rolling range when query contains `duration_seconds` paramters are given', () => { + it('returns a rolling range when query contains `duration_seconds` parameters are given', () => { const { seconds } = rollingRange.duration; - queryToObject.mockReturnValueOnce({ + urlUtils.queryToObject.mockReturnValueOnce({ dashboard: '.gitlab/dashboard/my_dashboard.yml', duration_seconds: `${seconds}`, }); @@ -158,23 +158,21 @@ describe('monitoring/utils', () => { expect(timeRangeFromUrl()).toEqual(rollingRange); }); - it('returns null when no time range paramters are given', () => { - const params = { + it('returns null when no time range parameters are given', () => { + urlUtils.queryToObject.mockReturnValueOnce({ dashboard: '.gitlab/dashboards/custom_dashboard.yml', param1: 'value1', param2: 'value2', - }; + }); - expect(timeRangeFromUrl(params, mockPath)).toBe(null); + expect(timeRangeFromUrl()).toBe(null); }); }); describe('removeTimeRangeParams', () => { const { removeTimeRangeParams } = monitoringUtils; - it('returns when query contains `start` and `end` paramters are given', () => { - removeParams.mockReturnValueOnce(mockPath); - + it('returns when query contains `start` and `end` parameters are given', () => { expect(removeTimeRangeParams(`${mockPath}?start=${range.start}&end=${range.end}`)).toEqual( mockPath, ); @@ -184,28 +182,116 @@ describe('monitoring/utils', () => { describe('timeRangeToUrl', () => { const { timeRangeToUrl } = monitoringUtils; - it('returns a fixed range when query contains `start` and `end` paramters are given', () => { + beforeEach(() => { + jest.spyOn(urlUtils, 'mergeUrlParams'); + jest.spyOn(urlUtils, 'removeParams'); + }); + + afterEach(() => { + urlUtils.mergeUrlParams.mockRestore(); + urlUtils.removeParams.mockRestore(); + }); + + it('returns a fixed range when query contains `start` and `end` parameters are given', () => { const toUrl = `${mockPath}?start=${range.start}&end=${range.end}`; const fromUrl = mockPath; - removeParams.mockReturnValueOnce(fromUrl); - mergeUrlParams.mockReturnValueOnce(toUrl); + urlUtils.removeParams.mockReturnValueOnce(fromUrl); + urlUtils.mergeUrlParams.mockReturnValueOnce(toUrl); expect(timeRangeToUrl(range)).toEqual(toUrl); - expect(mergeUrlParams).toHaveBeenCalledWith(range, fromUrl); + expect(urlUtils.mergeUrlParams).toHaveBeenCalledWith(range, fromUrl); }); - it('returns a rolling range when query contains `duration_seconds` paramters are given', () => { + it('returns a rolling range when query contains `duration_seconds` parameters are given', () => { const { seconds } = rollingRange.duration; const toUrl = `${mockPath}?duration_seconds=${seconds}`; const fromUrl = mockPath; - removeParams.mockReturnValueOnce(fromUrl); - mergeUrlParams.mockReturnValueOnce(toUrl); + urlUtils.removeParams.mockReturnValueOnce(fromUrl); + urlUtils.mergeUrlParams.mockReturnValueOnce(toUrl); expect(timeRangeToUrl(rollingRange)).toEqual(toUrl); - expect(mergeUrlParams).toHaveBeenCalledWith({ duration_seconds: `${seconds}` }, fromUrl); + expect(urlUtils.mergeUrlParams).toHaveBeenCalledWith( + { duration_seconds: `${seconds}` }, + fromUrl, + ); + }); + }); + + describe('expandedPanelPayloadFromUrl', () => { + const { expandedPanelPayloadFromUrl } = monitoringUtils; + const [panelGroup] = metricsDashboardViewModel.panelGroups; + const [panel] = panelGroup.panels; + + const { group } = panelGroup; + const { title, y_label: yLabel } = panel; + + it('returns payload for a panel when query parameters are given', () => { + const search = `?group=${group}&title=${title}&y_label=${yLabel}`; + + expect(expandedPanelPayloadFromUrl(metricsDashboardViewModel, search)).toEqual({ + group: panelGroup.group, + panel, + }); + }); + + it('returns null when no parameters are given', () => { + expect(expandedPanelPayloadFromUrl(metricsDashboardViewModel, '')).toBe(null); + }); + + it('throws an error when no group is provided', () => { + const search = `?title=${panel.title}&y_label=${yLabel}`; + expect(() => expandedPanelPayloadFromUrl(metricsDashboardViewModel, search)).toThrow(); + }); + + it('throws an error when no title is provided', () => { + const search = `?title=${title}&y_label=${yLabel}`; + expect(() => expandedPanelPayloadFromUrl(metricsDashboardViewModel, search)).toThrow(); + }); + + it('throws an error when no y_label group is provided', () => { + const search = `?group=${group}&title=${title}`; + expect(() => expandedPanelPayloadFromUrl(metricsDashboardViewModel, search)).toThrow(); + }); + + test.each` + group | title | yLabel | missingField + ${'NOT_A_GROUP'} | ${title} | ${yLabel} | ${'group'} + ${group} | ${'NOT_A_TITLE'} | ${yLabel} | ${'title'} + ${group} | ${title} | ${'NOT_A_Y_LABEL'} | ${'y_label'} + `('throws an error when $missingField is incorrect', params => { + const search = `?group=${params.group}&title=${params.title}&y_label=${params.yLabel}`; + expect(() => expandedPanelPayloadFromUrl(metricsDashboardViewModel, search)).toThrow(); + }); + }); + + describe('panelToUrl', () => { + const { panelToUrl } = monitoringUtils; + + const dashboard = 'metrics.yml'; + const [panelGroup] = metricsDashboardViewModel.panelGroups; + const [panel] = panelGroup.panels; + + it('returns URL for a panel when query parameters are given', () => { + const [, query] = panelToUrl(dashboard, panelGroup.group, panel).split('?'); + const params = urlUtils.queryToObject(query); + + expect(params).toEqual({ + dashboard, + group: panelGroup.group, + title: panel.title, + y_label: panel.y_label, + }); + }); + + it('returns `null` if group is missing', () => { + expect(panelToUrl(dashboard, null, panel)).toBe(null); + }); + + it('returns `null` if panel is missing', () => { + expect(panelToUrl(dashboard, panelGroup.group, null)).toBe(null); }); }); diff --git a/spec/frontend/snippets/components/__snapshots__/snippet_description_view_spec.js.snap b/spec/frontend/snippets/components/__snapshots__/snippet_description_view_spec.js.snap new file mode 100644 index 00000000000..9ebc4e81baf --- /dev/null +++ b/spec/frontend/snippets/components/__snapshots__/snippet_description_view_spec.js.snap @@ -0,0 +1,16 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Snippet Description component matches the snapshot 1`] = ` + +
+

+ The property of Thor +

+
+ +`; diff --git a/spec/frontend/snippets/components/snippet_description_view_spec.js b/spec/frontend/snippets/components/snippet_description_view_spec.js new file mode 100644 index 00000000000..46467ef311e --- /dev/null +++ b/spec/frontend/snippets/components/snippet_description_view_spec.js @@ -0,0 +1,27 @@ +import SnippetDescription from '~/snippets/components/snippet_description_view.vue'; +import { shallowMount } from '@vue/test-utils'; + +describe('Snippet Description component', () => { + let wrapper; + const description = '

The property of Thor

'; + + function createComponent() { + wrapper = shallowMount(SnippetDescription, { + propsData: { + description, + }, + }); + } + + beforeEach(() => { + createComponent(); + }); + + afterEach(() => { + wrapper.destroy(); + }); + + it('matches the snapshot', () => { + expect(wrapper.element).toMatchSnapshot(); + }); +}); diff --git a/spec/frontend/snippets/components/snippet_title_spec.js b/spec/frontend/snippets/components/snippet_title_spec.js index b49b2008610..b9b60883eb3 100644 --- a/spec/frontend/snippets/components/snippet_title_spec.js +++ b/spec/frontend/snippets/components/snippet_title_spec.js @@ -1,4 +1,5 @@ import SnippetTitle from '~/snippets/components/snippet_title.vue'; +import SnippetDescription from '~/snippets/components/snippet_description_view.vue'; import { GlSprintf } from '@gitlab/ui'; import { shallowMount } from '@vue/test-utils'; @@ -36,8 +37,9 @@ describe('Snippet header component', () => { it('renders snippets title and description', () => { createComponent(); + expect(wrapper.text().trim()).toContain(title); - expect(wrapper.find('.js-snippet-description').element.innerHTML).toBe(descriptionHtml); + expect(wrapper.find(SnippetDescription).props('description')).toBe(descriptionHtml); }); it('does not render recent changes time stamp if there were no updates', () => { diff --git a/spec/frontend/vue_shared/components/blob_viewers/rich_viewer_spec.js b/spec/frontend/vue_shared/components/blob_viewers/rich_viewer_spec.js index ce3f289eb6e..5cf42ecdc1d 100644 --- a/spec/frontend/vue_shared/components/blob_viewers/rich_viewer_spec.js +++ b/spec/frontend/vue_shared/components/blob_viewers/rich_viewer_spec.js @@ -1,5 +1,6 @@ import { shallowMount } from '@vue/test-utils'; import RichViewer from '~/vue_shared/components/blob_viewers/rich_viewer.vue'; +import MarkdownFieldView from '~/vue_shared/components/markdown/field_view.vue'; import { handleBlobRichViewer } from '~/blob/viewer'; jest.mock('~/blob/viewer'); @@ -33,4 +34,8 @@ describe('Blob Rich Viewer component', () => { it('queries for advanced viewer', () => { expect(handleBlobRichViewer).toHaveBeenCalledWith(expect.anything(), defaultType); }); + + it('is using Markdown View Field', () => { + expect(wrapper.contains(MarkdownFieldView)).toBe(true); + }); }); diff --git a/spec/frontend/vue_shared/components/markdown/field_view_spec.js b/spec/frontend/vue_shared/components/markdown/field_view_spec.js new file mode 100644 index 00000000000..80cf1f655c6 --- /dev/null +++ b/spec/frontend/vue_shared/components/markdown/field_view_spec.js @@ -0,0 +1,26 @@ +import $ from 'jquery'; +import { shallowMount } from '@vue/test-utils'; + +import MarkdownFieldView from '~/vue_shared/components/markdown/field_view.vue'; + +describe('Markdown Field View component', () => { + let renderGFMSpy; + let wrapper; + + function createComponent() { + wrapper = shallowMount(MarkdownFieldView); + } + + beforeEach(() => { + renderGFMSpy = jest.spyOn($.fn, 'renderGFM'); + createComponent(); + }); + + afterEach(() => { + wrapper.destroy(); + }); + + it('processes rendering with GFM', () => { + expect(renderGFMSpy).toHaveBeenCalledTimes(1); + }); +}); diff --git a/spec/graphql/mutations/alert_management/update_alert_status_spec.rb b/spec/graphql/mutations/alert_management/update_alert_status_spec.rb index 3cd5e217571..5263de117cf 100644 --- a/spec/graphql/mutations/alert_management/update_alert_status_spec.rb +++ b/spec/graphql/mutations/alert_management/update_alert_status_spec.rb @@ -20,7 +20,7 @@ describe Mutations::AlertManagement::UpdateAlertStatus do end it 'changes the status' do - expect { resolve }.to change { alert.reload.status }.from(alert.status).to(new_status) + expect { resolve }.to change { alert.reload.acknowledged? }.to(true) end it 'returns the alert with no errors' do diff --git a/spec/graphql/resolvers/alert_management_alert_resolver_spec.rb b/spec/graphql/resolvers/alert_management_alert_resolver_spec.rb index e550c13a3d2..c85d2cbccc6 100644 --- a/spec/graphql/resolvers/alert_management_alert_resolver_spec.rb +++ b/spec/graphql/resolvers/alert_management_alert_resolver_spec.rb @@ -7,8 +7,8 @@ describe Resolvers::AlertManagementAlertResolver do let_it_be(:current_user) { create(:user) } let_it_be(:project) { create(:project) } - let_it_be(:alert_1) { create(:alert_management_alert, project: project, ended_at: 1.year.ago, events: 2, severity: :high, status: :resolved) } - let_it_be(:alert_2) { create(:alert_management_alert, project: project, events: 1, severity: :critical, status: :ignored) } + let_it_be(:alert_1) { create(:alert_management_alert, :resolved, project: project, ended_at: 1.year.ago, events: 2, severity: :high) } + let_it_be(:alert_2) { create(:alert_management_alert, :ignored, project: project, events: 1, severity: :critical) } let_it_be(:alert_other_proj) { create(:alert_management_alert) } let(:args) { {} } diff --git a/spec/graphql/types/alert_management/status_enum_spec.rb b/spec/graphql/types/alert_management/status_enum_spec.rb index 8a1dc9841b0..240d8863c97 100644 --- a/spec/graphql/types/alert_management/status_enum_spec.rb +++ b/spec/graphql/types/alert_management/status_enum_spec.rb @@ -5,7 +5,20 @@ require 'spec_helper' describe GitlabSchema.types['AlertManagementStatus'] do specify { expect(described_class.graphql_name).to eq('AlertManagementStatus') } - it 'exposes all the severity values' do - expect(described_class.values.keys).to include(*%w[TRIGGERED ACKNOWLEDGED RESOLVED IGNORED]) + describe 'statuses' do + using RSpec::Parameterized::TableSyntax + + where(:status_name, :status_value) do + 'TRIGGERED' | 0 + 'ACKNOWLEDGED' | 1 + 'RESOLVED' | 2 + 'IGNORED' | 3 + end + + with_them do + it 'exposes a status with the correct value' do + expect(described_class.values[status_name].value).to eq(status_value) + end + end end end diff --git a/spec/javascripts/vue_mr_widget/components/mr_widget_pipeline_spec.js b/spec/javascripts/vue_mr_widget/components/mr_widget_pipeline_spec.js index 5997c93105e..883c41085fa 100644 --- a/spec/javascripts/vue_mr_widget/components/mr_widget_pipeline_spec.js +++ b/spec/javascripts/vue_mr_widget/components/mr_widget_pipeline_spec.js @@ -122,6 +122,19 @@ describe('MRWidgetPipeline', () => { ); }); + it('should render CI error when no CI is provided and pipeline must succeed is turned on', () => { + vm = mountComponent(Component, { + pipeline: {}, + hasCi: false, + pipelineMustSucceed: true, + troubleshootingDocsPath: 'help', + }); + + expect(vm.$el.querySelector('.media-body').textContent.trim()).toContain( + 'No pipeline has been run for this commit.', + ); + }); + describe('with a pipeline', () => { beforeEach(() => { vm = mountComponent(Component, { diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js index d396f2d9271..4e4f99f09e7 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js @@ -18,6 +18,7 @@ const createTestMr = customConfig => { isPipelineFailed: false, isPipelinePassing: false, isMergeAllowed: true, + isApproved: true, onlyAllowMergeIfPipelineSucceeds: false, ffOnlyEnabled: false, hasCI: false, diff --git a/spec/lib/gitlab/alert_management/alert_params_spec.rb b/spec/lib/gitlab/alert_management/alert_params_spec.rb index 8c60b502417..601be961c2e 100644 --- a/spec/lib/gitlab/alert_management/alert_params_spec.rb +++ b/spec/lib/gitlab/alert_management/alert_params_spec.rb @@ -42,4 +42,43 @@ describe Gitlab::AlertManagement::AlertParams do end end end + + describe '.from_prometheus_alert' do + let(:payload) do + { + 'status' => 'firing', + 'labels' => { + 'alertname' => 'GitalyFileServerDown', + 'channel' => 'gitaly', + 'pager' => 'pagerduty', + 'severity' => 's1' + }, + 'annotations' => { + 'description' => 'Alert description', + 'runbook' => 'troubleshooting/gitaly-down.md', + 'title' => 'Alert title' + }, + 'startsAt' => '2020-04-27T10:10:22.265949279Z', + 'endsAt' => '0001-01-01T00:00:00Z', + 'generatorURL' => 'http://8d467bd4607a:9090/graph?g0.expr=vector%281%29&g0.tab=1', + 'fingerprint' => 'b6ac4d42057c43c1' + } + end + let(:parsed_alert) { Gitlab::Alerting::Alert.new(project: project, payload: payload) } + + subject { described_class.from_prometheus_alert(project: project, parsed_alert: parsed_alert) } + + it 'returns Alert-compatible params' do + is_expected.to eq( + project_id: project.id, + title: 'Alert title', + description: 'Alert description', + monitoring_tool: 'Prometheus', + payload: payload, + started_at: parsed_alert.starts_at, + ended_at: parsed_alert.ends_at, + fingerprint: parsed_alert.gitlab_fingerprint + ) + end + end end diff --git a/spec/lib/gitlab/alerting/alert_spec.rb b/spec/lib/gitlab/alerting/alert_spec.rb index 6d97f08af91..a0582515f3d 100644 --- a/spec/lib/gitlab/alerting/alert_spec.rb +++ b/spec/lib/gitlab/alerting/alert_spec.rb @@ -246,6 +246,30 @@ describe Gitlab::Alerting::Alert do it_behaves_like 'parse payload', 'annotations/gitlab_incident_markdown' end + describe '#gitlab_fingerprint' do + subject { alert.gitlab_fingerprint } + + context 'when the alert is a GitLab managed alert' do + include_context 'gitlab alert' + + it 'returns a fingerprint' do + plain_fingerprint = [alert.metric_id, alert.starts_at].join('/') + + is_expected.to eq(Digest::SHA1.hexdigest(plain_fingerprint)) + end + end + + context 'when the alert is from self managed Prometheus' do + include_context 'full query' + + it 'returns a fingerprint' do + plain_fingerprint = [alert.starts_at, alert.title, alert.full_query].join('/') + + is_expected.to eq(Digest::SHA1.hexdigest(plain_fingerprint)) + end + end + end + describe '#valid?' do before do payload.update( diff --git a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb index 8c4071a7ed1..7603a5ccfc0 100644 --- a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb @@ -8,6 +8,7 @@ describe Gitlab::Metrics::Samplers::RubySampler do before do allow(Gitlab::Metrics::NullMetric).to receive(:instance).and_return(null_metric) + stub_env('enable_memory_uss_pss', "1") end describe '#initialize' do @@ -19,16 +20,6 @@ describe Gitlab::Metrics::Samplers::RubySampler do end describe '#sample' do - it 'samples various statistics' do - expect(Gitlab::Metrics::System).to receive(:cpu_time) - expect(Gitlab::Metrics::System).to receive(:file_descriptor_count) - expect(Gitlab::Metrics::System).to receive(:memory_usage) - expect(Gitlab::Metrics::System).to receive(:max_open_file_descriptors) - expect(sampler).to receive(:sample_gc) - - sampler.sample - end - it 'adds a metric containing the process resident memory bytes' do expect(Gitlab::Metrics::System).to receive(:memory_usage).and_return(9000) @@ -37,6 +28,15 @@ describe Gitlab::Metrics::Samplers::RubySampler do sampler.sample end + it 'adds a metric containing the process unique and proportional memory bytes' do + expect(Gitlab::Metrics::System).to receive(:memory_usage_uss_pss).and_return(uss: 9000, pss: 10_000) + + expect(sampler.metrics[:process_unique_memory_bytes]).to receive(:set).with({}, 9000) + expect(sampler.metrics[:process_proportional_memory_bytes]).to receive(:set).with({}, 10_000) + + sampler.sample + end + it 'adds a metric containing the amount of open file descriptors' do expect(Gitlab::Metrics::System).to receive(:file_descriptor_count) .and_return(4) diff --git a/spec/lib/gitlab/metrics/system_spec.rb b/spec/lib/gitlab/metrics/system_spec.rb index a5aa80686fd..37d26bd9d63 100644 --- a/spec/lib/gitlab/metrics/system_spec.rb +++ b/spec/lib/gitlab/metrics/system_spec.rb @@ -3,33 +3,122 @@ require 'spec_helper' describe Gitlab::Metrics::System do - if File.exist?('/proc') + context 'when /proc files exist' do + # Fixtures pulled from: + # Linux carbon 5.3.0-7648-generic #41~1586789791~19.10~9593806-Ubuntu SMP Mon Apr 13 17:50:40 UTC x86_64 x86_64 x86_64 GNU/Linux + let(:proc_status) do + # most rows omitted for brevity + <<~SNIP + Name: less + VmHWM: 2468 kB + VmRSS: 2468 kB + RssAnon: 260 kB + SNIP + end + + let(:proc_smaps_rollup) do + # full snapshot + <<~SNIP + Rss: 2564 kB + Pss: 503 kB + Pss_Anon: 312 kB + Pss_File: 191 kB + Pss_Shmem: 0 kB + Shared_Clean: 2100 kB + Shared_Dirty: 0 kB + Private_Clean: 152 kB + Private_Dirty: 312 kB + Referenced: 2564 kB + Anonymous: 312 kB + LazyFree: 0 kB + AnonHugePages: 0 kB + ShmemPmdMapped: 0 kB + Shared_Hugetlb: 0 kB + Private_Hugetlb: 0 kB + Swap: 0 kB + SwapPss: 0 kB + Locked: 0 kB + SNIP + end + + let(:proc_limits) do + # full snapshot + <<~SNIP + Limit Soft Limit Hard Limit Units + Max cpu time unlimited unlimited seconds + Max file size unlimited unlimited bytes + Max data size unlimited unlimited bytes + Max stack size 8388608 unlimited bytes + Max core file size 0 unlimited bytes + Max resident set unlimited unlimited bytes + Max processes 126519 126519 processes + Max open files 1024 1048576 files + Max locked memory 67108864 67108864 bytes + Max address space unlimited unlimited bytes + Max file locks unlimited unlimited locks + Max pending signals 126519 126519 signals + Max msgqueue size 819200 819200 bytes + Max nice priority 0 0 + Max realtime priority 0 0 + Max realtime timeout unlimited unlimited us + SNIP + end + describe '.memory_usage' do - it "returns the process' memory usage in bytes" do - expect(described_class.memory_usage).to be > 0 + it "returns the process' resident set size (RSS) in bytes" do + mock_existing_proc_file('/proc/self/status', proc_status) + + expect(described_class.memory_usage).to eq(2527232) end end describe '.file_descriptor_count' do it 'returns the amount of open file descriptors' do - expect(described_class.file_descriptor_count).to be > 0 + expect(Dir).to receive(:glob).and_return(['/some/path', '/some/other/path']) + + expect(described_class.file_descriptor_count).to eq(2) end end describe '.max_open_file_descriptors' do it 'returns the max allowed open file descriptors' do - expect(described_class.max_open_file_descriptors).to be > 0 + mock_existing_proc_file('/proc/self/limits', proc_limits) + + expect(described_class.max_open_file_descriptors).to eq(1024) end end - else + + describe '.memory_usage_uss_pss' do + it "returns the process' unique and porportional set size (USS/PSS) in bytes" do + mock_existing_proc_file('/proc/self/smaps_rollup', proc_smaps_rollup) + + # (Private_Clean (152 kB) + Private_Dirty (312 kB) + Private_Hugetlb (0 kB)) * 1024 + expect(described_class.memory_usage_uss_pss).to eq(uss: 475136, pss: 515072) + end + end + end + + context 'when /proc files do not exist' do + before do + mock_missing_proc_file + end + describe '.memory_usage' do - it 'returns 0.0' do - expect(described_class.memory_usage).to eq(0.0) + it 'returns 0' do + expect(described_class.memory_usage).to eq(0) + end + end + + describe '.memory_usage_uss_pss' do + it "returns 0 for all components" do + expect(described_class.memory_usage_uss_pss).to eq(uss: 0, pss: 0) end end describe '.file_descriptor_count' do it 'returns 0' do + expect(Dir).to receive(:glob).and_return([]) + expect(described_class.file_descriptor_count).to eq(0) end end @@ -98,4 +187,12 @@ describe Gitlab::Metrics::System do expect(described_class.thread_cpu_duration(start_time)).to be_nil end end + + def mock_existing_proc_file(path, content) + allow(File).to receive(:foreach).with(path) { |_path, &block| content.each_line(&block) } + end + + def mock_missing_proc_file + allow(File).to receive(:foreach).and_raise(Errno::ENOENT) + end end diff --git a/spec/models/alert_management/alert_spec.rb b/spec/models/alert_management/alert_spec.rb index b40b18aeb4c..8c0f1016cac 100644 --- a/spec/models/alert_management/alert_spec.rb +++ b/spec/models/alert_management/alert_spec.rb @@ -20,6 +20,62 @@ describe AlertManagement::Alert do it { is_expected.to validate_length_of(:service).is_at_most(100) } it { is_expected.to validate_length_of(:monitoring_tool).is_at_most(100) } + context 'when status is triggered' do + context 'when ended_at is blank' do + subject { build(:alert_management_alert) } + + it { is_expected.to be_valid } + end + + context 'when ended_at is present' do + subject { build(:alert_management_alert, ended_at: Time.current) } + + it { is_expected.to be_invalid } + end + end + + context 'when status is acknowledged' do + context 'when ended_at is blank' do + subject { build(:alert_management_alert, :acknowledged) } + + it { is_expected.to be_valid } + end + + context 'when ended_at is present' do + subject { build(:alert_management_alert, :acknowledged, ended_at: Time.current) } + + it { is_expected.to be_invalid } + end + end + + context 'when status is resolved' do + context 'when ended_at is blank' do + subject { build(:alert_management_alert, :resolved, ended_at: nil) } + + it { is_expected.to be_invalid } + end + + context 'when ended_at is present' do + subject { build(:alert_management_alert, :resolved, ended_at: Time.current) } + + it { is_expected.to be_valid } + end + end + + context 'when status is ignored' do + context 'when ended_at is blank' do + subject { build(:alert_management_alert, :ignored) } + + it { is_expected.to be_valid } + end + + context 'when ended_at is present' do + subject { build(:alert_management_alert, :ignored, ended_at: Time.current) } + + it { is_expected.to be_invalid } + end + end + describe 'fingerprint' do let_it_be(:fingerprint) { 'fingerprint' } let_it_be(:existing_alert) { create(:alert_management_alert, fingerprint: fingerprint) } @@ -64,57 +120,7 @@ describe AlertManagement::Alert do { critical: 0, high: 1, medium: 2, low: 3, info: 4, unknown: 5 } end - let(:status_values) do - { triggered: 0, acknowledged: 1, resolved: 2, ignored: 3 } - end - it { is_expected.to define_enum_for(:severity).with_values(severity_values) } - it { is_expected.to define_enum_for(:status).with_values(status_values) } - end - - describe 'fingerprint setter' do - let(:alert) { build(:alert_management_alert) } - - subject(:set_fingerprint) { alert.fingerprint = fingerprint } - - let(:fingerprint) { 'test' } - - it 'sets to the SHA1 of the value' do - expect { set_fingerprint } - .to change { alert.fingerprint } - .from(nil) - .to(Digest::SHA1.hexdigest(fingerprint)) - end - - describe 'testing length of 40' do - where(:input) do - [ - 'test', - 'another test', - 'a' * 1000, - 12345 - ] - end - - with_them do - let(:fingerprint) { input } - - it 'sets the fingerprint to 40 chars' do - set_fingerprint - expect(alert.fingerprint.size).to eq(40) - end - end - end - - context 'blank value given' do - let(:fingerprint) { '' } - - it 'does not set the fingerprint' do - expect { set_fingerprint } - .not_to change { alert.fingerprint } - .from(nil) - end - end end describe '.for_iid' do @@ -127,6 +133,18 @@ describe AlertManagement::Alert do it { is_expected.to match_array(alert_1) } end + describe '.for_fingerprint' do + let_it_be(:fingerprint) { SecureRandom.hex } + let_it_be(:project) { create(:project) } + let_it_be(:alert_1) { create(:alert_management_alert, project: project, fingerprint: fingerprint) } + let_it_be(:alert_2) { create(:alert_management_alert, project: project) } + let_it_be(:alert_3) { create(:alert_management_alert, fingerprint: fingerprint) } + + subject { described_class.for_fingerprint(project, fingerprint) } + + it { is_expected.to contain_exactly(alert_1) } + end + describe '.details' do let(:payload) do { @@ -152,4 +170,81 @@ describe AlertManagement::Alert do ) end end + + describe '#trigger' do + subject { alert.trigger } + + context 'when alert is in triggered state' do + let(:alert) { create(:alert_management_alert) } + + it 'does not change the alert status' do + expect { subject }.not_to change { alert.reload.status } + end + end + + context 'when alert not in triggered state' do + let(:alert) { create(:alert_management_alert, :resolved) } + + it 'changes the alert status to triggered' do + expect { subject }.to change { alert.triggered? }.to(true) + end + + it 'resets ended at' do + expect { subject }.to change { alert.reload.ended_at }.to nil + end + end + end + + describe '#acknowledge' do + subject { alert.acknowledge } + + let(:alert) { create(:alert_management_alert, :resolved) } + + it 'changes the alert status to acknowledged' do + expect { subject }.to change { alert.acknowledged? }.to(true) + end + + it 'resets ended at' do + expect { subject }.to change { alert.reload.ended_at }.to nil + end + end + + describe '#resolve' do + let!(:ended_at) { Time.current } + + subject do + alert.ended_at = ended_at + alert.resolve + end + + context 'when alert already resolved' do + let(:alert) { create(:alert_management_alert, :resolved) } + + it 'does not change the alert status' do + expect { subject }.not_to change { alert.reload.status } + end + end + + context 'when alert is not resolved' do + let(:alert) { create(:alert_management_alert) } + + it 'changes alert status to "resolved"' do + expect { subject }.to change { alert.resolved? }.to(true) + end + end + end + + describe '#ignore' do + subject { alert.ignore } + + let(:alert) { create(:alert_management_alert, :resolved) } + + it 'changes the alert status to ignored' do + expect { subject }.to change { alert.ignored? }.to(true) + end + + it 'resets ended at' do + expect { subject }.to change { alert.reload.ended_at }.to nil + end + end end diff --git a/spec/requests/api/graphql/project/alert_management/alerts_spec.rb b/spec/requests/api/graphql/project/alert_management/alerts_spec.rb index 50650d13644..6baa9d4b2f9 100644 --- a/spec/requests/api/graphql/project/alert_management/alerts_spec.rb +++ b/spec/requests/api/graphql/project/alert_management/alerts_spec.rb @@ -7,7 +7,7 @@ describe 'getting Alert Management Alerts' do let_it_be(:payload) { { 'custom' => { 'alert' => 'payload' } } } let_it_be(:project) { create(:project, :repository) } let_it_be(:current_user) { create(:user) } - let_it_be(:alert_1) { create(:alert_management_alert, :all_fields, project: project, severity: :low) } + let_it_be(:alert_1) { create(:alert_management_alert, :all_fields, :resolved, project: project, severity: :low) } let_it_be(:alert_2) { create(:alert_management_alert, :all_fields, project: project, severity: :critical, payload: payload) } let_it_be(:other_project_alert) { create(:alert_management_alert, :all_fields) } @@ -49,27 +49,34 @@ describe 'getting Alert Management Alerts' do end let(:first_alert) { alerts.first } + let(:second_alert) { alerts.second } it_behaves_like 'a working graphql query' it { expect(alerts.size).to eq(2) } - it 'returns the correct properties of the alert' do + + it 'returns the correct properties of the alerts' do expect(first_alert).to include( 'iid' => alert_2.iid.to_s, 'title' => alert_2.title, 'description' => alert_2.description, 'severity' => alert_2.severity.upcase, - 'status' => alert_2.status.upcase, + 'status' => 'TRIGGERED', 'monitoringTool' => alert_2.monitoring_tool, 'service' => alert_2.service, 'hosts' => alert_2.hosts, 'eventCount' => alert_2.events, 'startedAt' => alert_2.started_at.strftime('%Y-%m-%dT%H:%M:%SZ'), - 'endedAt' => alert_2.ended_at.strftime('%Y-%m-%dT%H:%M:%SZ'), + 'endedAt' => nil, 'details' => { 'custom.alert' => 'payload' }, 'createdAt' => alert_2.created_at.strftime('%Y-%m-%dT%H:%M:%SZ'), 'updatedAt' => alert_2.updated_at.strftime('%Y-%m-%dT%H:%M:%SZ') ) + + expect(second_alert).to include( + 'status' => 'RESOLVED', + 'endedAt' => alert_1.ended_at.strftime('%Y-%m-%dT%H:%M:%SZ') + ) end context 'with iid given' do diff --git a/spec/services/alert_management/process_prometheus_alert_service_spec.rb b/spec/services/alert_management/process_prometheus_alert_service_spec.rb new file mode 100644 index 00000000000..73f9f103902 --- /dev/null +++ b/spec/services/alert_management/process_prometheus_alert_service_spec.rb @@ -0,0 +1,136 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe AlertManagement::ProcessPrometheusAlertService do + let_it_be(:project) { create(:project) } + + describe '#execute' do + subject { described_class.new(project, nil, payload).execute } + + context 'when alert payload is valid' do + let(:parsed_alert) { Gitlab::Alerting::Alert.new(project: project, payload: payload) } + let(:payload) do + { + 'status' => status, + 'labels' => { + 'alertname' => 'GitalyFileServerDown', + 'channel' => 'gitaly', + 'pager' => 'pagerduty', + 'severity' => 's1' + }, + 'annotations' => { + 'description' => 'Alert description', + 'runbook' => 'troubleshooting/gitaly-down.md', + 'title' => 'Alert title' + }, + 'startsAt' => '2020-04-27T10:10:22.265949279Z', + 'endsAt' => '2020-04-27T10:20:22.265949279Z', + 'generatorURL' => 'http://8d467bd4607a:9090/graph?g0.expr=vector%281%29&g0.tab=1', + 'fingerprint' => 'b6ac4d42057c43c1' + } + end + + context 'when Prometheus alert status is firing' do + let(:status) { 'firing' } + + context 'when alert with the same fingerprint already exists' do + let!(:alert) { create(:alert_management_alert, :resolved, project: project, fingerprint: parsed_alert.gitlab_fingerprint) } + + context 'when status can be changed' do + it 'changes status to triggered' do + expect { subject }.to change { alert.reload.triggered? }.to(true) + end + end + + context 'when status change did not succeed' do + before do + allow(AlertManagement::Alert).to receive(:for_fingerprint).and_return([alert]) + allow(alert).to receive(:trigger).and_return(false) + end + + it 'writes a warning to the log' do + expect(Gitlab::AppLogger).to receive(:warn).with( + message: 'Unable to update AlertManagement::Alert status to triggered', + project_id: project.id, + alert_id: alert.id + ) + + subject + end + end + + it { is_expected.to be_success } + end + + context 'when alert does not exist' do + context 'when alert can be created' do + it 'creates a new alert' do + expect { subject }.to change { AlertManagement::Alert.where(project: project).count }.by(1) + end + end + + context 'when alert cannot be created' do + let(:errors) { double(messages: { hosts: ['hosts array is over 255 chars'] })} + let(:am_alert) { instance_double(AlertManagement::Alert, save: false, errors: errors) } + + before do + allow(AlertManagement::Alert).to receive(:new).and_return(am_alert) + end + + it 'writes a warning to the log' do + expect(Gitlab::AppLogger).to receive(:warn).with( + message: 'Unable to create AlertManagement::Alert', + project_id: project.id, + alert_errors: { hosts: ['hosts array is over 255 chars'] } + ) + + subject + end + end + + it { is_expected.to be_success } + end + end + + context 'when Prometheus alert status is resolved' do + let(:status) { 'resolved' } + let!(:alert) { create(:alert_management_alert, project: project, fingerprint: parsed_alert.gitlab_fingerprint) } + + context 'when status can be changed' do + it 'resolves an existing alert' do + expect { subject }.to change { alert.reload.resolved? }.to(true) + end + end + + context 'when status change did not succeed' do + before do + allow(AlertManagement::Alert).to receive(:for_fingerprint).and_return([alert]) + allow(alert).to receive(:resolve).and_return(false) + end + + it 'writes a warning to the log' do + expect(Gitlab::AppLogger).to receive(:warn).with( + message: 'Unable to update AlertManagement::Alert status to resolved', + project_id: project.id, + alert_id: alert.id + ) + + subject + end + end + + it { is_expected.to be_success } + end + end + + context 'when alert payload is invalid' do + let(:payload) { {} } + + it 'responds with bad_request' do + expect(subject).to be_error + expect(subject.http_status).to eq(:bad_request) + end + end + end +end diff --git a/spec/services/alert_management/update_alert_status_service_spec.rb b/spec/services/alert_management/update_alert_status_service_spec.rb index 325b03840d3..5bdad7a8e19 100644 --- a/spec/services/alert_management/update_alert_status_service_spec.rb +++ b/spec/services/alert_management/update_alert_status_service_spec.rb @@ -11,7 +11,7 @@ describe AlertManagement::UpdateAlertStatusService do let(:new_status) { 'acknowledged' } it 'updates the status' do - expect { execute }.to change { alert.status }.to(new_status) + expect { execute }.to change { alert.acknowledged? }.to(true) end context 'with unknown status' do diff --git a/spec/services/merge_requests/rebase_service_spec.rb b/spec/services/merge_requests/rebase_service_spec.rb index 22df3b84243..69d555f838d 100644 --- a/spec/services/merge_requests/rebase_service_spec.rb +++ b/spec/services/merge_requests/rebase_service_spec.rb @@ -72,12 +72,15 @@ describe MergeRequests::RebaseService do it_behaves_like 'sequence of failure and success' context 'when unexpected error occurs' do + let(:exception) { RuntimeError.new('Something went wrong') } + let(:merge_request_ref) { merge_request.to_reference(full: true) } + before do - allow(repository).to receive(:gitaly_operation_client).and_raise('Something went wrong') + allow(repository).to receive(:gitaly_operation_client).and_raise(exception) end it 'saves a generic error message' do - subject.execute(merge_request) + service.execute(merge_request) expect(merge_request.reload.merge_error).to eq(described_class::REBASE_ERROR) end @@ -86,6 +89,18 @@ describe MergeRequests::RebaseService do expect(service.execute(merge_request)).to match(status: :error, message: described_class::REBASE_ERROR) end + + it 'logs the error' do + expect(service).to receive(:log_error).with(exception: exception, message: described_class::REBASE_ERROR, save_message_on_model: true).and_call_original + expect(Gitlab::ErrorTracking).to receive(:track_exception).with(exception, + class: described_class.to_s, + merge_request: merge_request_ref, + merge_request_id: merge_request.id, + message: described_class::REBASE_ERROR, + save_message_on_model: true).and_call_original + + service.execute(merge_request) + end end context 'with git command failure' do diff --git a/spec/services/merge_requests/squash_service_spec.rb b/spec/services/merge_requests/squash_service_spec.rb index cb278eec692..a53314ed737 100644 --- a/spec/services/merge_requests/squash_service_spec.rb +++ b/spec/services/merge_requests/squash_service_spec.rb @@ -141,15 +141,14 @@ describe MergeRequests::SquashService do let(:merge_request) { merge_request_with_only_new_files } let(:error) { 'A test error' } - context 'with gitaly enabled' do + context 'with an error in Gitaly UserSquash RPC' do before do allow(repository.gitaly_operation_client).to receive(:user_squash) .and_raise(Gitlab::Git::Repository::GitError, error) end - it 'logs the stage and output' do - expect(service).to receive(:log_error).with(log_error) - expect(service).to receive(:log_error).with(error) + it 'logs the error' do + expect(service).to receive(:log_error).with(exception: an_instance_of(Gitlab::Git::Repository::GitError), message: 'Failed to squash merge request') service.execute end @@ -158,19 +157,42 @@ describe MergeRequests::SquashService do expect(service.execute).to match(status: :error, message: a_string_including('squash')) end end + + context 'with an error in squash in progress check' do + before do + allow(repository).to receive(:squash_in_progress?) + .and_raise(Gitlab::Git::Repository::GitError, error) + end + + it 'logs the stage and output' do + expect(service).to receive(:log_error).with(exception: an_instance_of(Gitlab::Git::Repository::GitError), message: 'Failed to check squash in progress') + + service.execute + end + + it 'returns an error' do + expect(service.execute).to match(status: :error, message: 'An error occurred while checking whether another squash is in progress.') + end + end end context 'when any other exception is thrown' do let(:merge_request) { merge_request_with_only_new_files } - let(:error) { 'A test error' } + let(:merge_request_ref) { merge_request.to_reference(full: true) } + let(:exception) { RuntimeError.new('A test error') } before do - allow(merge_request.target_project.repository).to receive(:squash).and_raise(error) + allow(merge_request.target_project.repository).to receive(:squash).and_raise(exception) end - it 'logs the MR reference and exception' do - expect(service).to receive(:log_error).with(a_string_including("#{project.full_path}#{merge_request.to_reference}")) - expect(service).to receive(:log_error).with(error) + it 'logs the error' do + expect(service).to receive(:log_error).with(exception: exception, message: 'Failed to squash merge request').and_call_original + expect(Gitlab::ErrorTracking).to receive(:track_exception).with(exception, + class: described_class.to_s, + merge_request: merge_request_ref, + merge_request_id: merge_request.id, + message: 'Failed to squash merge request', + save_message_on_model: false).and_call_original service.execute end diff --git a/spec/services/projects/alerting/notify_service_spec.rb b/spec/services/projects/alerting/notify_service_spec.rb index 8315d2292a0..bfd51874549 100644 --- a/spec/services/projects/alerting/notify_service_spec.rb +++ b/spec/services/projects/alerting/notify_service_spec.rb @@ -121,7 +121,7 @@ describe Projects::Alerting::NotifyService do 'hosts' => [], 'payload' => payload_raw, 'severity' => 'critical', - 'status' => 'triggered', + 'status' => AlertManagement::Alert::STATUSES[:triggered], 'events' => 1, 'started_at' => alert.started_at, 'ended_at' => nil diff --git a/spec/services/projects/prometheus/alerts/notify_service_spec.rb b/spec/services/projects/prometheus/alerts/notify_service_spec.rb index dce96dda1e3..bfa784cd212 100644 --- a/spec/services/projects/prometheus/alerts/notify_service_spec.rb +++ b/spec/services/projects/prometheus/alerts/notify_service_spec.rb @@ -217,6 +217,51 @@ describe Projects::Prometheus::Alerts::NotifyService do end end + context 'process Alert Management alerts' do + let(:process_service) { instance_double(AlertManagement::ProcessPrometheusAlertService) } + + before do + create(:prometheus_service, project: project) + create(:project_alerting_setting, project: project, token: token) + end + + context 'when alert_management_minimal feature enabled' do + before do + stub_feature_flags(alert_management_minimal: true) + end + + context 'with multiple firing alerts and resolving alerts' do + let(:payload_raw) do + payload_for(firing: [alert_firing, alert_firing], resolved: [alert_resolved]) + end + + it 'processes Prometheus alerts' do + expect(AlertManagement::ProcessPrometheusAlertService) + .to receive(:new) + .with(project, nil, kind_of(Hash)) + .exactly(3).times + .and_return(process_service) + expect(process_service).to receive(:execute).exactly(3).times + + subject + end + end + end + + context 'when alert_management_minimal feature disabled' do + before do + stub_feature_flags(alert_management_minimal: false) + end + + it 'does not process Prometheus alerts' do + expect(AlertManagement::ProcessPrometheusAlertService) + .not_to receive(:new) + + subject + end + end + end + context 'process incident issues' do before do create(:prometheus_service, project: project) @@ -286,6 +331,13 @@ describe Projects::Prometheus::Alerts::NotifyService do it_behaves_like 'no notifications', http_status: :bad_request + it 'does not process Prometheus alerts' do + expect(AlertManagement::ProcessPrometheusAlertService) + .not_to receive(:new) + + subject + end + it 'does not process issues' do expect(IncidentManagement::ProcessPrometheusAlertWorker) .not_to receive(:perform_async)