diff --git a/.gitlab/CODEOWNERS b/.gitlab/CODEOWNERS index 573ba8de595..a743a601127 100644 --- a/.gitlab/CODEOWNERS +++ b/.gitlab/CODEOWNERS @@ -57,6 +57,84 @@ Dangerfile @gl-quality/eng-prod /ee/lib/ee/gitlab/auth/ldap/ @dblessing @mkozono /lib/gitlab/auth/ldap/ @dblessing @mkozono +^[Verify] +/app/**/ci/ @gitlab-org/maintainers/cicd-verify +/app/controllers/admin/jobs_controller.rb @gitlab-org/maintainers/cicd-verify +/app/controllers/admin/runner_projects_controller.rb @gitlab-org/maintainers/cicd-verify +/app/controllers/admin/runners_controller.rb @gitlab-org/maintainers/cicd-verify +/app/controllers/projects/artifacts_controller.rb @gitlab-org/maintainers/cicd-verify +/app/controllers/projects/build_artifacts_controller.rb @gitlab-org/maintainers/cicd-verify +/app/controllers/projects/builds_controller.rb @gitlab-org/maintainers/cicd-verify +/app/controllers/projects/jobs_controller.rb @gitlab-org/maintainers/cicd-verify +/app/controllers/runner_setup_controller.rb @gitlab-org/maintainers/cicd-verify +/app/controllers/projects/pipeline_schedules_controller.rb @gitlab-org/maintainers/cicd-verify +/app/controllers/projects/pipelines_controller.rb @gitlab-org/maintainers/cicd-verify +/app/controllers/projects/pipelines_settings_controller.rb @gitlab-org/maintainers/cicd-verify +/app/controllers/projects/runner_projects_controller.rb @gitlab-org/maintainers/cicd-verify +/app/controllers/projects/runners_controller.rb @gitlab-org/maintainers/cicd-verify +/app/controllers/projects/triggers_controller.rb @gitlab-org/maintainers/cicd-verify +/app/controllers/projects/usage_quotas_controller.rb @gitlab-org/maintainers/cicd-verify +/app/controllers/projects/variables_controller.rb @gitlab-org/maintainers/cicd-verify +/app/models/commit_status.rb @gitlab-org/maintainers/cicd-verify +/app/models/external_pull_request.rb @gitlab-org/maintainers/cicd-verify +/app/models/generic_commit_status.rb @gitlab-org/maintainers/cicd-verify +/app/models/namespace_ci_cd_setting.rb @gitlab-org/maintainers/cicd-verify +/app/models/project_ci_cd_setting.rb @gitlab-org/maintainers/cicd-verify +/app/presenters/commit_status_presenter.rb @gitlab-org/maintainers/cicd-verify +/app/presenters/generic_commit_status_presenter.rb @gitlab-org/maintainers/cicd-verify +/app/views/projects/artifacts/ @gitlab-org/maintainers/cicd-verify +/app/views/projects/generic_commit_statuses/ @gitlab-org/maintainers/cicd-verify +/app/views/projects/jobs/ @gitlab-org/maintainers/cicd-verify +/app/views/projects/pipeline_schedules/ @gitlab-org/maintainers/cicd-verify +/app/views/projects/pipelines/ @gitlab-org/maintainers/cicd-verify +/app/views/projects/triggers/ @gitlab-org/maintainers/cicd-verify +/app/workers/build_hooks_worker.rb @gitlab-org/maintainers/cicd-verify +/app/workers/build_queue_worker.rb @gitlab-org/maintainers/cicd-verify +/app/workers/build_success_worker.rb @gitlab-org/maintainers/cicd-verify +/app/workers/ci_platform_metrics_update_cron_worker.rb @gitlab-org/maintainers/cicd-verify +/app/workers/create_pipeline_worker.rb @gitlab-org/maintainers/cicd-verify +/app/workers/expire_build_artifacts_worker.rb @gitlab-org/maintainers/cicd-verify +/app/workers/pipeline_hooks_worker.rb @gitlab-org/maintainers/cicd-verify +/app/workers/pipeline_metrics_worker.rb @gitlab-org/maintainers/cicd-verify +/app/workers/pipeline_notification_worker.rb @gitlab-org/maintainers/cicd-verify +/app/workers/pipeline_process_worker.rb @gitlab-org/maintainers/cicd-verify +/app/workers/pipeline_schedule_worker.rb @gitlab-org/maintainers/cicd-verify +/app/workers/run_pipeline_schedule_worker.rb @gitlab-org/maintainers/cicd-verify +/app/workers/stuck_ci_jobs_worker.rb @gitlab-org/maintainers/cicd-verify +/app/workers/update_external_pull_requests_worker.rb @gitlab-org/maintainers/cicd-verify +/lib/**/ci/ @gitlab-org/maintainers/cicd-verify +/lib/api/commit_statuses.rb @gitlab-org/maintainers/cicd-verify +/ee/app/**/ci/ @gitlab-org/maintainers/cicd-verify +/ee/app/**/merge_trains/ @gitlab-org/maintainers/cicd-verify +/ee/app/models/merge_train.rb @gitlab-org/maintainers/cicd-verify +/ee/app/finders/merge_trains_finder.rb @gitlab-org/maintainers/cicd-verify +/ee/app/services/auto_merge/add_to_merge_train_when_pipeline_succeeds_service.rb @gitlab-org/maintainers/cicd-verify +/ee/app/services/auto_merge/merge_train_service.rb @gitlab-org/maintainers/cicd-verify +/ee/app/services/system_notes/merge_train_service.rb @gitlab-org/maintainers/cicd-verify +/ee/app/controllers/ee/admin/runners_controller.rb @gitlab-org/maintainers/cicd-verify +/ee/app/controllers/ee/projects/pipelines_controller.rb @gitlab-org/maintainers/cicd-verify +/ee/app/controllers/projects/pipelines/ @gitlab-org/maintainers/cicd-verify +/ee/app/controllers/projects/subscriptions_controller.rb @gitlab-org/maintainers/cicd-verify +/ee/app/models/merge_train.rb @gitlab-org/maintainers/cicd-verify +/ee/app/helpers/ee/projects/pipeline_helper.rb @gitlab-org/maintainers/cicd-verify +/ee/app/views/ci_minutes_usage_mailer/ @gitlab-org/maintainers/cicd-verify +/ee/app/views/projects/pipelines/ @gitlab-org/maintainers/cicd-verify +/ee/app/views/projects/settings/ci_cd/ @gitlab-org/maintainers/cicd-verify +/ee/app/workers/clear_shared_runners_minutes_worker.rb @gitlab-org/maintainers/cicd-verify +/ee/lib/**/ci/ @gitlab-org/maintainers/cicd-verify +/ee/lib/ee/api/entities/merge_train.rb @gitlab-org/maintainers/cicd-verify +/**/javascripts/jobs/ @pburdette @jivanvl +/**/javascripts/pipelines/ @pburdette @f_caplette @jivanvl @mfluharty @bsandlin @mgandres +/app/assets/javascripts/pipeline_new/ @pburdette @f_caplette @jivanvl @mfluharty @bsandlin @mgandres +/app/assets/javascripts/ci_lint/ @f_caplette @bsandlin @mgandres +/app/assets/javascripts/ci_variable_list/ @pburdette @f_caplette @jivanvl @mfluharty @bsandlin @mgandres +/app/assets/javascripts/pipeline_schedules/ @pburdette @jivanvl +/app/assets/javascripts/pipeline_editor/ @f_caplette @bsandlin @mgandres +/ee/app/assets/javascripts/ci_minutes_usage/ @pburdette @jivanvl +/ee/app/assets/javascripts/usage_quotas/ci_minutes_usage/ @pburdette @jivanvl +/ee/app/assets/javascripts/usage_quotas/pipelines/ @pburdette @jivanvl +/ee/app/assets/javascripts/reports/ @mfluharty + ^[Templates] /lib/gitlab/ci/templates/ @gitlab-org/maintainers/cicd-templates /lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml @DylanGriffith @mayra-cabrera @tkuah diff --git a/.gitlab/ci/rails.gitlab-ci.yml b/.gitlab/ci/rails.gitlab-ci.yml index 2d92298cd63..9c06c390da0 100644 --- a/.gitlab/ci/rails.gitlab-ci.yml +++ b/.gitlab/ci/rails.gitlab-ci.yml @@ -620,7 +620,7 @@ rspec:feature-flags: stage: post-test needs: - job: "feature-flags-usage" - - job: "haml-lint foss" + - job: "haml-lint" - job: "haml-lint ee" optional: true script: diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml index 3702bec199e..8d0349fa117 100644 --- a/.gitlab/ci/rules.gitlab-ci.yml +++ b/.gitlab/ci/rules.gitlab-ci.yml @@ -264,7 +264,7 @@ - "Dockerfile.assets" - "config/**/*.js" - "vendor/assets/**/*" - - "{app/assets,app/helpers,app/presenters,app/views,locale,public,spec/frontend,symbol}/**/*" + - "{app/assets,app/components,app/helpers,app/presenters,app/views,locale,public,spec/frontend,symbol}/**/*" .controllers-patterns: &controllers-patterns - "{,ee/,jh/}{app/controllers}/**/*" @@ -284,7 +284,7 @@ - "Rakefile" - "config.ru" # List explicitly all the app/ dirs that are backend (i.e. all except app/assets). - - "{,ee/,jh/}{app/channels,app/controllers,app/finders,app/graphql,app/helpers,app/mailers,app/models,app/policies,app/presenters,app/serializers,app/services,app/uploaders,app/validators,app/views,app/workers}/**/*" + - "{,ee/,jh/}{app/channels,app/components,app/controllers,app/finders,app/graphql,app/helpers,app/mailers,app/models,app/policies,app/presenters,app/serializers,app/services,app/uploaders,app/validators,app/views,app/workers}/**/*" - "{,ee/,jh/}{bin,config,db,generator_templates,lib}/**/*" - "{,ee/,jh/}spec/**/*" # CI changes @@ -299,7 +299,7 @@ - "{,jh/}Gemfile.lock" - "GITLAB_ELASTICSEARCH_INDEXER_VERSION" # List explicitly all the app/ dirs that are backend (i.e. all except app/assets). - - "{,ee/,jh/}{app/channels,app/controllers,app/finders,app/graphql,app/helpers,app/mailers,app/models,app/policies,app/presenters,app/serializers,app/services,app/uploaders,app/validators,app/views,app/workers}/**/*" + - "{,ee/,jh/}{app/channels,app/components,app/controllers,app/finders,app/graphql,app/helpers,app/mailers,app/models,app/policies,app/presenters,app/serializers,app/services,app/uploaders,app/validators,app/views,app/workers}/**/*" - "{,ee/,jh/}{bin,config,db,generator_templates,lib}/**/*" - "{,ee/,jh/}spec/**/*" @@ -494,9 +494,14 @@ .static-analysis-patterns: &static-analysis-patterns - ".{codeclimate,eslintrc,haml-lint,haml-lint_todo}.yml" - - ".rubocop.yml" - - ".rubocop_todo.yml" + +.rubocop-patterns: &rubocop-patterns + - ".{rubocop,rubocop_todo}.yml" - ".rubocop_todo/**/*.yml" + - "{,ee/,jh/}rubocop/**/*" # We might be changing custom cops + - "{,ee/,jh/}Gemfile.lock" # This should include gitlab-styles, rubocop itself, and any plugins we might be using + - "lib/gitlab_edition.rb" # This is required in RuboCop::CodeReuseHelpers + - ".gitlab/ci/static-analysis.gitlab-ci.yml" .danger-patterns: &danger-patterns - "Dangerfile" @@ -1427,24 +1432,42 @@ # Static analysis rules # ######################### -.static-analysis:rules:ee-and-foss: +.static-analysis:rules:static-analysis: rules: - changes: *code-backstage-qa-patterns - changes: *static-analysis-patterns -.static-analysis:rules:ee-and-foss-qa: +.static-analysis:rules:static-verification-with-database: + rules: + - changes: *code-backstage-qa-patterns + +.static-analysis:rules:rubocop: + rules: + - changes: *rubocop-patterns + variables: + RUN_ALL_RUBOCOP: "true" + - changes: *code-backstage-qa-patterns + +.static-analysis:rules:qa:metadata-lint: rules: - changes: *qa-patterns - - changes: *static-analysis-patterns + - changes: [".gitlab/ci/static-analysis.gitlab-ci.yml"] -.static-analysis:rules:ee: +.static-analysis:rules:haml-lint: + rules: + - changes: *rubocop-patterns + - changes: *static-analysis-patterns + - changes: *code-backstage-qa-patterns + +.static-analysis:rules:haml-lint-ee: rules: - <<: *if-not-ee when: never - - changes: *code-backstage-qa-patterns + - changes: *rubocop-patterns - changes: *static-analysis-patterns + - changes: *code-backstage-qa-patterns -.static-analysis:rules:as-if-foss: +.static-analysis:rules:static-analysis-as-if-foss: rules: - <<: *if-not-ee when: never @@ -1453,7 +1476,7 @@ - <<: *if-security-merge-request changes: *code-backstage-qa-patterns - <<: *if-merge-request - changes: *ci-patterns + changes: [".gitlab/ci/static-analysis.gitlab-ci.yml"] - <<: *if-merge-request changes: *static-analysis-patterns diff --git a/.gitlab/ci/static-analysis.gitlab-ci.yml b/.gitlab/ci/static-analysis.gitlab-ci.yml index b4efd9e49bf..a9e806f12a3 100644 --- a/.gitlab/ci/static-analysis.gitlab-ci.yml +++ b/.gitlab/ci/static-analysis.gitlab-ci.yml @@ -25,7 +25,7 @@ static-analysis: extends: - .static-analysis-base - .static-analysis-cache - - .static-analysis:rules:ee-and-foss + - .static-analysis:rules:static-analysis parallel: 2 script: - run_timed_command "retry yarn install --frozen-lockfile" @@ -34,14 +34,14 @@ static-analysis: static-analysis as-if-foss: extends: - static-analysis - - .static-analysis:rules:as-if-foss + - .static-analysis:rules:static-analysis-as-if-foss - .as-if-foss static-verification-with-database: extends: - .static-analysis-base - .rubocop-job-cache - - .static-analysis:rules:ee-and-foss + - .static-analysis:rules:static-verification-with-database - .use-pg12 script: - bundle exec rake lint:static_verification_with_database @@ -91,11 +91,11 @@ eslint as-if-foss: - .as-if-foss needs: ['generate-apollo-graphql-schema as-if-foss'] -haml-lint foss: +haml-lint: extends: - .static-analysis-base - .ruby-cache - - .static-analysis:rules:ee-and-foss + - .static-analysis:rules:haml-lint script: - run_timed_command "bin/rake 'haml_lint[app/views]'" artifacts: @@ -106,8 +106,8 @@ haml-lint foss: haml-lint ee: extends: - - "haml-lint foss" - - .static-analysis:rules:ee + - "haml-lint" + - .static-analysis:rules:haml-lint-ee script: - run_timed_command "bin/rake 'haml_lint[ee/app/views]'" @@ -115,14 +115,21 @@ rubocop: extends: - .static-analysis-base - .rubocop-job-cache - - .static-analysis:rules:ee-and-foss + - .static-analysis:rules:rubocop + needs: ["detect-tests"] script: - - run_timed_command "bundle exec rubocop --parallel" + - | + # For non-merge request, or when RUN_ALL_RUBOCOP is 'true', run all RuboCop rules + if [ -z "${CI_MERGE_REQUEST_IID}" ] || [ "${RUN_ALL_RUBOCOP}" == "true" ]; then + run_timed_command "bundle exec rubocop --parallel" + else + run_timed_command "bundle exec rubocop --parallel --force-exclusion $(cat tmp/changed_files.txt)" + fi qa:metadata-lint: extends: - .static-analysis-base - - .static-analysis:rules:ee-and-foss-qa + - .static-analysis:rules:qa:metadata-lint before_script: - !reference [.default-before_script, before_script] - cd qa/ @@ -149,7 +156,7 @@ feature-flags-usage: extends: - .static-analysis-base - .rubocop-job-cache - - .static-analysis:rules:ee-and-foss + - .static-analysis:rules:rubocop script: # We need to disable the cache for this cop since it creates files under tmp/feature_flags/*.used, # the cache would prevent these files from being created. diff --git a/.rubocop_todo/layout/space_inside_block_braces.yml b/.rubocop_todo/layout/space_inside_block_braces.yml deleted file mode 100644 index 25fc6dba4d9..00000000000 --- a/.rubocop_todo/layout/space_inside_block_braces.yml +++ /dev/null @@ -1,74 +0,0 @@ ---- -# Cop supports --auto-correct. -Layout/SpaceInsideBlockBraces: - Exclude: - - 'spec/config/settings_spec.rb' - - 'spec/controllers/admin/application_settings_controller_spec.rb' - - 'spec/controllers/application_controller_spec.rb' - - 'spec/controllers/groups/labels_controller_spec.rb' - - 'spec/controllers/groups/releases_controller_spec.rb' - - 'spec/controllers/import/manifest_controller_spec.rb' - - 'spec/controllers/projects/blame_controller_spec.rb' - - 'spec/controllers/projects/deploy_keys_controller_spec.rb' - - 'spec/controllers/projects/feature_flags_controller_spec.rb' - - 'spec/controllers/projects/jobs_controller_spec.rb' - - 'spec/controllers/projects/labels_controller_spec.rb' - - 'spec/controllers/projects/notes_controller_spec.rb' - - 'spec/controllers/projects/releases_controller_spec.rb' - - 'spec/controllers/projects/tree_controller_spec.rb' - - 'spec/controllers/registrations/welcome_controller_spec.rb' - - 'spec/controllers/snippets/notes_controller_spec.rb' - - 'spec/dependencies/omniauth_saml_spec.rb' - - 'spec/experiments/application_experiment_spec.rb' - - 'spec/finders/ci/jobs_finder_spec.rb' - - 'spec/finders/ci/runners_finder_spec.rb' - - 'spec/finders/concerns/packages/finder_helper_spec.rb' - - 'spec/finders/container_repositories_finder_spec.rb' - - 'spec/finders/design_management/versions_finder_spec.rb' - - 'spec/finders/milestones_finder_spec.rb' - - 'spec/finders/packages/group_packages_finder_spec.rb' - - 'spec/finders/packages/npm/package_finder_spec.rb' - - 'spec/finders/projects_finder_spec.rb' - - 'spec/helpers/application_settings_helper_spec.rb' - - 'spec/helpers/blob_helper_spec.rb' - - 'spec/helpers/gitlab_script_tag_helper_spec.rb' - - 'spec/helpers/issuables_helper_spec.rb' - - 'spec/helpers/projects/pipeline_helper_spec.rb' - - 'spec/helpers/routing/pseudonymization_helper_spec.rb' - - 'spec/helpers/search_helper_spec.rb' - - 'spec/helpers/wiki_page_version_helper_spec.rb' - - 'spec/initializers/carrierwave_patch_spec.rb' - - 'spec/initializers/trusted_proxies_spec.rb' - - 'spec/mailers/emails/service_desk_spec.rb' - - 'spec/migrations/20210812013042_remove_duplicate_project_authorizations_spec.rb' - - 'spec/migrations/20210910194952_update_report_type_for_existing_approval_project_rules_spec.rb' - - 'spec/migrations/confirm_support_bot_user_spec.rb' - - 'spec/migrations/reset_job_token_scope_enabled_again_spec.rb' - - 'spec/migrations/reset_job_token_scope_enabled_spec.rb' - - 'spec/migrations/reset_severity_levels_to_new_default_spec.rb' - - 'spec/policies/clusters/agent_policy_spec.rb' - - 'spec/policies/group_member_policy_spec.rb' - - 'spec/policies/issue_policy_spec.rb' - - 'spec/policies/project_policy_spec.rb' - - 'spec/policies/terraform/state_policy_spec.rb' - - 'spec/policies/terraform/state_version_policy_spec.rb' - - 'spec/presenters/packages/composer/packages_presenter_spec.rb' - - 'spec/presenters/packages/conan/package_presenter_spec.rb' - - 'spec/presenters/packages/nuget/packages_metadata_presenter_spec.rb' - - 'spec/presenters/project_presenter_spec.rb' - - 'spec/rubocop/cop/migration/create_table_with_foreign_keys_spec.rb' - - 'spec/serializers/cluster_entity_spec.rb' - - 'spec/serializers/import/provider_repo_serializer_spec.rb' - - 'spec/tasks/gitlab/snippets_rake_spec.rb' - - 'spec/uploaders/packages/debian/distribution_release_file_uploader_spec.rb' - - 'spec/validators/addressable_url_validator_spec.rb' - - 'spec/views/help/instance_configuration.html.haml_spec.rb' - - 'spec/views/layouts/_header_search.html.haml_spec.rb' - - 'spec/views/layouts/_published_experiments.html.haml_spec.rb' - - 'spec/views/shared/runners/_runner_details.html.haml_spec.rb' - - 'spec/workers/bulk_imports/export_request_worker_spec.rb' - - 'spec/workers/clusters/cleanup/project_namespace_worker_spec.rb' - - 'spec/workers/packages/helm/extraction_worker_spec.rb' - - 'spec/workers/pages_worker_spec.rb' - - 'spec/workers/purge_dependency_proxy_cache_worker_spec.rb' - - 'spec/workers/releases/manage_evidence_worker_spec.rb' diff --git a/app/assets/javascripts/access_tokens/components/constants.js b/app/assets/javascripts/access_tokens/components/constants.js index 84e50bc099f..9cd7cb5bb3a 100644 --- a/app/assets/javascripts/access_tokens/components/constants.js +++ b/app/assets/javascripts/access_tokens/components/constants.js @@ -12,8 +12,6 @@ export const FIELDS = [ key: 'name', label: __('Token name'), sortable: true, - tdClass: `gl-text-black-normal`, - thClass: `gl-text-black-normal`, }, { formatter(scopes) { @@ -22,40 +20,30 @@ export const FIELDS = [ key: 'scopes', label: __('Scopes'), sortable: true, - tdClass: `gl-text-black-normal`, - thClass: `gl-text-black-normal`, }, { key: 'createdAt', label: s__('AccessTokens|Created'), sortable: true, - tdClass: `gl-text-black-normal`, - thClass: `gl-text-black-normal`, }, { key: 'lastUsedAt', label: __('Last Used'), sortable: true, - tdClass: `gl-text-black-normal`, - thClass: `gl-text-black-normal`, }, { key: 'expiresAt', label: __('Expires'), sortable: true, - tdClass: `gl-text-black-normal`, - thClass: `gl-text-black-normal`, }, { key: 'role', label: __('Role'), - tdClass: `gl-text-black-normal`, - thClass: `gl-text-black-normal`, sortable: true, }, { key: 'action', label: __('Action'), - thClass: `gl-text-black-normal`, + tdClass: 'gl-py-3!', }, ]; diff --git a/app/assets/javascripts/content_editor/components/content_editor.vue b/app/assets/javascripts/content_editor/components/content_editor.vue index c24186ae406..05d6f468d23 100644 --- a/app/assets/javascripts/content_editor/components/content_editor.vue +++ b/app/assets/javascripts/content_editor/components/content_editor.vue @@ -105,7 +105,11 @@ export default { - + diff --git a/app/assets/javascripts/security_configuration/components/constants.js b/app/assets/javascripts/security_configuration/components/constants.js index 5a9ef832e05..77216408c39 100644 --- a/app/assets/javascripts/security_configuration/components/constants.js +++ b/app/assets/javascripts/security_configuration/components/constants.js @@ -157,7 +157,7 @@ export const SCANNER_NAMES_MAP = { COVERAGE_FUZZING: COVERAGE_FUZZING_NAME, SECRET_DETECTION: SECRET_DETECTION_NAME, DEPENDENCY_SCANNING: DEPENDENCY_SCANNING_NAME, - GENERIC: s__('ciReport|Manually Added'), + GENERIC: s__('ciReport|Manually added'), }; export const securityFeatures = [ diff --git a/app/assets/javascripts/vue_shared/components/filtered_search_bar/constants.js b/app/assets/javascripts/vue_shared/components/filtered_search_bar/constants.js index 5d7f4ae2a01..ffe09634a3b 100644 --- a/app/assets/javascripts/vue_shared/components/filtered_search_bar/constants.js +++ b/app/assets/javascripts/vue_shared/components/filtered_search_bar/constants.js @@ -46,7 +46,7 @@ export const SortDirection = { export const FILTERED_SEARCH_LABELS = 'labels'; export const FILTERED_SEARCH_TERM = 'filtered-search-term'; -export const TOKEN_TITLE_ASSIGNEE = __('Assignee'); +export const TOKEN_TITLE_ASSIGNEE = s__('SearchToken|Assignee'); export const TOKEN_TITLE_AUTHOR = __('Author'); export const TOKEN_TITLE_CONFIDENTIAL = __('Confidential'); export const TOKEN_TITLE_CONTACT = s__('Crm|Contact'); diff --git a/app/assets/stylesheets/framework/contextual_sidebar.scss b/app/assets/stylesheets/framework/contextual_sidebar.scss index ad0036df607..34c7ffa58fe 100644 --- a/app/assets/stylesheets/framework/contextual_sidebar.scss +++ b/app/assets/stylesheets/framework/contextual_sidebar.scss @@ -149,7 +149,7 @@ margin: $sidebar-top-item-tb-margin $sidebar-top-item-lr-margin; &:hover { - background-color: $indigo-900-alpha-008; + background-color: $nav-active-bg; } } @@ -275,7 +275,7 @@ &:not(.fly-out-top-item) { > a:not(.has-sub-items) { - background-color: $indigo-900-alpha-008; + background-color: $nav-active-bg; } } } diff --git a/app/assets/stylesheets/framework/header.scss b/app/assets/stylesheets/framework/header.scss index ffefc5fead5..48223bd5416 100644 --- a/app/assets/stylesheets/framework/header.scss +++ b/app/assets/stylesheets/framework/header.scss @@ -560,7 +560,7 @@ } .frequent-items-list-item-container > a:hover { - background-color: $nav-active-bg; + background-color: $nav-active-bg !important; } } @@ -577,7 +577,7 @@ .top-nav-menu-item { &.active, &:hover { - background-color: $nav-active-bg; + background-color: $nav-active-bg !important; } .gl-icon { diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index e9ad930ef2b..bd32a817d5d 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -364,7 +364,7 @@ $well-expand-item: #e8f2f7 !default; $well-inner-border: #eef0f2 !default; $well-light-border: #f1f1f1; $well-light-text-color: #5b6169; -$nav-active-bg: var(--nav-active-bg, rgba($black, 0.08)) !important; +$nav-active-bg: rgba($black, 0.08); /* * Text diff --git a/app/assets/stylesheets/startup/startup-dark.scss b/app/assets/stylesheets/startup/startup-dark.scss index d4304abe241..71c99883297 100644 --- a/app/assets/stylesheets/startup/startup-dark.scss +++ b/app/assets/stylesheets/startup/startup-dark.scss @@ -18,7 +18,6 @@ body.gl-dark { --gl-text-color: #fafafa; --border-color: #4f4f4f; --black: #fff; - --nav-active-bg: rgba(255, 255, 255, 0.08); } :root { --white: #333; @@ -1128,7 +1127,7 @@ kbd { font-weight: 600; } .nav-sidebar li.active:not(.fly-out-top-item) > a:not(.has-sub-items) { - background-color: rgba(41, 41, 97, 0.08); + background-color: rgba(255, 255, 255, 0.08); } .nav-sidebar ul { padding-left: 0; @@ -1781,7 +1780,6 @@ body.gl-dark { --white: #333; --black: #fff; --svg-status-bg: #333; - --nav-active-bg: rgba(255, 255, 255, 0.08); } .nav-sidebar, .toggle-sidebar-button, @@ -1796,12 +1794,6 @@ body.gl-dark { .nav-sidebar li a { color: var(--gray-600); } -.nav-sidebar li.active { - box-shadow: none; -} -.nav-sidebar li.active:not(.fly-out-top-item) > a:not(.has-sub-items) { - background-color: var(--nav-active-bg); -} body.gl-dark { --gl-theme-accent: #868686; } @@ -2029,7 +2021,6 @@ body.gl-dark { --white: #333; --black: #fff; --svg-status-bg: #333; - --nav-active-bg: rgba(255, 255, 255, 0.08); } .tab-width-8 { tab-size: 8; diff --git a/app/assets/stylesheets/startup/startup-general.scss b/app/assets/stylesheets/startup/startup-general.scss index d08ea63ffed..d517fcf58ed 100644 --- a/app/assets/stylesheets/startup/startup-general.scss +++ b/app/assets/stylesheets/startup/startup-general.scss @@ -1107,7 +1107,7 @@ kbd { font-weight: 600; } .nav-sidebar li.active:not(.fly-out-top-item) > a:not(.has-sub-items) { - background-color: rgba(41, 41, 97, 0.08); + background-color: rgba(0, 0, 0, 0.08); } .nav-sidebar ul { padding-left: 0; diff --git a/app/assets/stylesheets/themes/_dark.scss b/app/assets/stylesheets/themes/_dark.scss index eeb4604f32a..4b74e449e06 100644 --- a/app/assets/stylesheets/themes/_dark.scss +++ b/app/assets/stylesheets/themes/_dark.scss @@ -101,7 +101,6 @@ $white-dark: #444; $theme-indigo-50: #1a1a40; $border-color: #4f4f4f; -$nav-active-bg: rgba(255, 255, 255, 0.08); :root { color-scheme: dark; @@ -206,7 +205,6 @@ body.gl-dark { --black: #{$black}; --svg-status-bg: #{$white}; - --nav-active-bg: #{$nav-active-bg}; .gl-button.gl-button, .gl-button.gl-button.btn-block { diff --git a/app/assets/stylesheets/themes/dark_mode_overrides.scss b/app/assets/stylesheets/themes/dark_mode_overrides.scss index 92740aaf89e..9348216c2ac 100644 --- a/app/assets/stylesheets/themes/dark_mode_overrides.scss +++ b/app/assets/stylesheets/themes/dark_mode_overrides.scss @@ -64,20 +64,6 @@ a { color: var(--gray-600); } - - > a:hover { - background-color: var(--nav-active-bg); - } - - &.active { - box-shadow: none; - - &:not(.fly-out-top-item) { - > a:not(.has-sub-items) { - background-color: var(--nav-active-bg); - } - } - } } .sidebar-sub-level-items.fly-out-list { diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 621d198d341..bf7eed9543d 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -473,7 +473,7 @@ module Ci end def uses_needs? - builds.where(scheduling_type: :dag).any? + processables.where(scheduling_type: :dag).any? end def stages_count diff --git a/config/feature_flags/development/ci_forked_source_public_cost_factor.yml b/config/feature_flags/development/ci_forked_source_public_cost_factor.yml deleted file mode 100644 index 6e5a6c1e1f1..00000000000 --- a/config/feature_flags/development/ci_forked_source_public_cost_factor.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: ci_forked_source_public_cost_factor -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/94870 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/370147 -milestone: '15.4' -type: development -group: group::pipeline execution -default_enabled: false diff --git a/config/metrics/counts_28d/20210216184850_deploy_token_packages_total_unique_counts_monthly.yml b/config/metrics/counts_28d/20210216184850_deploy_token_packages_total_unique_counts_monthly.yml index 720165425fa..6b6c40495a6 100644 --- a/config/metrics/counts_28d/20210216184850_deploy_token_packages_total_unique_counts_monthly.yml +++ b/config/metrics/counts_28d/20210216184850_deploy_token_packages_total_unique_counts_monthly.yml @@ -16,17 +16,13 @@ options: events: - i_package_composer_deploy_token - i_package_conan_deploy_token - - i_package_container_deploy_token - - i_package_debian_deploy_token - i_package_generic_deploy_token - - i_package_golang_deploy_token - i_package_helm_deploy_token - i_package_maven_deploy_token - i_package_npm_deploy_token - i_package_nuget_deploy_token - i_package_pypi_deploy_token - i_package_rubygems_deploy_token - - i_package_tag_deploy_token - i_package_terraform_module_deploy_token distribution: - ee diff --git a/config/metrics/counts_7d/20210216184848_deploy_token_packages_total_unique_counts_weekly.yml b/config/metrics/counts_7d/20210216184848_deploy_token_packages_total_unique_counts_weekly.yml index 671b8a1785a..28adf6d6d01 100644 --- a/config/metrics/counts_7d/20210216184848_deploy_token_packages_total_unique_counts_weekly.yml +++ b/config/metrics/counts_7d/20210216184848_deploy_token_packages_total_unique_counts_weekly.yml @@ -15,17 +15,13 @@ options: events: - i_package_composer_deploy_token - i_package_conan_deploy_token - - i_package_container_deploy_token - - i_package_debian_deploy_token - i_package_generic_deploy_token - - i_package_golang_deploy_token - i_package_helm_deploy_token - i_package_maven_deploy_token - i_package_npm_deploy_token - i_package_nuget_deploy_token - i_package_pypi_deploy_token - i_package_rubygems_deploy_token - - i_package_tag_deploy_token - i_package_terraform_module_deploy_token distribution: - ee diff --git a/doc/ci/pipelines/cicd_minutes.md b/doc/ci/pipelines/cicd_minutes.md index 4b7d3845361..7487bd3190c 100644 --- a/doc/ci/pipelines/cicd_minutes.md +++ b/doc/ci/pipelines/cicd_minutes.md @@ -201,9 +201,12 @@ can be higher than the end-to-end duration of a pipeline. The cost factors for jobs running on shared runners on GitLab.com are: -- `0.008` for public projects, and projects in the [GitLab for Open Source program](../../subscriptions/index.md#gitlab-for-open-source). - For every 125 minutes of job execution time, you use 1 CI/CD minute. - `1` for internal and private projects. +- `0.5` for public projects in the [GitLab for Open Source program](../../subscriptions/index.md#gitlab-for-open-source). +- `0.008` for public forks of public projects. For every 125 minutes of job execution time, + you use 1 CI/CD minute. +- `0.04` for other public projects, after September 1, 2022 (previously `0.008`). + For every 25 minutes of job execution time, you use 1 CI/CD minute. - Calculated differently for [community contributions to GitLab projects](#cost-factor-for-community-contributions-to-gitlab-projects). The cost factors on self-managed instances are: diff --git a/doc/development/merge_request_concepts/index.md b/doc/development/merge_request_concepts/index.md index 331f0e01579..d463f6ba290 100644 --- a/doc/development/merge_request_concepts/index.md +++ b/doc/development/merge_request_concepts/index.md @@ -10,7 +10,7 @@ info: "See the Technical Writers assigned to Development Guidelines: https://abo NOTE: The documentation below is the single source of truth for the merge request terminology and functionality. -The merge request is made up of several different key components and ideas that encompass the overall merge request experience. These concepts sometimes have competing and confusing terminology or overlap with other concepts. The concepts this will cover are: +The merge request is made up of several different key components and ideas that encompass the overall merge request experience. These concepts sometimes have competing and confusing terminology or overlap with other concepts. This page covers the following concepts: 1. Merge widget 1. Report widgets @@ -40,15 +40,17 @@ Reports are widgets within the merge request that report information about chang ## Merge checks -Merge checks are statuses that can either pass or fail and conditionally control the availability of the merge button being available within a merge request. The key distinguishing factor in a merge check is that users **do not** interact with the merge checks inside of the merge request, but are able to influence whether or not the check passes or fails. Results from the check are processed as true/false to determine whether or not a merge request can be merged. Examples include: +Merge checks are statuses that can either pass or fail and conditionally control the availability of the merge button being available within a merge request. The key distinguishing factor in a merge check is that users **do not** interact with the merge checks inside of the merge request, but are able to influence whether or not the check passes or fails. Results from the check are processed as true/false to determine whether or not a merge request can be merged. -- Merge conflicts. -- Pipeline success. -- Threads resolution. -- [External status checks](../../user/project/merge_requests/status_checks.md). -- Required approvals. +Examples of merge checks include: -When all of the required merge checks are satisfied a merge request becomes mergeable. +- Merge conflicts +- Pipeline success +- Threads resolution +- [External status checks](../../user/project/merge_requests/status_checks.md) +- Required approvals + +A merge request can be merged only when all of the required merge checks are satisfied. ## Approvals @@ -58,8 +60,8 @@ Additionally, approval settings provide configuration options to define how thos Examples of approval rules and settings include: -1. [merge request approval rules](../../user/project/merge_requests/approvals/rules.md) -1. [code owner approvals](../../user/project/code_owners.md) -1. [security approvals](../../user/application_security/index.md#security-approvals-in-merge-requests) -1. [prevent editing approval rules](../../user/project/merge_requests/approvals/settings.md#prevent-editing-approval-rules-in-merge-requests) -1. [remove all approvals when commits are added](../../user/project/merge_requests/approvals/settings.md#remove-all-approvals-when-commits-are-added-to-the-source-branch) +- [Merge request approval rules](../../user/project/merge_requests/approvals/rules.md) +- [Code owner approvals](../../user/project/code_owners.md) +- [Security approvals](../../user/application_security/index.md#security-approvals-in-merge-requests) +- [Prevent editing approval rules](../../user/project/merge_requests/approvals/settings.md#prevent-editing-approval-rules-in-merge-requests) +- [Remove all approvals when commits are added](../../user/project/merge_requests/approvals/settings.md#remove-all-approvals-when-commits-are-added-to-the-source-branch) diff --git a/doc/development/service_ping/implement.md b/doc/development/service_ping/implement.md index 0ebc58dd669..5ddf4e8fa73 100644 --- a/doc/development/service_ping/implement.md +++ b/doc/development/service_ping/implement.md @@ -127,7 +127,7 @@ Examples using `usage_data.rb` have been [deprecated](usage_data.md). We recomme #### Grouping and batch operations -The `count`, `distinct_count`, `sum`, and `average` batch counters can accept an `ActiveRecord::Relation` +The `count`, `distinct_count` and `sum` batch counters can accept an `ActiveRecord::Relation` object, which groups by a specified column. With a grouped relation, the methods do batch counting, handle errors, and returns a hash table of key-value pairs. @@ -142,9 +142,6 @@ distinct_count(Project.group(:visibility_level), :creator_id) sum(Issue.group(:state_id), :weight)) # returns => {1=>3542, 2=>6820} - -average(Issue.group(:state_id), :weight)) -# returns => {1=>3.5, 2=>2.5} ``` #### Add operation diff --git a/doc/development/service_ping/metrics_instrumentation.md b/doc/development/service_ping/metrics_instrumentation.md index bde838df940..ee0d701a5bb 100644 --- a/doc/development/service_ping/metrics_instrumentation.md +++ b/doc/development/service_ping/metrics_instrumentation.md @@ -337,7 +337,7 @@ To create a stub instrumentation for a Service Ping metric, you can use a dedica The generator takes the class name as an argument and the following options: - `--type=TYPE` Required. Indicates the metric type. It must be one of: `database`, `generic`, `redis`, `numbers`. -- `--operation` Required for `database` & `numebers` type. +- `--operation` Required for `database` & `numbers` type. - For `database` it must be one of: `count`, `distinct_count`, `estimate_batch_distinct_count`, `sum`, `average`. - For `numbers` it must be: `add`. - `--ee` Indicates if the metric is for EE. diff --git a/doc/user/project/merge_requests/reviews/img/mr_summary_comment_v15_3.png b/doc/user/project/merge_requests/reviews/img/mr_summary_comment_v15_3.png deleted file mode 100644 index 38e18115803..00000000000 Binary files a/doc/user/project/merge_requests/reviews/img/mr_summary_comment_v15_3.png and /dev/null differ diff --git a/doc/user/project/merge_requests/reviews/img/mr_summary_comment_v15_4.png b/doc/user/project/merge_requests/reviews/img/mr_summary_comment_v15_4.png new file mode 100644 index 00000000000..47b7be3886d Binary files /dev/null and b/doc/user/project/merge_requests/reviews/img/mr_summary_comment_v15_4.png differ diff --git a/doc/user/project/merge_requests/reviews/index.md b/doc/user/project/merge_requests/reviews/index.md index 27b223c48ec..5a788f595ea 100644 --- a/doc/user/project/merge_requests/reviews/index.md +++ b/doc/user/project/merge_requests/reviews/index.md @@ -56,12 +56,11 @@ displays next to your name. You can submit your completed review in multiple ways: - Use the `/submit_review` [quick action](../../quick_actions.md) in the text of a non-review comment. -- Select **Finish review** and then **Submit review** in the footer at the bottom of the screen. +- Select **Finish review**, then select **Submit review** at the bottom of the modal window. + In the modal window, you can supply a **Summary comment**, approve the merge request, and + include quick actions: -Selecting **Finish review** opens a modal window to add an optional comment to summarize your review. -You can also include quick actions: - -![Finish review with comment](img/mr_summary_comment_v15_3.png) + ![Finish review with comment](img/mr_summary_comment_v15_4.png) When you submit your review, GitLab: @@ -69,6 +68,7 @@ When you submit your review, GitLab: - Sends a single email to every notifiable user of the merge request, with your review comments attached. Replying to this email creates a new comment on the merge request. - Perform any quick actions you added to your review comments. +- Optional. Approves the merge request. ### Resolve or unresolve thread with a comment diff --git a/lib/gitlab/background_migration/backfill_vulnerability_reads_cluster_agent.rb b/lib/gitlab/background_migration/backfill_vulnerability_reads_cluster_agent.rb index 728b60f7a0e..0c41d6af209 100644 --- a/lib/gitlab/background_migration/backfill_vulnerability_reads_cluster_agent.rb +++ b/lib/gitlab/background_migration/backfill_vulnerability_reads_cluster_agent.rb @@ -10,16 +10,12 @@ module Gitlab vulnerability_reads.project_id = cluster_agents.project_id SQL - RELATION = ->(relation) do - relation - .where(report_type: 7) - end + CLUSTER_IMAGE_SCANNING_REPORT_TYPE = 7 + + scope_to ->(relation) { relation.where(report_type: CLUSTER_IMAGE_SCANNING_REPORT_TYPE) } def perform - each_sub_batch( - operation_name: :update_all, - batching_scope: RELATION - ) do |sub_batch| + each_sub_batch(operation_name: :update_all) do |sub_batch| sub_batch .joins(CLUSTER_AGENTS_JOIN) .update_all('casted_cluster_agent_id = CAST(vulnerability_reads.cluster_agent_id AS bigint)') diff --git a/lib/gitlab/background_migration/batching_strategies/backfill_project_statistics_with_container_registry_size_batching_strategy.rb b/lib/gitlab/background_migration/batching_strategies/backfill_project_statistics_with_container_registry_size_batching_strategy.rb index 9ad119310f7..72da2b5a2b7 100644 --- a/lib/gitlab/background_migration/batching_strategies/backfill_project_statistics_with_container_registry_size_batching_strategy.rb +++ b/lib/gitlab/background_migration/batching_strategies/backfill_project_statistics_with_container_registry_size_batching_strategy.rb @@ -3,18 +3,9 @@ module Gitlab module BackgroundMigration module BatchingStrategies - # Batching class to use for back-filling project_statistic's container_registry_size. - # Batches will be scoped to records where the project_ids are migrated - # - # If no more batches exist in the table, returns nil. + # Used to apply additional filters to the batching table, migrated to + # use BatchedMigrationJob#filter_batch with https://gitlab.com/gitlab-org/gitlab/-/merge_requests/93771 class BackfillProjectStatisticsWithContainerRegistrySizeBatchingStrategy < PrimaryKeyBatchingStrategy - MIGRATION_PHASE_1_ENDED_AT = Date.new(2022, 01, 23).freeze - - def apply_additional_filters(relation, job_arguments: [], job_class: nil) - relation.where(created_at: MIGRATION_PHASE_1_ENDED_AT..).or( - relation.where(migration_state: 'import_done') - ).select(:project_id).distinct - end end end end diff --git a/lib/gitlab/background_migration/batching_strategies/backfill_vulnerability_reads_cluster_agent_batching_strategy.rb b/lib/gitlab/background_migration/batching_strategies/backfill_vulnerability_reads_cluster_agent_batching_strategy.rb index f0d015198dc..c2fa00f66de 100644 --- a/lib/gitlab/background_migration/batching_strategies/backfill_vulnerability_reads_cluster_agent_batching_strategy.rb +++ b/lib/gitlab/background_migration/batching_strategies/backfill_vulnerability_reads_cluster_agent_batching_strategy.rb @@ -3,16 +3,9 @@ module Gitlab module BackgroundMigration module BatchingStrategies - # Batching class to use for back-filling vulnerability_read's casted_cluster_agent_id from cluster_agent_id. - # Batches will be scoped to records where the report_type belongs to cluster_image_scanning. - # - # If no more batches exist in the table, returns nil. + # Used to apply additional filters to the batching table, migrated to + # use BatchedMigrationJob#filter_batch with https://gitlab.com/gitlab-org/gitlab/-/merge_requests/93771 class BackfillVulnerabilityReadsClusterAgentBatchingStrategy < PrimaryKeyBatchingStrategy - CLUSTER_IMAGE_SCANNING_REPORT_TYPE = 7 - - def apply_additional_filters(relation, job_arguments: [], job_class: nil) - relation.where(report_type: CLUSTER_IMAGE_SCANNING_REPORT_TYPE) - end end end end diff --git a/lib/gitlab/background_migration/batching_strategies/dismissed_vulnerabilities_strategy.rb b/lib/gitlab/background_migration/batching_strategies/dismissed_vulnerabilities_strategy.rb index e1855b6cfee..9504d4eec11 100644 --- a/lib/gitlab/background_migration/batching_strategies/dismissed_vulnerabilities_strategy.rb +++ b/lib/gitlab/background_migration/batching_strategies/dismissed_vulnerabilities_strategy.rb @@ -3,14 +3,9 @@ module Gitlab module BackgroundMigration module BatchingStrategies - # Batching class to use for setting state in vulnerabilitites table. - # Batches will be scoped to records where the dismissed_at is set. - # - # If no more batches exist in the table, returns nil. + # Used to apply additional filters to the batching table, migrated to + # use BatchedMigrationJob#filter_batch with https://gitlab.com/gitlab-org/gitlab/-/merge_requests/93771 class DismissedVulnerabilitiesStrategy < PrimaryKeyBatchingStrategy - def apply_additional_filters(relation, job_arguments: [], job_class: nil) - relation.where.not(dismissed_at: nil) - end end end end diff --git a/lib/gitlab/background_migration/set_correct_vulnerability_state.rb b/lib/gitlab/background_migration/set_correct_vulnerability_state.rb index fd6cbcb8d05..a0cfeed618a 100644 --- a/lib/gitlab/background_migration/set_correct_vulnerability_state.rb +++ b/lib/gitlab/background_migration/set_correct_vulnerability_state.rb @@ -6,11 +6,10 @@ module Gitlab class SetCorrectVulnerabilityState < BatchedMigrationJob DISMISSED_STATE = 2 + scope_to ->(relation) { relation.where.not(dismissed_at: nil) } + def perform - each_sub_batch( - operation_name: :update_vulnerabilities_state, - batching_scope: -> (relation) { relation.where.not(dismissed_at: nil) } - ) do |sub_batch| + each_sub_batch(operation_name: :update_vulnerabilities_state) do |sub_batch| sub_batch.update_all(state: DISMISSED_STATE) end end diff --git a/lib/gitlab/database/batch_average_counter.rb b/lib/gitlab/database/batch_average_counter.rb new file mode 100644 index 00000000000..9cb1e34ab67 --- /dev/null +++ b/lib/gitlab/database/batch_average_counter.rb @@ -0,0 +1,103 @@ +# frozen_string_literal: true + +module Gitlab + module Database + class BatchAverageCounter + COLUMN_FALLBACK = 0 + DEFAULT_BATCH_SIZE = 1_000 + FALLBACK = -1 + MAX_ALLOWED_LOOPS = 10_000 + OFFSET_BY_ONE = 1 + SLEEP_TIME_IN_SECONDS = 0.01 # 10 msec sleep + + attr_reader :relation, :column + + def initialize(relation, column) + @relation = relation + @column = wrap_column(relation, column) + end + + def count(batch_size: nil) + raise 'BatchAverageCounter can not be run inside a transaction' if transaction_open? + + batch_size = batch_size.presence || DEFAULT_BATCH_SIZE + + start = column_start + finish = column_finish + + total_sum = 0 + total_records = 0 + + batch_start = start + + while batch_start < finish + begin + batch_end = [batch_start + batch_size, finish].min + batch_relation = build_relation_batch(batch_start, batch_end) + + # We use `sum` and `count` instead of `average` here to not run into an "average of averages" + # problem as batches will have different sizes, so we are essentially summing up the values for + # each batch separately, and then dividing that result on the total number of records. + batch_sum, batch_count = batch_relation.pick(column.sum, column.count) + + total_sum += batch_sum.to_i + total_records += batch_count + + batch_start = batch_end + rescue ActiveRecord::QueryCanceled => error # rubocop:disable Database/RescueQueryCanceled + # retry with a safe batch size & warmer cache + if batch_size >= 2 * DEFAULT_BATCH_SIZE + batch_size /= 2 + else + log_canceled_batch_fetch(batch_start, batch_relation.to_sql, error) + + return FALLBACK + end + end + + sleep(SLEEP_TIME_IN_SECONDS) + end + + return FALLBACK if total_records == 0 + + total_sum.to_f / total_records + end + + private + + def column_start + relation.unscope(:group, :having).minimum(column) || COLUMN_FALLBACK + end + + def column_finish + (relation.unscope(:group, :having).maximum(column) || COLUMN_FALLBACK) + OFFSET_BY_ONE + end + + def build_relation_batch(start, finish) + relation.where(column.between(start...finish)) + end + + def log_canceled_batch_fetch(batch_start, query, error) + Gitlab::AppJsonLogger + .error( + event: 'batch_count', + relation: relation.table_name, + operation: 'average', + start: batch_start, + query: query, + message: "Query has been canceled with message: #{error.message}" + ) + end + + def transaction_open? + relation.connection.transaction_open? + end + + def wrap_column(relation, column) + return column if column.is_a?(Arel::Attributes::Attribute) + + relation.arel_table[column] + end + end + end +end diff --git a/lib/gitlab/database/batch_count.rb b/lib/gitlab/database/batch_count.rb index b33f39e3cbe..7a064fb4005 100644 --- a/lib/gitlab/database/batch_count.rb +++ b/lib/gitlab/database/batch_count.rb @@ -48,7 +48,7 @@ module Gitlab end def batch_average(relation, column, batch_size: nil, start: nil, finish: nil) - BatchCounter.new(relation, column: nil, operation: :average, operation_args: [column]).count(batch_size: batch_size, start: start, finish: finish) + BatchAverageCounter.new(relation, column).count(batch_size: batch_size) end class << self diff --git a/lib/gitlab/database/batch_counter.rb b/lib/gitlab/database/batch_counter.rb index aa4abf15755..abb62140503 100644 --- a/lib/gitlab/database/batch_counter.rb +++ b/lib/gitlab/database/batch_counter.rb @@ -6,7 +6,6 @@ module Gitlab FALLBACK = -1 MIN_REQUIRED_BATCH_SIZE = 1_250 DEFAULT_SUM_BATCH_SIZE = 1_000 - DEFAULT_AVERAGE_BATCH_SIZE = 1_000 MAX_ALLOWED_LOOPS = 10_000 SLEEP_TIME_IN_SECONDS = 0.01 # 10 msec sleep ALLOWED_MODES = [:itself, :distinct].freeze @@ -27,7 +26,6 @@ module Gitlab def unwanted_configuration?(finish, batch_size, start) (@operation == :count && batch_size <= MIN_REQUIRED_BATCH_SIZE) || (@operation == :sum && batch_size < DEFAULT_SUM_BATCH_SIZE) || - (@operation == :average && batch_size < DEFAULT_AVERAGE_BATCH_SIZE) || (finish - start) / batch_size >= MAX_ALLOWED_LOOPS || start >= finish end @@ -110,7 +108,6 @@ module Gitlab def batch_size_for_mode_and_operation(mode, operation) return DEFAULT_SUM_BATCH_SIZE if operation == :sum - return DEFAULT_AVERAGE_BATCH_SIZE if operation == :average mode == :distinct ? DEFAULT_DISTINCT_BATCH_SIZE : DEFAULT_BATCH_SIZE end @@ -148,10 +145,6 @@ module Gitlab message: "Query has been canceled with message: #{error.message}" ) end - - def not_group_by_query? - !@relation.is_a?(ActiveRecord::Relation) || @relation.group_values.blank? - end end end end diff --git a/lib/gitlab/usage_data_counters/hll_redis_counter.rb b/lib/gitlab/usage_data_counters/hll_redis_counter.rb index a5db8ba4dcc..51dc3689dfd 100644 --- a/lib/gitlab/usage_data_counters/hll_redis_counter.rb +++ b/lib/gitlab/usage_data_counters/hll_redis_counter.rb @@ -22,7 +22,6 @@ module Gitlab analytics code_review compliance - deploy_token_packages ecosystem epic_boards_usage epics_usage @@ -37,6 +36,7 @@ module Gitlab CATEGORIES_COLLECTED_FROM_METRICS_DEFINITIONS = %w[ ci_users + deploy_token_packages error_tracking ide_edit importer diff --git a/locale/gitlab.pot b/locale/gitlab.pot index fea9aa2529c..e30601d3011 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -218,6 +218,11 @@ msgid_plural "%d contributions" msgstr[0] "" msgstr[1] "" +msgid "%d contributor" +msgid_plural "%d contributors" +msgstr[0] "" +msgstr[1] "" + msgid "%d day" msgid_plural "%d days" msgstr[0] "" @@ -368,6 +373,11 @@ msgid_plural "%d projects selected" msgstr[0] "" msgstr[1] "" +msgid "%d push" +msgid_plural "%d pushes" +msgstr[0] "" +msgstr[1] "" + msgid "%d remaining" msgid_plural "%d remaining" msgstr[0] "" @@ -10418,7 +10428,7 @@ msgstr "" msgid "ContributionAnalytics|%{created_count} created, %{merged_count} merged, %{closed_count} closed." msgstr "" -msgid "ContributionAnalytics|%{pushes} pushes, more than %{commits} commits by %{people} contributors." +msgid "ContributionAnalytics|%{pushes}, more than %{commits} by %{contributors}." msgstr "" msgid "ContributionAnalytics|Contribution analytics for issues, merge requests and push events since %{start_date}" @@ -13104,6 +13114,12 @@ msgstr "" msgid "DeploymentApproval|Manual job: %{jobName}" msgstr "" +msgid "DeploymentApproval|Rejected %{time}" +msgstr "" + +msgid "DeploymentApproval|Rejected by you %{time}" +msgstr "" + msgid "DeploymentTarget|GitLab Pages" msgstr "" @@ -46175,7 +46191,7 @@ msgstr "" msgid "ciReport|Manage licenses" msgstr "" -msgid "ciReport|Manually Added" +msgid "ciReport|Manually added" msgstr "" msgid "ciReport|New" diff --git a/spec/config/settings_spec.rb b/spec/config/settings_spec.rb index 1de0e7e6c26..10eeb3f5371 100644 --- a/spec/config/settings_spec.rb +++ b/spec/config/settings_spec.rb @@ -116,7 +116,7 @@ RSpec.describe Settings do describe '.cron_for_service_ping' do it 'returns correct crontab for some manually calculated example' do allow(Gitlab::CurrentSettings) - .to receive(:uuid) { 'd9e2f4e8-db1f-4e51-b03d-f427e1965c4a'} + .to receive(:uuid) { 'd9e2f4e8-db1f-4e51-b03d-f427e1965c4a' } expect(described_class.send(:cron_for_service_ping)).to eq('44 10 * * 4') end diff --git a/spec/controllers/admin/application_settings_controller_spec.rb b/spec/controllers/admin/application_settings_controller_spec.rb index cb25011fd96..ab0cad989cb 100644 --- a/spec/controllers/admin/application_settings_controller_spec.rb +++ b/spec/controllers/admin/application_settings_controller_spec.rb @@ -9,7 +9,7 @@ RSpec.describe Admin::ApplicationSettingsController, :do_not_mock_admin_mode_set let(:group) { create(:group) } let(:project) { create(:project, namespace: group) } let(:admin) { create(:admin) } - let(:user) { create(:user)} + let(:user) { create(:user) } before do stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 1e28ef4ba93..f1adb9020fa 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -1006,7 +1006,7 @@ RSpec.describe ApplicationController do end describe '.endpoint_id_for_action' do - controller(described_class) { } + controller(described_class) {} it 'returns an expected endpoint id' do expect(controller.class.endpoint_id_for_action('hello')).to eq('AnonymousController#hello') diff --git a/spec/controllers/groups/labels_controller_spec.rb b/spec/controllers/groups/labels_controller_spec.rb index 90da40cd5f0..37db26096d3 100644 --- a/spec/controllers/groups/labels_controller_spec.rb +++ b/spec/controllers/groups/labels_controller_spec.rb @@ -20,7 +20,7 @@ RSpec.describe Groups::LabelsController do it 'returns group and project labels by default' do get :index, params: { group_id: group }, format: :json - label_ids = json_response.map {|label| label['title']} + label_ids = json_response.map { |label| label['title'] } expect(label_ids).to match_array([label_1.title, group_label_1.title]) end @@ -36,7 +36,7 @@ RSpec.describe Groups::LabelsController do params = { group_id: subgroup, only_group_labels: true } get :index, params: params, format: :json - label_ids = json_response.map {|label| label['title']} + label_ids = json_response.map { |label| label['title'] } expect(label_ids).to match_array([group_label_1.title, subgroup_label_1.title]) end end diff --git a/spec/controllers/groups/releases_controller_spec.rb b/spec/controllers/groups/releases_controller_spec.rb index 9d372114d62..7dd0bc6206a 100644 --- a/spec/controllers/groups/releases_controller_spec.rb +++ b/spec/controllers/groups/releases_controller_spec.rb @@ -42,7 +42,7 @@ RSpec.describe Groups::ReleasesController do end it 'does not return any releases' do - expect(json_response.map {|r| r['tag'] } ).to be_empty + expect(json_response.map { |r| r['tag'] } ).to be_empty end it 'returns OK' do @@ -56,7 +56,7 @@ RSpec.describe Groups::ReleasesController do index - expect(json_response.map {|r| r['tag'] } ).to match_array(%w(p2 p1 v2 v1)) + expect(json_response.map { |r| r['tag'] } ).to match_array(%w(p2 p1 v2 v1)) end end diff --git a/spec/controllers/import/manifest_controller_spec.rb b/spec/controllers/import/manifest_controller_spec.rb index 0111ad9501f..6f805b44e89 100644 --- a/spec/controllers/import/manifest_controller_spec.rb +++ b/spec/controllers/import/manifest_controller_spec.rb @@ -6,7 +6,7 @@ RSpec.describe Import::ManifestController, :clean_gitlab_redis_shared_state do include ImportSpecHelper let_it_be(:user) { create(:user) } - let_it_be(:group) { create(:group)} + let_it_be(:group) { create(:group) } before(:all) do group.add_maintainer(user) diff --git a/spec/controllers/projects/blame_controller_spec.rb b/spec/controllers/projects/blame_controller_spec.rb index bf475f6135a..62a544bb3fc 100644 --- a/spec/controllers/projects/blame_controller_spec.rb +++ b/spec/controllers/projects/blame_controller_spec.rb @@ -41,7 +41,7 @@ RSpec.describe Projects::BlameController do end context "invalid branch, valid file" do - let(:id) { 'invalid-branch/files/ruby/missing_file.rb'} + let(:id) { 'invalid-branch/files/ruby/missing_file.rb' } it { is_expected.to respond_with(:not_found) } end diff --git a/spec/controllers/projects/deploy_keys_controller_spec.rb b/spec/controllers/projects/deploy_keys_controller_spec.rb index 821f7fca73d..308146ce792 100644 --- a/spec/controllers/projects/deploy_keys_controller_spec.rb +++ b/spec/controllers/projects/deploy_keys_controller_spec.rb @@ -27,8 +27,8 @@ RSpec.describe Projects::DeployKeysController do end context 'when json requested' do - let(:project2) { create(:project, :internal)} - let(:project_private) { create(:project, :private)} + let(:project2) { create(:project, :internal) } + let(:project_private) { create(:project, :private) } let(:deploy_key_internal) { create(:deploy_key) } let(:deploy_key_actual) { create(:deploy_key) } diff --git a/spec/controllers/projects/feature_flags_controller_spec.rb b/spec/controllers/projects/feature_flags_controller_spec.rb index fd95aa44568..d7850cc5f33 100644 --- a/spec/controllers/projects/feature_flags_controller_spec.rb +++ b/spec/controllers/projects/feature_flags_controller_spec.rb @@ -208,7 +208,7 @@ RSpec.describe Projects::FeatureFlagsController do end context 'when feature flag is not found' do - let!(:feature_flag) { } + let!(:feature_flag) {} let(:params) do { diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index e4e3151dd12..0793e790301 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -700,7 +700,7 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state do expect(response).to match_response_schema('job/build_trace') expect(json_response['id']).to eq job.id expect(json_response['status']).to eq job.status - expect(json_response['lines'].flat_map {|l| l['content'].map { |c| c['text'] } }).to include("ヾ(´༎ຶД༎ຶ`)ﾉ") + expect(json_response['lines'].flat_map { |l| l['content'].map { |c| c['text'] } }).to include("ヾ(´༎ຶД༎ຶ`)ﾉ") end end diff --git a/spec/controllers/projects/labels_controller_spec.rb b/spec/controllers/projects/labels_controller_spec.rb index 776ed9774b1..a5259522fe2 100644 --- a/spec/controllers/projects/labels_controller_spec.rb +++ b/spec/controllers/projects/labels_controller_spec.rb @@ -25,10 +25,10 @@ RSpec.describe Projects::LabelsController do let_it_be(:group_label_3) { create(:group_label, group: group, title: 'Group Label 3') } let_it_be(:group_label_4) { create(:group_label, group: group, title: 'Group Label 4') } - let_it_be(:group_labels) { [group_label_3, group_label_4]} - let_it_be(:project_labels) { [label_4, label_5]} - let_it_be(:group_priority_labels) { [group_label_1, group_label_2]} - let_it_be(:project_priority_labels) { [label_1, label_2, label_3]} + let_it_be(:group_labels) { [group_label_3, group_label_4] } + let_it_be(:project_labels) { [label_4, label_5] } + let_it_be(:group_priority_labels) { [group_label_1, group_label_2] } + let_it_be(:project_priority_labels) { [label_1, label_2, label_3] } before do create(:label_priority, project: project, label: group_label_1, priority: 3) diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index 9050765afd6..1f8e96258ca 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -659,7 +659,7 @@ RSpec.describe Projects::NotesController do context 'when target_id and noteable_id do not match' do let(:locked_issue) { create(:issue, :locked, project: project) } - let(:issue) {create(:issue, project: project)} + let(:issue) { create(:issue, project: project) } it 'uses target_id and ignores noteable_id' do request_params = { diff --git a/spec/controllers/projects/releases_controller_spec.rb b/spec/controllers/projects/releases_controller_spec.rb index ad6682601f3..b307bb357fa 100644 --- a/spec/controllers/projects/releases_controller_spec.rb +++ b/spec/controllers/projects/releases_controller_spec.rb @@ -312,7 +312,7 @@ RSpec.describe Projects::ReleasesController do end context 'suffix path abuse' do - let(:suffix_path) { 'downloads/zips/../../../../../../../robots.txt'} + let(:suffix_path) { 'downloads/zips/../../../../../../../robots.txt' } it 'raises attack error' do expect do diff --git a/spec/controllers/projects/tree_controller_spec.rb b/spec/controllers/projects/tree_controller_spec.rb index 143516e4712..9bc3065b6da 100644 --- a/spec/controllers/projects/tree_controller_spec.rb +++ b/spec/controllers/projects/tree_controller_spec.rb @@ -163,8 +163,8 @@ RSpec.describe Projects::TreeController do end context 'successful creation' do - let(:path) { 'files/new_dir'} - let(:branch_name) { 'master-test'} + let(:path) { 'files/new_dir' } + let(:branch_name) { 'master-test' } it 'redirects to the new directory' do expect(subject) @@ -175,7 +175,7 @@ RSpec.describe Projects::TreeController do context 'unsuccessful creation' do let(:path) { 'README.md' } - let(:branch_name) { 'master'} + let(:branch_name) { 'master' } it 'does not allow overwriting of existing files' do expect(subject) diff --git a/spec/controllers/registrations/welcome_controller_spec.rb b/spec/controllers/registrations/welcome_controller_spec.rb index 8a5a8490a23..14e88d469ba 100644 --- a/spec/controllers/registrations/welcome_controller_spec.rb +++ b/spec/controllers/registrations/welcome_controller_spec.rb @@ -36,7 +36,7 @@ RSpec.describe Registrations::WelcomeController do sign_in(user) end - it { is_expected.to redirect_to(dashboard_projects_path)} + it { is_expected.to redirect_to(dashboard_projects_path) } end context 'when role is set and setup_for_company is not set' do @@ -78,7 +78,7 @@ RSpec.describe Registrations::WelcomeController do sign_in(user) end - it { is_expected.to redirect_to(dashboard_projects_path)} + it { is_expected.to redirect_to(dashboard_projects_path) } context 'when the new user already has any accepted group membership' do let!(:member1) { create(:group_member, user: user) } diff --git a/spec/controllers/snippets/notes_controller_spec.rb b/spec/controllers/snippets/notes_controller_spec.rb index 8e85e283b31..00d99b46d0b 100644 --- a/spec/controllers/snippets/notes_controller_spec.rb +++ b/spec/controllers/snippets/notes_controller_spec.rb @@ -312,7 +312,7 @@ RSpec.describe Snippets::NotesController do describe 'POST toggle_award_emoji' do let(:note) { create(:note_on_personal_snippet, noteable: public_snippet) } - let(:emoji_name) { 'thumbsup'} + let(:emoji_name) { 'thumbsup' } before do sign_in(user) diff --git a/spec/dependencies/omniauth_saml_spec.rb b/spec/dependencies/omniauth_saml_spec.rb index 8956fa44b7a..470b1512a83 100644 --- a/spec/dependencies/omniauth_saml_spec.rb +++ b/spec/dependencies/omniauth_saml_spec.rb @@ -17,7 +17,7 @@ RSpec.describe 'processing of SAMLResponse in dependencies' do allow_next_instance_of(OneLogin::RubySaml::Response) do |instance| allow(instance).to receive(:is_valid?).and_return(true) end - saml_strategy.send(:handle_response, mock_saml_response, {}, settings ) { } + saml_strategy.send(:handle_response, mock_saml_response, {}, settings ) {} end it 'can extract AuthnContextClassRef from SAMLResponse param' do diff --git a/spec/experiments/application_experiment_spec.rb b/spec/experiments/application_experiment_spec.rb index a39890dd530..20050fae1cb 100644 --- a/spec/experiments/application_experiment_spec.rb +++ b/spec/experiments/application_experiment_spec.rb @@ -21,7 +21,7 @@ RSpec.describe ApplicationExperiment, :experiment do # them optional there. expect(experiment(:example)).to register_behavior(:control).with(nil) - expect { experiment(:example) { } }.not_to raise_error + expect { experiment(:example) {} }.not_to raise_error end describe "#publish" do @@ -289,11 +289,11 @@ RSpec.describe ApplicationExperiment, :experiment do end it "doesn't raise an exception" do - expect { experiment(:top) { |e| e.control { experiment(:nested) { } } } }.not_to raise_error + expect { experiment(:top) { |e| e.control { experiment(:nested) {} } } }.not_to raise_error end it "tracks an event", :snowplow do - experiment(:top) { |e| e.control { experiment(:nested) { } } } + experiment(:top) { |e| e.control { experiment(:nested) {} } } expect(Gitlab::Tracking).to have_received(:event).with( # rubocop:disable RSpec/ExpectGitlabTracking 'top', @@ -311,8 +311,8 @@ RSpec.describe ApplicationExperiment, :experiment do cache.clear(key: application_experiment.name) - application_experiment.control { } - application_experiment.candidate { } + application_experiment.control {} + application_experiment.candidate {} end it "caches the variant determined by the variant resolver" do @@ -378,7 +378,7 @@ RSpec.describe ApplicationExperiment, :experiment do it "doesn't warn on non dev/test environments" do allow(Gitlab).to receive(:dev_or_test_env?).and_return(false) - expect { experiment(:example) { |e| e.use { } } }.not_to raise_error + expect { experiment(:example) { |e| e.use {} } }.not_to raise_error expect(ActiveSupport::Deprecation).not_to have_received(:new).with(anything, 'Gitlab::Experiment') end @@ -387,7 +387,7 @@ RSpec.describe ApplicationExperiment, :experiment do # This will eventually raise an ActiveSupport::Deprecation exception, # it's ok to change it when that happens. - expect { experiment(:example) { |e| e.use { } } }.not_to raise_error + expect { experiment(:example) { |e| e.use {} } }.not_to raise_error expect(ActiveSupport::Deprecation).to have_received(:new).with(anything, 'Gitlab::Experiment') end diff --git a/spec/finders/ci/jobs_finder_spec.rb b/spec/finders/ci/jobs_finder_spec.rb index 45e8cf5a582..dd3ba9721e4 100644 --- a/spec/finders/ci/jobs_finder_spec.rb +++ b/spec/finders/ci/jobs_finder_spec.rb @@ -56,7 +56,7 @@ RSpec.describe Ci::JobsFinder, '#execute' do context 'scope is an array' do let(:jobs) { [pending_job, running_job, successful_job, canceled_job] } - let(:params) {{ scope: %w'running success' }} + let(:params) { { scope: %w'running success' } } it 'filters by the job statuses in the scope' do expect(subject).to contain_exactly(running_job, successful_job) diff --git a/spec/finders/ci/runners_finder_spec.rb b/spec/finders/ci/runners_finder_spec.rb index 96412c1e371..8d3c375385a 100644 --- a/spec/finders/ci/runners_finder_spec.rb +++ b/spec/finders/ci/runners_finder_spec.rb @@ -260,13 +260,13 @@ RSpec.describe Ci::RunnersFinder do let_it_be(:runner_sub_group_2) { create(:ci_runner, :group, contacted_at: 10.minutes.ago) } let_it_be(:runner_sub_group_3) { create(:ci_runner, :group, contacted_at: 9.minutes.ago) } let_it_be(:runner_sub_group_4) { create(:ci_runner, :group, contacted_at: 8.minutes.ago) } - let_it_be(:runner_project_1) { create(:ci_runner, :project, contacted_at: 7.minutes.ago, projects: [project])} - let_it_be(:runner_project_2) { create(:ci_runner, :project, contacted_at: 6.minutes.ago, projects: [project_2])} - let_it_be(:runner_project_3) { create(:ci_runner, :project, contacted_at: 5.minutes.ago, description: 'runner_project_search', projects: [project, project_2])} - let_it_be(:runner_project_4) { create(:ci_runner, :project, contacted_at: 4.minutes.ago, projects: [project_3])} - let_it_be(:runner_project_5) { create(:ci_runner, :project, contacted_at: 3.minutes.ago, tag_list: %w[runner_tag], projects: [project_4])} - let_it_be(:runner_project_6) { create(:ci_runner, :project, contacted_at: 2.minutes.ago, projects: [project_5])} - let_it_be(:runner_project_7) { create(:ci_runner, :project, contacted_at: 1.minute.ago, projects: [project_6])} + let_it_be(:runner_project_1) { create(:ci_runner, :project, contacted_at: 7.minutes.ago, projects: [project]) } + let_it_be(:runner_project_2) { create(:ci_runner, :project, contacted_at: 6.minutes.ago, projects: [project_2]) } + let_it_be(:runner_project_3) { create(:ci_runner, :project, contacted_at: 5.minutes.ago, description: 'runner_project_search', projects: [project, project_2]) } + let_it_be(:runner_project_4) { create(:ci_runner, :project, contacted_at: 4.minutes.ago, projects: [project_3]) } + let_it_be(:runner_project_5) { create(:ci_runner, :project, contacted_at: 3.minutes.ago, tag_list: %w[runner_tag], projects: [project_4]) } + let_it_be(:runner_project_6) { create(:ci_runner, :project, contacted_at: 2.minutes.ago, projects: [project_5]) } + let_it_be(:runner_project_7) { create(:ci_runner, :project, contacted_at: 1.minute.ago, projects: [project_6]) } let(:target_group) { nil } let(:membership) { nil } diff --git a/spec/finders/concerns/packages/finder_helper_spec.rb b/spec/finders/concerns/packages/finder_helper_spec.rb index e8648d131ff..94bcec6163e 100644 --- a/spec/finders/concerns/packages/finder_helper_spec.rb +++ b/spec/finders/concerns/packages/finder_helper_spec.rb @@ -24,7 +24,7 @@ RSpec.describe ::Packages::FinderHelper do subject { finder.execute(project1) } - it { is_expected.to eq [package1]} + it { is_expected.to eq [package1] } end describe '#packages_visible_to_user' do @@ -61,7 +61,7 @@ RSpec.describe ::Packages::FinderHelper do end shared_examples 'returning package1' do - it { is_expected.to eq [package1]} + it { is_expected.to eq [package1] } end shared_examples 'returning no packages' do @@ -165,7 +165,7 @@ RSpec.describe ::Packages::FinderHelper do end shared_examples 'returning project1' do - it { is_expected.to eq [project1]} + it { is_expected.to eq [project1] } end shared_examples 'returning no project' do diff --git a/spec/finders/container_repositories_finder_spec.rb b/spec/finders/container_repositories_finder_spec.rb index 5d449d1b811..472c39d1f23 100644 --- a/spec/finders/container_repositories_finder_spec.rb +++ b/spec/finders/container_repositories_finder_spec.rb @@ -28,9 +28,9 @@ RSpec.describe ContainerRepositoriesFinder do context "with name set to #{name}" do let(:params) { { name: name } } - it { is_expected.to contain_exactly(project_repository)} + it { is_expected.to contain_exactly(project_repository) } - it { is_expected.not_to include(not_searched_repository)} + it { is_expected.not_to include(not_searched_repository) } end end end @@ -50,7 +50,7 @@ RSpec.describe ContainerRepositoriesFinder do context "with sort set to #{order}" do let(:params) { { sort: order } } - it { is_expected.to eq([sort_repository2, sort_repository])} + it { is_expected.to eq([sort_repository2, sort_repository]) } end end @@ -58,7 +58,7 @@ RSpec.describe ContainerRepositoriesFinder do context "with sort set to #{order}" do let(:params) { { sort: order } } - it { is_expected.to eq([sort_repository, sort_repository2])} + it { is_expected.to eq([sort_repository, sort_repository2]) } end end end diff --git a/spec/finders/design_management/versions_finder_spec.rb b/spec/finders/design_management/versions_finder_spec.rb index 0d606ef46f1..7dafdcfda97 100644 --- a/spec/finders/design_management/versions_finder_spec.rb +++ b/spec/finders/design_management/versions_finder_spec.rb @@ -71,13 +71,13 @@ RSpec.describe DesignManagement::VersionsFinder do describe 'returning versions earlier or equal to a version' do context 'when argument is the first version' do - let(:params) { { earlier_or_equal_to: version_1 }} + let(:params) { { earlier_or_equal_to: version_1 } } it { is_expected.to eq([version_1]) } end context 'when argument is the second version' do - let(:params) { { earlier_or_equal_to: version_2 }} + let(:params) { { earlier_or_equal_to: version_2 } } it { is_expected.to contain_exactly(version_1, version_2) } end diff --git a/spec/finders/milestones_finder_spec.rb b/spec/finders/milestones_finder_spec.rb index 8b26599cbfa..8dd83df3a28 100644 --- a/spec/finders/milestones_finder_spec.rb +++ b/spec/finders/milestones_finder_spec.rb @@ -28,7 +28,7 @@ RSpec.describe MilestonesFinder do end context 'milestones for groups and project' do - let(:extra_params) {{}} + let(:extra_params) { {} } let(:result) do described_class.new({ project_ids: [project_1.id, project_2.id], group_ids: group.id, state: 'all' }.merge(extra_params)).execute end diff --git a/spec/finders/packages/group_packages_finder_spec.rb b/spec/finders/packages/group_packages_finder_spec.rb index 90a8cd3c57f..f78a356b13d 100644 --- a/spec/finders/packages/group_packages_finder_spec.rb +++ b/spec/finders/packages/group_packages_finder_spec.rb @@ -217,7 +217,7 @@ RSpec.describe Packages::GroupPackagesFinder do context 'group is nil' do subject { described_class.new(user, nil).execute } - it { is_expected.to be_empty} + it { is_expected.to be_empty } end context 'package type is nil' do @@ -225,7 +225,7 @@ RSpec.describe Packages::GroupPackagesFinder do subject { described_class.new(user, group, package_type: nil).execute } - it { is_expected.to match_array([package1])} + it { is_expected.to match_array([package1]) } end context 'with invalid package_type' do diff --git a/spec/finders/packages/npm/package_finder_spec.rb b/spec/finders/packages/npm/package_finder_spec.rb index 7fabb3eed86..8c9149a5a2d 100644 --- a/spec/finders/packages/npm/package_finder_spec.rb +++ b/spec/finders/packages/npm/package_finder_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' RSpec.describe ::Packages::Npm::PackageFinder do - let_it_be_with_reload(:project) { create(:project)} + let_it_be_with_reload(:project) { create(:project) } let_it_be_with_refind(:package) { create(:npm_package, project: project) } let(:project) { package.project } diff --git a/spec/finders/projects_finder_spec.rb b/spec/finders/projects_finder_spec.rb index 3bef4d85b33..1fa2a975ec3 100644 --- a/spec/finders/projects_finder_spec.rb +++ b/spec/finders/projects_finder_spec.rb @@ -62,7 +62,7 @@ RSpec.describe ProjectsFinder do describe 'with id_after' do context 'only returns projects with a project id greater than given' do - let(:params) { { id_after: internal_project.id }} + let(:params) { { id_after: internal_project.id } } it { is_expected.to eq([public_project]) } end @@ -70,7 +70,7 @@ RSpec.describe ProjectsFinder do describe 'with id_before' do context 'only returns projects with a project id less than given' do - let(:params) { { id_before: public_project.id }} + let(:params) { { id_before: public_project.id } } it { is_expected.to eq([internal_project]) } end @@ -79,7 +79,7 @@ RSpec.describe ProjectsFinder do describe 'with both id_before and id_after' do context 'only returns projects with a project id less than given' do let!(:projects) { create_list(:project, 5, :public) } - let(:params) { { id_after: projects.first.id, id_before: projects.last.id }} + let(:params) { { id_after: projects.first.id, id_before: projects.last.id } } it { is_expected.to contain_exactly(*projects[1..-2]) } end @@ -89,7 +89,7 @@ RSpec.describe ProjectsFinder do context 'only returns projects with a project id less than given and matching search' do subject { finder.execute.joins(:route) } - let(:params) { { id_before: public_project.id }} + let(:params) { { id_before: public_project.id } } it { is_expected.to eq([internal_project]) } end @@ -97,7 +97,7 @@ RSpec.describe ProjectsFinder do context 'only returns projects with a project id greater than given and matching search' do subject { finder.execute.joins(:route) } - let(:params) { { id_after: internal_project.id }} + let(:params) { { id_after: internal_project.id } } it { is_expected.to eq([public_project]) } end diff --git a/spec/helpers/application_settings_helper_spec.rb b/spec/helpers/application_settings_helper_spec.rb index ba6caf7cf17..fab1fed797c 100644 --- a/spec/helpers/application_settings_helper_spec.rb +++ b/spec/helpers/application_settings_helper_spec.rb @@ -313,7 +313,7 @@ RSpec.describe ApplicationSettingsHelper do allow(helper).to receive(:can?).with(user, :read_cluster, instance_of(Clusters::Instance)).and_return(true) end - it { is_expected.to be_truthy} + it { is_expected.to be_truthy } context ':certificate_based_clusters feature flag is disabled' do before do diff --git a/spec/helpers/blob_helper_spec.rb b/spec/helpers/blob_helper_spec.rb index 65e46b61882..fe652e905cc 100644 --- a/spec/helpers/blob_helper_spec.rb +++ b/spec/helpers/blob_helper_spec.rb @@ -357,7 +357,7 @@ RSpec.describe BlobHelper do describe '#ide_merge_request_path' do let_it_be(:project) { create(:project, :repository) } - let_it_be(:merge_request) { create(:merge_request, source_project: project)} + let_it_be(:merge_request) { create(:merge_request, source_project: project) } it 'returns IDE path for the given MR if MR is not merged' do expect(helper.ide_merge_request_path(merge_request)).to eq("/-/ide/project/#{project.full_path}/merge_requests/#{merge_request.iid}") diff --git a/spec/helpers/gitlab_script_tag_helper_spec.rb b/spec/helpers/gitlab_script_tag_helper_spec.rb index 9d71e25286e..cfe7b349cec 100644 --- a/spec/helpers/gitlab_script_tag_helper_spec.rb +++ b/spec/helpers/gitlab_script_tag_helper_spec.rb @@ -27,8 +27,8 @@ RSpec.describe GitlabScriptTagHelper do end describe 'inline script tag' do - let(:tag_with_nonce) {""} - let(:tag_with_nonce_and_type) {""} + let(:tag_with_nonce) { "" } + let(:tag_with_nonce_and_type) { "" } it 'returns a script tag with a nonce using block syntax' do expect(helper.javascript_tag { 'alert(1)' }.to_s).to eq tag_with_nonce diff --git a/spec/helpers/issuables_helper_spec.rb b/spec/helpers/issuables_helper_spec.rb index 069465c2fec..18a21b59409 100644 --- a/spec/helpers/issuables_helper_spec.rb +++ b/spec/helpers/issuables_helper_spec.rb @@ -448,7 +448,7 @@ RSpec.describe IssuablesHelper do allow(merge_request).to receive(:can_be_merged_by?).and_return(can_merge) end - it { is_expected.to include({ can_merge: can_merge })} + it { is_expected.to include({ can_merge: can_merge }) } end end end @@ -480,7 +480,7 @@ RSpec.describe IssuablesHelper do allow(merge_request).to receive(:can_be_merged_by?).and_return(can_merge) end - it { is_expected.to include({ can_merge: can_merge })} + it { is_expected.to include({ can_merge: can_merge }) } end end end diff --git a/spec/helpers/projects/pipeline_helper_spec.rb b/spec/helpers/projects/pipeline_helper_spec.rb index 1231dcef693..59fc278543f 100644 --- a/spec/helpers/projects/pipeline_helper_spec.rb +++ b/spec/helpers/projects/pipeline_helper_spec.rb @@ -8,7 +8,7 @@ RSpec.describe Projects::PipelineHelper do let_it_be(:user) { create(:user) } let_it_be(:project) { create(:project, :repository) } let_it_be(:raw_pipeline) { create(:ci_pipeline, project: project, ref: 'master', sha: project.commit.id) } - let_it_be(:pipeline) { Ci::PipelinePresenter.new(raw_pipeline, current_user: user)} + let_it_be(:pipeline) { Ci::PipelinePresenter.new(raw_pipeline, current_user: user) } describe '#js_pipeline_tabs_data' do before do diff --git a/spec/helpers/routing/pseudonymization_helper_spec.rb b/spec/helpers/routing/pseudonymization_helper_spec.rb index dd4cc55ed2b..eb2cb548f35 100644 --- a/spec/helpers/routing/pseudonymization_helper_spec.rb +++ b/spec/helpers/routing/pseudonymization_helper_spec.rb @@ -70,7 +70,7 @@ RSpec.describe ::Routing::PseudonymizationHelper do end context 'with controller for groups with subgroups and project' do - let(:masked_url) { "http://localhost/namespace#{subgroup.id}/project#{subproject.id}"} + let(:masked_url) { "http://localhost/namespace#{subgroup.id}/project#{subproject.id}" } let(:group) { subgroup } let(:project) { subproject } let(:request) do @@ -94,7 +94,7 @@ RSpec.describe ::Routing::PseudonymizationHelper do end context 'with controller for groups and subgroups' do - let(:masked_url) { "http://localhost/groups/namespace#{subgroup.id}/-/shared"} + let(:masked_url) { "http://localhost/groups/namespace#{subgroup.id}/-/shared" } let(:group) { subgroup } let(:request) do double(:Request, diff --git a/spec/helpers/search_helper_spec.rb b/spec/helpers/search_helper_spec.rb index 513e2865ee3..ad0705e4fbf 100644 --- a/spec/helpers/search_helper_spec.rb +++ b/spec/helpers/search_helper_spec.rb @@ -625,7 +625,7 @@ RSpec.describe SearchHelper do false | false end - let(:params) {{ confidential: confidential }} + let(:params) { { confidential: confidential } } with_them do it 'transforms confidentiality param' do diff --git a/spec/helpers/wiki_page_version_helper_spec.rb b/spec/helpers/wiki_page_version_helper_spec.rb index bc500c28c5a..a792e5df035 100644 --- a/spec/helpers/wiki_page_version_helper_spec.rb +++ b/spec/helpers/wiki_page_version_helper_spec.rb @@ -6,8 +6,8 @@ RSpec.describe WikiPageVersionHelper do let_it_be(:project) { create(:project, :public, :repository) } let_it_be(:user) { create(:user, username: 'foo') } - let(:commit_with_user) { create(:commit, project: project, author: user)} - let(:commit_without_user) { create(:commit, project: project, author_name: 'Foo', author_email: 'foo@example.com')} + let(:commit_with_user) { create(:commit, project: project, author: user) } + let(:commit_without_user) { create(:commit, project: project, author_name: 'Foo', author_email: 'foo@example.com') } let(:wiki_page_version) { Gitlab::Git::WikiPageVersion.new(commit, nil) } describe '#wiki_page_version_author_url' do diff --git a/spec/initializers/carrierwave_patch_spec.rb b/spec/initializers/carrierwave_patch_spec.rb index b0f337935ef..0910342f10f 100644 --- a/spec/initializers/carrierwave_patch_spec.rb +++ b/spec/initializers/carrierwave_patch_spec.rb @@ -8,7 +8,7 @@ RSpec.describe 'CarrierWave::Storage::Fog::File' do let(:storage) { CarrierWave::Storage::Fog.new(uploader) } let(:bucket_name) { 'some-bucket' } let(:connection) { ::Fog::Storage.new(connection_options) } - let(:bucket) { connection.directories.new(key: bucket_name )} + let(:bucket) { connection.directories.new(key: bucket_name ) } let(:test_filename) { 'test' } let(:test_data) { File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml')) } @@ -33,7 +33,7 @@ RSpec.describe 'CarrierWave::Storage::Fog::File' do end describe '#copy_to' do - let(:dest_filename) { 'copied.txt'} + let(:dest_filename) { 'copied.txt' } it 'copies the file' do fog_file = subject.send(:file) @@ -67,7 +67,7 @@ RSpec.describe 'CarrierWave::Storage::Fog::File' do end describe '#copy_to' do - let(:dest_filename) { 'copied.txt'} + let(:dest_filename) { 'copied.txt' } it 'copies the file' do result = subject.copy_to(dest_filename) diff --git a/spec/initializers/trusted_proxies_spec.rb b/spec/initializers/trusted_proxies_spec.rb index 2786f034969..63c96ce17d1 100644 --- a/spec/initializers/trusted_proxies_spec.rb +++ b/spec/initializers/trusted_proxies_spec.rb @@ -58,7 +58,7 @@ RSpec.describe 'trusted_proxies' do end def stub_request(headers = {}) - ActionDispatch::RemoteIp.new(proc { }, false, Rails.application.config.action_dispatch.trusted_proxies).call(headers) + ActionDispatch::RemoteIp.new(proc {}, false, Rails.application.config.action_dispatch.trusted_proxies).call(headers) ActionDispatch::Request.new(headers) end diff --git a/spec/lib/gitlab/background_migration/backfill_vulnerability_reads_cluster_agent_spec.rb b/spec/lib/gitlab/background_migration/backfill_vulnerability_reads_cluster_agent_spec.rb index 79699375a8d..f642ec8c20d 100644 --- a/spec/lib/gitlab/background_migration/backfill_vulnerability_reads_cluster_agent_spec.rb +++ b/spec/lib/gitlab/background_migration/backfill_vulnerability_reads_cluster_agent_spec.rb @@ -23,8 +23,6 @@ RSpec.describe Gitlab::BackgroundMigration::BackfillVulnerabilityReadsClusterAge let(:sub_batch_size) { 1_000 } let(:pause_ms) { 0 } - subject(:perform_migration) { migration.perform } - before do users_table.create!(id: 1, name: 'John Doe', email: 'test@example.com', projects_limit: 5) @@ -49,20 +47,30 @@ RSpec.describe Gitlab::BackgroundMigration::BackfillVulnerabilityReadsClusterAge add_vulnerability_read!(11, project_id: 1, cluster_agent_id: 1, report_type: 7) end - it 'backfills `casted_cluster_agent_id` for the selected records', :aggregate_failures do - queries = ActiveRecord::QueryRecorder.new do - perform_migration + describe '#filter_batch' do + it 'pick only vulnerability reads where report_type = 7' do + expect(migration.filter_batch(vulnerability_reads_table).pluck(:id)).to contain_exactly(1, 3, 7, 9, 10, 11) end - - expect(queries.count).to eq(3) - expect(vulnerability_reads_table.where.not(casted_cluster_agent_id: nil).count).to eq 3 - expect(vulnerability_reads_table.where.not(casted_cluster_agent_id: nil).pluck(:id)).to match_array([1, 9, 10]) end - it 'tracks timings of queries' do - expect(migration.batch_metrics.timings).to be_empty + describe '#perform' do + subject(:perform_migration) { migration.perform } - expect { perform_migration }.to change { migration.batch_metrics.timings } + it 'backfills `casted_cluster_agent_id` for the selected records', :aggregate_failures do + queries = ActiveRecord::QueryRecorder.new do + perform_migration + end + + expect(queries.count).to eq(3) + expect(vulnerability_reads_table.where.not(casted_cluster_agent_id: nil).count).to eq 3 + expect(vulnerability_reads_table.where.not(casted_cluster_agent_id: nil).pluck(:id)).to match_array([1, 9, 10]) + end + + it 'tracks timings of queries' do + expect(migration.batch_metrics.timings).to be_empty + + expect { perform_migration }.to change { migration.batch_metrics.timings } + end end private diff --git a/spec/lib/gitlab/background_migration/batching_strategies/backfill_project_statistics_with_container_registry_size_batching_strategy_spec.rb b/spec/lib/gitlab/background_migration/batching_strategies/backfill_project_statistics_with_container_registry_size_batching_strategy_spec.rb index 94e9bcf9207..7076e82ea34 100644 --- a/spec/lib/gitlab/background_migration/batching_strategies/backfill_project_statistics_with_container_registry_size_batching_strategy_spec.rb +++ b/spec/lib/gitlab/background_migration/batching_strategies/backfill_project_statistics_with_container_registry_size_batching_strategy_spec.rb @@ -2,137 +2,6 @@ require 'spec_helper' -RSpec.describe Gitlab::BackgroundMigration::BatchingStrategies::BackfillProjectStatisticsWithContainerRegistrySizeBatchingStrategy, '#next_batch' do # rubocop:disable Layout/LineLength - let(:batching_strategy) { described_class.new(connection: ActiveRecord::Base.connection) } - let(:namespace) { table(:namespaces) } - let(:project) { table(:projects) } - let(:container_repositories) { table(:container_repositories) } - - let!(:group) do - namespace.create!( - name: 'namespace1', type: 'Group', path: 'space1' - ) - end - - let!(:proj_namespace1) do - namespace.create!( - name: 'proj1', path: 'proj1', type: 'Project', parent_id: group.id - ) - end - - let!(:proj_namespace2) do - namespace.create!( - name: 'proj2', path: 'proj2', type: 'Project', parent_id: group.id - ) - end - - let!(:proj_namespace3) do - namespace.create!( - name: 'proj3', path: 'proj3', type: 'Project', parent_id: group.id - ) - end - - let!(:proj1) do - project.create!( - name: 'proj1', path: 'proj1', namespace_id: group.id, project_namespace_id: proj_namespace1.id - ) - end - - let!(:proj2) do - project.create!( - name: 'proj2', path: 'proj2', namespace_id: group.id, project_namespace_id: proj_namespace2.id - ) - end - - let!(:proj3) do - project.create!( - name: 'proj3', path: 'proj3', namespace_id: group.id, project_namespace_id: proj_namespace3.id - ) - end - - let!(:con1) do - container_repositories.create!( - project_id: proj1.id, - name: "ContReg_#{proj1.id}:1", - migration_state: 'import_done', - created_at: Date.new(2022, 01, 20) - ) - end - - let!(:con2) do - container_repositories.create!( - project_id: proj1.id, - name: "ContReg_#{proj1.id}:2", - migration_state: 'import_done', - created_at: Date.new(2022, 01, 20) - ) - end - - let!(:con3) do - container_repositories.create!( - project_id: proj2.id, - name: "ContReg_#{proj2.id}:1", - migration_state: 'import_done', - created_at: Date.new(2022, 01, 20) - ) - end - - let!(:con4) do - container_repositories.create!( - project_id: proj3.id, - name: "ContReg_#{proj3.id}:1", - migration_state: 'default', - created_at: Date.new(2022, 02, 20) - ) - end - - let!(:con5) do - container_repositories.create!( - project_id: proj3.id, - name: "ContReg_#{proj3.id}:2", - migration_state: 'default', - created_at: Date.new(2022, 02, 20) - ) - end - +RSpec.describe Gitlab::BackgroundMigration::BatchingStrategies::BackfillProjectStatisticsWithContainerRegistrySizeBatchingStrategy do # rubocop:disable Layout/LineLength it { expect(described_class).to be < Gitlab::BackgroundMigration::BatchingStrategies::PrimaryKeyBatchingStrategy } - - context 'when starting on the first batch' do - it 'returns the bounds of the next batch' do - batch_bounds = batching_strategy.next_batch( - :container_repositories, - :project_id, - batch_min_value: con1.project_id, - batch_size: 3, - job_arguments: [] - ) - expect(batch_bounds).to eq([con1.project_id, con4.project_id]) - end - end - - context 'when additional batches remain' do - it 'returns the bounds of the next batch' do - batch_bounds = batching_strategy.next_batch( - :container_repositories, - :project_id, - batch_min_value: con3.project_id, - batch_size: 3, - job_arguments: [] - ) - - expect(batch_bounds).to eq([con3.project_id, con5.project_id]) - end - end - - context 'when no additional batches remain' do - it 'returns nil' do - batch_bounds = batching_strategy.next_batch(:container_repositories, - :project_id, - batch_min_value: con5.project_id + 1, - batch_size: 1, job_arguments: [] - ) - - expect(batch_bounds).to be_nil - end - end end diff --git a/spec/lib/gitlab/background_migration/batching_strategies/dismissed_vulnerabilities_strategy_spec.rb b/spec/lib/gitlab/background_migration/batching_strategies/dismissed_vulnerabilities_strategy_spec.rb index f96c7de50f2..e4bef81e0bd 100644 --- a/spec/lib/gitlab/background_migration/batching_strategies/dismissed_vulnerabilities_strategy_spec.rb +++ b/spec/lib/gitlab/background_migration/batching_strategies/dismissed_vulnerabilities_strategy_spec.rb @@ -3,117 +3,5 @@ require 'spec_helper' RSpec.describe Gitlab::BackgroundMigration::BatchingStrategies::DismissedVulnerabilitiesStrategy, '#next_batch' do - let(:batching_strategy) { described_class.new(connection: ActiveRecord::Base.connection) } - let(:namespace) { table(:namespaces).create!(name: 'user', path: 'user') } - let(:users) { table(:users) } - let(:user) { create_user! } - let(:project) do - table(:projects).create!( - namespace_id: namespace.id, - project_namespace_id: namespace.id, - packages_enabled: false) - end - - let(:vulnerabilities) { table(:vulnerabilities) } - - let!(:vulnerability1) do - create_vulnerability!( - project_id: project.id, - author_id: user.id, - dismissed_at: Time.current - ) - end - - let!(:vulnerability2) do - create_vulnerability!( - project_id: project.id, - author_id: user.id, - dismissed_at: Time.current - ) - end - - let!(:vulnerability3) do - create_vulnerability!( - project_id: project.id, - author_id: user.id, - dismissed_at: Time.current - ) - end - - let!(:vulnerability4) do - create_vulnerability!( - project_id: project.id, - author_id: user.id, - dismissed_at: nil - ) - end - it { expect(described_class).to be < Gitlab::BackgroundMigration::BatchingStrategies::PrimaryKeyBatchingStrategy } - - context 'when starting on the first batch' do - it 'returns the bounds of the next batch' do - batch_bounds = batching_strategy.next_batch( - :vulnerabilities, - :id, - batch_min_value: vulnerability1.id, - batch_size: 2, - job_arguments: [] - ) - expect(batch_bounds).to eq([vulnerability1.id, vulnerability2.id]) - end - end - - context 'when additional batches remain' do - it 'returns the bounds of the next batch and skips the records that do not have `dismissed_at` set' do - batch_bounds = batching_strategy.next_batch( - :vulnerabilities, - :id, - batch_min_value: vulnerability3.id, - batch_size: 2, - job_arguments: [] - ) - - expect(batch_bounds).to eq([vulnerability3.id, vulnerability3.id]) - end - end - - context 'when no additional batches remain' do - it 'returns nil' do - batch_bounds = batching_strategy.next_batch( - :vulnerabilities, - :id, - batch_min_value: vulnerability4.id + 1, - batch_size: 1, - job_arguments: [] - ) - - expect(batch_bounds).to be_nil - end - end - - private - - def create_vulnerability!( - project_id:, author_id:, title: 'test', severity: 7, confidence: 7, report_type: 0, state: 1, dismissed_at: nil - ) - vulnerabilities.create!( - project_id: project_id, - author_id: author_id, - title: title, - severity: severity, - confidence: confidence, - report_type: report_type, - state: state, - dismissed_at: dismissed_at - ) - end - - def create_user!(name: "Example User", email: "user@example.com", user_type: nil) - users.create!( - name: name, - email: email, - username: name, - projects_limit: 10 - ) - end end diff --git a/spec/lib/gitlab/background_migration/set_correct_vulnerability_state_spec.rb b/spec/lib/gitlab/background_migration/set_correct_vulnerability_state_spec.rb index d5b98e49a31..2372ce21c4c 100644 --- a/spec/lib/gitlab/background_migration/set_correct_vulnerability_state_spec.rb +++ b/spec/lib/gitlab/background_migration/set_correct_vulnerability_state_spec.rb @@ -34,7 +34,7 @@ RSpec.describe Gitlab::BackgroundMigration::SetCorrectVulnerabilityState do let(:detected_state) { 1 } let(:dismissed_state) { 2 } - subject(:perform_migration) do + let(:migration_job) do described_class.new(start_id: vulnerability_with_dismissed_at.id, end_id: vulnerability_without_dismissed_at.id, batch_table: :vulnerabilities, @@ -42,15 +42,24 @@ RSpec.describe Gitlab::BackgroundMigration::SetCorrectVulnerabilityState do sub_batch_size: 1, pause_ms: 0, connection: ActiveRecord::Base.connection) - .perform end - it 'changes vulnerability state to `dismissed` when dismissed_at is not nil' do - expect { perform_migration }.to change { vulnerability_with_dismissed_at.reload.state }.to(dismissed_state) + describe '#filter_batch' do + it 'filters out vulnerabilities where dismissed_at is null' do + expect(migration_job.filter_batch(vulnerabilities)).to contain_exactly(vulnerability_with_dismissed_at) + end end - it 'does not change the state when dismissed_at is nil' do - expect { perform_migration }.not_to change { vulnerability_without_dismissed_at.reload.state } + describe '#perform' do + subject(:perform_migration) { migration_job.perform } + + it 'changes vulnerability state to `dismissed` when dismissed_at is not nil' do + expect { perform_migration }.to change { vulnerability_with_dismissed_at.reload.state }.to(dismissed_state) + end + + it 'does not change the state when dismissed_at is nil' do + expect { perform_migration }.not_to change { vulnerability_without_dismissed_at.reload.state } + end end private diff --git a/spec/lib/gitlab/database/batch_average_counter_spec.rb b/spec/lib/gitlab/database/batch_average_counter_spec.rb new file mode 100644 index 00000000000..43c7a1554f7 --- /dev/null +++ b/spec/lib/gitlab/database/batch_average_counter_spec.rb @@ -0,0 +1,107 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::BatchAverageCounter do + let(:model) { Issue } + let(:column) { :weight } + + let(:in_transaction) { false } + + before do + allow(model.connection).to receive(:transaction_open?).and_return(in_transaction) + end + + describe '#count' do + before do + create_list(:issue, 2, weight: 4) + create_list(:issue, 2, weight: 2) + create_list(:issue, 2, weight: 3) + end + + subject(:batch_average_counter) { described_class.new(model, column) } + + it 'returns correct average of weights' do + expect(subject.count).to eq(3.0) + end + + it 'does no raise an exception if transaction is not open' do + expect { subject.count }.not_to raise_error + end + + context 'when transaction is open' do + let(:in_transaction) { true } + + it 'raises an error' do + expect { subject.count }.to raise_error('BatchAverageCounter can not be run inside a transaction') + end + end + + context 'when batch size is small' do + let(:batch_size) { 2 } + + it 'returns correct average of weights' do + expect(subject.count(batch_size: batch_size)).to eq(3.0) + end + end + + context 'when column passed is an Arel attribute' do + let(:column) { model.arel_table[:weight] } + + it 'returns correct average of weights' do + expect(subject.count).to eq(3.0) + end + end + + context 'when column has total count of zero' do + before do + Issue.update_all(weight: nil) + end + + it 'returns the fallback value' do + expect(subject.count).to eq(-1) + end + end + + context 'when one batch has nil weights (no average)' do + before do + issues = Issue.where(weight: 4) + issues.update_all(weight: nil) + end + + let(:batch_size) { 2 } + + it 'calculates average of weights with no errors' do + expect(subject.count(batch_size: batch_size)).to eq(2.5) + end + end + + context 'when batch fetch query is cancelled' do + let(:batch_size) { 22_000 } + let(:relation) { instance_double(ActiveRecord::Relation, to_sql: batch_average_query) } + + context 'when all retries fail' do + let(:batch_average_query) { 'SELECT AVG(weight) FROM issues WHERE weight BETWEEN 2 and 5' } + let(:query_timed_out_exception) { ActiveRecord::QueryCanceled.new('query timed out') } + + before do + allow(model).to receive(:where).and_return(relation) + allow(relation).to receive(:pick).and_raise(query_timed_out_exception) + end + + it 'logs failing query' do + expect(Gitlab::AppJsonLogger).to receive(:error).with( + event: 'batch_count', + relation: model.table_name, + operation: 'average', + start: 2, + query: batch_average_query, + message: 'Query has been canceled with message: query timed out' + ) + + expect(subject.count(batch_size: batch_size)).to eq(-1) + end + end + end + end +end diff --git a/spec/lib/gitlab/database/batch_count_spec.rb b/spec/lib/gitlab/database/batch_count_spec.rb index 72a4af692b4..a87b0c1a3a8 100644 --- a/spec/lib/gitlab/database/batch_count_spec.rb +++ b/spec/lib/gitlab/database/batch_count_spec.rb @@ -493,56 +493,18 @@ RSpec.describe Gitlab::Database::BatchCount do end describe '#batch_average' do - let(:model) { Issue } let(:column) { :weight } before do - Issue.update_all(weight: 2) + allow_next_instance_of(Gitlab::Database::BatchAverageCounter) do |instance| + allow(instance).to receive(:count).and_return + end end - it 'returns the average of values in the given column' do - expect(described_class.batch_average(model, column)).to eq(2) - end - - it 'works when given an Arel column' do - expect(described_class.batch_average(model, model.arel_table[column])).to eq(2) - end - - it 'works with a batch size of 50K' do - expect(described_class.batch_average(model, column, batch_size: 50_000)).to eq(2) - end - - it 'works with start and finish provided' do - expect(described_class.batch_average(model, column, start: model.minimum(:id), finish: model.maximum(:id))).to eq(2) - end - - it "defaults the batch size to #{Gitlab::Database::BatchCounter::DEFAULT_AVERAGE_BATCH_SIZE}" do - min_id = model.minimum(:id) - relation = instance_double(ActiveRecord::Relation) - allow(model).to receive_message_chain(:select, public_send: relation) - batch_end_id = min_id + calculate_batch_size(Gitlab::Database::BatchCounter::DEFAULT_AVERAGE_BATCH_SIZE) - - expect(relation).to receive(:where).with("id" => min_id..batch_end_id).and_return(double(send: 1)) + it 'calls BatchAverageCounter' do + expect(Gitlab::Database::BatchAverageCounter).to receive(:new).with(model, column).and_call_original described_class.batch_average(model, column) end - - it_behaves_like 'when a transaction is open' do - subject { described_class.batch_average(model, column) } - end - - it_behaves_like 'disallowed configurations', :batch_average do - let(:args) { [model, column] } - let(:default_batch_size) { Gitlab::Database::BatchCounter::DEFAULT_AVERAGE_BATCH_SIZE } - let(:small_batch_size) { Gitlab::Database::BatchCounter::DEFAULT_AVERAGE_BATCH_SIZE - 1 } - end - - it_behaves_like 'when batch fetch query is canceled' do - let(:mode) { :itself } - let(:operation) { :average } - let(:operation_args) { [column] } - - subject { described_class.method(:batch_average) } - end end end diff --git a/spec/lib/gitlab/usage_data_metrics_spec.rb b/spec/lib/gitlab/usage_data_metrics_spec.rb index 485f2131d87..1968523dc4a 100644 --- a/spec/lib/gitlab/usage_data_metrics_spec.rb +++ b/spec/lib/gitlab/usage_data_metrics_spec.rb @@ -16,6 +16,10 @@ RSpec.describe Gitlab::UsageDataMetrics do allow_next_instance_of(Gitlab::Database::BatchCounter) do |batch_counter| allow(batch_counter).to receive(:transaction_open?).and_return(false) end + + allow_next_instance_of(Gitlab::Database::BatchAverageCounter) do |instance| + allow(instance).to receive(:transaction_open?).and_return(false) + end end context 'with instrumentation_class' do diff --git a/spec/mailers/emails/service_desk_spec.rb b/spec/mailers/emails/service_desk_spec.rb index 28011456a66..1523d9b986b 100644 --- a/spec/mailers/emails/service_desk_spec.rb +++ b/spec/mailers/emails/service_desk_spec.rb @@ -76,7 +76,7 @@ RSpec.describe Emails::ServiceDesk do shared_examples 'read template from repository' do |template_key| let(:template_content) { 'custom text' } - let(:issue) { create(:issue, project: project)} + let(:issue) { create(:issue, project: project) } before do issue.issue_email_participants.create!(email: email) diff --git a/spec/migrations/20210812013042_remove_duplicate_project_authorizations_spec.rb b/spec/migrations/20210812013042_remove_duplicate_project_authorizations_spec.rb index f734456b0b6..c88f94c6426 100644 --- a/spec/migrations/20210812013042_remove_duplicate_project_authorizations_spec.rb +++ b/spec/migrations/20210812013042_remove_duplicate_project_authorizations_spec.rb @@ -48,7 +48,7 @@ RSpec.describe RemoveDuplicateProjectAuthorizations, :migration do project_authorizations.create! project_id: project_1.id, user_id: user_1.id, access_level: Gitlab::Access::REPORTER end - it { expect { subject }.to change { ProjectAuthorization.count}.from(3).to(1) } + it { expect { subject }.to change { ProjectAuthorization.count }.from(3).to(1) } it 'retains the highest access level' do subject diff --git a/spec/migrations/20210910194952_update_report_type_for_existing_approval_project_rules_spec.rb b/spec/migrations/20210910194952_update_report_type_for_existing_approval_project_rules_spec.rb index c90eabbe4eb..69ee10eb0d1 100644 --- a/spec/migrations/20210910194952_update_report_type_for_existing_approval_project_rules_spec.rb +++ b/spec/migrations/20210910194952_update_report_type_for_existing_approval_project_rules_spec.rb @@ -39,7 +39,7 @@ RSpec.describe UpdateReportTypeForExistingApprovalProjectRules, :migration do end context 'with the rule name set to another value (e.g., Test Rule)' do - let(:rule_name) { 'Test Rule'} + let(:rule_name) { 'Test Rule' } it 'does not update report_type' do expect { migrate! }.not_to change { approval_project_rule.reload.report_type } diff --git a/spec/migrations/confirm_support_bot_user_spec.rb b/spec/migrations/confirm_support_bot_user_spec.rb index f6bcab4aa7d..c60c7fe45f7 100644 --- a/spec/migrations/confirm_support_bot_user_spec.rb +++ b/spec/migrations/confirm_support_bot_user_spec.rb @@ -52,7 +52,7 @@ RSpec.describe ConfirmSupportBotUser, :migration do end it 'does not change the `created_at` attribute' do - expect { migrate!}.not_to change { support_bot.reload.created_at }.from(nil) + expect { migrate! }.not_to change { support_bot.reload.created_at }.from(nil) end end diff --git a/spec/migrations/reset_job_token_scope_enabled_again_spec.rb b/spec/migrations/reset_job_token_scope_enabled_again_spec.rb index da6817f6f21..8f9e12852e1 100644 --- a/spec/migrations/reset_job_token_scope_enabled_again_spec.rb +++ b/spec/migrations/reset_job_token_scope_enabled_again_spec.rb @@ -9,8 +9,8 @@ RSpec.describe ResetJobTokenScopeEnabledAgain do let(:projects) { table(:projects) } let(:namespaces) { table(:namespaces) } let(:namespace) { namespaces.create!(name: 'gitlab', path: 'gitlab-org') } - let(:project_1) { projects.create!(name: 'proj-1', path: 'gitlab-org', namespace_id: namespace.id)} - let(:project_2) { projects.create!(name: 'proj-2', path: 'gitlab-org', namespace_id: namespace.id)} + let(:project_1) { projects.create!(name: 'proj-1', path: 'gitlab-org', namespace_id: namespace.id) } + let(:project_2) { projects.create!(name: 'proj-2', path: 'gitlab-org', namespace_id: namespace.id) } before do settings.create!(id: 1, project_id: project_1.id, job_token_scope_enabled: true) diff --git a/spec/migrations/reset_job_token_scope_enabled_spec.rb b/spec/migrations/reset_job_token_scope_enabled_spec.rb index 40dfe4de34b..fb7bd78c11f 100644 --- a/spec/migrations/reset_job_token_scope_enabled_spec.rb +++ b/spec/migrations/reset_job_token_scope_enabled_spec.rb @@ -9,8 +9,8 @@ RSpec.describe ResetJobTokenScopeEnabled do let(:projects) { table(:projects) } let(:namespaces) { table(:namespaces) } let(:namespace) { namespaces.create!(name: 'gitlab', path: 'gitlab-org') } - let(:project_1) { projects.create!(name: 'proj-1', path: 'gitlab-org', namespace_id: namespace.id)} - let(:project_2) { projects.create!(name: 'proj-2', path: 'gitlab-org', namespace_id: namespace.id)} + let(:project_1) { projects.create!(name: 'proj-1', path: 'gitlab-org', namespace_id: namespace.id) } + let(:project_2) { projects.create!(name: 'proj-2', path: 'gitlab-org', namespace_id: namespace.id) } before do settings.create!(id: 1, project_id: project_1.id, job_token_scope_enabled: true) diff --git a/spec/migrations/reset_severity_levels_to_new_default_spec.rb b/spec/migrations/reset_severity_levels_to_new_default_spec.rb index 18dc001db16..c352f1f3cee 100644 --- a/spec/migrations/reset_severity_levels_to_new_default_spec.rb +++ b/spec/migrations/reset_severity_levels_to_new_default_spec.rb @@ -6,10 +6,10 @@ require_migration! RSpec.describe ResetSeverityLevelsToNewDefault do let(:approval_project_rules) { table(:approval_project_rules) } - let(:projects) { table(:projects)} - let(:namespaces) { table(:namespaces)} - let(:namespace) { namespaces.create!(name: 'namespace', path: 'namespace')} - let(:project) { projects.create!(name: 'project', path: 'project', namespace_id: namespace.id)} + let(:projects) { table(:projects) } + let(:namespaces) { table(:namespaces) } + let(:namespace) { namespaces.create!(name: 'namespace', path: 'namespace') } + let(:project) { projects.create!(name: 'project', path: 'project', namespace_id: namespace.id) } let(:approval_project_rule) { approval_project_rules.create!(name: 'rule', project_id: project.id, severity_levels: severity_levels) } context 'without having all severity levels selected' do @@ -27,7 +27,7 @@ RSpec.describe ResetSeverityLevelsToNewDefault do it 'changes severity_levels to the default value' do expect(approval_project_rule.severity_levels).to eq(severity_levels) - expect { migrate! }.to change {approval_project_rule.reload.severity_levels}.from(severity_levels).to(default_levels) + expect { migrate! }.to change { approval_project_rule.reload.severity_levels }.from(severity_levels).to(default_levels) end end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 6fbff95f504..cf80eafd137 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -4482,10 +4482,20 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do let_it_be(:pipeline) { create(:ci_pipeline) } context 'when the scheduling type is `dag`' do - it 'returns true' do - create(:ci_build, pipeline: pipeline, scheduling_type: :dag) + context 'when the processable is a bridge' do + it 'returns true' do + create(:ci_bridge, pipeline: pipeline, scheduling_type: :dag) - expect(pipeline.uses_needs?).to eq(true) + expect(pipeline.uses_needs?).to eq(true) + end + end + + context 'when the processable is a build' do + it 'returns true' do + create(:ci_build, pipeline: pipeline, scheduling_type: :dag) + + expect(pipeline.uses_needs?).to eq(true) + end end end diff --git a/spec/policies/clusters/agent_policy_spec.rb b/spec/policies/clusters/agent_policy_spec.rb index 307d751b78b..8f778d318ed 100644 --- a/spec/policies/clusters/agent_policy_spec.rb +++ b/spec/policies/clusters/agent_policy_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Clusters::AgentPolicy do - let(:cluster_agent) { create(:cluster_agent, name: 'agent' )} + let(:cluster_agent) { create(:cluster_agent, name: 'agent' ) } let(:user) { create(:admin) } let(:policy) { described_class.new(user, cluster_agent) } let(:project) { cluster_agent.project } diff --git a/spec/policies/group_member_policy_spec.rb b/spec/policies/group_member_policy_spec.rb index 50774313aae..27ce683861c 100644 --- a/spec/policies/group_member_policy_spec.rb +++ b/spec/policies/group_member_policy_spec.rb @@ -128,7 +128,7 @@ RSpec.describe GroupMemberPolicy do context 'with the group parent' do let(:current_user) { create :user } - let(:subgroup) { create(:group, :private, parent: group)} + let(:subgroup) { create(:group, :private, parent: group) } before do group.add_owner(owner) @@ -143,7 +143,7 @@ RSpec.describe GroupMemberPolicy do context 'without group parent' do let(:current_user) { create :user } - let(:subgroup) { create(:group, :private)} + let(:subgroup) { create(:group, :private) } before do subgroup.add_owner(current_user) @@ -158,7 +158,7 @@ RSpec.describe GroupMemberPolicy do context 'without group parent with two owners' do let(:current_user) { create :user } let(:other_user) { create :user } - let(:subgroup) { create(:group, :private)} + let(:subgroup) { create(:group, :private) } before do subgroup.add_owner(current_user) diff --git a/spec/policies/issue_policy_spec.rb b/spec/policies/issue_policy_spec.rb index 7ca4baddb79..4d492deb54c 100644 --- a/spec/policies/issue_policy_spec.rb +++ b/spec/policies/issue_policy_spec.rb @@ -398,7 +398,7 @@ RSpec.describe IssuePolicy do context 'with a hidden issue' do let(:user) { create(:user) } let(:banned_user) { create(:user, :banned) } - let(:admin) { create(:user, :admin)} + let(:admin) { create(:user, :admin) } let(:hidden_issue) { create(:issue, project: project, author: banned_user) } it 'does not allow non-admin user to read the issue' do diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index e8fdf9a8e25..9bf96475fc1 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -777,13 +777,13 @@ RSpec.describe ProjectPolicy do project.add_developer(user) end - it { is_expected.not_to be_allowed(:project_bot_access)} + it { is_expected.not_to be_allowed(:project_bot_access) } end context "when project bot and not part of the project" do let(:current_user) { project_bot } - it { is_expected.not_to be_allowed(:project_bot_access)} + it { is_expected.not_to be_allowed(:project_bot_access) } end context "when project bot and part of the project" do @@ -793,7 +793,7 @@ RSpec.describe ProjectPolicy do project.add_developer(project_bot) end - it { is_expected.to be_allowed(:project_bot_access)} + it { is_expected.to be_allowed(:project_bot_access) } end end @@ -804,7 +804,7 @@ RSpec.describe ProjectPolicy do project.add_maintainer(project_bot) end - it { is_expected.not_to be_allowed(:create_resource_access_tokens)} + it { is_expected.not_to be_allowed(:create_resource_access_tokens) } end end @@ -946,7 +946,7 @@ RSpec.describe ProjectPolicy do context 'with anonymous' do let(:current_user) { anonymous } - it { is_expected.to be_disallowed(:metrics_dashboard)} + it { is_expected.to be_disallowed(:metrics_dashboard) } end end diff --git a/spec/policies/terraform/state_policy_spec.rb b/spec/policies/terraform/state_policy_spec.rb index 82152920997..d75e20a2c66 100644 --- a/spec/policies/terraform/state_policy_spec.rb +++ b/spec/policies/terraform/state_policy_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe Terraform::StatePolicy do let_it_be(:project) { create(:project) } - let_it_be(:terraform_state) { create(:terraform_state, project: project)} + let_it_be(:terraform_state) { create(:terraform_state, project: project) } subject { described_class.new(user, terraform_state) } diff --git a/spec/policies/terraform/state_version_policy_spec.rb b/spec/policies/terraform/state_version_policy_spec.rb index 6614e073332..4d41dd44455 100644 --- a/spec/policies/terraform/state_version_policy_spec.rb +++ b/spec/policies/terraform/state_version_policy_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe Terraform::StateVersionPolicy do let_it_be(:project) { create(:project) } - let_it_be(:terraform_state) { create(:terraform_state, :with_version, project: project)} + let_it_be(:terraform_state) { create(:terraform_state, :with_version, project: project) } subject { described_class.new(user, terraform_state.latest_version) } diff --git a/spec/presenters/packages/composer/packages_presenter_spec.rb b/spec/presenters/packages/composer/packages_presenter_spec.rb index 1f638e5b935..ae88acea61d 100644 --- a/spec/presenters/packages/composer/packages_presenter_spec.rb +++ b/spec/presenters/packages/composer/packages_presenter_spec.rb @@ -50,7 +50,7 @@ RSpec.describe ::Packages::Composer::PackagesPresenter do end describe '#provider' do - subject { presenter.provider} + subject { presenter.provider } let(:expected_json) do { diff --git a/spec/presenters/packages/conan/package_presenter_spec.rb b/spec/presenters/packages/conan/package_presenter_spec.rb index d35137cd820..9b74d2e637e 100644 --- a/spec/presenters/packages/conan/package_presenter_spec.rb +++ b/spec/presenters/packages/conan/package_presenter_spec.rb @@ -7,7 +7,7 @@ RSpec.describe ::Packages::Conan::PackagePresenter do let_it_be(:package) { create(:conan_package) } let_it_be(:project) { package.project } let_it_be(:package_file_pending_destruction) { create(:package_file, :pending_destruction, package: package) } - let_it_be(:conan_package_reference) { '123456789'} + let_it_be(:conan_package_reference) { '123456789' } let(:params) { { package_scope: :instance } } let(:presenter) { described_class.new(package, user, project, params) } diff --git a/spec/presenters/packages/nuget/packages_metadata_presenter_spec.rb b/spec/presenters/packages/nuget/packages_metadata_presenter_spec.rb index b2bcdf8f03d..39682a3311c 100644 --- a/spec/presenters/packages/nuget/packages_metadata_presenter_spec.rb +++ b/spec/presenters/packages/nuget/packages_metadata_presenter_spec.rb @@ -12,7 +12,7 @@ RSpec.describe Packages::Nuget::PackagesMetadataPresenter do describe '#count' do subject { presenter.count } - it {is_expected.to eq 1} + it { is_expected.to eq 1 } end describe '#items' do diff --git a/spec/presenters/project_presenter_spec.rb b/spec/presenters/project_presenter_spec.rb index df3e4b985ab..5eeaea8de06 100644 --- a/spec/presenters/project_presenter_spec.rb +++ b/spec/presenters/project_presenter_spec.rb @@ -613,7 +613,7 @@ RSpec.describe ProjectPresenter do end context 'empty repo' do - let(:project) { create(:project, :stubbed_repository)} + let(:project) { create(:project, :stubbed_repository) } context 'for a guest user' do it 'orders the items correctly' do diff --git a/spec/rubocop/cop/migration/create_table_with_foreign_keys_spec.rb b/spec/rubocop/cop/migration/create_table_with_foreign_keys_spec.rb index 39bb4fc208f..072edb5827b 100644 --- a/spec/rubocop/cop/migration/create_table_with_foreign_keys_spec.rb +++ b/spec/rubocop/cop/migration/create_table_with_foreign_keys_spec.rb @@ -192,7 +192,7 @@ RSpec.describe RuboCop::Cop::Migration::CreateTableWithForeignKeys do include_context 'when there is a target to a high traffic table', :foreign_key do let(:explicit_target_opts) { ", to_table: :#{table_name}" } - let(:implicit_target_opts) { } + let(:implicit_target_opts) {} end end end diff --git a/spec/serializers/cluster_entity_spec.rb b/spec/serializers/cluster_entity_spec.rb index 7c4c146575d..2de27deeffe 100644 --- a/spec/serializers/cluster_entity_spec.rb +++ b/spec/serializers/cluster_entity_spec.rb @@ -45,7 +45,7 @@ RSpec.describe ClusterEntity do context 'when no application has been installed' do let(:cluster) { create(:cluster, :instance) } - subject { described_class.new(cluster, request: request).as_json[:applications]} + subject { described_class.new(cluster, request: request).as_json[:applications] } it 'contains helm as not_installable' do expect(subject).not_to be_empty diff --git a/spec/serializers/import/provider_repo_serializer_spec.rb b/spec/serializers/import/provider_repo_serializer_spec.rb index 430bad151d3..905685c75e3 100644 --- a/spec/serializers/import/provider_repo_serializer_spec.rb +++ b/spec/serializers/import/provider_repo_serializer_spec.rb @@ -23,7 +23,7 @@ RSpec.describe Import::ProviderRepoSerializer do end it 'raises an error if invalid provider supplied' do - expect { described_class.new.represent({}, { provider: :invalid })}.to raise_error { NotImplementedError } + expect { described_class.new.represent({}, { provider: :invalid }) }.to raise_error { NotImplementedError } end end end diff --git a/spec/support/shared_examples/features/content_editor_shared_examples.rb b/spec/support/shared_examples/features/content_editor_shared_examples.rb index 3fa7beea97e..21f264a8b6a 100644 --- a/spec/support/shared_examples/features/content_editor_shared_examples.rb +++ b/spec/support/shared_examples/features/content_editor_shared_examples.rb @@ -1,22 +1,87 @@ # frozen_string_literal: true RSpec.shared_examples 'edits content using the content editor' do - content_editor_testid = '[data-testid="content-editor"] [contenteditable].ProseMirror' + let(:content_editor_testid) { '[data-testid="content-editor"] [contenteditable].ProseMirror' } + + def switch_to_content_editor + find('[data-testid="toggle-editing-mode-button"] label', text: 'Rich text').click + end + + def type_in_content_editor(keys) + find(content_editor_testid).send_keys keys + end + + def open_insert_media_dropdown + page.find('svg[data-testid="media-icon"]').click + end + + def set_source_editor_content(content) + find('.js-gfm-input').set content + end + + def expect_formatting_menu_to_be_visible + expect(page).to have_css('[data-testid="formatting-bubble-menu"]') + end + + def expect_formatting_menu_to_be_hidden + expect(page).not_to have_css('[data-testid="formatting-bubble-menu"]') + end + + def expect_media_bubble_menu_to_be_visible + expect(page).to have_css('[data-testid="media-bubble-menu"]') + end + + def upload_asset(fixture_name) + attach_file('content_editor_image', Rails.root.join('spec', 'fixtures', fixture_name), make_visible: true) + end describe 'formatting bubble menu' do - it 'shows a formatting bubble menu for a regular paragraph' do + it 'shows a formatting bubble menu for a regular paragraph and headings' do + switch_to_content_editor + expect(page).to have_css(content_editor_testid) - find(content_editor_testid).send_keys 'Typing text in the content editor' - find(content_editor_testid).send_keys [:shift, :left] + type_in_content_editor 'Typing text in the content editor' + type_in_content_editor [:shift, :left] - expect(page).to have_css('[data-testid="formatting-bubble-menu"]') + expect_formatting_menu_to_be_visible + + type_in_content_editor [:right, :right, :enter, '## Heading'] + + expect_formatting_menu_to_be_hidden + + type_in_content_editor [:shift, :left] + + expect_formatting_menu_to_be_visible + end + end + + describe 'media elements bubble menu' do + before do + switch_to_content_editor + + open_insert_media_dropdown end - it 'does not show a formatting bubble menu for code blocks' do - find(content_editor_testid).send_keys '```js ' + def test_displays_media_bubble_menu(media_element_selector, fixture_file) + upload_asset fixture_file - expect(page).not_to have_css('[data-testid="formatting-bubble-menu"]') + wait_for_requests + + expect(page).to have_css(media_element_selector) + + page.find(media_element_selector).click + + expect_formatting_menu_to_be_hidden + expect_media_bubble_menu_to_be_visible + end + + it 'displays correct media bubble menu for images', :js do + test_displays_media_bubble_menu '[data-testid="content_editor_editablebox"] img[src]', 'dk.png' + end + + it 'displays correct media bubble menu for video', :js do + test_displays_media_bubble_menu '[data-testid="content_editor_editablebox"] video', 'video_sample.mp4' end end @@ -30,45 +95,50 @@ RSpec.shared_examples 'edits content using the content editor' do page.go_back refresh + switch_to_content_editor end it 'applies theme classes to code blocks' do expect(page).not_to have_css('.content-editor-code-block.code.highlight.dark') - find(content_editor_testid).send_keys [:enter, :enter] - find(content_editor_testid).send_keys '```js ' # trigger input rule - find(content_editor_testid).send_keys 'var a = 0' + type_in_content_editor [:enter, :enter] + type_in_content_editor '```js ' # trigger input rule + type_in_content_editor 'var a = 0' expect(page).to have_css('.content-editor-code-block.code.highlight.dark') end end describe 'code block bubble menu' do + before do + switch_to_content_editor + end + it 'shows a code block bubble menu for a code block' do - find(content_editor_testid).send_keys [:enter, :enter] + type_in_content_editor [:enter, :enter] - find(content_editor_testid).send_keys '```js ' # trigger input rule - find(content_editor_testid).send_keys 'var a = 0' - find(content_editor_testid).send_keys [:shift, :left] + type_in_content_editor '```js ' # trigger input rule + type_in_content_editor 'var a = 0' + type_in_content_editor [:shift, :left] - expect(page).not_to have_css('[data-testid="formatting-bubble-menu"]') + expect_formatting_menu_to_be_hidden expect(page).to have_css('[data-testid="code-block-bubble-menu"]') end it 'sets code block type to "javascript" for `js`' do - find(content_editor_testid).send_keys [:enter, :enter] + type_in_content_editor [:enter, :enter] - find(content_editor_testid).send_keys '```js ' - find(content_editor_testid).send_keys 'var a = 0' + type_in_content_editor '```js ' + type_in_content_editor 'var a = 0' expect(find('[data-testid="code-block-bubble-menu"]')).to have_text('Javascript') end it 'sets code block type to "Custom (nomnoml)" for `nomnoml`' do - find(content_editor_testid).send_keys [:enter, :enter] + type_in_content_editor [:enter, :enter] - find(content_editor_testid).send_keys '```nomnoml ' - find(content_editor_testid).send_keys 'test' + type_in_content_editor '```nomnoml ' + type_in_content_editor 'test' expect(find('[data-testid="code-block-bubble-menu"]')).to have_text('Custom (nomnoml)') end @@ -76,10 +146,11 @@ RSpec.shared_examples 'edits content using the content editor' do describe 'mermaid diagram' do before do - find(content_editor_testid).send_keys [:enter, :enter] + switch_to_content_editor - find(content_editor_testid).send_keys '```mermaid ' - find(content_editor_testid).send_keys ['graph TD;', :enter, ' JohnDoe12 --> HelloWorld34'] + type_in_content_editor [:enter, :enter] + type_in_content_editor '```mermaid ' + type_in_content_editor ['graph TD;', :enter, ' JohnDoe12 --> HelloWorld34'] end it 'renders and updates the diagram correctly in a sandboxed iframe' do diff --git a/spec/support/shared_examples/features/wiki/user_updates_wiki_page_shared_examples.rb b/spec/support/shared_examples/features/wiki/user_updates_wiki_page_shared_examples.rb index 87067336a36..5c63d6a973d 100644 --- a/spec/support/shared_examples/features/wiki/user_updates_wiki_page_shared_examples.rb +++ b/spec/support/shared_examples/features/wiki/user_updates_wiki_page_shared_examples.rb @@ -137,16 +137,7 @@ RSpec.shared_examples 'User updates wiki page' do end end - context 'when using the content editor' do - context 'with feature flag on' do - before do - find('[data-testid="toggle-editing-mode-button"] label', text: 'Rich text').click - end - - it_behaves_like 'edits content using the content editor' - end - end - + it_behaves_like 'edits content using the content editor' it_behaves_like 'autocompletes items' end diff --git a/spec/tasks/gitlab/snippets_rake_spec.rb b/spec/tasks/gitlab/snippets_rake_spec.rb index c55bded1d5a..c50b04b4600 100644 --- a/spec/tasks/gitlab/snippets_rake_spec.rb +++ b/spec/tasks/gitlab/snippets_rake_spec.rb @@ -3,7 +3,7 @@ require 'rake_helper' RSpec.describe 'gitlab:snippets namespace rake task', :silence_stdout do - let_it_be(:user) { create(:user)} + let_it_be(:user) { create(:user) } let_it_be(:migrated) { create(:personal_snippet, :repository, author: user) } let(:non_migrated) { create_list(:personal_snippet, 3, author: user) } diff --git a/spec/uploaders/packages/debian/distribution_release_file_uploader_spec.rb b/spec/uploaders/packages/debian/distribution_release_file_uploader_spec.rb index 203a453bcdd..dbbf69e3c8d 100644 --- a/spec/uploaders/packages/debian/distribution_release_file_uploader_spec.rb +++ b/spec/uploaders/packages/debian/distribution_release_file_uploader_spec.rb @@ -49,12 +49,12 @@ RSpec.describe Packages::Debian::DistributionReleaseFileUploader do end describe '#filename' do - it { expect(subject.filename).to eq('Release')} + it { expect(subject.filename).to eq('Release') } context 'with signed_file' do let(:uploader) { described_class.new(distribution, :signed_file) } - it { expect(subject.filename).to eq('InRelease')} + it { expect(subject.filename).to eq('InRelease') } end end end diff --git a/spec/validators/addressable_url_validator_spec.rb b/spec/validators/addressable_url_validator_spec.rb index b3a4459db30..9109a899881 100644 --- a/spec/validators/addressable_url_validator_spec.rb +++ b/spec/validators/addressable_url_validator_spec.rb @@ -245,7 +245,7 @@ RSpec.describe AddressableUrlValidator do end context 'when enforce_user is' do - let(:url) { 'http://$user@example.com'} + let(:url) { 'http://$user@example.com' } let(:validator) { described_class.new(attributes: [:link_url], enforce_user: enforce_user) } context 'true' do @@ -274,7 +274,7 @@ RSpec.describe AddressableUrlValidator do end context 'when ascii_only is' do - let(:url) { 'https://𝕘itⅼαƄ.com/foo/foo.bar'} + let(:url) { 'https://𝕘itⅼαƄ.com/foo/foo.bar' } let(:validator) { described_class.new(attributes: [:link_url], ascii_only: ascii_only) } context 'true' do diff --git a/spec/views/help/instance_configuration.html.haml_spec.rb b/spec/views/help/instance_configuration.html.haml_spec.rb index fbf84a5d272..4461eadf1a3 100644 --- a/spec/views/help/instance_configuration.html.haml_spec.rb +++ b/spec/views/help/instance_configuration.html.haml_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe 'help/instance_configuration' do describe 'General Sections:' do - let(:instance_configuration) { build(:instance_configuration)} + let(:instance_configuration) { build(:instance_configuration) } let(:settings) { instance_configuration.settings } let(:ssh_settings) { settings[:ssh_algorithms_hashes] } diff --git a/spec/views/layouts/_header_search.html.haml_spec.rb b/spec/views/layouts/_header_search.html.haml_spec.rb index 3ab4ae6a483..3a21bb3a92c 100644 --- a/spec/views/layouts/_header_search.html.haml_spec.rb +++ b/spec/views/layouts/_header_search.html.haml_spec.rb @@ -8,7 +8,7 @@ RSpec.describe 'layouts/_header_search' do let(:scope) { nil } let(:ref) { nil } let(:code_search) { false } - let(:for_snippets) { false} + let(:for_snippets) { false } let(:header_search_context) do { diff --git a/spec/views/layouts/_published_experiments.html.haml_spec.rb b/spec/views/layouts/_published_experiments.html.haml_spec.rb index 84894554bd9..072e4f2074e 100644 --- a/spec/views/layouts/_published_experiments.html.haml_spec.rb +++ b/spec/views/layouts/_published_experiments.html.haml_spec.rb @@ -13,10 +13,10 @@ RSpec.describe 'layouts/_published_experiments', :experiment do test_variant: :variant_name ) - experiment(:test_control) { } + experiment(:test_control) {} experiment(:test_excluded) { |e| e.exclude! } - experiment(:test_candidate) { |e| e.candidate { } } - experiment(:test_variant) { |e| e.variant(:variant_name) { } } + experiment(:test_candidate) { |e| e.candidate {} } + experiment(:test_variant) { |e| e.variant(:variant_name) {} } experiment(:test_published_only).publish render diff --git a/spec/views/shared/runners/_runner_details.html.haml_spec.rb b/spec/views/shared/runners/_runner_details.html.haml_spec.rb index cdf5ec563d0..978750c8435 100644 --- a/spec/views/shared/runners/_runner_details.html.haml_spec.rb +++ b/spec/views/shared/runners/_runner_details.html.haml_spec.rb @@ -113,14 +113,14 @@ RSpec.describe 'shared/runners/_runner_details.html.haml' do describe 'Tags value' do context 'when runner does not have tags' do it { is_expected.to have_content('Tags') } - it { is_expected.not_to have_selector('span.gl-badge.badge.badge-info')} + it { is_expected.not_to have_selector('span.gl-badge.badge.badge-info') } end context 'when runner have tags' do let(:runner) { create(:ci_runner, tag_list: %w(tag2 tag3 tag1)) } it { is_expected.to have_content('Tags tag1 tag2 tag3') } - it { is_expected.to have_selector('span.gl-badge.badge.badge-info')} + it { is_expected.to have_selector('span.gl-badge.badge.badge-info') } end end diff --git a/spec/workers/bulk_imports/export_request_worker_spec.rb b/spec/workers/bulk_imports/export_request_worker_spec.rb index 846df63a4d7..a7f7aaa7dba 100644 --- a/spec/workers/bulk_imports/export_request_worker_spec.rb +++ b/spec/workers/bulk_imports/export_request_worker_spec.rb @@ -60,7 +60,7 @@ RSpec.describe BulkImports::ExportRequestWorker do context 'when entity is group' do let(:entity) { create(:bulk_import_entity, :group_entity, source_full_path: 'foo/bar', bulk_import: bulk_import) } - let(:expected) { '/groups/foo%2Fbar/export_relations'} + let(:expected) { '/groups/foo%2Fbar/export_relations' } include_examples 'requests relations export for api resource' end diff --git a/spec/workers/clusters/cleanup/project_namespace_worker_spec.rb b/spec/workers/clusters/cleanup/project_namespace_worker_spec.rb index b9219586a0b..c24ca71eb35 100644 --- a/spec/workers/clusters/cleanup/project_namespace_worker_spec.rb +++ b/spec/workers/clusters/cleanup/project_namespace_worker_spec.rb @@ -18,7 +18,7 @@ RSpec.describe Clusters::Cleanup::ProjectNamespaceWorker do end context 'when exceeded the execution limit' do - subject { worker_instance.perform(cluster.id, worker_instance.send(:execution_limit))} + subject { worker_instance.perform(cluster.id, worker_instance.send(:execution_limit)) } it 'logs the error' do expect(logger).to receive(:error) diff --git a/spec/workers/packages/helm/extraction_worker_spec.rb b/spec/workers/packages/helm/extraction_worker_spec.rb index daebbda3077..70a090d6989 100644 --- a/spec/workers/packages/helm/extraction_worker_spec.rb +++ b/spec/workers/packages/helm/extraction_worker_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe Packages::Helm::ExtractionWorker, type: :worker do describe '#perform' do - let_it_be(:package) { create(:helm_package, without_package_files: true, status: 'processing')} + let_it_be(:package) { create(:helm_package, without_package_files: true, status: 'processing') } let!(:package_file) { create(:helm_package_file, without_loaded_metadatum: true, package: package) } let(:package_file_id) { package_file.id } diff --git a/spec/workers/pages_worker_spec.rb b/spec/workers/pages_worker_spec.rb index 5ddfd5b43b9..ad714d8d11e 100644 --- a/spec/workers/pages_worker_spec.rb +++ b/spec/workers/pages_worker_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe PagesWorker, :sidekiq_inline do let(:project) { create(:project) } - let(:ci_build) { create(:ci_build, project: project)} + let(:ci_build) { create(:ci_build, project: project) } it 'calls UpdatePagesService' do expect_next_instance_of(Projects::UpdatePagesService, project, ci_build) do |service| diff --git a/spec/workers/purge_dependency_proxy_cache_worker_spec.rb b/spec/workers/purge_dependency_proxy_cache_worker_spec.rb index 3de59670f8d..84315fd6ee9 100644 --- a/spec/workers/purge_dependency_proxy_cache_worker_spec.rb +++ b/spec/workers/purge_dependency_proxy_cache_worker_spec.rb @@ -4,9 +4,9 @@ require 'spec_helper' RSpec.describe PurgeDependencyProxyCacheWorker do let_it_be(:user) { create(:admin) } - let_it_be_with_refind(:blob) { create(:dependency_proxy_blob )} + let_it_be_with_refind(:blob) { create(:dependency_proxy_blob ) } let_it_be_with_reload(:group) { blob.group } - let_it_be_with_refind(:manifest) { create(:dependency_proxy_manifest, group: group )} + let_it_be_with_refind(:manifest) { create(:dependency_proxy_manifest, group: group ) } let_it_be(:group_id) { group.id } subject { described_class.new.perform(user.id, group_id) } diff --git a/spec/workers/releases/manage_evidence_worker_spec.rb b/spec/workers/releases/manage_evidence_worker_spec.rb index 2fbfb6c9dc1..886fcd346eb 100644 --- a/spec/workers/releases/manage_evidence_worker_spec.rb +++ b/spec/workers/releases/manage_evidence_worker_spec.rb @@ -29,7 +29,7 @@ RSpec.describe Releases::ManageEvidenceWorker do context 'when evidence has already been created' do let(:release) { create(:release, project: project, released_at: 1.hour.since) } - let!(:evidence) { create(:evidence, release: release )} + let!(:evidence) { create(:evidence, release: release ) } it_behaves_like 'does not create a new Evidence record' end