diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-10-06 03:11:56 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-10-06 03:11:56 +0300 |
commit | 85c68f14bf3ec5fa77cf12633d33abfd2b9fd9e6 (patch) | |
tree | c1d57a63ddab2a20e624173b0b0dc68e059c00d0 | |
parent | ee2aa09a2417755bc9efbedaec48fc62b498f672 (diff) |
Add latest changes from gitlab-org/gitlab@master
44 files changed, 633 insertions, 189 deletions
diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index 39f46e993a4..ebebcdba91c 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -296,7 +296,6 @@ Rails/TimeZone: - 'spec/lib/gitlab/graphql_logger_spec.rb' - 'spec/lib/gitlab/graphs/commits_spec.rb' - 'spec/lib/gitlab/import_export/project/relation_factory_spec.rb' - - 'spec/lib/gitlab/instrumentation_helper_spec.rb' - 'spec/lib/gitlab/json_logger_spec.rb' - 'spec/lib/gitlab/lfs_token_spec.rb' - 'spec/lib/gitlab/log_timestamp_formatter_spec.rb' @@ -384,10 +383,8 @@ RSpec/TimecopFreeze: - 'spec/lib/gitlab/checks/timed_logger_spec.rb' - 'spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb' - 'spec/lib/gitlab/cycle_analytics/usage_data_spec.rb' - - 'spec/lib/gitlab/instrumentation_helper_spec.rb' - 'spec/lib/gitlab/omniauth_logging/json_formatter_spec.rb' - 'spec/lib/gitlab/puma_logging/json_formatter_spec.rb' - - 'spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb' - 'spec/lib/json_web_token/hmac_token_spec.rb' - 'spec/tooling/rspec_flaky/flaky_example_spec.rb' - 'spec/tooling/rspec_flaky/listener_spec.rb' diff --git a/app/assets/javascripts/diffs/components/compare_versions.vue b/app/assets/javascripts/diffs/components/compare_versions.vue index f098d20afd1..da918947cc5 100644 --- a/app/assets/javascripts/diffs/components/compare_versions.vue +++ b/app/assets/javascripts/diffs/components/compare_versions.vue @@ -100,6 +100,7 @@ export default { variant="default" icon="file-tree" class="gl-mr-3 js-toggle-tree-list btn-icon" + data-qa-selector="file_tree_button" :title="toggleFileBrowserTitle" :aria-label="toggleFileBrowserTitle" :selected="showTreeList" diff --git a/app/assets/javascripts/diffs/components/tree_list.vue b/app/assets/javascripts/diffs/components/tree_list.vue index 39ce849fc03..41d885d3dc1 100644 --- a/app/assets/javascripts/diffs/components/tree_list.vue +++ b/app/assets/javascripts/diffs/components/tree_list.vue @@ -62,7 +62,7 @@ export default { </script> <template> - <div class="tree-list-holder d-flex flex-column"> + <div class="tree-list-holder d-flex flex-column" data-qa-selector="file_tree_container"> <div class="gl-mb-3 position-relative tree-list-search d-flex"> <div class="flex-fill d-flex"> <gl-icon name="search" class="position-absolute tree-list-icon" /> diff --git a/app/models/release.rb b/app/models/release.rb index 0dd71c6ebfb..eac6346cc60 100644 --- a/app/models/release.rb +++ b/app/models/release.rb @@ -33,6 +33,7 @@ class Release < ApplicationRecord includes(:author, :evidences, :milestones, :links, :sorted_links, project: [:project_feature, :route, { namespace: :route }]) } + scope :with_milestones, -> { joins(:milestone_releases) } scope :recent, -> { sorted.limit(MAX_NUMBER_TO_DISPLAY) } scope :without_evidence, -> { left_joins(:evidences).where(::Releases::Evidence.arel_table[:id].eq(nil)) } scope :released_within_2hrs, -> { where(released_at: Time.zone.now - 1.hour..Time.zone.now + 1.hour) } diff --git a/app/services/issues/close_service.rb b/app/services/issues/close_service.rb index ea64239dd99..856f6932d61 100644 --- a/app/services/issues/close_service.rb +++ b/app/services/issues/close_service.rb @@ -3,8 +3,8 @@ module Issues class CloseService < Issues::BaseService # Closes the supplied issue if the current user is able to do so. - def execute(issue, commit: nil, notifications: true, system_note: true) - return issue unless can?(current_user, :update_issue, issue) || issue.is_a?(ExternalIssue) + def execute(issue, commit: nil, notifications: true, system_note: true, skip_authorization: false) + return issue unless can_close?(issue, skip_authorization: skip_authorization) close_issue(issue, closed_via: commit, @@ -24,7 +24,7 @@ module Issues return issue end - if project.issues_enabled? && issue.close(current_user) + if perform_close(issue) event_service.close_issue(issue, current_user) create_note(issue, closed_via) if system_note @@ -51,6 +51,15 @@ module Issues private + # Overridden on EE + def perform_close(issue) + issue.close(current_user) + end + + def can_close?(issue, skip_authorization: false) + skip_authorization || can?(current_user, :update_issue, issue) || issue.is_a?(ExternalIssue) + end + def perform_incident_management_actions(issue) resolve_alert(issue) end diff --git a/app/services/issues/reopen_service.rb b/app/services/issues/reopen_service.rb index 977b924ed72..4abd1dfbf4e 100644 --- a/app/services/issues/reopen_service.rb +++ b/app/services/issues/reopen_service.rb @@ -2,10 +2,10 @@ module Issues class ReopenService < Issues::BaseService - def execute(issue) - return issue unless can?(current_user, :reopen_issue, issue) + def execute(issue, skip_authorization: false) + return issue unless can_reopen?(issue, skip_authorization: skip_authorization) - if issue.reopen + if perform_reopen(issue) event_service.reopen_issue(issue, current_user) create_note(issue, 'reopened') notification_service.async.reopen_issue(issue, current_user) @@ -22,6 +22,15 @@ module Issues private + # Overriden on EE + def perform_reopen(issue) + issue.reopen + end + + def can_reopen?(issue, skip_authorization: false) + skip_authorization || can?(current_user, :reopen_issue, issue) + end + def perform_incident_management_actions(issue) end diff --git a/config/metrics/counts_28d/20210715094458_releases_with_milestones.yml b/config/metrics/counts_28d/20210715094458_releases_with_milestones.yml new file mode 100644 index 00000000000..74f9406a133 --- /dev/null +++ b/config/metrics/counts_28d/20210715094458_releases_with_milestones.yml @@ -0,0 +1,23 @@ +--- +key_path: usage_activity_by_stage_monthly.release.releases_with_milestones +description: Unique users creating releases with milestones associated +performance_indicator_type: [smau] +product_section: ops +product_stage: release +product_group: 'group::release' +product_category: Release Orchestration +value_type: number +status: active +milestone: "14.4" +introduced_by_url: 'https://gitlab.com/gitlab-org/gitlab/-/merge_requests/71287' +time_frame: 28d +data_source: database +instrumentation_class: 'CountUsersAssociatingMilestonesToReleasesMetric' +data_category: Optional +distribution: +- ce +- ee +tier: +- free +- premium +- ultimate diff --git a/config/metrics/counts_all/20210715094459_releases_with_milestones.yml b/config/metrics/counts_all/20210715094459_releases_with_milestones.yml new file mode 100644 index 00000000000..5d853604580 --- /dev/null +++ b/config/metrics/counts_all/20210715094459_releases_with_milestones.yml @@ -0,0 +1,23 @@ +--- +key_path: usage_activity_by_stage.release.releases_with_milestones +description: Unique users creating releases with milestones associated +performance_indicator_type: [] +product_section: ops +product_stage: release +product_group: 'group::release' +product_category: Release Orchestration +value_type: number +status: active +milestone: "14.4" +introduced_by_url: 'https://gitlab.com/gitlab-org/gitlab/-/merge_requests/71287' +time_frame: 28d +data_source: database +instrumentation_class: 'CountUsersAssociatingMilestonesToReleasesMetric' +data_category: Optional +distribution: +- ce +- ee +tier: +- free +- premium +- ultimate diff --git a/db/migrate/20210922172056_add_unique_namespaces_index_on_name_parent_id_and_type.rb b/db/migrate/20210922172056_add_unique_namespaces_index_on_name_parent_id_and_type.rb new file mode 100644 index 00000000000..6cbbe582ff6 --- /dev/null +++ b/db/migrate/20210922172056_add_unique_namespaces_index_on_name_parent_id_and_type.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class AddUniqueNamespacesIndexOnNameParentIdAndType < Gitlab::Database::Migration[1.0] + INDEX_NAME = 'index_namespaces_name_parent_id_type' + + disable_ddl_transaction! + + def up + add_concurrent_index :namespaces, [:name, :parent_id, :type], unique: true, name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :namespaces, INDEX_NAME + end +end diff --git a/db/migrate/20210922172156_drop_unique_namespaces_index_on_name_and_parent_id.rb b/db/migrate/20210922172156_drop_unique_namespaces_index_on_name_and_parent_id.rb new file mode 100644 index 00000000000..001f3a6964b --- /dev/null +++ b/db/migrate/20210922172156_drop_unique_namespaces_index_on_name_and_parent_id.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class DropUniqueNamespacesIndexOnNameAndParentId < Gitlab::Database::Migration[1.0] + INDEX_NAME = 'index_namespaces_on_name_and_parent_id' + + disable_ddl_transaction! + + def up + remove_concurrent_index_by_name :namespaces, INDEX_NAME + end + + def down + add_concurrent_index :namespaces, [:name, :parent_id], unique: true, name: INDEX_NAME + end +end diff --git a/db/migrate/20210929121516_add_releases_author_id_id_created_at_index.rb b/db/migrate/20210929121516_add_releases_author_id_id_created_at_index.rb new file mode 100644 index 00000000000..60ca3040d70 --- /dev/null +++ b/db/migrate/20210929121516_add_releases_author_id_id_created_at_index.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true +class AddReleasesAuthorIdIdCreatedAtIndex < Gitlab::Database::Migration[1.0] + disable_ddl_transaction! + + INDEX_NAME = 'index_releases_on_author_id_id_created_at' + + def up + add_concurrent_index :releases, [:author_id, :id, :created_at], name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :releases, INDEX_NAME + end +end diff --git a/db/schema_migrations/20210922172056 b/db/schema_migrations/20210922172056 new file mode 100644 index 00000000000..834280df2a8 --- /dev/null +++ b/db/schema_migrations/20210922172056 @@ -0,0 +1 @@ +c50ccd9986188356776c2d19c5544a6da6e31b5bb1b16ed259455604cb6fd862
\ No newline at end of file diff --git a/db/schema_migrations/20210922172156 b/db/schema_migrations/20210922172156 new file mode 100644 index 00000000000..06852710217 --- /dev/null +++ b/db/schema_migrations/20210922172156 @@ -0,0 +1 @@ +083b18b8e687ae8ff9d93ee77c6d4fc2916a2f1b77acf44132f216236b0ff06c
\ No newline at end of file diff --git a/db/schema_migrations/20210929121516 b/db/schema_migrations/20210929121516 new file mode 100644 index 00000000000..c42d39c9c34 --- /dev/null +++ b/db/schema_migrations/20210929121516 @@ -0,0 +1 @@ +432dc1f1e0280a79e4a6af56c2f2cb40c99edbc09e254b82b7f48c7c9217372b
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 8bae9a06707..e2acf112331 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -25683,6 +25683,8 @@ CREATE INDEX index_namespaces_id_parent_id_is_not_null ON namespaces USING btree CREATE INDEX index_namespaces_id_parent_id_is_null ON namespaces USING btree (id) WHERE (parent_id IS NULL); +CREATE UNIQUE INDEX index_namespaces_name_parent_id_type ON namespaces USING btree (name, parent_id, type); + CREATE INDEX index_namespaces_on_created_at ON namespaces USING btree (created_at); CREATE INDEX index_namespaces_on_custom_project_templates_group_id_and_type ON namespaces USING btree (custom_project_templates_group_id, type) WHERE (custom_project_templates_group_id IS NOT NULL); @@ -25693,8 +25695,6 @@ CREATE INDEX index_namespaces_on_ldap_sync_last_successful_update_at ON namespac CREATE INDEX index_namespaces_on_ldap_sync_last_update_at ON namespaces USING btree (ldap_sync_last_update_at); -CREATE UNIQUE INDEX index_namespaces_on_name_and_parent_id ON namespaces USING btree (name, parent_id); - CREATE INDEX index_namespaces_on_name_trigram ON namespaces USING gin (name gin_trgm_ops); CREATE INDEX index_namespaces_on_owner_id ON namespaces USING btree (owner_id); @@ -26263,6 +26263,8 @@ CREATE UNIQUE INDEX index_release_links_on_release_id_and_url ON release_links U CREATE INDEX index_releases_on_author_id ON releases USING btree (author_id); +CREATE INDEX index_releases_on_author_id_id_created_at ON releases USING btree (author_id, id, created_at); + CREATE INDEX index_releases_on_project_id_and_tag ON releases USING btree (project_id, tag); CREATE INDEX index_releases_on_released_at ON releases USING btree (released_at); diff --git a/lib/gitlab/database/migrations/instrumentation.rb b/lib/gitlab/database/migrations/instrumentation.rb index d1e55eb825c..6e5ffb74411 100644 --- a/lib/gitlab/database/migrations/instrumentation.rb +++ b/lib/gitlab/database/migrations/instrumentation.rb @@ -4,21 +4,21 @@ module Gitlab module Database module Migrations class Instrumentation - RESULT_DIR = Rails.root.join('tmp', 'migration-testing').freeze STATS_FILENAME = 'migration-stats.json' attr_reader :observations - def initialize(observer_classes = ::Gitlab::Database::Migrations::Observers.all_observers) + def initialize(result_dir:, observer_classes: ::Gitlab::Database::Migrations::Observers.all_observers) @observer_classes = observer_classes @observations = [] + @result_dir = result_dir end def observe(version:, name:, &block) observation = Observation.new(version, name) observation.success = true - observers = observer_classes.map { |c| c.new(observation) } + observers = observer_classes.map { |c| c.new(observation, @result_dir) } exception = nil diff --git a/lib/gitlab/database/migrations/observers/migration_observer.rb b/lib/gitlab/database/migrations/observers/migration_observer.rb index 85d18abb9ef..106f8f1f829 100644 --- a/lib/gitlab/database/migrations/observers/migration_observer.rb +++ b/lib/gitlab/database/migrations/observers/migration_observer.rb @@ -5,11 +5,12 @@ module Gitlab module Migrations module Observers class MigrationObserver - attr_reader :connection, :observation + attr_reader :connection, :observation, :output_dir - def initialize(observation) + def initialize(observation, output_dir) @connection = ActiveRecord::Base.connection @observation = observation + @output_dir = output_dir end def before diff --git a/lib/gitlab/database/migrations/observers/query_details.rb b/lib/gitlab/database/migrations/observers/query_details.rb index dadacd2d2fc..8f4406e79a5 100644 --- a/lib/gitlab/database/migrations/observers/query_details.rb +++ b/lib/gitlab/database/migrations/observers/query_details.rb @@ -6,7 +6,7 @@ module Gitlab module Observers class QueryDetails < MigrationObserver def before - file_path = File.join(Instrumentation::RESULT_DIR, "#{observation.version}_#{observation.name}-query-details.json") + file_path = File.join(output_dir, "#{observation.version}_#{observation.name}-query-details.json") @file = File.open(file_path, 'wb') @writer = Oj::StreamWriter.new(@file, {}) @writer.push_array diff --git a/lib/gitlab/database/migrations/observers/query_log.rb b/lib/gitlab/database/migrations/observers/query_log.rb index e15d733d2a2..c42fd8bd23d 100644 --- a/lib/gitlab/database/migrations/observers/query_log.rb +++ b/lib/gitlab/database/migrations/observers/query_log.rb @@ -7,7 +7,7 @@ module Gitlab class QueryLog < MigrationObserver def before @logger_was = ActiveRecord::Base.logger - file_path = File.join(Instrumentation::RESULT_DIR, "#{observation.version}_#{observation.name}.log") + file_path = File.join(output_dir, "#{observation.version}_#{observation.name}.log") @logger = Logger.new(file_path) ActiveRecord::Base.logger = @logger end diff --git a/lib/gitlab/database/migrations/runner.rb b/lib/gitlab/database/migrations/runner.rb new file mode 100644 index 00000000000..cafff44452a --- /dev/null +++ b/lib/gitlab/database/migrations/runner.rb @@ -0,0 +1,98 @@ +# frozen_string_literal: true + +module Gitlab + module Database + module Migrations + class Runner + BASE_RESULT_DIR = Rails.root.join('tmp', 'migration-testing').freeze + + class << self + def up(legacy_pipeline: false) + result_dir = if legacy_pipeline + BASE_RESULT_DIR + else + BASE_RESULT_DIR.join('up') + end + + Runner.new(direction: :up, migrations: migrations_for_up, result_dir: result_dir) + end + + def down + Runner.new(direction: :down, migrations: migrations_for_down, result_dir: BASE_RESULT_DIR.join('down')) + end + + def migration_context + @migration_context ||= ApplicationRecord.connection.migration_context + end + + private + + def migrations_for_up + existing_versions = migration_context.get_all_versions.to_set + + migration_context.migrations.reject do |migration| + existing_versions.include?(migration.version) + end + end + + def migration_file_names_this_branch + `git diff --name-only origin/HEAD...HEAD db/post_migrate db/migrate`.split("\n") + end + + def migrations_for_down + versions_this_branch = migration_file_names_this_branch.map do |m_name| + m_name.match(%r{^db/(post_)?migrate/(\d+)}) { |m| m.captures[1]&.to_i } + end.to_set + + existing_versions = migration_context.get_all_versions.to_set + migration_context.migrations.select do |migration| + existing_versions.include?(migration.version) && versions_this_branch.include?(migration.version) + end + end + end + + attr_reader :direction, :result_dir, :migrations + + delegate :migration_context, to: :class + + def initialize(direction:, migrations:, result_dir:) + raise "Direction must be up or down" unless %i[up down].include?(direction) + + @direction = direction + @migrations = migrations + @result_dir = result_dir + end + + def run + FileUtils.mkdir_p(result_dir) + + verbose_was = ActiveRecord::Migration.verbose + ActiveRecord::Migration.verbose = true + + sorted_migrations = migrations.sort_by(&:version) + sorted_migrations.reverse! if direction == :down + + instrumentation = Instrumentation.new(result_dir: result_dir) + + sorted_migrations.each do |migration| + instrumentation.observe(version: migration.version, name: migration.name) do + ActiveRecord::Migrator.new(direction, migration_context.migrations, migration_context.schema_migration, migration.version).run + end + end + ensure + if instrumentation + File.open(File.join(result_dir, Gitlab::Database::Migrations::Instrumentation::STATS_FILENAME), 'wb+') do |io| + io << instrumentation.observations.to_json + end + end + + # We clear the cache here to mirror the cache clearing that happens at the end of `db:migrate` tasks + # This clearing makes subsequent rake tasks in the same execution pick up database schema changes caused by + # the migrations that were just executed + ApplicationRecord.clear_cache! + ActiveRecord::Migration.verbose = verbose_was + end + end + end + end +end diff --git a/lib/gitlab/instrumentation_helper.rb b/lib/gitlab/instrumentation_helper.rb index 23acf1e8e86..26e44d7822e 100644 --- a/lib/gitlab/instrumentation_helper.rb +++ b/lib/gitlab/instrumentation_helper.rb @@ -131,18 +131,43 @@ module Gitlab enqueued_at_time = convert_to_time(enqueued_at) return unless enqueued_at_time - # Its possible that if theres clock-skew between two nodes - # this value may be less than zero. In that event, we record the value + round_elapsed_time(enqueued_at_time) + end + + # Returns the time it took for a scheduled job to be enqueued in seconds, as a float, + # if the `scheduled_at` and `enqueued_at` fields are available. + # + # * If the job doesn't contain sufficient information, returns nil + # * If the job has a start time in the future, returns 0 + # * If the job contains an invalid start time value, returns nil + # @param [Hash] job a Sidekiq job, represented as a hash + def self.enqueue_latency_for_scheduled_job(job) + scheduled_at = job['scheduled_at'] + enqueued_at = job['enqueued_at'] + + return unless scheduled_at && enqueued_at + + scheduled_at_time = convert_to_time(scheduled_at) + enqueued_at_time = convert_to_time(enqueued_at) + + return unless scheduled_at_time && enqueued_at_time + + round_elapsed_time(scheduled_at_time, enqueued_at_time) + end + + def self.round_elapsed_time(start, end_time = Time.now) + # It's possible that if there is clock-skew between two nodes this + # value may be less than zero. In that event, we record the value # as zero. - [elapsed_by_absolute_time(enqueued_at_time), 0].max.round(DURATION_PRECISION) + [elapsed_by_absolute_time(start, end_time), 0].max.round(DURATION_PRECISION) end # Calculates the time in seconds, as a float, from # the provided start time until now # # @param [Time] start - def self.elapsed_by_absolute_time(start) - (Time.now - start).to_f.round(DURATION_PRECISION) + def self.elapsed_by_absolute_time(start, end_time) + (end_time - start).to_f.round(DURATION_PRECISION) end private_class_method :elapsed_by_absolute_time diff --git a/lib/gitlab/sidekiq_logging/structured_logger.rb b/lib/gitlab/sidekiq_logging/structured_logger.rb index 8db6937747c..3438bc0f3ef 100644 --- a/lib/gitlab/sidekiq_logging/structured_logger.rb +++ b/lib/gitlab/sidekiq_logging/structured_logger.rb @@ -55,6 +55,9 @@ module Gitlab scheduling_latency_s = ::Gitlab::InstrumentationHelper.queue_duration_for_job(payload) payload['scheduling_latency_s'] = scheduling_latency_s if scheduling_latency_s + enqueue_latency_s = ::Gitlab::InstrumentationHelper.enqueue_latency_for_scheduled_job(payload) + payload['enqueue_latency_s'] = enqueue_latency_s if enqueue_latency_s + payload end diff --git a/lib/gitlab/sidekiq_middleware/client_metrics.rb b/lib/gitlab/sidekiq_middleware/client_metrics.rb index d54d817e407..ef80ed706f3 100644 --- a/lib/gitlab/sidekiq_middleware/client_metrics.rb +++ b/lib/gitlab/sidekiq_middleware/client_metrics.rb @@ -16,7 +16,12 @@ module Gitlab worker_class = worker_class.to_s.safe_constantize labels = create_labels(worker_class, queue, job) - labels[:scheduling] = job.key?('at') ? 'delayed' : 'immediate' + if job.key?('at') + labels[:scheduling] = 'delayed' + job[:scheduled_at] = job['at'] + else + labels[:scheduling] = 'immediate' + end @metrics.fetch(ENQUEUED).increment(labels, 1) diff --git a/lib/gitlab/usage/metrics/instrumentations/count_users_associating_milestones_to_releases_metric.rb b/lib/gitlab/usage/metrics/instrumentations/count_users_associating_milestones_to_releases_metric.rb new file mode 100644 index 00000000000..c10182e23aa --- /dev/null +++ b/lib/gitlab/usage/metrics/instrumentations/count_users_associating_milestones_to_releases_metric.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module Gitlab + module Usage + module Metrics + module Instrumentations + class CountUsersAssociatingMilestonesToReleasesMetric < DatabaseMetric + operation :distinct_count, column: :author_id + + relation { Release.with_milestones } + + start { Release.minimum(:author_id) } + finish { Release.maximum(:author_id) } + end + end + end + end +end diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb index 854242031be..87ba1f72428 100644 --- a/lib/gitlab/usage_data.rb +++ b/lib/gitlab/usage_data.rb @@ -647,7 +647,7 @@ module Gitlab # Omitted because of encrypted properties: `projects_jira_cloud_active`, `projects_jira_server_active` # rubocop: disable CodeReuse/ActiveRecord def usage_activity_by_stage_plan(time_period) - time_frame = time_period.present? ? '28d' : 'none' + time_frame = metric_time_period(time_period) { issues: add_metric('CountUsersCreatingIssuesMetric', time_frame: time_frame), notes: distinct_count(::Note.where(time_period), :author_id), @@ -665,11 +665,13 @@ module Gitlab # Omitted because no user, creator or author associated: `environments`, `feature_flags`, `in_review_folder`, `pages_domains` # rubocop: disable CodeReuse/ActiveRecord def usage_activity_by_stage_release(time_period) + time_frame = metric_time_period(time_period) { deployments: distinct_count(::Deployment.where(time_period), :user_id), failed_deployments: distinct_count(::Deployment.failed.where(time_period), :user_id), releases: distinct_count(::Release.where(time_period), :author_id), - successful_deployments: distinct_count(::Deployment.success.where(time_period), :user_id) + successful_deployments: distinct_count(::Deployment.success.where(time_period), :user_id), + releases_with_milestones: add_metric('CountUsersAssociatingMilestonesToReleasesMetric', time_frame: time_frame) } end # rubocop: enable CodeReuse/ActiveRecord @@ -755,6 +757,10 @@ module Gitlab private + def metric_time_period(time_period) + time_period.present? ? '28d' : 'none' + end + def gitaly_apdex with_prometheus_client(verify: false, fallback: FALLBACK) do |client| result = client.query('avg_over_time(gitlab_usage_ping:gitaly_apdex:ratio_avg_over_time_5m[1w])').first diff --git a/lib/tasks/gitlab/db.rake b/lib/tasks/gitlab/db.rake index a6738b01f18..b3406e815ed 100644 --- a/lib/tasks/gitlab/db.rake +++ b/lib/tasks/gitlab/db.rake @@ -211,37 +211,22 @@ namespace :gitlab do exit 0 end - desc 'Run migrations with instrumentation' - task migration_testing: :environment do - result_dir = Gitlab::Database::Migrations::Instrumentation::RESULT_DIR - FileUtils.mkdir_p(result_dir) - - verbose_was = ActiveRecord::Migration.verbose - ActiveRecord::Migration.verbose = true - - ctx = ActiveRecord::Base.connection.migration_context - existing_versions = ctx.get_all_versions.to_set - - pending_migrations = ctx.migrations.reject do |migration| - existing_versions.include?(migration.version) + namespace :migration_testing do + desc 'Run migrations with instrumentation' + task up: :environment do + Gitlab::Database::Migrations::Runner.up.run end - instrumentation = Gitlab::Database::Migrations::Instrumentation.new - - pending_migrations.each do |migration| - instrumentation.observe(version: migration.version, name: migration.name) do - ActiveRecord::Migrator.new(:up, ctx.migrations, ctx.schema_migration, migration.version).run - end - end - ensure - if instrumentation - File.open(File.join(result_dir, Gitlab::Database::Migrations::Instrumentation::STATS_FILENAME), 'wb+') do |io| - io << instrumentation.observations.to_json - end + desc 'Run down migrations in current branch with instrumentation' + task down: :environment do + Gitlab::Database::Migrations::Runner.down.run end + end - ActiveRecord::Base.clear_cache! - ActiveRecord::Migration.verbose = verbose_was + # TODO: Remove this rake task after migrating the database testing runner to :up / :down versions of it + desc 'Run migrations with instrumentation' + task migration_testing: :environment do + Gitlab::Database::Migrations::Runner.up(legacy_pipeline: true).run end desc 'Run all pending batched migrations' diff --git a/package.json b/package.json index 87d8dd7d674..b55981ce95e 100644 --- a/package.json +++ b/package.json @@ -63,19 +63,19 @@ "@rails/ujs": "6.1.4-1", "@sentry/browser": "5.30.0", "@sourcegraph/code-host-integration": "0.0.60", - "@tiptap/core": "^2.0.0-beta.116", + "@tiptap/core": "^2.0.0-beta.118", "@tiptap/extension-blockquote": "^2.0.0-beta.15", "@tiptap/extension-bold": "^2.0.0-beta.15", "@tiptap/extension-bullet-list": "^2.0.0-beta.15", "@tiptap/extension-code": "^2.0.0-beta.16", - "@tiptap/extension-code-block-lowlight": "2.0.0-beta.40", + "@tiptap/extension-code-block-lowlight": "2.0.0-beta.41", "@tiptap/extension-document": "^2.0.0-beta.13", "@tiptap/extension-dropcursor": "^2.0.0-beta.19", - "@tiptap/extension-gapcursor": "^2.0.0-beta.23", - "@tiptap/extension-hard-break": "^2.0.0-beta.20", + "@tiptap/extension-gapcursor": "^2.0.0-beta.24", + "@tiptap/extension-hard-break": "^2.0.0-beta.21", "@tiptap/extension-heading": "^2.0.0-beta.15", "@tiptap/extension-history": "^2.0.0-beta.16", - "@tiptap/extension-horizontal-rule": "^2.0.0-beta.20", + "@tiptap/extension-horizontal-rule": "^2.0.0-beta.21", "@tiptap/extension-image": "^2.0.0-beta.15", "@tiptap/extension-italic": "^2.0.0-beta.15", "@tiptap/extension-link": "^2.0.0-beta.20", @@ -92,7 +92,7 @@ "@tiptap/extension-task-item": "^2.0.0-beta.18", "@tiptap/extension-task-list": "^2.0.0-beta.17", "@tiptap/extension-text": "^2.0.0-beta.13", - "@tiptap/vue-2": "^2.0.0-beta.56", + "@tiptap/vue-2": "^2.0.0-beta.57", "@toast-ui/editor": "^2.5.2", "@toast-ui/vue-editor": "^2.5.2", "apollo-cache-inmemory": "^1.6.6", diff --git a/qa/qa/page/merge_request/show.rb b/qa/qa/page/merge_request/show.rb index 28f5d7b6998..d4fa3b38f02 100644 --- a/qa/qa/page/merge_request/show.rb +++ b/qa/qa/page/merge_request/show.rb @@ -25,6 +25,11 @@ module QA view 'app/assets/javascripts/diffs/components/compare_versions.vue' do element :target_version_dropdown + element :file_tree_button + end + + view 'app/assets/javascripts/diffs/components/tree_list.vue' do + element :file_tree_container end view 'app/assets/javascripts/diffs/components/diff_file_header.vue' do @@ -186,11 +191,17 @@ module QA end def has_file?(file_name) - has_element?(:file_name_content, text: file_name) + open_file_tree + has_element?(:file_name_content, file_name: file_name) end def has_no_file?(file_name) - has_no_element?(:file_name_content, text: file_name) + open_file_tree + has_no_element?(:file_name_content, file_name: file_name) + end + + def open_file_tree + click_element(:file_tree_button) unless has_element?(:file_tree_container) end def has_merge_button? diff --git a/qa/qa/specs/features/browser_ui/3_create/merge_request/revert/reverting_merge_request_spec.rb b/qa/qa/specs/features/browser_ui/3_create/merge_request/revert/reverting_merge_request_spec.rb index 0ea294b8e51..33a8fb49118 100644 --- a/qa/qa/specs/features/browser_ui/3_create/merge_request/revert/reverting_merge_request_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/merge_request/revert/reverting_merge_request_spec.rb @@ -1,11 +1,7 @@ # frozen_string_literal: true module QA - RSpec.describe 'Create', quarantine: { - only: { job: 'large-setup' }, - issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/338324', - type: :stale - } do + RSpec.describe 'Create' do describe 'Merged merge request' do let(:project) do Resource::Project.fabricate_via_api! do |project| diff --git a/spec/lib/gitlab/database/migrations/instrumentation_spec.rb b/spec/lib/gitlab/database/migrations/instrumentation_spec.rb index 5945e5a2039..841d2a98a16 100644 --- a/spec/lib/gitlab/database/migrations/instrumentation_spec.rb +++ b/spec/lib/gitlab/database/migrations/instrumentation_spec.rb @@ -2,8 +2,13 @@ require 'spec_helper' RSpec.describe Gitlab::Database::Migrations::Instrumentation do + let(:result_dir) { Dir.mktmpdir } + + after do + FileUtils.rm_rf(result_dir) + end describe '#observe' do - subject { described_class.new } + subject { described_class.new(result_dir: result_dir) } let(:migration_name) { 'test' } let(:migration_version) { '12345' } @@ -13,7 +18,7 @@ RSpec.describe Gitlab::Database::Migrations::Instrumentation do end context 'behavior with observers' do - subject { described_class.new([Gitlab::Database::Migrations::Observers::MigrationObserver]).observe(version: migration_version, name: migration_name) {} } + subject { described_class.new(observer_classes: [Gitlab::Database::Migrations::Observers::MigrationObserver], result_dir: result_dir).observe(version: migration_version, name: migration_name) {} } let(:observer) { instance_double('Gitlab::Database::Migrations::Observers::MigrationObserver', before: nil, after: nil, record: nil) } @@ -24,7 +29,7 @@ RSpec.describe Gitlab::Database::Migrations::Instrumentation do it 'instantiates observer with observation' do expect(Gitlab::Database::Migrations::Observers::MigrationObserver) .to receive(:new) - .with(instance_of(Gitlab::Database::Migrations::Observation)) { |observation| expect(observation.version).to eq(migration_version) } + .with(instance_of(Gitlab::Database::Migrations::Observation), anything) { |observation| expect(observation.version).to eq(migration_version) } .and_return(observer) subject @@ -58,7 +63,7 @@ RSpec.describe Gitlab::Database::Migrations::Instrumentation do end context 'on successful execution' do - subject { described_class.new.observe(version: migration_version, name: migration_name) {} } + subject { described_class.new(result_dir: result_dir).observe(version: migration_version, name: migration_name) {} } it 'records walltime' do expect(subject.walltime).not_to be_nil @@ -78,7 +83,7 @@ RSpec.describe Gitlab::Database::Migrations::Instrumentation do end context 'upon failure' do - subject { described_class.new.observe(version: migration_version, name: migration_name) { raise 'something went wrong' } } + subject { described_class.new(result_dir: result_dir).observe(version: migration_version, name: migration_name) { raise 'something went wrong' } } it 'raises the exception' do expect { subject }.to raise_error(/something went wrong/) @@ -93,7 +98,7 @@ RSpec.describe Gitlab::Database::Migrations::Instrumentation do # ignore end - let(:instance) { described_class.new } + let(:instance) { described_class.new(result_dir: result_dir) } it 'records walltime' do expect(subject.walltime).not_to be_nil @@ -114,7 +119,7 @@ RSpec.describe Gitlab::Database::Migrations::Instrumentation do end context 'sequence of migrations with failures' do - subject { described_class.new } + subject { described_class.new(result_dir: result_dir) } let(:migration1) { double('migration1', call: nil) } let(:migration2) { double('migration2', call: nil) } diff --git a/spec/lib/gitlab/database/migrations/observers/query_details_spec.rb b/spec/lib/gitlab/database/migrations/observers/query_details_spec.rb index 36885a1594f..191ac29e3b3 100644 --- a/spec/lib/gitlab/database/migrations/observers/query_details_spec.rb +++ b/spec/lib/gitlab/database/migrations/observers/query_details_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' RSpec.describe Gitlab::Database::Migrations::Observers::QueryDetails do - subject { described_class.new(observation) } + subject { described_class.new(observation, directory_path) } let(:observation) { Gitlab::Database::Migrations::Observation.new(migration_version, migration_name) } let(:connection) { ActiveRecord::Base.connection } @@ -14,10 +14,6 @@ RSpec.describe Gitlab::Database::Migrations::Observers::QueryDetails do let(:migration_version) { 20210422152437 } let(:migration_name) { 'test' } - before do - stub_const('Gitlab::Database::Migrations::Instrumentation::RESULT_DIR', directory_path) - end - after do FileUtils.remove_entry(directory_path) end diff --git a/spec/lib/gitlab/database/migrations/observers/query_log_spec.rb b/spec/lib/gitlab/database/migrations/observers/query_log_spec.rb index 2a49d8e8b73..2e70a85fd5b 100644 --- a/spec/lib/gitlab/database/migrations/observers/query_log_spec.rb +++ b/spec/lib/gitlab/database/migrations/observers/query_log_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' RSpec.describe Gitlab::Database::Migrations::Observers::QueryLog do - subject { described_class.new(observation) } + subject { described_class.new(observation, directory_path) } let(:observation) { Gitlab::Database::Migrations::Observation.new(migration_version, migration_name) } let(:connection) { ActiveRecord::Base.connection } @@ -11,10 +11,6 @@ RSpec.describe Gitlab::Database::Migrations::Observers::QueryLog do let(:migration_version) { 20210422152437 } let(:migration_name) { 'test' } - before do - stub_const('Gitlab::Database::Migrations::Instrumentation::RESULT_DIR', directory_path) - end - after do FileUtils.remove_entry(directory_path) end diff --git a/spec/lib/gitlab/database/migrations/observers/query_statistics_spec.rb b/spec/lib/gitlab/database/migrations/observers/query_statistics_spec.rb index 32a25fdaa28..9727a215d71 100644 --- a/spec/lib/gitlab/database/migrations/observers/query_statistics_spec.rb +++ b/spec/lib/gitlab/database/migrations/observers/query_statistics_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' RSpec.describe Gitlab::Database::Migrations::Observers::QueryStatistics do - subject { described_class.new(observation) } + subject { described_class.new(observation, double("unused path")) } let(:observation) { Gitlab::Database::Migrations::Observation.new } let(:connection) { ActiveRecord::Base.connection } diff --git a/spec/lib/gitlab/database/migrations/observers/total_database_size_change_spec.rb b/spec/lib/gitlab/database/migrations/observers/total_database_size_change_spec.rb index 61e28003e66..e689759c574 100644 --- a/spec/lib/gitlab/database/migrations/observers/total_database_size_change_spec.rb +++ b/spec/lib/gitlab/database/migrations/observers/total_database_size_change_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' RSpec.describe Gitlab::Database::Migrations::Observers::TotalDatabaseSizeChange do - subject { described_class.new(observation) } + subject { described_class.new(observation, double('unused path')) } let(:observation) { Gitlab::Database::Migrations::Observation.new } let(:connection) { ActiveRecord::Base.connection } diff --git a/spec/lib/gitlab/database/migrations/runner_spec.rb b/spec/lib/gitlab/database/migrations/runner_spec.rb new file mode 100644 index 00000000000..104a71d7d1b --- /dev/null +++ b/spec/lib/gitlab/database/migrations/runner_spec.rb @@ -0,0 +1,117 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe Gitlab::Database::Migrations::Runner do + let(:result_dir) { Pathname.new(Dir.mktmpdir) } + + let(:migration_runs) { [] } # This list gets populated as the runner tries to run migrations + + # Tests depend on all of these lists being sorted in the order migrations would be applied + let(:applied_migrations_other_branches) { [double(ActiveRecord::Migration, version: 1, name: 'migration_complete_other_branch')] } + + let(:applied_migrations_this_branch) do + [ + double(ActiveRecord::Migration, version: 2, name: 'older_migration_complete_this_branch'), + double(ActiveRecord::Migration, version: 3, name: 'newer_migration_complete_this_branch') + ].sort_by(&:version) + end + + let(:pending_migrations) do + [ + double(ActiveRecord::Migration, version: 4, name: 'older_migration_pending'), + double(ActiveRecord::Migration, version: 5, name: 'newer_migration_pending') + ].sort_by(&:version) + end + + before do + stub_const('Gitlab::Database::Migrations::Runner::BASE_RESULT_DIR', result_dir) + allow(ActiveRecord::Migrator).to receive(:new) do |dir, _all_migrations, _schema_migration_class, version_to_migrate| + migrator = double(ActiveRecord::Migrator) + expect(migrator).to receive(:run) do + migration_runs << OpenStruct.new(dir: dir, version_to_migrate: version_to_migrate) + end + migrator + end + + all_versions = (applied_migrations_other_branches + applied_migrations_this_branch).map(&:version) + migrations = applied_migrations_other_branches + applied_migrations_this_branch + pending_migrations + ctx = double(ActiveRecord::MigrationContext, get_all_versions: all_versions, migrations: migrations, schema_migration: ActiveRecord::SchemaMigration) + + allow(described_class).to receive(:migration_context).and_return(ctx) + + names_this_branch = (applied_migrations_this_branch + pending_migrations).map { |m| "db/migrate/#{m.version}_#{m.name}.rb"} + allow(described_class).to receive(:migration_file_names_this_branch).and_return(names_this_branch) + end + + after do + FileUtils.rm_rf(result_dir) + end + + it 'creates the results dir when one does not exist' do + FileUtils.rm_rf(result_dir) + + expect do + described_class.new(direction: :up, migrations: [], result_dir: result_dir).run + end.to change { Dir.exist?(result_dir) }.from(false).to(true) + end + + describe '.up' do + context 'result directory' do + context 'legacy mode' do + it 'uses the root result directory' do + expect(described_class.up(legacy_pipeline: true).result_dir).to eq(result_dir) + end + end + + context 'not legacy mode' do + it 'uses the /up subdirectory' do + expect(described_class.up.result_dir).to eq(result_dir.join('up')) + end + end + end + + context 'migrations to run' do + subject(:up) { described_class.up } + + it 'is the list of pending migrations' do + expect(up.migrations).to eq(pending_migrations) + end + end + + context 'running migrations' do + subject(:up) { described_class.up } + + it 'runs the unapplied migrations in version order', :aggregate_failures do + up.run + + expect(migration_runs.map(&:dir)).to eq([:up, :up]) + expect(migration_runs.map(&:version_to_migrate)).to eq(pending_migrations.map(&:version)) + end + end + end + + describe '.down' do + subject(:down) { described_class.down } + + context 'result directory' do + it 'is the /down subdirectory' do + expect(down.result_dir).to eq(result_dir.join('down')) + end + end + + context 'migrations to run' do + it 'is the list of migrations that are up and on this branch' do + expect(down.migrations).to eq(applied_migrations_this_branch) + end + end + + context 'running migrations' do + it 'runs the applied migrations for the current branch in reverse order', :aggregate_failures do + down.run + + expect(migration_runs.map(&:dir)).to eq([:down, :down]) + expect(migration_runs.map(&:version_to_migrate)).to eq(applied_migrations_this_branch.reverse.map(&:version)) + end + end + end +end diff --git a/spec/lib/gitlab/instrumentation_helper_spec.rb b/spec/lib/gitlab/instrumentation_helper_spec.rb index 6e95480830c..52d3623c304 100644 --- a/spec/lib/gitlab/instrumentation_helper_spec.rb +++ b/spec/lib/gitlab/instrumentation_helper_spec.rb @@ -149,8 +149,8 @@ RSpec.describe Gitlab::InstrumentationHelper do end end - describe '.queue_duration_for_job' do - where(:enqueued_at, :created_at, :time_now, :expected_duration) do + describe 'duration calculations' do + where(:end_time, :start_time, :time_now, :expected_duration) do "2019-06-01T00:00:00.000+0000" | nil | "2019-06-01T02:00:00.000+0000" | 2.hours.to_f "2019-06-01T02:00:00.000+0000" | nil | "2019-06-01T02:00:00.001+0000" | 0.001 "2019-06-01T02:00:00.000+0000" | "2019-05-01T02:00:00.000+0000" | "2019-06-01T02:00:01.000+0000" | 1 @@ -164,15 +164,29 @@ RSpec.describe Gitlab::InstrumentationHelper do 0 | nil | "2019-10-23T12:13:16.000+0200" | nil -1 | nil | "2019-10-23T12:13:16.000+0200" | nil "2019-06-01T02:00:00.000+0000" | nil | "2019-06-01T00:00:00.000+0000" | 0 - Time.at(1571999233) | nil | "2019-10-25T12:29:16.000+0200" | 123 + Time.at(1571999233).utc | nil | "2019-10-25T12:29:16.000+0200" | 123 end - with_them do - let(:job) { { 'enqueued_at' => enqueued_at, 'created_at' => created_at } } + describe '.queue_duration_for_job' do + with_them do + let(:job) { { 'enqueued_at' => end_time, 'created_at' => start_time } } - it "returns the correct duration" do - Timecop.freeze(Time.iso8601(time_now)) do - expect(described_class.queue_duration_for_job(job)).to eq(expected_duration) + it "returns the correct duration" do + travel_to(Time.iso8601(time_now)) do + expect(described_class.queue_duration_for_job(job)).to eq(expected_duration) + end + end + end + end + + describe '.enqueue_latency_for_scheduled_job' do + with_them do + let(:job) { { 'enqueued_at' => end_time, 'scheduled_at' => start_time } } + + it "returns the correct duration" do + travel_to(Time.iso8601(time_now)) do + expect(described_class.enqueue_latency_for_scheduled_job(job)).to eq(expected_duration) + end end end end diff --git a/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb b/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb index 44b593d8c0a..f10ca1e9216 100644 --- a/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb +++ b/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb @@ -18,7 +18,7 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do end it 'logs start and end of job' do - Timecop.freeze(timestamp) do + travel_to(timestamp) do expect(logger).to receive(:info).with(start_payload).ordered expect(logger).to receive(:info).with(end_payload).ordered expect(subject).to receive(:log_job_start).and_call_original @@ -34,7 +34,7 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do "wrapped" => "TestWorker" ) - Timecop.freeze(timestamp) do + travel_to(timestamp) do expect(logger).to receive(:info).with(start_payload).ordered expect(logger).to receive(:info).with(end_payload).ordered expect(subject).to receive(:log_job_start).and_call_original @@ -45,7 +45,7 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do end it 'logs an exception in job' do - Timecop.freeze(timestamp) do + travel_to(timestamp) do expect(logger).to receive(:info).with(start_payload) expect(logger).to receive(:warn).with(include(exception_payload)) expect(subject).to receive(:log_job_start).and_call_original @@ -60,7 +60,7 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do end it 'logs the root cause of an Sidekiq::JobRetry::Skip exception in the job' do - Timecop.freeze(timestamp) do + travel_to(timestamp) do expect(logger).to receive(:info).with(start_payload) expect(logger).to receive(:warn).with(include(exception_payload)) expect(subject).to receive(:log_job_start).and_call_original @@ -77,7 +77,7 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do end it 'logs the root cause of an Sidekiq::JobRetry::Handled exception in the job' do - Timecop.freeze(timestamp) do + travel_to(timestamp) do expect(logger).to receive(:info).with(start_payload) expect(logger).to receive(:warn).with(include(exception_payload)) expect(subject).to receive(:log_job_start).and_call_original @@ -94,7 +94,7 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do end it 'keeps Sidekiq::JobRetry::Handled exception if the cause does not exist' do - Timecop.freeze(timestamp) do + travel_to(timestamp) do expect(logger).to receive(:info).with(start_payload) expect(logger).to receive(:warn).with( include( @@ -116,7 +116,7 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do end it 'does not modify the job' do - Timecop.freeze(timestamp) do + travel_to(timestamp) do job_copy = job.deep_dup allow(logger).to receive(:info) @@ -130,7 +130,7 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do end it 'does not modify the wrapped job' do - Timecop.freeze(timestamp) do + travel_to(timestamp) do wrapped_job = job.merge( "class" => "ActiveJob::QueueAdapters::SidekiqAdapter::JobWrapper", "wrapped" => "TestWorker" @@ -154,7 +154,7 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do end it 'logs start and end of job without args' do - Timecop.freeze(timestamp) do + travel_to(timestamp) do expect(logger).to receive(:info).with(start_payload.except('args')).ordered expect(logger).to receive(:info).with(end_payload.except('args')).ordered expect(subject).to receive(:log_job_start).and_call_original @@ -165,7 +165,7 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do end it 'logs without created_at and enqueued_at fields' do - Timecop.freeze(timestamp) do + travel_to(timestamp) do excluded_fields = %w(created_at enqueued_at args scheduling_latency_s) expect(logger).to receive(:info).with(start_payload.except(*excluded_fields)).ordered @@ -183,7 +183,7 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do let(:scheduling_latency_s) { 7200.0 } it 'logs with scheduling latency' do - Timecop.freeze(timestamp) do + travel_to(timestamp) do expect(logger).to receive(:info).with(start_payload).ordered expect(logger).to receive(:info).with(end_payload).ordered expect(subject).to receive(:log_job_start).and_call_original @@ -194,6 +194,35 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do end end + context 'with enqueue latency' do + let(:expected_start_payload) do + start_payload.merge( + 'scheduled_at' => job['scheduled_at'], + 'enqueue_latency_s' => 1.hour.to_f + ) + end + + let(:expected_end_payload) do + end_payload.merge('enqueue_latency_s' => 1.hour.to_f) + end + + before do + # enqueued_at is set to created_at + job['scheduled_at'] = created_at - 1.hour + end + + it 'logs with scheduling latency' do + travel_to(timestamp) do + expect(logger).to receive(:info).with(expected_start_payload).ordered + expect(logger).to receive(:info).with(expected_end_payload).ordered + expect(subject).to receive(:log_job_start).and_call_original + expect(subject).to receive(:log_job_done).and_call_original + + call_subject(job, 'test_queue') { } + end + end + end + context 'with Gitaly, Rugged, and Redis calls' do let(:timing_data) do { @@ -218,7 +247,7 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do end it 'logs with Gitaly and Rugged timing data', :aggregate_failures do - Timecop.freeze(timestamp) do + travel_to(timestamp) do expect(logger).to receive(:info).with(start_payload).ordered expect(logger).to receive(:info).with(expected_end_payload).ordered @@ -323,7 +352,7 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do end it 'logs it in the done log' do - Timecop.freeze(timestamp) do + travel_to(timestamp) do expect(logger).to receive(:info).with(expected_start_payload).ordered expect(logger).to receive(:info).with(expected_end_payload).ordered @@ -365,7 +394,7 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do end it 'logs it in the done log' do - Timecop.freeze(timestamp) do + travel_to(timestamp) do expect(logger).to receive(:info).with(expected_start_payload).ordered expect(logger).to receive(:info).with(expected_end_payload).ordered @@ -390,13 +419,13 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do 'message' => 'my-message', 'job_status' => 'my-job-status', 'duration_s' => 0.123123, - 'completed_at' => current_utc_time.to_f } + 'completed_at' => current_utc_time.to_i } end subject { described_class.new } it 'update payload correctly' do - Timecop.freeze(current_utc_time) do + travel_to(current_utc_time) do subject.send(:add_time_keys!, time, payload) expect(payload).to eq(payload_with_time_keys) diff --git a/spec/lib/gitlab/sidekiq_middleware/client_metrics_spec.rb b/spec/lib/gitlab/sidekiq_middleware/client_metrics_spec.rb index de5360d2dcd..dca00c85e30 100644 --- a/spec/lib/gitlab/sidekiq_middleware/client_metrics_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/client_metrics_spec.rb @@ -62,6 +62,14 @@ RSpec.describe Gitlab::SidekiqMiddleware::ClientMetrics do Sidekiq::Testing.inline! { TestWorker.perform_in(1.second) } end + + it 'sets the scheduled_at field' do + job = { 'at' => Time.current } + + subject.call('TestWorker', job, 'queue', nil) do + expect(job[:scheduled_at]).to eq(job['at']) + end + end end context 'when the worker class cannot be found' do diff --git a/spec/lib/gitlab/usage/metrics/instrumentations/count_users_associating_milestones_to_releases_metric_spec.rb b/spec/lib/gitlab/usage/metrics/instrumentations/count_users_associating_milestones_to_releases_metric_spec.rb new file mode 100644 index 00000000000..e2bb99c832a --- /dev/null +++ b/spec/lib/gitlab/usage/metrics/instrumentations/count_users_associating_milestones_to_releases_metric_spec.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Usage::Metrics::Instrumentations::CountUsersAssociatingMilestonesToReleasesMetric do + let_it_be(:release) { create(:release, created_at: 3.days.ago) } + let_it_be(:release_with_milestone) { create(:release, :with_milestones, created_at: 3.days.ago) } + + it_behaves_like 'a correct instrumented metric value', { time_frame: '28d', data_source: 'database' } do + let(:expected_value) { 1 } + end +end diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index 5bd2d6fda4b..38382590828 100644 --- a/spec/lib/gitlab/usage_data_spec.rb +++ b/spec/lib/gitlab/usage_data_spec.rb @@ -469,7 +469,8 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do for_defined_days_back do user = create(:user) create(:deployment, :failed, user: user) - create(:release, author: user) + release = create(:release, author: user) + create(:milestone, project: release.project, releases: [release]) create(:deployment, :success, user: user) end @@ -477,13 +478,15 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do deployments: 2, failed_deployments: 2, releases: 2, - successful_deployments: 2 + successful_deployments: 2, + releases_with_milestones: 2 ) expect(described_class.usage_activity_by_stage_release(described_class.monthly_time_range_db_params)).to include( deployments: 1, failed_deployments: 1, releases: 1, - successful_deployments: 1 + successful_deployments: 1, + releases_with_milestones: 1 ) end end diff --git a/spec/services/issues/close_service_spec.rb b/spec/services/issues/close_service_spec.rb index 14e6b44f7b0..7a54647fb07 100644 --- a/spec/services/issues/close_service_spec.rb +++ b/spec/services/issues/close_service_spec.rb @@ -22,6 +22,18 @@ RSpec.describe Issues::CloseService do describe '#execute' do let(:service) { described_class.new(project: project, current_user: user) } + context 'when skip_authorization is true' do + it 'does close the issue even if user is not authorized' do + non_authorized_user = create(:user) + + service = described_class.new(project: project, current_user: non_authorized_user) + + expect do + service.execute(issue, skip_authorization: true) + end.to change { issue.reload.state }.from('opened').to('closed') + end + end + it 'checks if the user is authorized to update the issue' do expect(service).to receive(:can?).with(user, :update_issue, issue) .and_call_original diff --git a/spec/services/issues/reopen_service_spec.rb b/spec/services/issues/reopen_service_spec.rb index 86190c4e475..c9469b861ac 100644 --- a/spec/services/issues/reopen_service_spec.rb +++ b/spec/services/issues/reopen_service_spec.rb @@ -8,18 +8,26 @@ RSpec.describe Issues::ReopenService do describe '#execute' do context 'when user is not authorized to reopen issue' do - before do + it 'does not reopen the issue' do guest = create(:user) project.add_guest(guest) - perform_enqueued_jobs do - described_class.new(project: project, current_user: guest).execute(issue) - end - end + described_class.new(project: project, current_user: guest).execute(issue) - it 'does not reopen the issue' do expect(issue).to be_closed end + + context 'when skip_authorization is true' do + it 'does close the issue even if user is not authorized' do + non_authorized_user = create(:user) + + service = described_class.new(project: project, current_user: non_authorized_user) + + expect do + service.execute(issue, skip_authorization: true) + end.to change { issue.reload.state }.from('closed').to('opened') + end + end end context 'when user is authorized to reopen issue' do diff --git a/spec/tasks/gitlab/db_rake_spec.rb b/spec/tasks/gitlab/db_rake_spec.rb index 91cd09fc6e6..ba434a0a46c 100644 --- a/spec/tasks/gitlab/db_rake_spec.rb +++ b/spec/tasks/gitlab/db_rake_spec.rb @@ -293,53 +293,37 @@ RSpec.describe 'gitlab:db namespace rake task', :silence_stdout do end describe '#migrate_with_instrumentation' do - subject { run_rake_task('gitlab:db:migration_testing') } + describe '#up' do + subject { run_rake_task('gitlab:db:migration_testing:up') } - let(:ctx) { double('ctx', migrations: all_migrations, schema_migration: double, get_all_versions: existing_versions) } - let(:instrumentation) { instance_double(Gitlab::Database::Migrations::Instrumentation, observations: observations) } - let(:existing_versions) { [1] } - let(:all_migrations) { [double('migration1', version: 1, name: 'test'), pending_migration] } - let(:pending_migration) { double('migration2', version: 2, name: 'test') } - let(:filename) { Gitlab::Database::Migrations::Instrumentation::STATS_FILENAME } - let(:result_dir) { Dir.mktmpdir } - let(:observations) { %w[some data] } + it 'delegates to the migration runner' do + expect(::Gitlab::Database::Migrations::Runner).to receive_message_chain(:up, :run) - before do - allow(ActiveRecord::Base.connection).to receive(:migration_context).and_return(ctx) - allow(Gitlab::Database::Migrations::Instrumentation).to receive(:new).and_return(instrumentation) - allow(ActiveRecord::Migrator).to receive_message_chain('new.run').with(any_args).with(no_args) - - allow(instrumentation).to receive(:observe).and_yield - - stub_const('Gitlab::Database::Migrations::Instrumentation::RESULT_DIR', result_dir) - end - - after do - FileUtils.rm_rf(result_dir) + subject + end end - it 'creates result directory when one does not exist' do - FileUtils.rm_rf(result_dir) + describe '#down' do + subject { run_rake_task('gitlab:db:migration_testing:down') } - expect { subject }.to change { Dir.exist?(result_dir) }.from(false).to(true) - end + it 'delegates to the migration runner' do + expect(::Gitlab::Database::Migrations::Runner).to receive_message_chain(:down, :run) - it 'instruments the pending migration' do - expect(instrumentation).to receive(:observe).with(version: 2, name: 'test').and_yield - - subject + subject + end end - it 'executes the pending migration' do - expect(ActiveRecord::Migrator).to receive_message_chain('new.run').with(:up, ctx.migrations, ctx.schema_migration, pending_migration.version).with(no_args) + describe 'legacy rake task' do + subject { run_rake_task('gitlab:db:migration_testing') } - subject - end + let(:runner) { double(Gitlab::Database::Migrations::Runner) } - it 'writes observations out to JSON file' do - subject + it 'delegates to the migration runner in legacy mode' do + expect(::Gitlab::Database::Migrations::Runner).to receive(:up).with(legacy_pipeline: true).and_return(runner) + expect(runner).to receive(:run) - expect(File.read(File.join(result_dir, filename))).to eq(observations.to_json) + subject + end end end diff --git a/yarn.lock b/yarn.lock index 65e1d7a9420..dd43540d846 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1310,10 +1310,10 @@ dom-accessibility-api "^0.5.1" pretty-format "^26.4.2" -"@tiptap/core@^2.0.0-beta.116": - version "2.0.0-beta.116" - resolved "https://registry.yarnpkg.com/@tiptap/core/-/core-2.0.0-beta.116.tgz#990b846cf4c4b4823a7a7ae44dd92bf96ee09403" - integrity sha512-5x3HkT71IxF56lPEHPSTqOqkm1fVfHgVfyBbiNA3Qsz47npgJCDelDr6PHY8qlzxLCaN4dMDQUXEsvDDAiRouw== +"@tiptap/core@^2.0.0-beta.118": + version "2.0.0-beta.118" + resolved "https://registry.yarnpkg.com/@tiptap/core/-/core-2.0.0-beta.118.tgz#17e990da525a81d957494171c0c33d17e2039823" + integrity sha512-TApketXliv2PZSTf5WP8j/svwzeK795fOf4Ff6A0gDwcVYFPHlny4ZlpawYthyqDoe1fEusyZokVgiDHnb+EzA== dependencies: "@types/prosemirror-commands" "^1.0.4" "@types/prosemirror-inputrules" "^1.0.4" @@ -1344,10 +1344,10 @@ resolved "https://registry.yarnpkg.com/@tiptap/extension-bold/-/extension-bold-2.0.0-beta.15.tgz#cf9ddb3fc316be9707753ad4e497bfb8a3ebb0c2" integrity sha512-jKyV6iiwhxwa0+7uuKD74jNDVNLNOS1GmU14MgaA95pY5e1fyaRBPPX8Gtt89niz2CLOY711AV17RPZTe/e60w== -"@tiptap/extension-bubble-menu@^2.0.0-beta.38": - version "2.0.0-beta.38" - resolved "https://registry.yarnpkg.com/@tiptap/extension-bubble-menu/-/extension-bubble-menu-2.0.0-beta.38.tgz#a3f85b44a28667da0fe80a4de3d97833bebefc87" - integrity sha512-XBQemM+0w2VBe72e9AH8fql0ZcrhoDThxbldgq0Cx6Nr49ZmzuYYcFKQvTGTkxEcWSfXIsisSRbkaqUB9QM+ZQ== +"@tiptap/extension-bubble-menu@^2.0.0-beta.39": + version "2.0.0-beta.39" + resolved "https://registry.yarnpkg.com/@tiptap/extension-bubble-menu/-/extension-bubble-menu-2.0.0-beta.39.tgz#8971feeac93e685fc38564173a83ad078b4e7f2a" + integrity sha512-hmA+ePR+MnRaTJ5MxoZ3yqOcK54cW2KQllZx16ZwSyM+yU9bXVhfMmyZwqRD7GGQFkrfnPm5QnedXDBYJD19OQ== dependencies: prosemirror-state "^1.3.4" prosemirror-view "^1.20.1" @@ -1360,10 +1360,10 @@ dependencies: prosemirror-inputrules "^1.1.3" -"@tiptap/extension-code-block-lowlight@2.0.0-beta.40": - version "2.0.0-beta.40" - resolved "https://registry.yarnpkg.com/@tiptap/extension-code-block-lowlight/-/extension-code-block-lowlight-2.0.0-beta.40.tgz#c2285dd472030f013dcbc86a211d4ec5838c2898" - integrity sha512-upMgyy2WxNjLFL2e6LUbc4JLCJIlBn1K8wHDwX7DFP5o9pH77W+0MeAayvF9j3fPaPLt6JgzAxfKlSGPkbX1rg== +"@tiptap/extension-code-block-lowlight@2.0.0-beta.41": + version "2.0.0-beta.41" + resolved "https://registry.yarnpkg.com/@tiptap/extension-code-block-lowlight/-/extension-code-block-lowlight-2.0.0-beta.41.tgz#3ec98f509bbd5df689de6282f2a3881229262ed7" + integrity sha512-2+D/SwcRjWThJ8uFWJT/6B7R+gTUlp4h13/EZqrFMm3YCSOx+bzgTVndog6UJkWyAoNDxkTVwcOyYI4HWdvCiQ== dependencies: "@tiptap/extension-code-block" "^2.0.0-beta.18" "@types/lowlight" "^0.0.3" @@ -1397,27 +1397,27 @@ "@types/prosemirror-dropcursor" "^1.0.3" prosemirror-dropcursor "^1.3.5" -"@tiptap/extension-floating-menu@^2.0.0-beta.32": - version "2.0.0-beta.32" - resolved "https://registry.yarnpkg.com/@tiptap/extension-floating-menu/-/extension-floating-menu-2.0.0-beta.32.tgz#6600fb0fd5b1ddc6d161d03c74e062c9133842cf" - integrity sha512-frroKOpE99fOdfJoWeM1ByWHCda+7Fe1n/Li6rdrmP9NdOS9FluG5lzzYf1qZ8wANfZhrXM8DOjXEXow/L3wkw== +"@tiptap/extension-floating-menu@^2.0.0-beta.33": + version "2.0.0-beta.33" + resolved "https://registry.yarnpkg.com/@tiptap/extension-floating-menu/-/extension-floating-menu-2.0.0-beta.33.tgz#11068488f10fce697df2a48f79039e9c1d10eb7b" + integrity sha512-8s8DPnHIzXg7E7S/DjuS1AAFZKVYXY0KBKaEd1f2V45YOkKwN9El46Ugk/4Ir3yrrllvnisbP9ol+BAQmI0bMg== dependencies: prosemirror-state "^1.3.4" prosemirror-view "^1.20.1" tippy.js "^6.3.1" -"@tiptap/extension-gapcursor@^2.0.0-beta.23": - version "2.0.0-beta.23" - resolved "https://registry.yarnpkg.com/@tiptap/extension-gapcursor/-/extension-gapcursor-2.0.0-beta.23.tgz#d6ea2a97392e0970642b7ea55d778562016ff77e" - integrity sha512-yav1pAZ6YTZ4GzRaY3x0KutghLfC0Z6g/KXFzdE5KD5nxBhLUhid9rZ/kQiVPmZx2R0M/O5C+mdScYS2UzMmqA== +"@tiptap/extension-gapcursor@^2.0.0-beta.24": + version "2.0.0-beta.24" + resolved "https://registry.yarnpkg.com/@tiptap/extension-gapcursor/-/extension-gapcursor-2.0.0-beta.24.tgz#dc42a3610cea611755c6521e14a6995555e0ad49" + integrity sha512-/6Ru0wNLIb3fo30Ar3z/rcakoUA2EIJL9sBFiuyHWTAIujeEaBzA6oG5L4PpP+daKd31JF0I6LjeWMSU9CBSFw== dependencies: "@types/prosemirror-gapcursor" "^1.0.4" prosemirror-gapcursor "^1.2.0" -"@tiptap/extension-hard-break@^2.0.0-beta.20": - version "2.0.0-beta.20" - resolved "https://registry.yarnpkg.com/@tiptap/extension-hard-break/-/extension-hard-break-2.0.0-beta.20.tgz#308c8aaa935dcaf61296d8d0a8a0daed072d884b" - integrity sha512-tLZ53VMse2C1skj23tPFlq0wmdOCQ9vRsukz/KaR8VAFQBany0GOFmazu6QEFpC9+TI2gAckIGijEGFyP9QkMA== +"@tiptap/extension-hard-break@^2.0.0-beta.21": + version "2.0.0-beta.21" + resolved "https://registry.yarnpkg.com/@tiptap/extension-hard-break/-/extension-hard-break-2.0.0-beta.21.tgz#3b9108c7703f23ae186c1038033f0b1354f721bf" + integrity sha512-Ukl+wjfLhE0tW7lWRpSPPo2tajjGnEaSc/Irey1JineFf+x/azA9rREzQy0r2AhORTalH7lj/KDmSdG8IT6syA== "@tiptap/extension-heading@^2.0.0-beta.15": version "2.0.0-beta.15" @@ -1434,10 +1434,10 @@ "@types/prosemirror-history" "^1.0.3" prosemirror-history "^1.2.0" -"@tiptap/extension-horizontal-rule@^2.0.0-beta.20": - version "2.0.0-beta.20" - resolved "https://registry.yarnpkg.com/@tiptap/extension-horizontal-rule/-/extension-horizontal-rule-2.0.0-beta.20.tgz#84260374812bf9fdac2f54869f10ffae772dc2b8" - integrity sha512-phvZy1ckl4FJ8k6cXce8wOXxv0c50HjU5sA7r6b8u7+Mj0Dc5DZE7enjiDGxoBLP6Yf2lzOgq1phH/r9EihDUQ== +"@tiptap/extension-horizontal-rule@^2.0.0-beta.21": + version "2.0.0-beta.21" + resolved "https://registry.yarnpkg.com/@tiptap/extension-horizontal-rule/-/extension-horizontal-rule-2.0.0-beta.21.tgz#1c73a8547611f53935117ed0079542d958ba37fa" + integrity sha512-fgvRGuNEGWAitbcoz6VZSR9gcVIHksTy2QpXPnQC+N9Mi7havaxreYdMZn+oePW/5kdZoZNRx+jsf5DjKomvoQ== dependencies: prosemirror-state "^1.3.4" @@ -1530,13 +1530,13 @@ resolved "https://registry.yarnpkg.com/@tiptap/extension-text/-/extension-text-2.0.0-beta.13.tgz#da0af8d9a3f149d20076e15d88c6af21fb6d940f" integrity sha512-0EtAwuRldCAoFaL/iXgkRepEeOd55rPg5N4FQUN1xTwZT7PDofukP0DG/2jff/Uj17x4uTaJAa9qlFWuNnDvjw== -"@tiptap/vue-2@^2.0.0-beta.56": - version "2.0.0-beta.56" - resolved "https://registry.yarnpkg.com/@tiptap/vue-2/-/vue-2-2.0.0-beta.56.tgz#c339bb94d0ec6ea4b4a330bb0f9800468620780c" - integrity sha512-qU2rD6LHU5Xg30XVP8gdPPf/u3aEFhkQNOTk5m7rnRvqj8KXkZCTHLzBDgJfnoLfy7hVn62Iiq0KP7wKDda0Tw== +"@tiptap/vue-2@^2.0.0-beta.57": + version "2.0.0-beta.57" + resolved "https://registry.yarnpkg.com/@tiptap/vue-2/-/vue-2-2.0.0-beta.57.tgz#242f7aa47d3c99fdef0e66a05a193b6fef1a95a6" + integrity sha512-f8COWq84wOJeLwAmaYsHCqKVbGgMloW+1r4Rz/KhlFb1MNXYeDHibCiW/VtJe7bdae+iRyIwnfmnAp2u5s77hQ== dependencies: - "@tiptap/extension-bubble-menu" "^2.0.0-beta.38" - "@tiptap/extension-floating-menu" "^2.0.0-beta.32" + "@tiptap/extension-bubble-menu" "^2.0.0-beta.39" + "@tiptap/extension-floating-menu" "^2.0.0-beta.33" prosemirror-view "^1.20.1" "@toast-ui/editor@^2.5.2": |