diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-12-13 06:14:51 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-12-13 06:14:51 +0000 |
commit | 2d2181e35c3cff3411870100cd57c0ed8d95ec20 (patch) | |
tree | af6595ae56ada2e1ff8975e295aa60e0f1dd42e5 | |
parent | 8cc88269ff15fbe58ffad1f793478344d45a0403 (diff) | |
download | gitlab-ce-2d2181e35c3cff3411870100cd57c0ed8d95ec20.tar.gz |
Add latest changes from gitlab-org/gitlab@master
26 files changed, 504 insertions, 243 deletions
diff --git a/.gitlab/ci/rails.gitlab-ci.yml b/.gitlab/ci/rails.gitlab-ci.yml index 92f8e1ad2b1..9db4a2afd91 100644 --- a/.gitlab/ci/rails.gitlab-ci.yml +++ b/.gitlab/ci/rails.gitlab-ci.yml @@ -342,8 +342,9 @@ rspec fast_spec_helper minimal: db:rollback: extends: .db-job-base script: - - bundle exec rake db:migrate:main VERSION=20181228175414 - - bundle exec rake db:migrate:main SKIP_SCHEMA_VERSION_CHECK=true + - if [[ -d "ee/" ]]; then task="db:migrate:main"; else task="db:migrate"; fi + - bundle exec rake "${task}" VERSION=20181228175414 + - bundle exec rake "${task}" SKIP_SCHEMA_VERSION_CHECK=true db:migrate:reset: extends: .db-job-base @@ -368,7 +369,8 @@ db:migrate-from-previous-major-version: - git checkout -f $CI_COMMIT_SHA - SETUP_DB=false USE_BUNDLE_INSTALL=true bash scripts/prepare_build.sh script: - - run_timed_command "bundle exec rake db:migrate:main" + - if [[ -d "ee/" ]]; then task="db:migrate:main"; else task="db:migrate"; fi + - run_timed_command "bundle exec rake ${task}" db:check-schema: extends: @@ -377,7 +379,8 @@ db:check-schema: variables: TAG_TO_CHECKOUT: "v14.4.0" script: - - run_timed_command "bundle exec rake db:migrate:main" + - if [[ -d "ee/" ]]; then task="db:migrate:main"; else task="db:migrate"; fi + - run_timed_command "bundle exec rake ${task}" - scripts/schema_changed.sh - scripts/validate_migration_timestamps diff --git a/app/assets/javascripts/boards/components/board_list.vue b/app/assets/javascripts/boards/components/board_list.vue index 91bee8f3474..e4c3c3206a8 100644 --- a/app/assets/javascripts/boards/components/board_list.vue +++ b/app/assets/javascripts/boards/components/board_list.vue @@ -6,9 +6,9 @@ import { sortableStart, sortableEnd } from '~/boards/mixins/sortable_default_opt import { sprintf, __ } from '~/locale'; import defaultSortableConfig from '~/sortable/sortable_config'; import Tracking from '~/tracking'; +import listQuery from 'ee_else_ce/boards/graphql/board_lists_deferred.query.graphql'; import { toggleFormEventPrefix, DraggableItemTypes } from '../constants'; import eventHub from '../eventhub'; -import listQuery from '../graphql/board_lists_deferred.query.graphql'; import BoardCard from './board_card.vue'; import BoardNewIssue from './board_new_issue.vue'; diff --git a/app/assets/javascripts/boards/components/board_list_header.vue b/app/assets/javascripts/boards/components/board_list_header.vue index c9cf044aaa0..19004518edf 100644 --- a/app/assets/javascripts/boards/components/board_list_header.vue +++ b/app/assets/javascripts/boards/components/board_list_header.vue @@ -17,10 +17,10 @@ import sidebarEventHub from '~/sidebar/event_hub'; import Tracking from '~/tracking'; import { formatDate } from '~/lib/utils/datetime_utility'; import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; +import listQuery from 'ee_else_ce/boards/graphql/board_lists_deferred.query.graphql'; import AccessorUtilities from '../../lib/utils/accessor'; import { inactiveId, LIST, ListType, toggleFormEventPrefix } from '../constants'; import eventHub from '../eventhub'; -import listQuery from '../graphql/board_lists_deferred.query.graphql'; import ItemCount from './item_count.vue'; export default { diff --git a/app/assets/javascripts/repository/components/blob_button_group.vue b/app/assets/javascripts/repository/components/blob_button_group.vue index de6156d48dc..6f540bf8ece 100644 --- a/app/assets/javascripts/repository/components/blob_button_group.vue +++ b/app/assets/javascripts/repository/components/blob_button_group.vue @@ -87,6 +87,9 @@ export default { deleteModalTitle() { return sprintf(__('Delete %{name}'), { name: this.name }); }, + lockBtnQASelector() { + return this.canLock ? 'lock_button' : 'disabled_lock_button'; + }, }, }; </script> @@ -102,7 +105,7 @@ export default { :is-locked="isLocked" :can-lock="canLock" data-testid="lock" - data-qa-selector="lock_button" + :data-qa-selector="lockBtnQASelector" /> <gl-button v-gl-modal="replaceModalId" data-testid="replace"> {{ $options.i18n.replace }} diff --git a/app/assets/javascripts/vue_shared/issuable/list/components/issuable_list_root.vue b/app/assets/javascripts/vue_shared/issuable/list/components/issuable_list_root.vue index c1082987146..1916287e2da 100644 --- a/app/assets/javascripts/vue_shared/issuable/list/components/issuable_list_root.vue +++ b/app/assets/javascripts/vue_shared/issuable/list/components/issuable_list_root.vue @@ -272,6 +272,7 @@ export default { :show-checkbox="showBulkEditSidebar" :checkbox-checked="allIssuablesChecked" class="gl-flex-grow-1 gl-border-t-none row-content-block" + data-qa-selector="issuable_search_container" @checked-input="handleAllIssuablesCheckedInput" @onFilter="$emit('filter', $event)" @onSort="$emit('sort', $event)" @@ -302,6 +303,8 @@ export default { v-for="issuable in issuables" :key="issuableId(issuable)" :class="{ 'gl-cursor-grab': isManualOrdering }" + data-qa-selector="issuable_container" + :data-qa-issuable-title="issuable.title" :issuable-symbol="issuableSymbol" :issuable="issuable" :enable-label-permalinks="enableLabelPermalinks" diff --git a/app/assets/javascripts/vue_shared/issuable/list/components/issuable_tabs.vue b/app/assets/javascripts/vue_shared/issuable/list/components/issuable_tabs.vue index 96b07031a11..3ff87ba3c4f 100644 --- a/app/assets/javascripts/vue_shared/issuable/list/components/issuable_tabs.vue +++ b/app/assets/javascripts/vue_shared/issuable/list/components/issuable_tabs.vue @@ -46,7 +46,9 @@ export default { @click="$emit('click', tab.name)" > <template #title> - <span :title="tab.titleTooltip">{{ tab.title }}</span> + <span :title="tab.titleTooltip" :data-qa-selector="`${tab.name}_issuables_tab`"> + {{ tab.title }} + </span> <gl-badge v-if="tabCounts && isTabCountNumeric(tab)" variant="muted" diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 970efd9cdfb..8a6ae268376 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -51,7 +51,7 @@ class Projects::IssuesController < Projects::ApplicationController before_action only: :show do push_frontend_feature_flag(:real_time_issue_sidebar, @project, default_enabled: :yaml) - push_frontend_feature_flag(:confidential_notes, @project, default_enabled: :yaml) + push_frontend_feature_flag(:confidential_notes, project&.group, default_enabled: :yaml) push_frontend_feature_flag(:issue_assignees_widget, @project, default_enabled: :yaml) push_frontend_feature_flag(:paginated_issue_discussions, @project, default_enabled: :yaml) end diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index a7e60b90841..65472615f42 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -144,10 +144,6 @@ class ApplicationSetting < ApplicationRecord length: { maximum: 2000, message: _('is too long (maximum is %{count} characters)') }, allow_blank: true - validates :spam_check_api_key, - presence: true, - if: :spam_check_endpoint_enabled - validates :unique_ips_limit_per_user, numericality: { greater_than_or_equal_to: 1 }, presence: true, diff --git a/app/services/concerns/protected_ref_name_sanitizer.rb b/app/services/concerns/protected_ref_name_sanitizer.rb new file mode 100644 index 00000000000..3966c410fec --- /dev/null +++ b/app/services/concerns/protected_ref_name_sanitizer.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +module ProtectedRefNameSanitizer + def sanitize_name(name) + name = CGI.unescapeHTML(name) + name = Sanitize.fragment(name) + + # Sanitize.fragment escapes HTML chars, so unescape again to allow names + # like `feature->master` + CGI.unescapeHTML(name) + end +end diff --git a/app/services/protected_branches/base_service.rb b/app/services/protected_branches/base_service.rb index df801311aaf..1ab3ccfcaae 100644 --- a/app/services/protected_branches/base_service.rb +++ b/app/services/protected_branches/base_service.rb @@ -2,6 +2,8 @@ module ProtectedBranches class BaseService < ::BaseService + include ProtectedRefNameSanitizer + # current_user - The user that performs the action # params - A hash of parameters def initialize(project, current_user = nil, params = {}) @@ -14,22 +16,13 @@ module ProtectedBranches # overridden in EE::ProtectedBranches module end + private + def filtered_params return unless params - params[:name] = sanitize_branch_name(params[:name]) if params[:name].present? + params[:name] = sanitize_name(params[:name]) if params[:name].present? params end - - private - - def sanitize_branch_name(name) - name = CGI.unescapeHTML(name) - name = Sanitize.fragment(name) - - # Sanitize.fragment escapes HTML chars, so unescape again to allow names - # like `feature->master` - CGI.unescapeHTML(name) - end end end diff --git a/app/services/protected_tags/base_service.rb b/app/services/protected_tags/base_service.rb new file mode 100644 index 00000000000..e0181815f0f --- /dev/null +++ b/app/services/protected_tags/base_service.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module ProtectedTags + class BaseService < ::BaseService + include ProtectedRefNameSanitizer + + private + + def filtered_params + return unless params + + params[:name] = sanitize_name(params[:name]) if params[:name].present? + params + end + end +end diff --git a/app/services/protected_tags/create_service.rb b/app/services/protected_tags/create_service.rb index 9aff55986b2..7d2b583a295 100644 --- a/app/services/protected_tags/create_service.rb +++ b/app/services/protected_tags/create_service.rb @@ -1,13 +1,13 @@ # frozen_string_literal: true module ProtectedTags - class CreateService < BaseService + class CreateService < ProtectedTags::BaseService attr_reader :protected_tag def execute raise Gitlab::Access::AccessDeniedError unless can?(current_user, :admin_project, project) - project.protected_tags.create(params) + project.protected_tags.create(filtered_params) end end end diff --git a/app/services/protected_tags/update_service.rb b/app/services/protected_tags/update_service.rb index 3eb5f4955ee..e337ec39898 100644 --- a/app/services/protected_tags/update_service.rb +++ b/app/services/protected_tags/update_service.rb @@ -1,11 +1,11 @@ # frozen_string_literal: true module ProtectedTags - class UpdateService < BaseService + class UpdateService < ProtectedTags::BaseService def execute(protected_tag) raise Gitlab::Access::AccessDeniedError unless can?(current_user, :admin_project, project) - protected_tag.update(params) + protected_tag.update(filtered_params) protected_tag end end diff --git a/changelogs/unreleased/209975-oidc-claim-group-level.yml b/changelogs/unreleased/209975-oidc-claim-group-level.yml deleted file mode 100644 index 2793911a060..00000000000 --- a/changelogs/unreleased/209975-oidc-claim-group-level.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Provide group membership level in OIDC claim -merge_request: 27264 -author: Bastian Blank -type: added diff --git a/lib/api/settings.rb b/lib/api/settings.rb index 12e1d21a00d..508ccdb4b33 100644 --- a/lib/api/settings.rb +++ b/lib/api/settings.rb @@ -154,7 +154,6 @@ module API optional :spam_check_endpoint_enabled, type: Boolean, desc: 'Enable Spam Check via external API endpoint' given spam_check_endpoint_enabled: ->(val) { val } do requires :spam_check_endpoint_url, type: String, desc: 'The URL of the external Spam Check service endpoint' - requires :spam_check_api_key, type: String, desc: 'The API key used by GitLab for accessing the Spam Check service endpoint' end optional :terminal_max_session_time, type: Integer, desc: 'Maximum time for web terminal websocket connection (in seconds). Set to 0 for unlimited time.' optional :usage_ping_enabled, type: Boolean, desc: 'Every week GitLab will report license usage back to GitLab, Inc.' diff --git a/qa/qa/page/project/issue/index.rb b/qa/qa/page/project/issue/index.rb index fc46f7a2936..45f8cf2ccd3 100644 --- a/qa/qa/page/project/issue/index.rb +++ b/qa/qa/page/project/issue/index.rb @@ -5,9 +5,9 @@ module QA module Project module Issue class Index < Page::Base - view 'app/assets/javascripts/issues_list/components/issuable.vue' do - element :issue_container - element :issue_link + view 'app/assets/javascripts/vue_shared/issuable/list/components/issuable_list_root.vue' do + element :issuable_container + element :issuable_search_container end view 'app/assets/javascripts/vue_shared/components/issue/issue_assignees.vue' do @@ -25,8 +25,8 @@ module QA element :import_issues_dropdown end - view 'app/views/shared/issuable/_nav.html.haml' do - element :closed_issues_link + view 'app/assets/javascripts/vue_shared/issuable/list/components/issuable_tabs.vue' do + element :closed_issuables_tab, ':data-qa-selector="`${tab.name}_issuables_tab`"' # rubocop:disable QA/ElementWithPattern end def avatar_counter @@ -37,8 +37,8 @@ module QA click_link(title) end - def click_closed_issues_link - click_element :closed_issues_link + def click_closed_issues_tab + click_element(:closed_issuables_tab) end def click_export_as_csv_button @@ -73,11 +73,17 @@ module QA end def has_issue?(issue) - has_element? :issue_container, issue_title: issue.title + has_element? :issuable_container, issuable_title: issue.title end def has_no_issue?(issue) - has_no_element? :issue_container, issue_title: issue.title + has_no_element? :issuable_container, issuable_title: issue.title + end + + def wait_for_vue_issues_list_ff + Support::Retrier.retry_until(max_duration: 60, reload_page: page, retry_on_exception: true, sleep_interval: 5) do + find_element(:closed_issuables_tab) + end end end end diff --git a/qa/qa/specs/features/browser_ui/2_plan/issue/create_issue_spec.rb b/qa/qa/specs/features/browser_ui/2_plan/issue/create_issue_spec.rb index 81ae8b82ef6..eae0aa52170 100644 --- a/qa/qa/specs/features/browser_ui/2_plan/issue/create_issue_spec.rb +++ b/qa/qa/specs/features/browser_ui/2_plan/issue/create_issue_spec.rb @@ -1,19 +1,26 @@ # frozen_string_literal: true module QA - RSpec.describe 'Plan', :smoke do + # TODO: Remove :requires_admin when the `Runtime::Feature.enable` method call is removed + RSpec.describe 'Plan', :smoke, :requires_admin do describe 'Issue creation' do - let(:closed_issue) { Resource::Issue.fabricate_via_api! } + let(:project) { Resource::Project.fabricate_via_api! } + let(:closed_issue) { Resource::Issue.fabricate_via_api! { |issue| issue.project = project } } before do + Runtime::Feature.enable(:vue_issues_list, group: project.group) + Flow::Login.sign_in end it 'creates an issue', :mobile, testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/quality/test_cases/1185' do - issue = Resource::Issue.fabricate_via_browser_ui! + issue = Resource::Issue.fabricate_via_browser_ui! { |issue| issue.project = project } Page::Project::Menu.perform(&:click_issues) + # TODO: Remove this method when the `Runtime::Feature.enable` method call is removed + Page::Project::Issue::Index.perform(&:wait_for_vue_issues_list_ff) + Page::Project::Issue::Index.perform do |index| expect(index).to have_issue(issue) end @@ -29,10 +36,14 @@ module QA end Page::Project::Menu.perform(&:click_issues) + + # TODO: Remove this method when the `Runtime::Feature.enable` method call is removed + Page::Project::Issue::Index.perform(&:wait_for_vue_issues_list_ff) + Page::Project::Issue::Index.perform do |index| expect(index).not_to have_issue(closed_issue) - index.click_closed_issues_link + index.click_closed_issues_tab expect(index).to have_issue(closed_issue) end @@ -45,7 +56,7 @@ module QA end before do - Resource::Issue.fabricate_via_api!.visit! + Resource::Issue.fabricate_via_api! { |issue| issue.project = project }.visit! end # The following example is excluded from running in `review-qa-smoke` job diff --git a/qa/qa/specs/features/browser_ui/3_create/web_ide/web_terminal_spec.rb b/qa/qa/specs/features/browser_ui/3_create/web_ide/web_terminal_spec.rb index 51791c01048..3cd57f97a34 100644 --- a/qa/qa/specs/features/browser_ui/3_create/web_ide/web_terminal_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/web_ide/web_terminal_spec.rb @@ -65,6 +65,7 @@ module QA # a) The terminal JS package has loaded, and # b) It's not stuck in a "Loading/Starting" state, and # c) There's no alert stating there was a problem + # d) There are no JS console errors # # The terminal itself is a third-party package so we assume it is # adequately tested elsewhere. @@ -78,6 +79,17 @@ module QA expect(edit).to have_finished_loading expect(edit).to have_terminal_screen end + + # It takes a few seconds for console errors to appear + sleep 3 + + errors = page.driver.browser.logs.get(:browser) + .select { |e| e.level == "SEVERE" } + .to_a + + if errors.present? + raise("Console error(s):\n#{errors.join("\n\n")}") + end end end end diff --git a/qa/qa/tools/reliable_report.rb b/qa/qa/tools/reliable_report.rb index 7e7b9df1f41..40a452be36e 100644 --- a/qa/qa/tools/reliable_report.rb +++ b/qa/qa/tools/reliable_report.rb @@ -1,99 +1,210 @@ # frozen_string_literal: true +require_relative "../../qa" + require "influxdb-client" require "terminal-table" require "slack-notifier" +require "colorize" module QA module Tools class ReliableReport - def initialize(run_type, range = 30) - @results = 2 - @slack_channel = "#quality-reports" + include Support::API + + # Project for report creation: https://gitlab.com/gitlab-org/gitlab + PROJECT_ID = 278964 + + def initialize(range) @range = range - @run_type = run_type - @stable_title = "Top #{results} stable specs for past #{@range} days in '#{run_type}' runs" - @unstable_title = "Top #{results} unstable reliable specs for past #{@range} days in '#{run_type}' runs" + @influxdb_bucket = "e2e-test-stats" + @slack_channel = "#quality-reports" + @influxdb_url = ENV["QA_INFLUXDB_URL"] || raise("Missing QA_INFLUXDB_URL env variable") + @influxdb_token = ENV["QA_INFLUXDB_TOKEN"] || raise("Missing QA_INFLUXDB_TOKEN env variable") end - # Print top stable specs + # Run reliable reporter # + # @param [Integer] range amount of days for results range + # @param [String] report_in_issue_and_slack # @return [void] - def show_top_stable - results_table(:stable).each { |table| puts "#{table}\n\n" } + def self.run(range: 14, report_in_issue_and_slack: "false") + reporter = new(range) + + reporter.print_report + reporter.report_in_issue_and_slack if report_in_issue_and_slack == "true" + rescue StandardError => e + puts "Report creation failed! Error: '#{e}'".colorize(:red) + reporter.notify_failure(e) + exit(1) end - # Post top stable spec report to slack + # Print top stable specs # # @return [void] - def notify_top_stable - puts "\nSending top stable spec report to #{slack_channel} slack channel" - slack_args = { icon_emoji: ":mtg_green:", username: "Stable Spec Report" } - notifier.post(text: "*#{stable_title}*", **slack_args) - results_table(:stable).each { |table| notifier.post(text: "```#{table}```", **slack_args) } + def print_report + puts "#{stable_summary_table}\n\n" + stable_results_tables.each { |stage, table| puts "#{table}\n\n" } + return puts("No unstable reliable tests present!".colorize(:yellow)) if unstable_reliable_test_runs.empty? + + puts "#{unstable_summary_table}\n\n" + unstable_reliable_results_tables.each { |stage, table| puts "#{table}\n\n" } end - # Print top unstable specs + # Create report issue # # @return [void] - def show_top_unstable - return puts("No unstable tests present!") if top_unstable_reliable.empty? + def report_in_issue_and_slack + puts "Creating report".colorize(:green) + response = post( + "#{gitlab_api_url}/projects/#{PROJECT_ID}/issues", + { title: "Reliable spec report", description: report_issue_body, labels: "Quality,test" }, + headers: { "PRIVATE-TOKEN" => gitlab_access_token } + ) + web_url = parse_body(response)[:web_url] + puts "Created report issue: #{web_url}" - results_table(:unstable).each { |table| puts "#{table}\n\n" } + puts "Sending slack notification".colorize(:green) + notifier.post( + icon_emoji: ":tanuki-protect:", + text: <<~TEXT + ```#{stable_summary_table}``` + ```#{unstable_summary_table}``` + + #{web_url} + TEXT + ) + puts "Done!" end - # Post top unstable reliable spec report to slack + # Notify failure # + # @param [StandardError] error # @return [void] - def notify_top_unstable - return puts("No unstable tests present!") if top_unstable_reliable.empty? - - puts "\nSending top unstable reliable spec report to #{slack_channel} slack channel" - slack_args = { icon_emoji: ":sadpanda:", username: "Unstable Spec Report" } - notifier.post(text: "*#{unstable_title}*", **slack_args) - results_table(:unstable).each { |table| notifier.post(text: "```#{table}```", **slack_args) } + def notify_failure(error) + notifier.post( + text: "Reliable reporter failed to create report. Error: ```#{error}```", + icon_emoji: ":sadpanda:" + ) end private - attr_reader :results, - :slack_channel, - :range, - :run_type, - :stable_title, - :unstable_title + attr_reader :range, :influxdb_bucket, :slack_channel, :influxdb_url, :influxdb_token + + # Markdown formatted report issue body + # + # @return [String] + def report_issue_body + issue = [] + issue << "[[_TOC_]]" + issue << "# Candidates for promotion to reliable\n\n```\n#{stable_summary_table}\n```" + issue << results_markdown(stable_results_tables) + return issue.join("\n\n") if unstable_reliable_test_runs.empty? + + issue << "# Reliable specs with failures\n\n```\n#{unstable_summary_table}\n```" + issue << results_markdown(unstable_reliable_results_tables) + issue.join("\n\n") + end + + # Stable spec summary table + # + # @return [Terminal::Table] + def stable_summary_table + @stable_summary_table ||= terminal_table( + rows: stable_test_runs.map { |stage, specs| [stage, specs.length] }, + title: "Stable spec summary for past #{range} days".ljust(50), + headings: %w[STAGE COUNT] + ) + end + + # Unstable reliable summary table + # + # @return [Terminal::Table] + def unstable_summary_table + @unstable_summary_table ||= terminal_table( + rows: unstable_reliable_test_runs.map { |stage, specs| [stage, specs.length] }, + title: "Unstable spec summary for past #{range} days".ljust(50), + headings: %w[STAGE COUNT] + ) + end + + # Result tables for stable specs + # + # @return [Hash] + def stable_results_tables + @stable_results ||= results_tables(:stable) + end + + # Result table for unstable specs + # + # @return [Hash] + def unstable_reliable_results_tables + @unstable_results ||= results_tables(:unstable) + end + + # Markdown formatted tables + # + # @param [Hash] results + # @return [String] + def results_markdown(results) + results.map do |stage, table| + <<~STAGE.strip + ## #{stage} + + <details> + <summary>Executions table</summary> + + ``` + #{table} + ``` + + </details> + STAGE + end.join("\n\n") + end # Results table # # @param [Symbol] type result type - :stable, :unstable - # @return [Hash] - def results_table(type) - (type == :stable ? top_stable : top_unstable_reliable).map do |stage, specs| - terminal_table( + # @return [Hash<Symbol, Terminal::Table>] + def results_tables(type) + (type == :stable ? stable_test_runs : unstable_reliable_test_runs).to_h do |stage, specs| + headings = ["name", "runs", "failures", "failure rate"] + + [stage, terminal_table( rows: specs.map { |k, v| [name_column(k, v[:file]), *table_params(v.values)] }, - title: "Top #{type} specs in '#{stage}' stage" - ) + title: "Top #{type} specs in '#{stage}' stage for past #{range} days", + headings: headings.map(&:upcase) + )] end end - # Top stable specs + # Stable specs # # @return [Hash] - def top_stable - @top_stable ||= runs(reliable: false).transform_values do |specs| - specs.sort_by { |k, v| [v[:failure_rate], -v[:runs]] }[0..results - 1].to_h + def stable_test_runs + @top_stable ||= begin + stable_specs = test_runs(reliable: false).transform_values do |specs| + specs + .reject { |k, v| v[:failure_rate] != 0 } + .sort_by { |k, v| -v[:runs] } + .to_h + end + + stable_specs.reject { |k, v| v.empty? } end end - # Top unstable reliable specs + # Unstable reliable specs # # @return [Hash] - def top_unstable_reliable + def unstable_reliable_test_runs @top_unstable_reliable ||= begin - unstable = runs(reliable: true).transform_values do |specs| + unstable = test_runs(reliable: true).transform_values do |specs| specs .reject { |k, v| v[:failure_rate] == 0 } - .sort_by { |k, v| -v[:failure_rate] }[0..results - 1] + .sort_by { |k, v| -v[:failure_rate] } .to_h end @@ -104,9 +215,9 @@ module QA # Terminal table for result formatting # # @return [Terminal::Table] - def terminal_table(rows:, title: nil) + def terminal_table(rows:, headings:, title: nil) Terminal::Table.new( - headings: ["name", "runs", "failed", "failure rate"], + headings: headings, style: { all_separators: true }, title: title, rows: rows @@ -127,20 +238,19 @@ module QA # @param [String] file # @return [String] def name_column(name, file) - spec_name = name.length > 100 ? "#{name} ".scan(/.{1,100} /).map(&:strip).join("\n") : name + spec_name = name.length > 150 ? "#{name} ".scan(/.{1,150} /).map(&:strip).join("\n") : name name_line = "name: '#{spec_name}'" file_line = "file: '#{file}'" - "#{name_line}\n#{file_line.ljust(110)}" + "#{name_line}\n#{file_line.ljust(160)}" end # Test executions grouped by name # # @param [Boolean] reliable # @return [Hash<String, Hash>] - def runs(reliable:) - puts("Fetching data on #{reliable ? 'reliable ' : ''}test execution for past 30 days in '#{run_type}' runs") - puts + def test_runs(reliable:) + puts("Fetching data on #{reliable ? 'reliable ' : ''}test execution for past #{range} days\n".colorize(:green)) all_runs = query_api.query(query: query(reliable)).values all_runs.each_with_object(Hash.new { |hsh, key| hsh[key] = {} }) do |table, result| @@ -168,17 +278,24 @@ module QA # @return [String] def query(reliable) <<~QUERY - from(bucket: "e2e-test-stats") - |> range(start: -#{range}d) - |> filter(fn: (r) => r._measurement == "test-stats" and - r.run_type == "#{run_type}" and - r.status != "pending" and - r.merge_request == "false" and - r.quarantined == "false" and - r.reliable == "#{reliable}" and - r._field == "id" - ) - |> group(columns: ["name"]) + from(bucket: "#{influxdb_bucket}") + |> range(start: -#{range}d) + |> filter(fn: (r) => r._measurement == "test-stats") + |> filter(fn: (r) => r.run_type == "staging-full" or + r.run_type == "staging-sanity" or + r.run_type == "staging-sanity-no-admin" or + r.run_type == "production-full" or + r.run_type == "production-sanity" or + r.run_type == "package-and-qa" or + r.run_type == "nightly" + ) + |> filter(fn: (r) => r.status != "pending" and + r.merge_request == "false" and + r.quarantined == "false" and + r.reliable == "#{reliable}" and + r._field == "id" + ) + |> group(columns: ["name"]) QUERY end @@ -196,7 +313,7 @@ module QA @influx_client ||= InfluxDB2::Client.new( influxdb_url, influxdb_token, - bucket: "e2e-test-stats", + bucket: influxdb_bucket, org: "gitlab-qa", precision: InfluxDB2::WritePrecision::NANOSECOND ) @@ -209,29 +326,29 @@ module QA @notifier ||= Slack::Notifier.new( slack_webhook_url, channel: slack_channel, - username: "Reliable spec reporter" + username: "Reliable Spec Report" ) end - # InfluxDb instance url + # Gitlab access token # # @return [String] - def influxdb_url - @influxdb_url ||= ENV["QA_INFLUXDB_URL"] || raise("Missing QA_INFLUXDB_URL environment variable") + def gitlab_access_token + @gitlab_access_token ||= ENV["GITLAB_ACCESS_TOKEN"] || raise("Missing GITLAB_ACCESS_TOKEN env variable") end - # Influxdb token + # Gitlab api url # # @return [String] - def influxdb_token - @influxdb_token ||= ENV["QA_INFLUXDB_TOKEN"] || raise("Missing QA_INFLUXDB_TOKEN environment variable") + def gitlab_api_url + @gitlab_api_url ||= ENV["CI_API_V4_URL"] || raise("Missing CI_API_V4_URL env variable") end # Slack webhook url # # @return [String] def slack_webhook_url - @slack_webhook_url ||= ENV["CI_SLACK_WEBHOOK_URL"] || raise("Missing CI_SLACK_WEBHOOK_URL environment variable") + @slack_webhook_url ||= ENV["SLACK_WEBHOOK"] || raise("Missing SLACK_WEBHOOK env variable") end end end diff --git a/qa/spec/tools/reliable_report_spec.rb b/qa/spec/tools/reliable_report_spec.rb index 6562eb847b9..a048aa2e6ea 100644 --- a/qa/spec/tools/reliable_report_spec.rb +++ b/qa/spec/tools/reliable_report_spec.rb @@ -3,63 +3,94 @@ describe QA::Tools::ReliableReport do include QA::Support::Helpers::StubEnv - subject(:reporter) { described_class.new(run_type, range) } + subject(:run) { described_class.run(range: range, report_in_issue_and_slack: create_issue) } + let(:gitlab_response) { instance_double("RestClient::Response", code: 200, body: { web_url: issue_url }.to_json) } let(:slack_notifier) { instance_double("Slack::Notifier", post: nil) } let(:influx_client) { instance_double("InfluxDB2::Client", create_query_api: query_api) } let(:query_api) { instance_double("InfluxDB2::QueryApi") } let(:slack_channel) { "#quality-reports" } - let(:run_type) { "package-and-qa" } - let(:range) { 30 } - let(:results) { 2 } - - let(:runs) { { 0 => stable_spec, 1 => unstable_spec } } - - let(:spec_values) { { "file_path" => "some/spec.rb", "stage" => "manage" } } - let(:stable_spec) do - values = { "name" => "stable spec", "status" => "passed", **spec_values } - instance_double( - "InfluxDB2::FluxTable", - records: [ - instance_double("InfluxDB2::FluxRecord", values: values), - instance_double("InfluxDB2::FluxRecord", values: values), - instance_double("InfluxDB2::FluxRecord", values: values) - ] - ) + let(:range) { 14 } + let(:issue_url) { "https://gitlab.com/issue/1" } + + let(:runs) do + values = { "name" => "stable spec", "status" => "passed", "file_path" => "some/spec.rb", "stage" => "manage" } + { + 0 => instance_double( + "InfluxDB2::FluxTable", + records: [ + instance_double("InfluxDB2::FluxRecord", values: values), + instance_double("InfluxDB2::FluxRecord", values: values), + instance_double("InfluxDB2::FluxRecord", values: values) + ] + ) + } end - let(:unstable_spec) do - values = { "name" => "unstable spec", "status" => "failed", **spec_values } - instance_double( - "InfluxDB2::FluxTable", - records: [ - instance_double("InfluxDB2::FluxRecord", values: { **values, "status" => "passed" }), - instance_double("InfluxDB2::FluxRecord", values: values), - instance_double("InfluxDB2::FluxRecord", values: values) - ] - ) + let(:reliable_runs) do + values = { "name" => "unstable spec", "status" => "failed", "file_path" => "some/spec.rb", "stage" => "create" } + { + 0 => instance_double( + "InfluxDB2::FluxTable", + records: [ + instance_double("InfluxDB2::FluxRecord", values: { **values, "status" => "passed" }), + instance_double("InfluxDB2::FluxRecord", values: values), + instance_double("InfluxDB2::FluxRecord", values: values) + ] + ) + } end - def flux_query(reliable) + def flux_query(reliable:) <<~QUERY - from(bucket: "e2e-test-stats") - |> range(start: -#{range}d) - |> filter(fn: (r) => r._measurement == "test-stats" and - r.run_type == "#{run_type}" and - r.status != "pending" and - r.merge_request == "false" and - r.quarantined == "false" and - r.reliable == "#{reliable}" and - r._field == "id" - ) - |> group(columns: ["name"]) + from(bucket: "e2e-test-stats") + |> range(start: -#{range}d) + |> filter(fn: (r) => r._measurement == "test-stats") + |> filter(fn: (r) => r.run_type == "staging-full" or + r.run_type == "staging-sanity" or + r.run_type == "staging-sanity-no-admin" or + r.run_type == "production-full" or + r.run_type == "production-sanity" or + r.run_type == "package-and-qa" or + r.run_type == "nightly" + ) + |> filter(fn: (r) => r.status != "pending" and + r.merge_request == "false" and + r.quarantined == "false" and + r.reliable == "#{reliable}" and + r._field == "id" + ) + |> group(columns: ["name"]) QUERY end - def table(rows, title = nil) + def markdown_section(summary, result, stage, type) + <<~SECTION.strip + ``` + #{summary_table(summary, type)} + ``` + + ## #{stage} + + <details> + <summary>Executions table</summary> + + ``` + #{table(result, ['NAME', 'RUNS', 'FAILURES', 'FAILURE RATE'], "Top #{type} specs in '#{stage}' stage for past #{range} days")} + ``` + + </details> + SECTION + end + + def summary_table(summary, type) + table(summary, %w[STAGE COUNT], "#{type.capitalize} spec summary for past #{range} days".ljust(50)) + end + + def table(rows, headings, title) Terminal::Table.new( - headings: ["name", "runs", "failed", "failure rate"], + headings: headings, style: { all_separators: true }, title: title, rows: rows @@ -68,7 +99,7 @@ describe QA::Tools::ReliableReport do def name_column(spec_name) name = "name: '#{spec_name}'" - file = "file: 'spec.rb'".ljust(110) + file = "file: 'spec.rb'".ljust(160) "#{name}\n#{file}" end @@ -76,73 +107,85 @@ describe QA::Tools::ReliableReport do before do stub_env("QA_INFLUXDB_URL", "url") stub_env("QA_INFLUXDB_TOKEN", "token") - stub_env("CI_SLACK_WEBHOOK_URL", "slack_url") + stub_env("SLACK_WEBHOOK", "slack_url") + stub_env("CI_API_V4_URL", "gitlab_api_url") + stub_env("GITLAB_ACCESS_TOKEN", "gitlab_token") + allow(RestClient::Request).to receive(:execute).and_return(gitlab_response) allow(Slack::Notifier).to receive(:new).and_return(slack_notifier) allow(InfluxDB2::Client).to receive(:new).and_return(influx_client) - allow(query_api).to receive(:query).with(query: query).and_return(runs) - end - context "with stable spec report" do - let(:query) { flux_query(false) } - let(:fetch_message) { "Fetching data on test execution for past #{range} days in '#{run_type}' runs" } - let(:slack_send_message) { "Sending top stable spec report to #{slack_channel} slack channel" } - let(:message_title) { "Top #{results} stable specs for past #{range} days in '#{run_type}' runs" } - let(:table_title) { "Top stable specs in 'manage' stage" } - let(:rows) do - [ - [name_column("stable spec"), 3, 0, "0%"], - [name_column("unstable spec"), 3, 2, "66.67%"] - ] - end + allow(query_api).to receive(:query).with(query: flux_query(reliable: false)).and_return(runs) + allow(query_api).to receive(:query).with(query: flux_query(reliable: true)).and_return(reliable_runs) + end - it "prints top stable spec report to console" do - expect { reporter.show_top_stable }.to output("#{fetch_message}\n\n#{table(rows, table_title)}\n\n").to_stdout - end + context "without report creation" do + let(:create_issue) { "false" } - it "sends top stable spec report to slack" do - slack_args = { icon_emoji: ":mtg_green:", username: "Stable Spec Report" } + it "does not create report issue", :aggregate_failures do + expect { run }.to output.to_stdout - expect { reporter.notify_top_stable }.to output("\n#{slack_send_message}\n#{fetch_message}\n\n").to_stdout - expect(slack_notifier).to have_received(:post).with(text: "*#{message_title}*", **slack_args) - expect(slack_notifier).to have_received(:post).with(text: "```#{table(rows, table_title)}```", **slack_args) + expect(RestClient::Request).not_to have_received(:execute) + expect(slack_notifier).not_to have_received(:post) end end - context "with unstable spec report" do - let(:query) { flux_query(true) } - let(:fetch_message) { "Fetching data on reliable test execution for past #{range} days in '#{run_type}' runs" } - let(:slack_send_message) { "Sending top unstable reliable spec report to #{slack_channel} slack channel" } - let(:message_title) { "Top #{results} unstable reliable specs for past #{range} days in '#{run_type}' runs" } - let(:table_title) { "Top unstable specs in 'manage' stage" } - let(:rows) { [[name_column("unstable spec"), 3, 2, "66.67%"]] } + context "with report creation" do + let(:create_issue) { "true" } + let(:issue_body) do + <<~TXT.strip + [[_TOC_]] - it "prints top unstable spec report to console" do - expect { reporter.show_top_unstable }.to output("#{fetch_message}\n\n#{table(rows, table_title)}\n\n").to_stdout - end + # Candidates for promotion to reliable - it "sends top unstable reliable spec report to slack" do - slack_args = { icon_emoji: ":sadpanda:", username: "Unstable Spec Report" } + #{markdown_section([['manage', 1]], [[name_column('stable spec'), 3, 0, '0%']], 'manage', 'stable')} - expect { reporter.notify_top_unstable }.to output("#{fetch_message}\n\n\n#{slack_send_message}\n").to_stdout - expect(slack_notifier).to have_received(:post).with(text: "*#{message_title}*", **slack_args) - expect(slack_notifier).to have_received(:post).with(text: "```#{table(rows, table_title)}```", **slack_args) + # Reliable specs with failures + + #{markdown_section([['create', 1]], [[name_column('unstable spec'), 3, 2, '66.67%']], 'create', 'unstable')} + TXT + end + + it "creates report issue", :aggregate_failures do + expect { run }.to output.to_stdout + + expect(RestClient::Request).to have_received(:execute).with( + method: :post, + url: "gitlab_api_url/projects/278964/issues", + verify_ssl: false, + headers: { "PRIVATE-TOKEN" => "gitlab_token" }, + payload: { + title: "Reliable spec report", + description: issue_body, + labels: "Quality,test" + } + ) + expect(slack_notifier).to have_received(:post).with( + icon_emoji: ":tanuki-protect:", + text: <<~TEXT + ```#{summary_table([['manage', 1]], 'stable')}``` + ```#{summary_table([['create', 1]], 'unstable')}``` + + #{issue_url} + TEXT + ) end end - context "without unstable reliable specs" do - let(:query) { flux_query(true) } - let(:runs) { { 0 => stable_spec } } - let(:fetch_message) { "Fetching data on reliable test execution for past #{range} days in '#{run_type}' runs" } - let(:no_result_message) { "No unstable tests present!" } + context "with failure" do + let(:create_issue) { "true" } - it "prints no result message to console" do - expect { reporter.show_top_unstable }.to output("#{fetch_message}\n\n#{no_result_message}\n").to_stdout + before do + allow(query_api).to receive(:query).and_raise("Connection error!") end - it "skips slack notification" do - expect { reporter.notify_top_unstable }.to output("#{fetch_message}\n\n#{no_result_message}\n").to_stdout - expect(slack_notifier).not_to have_received(:post) + it "notifies failure", :aggregate_failures do + expect { expect { run }.to raise_error(SystemExit) }.to output.to_stdout + + expect(slack_notifier).to have_received(:post).with( + icon_emoji: ":sadpanda:", + text: "Reliable reporter failed to create report. Error: ```Connection error!```" + ) end end end diff --git a/qa/tasks/reliable_report.rake b/qa/tasks/reliable_report.rake index 204c959093a..4ec86779704 100644 --- a/qa/tasks/reliable_report.rake +++ b/qa/tasks/reliable_report.rake @@ -3,19 +3,8 @@ require_relative "../qa/tools/reliable_report" -desc "Fetch top most reliable specs" -task :reliable_spec_report, [:run_type, :range, :create_slack_report] do |_task, args| - report = QA::Tools::ReliableReport.new(args[:run_type] || "package-and-qa", args[:range]) - - report.show_top_stable - report.notify_top_stable if args[:create_slack_report] == 'true' -end - -desc "Fetch top most unstable reliable specs" -task :unreliable_spec_report, [:run_type, :range, :create_slack_report] do |_task, args| - report = QA::Tools::ReliableReport.new(args[:run_type] || "package-and-qa", args[:range]) - - report.show_top_unstable - report.notify_top_unstable if args[:create_slack_report] == 'true' +desc "Fetch reliable and unreliable spec data and create report" +task :reliable_spec_report, [:range, :report_in_issue_and_slack] do |_task, args| + QA::Tools::ReliableReport.run(**args) end # rubocop:enable Rails/RakeEnvironment diff --git a/spec/frontend/boards/board_list_helper.js b/spec/frontend/boards/board_list_helper.js index 27715cc25a4..d0f14bd37c1 100644 --- a/spec/frontend/boards/board_list_helper.js +++ b/spec/frontend/boards/board_list_helper.js @@ -8,7 +8,7 @@ import BoardNewIssue from '~/boards/components/board_new_issue.vue'; import BoardNewItem from '~/boards/components/board_new_item.vue'; import defaultState from '~/boards/stores/state'; import createMockApollo from 'helpers/mock_apollo_helper'; -import listQuery from '~/boards/graphql/board_lists_deferred.query.graphql'; +import listQuery from 'ee_else_ce/boards/graphql/board_lists_deferred.query.graphql'; import { mockList, mockIssuesByListId, diff --git a/spec/frontend/boards/components/board_list_header_spec.js b/spec/frontend/boards/components/board_list_header_spec.js index ec6f7e06e61..148d0c5684d 100644 --- a/spec/frontend/boards/components/board_list_header_spec.js +++ b/spec/frontend/boards/components/board_list_header_spec.js @@ -8,7 +8,7 @@ import { extendedWrapper } from 'helpers/vue_test_utils_helper'; import { boardListQueryResponse, mockLabelList } from 'jest/boards/mock_data'; import BoardListHeader from '~/boards/components/board_list_header.vue'; import { ListType } from '~/boards/constants'; -import listQuery from '~/boards/graphql/board_lists_deferred.query.graphql'; +import listQuery from 'ee_else_ce/boards/graphql/board_lists_deferred.query.graphql'; Vue.use(VueApollo); Vue.use(Vuex); diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index 641c6a2cd91..7e940d52a41 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -523,15 +523,6 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do end end - context "missing spam_check_api_key value when spam_check_endpoint_enabled is true" do - it "returns a blank parameter error message" do - put api("/application/settings", admin), params: { spam_check_endpoint_enabled: true, spam_check_endpoint_url: "https://example.com/spam_check" } - - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['error']).to eq('spam_check_api_key is missing') - end - end - context "overly long spam_check_api_key" do it "fails to update the settings with too long spam_check_api_key" do put api("/application/settings", admin), params: { spam_check_api_key: "0123456789" * 500 } diff --git a/spec/services/protected_tags/create_service_spec.rb b/spec/services/protected_tags/create_service_spec.rb index e85a43eb51c..3d06cc9fb6c 100644 --- a/spec/services/protected_tags/create_service_spec.rb +++ b/spec/services/protected_tags/create_service_spec.rb @@ -7,17 +7,54 @@ RSpec.describe ProtectedTags::CreateService do let(:user) { project.owner } let(:params) do { - name: 'master', + name: name, create_access_levels_attributes: [{ access_level: Gitlab::Access::MAINTAINER }] } end describe '#execute' do + let(:name) { 'tag' } + subject(:service) { described_class.new(project, user, params) } it 'creates a new protected tag' do expect { service.execute }.to change(ProtectedTag, :count).by(1) expect(project.protected_tags.last.create_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER]) end + + context 'when name has escaped HTML' do + let(:name) { 'tag->test' } + + it 'creates the new protected tag matching the unescaped version' do + expect { service.execute }.to change(ProtectedTag, :count).by(1) + expect(project.protected_tags.last.name).to eq('tag->test') + end + + context 'and name contains HTML tags' do + let(:name) { '<b>tag</b>' } + + it 'creates the new protected tag with sanitized name' do + expect { service.execute }.to change(ProtectedTag, :count).by(1) + expect(project.protected_tags.last.name).to eq('tag') + end + + context 'and contains unsafe HTML' do + let(:name) { '<script>alert('foo');</script>' } + + it 'does not create the new protected tag' do + expect { service.execute }.not_to change(ProtectedTag, :count) + end + end + end + + context 'when name contains unescaped HTML tags' do + let(:name) { '<b>tag</b>' } + + it 'creates the new protected tag with sanitized name' do + expect { service.execute }.to change(ProtectedTag, :count).by(1) + expect(project.protected_tags.last.name).to eq('tag') + end + end + end end end diff --git a/spec/services/protected_tags/update_service_spec.rb b/spec/services/protected_tags/update_service_spec.rb index ed151ca2347..22005bb9b89 100644 --- a/spec/services/protected_tags/update_service_spec.rb +++ b/spec/services/protected_tags/update_service_spec.rb @@ -6,17 +6,50 @@ RSpec.describe ProtectedTags::UpdateService do let(:protected_tag) { create(:protected_tag) } let(:project) { protected_tag.project } let(:user) { project.owner } - let(:params) { { name: 'new protected tag name' } } + let(:params) { { name: new_name } } describe '#execute' do + let(:new_name) { 'new protected tag name' } + let(:result) { service.execute(protected_tag) } + subject(:service) { described_class.new(project, user, params) } it 'updates a protected tag' do - result = service.execute(protected_tag) - expect(result.reload.name).to eq(params[:name]) end + context 'when name has escaped HTML' do + let(:new_name) { 'tag->test' } + + it 'updates protected tag name with unescaped HTML' do + expect(result.reload.name).to eq('tag->test') + end + + context 'and name contains HTML tags' do + let(:new_name) { '<b>tag</b>' } + + it 'updates protected tag name with sanitized name' do + expect(result.reload.name).to eq('tag') + end + + context 'and contains unsafe HTML' do + let(:new_name) { '<script>alert('foo');</script>' } + + it 'does not update the protected tag' do + expect(result.reload.name).to eq(protected_tag.name) + end + end + end + end + + context 'when name contains unescaped HTML tags' do + let(:new_name) { '<b>tag</b>' } + + it 'updates protected tag name with sanitized name' do + expect(result.reload.name).to eq('tag') + end + end + context 'without admin_project permissions' do let(:user) { create(:user) } |