diff --git a/.gitlab/ci/review-apps/qa.gitlab-ci.yml b/.gitlab/ci/review-apps/qa.gitlab-ci.yml index d2192a7511a..bf013abdd43 100644 --- a/.gitlab/ci/review-apps/qa.gitlab-ci.yml +++ b/.gitlab/ci/review-apps/qa.gitlab-ci.yml @@ -1,6 +1,6 @@ include: - project: gitlab-org/quality/pipeline-common - ref: 0.3.6 + ref: 0.3.9 file: - /ci/allure-report.yml - /ci/knapsack-report.yml diff --git a/.projections.json.example b/.projections.json.example index eff09c73140..dc1b42b917a 100644 --- a/.projections.json.example +++ b/.projections.json.example @@ -1,4 +1,5 @@ { + "ee/*": { "type": "ee" }, "config/initializers/*.rb": { "alternate": "spec/initializers/{}_spec.rb", "type": "source" @@ -7,6 +8,82 @@ "alternate": "config/initializers/{}.rb", "type": "test" }, + "app/channels/*.rb": { + "related": "ee/app/channels/ee/{}.rb", + "type": "source" + }, + "app/components/*.rb": { + "related": "ee/app/components/ee/{}.rb", + "type": "source" + }, + "app/controllers/*.rb": { + "related": "ee/app/controllers/ee/{}.rb", + "type": "source" + }, + "app/enums/*.rb": { + "related": "ee/app/enums/ee/{}.rb", + "type": "source" + }, + "app/events/*.rb": { + "related": "ee/app/events/ee/{}.rb", + "type": "source" + }, + "app/experiments/*.rb": { + "related": "ee/app/experiments/ee/{}.rb", + "type": "source" + }, + "app/finders/*.rb": { + "related": "ee/app/finders/ee/{}.rb", + "type": "source" + }, + "app/graphql/*.rb": { + "related": "ee/app/graphql/ee/{}.rb", + "type": "source" + }, + "app/helpers/*.rb": { + "related": "ee/app/helpers/ee/{}.rb", + "type": "source" + }, + "app/mailers/*.rb": { + "related": "ee/app/mailers/ee/{}.rb", + "type": "source" + }, + "app/models/*.rb": { + "related": "ee/app/models/ee/{}.rb", + "type": "source" + }, + "app/policies/*.rb": { + "related": "ee/app/policies/ee/{}.rb", + "type": "source" + }, + "app/presenters/*.rb": { + "related": "ee/app/presenters/ee/{}.rb", + "type": "source" + }, + "app/serializers/*.rb": { + "related": "ee/app/serializers/ee/{}.rb", + "type": "source" + }, + "app/services/*.rb": { + "related": "ee/app/services/ee/{}.rb", + "type": "source" + }, + "app/uploaders/*.rb": { + "related": "ee/app/uploaders/ee/{}.rb", + "type": "source" + }, + "app/validators/*.rb": { + "related": "ee/app/validators/ee/{}.rb", + "type": "source" + }, + "app/views/*.rb": { + "related": "ee/app/views/ee/{}.rb", + "type": "source" + }, + "app/workers/*.rb": { + "related": "ee/app/workers/ee/{}.rb", + "type": "source" + }, "app/*.rb": { "alternate": "spec/{}_spec.rb", "type": "source" diff --git a/.rubocop.yml b/.rubocop.yml index 50729efd1ce..a944e783e3d 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -329,17 +329,12 @@ GitlabSecurity/PublicSend: Database/MultipleDatabases: Enabled: true - Include: - - 'app/**/*.rb' - - 'ee/app/**/*.rb' - - 'lib/**/*.rb' - - 'ee/lib/**/*.rb' - - 'spec/**/*.rb' - - 'ee/spec/**/*.rb' Exclude: - 'ee/db/**/*.rb' - 'spec/migrations/**/*.rb' + - 'lib/tasks/gitlab/db.rake' - 'lib/gitlab/background_migration/**/*.rb' + - 'ee/lib/ee/gitlab/background_migration/**/*.rb' - 'spec/lib/gitlab/background_migration/**/*.rb' - 'spec/lib/gitlab/database/**/*.rb' diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 1ce7cbaabfb..673e9171ef9 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -58,11 +58,6 @@ Performance/AncestorsInclude: Exclude: - 'lib/gitlab/ci/config/extendable/entry.rb' -# Offense count: 40 -# Cop supports --auto-correct. -Performance/BlockGivenWithExplicitBlock: - Enabled: false - # Offense count: 29 # Configuration parameters: MinSize. Performance/CollectionLiteralInLoop: diff --git a/.rubocop_todo/database/multiple_databases.yml b/.rubocop_todo/database/multiple_databases.yml index 27e28128a98..8e372b89bbd 100644 --- a/.rubocop_todo/database/multiple_databases.yml +++ b/.rubocop_todo/database/multiple_databases.yml @@ -1,33 +1,38 @@ --- Database/MultipleDatabases: Exclude: - - ee/spec/services/ee/merge_requests/update_service_spec.rb - - lib/backup/database.rb - - lib/backup/manager.rb - - lib/gitlab/database/load_balancing/load_balancer.rb - - lib/gitlab/database/load_balancing.rb - - lib/gitlab/database/load_balancing/sticking.rb - - lib/gitlab/database/migrations/observers/migration_observer.rb - - lib/gitlab/database/migrations/observers/query_log.rb - - lib/gitlab/database/partitioning_migration_helpers/backfill_partitioned_table.rb - - lib/gitlab/database.rb - - lib/gitlab/health_checks/db_check.rb - - lib/gitlab/seeder.rb - - spec/db/schema_spec.rb - - spec/initializers/database_config_spec.rb - - spec/lib/backup/manager_spec.rb - - spec/lib/gitlab/database_spec.rb - - spec/lib/gitlab/metrics/subscribers/active_record_spec.rb - - spec/lib/gitlab/profiler_spec.rb - - spec/lib/gitlab/usage/metrics/names_suggestions/relation_parsers/constraints_spec.rb - - spec/lib/gitlab/usage/metrics/names_suggestions/relation_parsers/joins_spec.rb - - spec/support/caching.rb - - spec/support/gitlab/usage/metrics_instrumentation_shared_examples.rb - - spec/support/helpers/database_connection_helpers.rb - - spec/support/helpers/database/database_helpers.rb - - spec/support/helpers/database/table_schema_helpers.rb - - spec/support/helpers/migrations_helpers.rb - - spec/support/helpers/query_recorder.rb - - spec/support/helpers/usage_data_helpers.rb - - spec/tasks/gitlab/backup_rake_spec.rb - - spec/tasks/gitlab/db_rake_spec.rb + - 'config/application.rb' + - 'config/initializers/active_record_data_types.rb' + - 'config/initializers/active_record_force_reconnects.rb' + - 'config/initializers/active_record_lifecycle.rb' + - 'config/initializers/sidekiq.rb' + - 'config/initializers/validate_database_config.rb' + - 'db/post_migrate/20210317104032_set_iteration_cadence_automatic_to_false.rb' + - 'db/post_migrate/20210811122206_update_external_project_bots.rb' + - 'db/post_migrate/20210812013042_remove_duplicate_project_authorizations.rb' + - 'ee/lib/tasks/geo.rake' + - 'ee/spec/services/ee/merge_requests/update_service_spec.rb' + - 'lib/backup/database.rb' + - 'lib/backup/manager.rb' + - 'lib/gitlab/database.rb' + - 'lib/gitlab/database/load_balancing/load_balancer.rb' + - 'lib/gitlab/database/migrations/observers/query_log.rb' + - 'lib/tasks/dev.rake' + - 'lib/tasks/gitlab/db/validate_config.rake' + - 'spec/db/schema_spec.rb' + - 'spec/initializers/database_config_spec.rb' + - 'spec/lib/backup/manager_spec.rb' + - 'spec/lib/gitlab/database_spec.rb' + - 'spec/lib/gitlab/metrics/subscribers/active_record_spec.rb' + - 'spec/lib/gitlab/profiler_spec.rb' + - 'spec/lib/gitlab/usage/metrics/names_suggestions/relation_parsers/constraints_spec.rb' + - 'spec/lib/gitlab/usage/metrics/names_suggestions/relation_parsers/joins_spec.rb' + - 'spec/support/caching.rb' + - 'spec/support/gitlab/usage/metrics_instrumentation_shared_examples.rb' + - 'spec/support/helpers/database/database_helpers.rb' + - 'spec/support/helpers/database/table_schema_helpers.rb' + - 'spec/support/helpers/migrations_helpers.rb' + - 'spec/support/helpers/query_recorder.rb' + - 'spec/support/helpers/usage_data_helpers.rb' + - 'spec/tasks/gitlab/backup_rake_spec.rb' + - 'spec/tasks/gitlab/db_rake_spec.rb' diff --git a/.rubocop_todo/performance/block_given_with_explicit_block.yml b/.rubocop_todo/performance/block_given_with_explicit_block.yml new file mode 100644 index 00000000000..ae61c5a86e6 --- /dev/null +++ b/.rubocop_todo/performance/block_given_with_explicit_block.yml @@ -0,0 +1,44 @@ +--- +# Cop supports --auto-correct. +Performance/BlockGivenWithExplicitBlock: + # Offense count: 53 + # Temporarily disabled due to too many offenses + Enabled: false + Exclude: + - 'app/controllers/concerns/redis_tracking.rb' + - 'app/helpers/badges_helper.rb' + - 'app/helpers/instance_configuration_helper.rb' + - 'app/helpers/labels_helper.rb' + - 'app/helpers/tab_helper.rb' + - 'app/services/base_count_service.rb' + - 'app/services/error_tracking/base_service.rb' + - 'app/services/projects/open_issues_count_service.rb' + - 'app/services/users/update_service.rb' + - 'ee/lib/elastic/latest/query_context.rb' + - 'ee/lib/gitlab/geo.rb' + - 'lib/bulk_imports/clients/http.rb' + - 'lib/gitlab/batch_pop_queueing.rb' + - 'lib/gitlab/cache/request_cache.rb' + - 'lib/gitlab/ci/trace/chunked_io.rb' + - 'lib/gitlab/database/bulk_update.rb' + - 'lib/gitlab/database/with_lock_retries.rb' + - 'lib/gitlab/github_import/client.rb' + - 'lib/gitlab/legacy_github_import/client.rb' + - 'lib/gitlab/metrics/methods/metric_options.rb' + - 'lib/gitlab/null_request_store.rb' + - 'lib/gitlab/quick_actions/dsl.rb' + - 'lib/gitlab/safe_request_loader.rb' + - 'lib/gitlab/search/query.rb' + - 'lib/gitlab/string_placeholder_replacer.rb' + - 'lib/gitlab/terraform/state_migration_helper.rb' + - 'lib/gitlab/usage/metrics/instrumentations/database_metric.rb' + - 'lib/gitlab/usage_data_queries.rb' + - 'lib/gitlab/utils/usage_data.rb' + - 'qa/qa/page/view.rb' + - 'qa/qa/runtime/browser.rb' + - 'spec/lib/api/helpers/authentication_spec.rb' + - 'spec/lib/gitlab/slash_commands/deploy_spec.rb' + - 'spec/support/helpers/graphql_helpers.rb' + - 'spec/support/helpers/query_recorder.rb' + - 'tooling/lib/tooling/helm3_client.rb' + - 'tooling/lib/tooling/test_map_packer.rb' diff --git a/app/assets/javascripts/behaviors/copy_to_clipboard.js b/app/assets/javascripts/behaviors/copy_to_clipboard.js index c3c28aeafc0..07fd6dae76a 100644 --- a/app/assets/javascripts/behaviors/copy_to_clipboard.js +++ b/app/assets/javascripts/behaviors/copy_to_clipboard.js @@ -43,7 +43,7 @@ function genericSuccess(e) { } /** - * Safari > 10 doesn't support `execCommand`, so instead we inform the user to copy manually. + * Safari < 10 doesn't support `execCommand`, so instead we inform the user to copy manually. * See http://clipboardjs.com/#browser-support */ function genericError(e) { diff --git a/app/assets/javascripts/lib/utils/common_utils.js b/app/assets/javascripts/lib/utils/common_utils.js index 96d019f62f2..7ff623a1adc 100644 --- a/app/assets/javascripts/lib/utils/common_utils.js +++ b/app/assets/javascripts/lib/utils/common_utils.js @@ -282,23 +282,51 @@ export const getSelectedFragment = (restrictToNode) => { return documentFragment; }; -export const insertText = (target, text) => { - // Firefox doesn't support `document.execCommand('insertText', false, text)` on textareas - const { selectionStart, selectionEnd, value } = target; +function execInsertText(text) { + if (text === '') return document.execCommand('delete'); + return document.execCommand('insertText', false, text); +} + +/** + * This method inserts text into a textarea/input field. + * Uses `execCommand` if supported + * + * @param {HTMLElement} target - textarea/input to have text inserted into + * @param {String | function} text - text to be inserted + */ +export const insertText = (target, text) => { + const { selectionStart, selectionEnd, value } = target; const textBefore = value.substring(0, selectionStart); const textAfter = value.substring(selectionEnd, value.length); - const insertedText = text instanceof Function ? text(textBefore, textAfter) : text; - const newText = textBefore + insertedText + textAfter; - // eslint-disable-next-line no-param-reassign - target.value = newText; - // eslint-disable-next-line no-param-reassign - target.selectionStart = selectionStart + insertedText.length; + // The `execCommand` is officially deprecated. However, for `insertText`, + // there is currently no alternative. We need to use it in order to trigger + // the browser's undo tracking when we insert text. + // Per https://developer.mozilla.org/en-US/docs/Web/API/Document/execCommand on 2022-04-11, + // The Clipboard API can be used instead of execCommand in many cases, + // but execCommand is still sometimes useful. In particular, the Clipboard + // API doesn't replace the insertText command + // So we attempt to use it if possible. Otherwise, fall back to just replacing + // the value as before. In this case, Undo will be broken with inserted text. + // Testing on older versions of Firefox: + // 87 and below: does not work and falls through to just replacing value. + // 87 was released in Mar of 2021 + // 89 and above: works well + // 89 was released in May of 2021 + if (!execInsertText(insertedText)) { + const newText = textBefore + insertedText + textAfter; - // eslint-disable-next-line no-param-reassign - target.selectionEnd = selectionStart + insertedText.length; + // eslint-disable-next-line no-param-reassign + target.value = newText; + + // eslint-disable-next-line no-param-reassign + target.selectionStart = selectionStart + insertedText.length; + + // eslint-disable-next-line no-param-reassign + target.selectionEnd = selectionStart + insertedText.length; + } // Trigger autosave target.dispatchEvent(new Event('input')); diff --git a/app/controllers/admin/runners_controller.rb b/app/controllers/admin/runners_controller.rb index 06880ace899..6d552808f28 100644 --- a/app/controllers/admin/runners_controller.rb +++ b/app/controllers/admin/runners_controller.rb @@ -22,7 +22,7 @@ class Admin::RunnersController < Admin::ApplicationController end def edit - assign_builds_and_projects + assign_projects end def update @@ -31,7 +31,7 @@ class Admin::RunnersController < Admin::ApplicationController format.html { redirect_to edit_admin_runner_path(@runner) } end else - assign_builds_and_projects + assign_projects render 'show' end end @@ -87,12 +87,7 @@ class Admin::RunnersController < Admin::ApplicationController end # rubocop: disable CodeReuse/ActiveRecord - def assign_builds_and_projects - @builds = runner - .builds - .order_id_desc - .preload_project_and_pipeline_project.first(30) - + def assign_projects @projects = if params[:search].present? ::Project.search(params[:search]) diff --git a/app/views/admin/runners/edit.html.haml b/app/views/admin/runners/edit.html.haml index 5570c46c17f..ab6ad709068 100644 --- a/app/views/admin/runners/edit.html.haml +++ b/app/views/admin/runners/edit.html.haml @@ -1,5 +1,3 @@ -- add_page_specific_style 'page_bundles/ci_status' - - runner_name = "##{@runner.id} (#{@runner.short_sha})" - if Feature.enabled?(:runner_read_only_admin_view, default_enabled: :yaml) - breadcrumb_title _('Edit') @@ -12,84 +10,47 @@ #js-admin-runner-edit{ data: {runner_id: @runner.id} } -.row - .col-md-6 - %h4= _('Restrict projects for this runner') - - if @runner.runner_projects.any? - %table.table{ data: { testid: 'assigned-projects' } } - %thead - %tr - %th= _('Assigned projects') - - @runner.runner_projects.each do |runner_project| - - project = runner_project.project - - if project - %tr - %td - = render Pajamas::AlertComponent.new(variant: :danger, - dismissible: false, - title: project.full_name) do - .gl-alert-actions - = link_to _('Disable'), admin_namespace_project_runner_project_path(project.namespace, project, runner_project), method: :delete, class: 'btn gl-alert-action btn-confirm btn-md gl-button' - - %table.table{ data: { testid: 'unassigned-projects' } } - %thead - %tr - %th= _('Project') - %th +.gl-overflow-auto + %h4= _('Restrict projects for this runner') +- if @runner.runner_projects.any? + %table.table{ data: { testid: 'assigned-projects' } } + %thead %tr - %td - = form_tag edit_admin_runner_path(@runner), id: 'runner-projects-search', class: 'form-inline', method: :get do - .input-group - = search_field_tag :search, params[:search], class: 'form-control gl-form-input', spellcheck: false - .input-group-append - = submit_tag _('Search'), class: 'gl-button btn btn-default' - - %td - - @projects.each do |project| + %th= _('Assigned projects') + - @runner.runner_projects.each do |runner_project| + - project = runner_project.project + - if project %tr %td - = project.full_name - %td - .float-right - = form_for project.runner_projects.new, url: admin_namespace_project_runner_projects_path(project.namespace, project), method: :post do |f| - = f.hidden_field :runner_id, value: @runner.id - = f.submit _('Enable'), aria: { label: s_('Runners|Change to project runner') }, class: 'gl-button btn btn-sm', data: { confirm: (s_('Runners|You are about to change this instance runner to a project runner. This operation is not reversible. Are you sure you want to continue?') if @runner.instance_type?), confirm_btn_variant: 'danger' } - = paginate_without_count @projects + = render Pajamas::AlertComponent.new(variant: :danger, + dismissible: false, + title: project.full_name) do + .gl-alert-actions + = link_to _('Disable'), admin_namespace_project_runner_project_path(project.namespace, project, runner_project), method: :delete, class: 'btn gl-alert-action btn-confirm btn-md gl-button' - .col-md-6 - %h4= _('Recent jobs served by this runner') - %table.table.ci-table.runner-builds - %thead - %tr - %th= _('Job') - %th= _('Status') - %th= _('Project') - %th= _('Commit') - %th= _('Finished at') +%table.table{ data: { testid: 'unassigned-projects' } } + %thead + %tr + %th= _('Project') + %th - - @builds.each do |build| - - project = build.project - %tr.build - %td.id - - if project - = link_to project_job_path(project, build) do - %strong ##{build.id} - - else - %strong ##{build.id} + %tr + %td + = form_tag edit_admin_runner_path(@runner), id: 'runner-projects-search', class: 'form-inline', method: :get do + .input-group + = search_field_tag :search, params[:search], class: 'form-control gl-form-input', spellcheck: false + .input-group-append + = submit_tag _('Search'), class: 'gl-button btn btn-default' - %td.status - = render 'ci/status/badge', status: build.detailed_status(current_user) - - %td.status - - if project - = project.full_name - - %td.build-link - - if project - = link_to pipeline_path(build.pipeline) do - %strong= build.pipeline.short_sha - - %td.timestamp - - if build.finished_at - %span= time_ago_with_tooltip build.finished_at + %td + - @projects.each do |project| + %tr + %td + = project.full_name + %td + .float-right + = form_for project.runner_projects.new, url: admin_namespace_project_runner_projects_path(project.namespace, project), method: :post do |f| + = f.hidden_field :runner_id, value: @runner.id + = f.submit _('Enable'), aria: { label: s_('Runners|Change to project runner') }, class: 'gl-button btn btn-sm', data: { confirm: (s_('Runners|You are about to change this instance runner to a project runner. This operation is not reversible. Are you sure you want to continue?') if @runner.instance_type?), confirm_btn_variant: 'danger' } += paginate_without_count @projects diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 4ba268fe941..369cf5b3cd0 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -16040,9 +16040,6 @@ msgstr "" msgid "Finished" msgstr "" -msgid "Finished at" -msgstr "" - msgid "First Name" msgstr "" @@ -31003,9 +31000,6 @@ msgstr "" msgid "Recent events" msgstr "" -msgid "Recent jobs served by this runner" -msgstr "" - msgid "Recent searches" msgstr "" 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 eb4eb1d3e71..5fc9f8c2f41 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,8 +1,7 @@ # frozen_string_literal: true module QA - # TODO: Remove :requires_admin when the `Runtime::Feature.enable` method call is removed - RSpec.describe 'Plan', :smoke, :requires_admin do + RSpec.describe 'Plan', :smoke, feature_flag: { name: 'vue_issues_list', scope: :group } do describe 'Issue creation' do let(:project) { Resource::Project.fabricate_via_api! } let(:closed_issue) { Resource::Issue.fabricate_via_api! { |issue| issue.project = project } } diff --git a/qa/qa/support/formatters/allure_metadata_formatter.rb b/qa/qa/support/formatters/allure_metadata_formatter.rb index 2ed4ee2066a..9729d6e8d96 100644 --- a/qa/qa/support/formatters/allure_metadata_formatter.rb +++ b/qa/qa/support/formatters/allure_metadata_formatter.rb @@ -20,11 +20,11 @@ module QA def start(_start_notification) return unless merge_request_iid # on main runs allure native history has pass rate already - save_failures - log(:debug, "Fetched #{failures.length} flaky testcases!") + save_flaky_specs + log(:debug, "Fetched #{flaky_specs.length} flaky testcases!") rescue StandardError => e log(:error, "Failed to fetch flaky spec data for report: #{e}") - @failures = {} + @flaky_specs = {} end # Finished example @@ -82,21 +82,20 @@ module QA # @param [RSpec::Core::Example] example # @return [void] def set_flaky_status(example) - return unless merge_request_iid - return unless example.execution_result.status == :failed && failures.key?(example.metadata[:testcase]) + return unless merge_request_iid && flaky_specs.key?(example.metadata[:testcase]) example.set_flaky - example.parameter("pass_rate", "#{failures[example.metadata[:testcase]].round(1)}%") - log(:debug, "Setting spec as flaky due to present failures in last 14 days!") + example.parameter("pass_rate", "#{flaky_specs[example.metadata[:testcase]].round(1)}%") + log(:debug, "Setting spec as flaky because it's pass rate is below 98%") end - # Failed spec testcases + # Flaky specs with pass rate below 98% # # @return [Array] - def failures - @failures ||= influx_data.lazy.each_with_object({}) do |data, result| - # TODO: replace with mr_iid once stats are populated - records = data.records.reject { |r| r.values["_value"] == env("CI_PIPELINE_ID") } + def flaky_specs + @flaky_specs ||= influx_data.lazy.each_with_object({}) do |data, result| + # Do not consider failures in same merge request + records = data.records.reject { |r| r.values["_value"] == merge_request_iid } runs = records.count failed = records.count { |r| r.values["status"] == "failed" } @@ -107,7 +106,7 @@ module QA end.compact end - alias_method :save_failures, :failures + alias_method :save_flaky_specs, :flaky_specs # Records of previous failures for runs of same type # @@ -122,7 +121,7 @@ module QA |> filter(fn: (r) => r.run_type == "#{run_type}" and r.status != "pending" and r.quarantined == "false" and - r._field == "pipeline_id" + r._field == "merge_request_iid" ) |> group(columns: ["testcase"]) QUERY diff --git a/spec/controllers/admin/runners_controller_spec.rb b/spec/controllers/admin/runners_controller_spec.rb index 8f70cb32d3e..8bdfbdcb46f 100644 --- a/spec/controllers/admin/runners_controller_spec.rb +++ b/spec/controllers/admin/runners_controller_spec.rb @@ -26,12 +26,6 @@ RSpec.describe Admin::RunnersController do describe '#show' do render_views - let_it_be(:project) { create(:project) } - - before_all do - create(:ci_build, runner: runner, project: project) - end - it 'shows a runner show page' do get :show, params: { id: runner.id } @@ -55,11 +49,6 @@ RSpec.describe Admin::RunnersController do let_it_be(:project) { create(:project) } let_it_be(:project_two) { create(:project) } - before_all do - create(:ci_build, runner: runner, project: project) - create(:ci_build, runner: runner, project: project_two) - end - it 'shows a runner edit page' do get :edit, params: { id: runner.id } @@ -77,9 +66,6 @@ RSpec.describe Admin::RunnersController do control_count = ActiveRecord::QueryRecorder.new { get :edit, params: { id: runner.id } }.count - new_project = create(:project) - create(:ci_build, runner: runner, project: new_project) - # There is one additional query looking up subject.group in ProjectPolicy for the # needs_new_sso_session permission expect { get :edit, params: { id: runner.id } }.not_to exceed_query_limit(control_count + 1) @@ -89,17 +75,42 @@ RSpec.describe Admin::RunnersController do end describe '#update' do - it 'updates the runner and ticks the queue' do - new_desc = runner.description.swapcase + let(:new_desc) { runner.description.swapcase } + let(:runner_params) { { id: runner.id, runner: { description: new_desc } } } - expect do - post :update, params: { id: runner.id, runner: { description: new_desc } } - end.to change { runner.ensure_runner_queue_value } + subject(:request) { post :update, params: runner_params } - runner.reload + context 'with update succeeding' do + before do + expect_next_instance_of(Ci::Runners::UpdateRunnerService, runner) do |service| + expect(service).to receive(:update).with(anything).and_call_original + end + end - expect(response).to have_gitlab_http_status(:found) - expect(runner.description).to eq(new_desc) + it 'updates the runner and ticks the queue' do + expect { request }.to change { runner.ensure_runner_queue_value } + + runner.reload + + expect(response).to have_gitlab_http_status(:found) + expect(runner.description).to eq(new_desc) + end + end + + context 'with update failing' do + before do + expect_next_instance_of(Ci::Runners::UpdateRunnerService, runner) do |service| + expect(service).to receive(:update).with(anything).and_return(false) + end + end + + it 'does not update runner or tick the queue' do + expect { request }.not_to change { runner.ensure_runner_queue_value } + expect { request }.not_to change { runner.reload.description } + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:show) + end end end diff --git a/spec/frontend/behaviors/copy_as_gfm_spec.js b/spec/frontend/behaviors/copy_as_gfm_spec.js index c96db09cc76..2032faa1c33 100644 --- a/spec/frontend/behaviors/copy_as_gfm_spec.js +++ b/spec/frontend/behaviors/copy_as_gfm_spec.js @@ -8,6 +8,9 @@ describe('CopyAsGFM', () => { beforeEach(() => { target = document.createElement('input'); target.value = 'This is code: '; + + // needed for the underlying insertText to work + document.execCommand = jest.fn(() => false); }); // When GFM code is copied, we put the regular plain text diff --git a/spec/frontend/lib/utils/common_utils_spec.js b/spec/frontend/lib/utils/common_utils_spec.js index 763a9bd30fe..8e499844406 100644 --- a/spec/frontend/lib/utils/common_utils_spec.js +++ b/spec/frontend/lib/utils/common_utils_spec.js @@ -283,6 +283,75 @@ describe('common_utils', () => { }); }); + describe('insertText', () => { + let textArea; + + beforeAll(() => { + textArea = document.createElement('textarea'); + document.querySelector('body').appendChild(textArea); + textArea.value = 'two'; + textArea.setSelectionRange(0, 0); + textArea.focus(); + }); + + afterAll(() => { + textArea.parentNode.removeChild(textArea); + }); + + describe('using execCommand', () => { + beforeAll(() => { + document.execCommand = jest.fn(() => true); + }); + + it('inserts the text', () => { + commonUtils.insertText(textArea, 'one'); + + expect(document.execCommand).toHaveBeenCalledWith('insertText', false, 'one'); + }); + + it('removes selected text', () => { + textArea.setSelectionRange(0, textArea.value.length); + + commonUtils.insertText(textArea, ''); + + expect(document.execCommand).toHaveBeenCalledWith('delete'); + }); + }); + + describe('using fallback', () => { + beforeEach(() => { + document.execCommand = jest.fn(() => false); + jest.spyOn(textArea, 'dispatchEvent'); + textArea.value = 'two'; + textArea.setSelectionRange(0, 0); + }); + + it('inserts the text', () => { + commonUtils.insertText(textArea, 'one'); + + expect(textArea.value).toBe('onetwo'); + expect(textArea.dispatchEvent).toHaveBeenCalled(); + }); + + it('replaces the selection', () => { + textArea.setSelectionRange(0, textArea.value.length); + + commonUtils.insertText(textArea, 'one'); + + expect(textArea.value).toBe('one'); + expect(textArea.selectionStart).toBe(textArea.value.length); + }); + + it('removes selected text', () => { + textArea.setSelectionRange(0, textArea.value.length); + + commonUtils.insertText(textArea, ''); + + expect(textArea.value).toBe(''); + }); + }); + }); + describe('normalizedHeaders', () => { it('should upperCase all the header keys to keep them consistent', () => { const apiHeaders = { diff --git a/spec/frontend/lib/utils/text_markdown_spec.js b/spec/frontend/lib/utils/text_markdown_spec.js index 99204639197..cc7d759274d 100644 --- a/spec/frontend/lib/utils/text_markdown_spec.js +++ b/spec/frontend/lib/utils/text_markdown_spec.js @@ -14,6 +14,9 @@ describe('init markdown', () => { textArea = document.createElement('textarea'); document.querySelector('body').appendChild(textArea); textArea.focus(); + + // needed for the underlying insertText to work + document.execCommand = jest.fn(() => false); }); afterAll(() => { diff --git a/spec/frontend/vue_shared/components/markdown/field_spec.js b/spec/frontend/vue_shared/components/markdown/field_spec.js index d1c4d777d44..ab74ea868a2 100644 --- a/spec/frontend/vue_shared/components/markdown/field_spec.js +++ b/spec/frontend/vue_shared/components/markdown/field_spec.js @@ -187,6 +187,11 @@ describe('Markdown field component', () => { }); describe('markdown buttons', () => { + beforeEach(() => { + // needed for the underlying insertText to work + document.execCommand = jest.fn(() => false); + }); + it('converts single words', async () => { const textarea = subject.find('textarea').element; textarea.setSelectionRange(0, 7);