diff options
49 files changed, 1350 insertions, 658 deletions
diff --git a/app/assets/javascripts/behaviors/markdown/paste_markdown_table.js b/app/assets/javascripts/behaviors/markdown/paste_markdown_table.js index d14799c976b..665a7216424 100644 --- a/app/assets/javascripts/behaviors/markdown/paste_markdown_table.js +++ b/app/assets/javascripts/behaviors/markdown/paste_markdown_table.js @@ -1,58 +1,81 @@ +const maxColumnWidth = (rows, columnIndex) => Math.max(...rows.map(row => row[columnIndex].length)); + export default class PasteMarkdownTable { constructor(clipboardData) { this.data = clipboardData; + this.columnWidths = []; + this.rows = []; + this.tableFound = this.parseTable(); + } + + isTable() { + return this.tableFound; } - static maxColumnWidth(rows, columnIndex) { - return Math.max.apply(null, rows.map(row => row[columnIndex].length)); + convertToTableMarkdown() { + this.calculateColumnWidths(); + + const markdownRows = this.rows.map( + row => + // | Name | Title | Email Address | + // |--------------|-------|----------------| + // | Jane Atler | CEO | jane@acme.com | + // | John Doherty | CTO | john@acme.com | + // | Sally Smith | CFO | sally@acme.com | + `| ${row.map((column, index) => this.formatColumn(column, index)).join(' | ')} |`, + ); + + // Insert a header break (e.g. -----) to the second row + markdownRows.splice(1, 0, this.generateHeaderBreak()); + + return markdownRows.join('\n'); } + // Private methods below + // To determine whether the cut data is a table, the following criteria // must be satisfied with the clipboard data: // // 1. MIME types "text/plain" and "text/html" exist // 2. The "text/html" data must have a single <table> element - static isTable(data) { - const types = new Set(data.types); - - if (!types.has('text/html') || !types.has('text/plain')) { + // 3. The number of rows in the "text/plain" data matches that of the "text/html" data + // 4. The max number of columns in "text/plain" matches that of the "text/html" data + parseTable() { + if (!this.data.types.includes('text/html') || !this.data.types.includes('text/plain')) { return false; } - const htmlData = data.getData('text/html'); - const doc = new DOMParser().parseFromString(htmlData, 'text/html'); + const htmlData = this.data.getData('text/html'); + this.doc = new DOMParser().parseFromString(htmlData, 'text/html'); + const tables = this.doc.querySelectorAll('table'); // We're only looking for exactly one table. If there happens to be // multiple tables, it's possible an application copied data into // the clipboard that is not related to a simple table. It may also be // complicated converting multiple tables into Markdown. - if (doc.querySelectorAll('table').length === 1) { - return true; + if (tables.length !== 1) { + return false; } - return false; - } - - convertToTableMarkdown() { const text = this.data.getData('text/plain').trim(); - this.rows = text.split(/[\n\u0085\u2028\u2029]|\r\n?/g).map(row => row.split('\t')); - this.normalizeRows(); - this.calculateColumnWidths(); + const splitRows = text.split(/[\n\u0085\u2028\u2029]|\r\n?/g); - const markdownRows = this.rows.map( - row => - // | Name | Title | Email Address | - // |--------------|-------|----------------| - // | Jane Atler | CEO | jane@acme.com | - // | John Doherty | CTO | john@acme.com | - // | Sally Smith | CFO | sally@acme.com | - `| ${row.map((column, index) => this.formatColumn(column, index)).join(' | ')} |`, - ); + // Now check that the number of rows matches between HTML and text + if (this.doc.querySelectorAll('tr').length !== splitRows.length) { + return false; + } - // Insert a header break (e.g. -----) to the second row - markdownRows.splice(1, 0, this.generateHeaderBreak()); + this.rows = splitRows.map(row => row.split('\t')); + this.normalizeRows(); - return markdownRows.join('\n'); + // Check that the max number of columns in the HTML matches the number of + // columns in the text. GitHub, for example, copies a line number and the + // line itself into the HTML data. + if (!this.columnCountsMatch()) { + return false; + } + + return true; } // Ensure each row has the same number of columns @@ -69,10 +92,21 @@ export default class PasteMarkdownTable { calculateColumnWidths() { this.columnWidths = this.rows[0].map((_column, columnIndex) => - PasteMarkdownTable.maxColumnWidth(this.rows, columnIndex), + maxColumnWidth(this.rows, columnIndex), ); } + columnCountsMatch() { + const textColumnCount = this.rows[0].length; + let htmlColumnCount = 0; + + this.doc.querySelectorAll('table tr').forEach(row => { + htmlColumnCount = Math.max(row.cells.length, htmlColumnCount); + }); + + return textColumnCount === htmlColumnCount; + } + formatColumn(column, index) { const spaces = Array(this.columnWidths[index] - column.length + 1).join(' '); return column + spaces; diff --git a/app/assets/javascripts/dropzone_input.js b/app/assets/javascripts/dropzone_input.js index 79739072abb..86590865892 100644 --- a/app/assets/javascripts/dropzone_input.js +++ b/app/assets/javascripts/dropzone_input.js @@ -176,11 +176,11 @@ export default function dropzoneInput(form) { const pasteEvent = event.originalEvent; const { clipboardData } = pasteEvent; if (clipboardData && clipboardData.items) { + const converter = new PasteMarkdownTable(clipboardData); // Apple Numbers copies a table as an image, HTML, and text, so // we need to check for the presence of a table first. - if (PasteMarkdownTable.isTable(clipboardData)) { + if (converter.isTable()) { event.preventDefault(); - const converter = new PasteMarkdownTable(clipboardData); const text = converter.convertToTableMarkdown(); pasteText(text); } else { diff --git a/app/assets/javascripts/monitoring/components/dashboard.vue b/app/assets/javascripts/monitoring/components/dashboard.vue index c1ca5449ba3..797fd0e7e19 100644 --- a/app/assets/javascripts/monitoring/components/dashboard.vue +++ b/app/assets/javascripts/monitoring/components/dashboard.vue @@ -22,9 +22,11 @@ import GraphGroup from './graph_group.vue'; import EmptyState from './empty_state.vue'; import GroupEmptyState from './group_empty_state.vue'; import TrackEventDirective from '~/vue_shared/directives/track_event'; -import { getTimeDiff, isValidDate, getAddMetricTrackingOptions } from '../utils'; +import { getTimeDiff, getAddMetricTrackingOptions } from '../utils'; import { metricStates } from '../constants'; +const defaultTimeDiff = getTimeDiff(); + export default { components: { VueDraggable, @@ -168,9 +170,10 @@ export default { return { state: 'gettingStarted', formIsValid: null, - selectedTimeWindow: {}, - isRearrangingPanels: false, + startDate: getParameterValues('start')[0] || defaultTimeDiff.start, + endDate: getParameterValues('end')[0] || defaultTimeDiff.end, hasValidDates: true, + isRearrangingPanels: false, }; }, computed: { @@ -228,24 +231,10 @@ export default { if (!this.hasMetrics) { this.setGettingStartedEmptyState(); } else { - const defaultRange = getTimeDiff(); - const start = getParameterValues('start')[0] || defaultRange.start; - const end = getParameterValues('end')[0] || defaultRange.end; - - const range = { - start, - end, - }; - - this.selectedTimeWindow = range; - - if (!isValidDate(start) || !isValidDate(end)) { - this.hasValidDates = false; - this.showInvalidDateError(); - } else { - this.hasValidDates = true; - this.fetchData(range); - } + this.fetchData({ + start: this.startDate, + end: this.endDate, + }); } }, methods: { @@ -267,9 +256,20 @@ export default { key, }); }, - showInvalidDateError() { - createFlash(s__('Metrics|Link contains an invalid time window.')); + + onDateTimePickerApply(params) { + redirectTo(mergeUrlParams(params, window.location.href)); + }, + onDateTimePickerInvalid() { + createFlash( + s__( + 'Metrics|Link contains an invalid time window, please verify the link to see the requested time range.', + ), + ); + this.startDate = defaultTimeDiff.start; + this.endDate = defaultTimeDiff.end; }, + generateLink(group, title, yLabel) { const dashboard = this.currentDashboard || this.firstDashboard.path; const params = _.pick({ dashboard, group, title, y_label: yLabel }, value => value != null); @@ -287,9 +287,6 @@ export default { submitCustomMetricsForm() { this.$refs.customMetricsForm.submit(); }, - onDateTimePickerApply(timeWindowUrlParams) { - return redirectTo(mergeUrlParams(timeWindowUrlParams, window.location.href)); - }, /** * Return a single empty state for a group. * @@ -378,15 +375,16 @@ export default { </gl-form-group> <gl-form-group - v-if="hasValidDates" :label="s__('Metrics|Show last')" label-size="sm" label-for="monitor-time-window-dropdown" class="col-sm-6 col-md-6 col-lg-4" > <date-time-picker - :selected-time-window="selectedTimeWindow" - @onApply="onDateTimePickerApply" + :start="startDate" + :end="endDate" + @apply="onDateTimePickerApply" + @invalid="onDateTimePickerInvalid" /> </gl-form-group> </template> diff --git a/app/assets/javascripts/monitoring/components/date_time_picker/date_time_picker.vue b/app/assets/javascripts/monitoring/components/date_time_picker/date_time_picker.vue index 8749019c5cd..0aa710b1b3a 100644 --- a/app/assets/javascripts/monitoring/components/date_time_picker/date_time_picker.vue +++ b/app/assets/javascripts/monitoring/components/date_time_picker/date_time_picker.vue @@ -5,14 +5,21 @@ import Icon from '~/vue_shared/components/icon.vue'; import DateTimePickerInput from './date_time_picker_input.vue'; import { getTimeDiff, + isValidDate, getTimeWindow, stringToISODate, ISODateToString, truncateZerosInDateTime, isDateTimePickerInputValid, } from '~/monitoring/utils'; + import { timeWindows } from '~/monitoring/constants'; +const events = { + apply: 'apply', + invalid: 'invalid', +}; + export default { components: { Icon, @@ -23,77 +30,94 @@ export default { GlDropdownItem, }, props: { + start: { + type: String, + required: true, + }, + end: { + type: String, + required: true, + }, timeWindows: { type: Object, required: false, default: () => timeWindows, }, - selectedTimeWindow: { - type: Object, - required: false, - default: () => {}, - }, }, data() { return { - selectedTimeWindowText: '', - customTime: { - from: null, - to: null, - }, + startDate: this.start, + endDate: this.end, }; }, computed: { - applyEnabled() { - return Boolean(this.inputState.from && this.inputState.to); + startInputValid() { + return isValidDate(this.startDate); }, - inputState() { - const { from, to } = this.customTime; - return { - from: from && isDateTimePickerInputValid(from), - to: to && isDateTimePickerInputValid(to), - }; + endInputValid() { + return isValidDate(this.endDate); }, - }, - watch: { - selectedTimeWindow() { - this.verifyTimeRange(); + isValid() { + return this.startInputValid && this.endInputValid; + }, + + startInput: { + get() { + return this.startInputValid ? this.formatDate(this.startDate) : this.startDate; + }, + set(val) { + // Attempt to set a formatted date if possible + this.startDate = isDateTimePickerInputValid(val) ? stringToISODate(val) : val; + }, + }, + endInput: { + get() { + return this.endInputValid ? this.formatDate(this.endDate) : this.endDate; + }, + set(val) { + // Attempt to set a formatted date if possible + this.endDate = isDateTimePickerInputValid(val) ? stringToISODate(val) : val; + }, + }, + + timeWindowText() { + const timeWindow = getTimeWindow({ start: this.start, end: this.end }); + if (timeWindow) { + return this.timeWindows[timeWindow]; + } else if (isValidDate(this.start) && isValidDate(this.end)) { + return sprintf(s__('%{start} to %{end}'), { + start: this.formatDate(this.start), + end: this.formatDate(this.end), + }); + } + return ''; }, }, mounted() { - this.verifyTimeRange(); + // Validate on mounted, and trigger an update if needed + if (!this.isValid) { + this.$emit(events.invalid); + } }, methods: { - activeTimeWindow(key) { - return this.timeWindows[key] === this.selectedTimeWindowText; + formatDate(date) { + return truncateZerosInDateTime(ISODateToString(date)); }, - setCustomTimeWindowParameter() { - this.$emit('onApply', { - start: stringToISODate(this.customTime.from), - end: stringToISODate(this.customTime.to), - }); - }, - setTimeWindowParameter(key) { + setTimeWindow(key) { const { start, end } = getTimeDiff(key); - this.$emit('onApply', { - start, - end, - }); + this.startDate = start; + this.endDate = end; + + this.apply(); }, closeDropdown() { this.$refs.dropdown.hide(); }, - verifyTimeRange() { - const range = getTimeWindow(this.selectedTimeWindow); - if (range) { - this.selectedTimeWindowText = this.timeWindows[range]; - } else { - this.customTime = { - from: truncateZerosInDateTime(ISODateToString(this.selectedTimeWindow.start)), - to: truncateZerosInDateTime(ISODateToString(this.selectedTimeWindow.end)), - }; - this.selectedTimeWindowText = sprintf(s__('%{from} to %{to}'), this.customTime); - } + apply() { + this.$emit(events.apply, { + start: this.startDate, + end: this.endDate, + }); }, }, }; @@ -101,7 +125,7 @@ export default { <template> <gl-dropdown ref="dropdown" - :text="selectedTimeWindowText" + :text="timeWindowText" menu-class="time-window-dropdown-menu" class="js-time-window-dropdown" > @@ -113,24 +137,21 @@ export default { > <date-time-picker-input id="custom-time-from" - v-model="customTime.from" + v-model="startInput" :label="__('From')" - :state="inputState.from" + :state="startInputValid" /> <date-time-picker-input id="custom-time-to" - v-model="customTime.to" + v-model="endInput" :label="__('To')" - :state="inputState.to" + :state="endInputValid" /> <gl-form-group> <gl-button @click="closeDropdown">{{ __('Cancel') }}</gl-button> - <gl-button - variant="success" - :disabled="!applyEnabled" - @click="setCustomTimeWindowParameter" - >{{ __('Apply') }}</gl-button - > + <gl-button variant="success" :disabled="!isValid" @click="apply()"> + {{ __('Apply') }} + </gl-button> </gl-form-group> </gl-form-group> <gl-form-group @@ -142,14 +163,14 @@ export default { <gl-dropdown-item v-for="(value, key) in timeWindows" :key="key" - :active="activeTimeWindow(key)" + :active="value === timeWindowText" active-class="active" - @click="setTimeWindowParameter(key)" + @click="setTimeWindow(key)" > <icon name="mobile-issue-close" class="align-bottom" - :class="{ invisible: !activeTimeWindow(key) }" + :class="{ invisible: value !== timeWindowText }" /> {{ value }} </gl-dropdown-item> diff --git a/changelogs/unreleased/36235-services-usage-ping.yml b/changelogs/unreleased/36235-services-usage-ping.yml new file mode 100644 index 00000000000..2e4f7b5c29c --- /dev/null +++ b/changelogs/unreleased/36235-services-usage-ping.yml @@ -0,0 +1,5 @@ +--- +title: Add remaining project services to usage ping +merge_request: 21843 +author: +type: added diff --git a/changelogs/unreleased/36955-snowplow-custom-events-for-monitor-apm-alerts.yml b/changelogs/unreleased/36955-snowplow-custom-events-for-monitor-apm-alerts.yml new file mode 100644 index 00000000000..0e362a171b2 --- /dev/null +++ b/changelogs/unreleased/36955-snowplow-custom-events-for-monitor-apm-alerts.yml @@ -0,0 +1,5 @@ +--- +title: Custom snowplow events for monitoring alerts +merge_request: 21963 +author: +type: added diff --git a/doc/development/testing_guide/end_to_end/style_guide.md b/doc/development/testing_guide/end_to_end/style_guide.md index 9088e9e9bfb..7f4616f394b 100644 --- a/doc/development/testing_guide/end_to_end/style_guide.md +++ b/doc/development/testing_guide/end_to_end/style_guide.md @@ -54,18 +54,20 @@ We follow a simple formula roughly based on hungarian notation. *Formula*: `element :<descriptor>_<type>` - `descriptor`: The natural-language description of what the element is. On the login page, this could be `username`, or `password`. -- `type`: A physical control on the page that can be seen by a user. +- `type`: A generic control on the page that can be seen by a user. - `_button` - - `_link` - - `_tab` - - `_dropdown` - - `_field` - `_checkbox` + - `_container`: an element that includes other elements, but doesn't present visible content itself. E.g., an element that has a third-party editor inside it, but which isn't the editor itself and so doesn't include the editor's content. + - `_content`: any element that contains text, images, or any other content displayed to the user. + - `_dropdown` + - `_field`: a text input element. + - `_link` + - `_modal`: a popup modal dialog, e.g., a confirmation prompt. + - `_placeholder`: a temporary element that appears while content is loading. For example, the elements that are displayed instead of discussions while the discussions are being fetched. - `_radio` - - `_content` + - `_tab` -*Note: This list is a work in progress. This list will eventually be the end-all enumeration of all available types. - I.e., any element that does not end with something in this list is bad form.* +*Note: If none of the listed types are suitable, please open a merge request to add an appropriate type to the list.* ### Examples diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb index b200596a500..e00b49b9042 100644 --- a/lib/gitlab/usage_data.rb +++ b/lib/gitlab/usage_data.rb @@ -178,18 +178,17 @@ module Gitlab # rubocop: disable CodeReuse/ActiveRecord def services_usage - types = { - SlackService: :projects_slack_notifications_active, - SlackSlashCommandsService: :projects_slack_slash_active, - PrometheusService: :projects_prometheus_active, - CustomIssueTrackerService: :projects_custom_issue_tracker_active, - JenkinsService: :projects_jenkins_active, - MattermostService: :projects_mattermost_active - } + service_counts = count(Service.active.where(template: false).where.not(type: 'JiraService').group(:type), fallback: Hash.new(-1)) + + results = Service.available_services_names.each_with_object({}) do |service_name, response| + response["projects_#{service_name}_active".to_sym] = service_counts["#{service_name}_service".camelize] || 0 + end - results = count(Service.active.by_type(types.keys).group(:type), fallback: Hash.new(-1)) - types.each_with_object({}) { |(klass, key), response| response[key] = results[klass.to_s] || 0 } - .merge(jira_usage) + # Keep old Slack keys for backward compatibility, https://gitlab.com/gitlab-data/analytics/issues/3241 + results[:projects_slack_notifications_active] = results[:projects_slack_active] + results[:projects_slack_slash_active] = results[:projects_slack_slash_commands_active] + + results.merge(jira_usage) end def jira_usage @@ -223,6 +222,7 @@ module Gitlab results end + # rubocop: enable CodeReuse/ActiveRecord def user_preferences_usage {} # augmented in EE @@ -233,7 +233,6 @@ module Gitlab rescue ActiveRecord::StatementInvalid fallback end - # rubocop: enable CodeReuse/ActiveRecord def approximate_counts approx_counts = Gitlab::Database::Count.approximate_counts(APPROXIMATE_COUNT_MODELS) diff --git a/lib/sentry/client.rb b/lib/sentry/client.rb index e3b8305b664..40821e2b233 100644 --- a/lib/sentry/client.rb +++ b/lib/sentry/client.rb @@ -9,14 +9,6 @@ module Sentry Error = Class.new(StandardError) MissingKeysError = Class.new(StandardError) ResponseInvalidSizeError = Class.new(StandardError) - BadRequestError = Class.new(StandardError) - - SENTRY_API_SORT_VALUE_MAP = { - # <accepted_by_client> => <accepted_by_sentry_api> - 'frequency' => 'freq', - 'first_seen' => 'new', - 'last_seen' => nil - }.freeze attr_accessor :url, :token @@ -25,30 +17,8 @@ module Sentry @token = token end - def list_issues(**keyword_args) - response = get_issues(keyword_args) - - issues = response[:issues] - pagination = response[:pagination] - - validate_size(issues) - - handle_mapping_exceptions do - { - issues: map_to_errors(issues), - pagination: pagination - } - end - end - private - def validate_size(issues) - return if Gitlab::Utils::DeepSize.new(issues).valid? - - raise ResponseInvalidSizeError, "Sentry API response is too big. Limit is #{Gitlab::Utils::DeepSize.human_default_max_size}." - end - def handle_mapping_exceptions(&block) yield rescue KeyError => e @@ -85,31 +55,6 @@ module Sentry handle_response(response) end - def get_issues(**keyword_args) - response = http_get( - issues_api_url, - query: list_issue_sentry_query(keyword_args) - ) - - { - issues: response[:body], - pagination: Sentry::PaginationParser.parse(response[:headers]) - } - end - - def list_issue_sentry_query(issue_status:, limit:, sort: nil, search_term: '', cursor: nil) - unless SENTRY_API_SORT_VALUE_MAP.key?(sort) - raise BadRequestError, 'Invalid value for sort param' - end - - { - query: "is:#{issue_status} #{search_term}".strip, - limit: limit, - sort: SENTRY_API_SORT_VALUE_MAP[sort], - cursor: cursor - }.compact - end - def handle_request_exceptions yield rescue Gitlab::HTTP::Error => e @@ -139,58 +84,5 @@ module Sentry def raise_error(message) raise Client::Error, message end - - def issues_api_url - issues_url = URI(@url + '/issues/') - issues_url.path.squeeze!('/') - - issues_url - end - - def map_to_errors(issues) - issues.map(&method(:map_to_error)) - end - - def issue_url(id) - issues_url = @url + "/issues/#{id}" - - parse_sentry_url(issues_url) - end - - def project_url - parse_sentry_url(@url) - end - - def parse_sentry_url(api_url) - url = ErrorTracking::ProjectErrorTrackingSetting.extract_sentry_external_url(api_url) - - uri = URI(url) - uri.path.squeeze!('/') - # Remove trailing slash - uri = uri.to_s.gsub(/\/\z/, '') - - uri - end - - def map_to_error(issue) - Gitlab::ErrorTracking::Error.new( - id: issue.fetch('id'), - first_seen: issue.fetch('firstSeen', nil), - last_seen: issue.fetch('lastSeen', nil), - title: issue.fetch('title', nil), - type: issue.fetch('type', nil), - user_count: issue.fetch('userCount', nil), - count: issue.fetch('count', nil), - message: issue.dig('metadata', 'value'), - culprit: issue.fetch('culprit', nil), - external_url: issue_url(issue.fetch('id')), - short_id: issue.fetch('shortId', nil), - status: issue.fetch('status', nil), - frequency: issue.dig('stats', '24h'), - project_id: issue.dig('project', 'id'), - project_name: issue.dig('project', 'name'), - project_slug: issue.dig('project', 'slug') - ) - end end end diff --git a/lib/sentry/client/issue.rb b/lib/sentry/client/issue.rb index 4a11c87faa4..b6f2e07d233 100644 --- a/lib/sentry/client/issue.rb +++ b/lib/sentry/client/issue.rb @@ -3,6 +3,31 @@ module Sentry class Client module Issue + BadRequestError = Class.new(StandardError) + + SENTRY_API_SORT_VALUE_MAP = { + # <accepted_by_client> => <accepted_by_sentry_api> + 'frequency' => 'freq', + 'first_seen' => 'new', + 'last_seen' => nil + }.freeze + + def list_issues(**keyword_args) + response = get_issues(keyword_args) + + issues = response[:issues] + pagination = response[:pagination] + + validate_size(issues) + + handle_mapping_exceptions do + { + issues: map_to_errors(issues), + pagination: pagination + } + end + end + def issue_details(issue_id:) issue = get_issue(issue_id: issue_id) @@ -11,6 +36,37 @@ module Sentry private + def get_issues(**keyword_args) + response = http_get( + issues_api_url, + query: list_issue_sentry_query(keyword_args) + ) + + { + issues: response[:body], + pagination: Sentry::PaginationParser.parse(response[:headers]) + } + end + + def list_issue_sentry_query(issue_status:, limit:, sort: nil, search_term: '', cursor: nil) + unless SENTRY_API_SORT_VALUE_MAP.key?(sort) + raise BadRequestError, 'Invalid value for sort param' + end + + { + query: "is:#{issue_status} #{search_term}".strip, + limit: limit, + sort: SENTRY_API_SORT_VALUE_MAP[sort], + cursor: cursor + }.compact + end + + def validate_size(issues) + return if Gitlab::Utils::DeepSize.new(issues).valid? + + raise ResponseInvalidSizeError, "Sentry API response is too big. Limit is #{Gitlab::Utils::DeepSize.human_default_max_size}." + end + def get_issue(issue_id:) http_get(issue_api_url(issue_id))[:body] end @@ -19,6 +75,13 @@ module Sentry http_put(issue_api_url(issue_id), params)[:body] end + def issues_api_url + issues_url = URI("#{url}/issues/") + issues_url.path.squeeze!('/') + + issues_url + end + def issue_api_url(issue_id) issue_url = URI(url) issue_url.path = "/api/0/issues/#{CGI.escape(issue_id.to_s)}/" @@ -35,6 +98,50 @@ module Sentry gitlab_plugin.dig('issue', 'url') end + def issue_url(id) + parse_sentry_url("#{url}/issues/#{id}") + end + + def project_url + parse_sentry_url(url) + end + + def parse_sentry_url(api_url) + url = ErrorTracking::ProjectErrorTrackingSetting.extract_sentry_external_url(api_url) + + uri = URI(url) + uri.path.squeeze!('/') + # Remove trailing slash + uri = uri.to_s.gsub(/\/\z/, '') + + uri + end + + def map_to_errors(issues) + issues.map(&method(:map_to_error)) + end + + def map_to_error(issue) + Gitlab::ErrorTracking::Error.new( + id: issue.fetch('id'), + first_seen: issue.fetch('firstSeen', nil), + last_seen: issue.fetch('lastSeen', nil), + title: issue.fetch('title', nil), + type: issue.fetch('type', nil), + user_count: issue.fetch('userCount', nil), + count: issue.fetch('count', nil), + message: issue.dig('metadata', 'value'), + culprit: issue.fetch('culprit', nil), + external_url: issue_url(issue.fetch('id')), + short_id: issue.fetch('shortId', nil), + status: issue.fetch('status', nil), + frequency: issue.dig('stats', '24h'), + project_id: issue.dig('project', 'id'), + project_name: issue.dig('project', 'name'), + project_slug: issue.dig('project', 'slug') + ) + end + def map_to_detailed_error(issue) Gitlab::ErrorTracking::DetailedError.new( id: issue.fetch('id'), diff --git a/locale/gitlab.pot b/locale/gitlab.pot index f06a19b6321..ef8e20ec830 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -269,9 +269,6 @@ msgstr "" msgid "%{firstLabel} +%{labelCount} more" msgstr "" -msgid "%{from} to %{to}" -msgstr "" - msgid "%{global_id} is not a valid id for %{expected_type}." msgstr "" @@ -370,6 +367,9 @@ msgstr "" msgid "%{spammable_titlecase} was submitted to Akismet successfully." msgstr "" +msgid "%{start} to %{end}" +msgstr "" + msgid "%{state} epics" msgstr "" @@ -557,6 +557,9 @@ msgid_plural "%d groups" msgstr[0] "" msgstr[1] "" +msgid "1 hour" +msgstr "" + msgid "1 merged merge request" msgid_plural "%{merge_requests} merged merge requests" msgstr[0] "" @@ -607,6 +610,9 @@ msgstr "" msgid "20-29 contributions" msgstr "" +msgid "24 hours" +msgstr "" + msgid "2FA" msgstr "" @@ -619,6 +625,9 @@ msgstr "" msgid "3 hours" msgstr "" +msgid "30 days" +msgstr "" + msgid "30 minutes" msgstr "" @@ -640,6 +649,9 @@ msgstr "" msgid "404|Please contact your GitLab administrator if you think this is a mistake." msgstr "" +msgid "7 days" +msgstr "" + msgid "8 hours" msgstr "" @@ -11478,7 +11490,7 @@ msgstr "" msgid "Metrics|Legend label (optional)" msgstr "" -msgid "Metrics|Link contains an invalid time window." +msgid "Metrics|Link contains an invalid time window, please verify the link to see the requested time range." msgstr "" msgid "Metrics|Max" @@ -18797,6 +18809,9 @@ msgstr "" msgid "ThreatMonitoring|Requests" msgstr "" +msgid "ThreatMonitoring|Show last" +msgstr "" + msgid "ThreatMonitoring|Something went wrong, unable to fetch WAF statistics" msgstr "" diff --git a/qa/Gemfile b/qa/Gemfile index 3575ecf13e9..58118340f24 100644 --- a/qa/Gemfile +++ b/qa/Gemfile @@ -19,4 +19,5 @@ group :test do gem 'pry-byebug', '~> 3.5.1', platform: :mri gem "ruby-debug-ide", "~> 0.7.0" gem "debase", "~> 0.2.4.1" + gem 'timecop', '~> 0.9.1' end diff --git a/qa/Gemfile.lock b/qa/Gemfile.lock index 25c7703ef52..6d48a9449a5 100644 --- a/qa/Gemfile.lock +++ b/qa/Gemfile.lock @@ -99,6 +99,7 @@ GEM childprocess (>= 0.5, < 4.0) rubyzip (>= 1.2.2) thread_safe (0.3.6) + timecop (0.9.1) tzinfo (1.2.5) thread_safe (~> 0.1) unf (0.1.4) @@ -128,6 +129,7 @@ DEPENDENCIES rspec_junit_formatter (~> 0.4.1) ruby-debug-ide (~> 0.7.0) selenium-webdriver (~> 3.12) + timecop (~> 0.9.1) BUNDLED WITH 1.17.3 @@ -488,8 +488,9 @@ module QA end autoload :Api, 'qa/support/api' autoload :Dates, 'qa/support/dates' - autoload :Waiter, 'qa/support/waiter' + autoload :Repeater, 'qa/support/repeater' autoload :Retrier, 'qa/support/retrier' + autoload :Waiter, 'qa/support/waiter' autoload :WaitForRequests, 'qa/support/wait_for_requests' end end diff --git a/qa/qa/page/base.rb b/qa/qa/page/base.rb index 2c04fb53440..13f0e1e1994 100644 --- a/qa/qa/page/base.rb +++ b/qa/qa/page/base.rb @@ -26,20 +26,20 @@ module QA wait_for_requests end - def wait(max: 60, interval: 0.1, reload: true) - QA::Support::Waiter.wait(max: max, interval: interval) do + def wait(max: 60, interval: 0.1, reload: true, raise_on_failure: false) + Support::Waiter.wait_until(max_duration: max, sleep_interval: interval, raise_on_failure: raise_on_failure) do yield || (reload && refresh && false) end end - def retry_until(max_attempts: 3, reload: false, sleep_interval: 0) - QA::Support::Retrier.retry_until(max_attempts: max_attempts, reload_page: (reload && self), sleep_interval: sleep_interval) do + def retry_until(max_attempts: 3, reload: false, sleep_interval: 0, raise_on_failure: false) + Support::Retrier.retry_until(max_attempts: max_attempts, reload_page: (reload && self), sleep_interval: sleep_interval, raise_on_failure: raise_on_failure) do yield end end def retry_on_exception(max_attempts: 3, reload: false, sleep_interval: 0.5) - QA::Support::Retrier.retry_on_exception(max_attempts: max_attempts, reload_page: (reload && self), sleep_interval: sleep_interval) do + Support::Retrier.retry_on_exception(max_attempts: max_attempts, reload_page: (reload && self), sleep_interval: sleep_interval) do yield end end diff --git a/qa/qa/resource/events/base.rb b/qa/qa/resource/events/base.rb index b50b620b143..f98a54a6f57 100644 --- a/qa/qa/resource/events/base.rb +++ b/qa/qa/resource/events/base.rb @@ -4,6 +4,7 @@ module QA module Resource module Events MAX_WAIT = 10 + RAISE_ON_FAILURE = true EventNotFoundError = Class.new(RuntimeError) @@ -21,7 +22,7 @@ module QA end def wait_for_event - event_found = QA::Support::Waiter.wait(max: max_wait) do + event_found = Support::Waiter.wait_until(max_duration: max_wait, raise_on_failure: raise_on_failure) do yield end @@ -31,6 +32,10 @@ module QA def max_wait MAX_WAIT end + + def raise_on_failure + RAISE_ON_FAILURE + end end end end diff --git a/qa/qa/support/repeater.rb b/qa/qa/support/repeater.rb new file mode 100644 index 00000000000..53d72f2f410 --- /dev/null +++ b/qa/qa/support/repeater.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +require 'active_support/inflector' + +module QA + module Support + module Repeater + DEFAULT_MAX_WAIT_TIME = 60 + + RetriesExceededError = Class.new(RuntimeError) + WaitExceededError = Class.new(RuntimeError) + + def repeat_until(max_attempts: nil, max_duration: nil, reload_page: nil, sleep_interval: 0, raise_on_failure: true, retry_on_exception: false) + attempts = 0 + start = Time.now + + begin + while remaining_attempts?(attempts, max_attempts) && remaining_time?(start, max_duration) + QA::Runtime::Logger.debug("Attempt number #{attempts + 1}") if max_attempts + + result = yield + return result if result + + sleep_and_reload_if_needed(sleep_interval, reload_page) + attempts += 1 + end + rescue StandardError, RSpec::Expectations::ExpectationNotMetError + raise unless retry_on_exception + + attempts += 1 + if remaining_attempts?(attempts, max_attempts) && remaining_time?(start, max_duration) + sleep_and_reload_if_needed(sleep_interval, reload_page) + + retry + else + raise + end + end + + if raise_on_failure + raise RetriesExceededError, "Retry condition not met after #{max_attempts} #{'attempt'.pluralize(max_attempts)}" unless remaining_attempts?(attempts, max_attempts) + + raise WaitExceededError, "Wait condition not met after #{max_duration} #{'second'.pluralize(max_duration)}" + end + + false + end + + private + + def sleep_and_reload_if_needed(sleep_interval, reload_page) + sleep(sleep_interval) + reload_page.refresh if reload_page + end + + def remaining_attempts?(attempts, max_attempts) + max_attempts ? attempts < max_attempts : true + end + + def remaining_time?(start, max_duration) + max_duration ? Time.now - start < max_duration : true + end + end + end +end diff --git a/qa/qa/support/retrier.rb b/qa/qa/support/retrier.rb index 3b02cb4855b..7b548e95453 100644 --- a/qa/qa/support/retrier.rb +++ b/qa/qa/support/retrier.rb @@ -3,49 +3,61 @@ module QA module Support module Retrier + extend Repeater + module_function def retry_on_exception(max_attempts: 3, reload_page: nil, sleep_interval: 0.5) - QA::Runtime::Logger.debug("with retry_on_exception: max_attempts #{max_attempts}; sleep_interval #{sleep_interval}") - - attempts = 0 + QA::Runtime::Logger.debug( + <<~MSG.tr("\n", ' ') + with retry_on_exception: max_attempts: #{max_attempts}; + reload_page: #{reload_page}; + sleep_interval: #{sleep_interval} + MSG + ) - begin - QA::Runtime::Logger.debug("Attempt number #{attempts + 1}") - yield - rescue StandardError, RSpec::Expectations::ExpectationNotMetError - sleep sleep_interval - reload_page.refresh if reload_page - attempts += 1 + result = nil + repeat_until( + max_attempts: max_attempts, + reload_page: reload_page, + sleep_interval: sleep_interval, + retry_on_exception: true + ) do + result = yield - retry if attempts < max_attempts - QA::Runtime::Logger.debug("Raising exception after #{max_attempts} attempts") - raise + # This method doesn't care what the return value of the block is. + # We set it to `true` so that it doesn't repeat if there's no exception + true end - end - - def retry_until(max_attempts: 3, reload_page: nil, sleep_interval: 0, exit_on_failure: false) - QA::Runtime::Logger.debug("with retry_until: max_attempts #{max_attempts}; sleep_interval #{sleep_interval}; reload_page:#{reload_page}") - attempts = 0 + QA::Runtime::Logger.debug("ended retry_on_exception") - while attempts < max_attempts - QA::Runtime::Logger.debug("Attempt number #{attempts + 1}") - result = yield - return result if result + result + end - sleep sleep_interval + def retry_until(max_attempts: nil, max_duration: nil, reload_page: nil, sleep_interval: 0, raise_on_failure: false, retry_on_exception: false) + # For backwards-compatibility + max_attempts = 3 if max_attempts.nil? && max_duration.nil? - reload_page.refresh if reload_page + start_msg ||= ["with retry_until:"] + start_msg << "max_attempts: #{max_attempts};" if max_attempts + start_msg << "max_duration: #{max_duration};" if max_duration + start_msg << "reload_page: #{reload_page}; sleep_interval: #{sleep_interval}; raise_on_failure: #{raise_on_failure}; retry_on_exception: #{retry_on_exception}" + QA::Runtime::Logger.debug(start_msg.join(' ')) - attempts += 1 - end - - if exit_on_failure - QA::Runtime::Logger.debug("Raising exception after #{max_attempts} attempts") - raise + result = nil + repeat_until( + max_attempts: max_attempts, + max_duration: max_duration, + reload_page: reload_page, + sleep_interval: sleep_interval, + raise_on_failure: raise_on_failure, + retry_on_exception: retry_on_exception + ) do + result = yield end + QA::Runtime::Logger.debug("ended retry_until") - false + result end end end diff --git a/qa/qa/support/waiter.rb b/qa/qa/support/waiter.rb index fdcf2d7e157..73ca0182464 100644 --- a/qa/qa/support/waiter.rb +++ b/qa/qa/support/waiter.rb @@ -3,30 +3,39 @@ module QA module Support module Waiter - DEFAULT_MAX_WAIT_TIME = 60 + extend Repeater module_function - def wait(max: DEFAULT_MAX_WAIT_TIME, interval: 0.1) - QA::Runtime::Logger.debug("with wait: max #{max}; interval #{interval}") - start = Time.now + def wait(max: singleton_class::DEFAULT_MAX_WAIT_TIME, interval: 0.1) + wait_until(max_duration: max, sleep_interval: interval, raise_on_failure: false) do + yield + end + end - while Time.now - start < max - result = yield - if result - log_end(Time.now - start) - return result - end + def wait_until(max_duration: singleton_class::DEFAULT_MAX_WAIT_TIME, reload_page: nil, sleep_interval: 0.1, raise_on_failure: false, retry_on_exception: false) + QA::Runtime::Logger.debug( + <<~MSG.tr("\n", ' ') + with wait_until: max_duration: #{max_duration}; + reload_page: #{reload_page}; + sleep_interval: #{sleep_interval}; + raise_on_failure: #{raise_on_failure} + MSG + ) - sleep(interval) + result = nil + self.repeat_until( + max_duration: max_duration, + reload_page: reload_page, + sleep_interval: sleep_interval, + raise_on_failure: raise_on_failure, + retry_on_exception: retry_on_exception + ) do + result = yield end - log_end(Time.now - start) - - false - end + QA::Runtime::Logger.debug("ended wait_until") - def self.log_end(duration) - QA::Runtime::Logger.debug("ended wait after #{duration} seconds") + result end end end diff --git a/qa/qa/vendor/github/page/login.rb b/qa/qa/vendor/github/page/login.rb index e581edcb7c7..8dd79148043 100644 --- a/qa/qa/vendor/github/page/login.rb +++ b/qa/qa/vendor/github/page/login.rb @@ -12,7 +12,7 @@ module QA fill_in 'password', with: QA::Runtime::Env.github_password click_on 'Sign in' - Support::Retrier.retry_until(exit_on_failure: true, sleep_interval: 35) do + Support::Retrier.retry_until(raise_on_failure: true, sleep_interval: 35) do otp = OnePassword::CLI.new.otp fill_in 'otp', with: otp diff --git a/qa/qa/vendor/jenkins/page/configure.rb b/qa/qa/vendor/jenkins/page/configure.rb index 8851a2564fd..da59060152d 100644 --- a/qa/qa/vendor/jenkins/page/configure.rb +++ b/qa/qa/vendor/jenkins/page/configure.rb @@ -18,7 +18,7 @@ module QA dropdown_element = find('.setting-name', text: "Credentials").find(:xpath, "..").find('select') - QA::Support::Retrier.retry_until(exit_on_failure: true) do + QA::Support::Retrier.retry_until(raise_on_failure: true) do dropdown_element.select "GitLab API token (#{token_description})" dropdown_element.value != '' end diff --git a/qa/qa/vendor/jenkins/page/login.rb b/qa/qa/vendor/jenkins/page/login.rb index 7b3558b25e2..b18c02b5a44 100644 --- a/qa/qa/vendor/jenkins/page/login.rb +++ b/qa/qa/vendor/jenkins/page/login.rb @@ -14,7 +14,7 @@ module QA def visit! super - QA::Support::Retrier.retry_until(sleep_interval: 3, reload_page: page, max_attempts: 20, exit_on_failure: true) do + QA::Support::Retrier.retry_until(sleep_interval: 3, reload_page: page, max_attempts: 20, raise_on_failure: true) do page.has_text? 'Welcome to Jenkins!' end end diff --git a/qa/spec/page/base_spec.rb b/qa/spec/page/base_spec.rb index 9e3f143ea5b..e157eb6ac3e 100644 --- a/qa/spec/page/base_spec.rb +++ b/qa/spec/page/base_spec.rb @@ -69,11 +69,11 @@ describe QA::Page::Base do it 'does not refresh' do expect(subject).not_to receive(:refresh) - subject.wait(max: 0.01) { true } + subject.wait(max: 0.01, raise_on_failure: false) { true } end it 'returns true' do - expect(subject.wait(max: 0.1) { true }).to be_truthy + expect(subject.wait(max: 0.1, raise_on_failure: false) { true }).to be_truthy end end @@ -81,13 +81,13 @@ describe QA::Page::Base do it 'refreshes' do expect(subject).to receive(:refresh).at_least(:once) - subject.wait(max: 0.01) { false } + subject.wait(max: 0.01, raise_on_failure: false) { false } end it 'returns false' do allow(subject).to receive(:refresh) - expect(subject.wait(max: 0.01) { false }).to be_falsey + expect(subject.wait(max: 0.01, raise_on_failure: false) { false }).to be_falsey end end end diff --git a/qa/spec/page/logging_spec.rb b/qa/spec/page/logging_spec.rb index fb89bcd3ab4..0a394e1c38f 100644 --- a/qa/spec/page/logging_spec.rb +++ b/qa/spec/page/logging_spec.rb @@ -31,18 +31,18 @@ describe QA::Support::Page::Logging do expect { subject.wait(max: 0) {} } .to output(/next wait uses reload: true/).to_stdout_from_any_process expect { subject.wait(max: 0) {} } - .to output(/with wait/).to_stdout_from_any_process + .to output(/with wait_until/).to_stdout_from_any_process expect { subject.wait(max: 0) {} } - .to output(/ended wait after .* seconds$/).to_stdout_from_any_process + .to output(/ended wait_until$/).to_stdout_from_any_process end it 'logs wait with reload false' do expect { subject.wait(max: 0, reload: false) {} } .to output(/next wait uses reload: false/).to_stdout_from_any_process expect { subject.wait(max: 0, reload: false) {} } - .to output(/with wait/).to_stdout_from_any_process + .to output(/with wait_until/).to_stdout_from_any_process expect { subject.wait(max: 0, reload: false) {} } - .to output(/ended wait after .* seconds$/).to_stdout_from_any_process + .to output(/ended wait_until$/).to_stdout_from_any_process end it 'logs scroll_to' do diff --git a/qa/spec/resource/events/project_spec.rb b/qa/spec/resource/events/project_spec.rb index b3efdb518f3..dd544ec7ac8 100644 --- a/qa/spec/resource/events/project_spec.rb +++ b/qa/spec/resource/events/project_spec.rb @@ -33,6 +33,7 @@ describe QA::Resource::Events::Project do before do allow(subject).to receive(:max_wait).and_return(0.01) + allow(subject).to receive(:raise_on_failure).and_return(false) allow(subject).to receive(:parse_body).and_return(all_events) end diff --git a/qa/spec/support/repeater_spec.rb b/qa/spec/support/repeater_spec.rb new file mode 100644 index 00000000000..20dca6608f6 --- /dev/null +++ b/qa/spec/support/repeater_spec.rb @@ -0,0 +1,385 @@ +# frozen_string_literal: true + +require 'logger' +require 'timecop' +require 'active_support/core_ext/integer/time' + +describe QA::Support::Repeater do + before do + logger = ::Logger.new $stdout + logger.level = ::Logger::DEBUG + QA::Runtime::Logger.logger = logger + end + + subject do + Module.new do + extend QA::Support::Repeater + end + end + + let(:time_start) { Time.now } + let(:return_value) { "test passed" } + + describe '.repeat_until' do + context 'when raise_on_failure is not provided (default: true)' do + context 'when retry_on_exception is not provided (default: false)' do + context 'when max_duration is provided' do + context 'when max duration is reached' do + it 'raises an exception' do + expect do + Timecop.freeze do + subject.repeat_until(max_duration: 1) do + Timecop.travel(2) + false + end + end + end.to raise_error(QA::Support::Repeater::WaitExceededError, "Wait condition not met after 1 second") + end + + it 'ignores attempts' do + loop_counter = 0 + + expect( + Timecop.freeze do + subject.repeat_until(max_duration: 1) do + loop_counter += 1 + + if loop_counter > 3 + Timecop.travel(1) + return_value + else + false + end + end + end + ).to eq(return_value) + expect(loop_counter).to eq(4) + end + end + + context 'when max duration is not reached' do + it 'returns value from block' do + Timecop.freeze(time_start) do + expect( + subject.repeat_until(max_duration: 1) do + return_value + end + ).to eq(return_value) + end + end + end + end + + context 'when max_attempts is provided' do + context 'when max_attempts is reached' do + it 'raises an exception' do + expect do + Timecop.freeze do + subject.repeat_until(max_attempts: 1) do + false + end + end + end.to raise_error(QA::Support::Repeater::RetriesExceededError, "Retry condition not met after 1 attempt") + end + + it 'ignores duration' do + loop_counter = 0 + + expect( + Timecop.freeze do + subject.repeat_until(max_attempts: 2) do + loop_counter += 1 + Timecop.travel(1.year) + + if loop_counter > 1 + return_value + else + false + end + end + end + ).to eq(return_value) + expect(loop_counter).to eq(2) + end + end + + context 'when max_attempts is not reached' do + it 'returns value from block' do + expect( + Timecop.freeze do + subject.repeat_until(max_attempts: 1) do + return_value + end + end + ).to eq(return_value) + end + end + end + + context 'when both max_attempts and max_duration are provided' do + context 'when max_attempts is reached first' do + it 'raises an exception' do + loop_counter = 0 + expect do + Timecop.freeze do + subject.repeat_until(max_attempts: 1, max_duration: 2) do + loop_counter += 1 + Timecop.travel(time_start + loop_counter) + false + end + end + end.to raise_error(QA::Support::Repeater::RetriesExceededError, "Retry condition not met after 1 attempt") + end + end + + context 'when max_duration is reached first' do + it 'raises an exception' do + loop_counter = 0 + expect do + Timecop.freeze do + subject.repeat_until(max_attempts: 2, max_duration: 1) do + loop_counter += 1 + Timecop.travel(time_start + loop_counter) + false + end + end + end.to raise_error(QA::Support::Repeater::WaitExceededError, "Wait condition not met after 1 second") + end + end + end + end + + context 'when retry_on_exception is true' do + context 'when max duration is reached' do + it 'raises an exception' do + Timecop.freeze do + expect do + subject.repeat_until(max_duration: 1, retry_on_exception: true) do + Timecop.travel(2) + + raise "this should be raised" + end + end.to raise_error(RuntimeError, "this should be raised") + end + end + + it 'does not raise an exception until max_duration is reached' do + loop_counter = 0 + + Timecop.freeze(time_start) do + expect do + subject.repeat_until(max_duration: 2, retry_on_exception: true) do + loop_counter += 1 + Timecop.travel(time_start + loop_counter) + + raise "this should be raised" + end + end.to raise_error(RuntimeError, "this should be raised") + end + expect(loop_counter).to eq(2) + end + end + + context 'when max duration is not reached' do + it 'returns value from block' do + loop_counter = 0 + + Timecop.freeze(time_start) do + expect( + subject.repeat_until(max_duration: 3, retry_on_exception: true) do + loop_counter += 1 + Timecop.travel(time_start + loop_counter) + + raise "this should not be raised" if loop_counter == 1 + + return_value + end + ).to eq(return_value) + end + expect(loop_counter).to eq(2) + end + end + + context 'when both max_attempts and max_duration are provided' do + context 'when max_attempts is reached first' do + it 'raises an exception' do + loop_counter = 0 + expect do + Timecop.freeze do + subject.repeat_until(max_attempts: 1, max_duration: 2, retry_on_exception: true) do + loop_counter += 1 + Timecop.travel(time_start + loop_counter) + false + end + end + end.to raise_error(QA::Support::Repeater::RetriesExceededError, "Retry condition not met after 1 attempt") + end + end + + context 'when max_duration is reached first' do + it 'raises an exception' do + loop_counter = 0 + expect do + Timecop.freeze do + subject.repeat_until(max_attempts: 2, max_duration: 1, retry_on_exception: true) do + loop_counter += 1 + Timecop.travel(time_start + loop_counter) + false + end + end + end.to raise_error(QA::Support::Repeater::WaitExceededError, "Wait condition not met after 1 second") + end + end + end + end + end + + context 'when raise_on_failure is false' do + context 'when retry_on_exception is not provided (default: false)' do + context 'when max duration is reached' do + def test_wait + Timecop.freeze do + subject.repeat_until(max_duration: 1, raise_on_failure: false) do + Timecop.travel(2) + return_value + end + end + end + + it 'does not raise an exception' do + expect { test_wait }.not_to raise_error + end + + it 'returns the value from the block' do + expect(test_wait).to eq(return_value) + end + end + + context 'when max duration is not reached' do + it 'returns the value from the block' do + Timecop.freeze do + expect( + subject.repeat_until(max_duration: 1, raise_on_failure: false) do + return_value + end + ).to eq(return_value) + end + end + + it 'raises an exception' do + Timecop.freeze do + expect do + subject.repeat_until(max_duration: 1, raise_on_failure: false) do + raise "this should be raised" + end + end.to raise_error(RuntimeError, "this should be raised") + end + end + end + + context 'when both max_attempts and max_duration are provided' do + shared_examples 'repeat until' do |max_attempts:, max_duration:| + it "returns when #{max_attempts < max_duration ? 'max_attempts' : 'max_duration'} is reached" do + loop_counter = 0 + + expect( + Timecop.freeze do + subject.repeat_until(max_attempts: max_attempts, max_duration: max_duration, raise_on_failure: false) do + loop_counter += 1 + Timecop.travel(time_start + loop_counter) + false + end + end + ).to eq(false) + expect(loop_counter).to eq(1) + end + end + + context 'when max_attempts is reached first' do + it_behaves_like 'repeat until', max_attempts: 1, max_duration: 2 + end + + context 'when max_duration is reached first' do + it_behaves_like 'repeat until', max_attempts: 2, max_duration: 1 + end + end + end + + context 'when retry_on_exception is true' do + context 'when max duration is reached' do + def test_wait + Timecop.freeze do + subject.repeat_until(max_duration: 1, raise_on_failure: false, retry_on_exception: true) do + Timecop.travel(2) + return_value + end + end + end + + it 'does not raise an exception' do + expect { test_wait }.not_to raise_error + end + + it 'returns the value from the block' do + expect(test_wait).to eq(return_value) + end + end + + context 'when max duration is not reached' do + before do + @loop_counter = 0 + end + + def test_wait_with_counter + Timecop.freeze(time_start) do + subject.repeat_until(max_duration: 3, raise_on_failure: false, retry_on_exception: true) do + @loop_counter += 1 + Timecop.travel(time_start + @loop_counter) + + raise "this should not be raised" if @loop_counter == 1 + + return_value + end + end + end + + it 'does not raise an exception' do + expect { test_wait_with_counter }.not_to raise_error + end + + it 'returns the value from the block' do + expect(test_wait_with_counter).to eq(return_value) + expect(@loop_counter).to eq(2) + end + end + + context 'when both max_attempts and max_duration are provided' do + shared_examples 'repeat until' do |max_attempts:, max_duration:| + it "returns when #{max_attempts < max_duration ? 'max_attempts' : 'max_duration'} is reached" do + loop_counter = 0 + + expect( + Timecop.freeze do + subject.repeat_until(max_attempts: max_attempts, max_duration: max_duration, raise_on_failure: false, retry_on_exception: true) do + loop_counter += 1 + Timecop.travel(time_start + loop_counter) + false + end + end + ).to eq(false) + expect(loop_counter).to eq(1) + end + end + + context 'when max_attempts is reached first' do + it_behaves_like 'repeat until', max_attempts: 1, max_duration: 2 + end + + context 'when max_duration is reached first' do + it_behaves_like 'repeat until', max_attempts: 2, max_duration: 1 + end + end + end + end + end +end diff --git a/qa/spec/support/retrier_spec.rb b/qa/spec/support/retrier_spec.rb new file mode 100644 index 00000000000..fbe66a680f9 --- /dev/null +++ b/qa/spec/support/retrier_spec.rb @@ -0,0 +1,126 @@ +# frozen_string_literal: true + +require 'logger' +require 'timecop' + +describe QA::Support::Retrier do + before do + logger = ::Logger.new $stdout + logger.level = ::Logger::DEBUG + QA::Runtime::Logger.logger = logger + end + + describe '.retry_until' do + context 'when the condition is true' do + it 'logs max attempts (3 by default)' do + expect { subject.retry_until { true } } + .to output(/with retry_until: max_attempts: 3; reload_page: ; sleep_interval: 0; raise_on_failure: false; retry_on_exception: false/).to_stdout_from_any_process + end + + it 'logs max duration' do + expect { subject.retry_until(max_duration: 1) { true } } + .to output(/with retry_until: max_duration: 1; reload_page: ; sleep_interval: 0; raise_on_failure: false; retry_on_exception: false/).to_stdout_from_any_process + end + + it 'logs the end' do + expect { subject.retry_until { true } } + .to output(/ended retry_until$/).to_stdout_from_any_process + end + end + + context 'when the condition is false' do + it 'logs the start' do + expect { subject.retry_until(max_duration: 0) { false } } + .to output(/with retry_until: max_duration: 0; reload_page: ; sleep_interval: 0; raise_on_failure: false; retry_on_exception: false/).to_stdout_from_any_process + end + + it 'logs the end' do + expect { subject.retry_until(max_duration: 0) { false } } + .to output(/ended retry_until$/).to_stdout_from_any_process + end + end + + context 'when max_duration and max_attempts are nil' do + it 'sets max attempts to 3 by default' do + expect(subject).to receive(:repeat_until).with(hash_including(max_attempts: 3)) + + subject.retry_until + end + end + + it 'sets sleep_interval to 0 by default' do + expect(subject).to receive(:repeat_until).with(hash_including(sleep_interval: 0)) + + subject.retry_until + end + + it 'sets raise_on_failure to false by default' do + expect(subject).to receive(:repeat_until).with(hash_including(raise_on_failure: false)) + + subject.retry_until + end + + it 'sets retry_on_exception to false by default' do + expect(subject).to receive(:repeat_until).with(hash_including(retry_on_exception: false)) + + subject.retry_until + end + end + + describe '.retry_on_exception' do + context 'when the condition is true' do + it 'logs max_attempts, reload_page, and sleep_interval parameters' do + expect { subject.retry_on_exception(max_attempts: 1, reload_page: nil, sleep_interval: 0) { true } } + .to output(/with retry_on_exception: max_attempts: 1; reload_page: ; sleep_interval: 0/).to_stdout_from_any_process + end + + it 'logs the end' do + expect { subject.retry_on_exception(max_attempts: 1, reload_page: nil, sleep_interval: 0) { true } } + .to output(/ended retry_on_exception$/).to_stdout_from_any_process + end + end + + context 'when the condition is false' do + it 'logs the start' do + expect { subject.retry_on_exception(max_attempts: 1, reload_page: nil, sleep_interval: 0) { false } } + .to output(/with retry_on_exception: max_attempts: 1; reload_page: ; sleep_interval: 0/).to_stdout_from_any_process + end + + it 'logs the end' do + expect { subject.retry_on_exception(max_attempts: 1, reload_page: nil, sleep_interval: 0) { false } } + .to output(/ended retry_on_exception$/).to_stdout_from_any_process + end + end + + it 'does not repeat if no exception is raised' do + loop_counter = 0 + return_value = "test passed" + + expect( + subject.retry_on_exception(max_attempts: 2) do + loop_counter += 1 + return_value + end + ).to eq(return_value) + expect(loop_counter).to eq(1) + end + + it 'sets retry_on_exception to true' do + expect(subject).to receive(:repeat_until).with(hash_including(retry_on_exception: true)) + + subject.retry_on_exception + end + + it 'sets max_attempts to 3 by default' do + expect(subject).to receive(:repeat_until).with(hash_including(max_attempts: 3)) + + subject.retry_on_exception + end + + it 'sets sleep_interval to 0.5 by default' do + expect(subject).to receive(:repeat_until).with(hash_including(sleep_interval: 0.5)) + + subject.retry_on_exception + end + end +end diff --git a/qa/spec/support/waiter_spec.rb b/qa/spec/support/waiter_spec.rb index 8283b65e1be..06e404c862a 100644 --- a/qa/spec/support/waiter_spec.rb +++ b/qa/spec/support/waiter_spec.rb @@ -9,29 +9,53 @@ describe QA::Support::Waiter do QA::Runtime::Logger.logger = logger end - describe '.wait' do + describe '.wait_until' do context 'when the condition is true' do it 'logs the start' do - expect { subject.wait(max: 0) {} } - .to output(/with wait: max 0; interval 0.1/).to_stdout_from_any_process + expect { subject.wait_until(max_duration: 0, raise_on_failure: false) { true } } + .to output(/with wait_until: max_duration: 0; reload_page: ; sleep_interval: 0.1/).to_stdout_from_any_process end it 'logs the end' do - expect { subject.wait(max: 0) {} } - .to output(/ended wait after .* seconds$/).to_stdout_from_any_process + expect { subject.wait_until(max_duration: 0, raise_on_failure: false) { true } } + .to output(/ended wait_until$/).to_stdout_from_any_process end end context 'when the condition is false' do it 'logs the start' do - expect { subject.wait(max: 0) { false } } - .to output(/with wait: max 0; interval 0.1/).to_stdout_from_any_process + expect { subject.wait_until(max_duration: 0, raise_on_failure: false) { false } } + .to output(/with wait_until: max_duration: 0; reload_page: ; sleep_interval: 0.1/).to_stdout_from_any_process end it 'logs the end' do - expect { subject.wait(max: 0) { false } } - .to output(/ended wait after .* seconds$/).to_stdout_from_any_process + expect { subject.wait_until(max_duration: 0, raise_on_failure: false) { false } } + .to output(/ended wait_until$/).to_stdout_from_any_process end end + + it 'sets max_duration to 60 by default' do + expect(subject).to receive(:repeat_until).with(hash_including(max_duration: 60)) + + subject.wait_until + end + + it 'sets sleep_interval to 0.1 by default' do + expect(subject).to receive(:repeat_until).with(hash_including(sleep_interval: 0.1)) + + subject.wait_until + end + + it 'sets raise_on_failure to false by default' do + expect(subject).to receive(:repeat_until).with(hash_including(raise_on_failure: false)) + + subject.wait_until + end + + it 'sets retry_on_exception to false by default' do + expect(subject).to receive(:repeat_until).with(hash_including(retry_on_exception: false)) + + subject.wait_until + end end end diff --git a/spec/frontend/behaviors/markdown/paste_markdown_table_spec.js b/spec/frontend/behaviors/markdown/paste_markdown_table_spec.js index a8177a5ad39..a98919e2113 100644 --- a/spec/frontend/behaviors/markdown/paste_markdown_table_spec.js +++ b/spec/frontend/behaviors/markdown/paste_markdown_table_spec.js @@ -10,9 +10,9 @@ describe('PasteMarkdownTable', () => { value: { getData: jest.fn().mockImplementation(type => { if (type === 'text/html') { - return '<table><tr><td></td></tr></table>'; + return '<table><tr><td>First</td><td>Second</td></tr></table>'; } - return 'hello world'; + return 'First\tSecond'; }), }, }); @@ -24,39 +24,48 @@ describe('PasteMarkdownTable', () => { it('return false when no HTML data is provided', () => { data.types = ['text/plain']; - expect(PasteMarkdownTable.isTable(data)).toBe(false); + expect(new PasteMarkdownTable(data).isTable()).toBe(false); }); it('returns false when no text data is provided', () => { data.types = ['text/html']; - expect(PasteMarkdownTable.isTable(data)).toBe(false); + expect(new PasteMarkdownTable(data).isTable()).toBe(false); }); it('returns true when a table is provided in both text and HTML', () => { data.types = ['text/html', 'text/plain']; - expect(PasteMarkdownTable.isTable(data)).toBe(true); + expect(new PasteMarkdownTable(data).isTable()).toBe(true); }); it('returns false when no HTML table is included', () => { data.types = ['text/html', 'text/plain']; data.getData = jest.fn().mockImplementation(() => 'nothing'); - expect(PasteMarkdownTable.isTable(data)).toBe(false); + expect(new PasteMarkdownTable(data).isTable()).toBe(false); }); - }); - describe('convertToTableMarkdown', () => { - let converter; + it('returns false when the number of rows are not consistent', () => { + data.types = ['text/html', 'text/plain']; + data.getData = jest.fn().mockImplementation(mimeType => { + if (mimeType === 'text/html') { + return '<table><tr><td>def test<td></tr></table>'; + } + return "def test\n 'hello'\n"; + }); - beforeEach(() => { - converter = new PasteMarkdownTable(data); + expect(new PasteMarkdownTable(data).isTable()).toBe(false); }); + }); + describe('convertToTableMarkdown', () => { it('returns a Markdown table', () => { + data.types = ['text/html', 'text/plain']; data.getData = jest.fn().mockImplementation(type => { - if (type === 'text/plain') { + if (type === 'text/html') { + return '<table><tr><td>First</td><td>Last</td><tr><td>John</td><td>Doe</td><tr><td>Jane</td><td>Doe</td></table>'; + } else if (type === 'text/plain') { return 'First\tLast\nJohn\tDoe\nJane\tDoe'; } @@ -70,12 +79,18 @@ describe('PasteMarkdownTable', () => { '| Jane | Doe |', ].join('\n'); + const converter = new PasteMarkdownTable(data); + + expect(converter.isTable()).toBe(true); expect(converter.convertToTableMarkdown()).toBe(expected); }); it('returns a Markdown table with rows normalized', () => { + data.types = ['text/html', 'text/plain']; data.getData = jest.fn().mockImplementation(type => { - if (type === 'text/plain') { + if (type === 'text/html') { + return '<table><tr><td>First</td><td>Last</td><tr><td>John</td><td>Doe</td><tr><td>Jane</td><td>/td></table>'; + } else if (type === 'text/plain') { return 'First\tLast\nJohn\tDoe\nJane'; } @@ -89,6 +104,9 @@ describe('PasteMarkdownTable', () => { '| Jane | |', ].join('\n'); + const converter = new PasteMarkdownTable(data); + + expect(converter.isTable()).toBe(true); expect(converter.convertToTableMarkdown()).toBe(expected); }); }); diff --git a/spec/frontend/monitoring/components/dashboard_spec.js b/spec/frontend/monitoring/components/dashboard_spec.js index 8a10857d0ff..f77e8b61050 100644 --- a/spec/frontend/monitoring/components/dashboard_spec.js +++ b/spec/frontend/monitoring/components/dashboard_spec.js @@ -10,7 +10,6 @@ import DateTimePicker from '~/monitoring/components/date_time_picker/date_time_p import GroupEmptyState from '~/monitoring/components/group_empty_state.vue'; import { createStore } from '~/monitoring/stores'; import * as types from '~/monitoring/stores/mutation_types'; -import * as monitoringUtils from '~/monitoring/utils'; import { setupComponentStore, propsData } from '../init_utils'; import { metricsGroupsAPIResponse, @@ -24,13 +23,12 @@ const localVue = createLocalVue(); const expectedPanelCount = 2; describe('Dashboard', () => { - let DashboardComponent; let store; let wrapper; let mock; const createShallowWrapper = (props = {}, options = {}) => { - wrapper = shallowMount(localVue.extend(DashboardComponent), { + wrapper = shallowMount(Dashboard, { localVue, sync: false, propsData: { ...propsData, ...props }, @@ -40,7 +38,7 @@ describe('Dashboard', () => { }; const createMountedWrapper = (props = {}, options = {}) => { - wrapper = mount(localVue.extend(DashboardComponent), { + wrapper = mount(Dashboard, { localVue, sync: false, propsData: { ...propsData, ...props }, @@ -51,7 +49,6 @@ describe('Dashboard', () => { beforeEach(() => { store = createStore(); - DashboardComponent = localVue.extend(Dashboard); mock = new MockAdapter(axios); }); @@ -137,7 +134,6 @@ describe('Dashboard', () => { }); it('fetches the metrics data with proper time window', done => { - const getTimeDiffSpy = jest.spyOn(monitoringUtils, 'getTimeDiff'); jest.spyOn(store, 'dispatch'); createMountedWrapper( @@ -154,7 +150,6 @@ describe('Dashboard', () => { .$nextTick() .then(() => { expect(store.dispatch).toHaveBeenCalled(); - expect(getTimeDiffSpy).toHaveBeenCalled(); done(); }) diff --git a/spec/frontend/monitoring/components/dashboard_time_url_spec.js b/spec/frontend/monitoring/components/dashboard_time_url_spec.js index 8dc450cf131..747f07bcd0c 100644 --- a/spec/frontend/monitoring/components/dashboard_time_url_spec.js +++ b/spec/frontend/monitoring/components/dashboard_time_url_spec.js @@ -1,10 +1,10 @@ -import { mount, createLocalVue } from '@vue/test-utils'; +import { mount } from '@vue/test-utils'; import createFlash from '~/flash'; +import MockAdapter from 'axios-mock-adapter'; import Dashboard from '~/monitoring/components/dashboard.vue'; import { createStore } from '~/monitoring/stores'; import { propsData } from '../init_utils'; - -const localVue = createLocalVue(); +import axios from '~/lib/utils/axios_utils'; jest.mock('~/flash'); @@ -15,10 +15,10 @@ jest.mock('~/lib/utils/url_utility', () => ({ describe('dashboard invalid url parameters', () => { let store; let wrapper; + let mock; const createMountedWrapper = (props = {}, options = {}) => { - wrapper = mount(localVue.extend(Dashboard), { - localVue, + wrapper = mount(Dashboard, { sync: false, propsData: { ...propsData, ...props }, store, @@ -28,12 +28,14 @@ describe('dashboard invalid url parameters', () => { beforeEach(() => { store = createStore(); + mock = new MockAdapter(axios); }); afterEach(() => { if (wrapper) { wrapper.destroy(); } + mock.restore(); }); it('shows an error message if invalid url parameters are passed', done => { @@ -46,7 +48,6 @@ describe('dashboard invalid url parameters', () => { .$nextTick() .then(() => { expect(createFlash).toHaveBeenCalled(); - done(); }) .catch(done.fail); diff --git a/spec/frontend/monitoring/components/dashboard_time_window_spec.js b/spec/frontend/monitoring/components/dashboard_time_window_spec.js index d49af6f84cb..658d3b68a76 100644 --- a/spec/frontend/monitoring/components/dashboard_time_window_spec.js +++ b/spec/frontend/monitoring/components/dashboard_time_window_spec.js @@ -1,4 +1,4 @@ -import { mount, createLocalVue } from '@vue/test-utils'; +import { mount } from '@vue/test-utils'; import { GlDropdownItem } from '@gitlab/ui'; import MockAdapter from 'axios-mock-adapter'; import axios from '~/lib/utils/axios_utils'; @@ -8,8 +8,6 @@ import { createStore } from '~/monitoring/stores'; import { propsData, setupComponentStore } from '../init_utils'; import { metricsGroupsAPIResponse, mockApiEndpoint } from '../mock_data'; -const localVue = createLocalVue(); - jest.mock('~/lib/utils/url_utility', () => ({ getParameterValues: jest.fn().mockImplementation(param => { if (param === 'start') return ['2019-10-01T18:27:47.000Z']; @@ -25,8 +23,7 @@ describe('dashboard time window', () => { let mock; const createComponentWrapperMounted = (props = {}, options = {}) => { - wrapper = mount(localVue.extend(Dashboard), { - localVue, + wrapper = mount(Dashboard, { sync: false, propsData: { ...propsData, ...props }, store, diff --git a/spec/frontend/monitoring/components/date_time_picker/date_time_picker_spec.js b/spec/frontend/monitoring/components/date_time_picker/date_time_picker_spec.js index 88463d781ee..ba40ced9545 100644 --- a/spec/frontend/monitoring/components/date_time_picker/date_time_picker_spec.js +++ b/spec/frontend/monitoring/components/date_time_picker/date_time_picker_spec.js @@ -3,10 +3,8 @@ import DateTimePicker from '~/monitoring/components/date_time_picker/date_time_p import { timeWindows } from '~/monitoring/constants'; const timeWindowsCount = Object.keys(timeWindows).length; -const selectedTimeWindow = { - start: '2019-10-10T07:00:00.000Z', - end: '2019-10-13T07:00:00.000Z', -}; +const start = '2019-10-10T07:00:00.000Z'; +const end = '2019-10-13T07:00:00.000Z'; const selectedTimeWindowText = `3 days`; describe('DateTimePicker', () => { @@ -28,7 +26,8 @@ describe('DateTimePicker', () => { dateTimePicker = mount(DateTimePicker, { propsData: { timeWindows, - selectedTimeWindow, + start, + end, ...props, }, sync: false, @@ -66,10 +65,8 @@ describe('DateTimePicker', () => { it('renders inputs with h/m/s truncated if its all 0s', done => { createComponent({ - selectedTimeWindow: { - start: '2019-10-10T00:00:00.000Z', - end: '2019-10-14T00:10:00.000Z', - }, + start: '2019-10-10T00:00:00.000Z', + end: '2019-10-14T00:10:00.000Z', }); dateTimePicker.vm.$nextTick(() => { expect(dateTimePicker.find('#custom-time-from').element.value).toBe('2019-10-10'); @@ -98,8 +95,10 @@ describe('DateTimePicker', () => { }); }); - it('renders a disabled apply button on load', () => { - createComponent(); + it('renders a disabled apply button on wrong input', () => { + createComponent({ + start: 'invalid-input-date', + }); expect(applyButtonElement().getAttribute('disabled')).toBe('disabled'); }); @@ -131,29 +130,29 @@ describe('DateTimePicker', () => { fillInputAndBlur('#custom-time-from', '2019-10-01') .then(() => fillInputAndBlur('#custom-time-to', '2019-10-19')) .then(() => { - dateTimePicker.vm.$nextTick(() => { - expect(applyButtonElement().getAttribute('disabled')).toBeNull(); - done(); - }); + expect(applyButtonElement().getAttribute('disabled')).toBeNull(); + done(); }) - .catch(done); + .catch(done.fail); }); - it('returns an object when apply is clicked', done => { + it('emits dates in an object when apply is clicked', done => { createComponent(); fillInputAndBlur('#custom-time-from', '2019-10-01') .then(() => fillInputAndBlur('#custom-time-to', '2019-10-19')) .then(() => { - jest.spyOn(dateTimePicker.vm, '$emit'); applyButtonElement().click(); - expect(dateTimePicker.vm.$emit).toHaveBeenCalledWith('onApply', { - end: '2019-10-19T00:00:00Z', - start: '2019-10-01T00:00:00Z', - }); + expect(dateTimePicker.emitted().apply).toHaveLength(1); + expect(dateTimePicker.emitted().apply[0]).toEqual([ + { + end: '2019-10-19T00:00:00Z', + start: '2019-10-01T00:00:00Z', + }, + ]); done(); }) - .catch(done); + .catch(done.fail); }); it('hides the popover with cancel button', done => { diff --git a/spec/frontend/monitoring/components/graph_group_spec.js b/spec/frontend/monitoring/components/graph_group_spec.js index 43ca17c3cbc..edd08cdb4c9 100644 --- a/spec/frontend/monitoring/components/graph_group_spec.js +++ b/spec/frontend/monitoring/components/graph_group_spec.js @@ -1,9 +1,7 @@ -import { shallowMount, createLocalVue } from '@vue/test-utils'; +import { shallowMount } from '@vue/test-utils'; import GraphGroup from '~/monitoring/components/graph_group.vue'; import Icon from '~/vue_shared/components/icon.vue'; -const localVue = createLocalVue(); - describe('Graph group component', () => { let wrapper; @@ -12,10 +10,9 @@ describe('Graph group component', () => { const findCaretIcon = () => wrapper.find(Icon); const createComponent = propsData => { - wrapper = shallowMount(localVue.extend(GraphGroup), { + wrapper = shallowMount(GraphGroup, { propsData, sync: false, - localVue, }); }; diff --git a/spec/frontend/vue_shared/components/callout_spec.js b/spec/frontend/vue_shared/components/callout_spec.js index 91208dfb31a..7c9bb6b4650 100644 --- a/spec/frontend/vue_shared/components/callout_spec.js +++ b/spec/frontend/vue_shared/components/callout_spec.js @@ -1,17 +1,14 @@ -import { createLocalVue, shallowMount } from '@vue/test-utils'; +import { shallowMount } from '@vue/test-utils'; import Callout from '~/vue_shared/components/callout.vue'; const TEST_MESSAGE = 'This is a callout message!'; const TEST_SLOT = '<button>This is a callout slot!</button>'; -const localVue = createLocalVue(); - describe('Callout Component', () => { let wrapper; const factory = options => { - wrapper = shallowMount(localVue.extend(Callout), { - localVue, + wrapper = shallowMount(Callout, { ...options, }); }; diff --git a/spec/frontend/vue_shared/components/expand_button_spec.js b/spec/frontend/vue_shared/components/expand_button_spec.js index e0893d02843..3b1c8f6219c 100644 --- a/spec/frontend/vue_shared/components/expand_button_spec.js +++ b/spec/frontend/vue_shared/components/expand_button_spec.js @@ -1,5 +1,5 @@ import Vue from 'vue'; -import { mount, createLocalVue } from '@vue/test-utils'; +import { mount } from '@vue/test-utils'; import ExpandButton from '~/vue_shared/components/expand_button.vue'; const text = { @@ -14,10 +14,7 @@ describe('Expand button', () => { const expanderAppendEl = () => wrapper.find('.js-text-expander-append'); const factory = (options = {}) => { - const localVue = createLocalVue(); - - wrapper = mount(localVue.extend(ExpandButton), { - localVue, + wrapper = mount(ExpandButton, { ...options, }); }; diff --git a/spec/frontend/vue_shared/components/issue/related_issuable_item_spec.js b/spec/frontend/vue_shared/components/issue/related_issuable_item_spec.js index 21d05471d51..85cd90d2f8c 100644 --- a/spec/frontend/vue_shared/components/issue/related_issuable_item_spec.js +++ b/spec/frontend/vue_shared/components/issue/related_issuable_item_spec.js @@ -1,5 +1,5 @@ import Vue from 'vue'; -import { mount, createLocalVue } from '@vue/test-utils'; +import { mount } from '@vue/test-utils'; import { formatDate } from '~/lib/utils/datetime_utility'; import RelatedIssuableItem from '~/vue_shared/components/issue/related_issuable_item.vue'; import { @@ -29,10 +29,7 @@ describe('RelatedIssuableItem', () => { }; beforeEach(() => { - const localVue = createLocalVue(); - - wrapper = mount(localVue.extend(RelatedIssuableItem), { - localVue, + wrapper = mount(RelatedIssuableItem, { slots, sync: false, attachToDocument: true, diff --git a/spec/frontend/vue_shared/components/markdown/suggestion_diff_header_spec.js b/spec/frontend/vue_shared/components/markdown/suggestion_diff_header_spec.js index 71f9b5e3244..3d42c02ebb6 100644 --- a/spec/frontend/vue_shared/components/markdown/suggestion_diff_header_spec.js +++ b/spec/frontend/vue_shared/components/markdown/suggestion_diff_header_spec.js @@ -1,9 +1,7 @@ import { GlLoadingIcon } from '@gitlab/ui'; -import { shallowMount, createLocalVue } from '@vue/test-utils'; +import { shallowMount } from '@vue/test-utils'; import SuggestionDiffHeader from '~/vue_shared/components/markdown/suggestion_diff_header.vue'; -const localVue = createLocalVue(); - const DEFAULT_PROPS = { canApply: true, isApplied: false, @@ -14,12 +12,11 @@ describe('Suggestion Diff component', () => { let wrapper; const createComponent = props => { - wrapper = shallowMount(localVue.extend(SuggestionDiffHeader), { + wrapper = shallowMount(SuggestionDiffHeader, { propsData: { ...DEFAULT_PROPS, ...props, }, - localVue, sync: false, attachToDocument: true, }); diff --git a/spec/frontend/vue_shared/components/notes/system_note_spec.js b/spec/frontend/vue_shared/components/notes/system_note_spec.js index 603c37c6c49..080dd778e29 100644 --- a/spec/frontend/vue_shared/components/notes/system_note_spec.js +++ b/spec/frontend/vue_shared/components/notes/system_note_spec.js @@ -1,12 +1,10 @@ -import { createLocalVue, mount } from '@vue/test-utils'; +import { mount } from '@vue/test-utils'; import IssueSystemNote from '~/vue_shared/components/notes/system_note.vue'; import createStore from '~/notes/stores'; import initMRPopovers from '~/mr_popover/index'; jest.mock('~/mr_popover/index', () => jest.fn()); -const localVue = createLocalVue(); - describe('system note component', () => { let vm; let props; @@ -34,7 +32,6 @@ describe('system note component', () => { vm = mount(IssueSystemNote, { store, - localVue, propsData: props, attachToDocument: true, sync: false, diff --git a/spec/frontend/vue_shared/components/notes/timeline_entry_item_spec.js b/spec/frontend/vue_shared/components/notes/timeline_entry_item_spec.js index be6c58f0683..f73d3edec5d 100644 --- a/spec/frontend/vue_shared/components/notes/timeline_entry_item_spec.js +++ b/spec/frontend/vue_shared/components/notes/timeline_entry_item_spec.js @@ -1,14 +1,11 @@ -import { shallowMount, createLocalVue } from '@vue/test-utils'; +import { shallowMount } from '@vue/test-utils'; import TimelineEntryItem from '~/vue_shared/components/notes/timeline_entry_item.vue'; describe(`TimelineEntryItem`, () => { let wrapper; const factory = (options = {}) => { - const localVue = createLocalVue(); - wrapper = shallowMount(TimelineEntryItem, { - localVue, ...options, }); }; diff --git a/spec/frontend/vue_shared/components/pagination_links_spec.js b/spec/frontend/vue_shared/components/pagination_links_spec.js index efa5825d92f..3c53cda45f5 100644 --- a/spec/frontend/vue_shared/components/pagination_links_spec.js +++ b/spec/frontend/vue_shared/components/pagination_links_spec.js @@ -1,4 +1,4 @@ -import { mount, createLocalVue } from '@vue/test-utils'; +import { mount } from '@vue/test-utils'; import { GlPagination } from '@gitlab/ui'; import PaginationLinks from '~/vue_shared/components/pagination_links.vue'; import { @@ -10,8 +10,6 @@ import { LABEL_LAST_PAGE, } from '~/vue_shared/components/pagination/constants'; -const localVue = createLocalVue(); - describe('Pagination links component', () => { const pageInfo = { page: 3, @@ -38,7 +36,6 @@ describe('Pagination links component', () => { change: changeMock, pageInfo, }, - localVue, sync: false, }); }; diff --git a/spec/frontend/vue_shared/components/time_ago_tooltip_spec.js b/spec/frontend/vue_shared/components/time_ago_tooltip_spec.js index ebba0cc4ad4..49591c3ce1c 100644 --- a/spec/frontend/vue_shared/components/time_ago_tooltip_spec.js +++ b/spec/frontend/vue_shared/components/time_ago_tooltip_spec.js @@ -1,4 +1,4 @@ -import { shallowMount, createLocalVue } from '@vue/test-utils'; +import { shallowMount } from '@vue/test-utils'; import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; import { formatDate, getTimeago } from '~/lib/utils/datetime_utility'; @@ -10,7 +10,6 @@ describe('Time ago with tooltip component', () => { attachToDocument: true, sync: false, propsData, - localVue: createLocalVue(), }); }; const timestamp = '2017-05-08T14:57:39.781Z'; diff --git a/spec/frontend/vue_shared/directives/track_event_spec.js b/spec/frontend/vue_shared/directives/track_event_spec.js index c9b0520ab2c..e1009e5079a 100644 --- a/spec/frontend/vue_shared/directives/track_event_spec.js +++ b/spec/frontend/vue_shared/directives/track_event_spec.js @@ -1,5 +1,5 @@ import Vue from 'vue'; -import { createLocalVue, shallowMount } from '@vue/test-utils'; +import { shallowMount } from '@vue/test-utils'; import Tracking from '~/tracking'; import TrackEvent from '~/vue_shared/directives/track_event'; @@ -17,15 +17,12 @@ const Component = Vue.component('dummy-element', { template: '<button id="trackable" v-track-event="trackingOptions"></button>', }); -const localVue = createLocalVue(); let wrapper; let button; describe('Error Tracking directive', () => { beforeEach(() => { - wrapper = shallowMount(localVue.extend(Component), { - localVue, - }); + wrapper = shallowMount(Component, {}); button = wrapper.find('#trackable'); }); diff --git a/spec/frontend/vue_shared/droplab_dropdown_button_spec.js b/spec/frontend/vue_shared/droplab_dropdown_button_spec.js index 22295721328..e57c730ecee 100644 --- a/spec/frontend/vue_shared/droplab_dropdown_button_spec.js +++ b/spec/frontend/vue_shared/droplab_dropdown_button_spec.js @@ -1,4 +1,4 @@ -import { mount, createLocalVue } from '@vue/test-utils'; +import { mount } from '@vue/test-utils'; import DroplabDropdownButton from '~/vue_shared/components/droplab_dropdown_button.vue'; @@ -18,11 +18,8 @@ const createComponent = ({ dropdownClass = '', actions = mockActions, defaultAction = 0, -}) => { - const localVue = createLocalVue(); - - return mount(DroplabDropdownButton, { - localVue, +}) => + mount(DroplabDropdownButton, { propsData: { size, dropdownClass, @@ -30,7 +27,6 @@ const createComponent = ({ defaultAction, }, }); -}; describe('DroplabDropdownButton', () => { let wrapper; diff --git a/spec/frontend/vue_shared/mixins/gl_feature_flags_mixin_spec.js b/spec/frontend/vue_shared/mixins/gl_feature_flags_mixin_spec.js index a3e3270a4e8..3ce12caf95a 100644 --- a/spec/frontend/vue_shared/mixins/gl_feature_flags_mixin_spec.js +++ b/spec/frontend/vue_shared/mixins/gl_feature_flags_mixin_spec.js @@ -1,8 +1,6 @@ -import { createLocalVue, shallowMount } from '@vue/test-utils'; +import { shallowMount } from '@vue/test-utils'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; -const localVue = createLocalVue(); - describe('GitLab Feature Flags Mixin', () => { let wrapper; @@ -20,7 +18,6 @@ describe('GitLab Feature Flags Mixin', () => { }; wrapper = shallowMount(component, { - localVue, provide: { glFeatures: { ...(gon.features || {}) }, }, diff --git a/spec/javascripts/dropzone_input_spec.js b/spec/javascripts/dropzone_input_spec.js index 44a11097815..6f6f20ccca2 100644 --- a/spec/javascripts/dropzone_input_spec.js +++ b/spec/javascripts/dropzone_input_spec.js @@ -39,17 +39,17 @@ describe('dropzone_input', () => { const event = $.Event('paste'); const origEvent = new Event('paste'); const pasteData = new DataTransfer(); - pasteData.setData('text/plain', 'hello world'); - pasteData.setData('text/html', '<table></table>'); + pasteData.setData('text/plain', 'Hello World'); + pasteData.setData('text/html', '<table><tr><td>Hello World</td></tr></table>'); origEvent.clipboardData = pasteData; event.originalEvent = origEvent; - spyOn(PasteMarkdownTable, 'isTable').and.callThrough(); + spyOn(PasteMarkdownTable.prototype, 'isTable').and.callThrough(); spyOn(PasteMarkdownTable.prototype, 'convertToTableMarkdown').and.callThrough(); $('.js-gfm-input').trigger(event); - expect(PasteMarkdownTable.isTable).toHaveBeenCalled(); + expect(PasteMarkdownTable.prototype.isTable).toHaveBeenCalled(); expect(PasteMarkdownTable.prototype.convertToTableMarkdown).toHaveBeenCalled(); }); }); diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index 3a56462ec1b..cf1dacd088e 100644 --- a/spec/lib/gitlab/usage_data_spec.rb +++ b/spec/lib/gitlab/usage_data_spec.rb @@ -17,8 +17,8 @@ describe Gitlab::UsageData do create(:service, project: projects[0], type: 'SlackSlashCommandsService', active: true) create(:service, project: projects[1], type: 'SlackService', active: true) create(:service, project: projects[2], type: 'SlackService', active: true) - create(:service, project: projects[2], type: 'MattermostService', active: true) - create(:service, project: projects[2], type: 'JenkinsService', active: true) + create(:service, project: projects[2], type: 'MattermostService', active: false) + create(:service, project: projects[2], type: 'MattermostService', active: true, template: true) create(:service, project: projects[2], type: 'CustomIssueTrackerService', active: true) create(:project_error_tracking_setting, project: projects[0]) create(:project_error_tracking_setting, project: projects[1], enabled: false) @@ -168,13 +168,15 @@ describe Gitlab::UsageData do pool_repositories projects projects_imported_from_github + projects_asana_active projects_jira_active projects_jira_server_active projects_jira_cloud_active projects_slack_notifications_active projects_slack_slash_active + projects_slack_active + projects_slack_slash_commands_active projects_custom_issue_tracker_active - projects_jenkins_active projects_mattermost_active projects_prometheus_active projects_with_repositories_enabled @@ -203,15 +205,17 @@ describe Gitlab::UsageData do count_data = subject[:counts] expect(count_data[:projects]).to eq(4) + expect(count_data[:projects_asana_active]).to eq(0) expect(count_data[:projects_prometheus_active]).to eq(1) expect(count_data[:projects_jira_active]).to eq(4) expect(count_data[:projects_jira_server_active]).to eq(2) expect(count_data[:projects_jira_cloud_active]).to eq(2) expect(count_data[:projects_slack_notifications_active]).to eq(2) expect(count_data[:projects_slack_slash_active]).to eq(1) + expect(count_data[:projects_slack_active]).to eq(2) + expect(count_data[:projects_slack_slash_commands_active]).to eq(1) expect(count_data[:projects_custom_issue_tracker_active]).to eq(1) - expect(count_data[:projects_jenkins_active]).to eq(1) - expect(count_data[:projects_mattermost_active]).to eq(1) + expect(count_data[:projects_mattermost_active]).to eq(0) expect(count_data[:projects_with_repositories_enabled]).to eq(3) expect(count_data[:projects_with_error_tracking_enabled]).to eq(1) expect(count_data[:issues_created_from_gitlab_error_tracking_ui]).to eq(1) diff --git a/spec/lib/sentry/client/issue_spec.rb b/spec/lib/sentry/client/issue_spec.rb index 20665c59a8d..b5ee5063e86 100644 --- a/spec/lib/sentry/client/issue_spec.rb +++ b/spec/lib/sentry/client/issue_spec.rb @@ -8,6 +8,216 @@ describe Sentry::Client::Issue do let(:token) { 'test-token' } let(:client) { Sentry::Client.new(sentry_url, token) } + describe '#list_issues' do + shared_examples 'issues have correct return type' do |klass| + it "returns objects of type #{klass}" do + expect(subject[:issues]).to all( be_a(klass) ) + end + end + + shared_examples 'issues have correct length' do |length| + it { expect(subject[:issues].length).to eq(length) } + end + + let(:issues_sample_response) do + Gitlab::Utils.deep_indifferent_access( + JSON.parse(fixture_file('sentry/issues_sample_response.json')) + ) + end + + let(:default_httparty_options) do + { + follow_redirects: false, + headers: { "Authorization" => "Bearer test-token" } + } + end + + let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' } + let(:issue_status) { 'unresolved' } + let(:limit) { 20 } + let(:search_term) { '' } + let(:cursor) { nil } + let(:sort) { 'last_seen' } + let(:sentry_api_response) { issues_sample_response } + let(:sentry_request_url) { sentry_url + '/issues/?limit=20&query=is:unresolved' } + let!(:sentry_api_request) { stub_sentry_request(sentry_request_url, body: sentry_api_response) } + + subject { client.list_issues(issue_status: issue_status, limit: limit, search_term: search_term, sort: sort, cursor: cursor) } + + it_behaves_like 'calls sentry api' + + it_behaves_like 'issues have correct return type', Gitlab::ErrorTracking::Error + it_behaves_like 'issues have correct length', 1 + + shared_examples 'has correct external_url' do + context 'external_url' do + it 'is constructed correctly' do + expect(subject[:issues][0].external_url).to eq('https://sentrytest.gitlab.com/sentry-org/sentry-project/issues/11') + end + end + end + + context 'when response has a pagination info' do + let(:headers) do + { + link: '<https://sentrytest.gitlab.com>; rel="previous"; results="true"; cursor="1573556671000:0:1", <https://sentrytest.gitlab.com>; rel="next"; results="true"; cursor="1572959139000:0:0"' + } + end + let!(:sentry_api_request) { stub_sentry_request(sentry_request_url, body: sentry_api_response, headers: headers) } + + it 'parses the pagination' do + expect(subject[:pagination]).to eq( + 'previous' => { 'cursor' => '1573556671000:0:1' }, + 'next' => { 'cursor' => '1572959139000:0:0' } + ) + end + end + + context 'error object created from sentry response' do + using RSpec::Parameterized::TableSyntax + + where(:error_object, :sentry_response) do + :id | :id + :first_seen | :firstSeen + :last_seen | :lastSeen + :title | :title + :type | :type + :user_count | :userCount + :count | :count + :message | [:metadata, :value] + :culprit | :culprit + :short_id | :shortId + :status | :status + :frequency | [:stats, '24h'] + :project_id | [:project, :id] + :project_name | [:project, :name] + :project_slug | [:project, :slug] + end + + with_them do + it { expect(subject[:issues][0].public_send(error_object)).to eq(sentry_api_response[0].dig(*sentry_response)) } + end + + it_behaves_like 'has correct external_url' + end + + context 'redirects' do + let(:sentry_api_url) { sentry_url + '/issues/?limit=20&query=is:unresolved' } + + it_behaves_like 'no Sentry redirects' + end + + # Sentry API returns 404 if there are extra slashes in the URL! + context 'extra slashes in URL' do + let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects//sentry-org/sentry-project/' } + + let(:sentry_request_url) do + 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project/' \ + 'issues/?limit=20&query=is:unresolved' + end + + it 'removes extra slashes in api url' do + expect(client.url).to eq(sentry_url) + expect(Gitlab::HTTP).to receive(:get).with( + URI('https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project/issues/'), + anything + ).and_call_original + + subject + + expect(sentry_api_request).to have_been_requested + end + end + + context 'requests with sort parameter in sentry api' do + let(:sentry_request_url) do + 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project/' \ + 'issues/?limit=20&query=is:unresolved&sort=freq' + end + let!(:sentry_api_request) { stub_sentry_request(sentry_request_url, body: sentry_api_response) } + + subject { client.list_issues(issue_status: issue_status, limit: limit, sort: 'frequency') } + + it 'calls the sentry api with sort params' do + expect(Gitlab::HTTP).to receive(:get).with( + URI("#{sentry_url}/issues/"), + default_httparty_options.merge(query: { limit: 20, query: "is:unresolved", sort: "freq" }) + ).and_call_original + + subject + + expect(sentry_api_request).to have_been_requested + end + end + + context 'with invalid sort params' do + subject { client.list_issues(issue_status: issue_status, limit: limit, sort: 'fish') } + + it 'throws an error' do + expect { subject }.to raise_error(Sentry::Client::BadRequestError, 'Invalid value for sort param') + end + end + + context 'Older sentry versions where keys are not present' do + let(:sentry_api_response) do + issues_sample_response[0...1].map do |issue| + issue[:project].delete(:id) + issue + end + end + + it_behaves_like 'calls sentry api' + + it_behaves_like 'issues have correct return type', Gitlab::ErrorTracking::Error + it_behaves_like 'issues have correct length', 1 + + it_behaves_like 'has correct external_url' + end + + context 'essential keys missing in API response' do + let(:sentry_api_response) do + issues_sample_response[0...1].map do |issue| + issue.except(:id) + end + end + + it 'raises exception' do + expect { subject }.to raise_error(Sentry::Client::MissingKeysError, 'Sentry API response is missing keys. key not found: "id"') + end + end + + context 'sentry api response too large' do + it 'raises exception' do + deep_size = double('Gitlab::Utils::DeepSize', valid?: false) + allow(Gitlab::Utils::DeepSize).to receive(:new).with(sentry_api_response).and_return(deep_size) + + expect { subject }.to raise_error(Sentry::Client::ResponseInvalidSizeError, 'Sentry API response is too big. Limit is 1 MB.') + end + end + + it_behaves_like 'maps Sentry exceptions' + + context 'when search term is present' do + let(:search_term) { 'NoMethodError' } + let(:sentry_request_url) { "#{sentry_url}/issues/?limit=20&query=is:unresolved NoMethodError" } + + it_behaves_like 'calls sentry api' + + it_behaves_like 'issues have correct return type', Gitlab::ErrorTracking::Error + it_behaves_like 'issues have correct length', 1 + end + + context 'when cursor is present' do + let(:cursor) { '1572959139000:0:0' } + let(:sentry_request_url) { "#{sentry_url}/issues/?limit=20&cursor=#{cursor}&query=is:unresolved" } + + it_behaves_like 'calls sentry api' + + it_behaves_like 'issues have correct return type', Gitlab::ErrorTracking::Error + it_behaves_like 'issues have correct length', 1 + end + end + describe '#issue_details' do let(:issue_sample_response) do Gitlab::Utils.deep_indifferent_access( diff --git a/spec/lib/sentry/client_spec.rb b/spec/lib/sentry/client_spec.rb index 8500f67b8e9..409e8be3198 100644 --- a/spec/lib/sentry/client_spec.rb +++ b/spec/lib/sentry/client_spec.rb @@ -3,219 +3,13 @@ require 'spec_helper' describe Sentry::Client do - include SentryClientHelpers - let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' } let(:token) { 'test-token' } - let(:default_httparty_options) do - { - follow_redirects: false, - headers: { "Authorization" => "Bearer test-token" } - } - end - - subject(:client) { described_class.new(sentry_url, token) } - - shared_examples 'issues has correct return type' do |klass| - it "returns objects of type #{klass}" do - expect(subject[:issues]).to all( be_a(klass) ) - end - end - - shared_examples 'issues has correct length' do |length| - it { expect(subject[:issues].length).to eq(length) } - end - - describe '#list_issues' do - let(:issues_sample_response) do - Gitlab::Utils.deep_indifferent_access( - JSON.parse(fixture_file('sentry/issues_sample_response.json')) - ) - end - - let(:issue_status) { 'unresolved' } - let(:limit) { 20 } - let(:search_term) { '' } - let(:cursor) { nil } - let(:sort) { 'last_seen' } - let(:sentry_api_response) { issues_sample_response } - let(:sentry_request_url) { sentry_url + '/issues/?limit=20&query=is:unresolved' } - - let!(:sentry_api_request) { stub_sentry_request(sentry_request_url, body: sentry_api_response) } - - subject { client.list_issues(issue_status: issue_status, limit: limit, search_term: search_term, sort: sort, cursor: cursor) } - - it_behaves_like 'calls sentry api' - - it_behaves_like 'issues has correct return type', Gitlab::ErrorTracking::Error - it_behaves_like 'issues has correct length', 1 - - shared_examples 'has correct external_url' do - context 'external_url' do - it 'is constructed correctly' do - expect(subject[:issues][0].external_url).to eq('https://sentrytest.gitlab.com/sentry-org/sentry-project/issues/11') - end - end - end - - context 'when response has a pagination info' do - let(:headers) do - { - link: '<https://sentrytest.gitlab.com>; rel="previous"; results="true"; cursor="1573556671000:0:1", <https://sentrytest.gitlab.com>; rel="next"; results="true"; cursor="1572959139000:0:0"' - } - end - let!(:sentry_api_request) { stub_sentry_request(sentry_request_url, body: sentry_api_response, headers: headers) } - - it 'parses the pagination' do - expect(subject[:pagination]).to eq( - 'previous' => { 'cursor' => '1573556671000:0:1' }, - 'next' => { 'cursor' => '1572959139000:0:0' } - ) - end - end - - context 'error object created from sentry response' do - using RSpec::Parameterized::TableSyntax - - where(:error_object, :sentry_response) do - :id | :id - :first_seen | :firstSeen - :last_seen | :lastSeen - :title | :title - :type | :type - :user_count | :userCount - :count | :count - :message | [:metadata, :value] - :culprit | :culprit - :short_id | :shortId - :status | :status - :frequency | [:stats, '24h'] - :project_id | [:project, :id] - :project_name | [:project, :name] - :project_slug | [:project, :slug] - end - - with_them do - it { expect(subject[:issues][0].public_send(error_object)).to eq(sentry_api_response[0].dig(*sentry_response)) } - end - - it_behaves_like 'has correct external_url' - end - - context 'redirects' do - let(:sentry_api_url) { sentry_url + '/issues/?limit=20&query=is:unresolved' } - - it_behaves_like 'no Sentry redirects' - end - - # Sentry API returns 404 if there are extra slashes in the URL! - context 'extra slashes in URL' do - let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects//sentry-org/sentry-project/' } - - let(:sentry_request_url) do - 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project/' \ - 'issues/?limit=20&query=is:unresolved' - end - - it 'removes extra slashes in api url' do - expect(client.url).to eq(sentry_url) - expect(Gitlab::HTTP).to receive(:get).with( - URI('https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project/issues/'), - anything - ).and_call_original - - subject - - expect(sentry_api_request).to have_been_requested - end - end - - context 'requests with sort parameter in sentry api' do - let(:sentry_request_url) do - 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project/' \ - 'issues/?limit=20&query=is:unresolved&sort=freq' - end - let!(:sentry_api_request) { stub_sentry_request(sentry_request_url, body: sentry_api_response) } - - subject { client.list_issues(issue_status: issue_status, limit: limit, sort: 'frequency') } - - it 'calls the sentry api with sort params' do - expect(Gitlab::HTTP).to receive(:get).with( - URI("#{sentry_url}/issues/"), - default_httparty_options.merge(query: { limit: 20, query: "is:unresolved", sort: "freq" }) - ).and_call_original - - subject - - expect(sentry_api_request).to have_been_requested - end - end - - context 'with invalid sort params' do - subject { client.list_issues(issue_status: issue_status, limit: limit, sort: 'fish') } - - it 'throws an error' do - expect { subject }.to raise_error(Sentry::Client::BadRequestError, 'Invalid value for sort param') - end - end - - context 'Older sentry versions where keys are not present' do - let(:sentry_api_response) do - issues_sample_response[0...1].map do |issue| - issue[:project].delete(:id) - issue - end - end - - it_behaves_like 'calls sentry api' - - it_behaves_like 'issues has correct return type', Gitlab::ErrorTracking::Error - it_behaves_like 'issues has correct length', 1 - - it_behaves_like 'has correct external_url' - end - - context 'essential keys missing in API response' do - let(:sentry_api_response) do - issues_sample_response[0...1].map do |issue| - issue.except(:id) - end - end - - it 'raises exception' do - expect { subject }.to raise_error(Sentry::Client::MissingKeysError, 'Sentry API response is missing keys. key not found: "id"') - end - end - - context 'sentry api response too large' do - it 'raises exception' do - deep_size = double('Gitlab::Utils::DeepSize', valid?: false) - allow(Gitlab::Utils::DeepSize).to receive(:new).with(sentry_api_response).and_return(deep_size) - - expect { subject }.to raise_error(Sentry::Client::ResponseInvalidSizeError, 'Sentry API response is too big. Limit is 1 MB.') - end - end - - it_behaves_like 'maps Sentry exceptions' - - context 'when search term is present' do - let(:search_term) { 'NoMethodError' } - let(:sentry_request_url) { "#{sentry_url}/issues/?limit=20&query=is:unresolved NoMethodError" } - - it_behaves_like 'calls sentry api' - - it_behaves_like 'issues has correct return type', Gitlab::ErrorTracking::Error - it_behaves_like 'issues has correct length', 1 - end - - context 'when cursor is present' do - let(:cursor) { '1572959139000:0:0' } - let(:sentry_request_url) { "#{sentry_url}/issues/?limit=20&cursor=#{cursor}&query=is:unresolved" } - it_behaves_like 'calls sentry api' + subject { Sentry::Client.new(sentry_url, token) } - it_behaves_like 'issues has correct return type', Gitlab::ErrorTracking::Error - it_behaves_like 'issues has correct length', 1 - end - end + it { is_expected.to respond_to :projects } + it { is_expected.to respond_to :list_issues } + it { is_expected.to respond_to :issue_details } + it { is_expected.to respond_to :issue_latest_event } end |