diff options
50 files changed, 396 insertions, 267 deletions
diff --git a/app/assets/javascripts/alert_management/components/alert_management_list.vue b/app/assets/javascripts/alert_management/components/alert_management_list.vue index 74fc19ff3d4..1909840ed19 100644 --- a/app/assets/javascripts/alert_management/components/alert_management_list.vue +++ b/app/assets/javascripts/alert_management/components/alert_management_list.vue @@ -60,8 +60,8 @@ export default { { key: 'eventCount', label: s__('AlertManagement|Events'), - thClass: 'text-right event-count', - tdClass: `${tdClass} text-md-right event-count`, + thClass: 'text-right gl-pr-9', + tdClass: `${tdClass} text-md-right`, }, { key: 'status', diff --git a/app/assets/javascripts/gl_dropdown.js b/app/assets/javascripts/gl_dropdown.js index be4b4b5f87d..ec0d0cf6aef 100644 --- a/app/assets/javascripts/gl_dropdown.js +++ b/app/assets/javascripts/gl_dropdown.js @@ -4,7 +4,7 @@ import $ from 'jquery'; import { escape } from 'lodash'; import fuzzaldrinPlus from 'fuzzaldrin-plus'; import axios from './lib/utils/axios_utils'; -import { visitUrl } from './lib/utils/url_utility'; +import { visitUrl } from '~/lib/utils/url_utility'; import { isObject } from './lib/utils/type_utility'; import renderItem from './gl_dropdown/render'; diff --git a/app/assets/javascripts/pager.js b/app/assets/javascripts/pager.js index 46e80ba72e3..4b5a645ca5f 100644 --- a/app/assets/javascripts/pager.js +++ b/app/assets/javascripts/pager.js @@ -1,7 +1,7 @@ import $ from 'jquery'; import { getParameterByName } from '~/lib/utils/common_utils'; -import axios from './lib/utils/axios_utils'; -import { removeParams } from './lib/utils/url_utility'; +import axios from '~/lib/utils/axios_utils'; +import { removeParams } from '~/lib/utils/url_utility'; const ENDLESS_SCROLL_BOTTOM_PX = 400; const ENDLESS_SCROLL_FIRE_DELAY_MS = 1000; diff --git a/app/assets/stylesheets/pages/alert_management/list.scss b/app/assets/stylesheets/pages/alert_management/list.scss index dc181342def..c5930a087c9 100644 --- a/app/assets/stylesheets/pages/alert_management/list.scss +++ b/app/assets/stylesheets/pages/alert_management/list.scss @@ -30,10 +30,6 @@ th { @include gl-p-5; border: 0; // Remove cell border styling so that we can set border styling per row - - &.event-count { - @include gl-pr-9; - } } th { diff --git a/app/controllers/projects/alert_management_controller.rb b/app/controllers/projects/alert_management_controller.rb index 0c0a91e136f..32e0b2c3fb6 100644 --- a/app/controllers/projects/alert_management_controller.rb +++ b/app/controllers/projects/alert_management_controller.rb @@ -5,6 +5,7 @@ class Projects::AlertManagementController < Projects::ApplicationController before_action do push_frontend_feature_flag(:alert_list_status_filtering_enabled) push_frontend_feature_flag(:create_issue_from_alert_enabled) + push_frontend_feature_flag(:alert_assignee, project) end def index diff --git a/app/graphql/mutations/base_mutation.rb b/app/graphql/mutations/base_mutation.rb index 30510cfab50..33f3f33a440 100644 --- a/app/graphql/mutations/base_mutation.rb +++ b/app/graphql/mutations/base_mutation.rb @@ -9,7 +9,7 @@ module Mutations field :errors, [GraphQL::STRING_TYPE], null: false, - description: "Errors encountered during execution of the mutation." + description: 'Errors encountered during execution of the mutation.' def current_user context[:current_user] diff --git a/app/graphql/mutations/issues/set_confidential.rb b/app/graphql/mutations/issues/set_confidential.rb index 0fff5518665..75befddc261 100644 --- a/app/graphql/mutations/issues/set_confidential.rb +++ b/app/graphql/mutations/issues/set_confidential.rb @@ -19,7 +19,7 @@ module Mutations { issue: issue, - errors: issue.errors.full_messages + errors: errors_on_object(issue) } end end diff --git a/app/graphql/mutations/issues/set_due_date.rb b/app/graphql/mutations/issues/set_due_date.rb index 1855c6f053b..effd863c541 100644 --- a/app/graphql/mutations/issues/set_due_date.rb +++ b/app/graphql/mutations/issues/set_due_date.rb @@ -19,7 +19,7 @@ module Mutations { issue: issue, - errors: issue.errors.full_messages + errors: errors_on_object(issue) } end end diff --git a/app/graphql/mutations/issues/update.rb b/app/graphql/mutations/issues/update.rb index 3710144fff5..7f6d9b0f988 100644 --- a/app/graphql/mutations/issues/update.rb +++ b/app/graphql/mutations/issues/update.rb @@ -33,7 +33,7 @@ module Mutations { issue: issue, - errors: issue.errors.full_messages + errors: errors_on_object(issue) } end end diff --git a/app/graphql/mutations/merge_requests/set_assignees.rb b/app/graphql/mutations/merge_requests/set_assignees.rb index 8f0025f0a58..de244b62d0f 100644 --- a/app/graphql/mutations/merge_requests/set_assignees.rb +++ b/app/graphql/mutations/merge_requests/set_assignees.rb @@ -40,7 +40,7 @@ module Mutations { merge_request: merge_request, - errors: merge_request.errors.full_messages + errors: errors_on_object(merge_request) } end end diff --git a/app/graphql/mutations/merge_requests/set_labels.rb b/app/graphql/mutations/merge_requests/set_labels.rb index 71f7a353bc9..9560989a421 100644 --- a/app/graphql/mutations/merge_requests/set_labels.rb +++ b/app/graphql/mutations/merge_requests/set_labels.rb @@ -41,7 +41,7 @@ module Mutations { merge_request: merge_request, - errors: merge_request.errors.full_messages + errors: errors_on_object(merge_request) } end diff --git a/app/graphql/mutations/merge_requests/set_locked.rb b/app/graphql/mutations/merge_requests/set_locked.rb index 09aaa0b39aa..c49d5186a03 100644 --- a/app/graphql/mutations/merge_requests/set_locked.rb +++ b/app/graphql/mutations/merge_requests/set_locked.rb @@ -21,7 +21,7 @@ module Mutations { merge_request: merge_request, - errors: merge_request.errors.full_messages + errors: errors_on_object(merge_request) } end end diff --git a/app/graphql/mutations/merge_requests/set_milestone.rb b/app/graphql/mutations/merge_requests/set_milestone.rb index 707d6677952..b3412dd9ed2 100644 --- a/app/graphql/mutations/merge_requests/set_milestone.rb +++ b/app/graphql/mutations/merge_requests/set_milestone.rb @@ -22,7 +22,7 @@ module Mutations { merge_request: merge_request, - errors: merge_request.errors.full_messages + errors: errors_on_object(merge_request) } end end diff --git a/app/graphql/mutations/merge_requests/set_subscription.rb b/app/graphql/mutations/merge_requests/set_subscription.rb index 86750152775..1535481ab37 100644 --- a/app/graphql/mutations/merge_requests/set_subscription.rb +++ b/app/graphql/mutations/merge_requests/set_subscription.rb @@ -18,7 +18,7 @@ module Mutations { merge_request: merge_request, - errors: merge_request.errors.full_messages + errors: errors_on_object(merge_request) } end end diff --git a/app/graphql/mutations/merge_requests/set_wip.rb b/app/graphql/mutations/merge_requests/set_wip.rb index a2aa0c84ee4..5d2077c12f2 100644 --- a/app/graphql/mutations/merge_requests/set_wip.rb +++ b/app/graphql/mutations/merge_requests/set_wip.rb @@ -21,7 +21,7 @@ module Mutations { merge_request: merge_request, - errors: merge_request.errors.full_messages + errors: errors_on_object(merge_request) } end diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 7f64ea7dd97..645b87ce68c 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -576,7 +576,7 @@ module Ci def environment_changed_page_variables Gitlab::Ci::Variables::Collection.new.tap do |variables| - break variables unless environment_status + break variables unless environment_status && Feature.enabled?(:modifed_path_ci_variables, project) variables.append(key: 'CI_MERGE_REQUEST_CHANGED_PAGE_PATHS', value: environment_status.changed_paths.join(',')) variables.append(key: 'CI_MERGE_REQUEST_CHANGED_PAGE_URLS', value: environment_status.changed_urls.join(',')) diff --git a/app/models/project.rb b/app/models/project.rb index c0dd2eb8584..ff39218460d 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1273,7 +1273,7 @@ class Project < ApplicationRecord template = find_service(services_templates, name) if template - Service.build_from_template(id, template) + Service.build_from_integration(id, template) else public_send("build_#{name}_service") # rubocop:disable GitlabSecurity/PublicSend end diff --git a/app/models/service.rb b/app/models/service.rb index fb4d9a77077..396c0c530ab 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -335,17 +335,18 @@ class Service < ApplicationRecord services_names.map { |service_name| "#{service_name}_service".camelize } end - def self.build_from_template(project_id, template) - service = template.dup + def self.build_from_integration(project_id, integration) + service = integration.dup - if template.supports_data_fields? - data_fields = template.data_fields.dup + if integration.supports_data_fields? + data_fields = integration.data_fields.dup data_fields.service = service end service.template = false + service.instance = false service.project_id = project_id - service.active = false if service.active? && service.invalid? + service.active = false if service.invalid? service end diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index 3233d1799b8..8acc83a0d27 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -178,7 +178,7 @@ module Projects # rubocop: disable CodeReuse/ActiveRecord def create_services_from_active_templates(project) Service.where(template: true, active: true).each do |template| - service = Service.build_from_template(project.id, template) + service = Service.build_from_integration(project.id, template) service.save! end end diff --git a/app/services/projects/update_pages_service.rb b/app/services/projects/update_pages_service.rb index 59389a0fa65..7bebaca684a 100644 --- a/app/services/projects/update_pages_service.rb +++ b/app/services/projects/update_pages_service.rb @@ -2,6 +2,8 @@ module Projects class UpdatePagesService < BaseService + include Gitlab::OptimisticLocking + InvalidStateError = Class.new(StandardError) FailedToExtractError = Class.new(StandardError) @@ -23,8 +25,8 @@ module Projects # Create status notifying the deployment of pages @status = create_status - @status.enqueue! - @status.run! + retry_optimistic_lock(@status, &:enqueue!) + retry_optimistic_lock(@status, &:run!) raise InvalidStateError, 'missing pages artifacts' unless build.artifacts? raise InvalidStateError, 'build SHA is outdated for this ref' unless latest? @@ -51,7 +53,7 @@ module Projects private def success - @status.success + retry_optimistic_lock(@status, &:success) @project.mark_pages_as_deployed super end @@ -61,7 +63,7 @@ module Projects log_error("Projects::UpdatePagesService: #{message}") @status.allow_failure = !latest? @status.description = message - @status.drop(:script_failure) + retry_optimistic_lock(@status) { |status| status.drop(:script_failure) } super end diff --git a/changelogs/unreleased/alert-management-mobile-alignment.yml b/changelogs/unreleased/alert-management-mobile-alignment.yml new file mode 100644 index 00000000000..04f2c3224ff --- /dev/null +++ b/changelogs/unreleased/alert-management-mobile-alignment.yml @@ -0,0 +1,5 @@ +--- +title: Update alert management mobile table alignment +merge_request: 32295 +author: +type: other diff --git a/changelogs/unreleased/osw-add-redis-metrics-to-sidekiq-job-run.yml b/changelogs/unreleased/osw-add-redis-metrics-to-sidekiq-job-run.yml new file mode 100644 index 00000000000..88efab2233c --- /dev/null +++ b/changelogs/unreleased/osw-add-redis-metrics-to-sidekiq-job-run.yml @@ -0,0 +1,5 @@ +--- +title: Add metrics for Redis usage during Sidekiq job execution +merge_request: 32265 +author: +type: added diff --git a/db/migrate/20200519115908_add_epics_confidential_index.rb b/db/migrate/20200519115908_add_epics_confidential_index.rb new file mode 100644 index 00000000000..68a1715acb4 --- /dev/null +++ b/db/migrate/20200519115908_add_epics_confidential_index.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddEpicsConfidentialIndex < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :epics, :confidential + end + + def down + remove_concurrent_index :epics, :confidential + end +end diff --git a/db/structure.sql b/db/structure.sql index 38a8f98a1f3..98a111df419 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -9582,6 +9582,8 @@ CREATE INDEX index_epics_on_author_id ON public.epics USING btree (author_id); CREATE INDEX index_epics_on_closed_by_id ON public.epics USING btree (closed_by_id); +CREATE INDEX index_epics_on_confidential ON public.epics USING btree (confidential); + CREATE INDEX index_epics_on_due_date_sourcing_epic_id ON public.epics USING btree (due_date_sourcing_epic_id) WHERE (due_date_sourcing_epic_id IS NOT NULL); CREATE INDEX index_epics_on_due_date_sourcing_milestone_id ON public.epics USING btree (due_date_sourcing_milestone_id); @@ -13867,5 +13869,6 @@ COPY "schema_migrations" (version) FROM STDIN; 20200514000132 20200514000340 20200515155620 +20200519115908 \. diff --git a/doc/administration/monitoring/prometheus/gitlab_metrics.md b/doc/administration/monitoring/prometheus/gitlab_metrics.md index f725db9a039..65eacefdffc 100644 --- a/doc/administration/monitoring/prometheus/gitlab_metrics.md +++ b/doc/administration/monitoring/prometheus/gitlab_metrics.md @@ -123,9 +123,11 @@ configuration option in `gitlab.yml`. These metrics are served from the | `sidekiq_jobs_completion_seconds` | Histogram | 12.2 | Seconds to complete Sidekiq job | `queue`, `boundary`, `external_dependencies`, `feature_category`, `job_status`, `urgency` | | `sidekiq_jobs_db_seconds` | Histogram | 12.9 | Seconds of DB time to run Sidekiq job | `queue`, `boundary`, `external_dependencies`, `feature_category`, `job_status`, `urgency` | | `sidekiq_jobs_gitaly_seconds` | Histogram | 12.9 | Seconds of Gitaly time to run Sidekiq job | `queue`, `boundary`, `external_dependencies`, `feature_category`, `job_status`, `urgency` | +| `sidekiq_redis_requests_duration_seconds` | Histogram | 13.1 | Duration in seconds that a Sidekiq job spent querying a Redis server | `queue`, `boundary`, `external_dependencies`, `feature_category`, `job_status`, `urgency` | | `sidekiq_jobs_queue_duration_seconds` | Histogram | 12.5 | Duration in seconds that a Sidekiq job was queued before being executed | `queue`, `boundary`, `external_dependencies`, `feature_category`, `urgency` | | `sidekiq_jobs_failed_total` | Counter | 12.2 | Sidekiq jobs failed | `queue`, `boundary`, `external_dependencies`, `feature_category`, `urgency` | | `sidekiq_jobs_retried_total` | Counter | 12.2 | Sidekiq jobs retried | `queue`, `boundary`, `external_dependencies`, `feature_category`, `urgency` | +| `sidekiq_redis_requests_total` | Counter | 13.1 | Redis requests during a Sidekiq job execution | `queue`, `boundary`, `external_dependencies`, `feature_category`, `job_status`, `urgency` | | `sidekiq_running_jobs` | Gauge | 12.2 | Number of Sidekiq jobs running | `queue`, `boundary`, `external_dependencies`, `feature_category`, `urgency` | | `sidekiq_concurrency` | Gauge | 12.5 | Maximum number of Sidekiq jobs | | | `geo_db_replication_lag_seconds` | Gauge | 10.2 | Database replication lag (seconds) | `url` | diff --git a/doc/development/api_graphql_styleguide.md b/doc/development/api_graphql_styleguide.md index 6d3c0cf0eac..9ab5a5967a6 100644 --- a/doc/development/api_graphql_styleguide.md +++ b/doc/development/api_graphql_styleguide.md @@ -690,8 +690,9 @@ should look like this: # The merge request modified, this will be wrapped in the type # defined on the field merge_request: merge_request, - # An array if strings if the mutation failed after authorization - errors: merge_request.errors.full_messages + # An array of strings if the mutation failed after authorization. + # The `errors_on_object` helper collects `errors.full_messages` + errors: errors_on_object(merge_request) } ``` diff --git a/doc/development/code_review.md b/doc/development/code_review.md index a5ad7dc0f46..9ebdc81fd91 100644 --- a/doc/development/code_review.md +++ b/doc/development/code_review.md @@ -308,10 +308,11 @@ experience, refactors the existing code). Then: - Seek to understand the author's perspective. - If you don't understand a piece of code, _say so_. There's a good chance someone else would be confused by it as well. -- Do prefix your comment with "Not blocking:" if you have a small, - non-mandatory improvement you wish to suggest. This lets the author - know that they can optionally resolve this issue in this merge request - or follow-up at a later stage. +- Ensure the author is clear on what is required from them to address/resolve the suggestion. + - Consider using the [Conventional Comment format](https://conventionalcomments.org#format) to + convey your intent. + - For non-mandatory suggestions, decorate with (non-blocking) so the author knows they can + optionally resolve within the merge request or follow-up at a later stage. - After a round of line notes, it can be helpful to post a summary note such as "LGTM :thumbsup:", or "Just a couple things to address." - Assign the merge request to the author if changes are required following your diff --git a/doc/development/internal_api.md b/doc/development/internal_api.md index 5b53f223eb0..731325d930c 100644 --- a/doc/development/internal_api.md +++ b/doc/development/internal_api.md @@ -43,7 +43,7 @@ POST /internal/allowed | `key_id` | string | no | ID of the SSH-key used to connect to GitLab-shell | | `username` | string | no | Username from the certificate used to connect to GitLab-Shell | | `project` | string | no (if `gl_repository` is passed) | Path to the project | -| `gl_repository` | string | no (if `project` is passed) | Path to the project | +| `gl_repository` | string | no (if `project` is passed) | Repository identifier (e.g. `project-7`) | | `protocol` | string | yes | SSH when called from GitLab-shell, HTTP or SSH when called from Gitaly | | `action` | string | yes | Git command being run (`git-upload-pack`, `git-receive-pack`, `git-upload-archive`) | | `changes` | string | yes | `<oldrev> <newrev> <refname>` when called from Gitaly, The magic string `_any` when called from GitLab Shell | diff --git a/doc/user/group/roadmap/index.md b/doc/user/group/roadmap/index.md index 6bee552d433..e059fee2651 100644 --- a/doc/user/group/roadmap/index.md +++ b/doc/user/group/roadmap/index.md @@ -7,8 +7,8 @@ type: reference > - Introduced in [GitLab Ultimate](https://about.gitlab.com/pricing/) 10.5. > - In [GitLab 12.9](https://gitlab.com/gitlab-org/gitlab/issues/198062), Roadmaps were moved to the Premium tier. > - In [GitLab 12.9](https://gitlab.com/gitlab-org/gitlab/issues/5164) and later, the epic bars show epics' title, progress, and completed weight percentage. -> - Milestones appear in Roadmaps in [GitLab 12.10](https://gitlab.com/gitlab-org/gitlab/-/issues/6802), and later. -> - Feature flag removed in [GitLab 13.0](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/29641). +> - Milestones appear in roadmaps in [GitLab 12.10](https://gitlab.com/gitlab-org/gitlab/-/issues/6802), and later. +> - Feature flag for milestones visible in roadmaps removed in [GitLab 13.0](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/29641). Epics and milestones within a group containing **Start date** and/or **Due date** can be visualized in a form of a timeline (that is, a Gantt chart). The Roadmap page diff --git a/lib/api/commit_statuses.rb b/lib/api/commit_statuses.rb index b4c5d7869a2..fd09e2d4ee6 100644 --- a/lib/api/commit_statuses.rb +++ b/lib/api/commit_statuses.rb @@ -106,7 +106,7 @@ module API status.enqueue! when 'running' status.enqueue - status.run! + Gitlab::OptimisticLocking.retry_lock(status, &:run!) when 'success' status.success! when 'failed' diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index c6f6dc255d4..bbdb45da3b1 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -437,7 +437,7 @@ module API if report_exception?(exception) define_params_for_grape_middleware Gitlab::ErrorTracking.with_context(current_user) do - Gitlab::ErrorTracking.track_exception(exception, params) + Gitlab::ErrorTracking.track_exception(exception) end end diff --git a/lib/gitlab/instrumentation/redis.rb b/lib/gitlab/instrumentation/redis.rb index cc99e828251..902de55bc86 100644 --- a/lib/gitlab/instrumentation/redis.rb +++ b/lib/gitlab/instrumentation/redis.rb @@ -24,6 +24,9 @@ module Gitlab REDIS_CALL_DURATION = :redis_call_duration REDIS_CALL_DETAILS = :redis_call_details + # Milliseconds represented in seconds (from 1 to 500 milliseconds). + QUERY_TIME_BUCKETS = [0.001, 0.0025, 0.005, 0.01, 0.025, 0.05, 0.1, 0.25, 0.5].freeze + def self.get_request_count ::RequestStore[REDIS_REQUEST_COUNT] || 0 end diff --git a/lib/gitlab/sidekiq_middleware/server_metrics.rb b/lib/gitlab/sidekiq_middleware/server_metrics.rb index 61ed2fe1a06..00dd3b5937a 100644 --- a/lib/gitlab/sidekiq_middleware/server_metrics.rb +++ b/lib/gitlab/sidekiq_middleware/server_metrics.rb @@ -47,6 +47,8 @@ module Gitlab @metrics[:sidekiq_jobs_completion_seconds].observe(labels, monotonic_time) @metrics[:sidekiq_jobs_db_seconds].observe(labels, ActiveRecord::LogSubscriber.runtime / 1000) @metrics[:sidekiq_jobs_gitaly_seconds].observe(labels, get_gitaly_time(job)) + @metrics[:sidekiq_redis_requests_total].increment(labels, get_redis_calls(job)) + @metrics[:sidekiq_redis_requests_duration_seconds].observe(labels, get_redis_time(job)) end end @@ -54,15 +56,17 @@ module Gitlab def init_metrics { - sidekiq_jobs_cpu_seconds: ::Gitlab::Metrics.histogram(:sidekiq_jobs_cpu_seconds, 'Seconds of cpu time to run Sidekiq job', {}, SIDEKIQ_LATENCY_BUCKETS), - sidekiq_jobs_completion_seconds: ::Gitlab::Metrics.histogram(:sidekiq_jobs_completion_seconds, 'Seconds to complete Sidekiq job', {}, SIDEKIQ_LATENCY_BUCKETS), - sidekiq_jobs_db_seconds: ::Gitlab::Metrics.histogram(:sidekiq_jobs_db_seconds, 'Seconds of database time to run Sidekiq job', {}, SIDEKIQ_LATENCY_BUCKETS), - sidekiq_jobs_gitaly_seconds: ::Gitlab::Metrics.histogram(:sidekiq_jobs_gitaly_seconds, 'Seconds of Gitaly time to run Sidekiq job', {}, SIDEKIQ_LATENCY_BUCKETS), - sidekiq_jobs_queue_duration_seconds: ::Gitlab::Metrics.histogram(:sidekiq_jobs_queue_duration_seconds, 'Duration in seconds that a Sidekiq job was queued before being executed', {}, SIDEKIQ_LATENCY_BUCKETS), - sidekiq_jobs_failed_total: ::Gitlab::Metrics.counter(:sidekiq_jobs_failed_total, 'Sidekiq jobs failed'), - sidekiq_jobs_retried_total: ::Gitlab::Metrics.counter(:sidekiq_jobs_retried_total, 'Sidekiq jobs retried'), - sidekiq_running_jobs: ::Gitlab::Metrics.gauge(:sidekiq_running_jobs, 'Number of Sidekiq jobs running', {}, :all), - sidekiq_concurrency: ::Gitlab::Metrics.gauge(:sidekiq_concurrency, 'Maximum number of Sidekiq jobs', {}, :all) + sidekiq_jobs_cpu_seconds: ::Gitlab::Metrics.histogram(:sidekiq_jobs_cpu_seconds, 'Seconds of cpu time to run Sidekiq job', {}, SIDEKIQ_LATENCY_BUCKETS), + sidekiq_jobs_completion_seconds: ::Gitlab::Metrics.histogram(:sidekiq_jobs_completion_seconds, 'Seconds to complete Sidekiq job', {}, SIDEKIQ_LATENCY_BUCKETS), + sidekiq_jobs_db_seconds: ::Gitlab::Metrics.histogram(:sidekiq_jobs_db_seconds, 'Seconds of database time to run Sidekiq job', {}, SIDEKIQ_LATENCY_BUCKETS), + sidekiq_jobs_gitaly_seconds: ::Gitlab::Metrics.histogram(:sidekiq_jobs_gitaly_seconds, 'Seconds of Gitaly time to run Sidekiq job', {}, SIDEKIQ_LATENCY_BUCKETS), + sidekiq_jobs_queue_duration_seconds: ::Gitlab::Metrics.histogram(:sidekiq_jobs_queue_duration_seconds, 'Duration in seconds that a Sidekiq job was queued before being executed', {}, SIDEKIQ_LATENCY_BUCKETS), + sidekiq_redis_requests_duration_seconds: ::Gitlab::Metrics.histogram(:sidekiq_redis_requests_duration_seconds, 'Duration in seconds that a Sidekiq job spent requests a Redis server', {}, Gitlab::Instrumentation::Redis::QUERY_TIME_BUCKETS), + sidekiq_jobs_failed_total: ::Gitlab::Metrics.counter(:sidekiq_jobs_failed_total, 'Sidekiq jobs failed'), + sidekiq_jobs_retried_total: ::Gitlab::Metrics.counter(:sidekiq_jobs_retried_total, 'Sidekiq jobs retried'), + sidekiq_redis_requests_total: ::Gitlab::Metrics.counter(:sidekiq_redis_requests_total, 'Redis requests during a Sidekiq job execution'), + sidekiq_running_jobs: ::Gitlab::Metrics.gauge(:sidekiq_running_jobs, 'Number of Sidekiq jobs running', {}, :all), + sidekiq_concurrency: ::Gitlab::Metrics.gauge(:sidekiq_concurrency, 'Maximum number of Sidekiq jobs', {}, :all) } end @@ -70,6 +74,14 @@ module Gitlab defined?(Process::CLOCK_THREAD_CPUTIME_ID) ? Process.clock_gettime(Process::CLOCK_THREAD_CPUTIME_ID) : 0 end + def get_redis_time(job) + job.fetch(:redis_duration_s, 0) + end + + def get_redis_calls(job) + job.fetch(:redis_calls, 0) + end + def get_gitaly_time(job) job.fetch(:gitaly_duration_s, 0) end diff --git a/spec/fixtures/trace/sample_trace b/spec/fixtures/trace/sample_trace index e9d1e79fc71..ebd2853e558 100644 --- a/spec/fixtures/trace/sample_trace +++ b/spec/fixtures/trace/sample_trace @@ -2736,9 +2736,6 @@ Service when repository is empty test runs execute Template - .build_from_template - when template is invalid - sets service template to inactive when template is invalid for pushover service is prefilled for projects pushover service has all fields prefilled diff --git a/spec/javascripts/gl_dropdown_spec.js b/spec/frontend/gl_dropdown_spec.js index 06f76c581f2..8bfe7f56e37 100644 --- a/spec/javascripts/gl_dropdown_spec.js +++ b/spec/frontend/gl_dropdown_spec.js @@ -1,19 +1,22 @@ /* eslint-disable no-param-reassign */ import $ from 'jquery'; -import GLDropdown from '~/gl_dropdown'; +import '~/gl_dropdown'; import '~/lib/utils/common_utils'; +import { visitUrl } from '~/lib/utils/url_utility'; -describe('glDropdown', function describeDropdown() { +jest.mock('~/lib/utils/url_utility', () => ({ + visitUrl: jest.fn().mockName('visitUrl'), +})); + +describe('glDropdown', () => { preloadFixtures('static/gl_dropdown.html'); - loadJSONFixtures('static/projects.json'); const NON_SELECTABLE_CLASSES = '.divider, .separator, .dropdown-header, .dropdown-menu-empty-item'; const SEARCH_INPUT_SELECTOR = '.dropdown-input-field'; const ITEM_SELECTOR = `.dropdown-content li:not(${NON_SELECTABLE_CLASSES})`; const FOCUSED_ITEM_SELECTOR = `${ITEM_SELECTOR} a.is-focused`; - const ARROW_KEYS = { DOWN: 40, UP: 38, @@ -23,7 +26,9 @@ describe('glDropdown', function describeDropdown() { let remoteCallback; - const navigateWithKeys = function navigateWithKeys(direction, steps, cb, i) { + const test = {}; + + const navigateWithKeys = (direction, steps, cb, i) => { i = i || 0; if (!i) direction = direction.toUpperCase(); $('body').trigger({ @@ -39,7 +44,7 @@ describe('glDropdown', function describeDropdown() { } }; - const remoteMock = function remoteMock(data, term, callback) { + const remoteMock = (data, term, callback) => { remoteCallback = callback.bind({}, data); }; @@ -47,7 +52,7 @@ describe('glDropdown', function describeDropdown() { const options = { selectable: true, filterable: isFilterable, - data: hasRemote ? remoteMock.bind({}, this.projectsData) : this.projectsData, + data: hasRemote ? remoteMock.bind({}, test.projectsData) : test.projectsData, search: { fields: ['name'], }, @@ -55,52 +60,52 @@ describe('glDropdown', function describeDropdown() { id: project => project.id, ...extraOpts, }; - this.dropdownButtonElement = $( + test.dropdownButtonElement = $( '#js-project-dropdown', - this.dropdownContainerElement, + test.dropdownContainerElement, ).glDropdown(options); } beforeEach(() => { loadFixtures('static/gl_dropdown.html'); - this.dropdownContainerElement = $('.dropdown.inline'); - this.$dropdownMenuElement = $('.dropdown-menu', this.dropdownContainerElement); - this.projectsData = getJSONFixture('static/projects.json'); + test.dropdownContainerElement = $('.dropdown.inline'); + test.$dropdownMenuElement = $('.dropdown-menu', test.dropdownContainerElement); + test.projectsData = getJSONFixture('static/projects.json'); }); afterEach(() => { $('body').off('keydown'); - this.dropdownContainerElement.off('keyup'); + test.dropdownContainerElement.off('keyup'); }); it('should open on click', () => { initDropDown.call(this, false); - expect(this.dropdownContainerElement).not.toHaveClass('show'); - this.dropdownButtonElement.click(); + expect(test.dropdownContainerElement).not.toHaveClass('show'); + test.dropdownButtonElement.click(); - expect(this.dropdownContainerElement).toHaveClass('show'); + expect(test.dropdownContainerElement).toHaveClass('show'); }); it('escapes HTML as text', () => { - this.projectsData[0].name_with_namespace = '<script>alert("testing");</script>'; + test.projectsData[0].name_with_namespace = '<script>alert("testing");</script>'; initDropDown.call(this, false); - this.dropdownButtonElement.click(); + test.dropdownButtonElement.click(); expect($('.dropdown-content li:first-child').text()).toBe('<script>alert("testing");</script>'); }); it('should output HTML when highlighting', () => { - this.projectsData[0].name_with_namespace = 'testing'; + test.projectsData[0].name_with_namespace = 'testing'; $('.dropdown-input .dropdown-input-field').val('test'); initDropDown.call(this, false, true, { highlight: true, }); - this.dropdownButtonElement.click(); + test.dropdownButtonElement.click(); expect($('.dropdown-content li:first-child').text()).toBe('testing'); @@ -112,31 +117,31 @@ describe('glDropdown', function describeDropdown() { describe('that is open', () => { beforeEach(() => { initDropDown.call(this, false, false); - this.dropdownButtonElement.click(); + test.dropdownButtonElement.click(); }); it('should select a following item on DOWN keypress', () => { - expect($(FOCUSED_ITEM_SELECTOR, this.$dropdownMenuElement).length).toBe(0); - const randomIndex = Math.floor(Math.random() * (this.projectsData.length - 1)) + 0; + expect($(FOCUSED_ITEM_SELECTOR, test.$dropdownMenuElement).length).toBe(0); + const randomIndex = Math.floor(Math.random() * (test.projectsData.length - 1)) + 0; navigateWithKeys('down', randomIndex, () => { - expect($(FOCUSED_ITEM_SELECTOR, this.$dropdownMenuElement).length).toBe(1); - expect($(`${ITEM_SELECTOR}:eq(${randomIndex}) a`, this.$dropdownMenuElement)).toHaveClass( + expect($(FOCUSED_ITEM_SELECTOR, test.$dropdownMenuElement).length).toBe(1); + expect($(`${ITEM_SELECTOR}:eq(${randomIndex}) a`, test.$dropdownMenuElement)).toHaveClass( 'is-focused', ); }); }); it('should select a previous item on UP keypress', () => { - expect($(FOCUSED_ITEM_SELECTOR, this.$dropdownMenuElement).length).toBe(0); - navigateWithKeys('down', this.projectsData.length - 1, () => { - expect($(FOCUSED_ITEM_SELECTOR, this.$dropdownMenuElement).length).toBe(1); - const randomIndex = Math.floor(Math.random() * (this.projectsData.length - 2)) + 0; + expect($(FOCUSED_ITEM_SELECTOR, test.$dropdownMenuElement).length).toBe(0); + navigateWithKeys('down', test.projectsData.length - 1, () => { + expect($(FOCUSED_ITEM_SELECTOR, test.$dropdownMenuElement).length).toBe(1); + const randomIndex = Math.floor(Math.random() * (test.projectsData.length - 2)) + 0; navigateWithKeys('up', randomIndex, () => { - expect($(FOCUSED_ITEM_SELECTOR, this.$dropdownMenuElement).length).toBe(1); + expect($(FOCUSED_ITEM_SELECTOR, test.$dropdownMenuElement).length).toBe(1); expect( $( - `${ITEM_SELECTOR}:eq(${this.projectsData.length - 2 - randomIndex}) a`, - this.$dropdownMenuElement, + `${ITEM_SELECTOR}:eq(${test.projectsData.length - 2 - randomIndex}) a`, + test.$dropdownMenuElement, ), ).toHaveClass('is-focused'); }); @@ -144,13 +149,12 @@ describe('glDropdown', function describeDropdown() { }); it('should click the selected item on ENTER keypress', () => { - expect(this.dropdownContainerElement).toHaveClass('show'); - const randomIndex = Math.floor(Math.random() * (this.projectsData.length - 1)) + 0; + expect(test.dropdownContainerElement).toHaveClass('show'); + const randomIndex = Math.floor(Math.random() * (test.projectsData.length - 1)) + 0; navigateWithKeys('down', randomIndex, () => { - const visitUrl = spyOnDependency(GLDropdown, 'visitUrl').and.stub(); navigateWithKeys('enter', null, () => { - expect(this.dropdownContainerElement).not.toHaveClass('show'); - const link = $(`${ITEM_SELECTOR}:eq(${randomIndex}) a`, this.$dropdownMenuElement); + expect(test.dropdownContainerElement).not.toHaveClass('show'); + const link = $(`${ITEM_SELECTOR}:eq(${randomIndex}) a`, test.$dropdownMenuElement); expect(link).toHaveClass('is-active'); const linkedLocation = link.attr('href'); @@ -162,21 +166,21 @@ describe('glDropdown', function describeDropdown() { }); it('should close on ESC keypress', () => { - expect(this.dropdownContainerElement).toHaveClass('show'); - this.dropdownContainerElement.trigger({ + expect(test.dropdownContainerElement).toHaveClass('show'); + test.dropdownContainerElement.trigger({ type: 'keyup', which: ARROW_KEYS.ESC, keyCode: ARROW_KEYS.ESC, }); - expect(this.dropdownContainerElement).not.toHaveClass('show'); + expect(test.dropdownContainerElement).not.toHaveClass('show'); }); }); describe('opened and waiting for a remote callback', () => { beforeEach(() => { initDropDown.call(this, true, true); - this.dropdownButtonElement.click(); + test.dropdownButtonElement.click(); }); it('should show loading indicator while search results are being fetched by backend', () => { @@ -203,13 +207,13 @@ describe('glDropdown', function describeDropdown() { it('should focus on input when opening for the second time after transition', () => { remoteCallback(); - this.dropdownContainerElement.trigger({ + test.dropdownContainerElement.trigger({ type: 'keyup', which: ARROW_KEYS.ESC, keyCode: ARROW_KEYS.ESC, }); - this.dropdownButtonElement.click(); - this.dropdownContainerElement.trigger('transitionend'); + test.dropdownButtonElement.click(); + test.dropdownContainerElement.trigger('transitionend'); expect($(document.activeElement)).toEqual($(SEARCH_INPUT_SELECTOR)); }); @@ -218,8 +222,8 @@ describe('glDropdown', function describeDropdown() { describe('input focus with array data', () => { it('should focus input when passing array data to drop down', () => { initDropDown.call(this, false, true); - this.dropdownButtonElement.click(); - this.dropdownContainerElement.trigger('transitionend'); + test.dropdownButtonElement.click(); + test.dropdownContainerElement.trigger('transitionend'); expect($(document.activeElement)).toEqual($(SEARCH_INPUT_SELECTOR)); }); @@ -234,7 +238,7 @@ describe('glDropdown', function describeDropdown() { .trigger('input'); expect($searchInput.val()).toEqual('g'); - this.dropdownButtonElement.trigger('hidden.bs.dropdown'); + test.dropdownButtonElement.trigger('hidden.bs.dropdown'); $searchInput.trigger('blur').trigger('focus'); expect($searchInput.val()).toEqual('g'); @@ -323,19 +327,19 @@ describe('glDropdown', function describeDropdown() { }, }; initDropDown.call(this, false, false, options); - const $item = $(`${ITEM_SELECTOR}:first() a`, this.$dropdownMenuElement); + const $item = $(`${ITEM_SELECTOR}:first() a`, test.$dropdownMenuElement); // select item the first time - this.dropdownButtonElement.click(); + test.dropdownButtonElement.click(); $item.click(); expect($item).toHaveClass('is-active'); // select item the second time - this.dropdownButtonElement.click(); + test.dropdownButtonElement.click(); $item.click(); expect($item).toHaveClass('is-active'); - expect($('.dropdown-toggle-text')).toHaveText(this.projectsData[0].id.toString()); + expect($('.dropdown-toggle-text')).toHaveText(test.projectsData[0].id.toString()); }); }); diff --git a/spec/frontend/helpers/local_storage_helper.js b/spec/frontend/helpers/local_storage_helper.js index 48e66b11767..a66c31d1353 100644 --- a/spec/frontend/helpers/local_storage_helper.js +++ b/spec/frontend/helpers/local_storage_helper.js @@ -28,12 +28,20 @@ const useLocalStorage = fn => { /** * Create an object with the localStorage interface but `jest.fn()` implementations. */ -export const createLocalStorageSpy = () => ({ - clear: jest.fn(), - getItem: jest.fn(), - setItem: jest.fn(), - removeItem: jest.fn(), -}); +export const createLocalStorageSpy = () => { + let storage = {}; + + return { + clear: jest.fn(() => { + storage = {}; + }), + getItem: jest.fn(key => storage[key]), + setItem: jest.fn((key, value) => { + storage[key] = value; + }), + removeItem: jest.fn(key => delete storage[key]), + }; +}; /** * Before each test, overwrite `window.localStorage` with a spy implementation. diff --git a/spec/frontend/helpers/local_storage_helper_spec.js b/spec/frontend/helpers/local_storage_helper_spec.js new file mode 100644 index 00000000000..18aec0f329a --- /dev/null +++ b/spec/frontend/helpers/local_storage_helper_spec.js @@ -0,0 +1,21 @@ +import { useLocalStorageSpy } from './local_storage_helper'; + +useLocalStorageSpy(); + +describe('localStorage helper', () => { + it('mocks localStorage but works exactly like original localStorage', () => { + localStorage.setItem('test', 'testing'); + localStorage.setItem('test2', 'testing'); + + expect(localStorage.getItem('test')).toBe('testing'); + + localStorage.removeItem('test', 'testing'); + + expect(localStorage.getItem('test')).toBeUndefined(); + expect(localStorage.getItem('test2')).toBe('testing'); + + localStorage.clear(); + + expect(localStorage.getItem('test2')).toBeUndefined(); + }); +}); diff --git a/spec/javascripts/importer_status_spec.js b/spec/frontend/importer_status_spec.js index 90835e1cc21..4ef74a2fe84 100644 --- a/spec/javascripts/importer_status_spec.js +++ b/spec/frontend/importer_status_spec.js @@ -16,9 +16,8 @@ describe('Importer Status', () => { describe('addToImport', () => { const importUrl = '/import_url'; - - beforeEach(() => { - setFixtures(` + const fixtures = ` + <table> <tr id="repo_123"> <td class="import-target"></td> <td class="import-actions job-status"> @@ -26,9 +25,13 @@ describe('Importer Status', () => { </button> </td> </tr> - `); - spyOn(ImporterStatus.prototype, 'initStatusPage').and.callFake(() => {}); - spyOn(ImporterStatus.prototype, 'setAutoUpdate').and.callFake(() => {}); + </table> + `; + + beforeEach(() => { + setFixtures(fixtures); + jest.spyOn(ImporterStatus.prototype, 'initStatusPage').mockImplementation(() => {}); + jest.spyOn(ImporterStatus.prototype, 'setAutoUpdate').mockImplementation(() => {}); instance = new ImporterStatus({ jobsUrl: '', importUrl, @@ -53,7 +56,7 @@ describe('Importer Status', () => { }); it('shows error message after failed POST request', done => { - appendSetFixtures('<div class="flash-container"></div>'); + setFixtures(`${fixtures}<div class="flash-container"></div>`); mock.onPost(importUrl).reply(422, { errors: 'You forgot your lunch', @@ -89,8 +92,8 @@ describe('Importer Status', () => { document.body.appendChild(div); - spyOn(ImporterStatus.prototype, 'initStatusPage').and.callFake(() => {}); - spyOn(ImporterStatus.prototype, 'setAutoUpdate').and.callFake(() => {}); + jest.spyOn(ImporterStatus.prototype, 'initStatusPage').mockImplementation(() => {}); + jest.spyOn(ImporterStatus.prototype, 'setAutoUpdate').mockImplementation(() => {}); instance = new ImporterStatus({ jobsUrl, }); diff --git a/spec/javascripts/merge_request_spec.js b/spec/frontend/merge_request_spec.js index b6173b9b171..f4f2a78f5f7 100644 --- a/spec/javascripts/merge_request_spec.js +++ b/spec/frontend/merge_request_spec.js @@ -4,24 +4,26 @@ import axios from '~/lib/utils/axios_utils'; import MergeRequest from '~/merge_request'; import CloseReopenReportToggle from '~/close_reopen_report_toggle'; import IssuablesHelper from '~/helpers/issuables_helper'; +import { TEST_HOST } from 'spec/test_constants'; -describe('MergeRequest', function() { - describe('task lists', function() { +describe('MergeRequest', () => { + const test = {}; + describe('task lists', () => { let mock; preloadFixtures('merge_requests/merge_request_with_task_list.html'); - beforeEach(function() { + beforeEach(() => { loadFixtures('merge_requests/merge_request_with_task_list.html'); - spyOn(axios, 'patch').and.callThrough(); + jest.spyOn(axios, 'patch'); mock = new MockAdapter(axios); mock - .onPatch(`${gl.TEST_HOST}/frontend-fixtures/merge-requests-project/-/merge_requests/1.json`) + .onPatch(`${TEST_HOST}/frontend-fixtures/merge-requests-project/-/merge_requests/1.json`) .reply(200, {}); - this.merge = new MergeRequest(); - return this.merge; + test.merge = new MergeRequest(); + return test.merge; }); afterEach(() => { @@ -29,14 +31,14 @@ describe('MergeRequest', function() { }); it('modifies the Markdown field', done => { - spyOn($, 'ajax').and.stub(); + jest.spyOn($, 'ajax').mockImplementation(); const changeEvent = document.createEvent('HTMLEvents'); changeEvent.initEvent('change', true, true); $('input[type=checkbox]') .first() .attr('checked', true)[0] .dispatchEvent(changeEvent); - setTimeout(() => { + setImmediate(() => { expect($('.js-task-list-field').val()).toBe( '- [x] Task List Item\n- [ ] \n- [ ] Task List Item 2\n', ); @@ -46,14 +48,14 @@ describe('MergeRequest', function() { it('ensure that task with only spaces does not get checked incorrectly', done => { // fixed in 'deckar01-task_list', '2.2.1' gem - spyOn($, 'ajax').and.stub(); + jest.spyOn($, 'ajax').mockImplementation(); const changeEvent = document.createEvent('HTMLEvents'); changeEvent.initEvent('change', true, true); $('input[type=checkbox]') .last() .attr('checked', true)[0] .dispatchEvent(changeEvent); - setTimeout(() => { + setImmediate(() => { expect($('.js-task-list-field').val()).toBe( '- [ ] Task List Item\n- [ ] \n- [x] Task List Item 2\n', ); @@ -73,9 +75,9 @@ describe('MergeRequest', function() { detail: { lineNumber, lineSource, index, checked }, }); - setTimeout(() => { + setImmediate(() => { expect(axios.patch).toHaveBeenCalledWith( - `${gl.TEST_HOST}/frontend-fixtures/merge-requests-project/-/merge_requests/1.json`, + `${TEST_HOST}/frontend-fixtures/merge-requests-project/-/merge_requests/1.json`, { merge_request: { description: '- [ ] Task List Item\n- [ ] \n- [ ] Task List Item 2\n', @@ -89,13 +91,9 @@ describe('MergeRequest', function() { }); }); - // https://gitlab.com/gitlab-org/gitlab/issues/34861 - // eslint-disable-next-line jasmine/no-disabled-tests - xit('shows an error notification when tasklist update failed', done => { + it('shows an error notification when tasklist update failed', done => { mock - .onPatch( - `${gl.TEST_HOST}/frontend-fixtures/merge-requests-project/-/merge_requests/1.json`, - ) + .onPatch(`${TEST_HOST}/frontend-fixtures/merge-requests-project/-/merge_requests/1.json`) .reply(409, {}); $('.js-task-list-field').trigger({ @@ -103,7 +101,7 @@ describe('MergeRequest', function() { detail: { lineNumber, lineSource, index, checked }, }); - setTimeout(() => { + setImmediate(() => { expect(document.querySelector('.flash-container .flash-text').innerText.trim()).toBe( 'Someone edited this merge request at the same time you did. Please refresh the page to see changes.', ); @@ -116,11 +114,11 @@ describe('MergeRequest', function() { describe('class constructor', () => { beforeEach(() => { - spyOn($, 'ajax').and.stub(); + jest.spyOn($, 'ajax').mockImplementation(); }); it('calls .initCloseReopenReport', () => { - spyOn(IssuablesHelper, 'initCloseReopenReport'); + jest.spyOn(IssuablesHelper, 'initCloseReopenReport').mockImplementation(() => {}); new MergeRequest(); // eslint-disable-line no-new @@ -128,14 +126,20 @@ describe('MergeRequest', function() { }); it('calls .initDroplab', () => { - const container = jasmine.createSpyObj('container', ['querySelector']); + const container = { + querySelector: jest.fn().mockName('container.querySelector'), + }; const dropdownTrigger = {}; const dropdownList = {}; const button = {}; - spyOn(CloseReopenReportToggle.prototype, 'initDroplab'); - spyOn(document, 'querySelector').and.returnValue(container); - container.querySelector.and.returnValues(dropdownTrigger, dropdownList, button); + jest.spyOn(CloseReopenReportToggle.prototype, 'initDroplab').mockImplementation(() => {}); + jest.spyOn(document, 'querySelector').mockReturnValue(container); + + container.querySelector + .mockReturnValueOnce(dropdownTrigger) + .mockReturnValueOnce(dropdownList) + .mockReturnValueOnce(button); new MergeRequest(); // eslint-disable-line no-new @@ -151,15 +155,15 @@ describe('MergeRequest', function() { describe('merge request of another user', () => { beforeEach(() => { loadFixtures('merge_requests/merge_request_with_task_list.html'); - this.el = document.querySelector('.js-issuable-actions'); + test.el = document.querySelector('.js-issuable-actions'); new MergeRequest(); // eslint-disable-line no-new MergeRequest.hideCloseButton(); }); it('hides the dropdown close item and selects the next item', () => { - const closeItem = this.el.querySelector('li.close-item'); - const smallCloseItem = this.el.querySelector('.js-close-item'); - const reportItem = this.el.querySelector('li.report-item'); + const closeItem = test.el.querySelector('li.close-item'); + const smallCloseItem = test.el.querySelector('.js-close-item'); + const reportItem = test.el.querySelector('li.report-item'); expect(closeItem).toHaveClass('hidden'); expect(smallCloseItem).toHaveClass('hidden'); @@ -171,13 +175,13 @@ describe('MergeRequest', function() { describe('merge request of current_user', () => { beforeEach(() => { loadFixtures('merge_requests/merge_request_of_current_user.html'); - this.el = document.querySelector('.js-issuable-actions'); + test.el = document.querySelector('.js-issuable-actions'); MergeRequest.hideCloseButton(); }); it('hides the close button', () => { - const closeButton = this.el.querySelector('.btn-close'); - const smallCloseItem = this.el.querySelector('.js-close-item'); + const closeButton = test.el.querySelector('.btn-close'); + const smallCloseItem = test.el.querySelector('.js-close-item'); expect(closeButton).toHaveClass('hidden'); expect(smallCloseItem).toHaveClass('hidden'); diff --git a/spec/javascripts/mini_pipeline_graph_dropdown_spec.js b/spec/frontend/mini_pipeline_graph_dropdown_spec.js index aa4a376caf7..506290834c8 100644 --- a/spec/javascripts/mini_pipeline_graph_dropdown_spec.js +++ b/spec/frontend/mini_pipeline_graph_dropdown_spec.js @@ -2,7 +2,7 @@ import $ from 'jquery'; import MockAdapter from 'axios-mock-adapter'; import axios from '~/lib/utils/axios_utils'; import MiniPipelineGraph from '~/mini_pipeline_graph_dropdown'; -import timeoutPromise from './helpers/set_timeout_promise_helper'; +import waitForPromises from './helpers/wait_for_promises'; describe('Mini Pipeline Graph Dropdown', () => { preloadFixtures('static/mini_dropdown_graph.html'); @@ -39,9 +39,9 @@ describe('Mini Pipeline Graph Dropdown', () => { }); it('should call getBuildsList', () => { - const getBuildsListSpy = spyOn(MiniPipelineGraph.prototype, 'getBuildsList').and.callFake( - function() {}, - ); + const getBuildsListSpy = jest + .spyOn(MiniPipelineGraph.prototype, 'getBuildsList') + .mockImplementation(() => {}); new MiniPipelineGraph({ container: '.js-builds-dropdown-tests' }).bindEvents(); @@ -51,7 +51,7 @@ describe('Mini Pipeline Graph Dropdown', () => { }); it('should make a request to the endpoint provided in the html', () => { - const ajaxSpy = spyOn(axios, 'get').and.callThrough(); + const ajaxSpy = jest.spyOn(axios, 'get'); mock.onGet('foobar').reply(200, { html: '', @@ -61,7 +61,7 @@ describe('Mini Pipeline Graph Dropdown', () => { document.querySelector('.js-builds-dropdown-button').click(); - expect(ajaxSpy.calls.allArgs()[0][0]).toEqual('foobar'); + expect(ajaxSpy.mock.calls[0][0]).toEqual('foobar'); }); it('should not close when user uses cmd/ctrl + click', done => { @@ -78,11 +78,11 @@ describe('Mini Pipeline Graph Dropdown', () => { document.querySelector('.js-builds-dropdown-button').click(); - timeoutPromise() + waitForPromises() .then(() => { document.querySelector('a.mini-pipeline-graph-dropdown-item').click(); }) - .then(timeoutPromise) + .then(waitForPromises) .then(() => { expect($('.js-builds-dropdown-list').is(':visible')).toEqual(true); }) @@ -97,7 +97,7 @@ describe('Mini Pipeline Graph Dropdown', () => { document.querySelector('.js-builds-dropdown-button').click(); - setTimeout(() => { + setImmediate(() => { expect($('.js-builds-dropdown-tests .dropdown').hasClass('open')).toEqual(false); done(); }); diff --git a/spec/javascripts/pager_spec.js b/spec/frontend/pager_spec.js index c95a8400c6c..d7177a32cde 100644 --- a/spec/javascripts/pager_spec.js +++ b/spec/frontend/pager_spec.js @@ -2,6 +2,11 @@ import $ from 'jquery'; import MockAdapter from 'axios-mock-adapter'; import axios from '~/lib/utils/axios_utils'; import Pager from '~/pager'; +import { removeParams } from '~/lib/utils/url_utility'; + +jest.mock('~/lib/utils/url_utility', () => ({ + removeParams: jest.fn().mockName('removeParams'), +})); describe('pager', () => { let axiosMock; @@ -19,7 +24,7 @@ describe('pager', () => { beforeEach(() => { setFixtures('<div class="content_list"></div><div class="loading"></div>'); - spyOn($.fn, 'endlessScroll').and.stub(); + jest.spyOn($.fn, 'endlessScroll').mockImplementation(); }); afterEach(() => { @@ -36,7 +41,7 @@ describe('pager', () => { it('should use current url if data-href attribute not provided', () => { const href = `${gl.TEST_HOST}/some_list`; - spyOnDependency(Pager, 'removeParams').and.returnValue(href); + removeParams.mockReturnValue(href); Pager.init(); expect(Pager.url).toBe(href); @@ -52,7 +57,7 @@ describe('pager', () => { it('keeps extra query parameters from url', () => { window.history.replaceState({}, null, '?filter=test&offset=100'); const href = `${gl.TEST_HOST}/some_list?filter=test`; - const removeParams = spyOnDependency(Pager, 'removeParams').and.returnValue(href); + removeParams.mockReturnValue(href); Pager.init(); expect(removeParams).toHaveBeenCalledWith(['limit', 'offset']); @@ -78,7 +83,7 @@ describe('pager', () => { setFixtures( '<div class="content_list" data-href="/some_list"></div><div class="loading"></div>', ); - spyOn(axios, 'get').and.callThrough(); + jest.spyOn(axios, 'get'); Pager.init(); }); @@ -86,10 +91,10 @@ describe('pager', () => { it('shows loader while loading next page', done => { mockSuccess(); - spyOn(Pager.loading, 'show'); + jest.spyOn(Pager.loading, 'show').mockImplementation(() => {}); Pager.getOld(); - setTimeout(() => { + setImmediate(() => { expect(Pager.loading.show).toHaveBeenCalled(); done(); @@ -99,10 +104,10 @@ describe('pager', () => { it('hides loader on success', done => { mockSuccess(); - spyOn(Pager.loading, 'hide'); + jest.spyOn(Pager.loading, 'hide').mockImplementation(() => {}); Pager.getOld(); - setTimeout(() => { + setImmediate(() => { expect(Pager.loading.hide).toHaveBeenCalled(); done(); @@ -112,10 +117,10 @@ describe('pager', () => { it('hides loader on error', done => { mockError(); - spyOn(Pager.loading, 'hide'); + jest.spyOn(Pager.loading, 'hide').mockImplementation(() => {}); Pager.getOld(); - setTimeout(() => { + setImmediate(() => { expect(Pager.loading.hide).toHaveBeenCalled(); done(); @@ -127,8 +132,8 @@ describe('pager', () => { Pager.limit = 20; Pager.getOld(); - setTimeout(() => { - const [url, params] = axios.get.calls.argsFor(0); + setImmediate(() => { + const [url, params] = axios.get.mock.calls[0]; expect(params).toEqual({ params: { @@ -148,10 +153,10 @@ describe('pager', () => { Pager.limit = 20; mockSuccess(1); - spyOn(Pager.loading, 'hide'); + jest.spyOn(Pager.loading, 'hide').mockImplementation(() => {}); Pager.getOld(); - setTimeout(() => { + setImmediate(() => { expect(Pager.loading.hide).toHaveBeenCalled(); expect(Pager.disable).toBe(true); diff --git a/spec/javascripts/todos_spec.js b/spec/frontend/pages/dashboard/todos/index/todos_spec.js index dc3c547c632..204fe3d0a68 100644 --- a/spec/javascripts/todos_spec.js +++ b/spec/frontend/pages/dashboard/todos/index/todos_spec.js @@ -5,6 +5,11 @@ import '~/lib/utils/common_utils'; import '~/gl_dropdown'; import axios from '~/lib/utils/axios_utils'; import { addDelimiter } from '~/lib/utils/text_utility'; +import { visitUrl } from '~/lib/utils/url_utility'; + +jest.mock('~/lib/utils/url_utility', () => ({ + visitUrl: jest.fn().mockName('visitUrl'), +})); const TEST_COUNT_BIG = 2000; const TEST_DONE_COUNT_BIG = 7300; @@ -30,7 +35,7 @@ describe('Todos', () => { it('opens the todo url', done => { const todoLink = todoItem.dataset.url; - spyOnDependency(Todos, 'visitUrl').and.callFake(url => { + visitUrl.mockImplementation(url => { expect(url).toEqual(todoLink); done(); }); @@ -39,14 +44,12 @@ describe('Todos', () => { }); describe('meta click', () => { - let visitUrlSpy; let windowOpenSpy; let metakeyEvent; beforeEach(() => { metakeyEvent = $.Event('click', { keyCode: 91, ctrlKey: true }); - visitUrlSpy = spyOnDependency(Todos, 'visitUrl').and.callFake(() => {}); - windowOpenSpy = spyOn(window, 'open').and.callFake(() => {}); + windowOpenSpy = jest.spyOn(window, 'open').mockImplementation(() => {}); }); it('opens the todo url in another tab', () => { @@ -54,7 +57,7 @@ describe('Todos', () => { $('.todos-list .todo').trigger(metakeyEvent); - expect(visitUrlSpy).not.toHaveBeenCalled(); + expect(visitUrl).not.toHaveBeenCalled(); expect(windowOpenSpy).toHaveBeenCalledWith(todoLink, '_blank'); }); @@ -62,7 +65,7 @@ describe('Todos', () => { $('.todos-list a').on('click', e => e.preventDefault()); $('.todos-list img').trigger(metakeyEvent); - expect(visitUrlSpy).not.toHaveBeenCalled(); + expect(visitUrl).not.toHaveBeenCalled(); expect(windowOpenSpy).not.toHaveBeenCalled(); }); }); @@ -78,7 +81,7 @@ describe('Todos', () => { mock .onDelete(path) .replyOnce(200, { count: TEST_COUNT_BIG, done_count: TEST_DONE_COUNT_BIG }); - onToggleSpy = jasmine.createSpy('onToggle'); + onToggleSpy = jest.fn(); $(document).on('todo:toggle', onToggleSpy); // Act @@ -89,7 +92,7 @@ describe('Todos', () => { }); it('dispatches todo:toggle', () => { - expect(onToggleSpy).toHaveBeenCalledWith(jasmine.anything(), TEST_COUNT_BIG); + expect(onToggleSpy).toHaveBeenCalledWith(expect.anything(), TEST_COUNT_BIG); }); it('updates pending text', () => { diff --git a/spec/javascripts/signin_tabs_memoizer_spec.js b/spec/frontend/pages/sessions/new/signin_tabs_memoizer_spec.js index 966ae55ce14..738498edbd3 100644 --- a/spec/javascripts/signin_tabs_memoizer_spec.js +++ b/spec/frontend/pages/sessions/new/signin_tabs_memoizer_spec.js @@ -2,6 +2,9 @@ import AccessorUtilities from '~/lib/utils/accessor'; import SigninTabsMemoizer from '~/pages/sessions/new/signin_tabs_memoizer'; import trackData from '~/pages/sessions/new/index'; import Tracking from '~/tracking'; +import { useLocalStorageSpy } from 'helpers/local_storage_helper'; + +useLocalStorageSpy(); describe('SigninTabsMemoizer', () => { const fixtureTemplate = 'static/signin_tabs.html'; @@ -22,7 +25,7 @@ describe('SigninTabsMemoizer', () => { beforeEach(() => { loadFixtures(fixtureTemplate); - spyOn(AccessorUtilities, 'isLocalStorageAccessSafe').and.returnValue(true); + jest.spyOn(AccessorUtilities, 'isLocalStorageAccessSafe').mockReturnValue(true); }); it('does nothing if no tab was previously selected', () => { @@ -38,8 +41,8 @@ describe('SigninTabsMemoizer', () => { const fakeTab = { click: () => {}, }; - spyOn(document, 'querySelector').and.returnValue(fakeTab); - spyOn(fakeTab, 'click'); + jest.spyOn(document, 'querySelector').mockReturnValue(fakeTab); + jest.spyOn(fakeTab, 'click').mockImplementation(() => {}); memo.bootstrap(); @@ -51,17 +54,18 @@ describe('SigninTabsMemoizer', () => { it('clicks the first tab if value in local storage is bad', () => { createMemoizer().saveData('#bogus'); const fakeTab = { - click: () => {}, + click: jest.fn().mockName('fakeTab_click'), }; - spyOn(document, 'querySelector').and.callFake(selector => - selector === `${tabSelector} a[href="#bogus"]` ? null : fakeTab, - ); - spyOn(fakeTab, 'click'); + jest + .spyOn(document, 'querySelector') + .mockImplementation(selector => + selector === `${tabSelector} a[href="#bogus"]` ? null : fakeTab, + ); memo.bootstrap(); // verify that triggers click on stored selector and fallback - expect(document.querySelector.calls.allArgs()).toEqual([ + expect(document.querySelector.mock.calls).toEqual([ ['ul.new-session-tabs a[href="#bogus"]'], ['ul.new-session-tabs a'], ]); @@ -97,7 +101,7 @@ describe('SigninTabsMemoizer', () => { describe('trackData', () => { beforeEach(() => { - spyOn(Tracking, 'event'); + jest.spyOn(Tracking, 'event').mockImplementation(() => {}); }); describe('with tracking data', () => { @@ -144,12 +148,10 @@ describe('SigninTabsMemoizer', () => { memo = { currentTabKey, }; - - spyOn(localStorage, 'setItem'); }); describe('if .isLocalStorageAvailable is `false`', () => { - beforeEach(function() { + beforeEach(() => { memo.isLocalStorageAvailable = false; SigninTabsMemoizer.prototype.saveData.call(memo); @@ -163,7 +165,7 @@ describe('SigninTabsMemoizer', () => { describe('if .isLocalStorageAvailable is `true`', () => { const value = 'value'; - beforeEach(function() { + beforeEach(() => { memo.isLocalStorageAvailable = true; SigninTabsMemoizer.prototype.saveData.call(memo, value); @@ -184,11 +186,11 @@ describe('SigninTabsMemoizer', () => { currentTabKey, }; - spyOn(localStorage, 'getItem').and.returnValue(itemValue); + localStorage.getItem.mockReturnValue(itemValue); }); describe('if .isLocalStorageAvailable is `false`', () => { - beforeEach(function() { + beforeEach(() => { memo.isLocalStorageAvailable = false; readData = SigninTabsMemoizer.prototype.readData.call(memo); @@ -201,7 +203,7 @@ describe('SigninTabsMemoizer', () => { }); describe('if .isLocalStorageAvailable is `true`', () => { - beforeEach(function() { + beforeEach(() => { memo.isLocalStorageAvailable = true; readData = SigninTabsMemoizer.prototype.readData.call(memo); diff --git a/spec/javascripts/persistent_user_callout_spec.js b/spec/frontend/persistent_user_callout_spec.js index d4cb92cacfd..db324990e71 100644 --- a/spec/javascripts/persistent_user_callout_spec.js +++ b/spec/frontend/persistent_user_callout_spec.js @@ -1,7 +1,10 @@ import MockAdapter from 'axios-mock-adapter'; -import setTimeoutPromise from 'spec/helpers/set_timeout_promise_helper'; +import waitForPromises from 'helpers/wait_for_promises'; import axios from '~/lib/utils/axios_utils'; import PersistentUserCallout from '~/persistent_user_callout'; +import Flash from '~/flash'; + +jest.mock('~/flash'); describe('PersistentUserCallout', () => { const dismissEndpoint = '/dismiss'; @@ -51,44 +54,35 @@ describe('PersistentUserCallout', () => { button = fixture.querySelector('.js-close'); mockAxios = new MockAdapter(axios); persistentUserCallout = new PersistentUserCallout(container); - spyOn(persistentUserCallout.container, 'remove'); + jest.spyOn(persistentUserCallout.container, 'remove').mockImplementation(() => {}); }); afterEach(() => { mockAxios.restore(); }); - it('POSTs endpoint and removes container when clicking close', done => { + it('POSTs endpoint and removes container when clicking close', () => { mockAxios.onPost(dismissEndpoint).replyOnce(200); button.click(); - setTimeoutPromise() - .then(() => { - expect(persistentUserCallout.container.remove).toHaveBeenCalled(); - expect(mockAxios.history.post[0].data).toBe( - JSON.stringify({ feature_name: featureName }), - ); - }) - .then(done) - .catch(done.fail); + return waitForPromises().then(() => { + expect(persistentUserCallout.container.remove).toHaveBeenCalled(); + expect(mockAxios.history.post[0].data).toBe(JSON.stringify({ feature_name: featureName })); + }); }); - it('invokes Flash when the dismiss request fails', done => { - const Flash = spyOnDependency(PersistentUserCallout, 'Flash'); + it('invokes Flash when the dismiss request fails', () => { mockAxios.onPost(dismissEndpoint).replyOnce(500); button.click(); - setTimeoutPromise() - .then(() => { - expect(persistentUserCallout.container.remove).not.toHaveBeenCalled(); - expect(Flash).toHaveBeenCalledWith( - 'An error occurred while dismissing the alert. Refresh the page and try again.', - ); - }) - .then(done) - .catch(done.fail); + return waitForPromises().then(() => { + expect(persistentUserCallout.container.remove).not.toHaveBeenCalled(); + expect(Flash).toHaveBeenCalledWith( + 'An error occurred while dismissing the alert. Refresh the page and try again.', + ); + }); }); }); @@ -108,56 +102,45 @@ describe('PersistentUserCallout', () => { normalLink = fixture.querySelector('.normal-link'); mockAxios = new MockAdapter(axios); persistentUserCallout = new PersistentUserCallout(container); - spyOn(persistentUserCallout.container, 'remove'); - windowSpy = spyOn(window, 'open').and.callFake(() => {}); + jest.spyOn(persistentUserCallout.container, 'remove').mockImplementation(() => {}); + windowSpy = jest.spyOn(window, 'open').mockImplementation(() => {}); }); afterEach(() => { mockAxios.restore(); }); - it('defers loading of a link until callout is dismissed', done => { + it('defers loading of a link until callout is dismissed', () => { const { href, target } = deferredLink; mockAxios.onPost(dismissEndpoint).replyOnce(200); deferredLink.click(); - setTimeoutPromise() - .then(() => { - expect(windowSpy).toHaveBeenCalledWith(href, target); - expect(persistentUserCallout.container.remove).toHaveBeenCalled(); - expect(mockAxios.history.post[0].data).toBe( - JSON.stringify({ feature_name: featureName }), - ); - }) - .then(done) - .catch(done.fail); + return waitForPromises().then(() => { + expect(windowSpy).toHaveBeenCalledWith(href, target); + expect(persistentUserCallout.container.remove).toHaveBeenCalled(); + expect(mockAxios.history.post[0].data).toBe(JSON.stringify({ feature_name: featureName })); + }); }); - it('does not dismiss callout on non-deferred links', done => { + it('does not dismiss callout on non-deferred links', () => { normalLink.click(); - setTimeoutPromise() - .then(() => { - expect(windowSpy).not.toHaveBeenCalled(); - expect(persistentUserCallout.container.remove).not.toHaveBeenCalled(); - }) - .then(done) - .catch(done.fail); + return waitForPromises().then(() => { + expect(windowSpy).not.toHaveBeenCalled(); + expect(persistentUserCallout.container.remove).not.toHaveBeenCalled(); + }); }); - it('does not follow link when notification is closed', done => { + it('does not follow link when notification is closed', () => { mockAxios.onPost(dismissEndpoint).replyOnce(200); button.click(); - setTimeoutPromise() - .then(() => { - expect(windowSpy).not.toHaveBeenCalled(); - expect(persistentUserCallout.container.remove).toHaveBeenCalled(); - }) - .then(done) - .catch(done.fail); + return waitForPromises().then(() => { + expect(windowSpy).not.toHaveBeenCalled(); + expect(persistentUserCallout.container.remove).toHaveBeenCalled(); + }); }); }); diff --git a/spec/javascripts/read_more_spec.js b/spec/frontend/read_more_spec.js index d1d01272403..d1d01272403 100644 --- a/spec/javascripts/read_more_spec.js +++ b/spec/frontend/read_more_spec.js diff --git a/spec/lib/gitlab/sidekiq_middleware/server_metrics_spec.rb b/spec/lib/gitlab/sidekiq_middleware/server_metrics_spec.rb index 3214bd758e7..da8d17b1272 100644 --- a/spec/lib/gitlab/sidekiq_middleware/server_metrics_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/server_metrics_spec.rb @@ -31,7 +31,9 @@ describe Gitlab::SidekiqMiddleware::ServerMetrics do let(:gitaly_seconds_metric) { double('gitaly seconds metric') } let(:failed_total_metric) { double('failed total metric') } let(:retried_total_metric) { double('retried total metric') } + let(:redis_requests_total) { double('redis calls total metric') } let(:running_jobs_metric) { double('running jobs metric') } + let(:redis_seconds_metric) { double('redis seconds metric') } before do allow(Gitlab::Metrics).to receive(:histogram).with(:sidekiq_jobs_queue_duration_seconds, anything, anything, anything).and_return(queue_duration_seconds) @@ -39,8 +41,10 @@ describe Gitlab::SidekiqMiddleware::ServerMetrics do allow(Gitlab::Metrics).to receive(:histogram).with(:sidekiq_jobs_cpu_seconds, anything, anything, anything).and_return(user_execution_seconds_metric) allow(Gitlab::Metrics).to receive(:histogram).with(:sidekiq_jobs_db_seconds, anything, anything, anything).and_return(db_seconds_metric) allow(Gitlab::Metrics).to receive(:histogram).with(:sidekiq_jobs_gitaly_seconds, anything, anything, anything).and_return(gitaly_seconds_metric) + allow(Gitlab::Metrics).to receive(:histogram).with(:sidekiq_redis_requests_duration_seconds, anything, anything, anything).and_return(redis_seconds_metric) allow(Gitlab::Metrics).to receive(:counter).with(:sidekiq_jobs_failed_total, anything).and_return(failed_total_metric) allow(Gitlab::Metrics).to receive(:counter).with(:sidekiq_jobs_retried_total, anything).and_return(retried_total_metric) + allow(Gitlab::Metrics).to receive(:counter).with(:sidekiq_redis_requests_total, anything).and_return(redis_requests_total) allow(Gitlab::Metrics).to receive(:gauge).with(:sidekiq_running_jobs, anything, {}, :all).and_return(running_jobs_metric) allow(Gitlab::Metrics).to receive(:gauge).with(:sidekiq_concurrency, anything, {}, :all).and_return(concurrency_metric) @@ -69,21 +73,27 @@ describe Gitlab::SidekiqMiddleware::ServerMetrics do let(:db_duration) { 3 } let(:gitaly_duration) { 4 } + let(:redis_calls) { 2 } + let(:redis_duration) { 0.01 } + before do allow(subject).to receive(:get_thread_cputime).and_return(thread_cputime_before, thread_cputime_after) allow(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(monotonic_time_before, monotonic_time_after) allow(Gitlab::InstrumentationHelper).to receive(:queue_duration_for_job).with(job).and_return(queue_duration_for_job) allow(ActiveRecord::LogSubscriber).to receive(:runtime).and_return(db_duration * 1000) - allow(subject).to receive(:get_gitaly_time).and_return(gitaly_duration) - expect(running_jobs_metric).to receive(:increment).with(labels, 1) - expect(running_jobs_metric).to receive(:increment).with(labels, -1) - - expect(queue_duration_seconds).to receive(:observe).with(labels, queue_duration_for_job) if queue_duration_for_job - expect(user_execution_seconds_metric).to receive(:observe).with(labels_with_job_status, thread_cputime_duration) - expect(db_seconds_metric).to receive(:observe).with(labels_with_job_status, db_duration) - expect(gitaly_seconds_metric).to receive(:observe).with(labels_with_job_status, gitaly_duration) - expect(completion_seconds_metric).to receive(:observe).with(labels_with_job_status, monotonic_time_duration) + job[:gitaly_duration_s] = gitaly_duration + job[:redis_calls] = redis_calls + job[:redis_duration_s] = redis_duration + + allow(running_jobs_metric).to receive(:increment) + allow(redis_requests_total).to receive(:increment) + allow(queue_duration_seconds).to receive(:observe) + allow(user_execution_seconds_metric).to receive(:observe) + allow(db_seconds_metric).to receive(:observe) + allow(gitaly_seconds_metric).to receive(:observe) + allow(completion_seconds_metric).to receive(:observe) + allow(redis_seconds_metric).to receive(:observe) end it 'yields block' do @@ -91,6 +101,16 @@ describe Gitlab::SidekiqMiddleware::ServerMetrics do end it 'sets queue specific metrics' do + expect(running_jobs_metric).to receive(:increment).with(labels, -1) + expect(running_jobs_metric).to receive(:increment).with(labels, 1) + expect(queue_duration_seconds).to receive(:observe).with(labels, queue_duration_for_job) if queue_duration_for_job + expect(user_execution_seconds_metric).to receive(:observe).with(labels_with_job_status, thread_cputime_duration) + expect(db_seconds_metric).to receive(:observe).with(labels_with_job_status, db_duration) + expect(gitaly_seconds_metric).to receive(:observe).with(labels_with_job_status, gitaly_duration) + expect(completion_seconds_metric).to receive(:observe).with(labels_with_job_status, monotonic_time_duration) + expect(redis_seconds_metric).to receive(:observe).with(labels_with_job_status, redis_duration) + expect(redis_requests_total).to receive(:increment).with(labels_with_job_status, redis_calls) + subject.call(worker, job, :test) { nil } end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 6605866d9c0..c99a11b7401 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -2512,6 +2512,17 @@ describe Ci::Build do end end end + + context 'with the :modified_path_ci_variables feature flag disabled' do + before do + stub_feature_flags(modified_path_ci_variables: false) + end + + it 'does not set CI_MERGE_REQUEST_CHANGED_PAGES_* variables' do + expect(subject.find { |var| var[:key] == 'CI_MERGE_REQUEST_CHANGED_PAGE_PATHS' }).to be_nil + expect(subject.find { |var| var[:key] == 'CI_MERGE_REQUEST_CHANGED_PAGE_URLS' }).to be_nil + end + end end context 'when build has user' do diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index 106f8def42d..6d0fe905aed 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -264,13 +264,13 @@ describe Service do end end - describe '.build_from_template' do + describe '.build_from_integration' do context 'when template is invalid' do it 'sets service template to inactive when template is invalid' do template = build(:prometheus_service, template: true, active: true, properties: {}) template.save(validate: false) - service = described_class.build_from_template(project.id, template) + service = described_class.build_from_integration(project.id, template) expect(service).to be_valid expect(service.active).to be false @@ -293,7 +293,7 @@ describe Service do shared_examples 'service creation from a template' do it 'creates a correct service' do - service = described_class.build_from_template(project.id, template) + service = described_class.build_from_integration(project.id, template) expect(service).to be_active expect(service.title).to eq(title) @@ -302,6 +302,8 @@ describe Service do expect(service.api_url).to eq(api_url) expect(service.username).to eq(username) expect(service.password).to eq(password) + expect(service.template).to eq(false) + expect(service.instance).to eq(false) end end diff --git a/spec/services/projects/propagate_service_template_spec.rb b/spec/services/projects/propagate_service_template_spec.rb index 7188ac5f733..ddc27c037f8 100644 --- a/spec/services/projects/propagate_service_template_spec.rb +++ b/spec/services/projects/propagate_service_template_spec.rb @@ -62,8 +62,8 @@ describe Projects::PropagateServiceTemplate do } ) - Service.build_from_template(project.id, service_template).save! - Service.build_from_template(project.id, other_service).save! + Service.build_from_integration(project.id, service_template).save! + Service.build_from_integration(project.id, other_service).save! expect { described_class.propagate(service_template) } .not_to change { Service.count } diff --git a/spec/services/projects/update_pages_service_spec.rb b/spec/services/projects/update_pages_service_spec.rb index f561a303be4..29c3c300d1b 100644 --- a/spec/services/projects/update_pages_service_spec.rb +++ b/spec/services/projects/update_pages_service_spec.rb @@ -158,6 +158,23 @@ describe Projects::UpdatePagesService do expect(project.pages_metadatum).not_to be_deployed end end + + context 'with background jobs running', :sidekiq_inline do + where(:ci_atomic_processing) do + [true, false] + end + + with_them do + before do + stub_feature_flags(ci_atomic_processing: ci_atomic_processing) + end + + it 'succeeds' do + expect(project.pages_deployed?).to be_falsey + expect(execute).to eq(:success) + end + end + end end end |