diff --git a/.eslintrc.yml b/.eslintrc.yml index 1dfba84d4cb..956758255cb 100644 --- a/.eslintrc.yml +++ b/.eslintrc.yml @@ -117,6 +117,14 @@ rules: message: 'Migrate to GlSkeletonLoader, or import GlDeprecatedSkeletonLoading.' - selector: ImportSpecifier[imported.name='GlSafeHtmlDirective'] message: 'Use directive at ~/vue_shared/directives/safe_html.js instead.' + - selector: Literal[value=/docs.gitlab.+\u002Fee/] + message: 'No hard coded url, use `DOCS_URL_IN_EE_DIR` in `jh_else_ce/lib/utils/url_utility`' + - selector: Literal[value=/(?=.*docs.gitlab.*)(?=^(?!.*\u002Fee\b).*$)/] + message: 'No hard coded url, use `DOCS_URL` in `jh_else_ce/lib/utils/url_utility`' + - selector: Literal[value=/(?=.*about.gitlab.*)(?=^(?!.*\u002Fblog\b).*$)/] + message: 'No hard coded url, use `PROMO_URL` in `jh_else_ce/lib/utils/url_utility`' + - selector: TemplateLiteral[expressions.0.name=DOCS_URL] > TemplateElement[value.cooked=/\u002Fjh/] + message: '`/ee` or `/jh` path found in docs url, use `DOCS_URL_IN_EE_DIR` in `jh_else_ce/lib/utils/url_utility`' no-restricted-imports: - error - paths: diff --git a/.gitlab/ci/templates/gem.gitlab-ci.yml b/.gitlab/ci/templates/gem.gitlab-ci.yml index 46c5e1342c6..f1ee3426070 100644 --- a/.gitlab/ci/templates/gem.gitlab-ci.yml +++ b/.gitlab/ci/templates/gem.gitlab-ci.yml @@ -11,6 +11,7 @@ spec: --- .gems:rules:$[[inputs.gem_name]]: rules: + - if: '$CI_PIPELINE_SOURCE == "schedule" && $SCHEDULE_TYPE == "maintenance"' - if: '$CI_MERGE_REQUEST_EVENT_TYPE == "merged_result" || $CI_MERGE_REQUEST_EVENT_TYPE == "detached"' changes: - "$[[inputs.gem_path_prefix]]$[[inputs.gem_name]]/**/*" diff --git a/app/assets/javascripts/design_management/components/design_notes/design_discussion.vue b/app/assets/javascripts/design_management/components/design_notes/design_discussion.vue index a890d2ba933..45f33967476 100644 --- a/app/assets/javascripts/design_management/components/design_notes/design_discussion.vue +++ b/app/assets/javascripts/design_management/components/design_notes/design_discussion.vue @@ -302,6 +302,7 @@ export default { :is-resolving="isResolving" :is-discussion="true" :noteable-id="noteableId" + :design-variables="designVariables" @delete-note="showDeleteNoteConfirmationModal($event)" > + +import AwardsList from '~/vue_shared/components/awards_list.vue'; + +export default { + components: { + AwardsList, + }, + props: { + awards: { + type: Array, + required: true, + }, + canAwardEmoji: { + type: Boolean, + required: true, + }, + }, + computed: { + currentUserId() { + return window.gon.current_user_id; + }, + }, +}; + + + diff --git a/app/assets/javascripts/design_management/graphql/fragments/design_note.fragment.graphql b/app/assets/javascripts/design_management/graphql/fragments/design_note.fragment.graphql index 28224671326..9af4733d5dc 100644 --- a/app/assets/javascripts/design_management/graphql/fragments/design_note.fragment.graphql +++ b/app/assets/javascripts/design_management/graphql/fragments/design_note.fragment.graphql @@ -11,6 +11,15 @@ fragment DesignNote on Note { bodyHtml createdAt resolved + awardEmoji { + nodes { + name + user { + id + name + } + } + } position { diffRefs { ...DesignDiffRefs diff --git a/app/assets/javascripts/design_management/graphql/fragments/note_permissions.fragment.graphql b/app/assets/javascripts/design_management/graphql/fragments/note_permissions.fragment.graphql index e599ab19c2d..acc52e5de59 100644 --- a/app/assets/javascripts/design_management/graphql/fragments/note_permissions.fragment.graphql +++ b/app/assets/javascripts/design_management/graphql/fragments/note_permissions.fragment.graphql @@ -1,4 +1,5 @@ fragment DesignNotePermissions on NotePermissions { adminNote repositionNote + awardEmoji } diff --git a/app/assets/javascripts/design_management/graphql/mutations/design_note_award_emoji_toggle.mutation.graphql b/app/assets/javascripts/design_management/graphql/mutations/design_note_award_emoji_toggle.mutation.graphql new file mode 100644 index 00000000000..3e274d0b65f --- /dev/null +++ b/app/assets/javascripts/design_management/graphql/mutations/design_note_award_emoji_toggle.mutation.graphql @@ -0,0 +1,6 @@ +mutation designNoteNoteToggleAwardEmoji($awardableId: AwardableID!, $name: String!) { + awardEmojiToggle(input: { awardableId: $awardableId, name: $name }) { + errors + toggledOn + } +} diff --git a/app/assets/javascripts/lib/logger/hello.js b/app/assets/javascripts/lib/logger/hello.js index ccfdfe91e60..4ad99ec09d8 100644 --- a/app/assets/javascripts/lib/logger/hello.js +++ b/app/assets/javascripts/lib/logger/hello.js @@ -1,4 +1,5 @@ import { s__, sprintf } from '~/locale'; +import { PROMO_URL } from 'jh_else_ce/lib/utils/url_utility'; const HANDSHAKE = String.fromCodePoint(0x1f91d); const MAG = String.fromCodePoint(0x1f50e); @@ -15,7 +16,7 @@ ${s__( ${sprintf(s__('HelloMessage|%{handshake_emoji} Contribute to GitLab: %{contribute_link}'), { handshake_emoji: `${HANDSHAKE}`, - contribute_link: 'https://about.gitlab.com/community/contribute/', + contribute_link: `${PROMO_URL}/community/contribute/`, })} ${sprintf(s__('HelloMessage|%{magnifier_emoji} Create a new GitLab issue: %{new_issue_link}'), { magnifier_emoji: `${MAG}`, @@ -27,7 +28,7 @@ ${ s__( 'HelloMessage|%{rocket_emoji} We like your curiosity! Help us improve GitLab by joining the team: %{jobs_page_link}', ), - { rocket_emoji: `${ROCKET}`, jobs_page_link: 'https://about.gitlab.com/jobs/' }, + { rocket_emoji: `${ROCKET}`, jobs_page_link: `${PROMO_URL}/jobs/` }, )}` : '' }`, diff --git a/app/assets/javascripts/projects/compare/constants.js b/app/assets/javascripts/projects/compare/constants.js index f689d543455..2f07cf57521 100644 --- a/app/assets/javascripts/projects/compare/constants.js +++ b/app/assets/javascripts/projects/compare/constants.js @@ -1,4 +1,5 @@ import { __, s__ } from '~/locale'; +import { DOCS_URL_IN_EE_DIR } from 'jh_else_ce/lib/utils/url_utility'; export const COMPARE_OPTIONS_INPUT_NAME = 'straight'; export const COMPARE_OPTIONS = [ @@ -21,5 +22,4 @@ export const I18N = { openMr: s__('CompareRevisions|Create merge request'), }; -export const COMPARE_REVISIONS_DOCS_URL = - 'https://docs.gitlab.com/ee/user/project/repository/branches/#compare-branches'; +export const COMPARE_REVISIONS_DOCS_URL = `${DOCS_URL_IN_EE_DIR}/user/project/repository/branches/#compare-branches`; diff --git a/app/assets/javascripts/vue_shared/components/awards_list.vue b/app/assets/javascripts/vue_shared/components/awards_list.vue index 8f1f7ba0ad8..fd74ded7dc8 100644 --- a/app/assets/javascripts/vue_shared/components/awards_list.vue +++ b/app/assets/javascripts/vue_shared/components/awards_list.vue @@ -184,6 +184,7 @@ export default { class="gl-mr-3 gl-my-2" :class="awardList.classes" :title="awardList.title" + :data-emoji-name="awardList.name" data-testid="award-button" @click="handleAward(awardList.name)" > diff --git a/app/assets/javascripts/vue_shared/components/entity_select/entity_select.vue b/app/assets/javascripts/vue_shared/components/entity_select/entity_select.vue index 1a3220d8db9..970c24c6e87 100644 --- a/app/assets/javascripts/vue_shared/components/entity_select/entity_select.vue +++ b/app/assets/javascripts/vue_shared/components/entity_select/entity_select.vue @@ -75,10 +75,13 @@ export default { computed: { selected: { set(value) { - this.$emit('input', value); this.selectedValue = value; this.selectedText = value === null ? null : this.items.find((item) => item.value === value).text; + this.$emit('input', { + value: this.selectedValue, + text: this.selectedText, + }); }, get() { return this.selectedValue; @@ -161,7 +164,7 @@ export default { }, onReset() { this.selected = null; - this.$emit('input', null); + this.$emit('input', {}); }, onBottomReached() { this.fetchEntities(this.page + 1); diff --git a/config/environments/test.rb b/config/environments/test.rb index b919df45214..e29fcf31664 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -4,6 +4,7 @@ require 'gitlab/testing/request_blocker_middleware' require 'gitlab/testing/robots_blocker_middleware' require 'gitlab/testing/request_inspector_middleware' require 'gitlab/testing/clear_process_memory_cache_middleware' +require 'gitlab/testing/action_cable_blocker' require 'gitlab/utils/all' Rails.application.configure do @@ -13,6 +14,8 @@ Rails.application.configure do config.middleware.insert_before(ActionDispatch::Static, Gitlab::Testing::RequestInspectorMiddleware) config.middleware.insert_before(ActionDispatch::Static, Gitlab::Testing::ClearProcessMemoryCacheMiddleware) + Gitlab::Testing::ActionCableBlocker.install + # Settings specified here will take precedence over those in config/application.rb # The test environment is used exclusively to run your application's diff --git a/doc/architecture/blueprints/gitlab_observability_backend/metrics/index.md b/doc/architecture/blueprints/gitlab_observability_backend/index.md similarity index 100% rename from doc/architecture/blueprints/gitlab_observability_backend/metrics/index.md rename to doc/architecture/blueprints/gitlab_observability_backend/index.md diff --git a/doc/architecture/blueprints/gitlab_observability_backend/metrics/supported-deployments.png b/doc/architecture/blueprints/gitlab_observability_backend/supported-deployments.png similarity index 100% rename from doc/architecture/blueprints/gitlab_observability_backend/metrics/supported-deployments.png rename to doc/architecture/blueprints/gitlab_observability_backend/supported-deployments.png diff --git a/doc/security/img/ssh_keys_restrictions_settings.png b/doc/security/img/ssh_keys_restrictions_settings.png index 94258af3bf9..1f37a25d19f 100644 Binary files a/doc/security/img/ssh_keys_restrictions_settings.png and b/doc/security/img/ssh_keys_restrictions_settings.png differ diff --git a/doc/update/index.md b/doc/update/index.md index 5f6a7c052e6..fb303d06eb5 100644 --- a/doc/update/index.md +++ b/doc/update/index.md @@ -296,7 +296,7 @@ and [Helm Chart deployments](https://docs.gitlab.com/charts/). They come with ap - Geo: Some project imports do not initialize wiki repositories on project creation. Since the migration of project wikis to SSF, [missing wiki repositories are being incorrectly flagged as failing verification](https://gitlab.com/gitlab-org/gitlab/-/issues/409704). This is not a result of an actual replication/verification failure but an invalid internal state for these missing repositories inside Geo and results in errors in the logs and the verification progress reporting a failed state for these wiki repositories. If you have not imported projects you are not impacted by this issue. - Impacted versions: GitLab versions 15.11.x, 16.0.x, and 16.1.0 - 16.1.2. - Versions containing fix: GitLab 16.1.3 and later. -- Geo: Some project imports do not initialize design repositories on project creation. Since the migration of project designs to SSF, [missing design repositories are being incorrectly flagged as failing verification](https://gitlab.com/gitlab-org/gitlab/-/issues/414279). This is not a result of an actual replication/verification failure but an invalid internal state for these missing repositories inside Geo and results in errors in the logs and the verification progress reporting a failed state for these design repositories. If you have not imported projects you are not impacted by this issue. +- Geo: Since the migration of project designs to SSF, [missing design repositories are being incorrectly flagged as failing verification](https://gitlab.com/gitlab-org/gitlab/-/issues/414279). This is not a result of an actual replication/verification failure but an invalid internal state for these missing repositories inside Geo and results in errors in the logs and the verification progress reporting a failed state for these design repositories. You could be impacted by this issue even if you have not imported projects. - Impacted versions: GitLab versions 16.1.x. - Versions containing fix: GitLab 16.2.0 and later. diff --git a/doc/user/award_emojis.md b/doc/user/award_emojis.md index 4e393c54d8e..c83148eac20 100644 --- a/doc/user/award_emojis.md +++ b/doc/user/award_emojis.md @@ -8,7 +8,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w > - [Renamed](https://gitlab.com/gitlab-org/gitlab/-/issues/409884) from "award emoji" to "emoji reactions" in GitLab 16.0. > - Reacting with emoji on work items (such as tasks, objectives, and key results) [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/393599) in GitLab 16.0. - +> - Reacting with emoji on design discussion comments [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/29756) in GitLab 16.2. When you're collaborating online, you get fewer opportunities for high-fives and thumbs-ups. React with emoji on: diff --git a/lib/gitlab/testing/action_cable_blocker.rb b/lib/gitlab/testing/action_cable_blocker.rb new file mode 100644 index 00000000000..aebb0732035 --- /dev/null +++ b/lib/gitlab/testing/action_cable_blocker.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +# rubocop:disable Style/ClassVars + +# This is inspired by http://www.salsify.com/blog/engineering/tearing-capybara-ajax-tests +# Rack middleware that keeps track of the number of active requests and can block new requests. +module Gitlab + module Testing + class ActionCableBlocker + @@num_active_requests = Concurrent::AtomicFixnum.new(0) + @@block_requests = Concurrent::AtomicBoolean.new(false) + + # Returns the number of requests the server is currently processing. + def self.num_active_requests + @@num_active_requests.value + end + + # Prevents the server from accepting new requests. Any new requests will be skipped. + def self.block_requests! + @@block_requests.value = true + end + + # Allows the server to accept requests again. + def self.allow_requests! + @@block_requests.value = false + end + + def self.install + ::ActionCable::Server::Worker.set_callback :work, :around do |_, inner| + @@num_active_requests.increment + + inner.call if @@block_requests.false? + ensure + @@num_active_requests.decrement + end + end + end + end +end +# rubocop:enable Style/ClassVars diff --git a/lib/tasks/gitlab/metrics_exporter.rake b/lib/tasks/gitlab/metrics_exporter.rake deleted file mode 100644 index 70719648fc5..00000000000 --- a/lib/tasks/gitlab/metrics_exporter.rake +++ /dev/null @@ -1,27 +0,0 @@ -# frozen_string_literal: true - -namespace :gitlab do - require_relative Rails.root.join('metrics_server', 'dependencies') - require_relative Rails.root.join('metrics_server', 'metrics_server') - - namespace :metrics_exporter do - REPO = 'https://gitlab.com/gitlab-org/gitlab-metrics-exporter.git' - - desc "GitLab | Metrics Exporter | Install or upgrade gitlab-metrics-exporter" - task :install, [:dir] => :gitlab_environment do |t, args| - unless args.dir.present? - abort %(Please specify the directory where you want to install the exporter -Usage: rake "gitlab:metrics_exporter:install[/installation/dir]") - end - - version = ENV['GITLAB_METRICS_EXPORTER_VERSION'] || MetricsServer.version - make = Gitlab::Utils.which('gmake') || Gitlab::Utils.which('make') - - abort "Couldn't find a 'make' binary" unless make - - checkout_or_clone_version(version: version, repo: REPO, target_dir: args.dir, clone_opts: %w[--depth 1]) - - Dir.chdir(args.dir) { run_command!([make]) } - end - end -end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 6483fbf7a21..4cdc81b693a 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -5245,7 +5245,7 @@ msgstr "" msgid "Analytics|Configure Dashboard Project" msgstr "" -msgid "Analytics|Create dashboard %{dashboardId}" +msgid "Analytics|Create dashboard %{dashboardSlug}" msgstr "" msgid "Analytics|Custom dashboards" @@ -5353,7 +5353,7 @@ msgstr "" msgid "Analytics|URL" msgstr "" -msgid "Analytics|Updating dashboard %{dashboardId}" +msgid "Analytics|Updating dashboard %{dashboardSlug}" msgstr "" msgid "Analytics|Updating visualization %{visualizationName}" diff --git a/metrics_server/metrics_server.rb b/metrics_server/metrics_server.rb index 530f3a000a9..7d4968f930c 100644 --- a/metrics_server/metrics_server.rb +++ b/metrics_server/metrics_server.rb @@ -7,10 +7,6 @@ class MetricsServer # rubocop:disable Gitlab/NamespacedClass PumaProcessSupervisor = Class.new(Gitlab::ProcessSupervisor) class << self - def version - Rails.root.join('GITLAB_METRICS_EXPORTER_VERSION').read.chomp - end - def start_for_puma metrics_dir = ::Prometheus::Client.configuration.multiprocess_files_dir @@ -28,45 +24,10 @@ class MetricsServer # rubocop:disable Gitlab/NamespacedClass end def start_for_sidekiq(**options) - if new_metrics_server? - self.spawn('sidekiq', **options) - else - self.fork('sidekiq', **options) - end + self.fork('sidekiq', **options) end - def spawn(target, metrics_dir:, **options) - return spawn_ruby_server(target, metrics_dir: metrics_dir, **options) unless new_metrics_server? - - settings = settings_value(target) - path = options[:path]&.then { |p| Pathname.new(p) } || Pathname.new('') - cmd = path.join('gitlab-metrics-exporter').to_path - env = { - 'GOGC' => '10', # Set Go GC heap goal to 10% to curb memory growth. - 'GME_MMAP_METRICS_DIR' => metrics_dir.to_s, - 'GME_PROBES' => 'self,mmap,mmap_stats', - 'GME_SERVER_HOST' => settings['address'], - 'GME_SERVER_PORT' => settings['port'].to_s - } - - if settings['log_enabled'] - env['GME_LOG_FILE'] = File.join(Rails.root, 'log', "#{name(target)}.log") - env['GME_LOG_LEVEL'] = 'info' - else - env['GME_LOG_LEVEL'] = 'quiet' - end - - if settings['tls_enabled'] - env['GME_CERT_FILE'] = settings['tls_cert_path'] - env['GME_CERT_KEY'] = settings['tls_key_path'] - end - - Process.spawn(env, cmd, err: $stderr, out: $stdout, pgroup: true).tap do |pid| - Process.detach(pid) - end - end - - def spawn_ruby_server(target, metrics_dir:, wipe_metrics_dir: false, **options) + def spawn(target, metrics_dir:, wipe_metrics_dir: false) ensure_valid_target!(target) cmd = "#{Rails.root}/bin/metrics-server" @@ -126,10 +87,6 @@ class MetricsServer # rubocop:disable Gitlab/NamespacedClass end end - def new_metrics_server? - Gitlab::Utils.to_boolean(ENV['GITLAB_GOLANG_METRICS_SERVER']) - end - def ensure_valid_target!(target) raise "Target must be one of [puma,sidekiq]" unless %w(puma sidekiq).include?(target) end diff --git a/package.json b/package.json index 7ace0b7fadf..e114b43ead6 100644 --- a/package.json +++ b/package.json @@ -57,7 +57,7 @@ "@gitlab/favicon-overlay": "2.0.0", "@gitlab/fonts": "^1.3.0", "@gitlab/svgs": "3.54.0", - "@gitlab/ui": "64.18.3", + "@gitlab/ui": "64.19.0", "@gitlab/visual-review-tools": "1.7.3", "@gitlab/web-ide": "0.0.1-dev-20230620122149", "@mattiasbuelens/web-streams-adapter": "^0.1.0", diff --git a/qa/gdk/Dockerfile.gdk b/qa/gdk/Dockerfile.gdk index 2b296535622..51e058a7088 100644 --- a/qa/gdk/Dockerfile.gdk +++ b/qa/gdk/Dockerfile.gdk @@ -88,13 +88,6 @@ RUN set -eux; \ rm -rf ${GEM_HOME}/cache \ && go clean -cache -# Build metrics-exporter -# -COPY --chown=gdk:gdk GITLAB_METRICS_EXPORTER_VERSION ./gitlab/ -RUN set -eux; \ - make gitlab-metrics-exporter-setup; \ - go clean -cache - # Install gitlab gem dependencies # COPY --chown=gdk:gdk Gemfile Gemfile.lock ./gitlab/ diff --git a/spec/commands/metrics_server/metrics_server_spec.rb b/spec/commands/metrics_server/metrics_server_spec.rb index 88a28b02903..ee07602016f 100644 --- a/spec/commands/metrics_server/metrics_server_spec.rb +++ b/spec/commands/metrics_server/metrics_server_spec.rb @@ -30,18 +30,6 @@ RSpec.describe 'GitLab metrics server', :aggregate_failures do } end - before(:all) do - Rake.application.rake_require 'tasks/gitlab/metrics_exporter' - - @exporter_path = Rails.root.join('tmp', 'test', 'gme') - - run_rake_task('gitlab:metrics_exporter:install', @exporter_path) - end - - after(:all) do - FileUtils.rm_rf(@exporter_path) - end - shared_examples 'serves metrics endpoint' do it 'serves /metrics endpoint' do start_server! @@ -59,24 +47,18 @@ RSpec.describe 'GitLab metrics server', :aggregate_failures do end end - shared_examples 'spawns a server' do |target, use_golang_server| - context "targeting #{target} when using Golang server is #{use_golang_server}" do + shared_examples 'spawns a server' do |target| + context "targeting #{target}" do let(:metrics_dir) { Dir.mktmpdir } subject(:start_server!) do - @pid = MetricsServer.spawn(target, metrics_dir: metrics_dir, path: @exporter_path.join('bin')) + @pid = MetricsServer.spawn(target, metrics_dir: metrics_dir) end before do - if use_golang_server - stub_env('GITLAB_GOLANG_METRICS_SERVER', '1') - allow(Settings).to receive(:monitoring).and_return( - GitlabSettings::Options.build(config.dig('test', 'monitoring'))) - else - config_file.write(YAML.dump(config)) - config_file.close - stub_env('GITLAB_CONFIG', config_file.path) - end + config_file.write(YAML.dump(config)) + config_file.close + stub_env('GITLAB_CONFIG', config_file.path) # We need to send a request to localhost WebMock.allow_net_connect! end @@ -111,8 +93,6 @@ RSpec.describe 'GitLab metrics server', :aggregate_failures do end end - it_behaves_like 'spawns a server', 'puma', true - it_behaves_like 'spawns a server', 'puma', false - it_behaves_like 'spawns a server', 'sidekiq', true - it_behaves_like 'spawns a server', 'sidekiq', false + it_behaves_like 'spawns a server', 'puma' + it_behaves_like 'spawns a server', 'sidekiq' end diff --git a/spec/features/projects/issues/design_management/user_views_design_spec.rb b/spec/features/projects/issues/design_management/user_views_design_spec.rb index 268c209cba1..bd9d1092e17 100644 --- a/spec/features/projects/issues/design_management/user_views_design_spec.rb +++ b/spec/features/projects/issues/design_management/user_views_design_spec.rb @@ -5,16 +5,67 @@ require 'spec_helper' RSpec.describe 'User views issue designs', :js, feature_category: :design_management do include DesignManagementTestHelpers + let_it_be(:user) { create(:user) } + let_it_be(:guest_user) { create(:user) } let_it_be(:project) { create(:project_empty_repo, :public) } let_it_be(:issue) { create(:issue, project: project) } let_it_be(:design) { create(:design, :with_file, issue: issue) } + let_it_be(:note) { create(:diff_note_on_design, noteable: design, author: user) } + + def add_diff_note_emoji(diff_note, emoji_name) + page.within(first(".image-notes li#note_#{diff_note.id}.design-note")) do + page.find('[data-testid="note-emoji-button"] .note-emoji-button').click + + page.within('ul.dropdown-menu') do + page.find('input[type="search"]').set(emoji_name) + page.find('button[data-testid="emoji-button"]:first-child').click + end + end + end + + def remove_diff_note_emoji(diff_note, emoji_name) + page.within(first(".image-notes li#note_#{diff_note.id}.design-note")) do + page.find(".awards button[data-emoji-name='#{emoji_name}']").click + end + end + + before_all do + project.add_maintainer(user) + project.add_guest(guest_user) + end before do enable_design_management + sign_in(user) + visit project_issue_path(project, issue) end + shared_examples 'design discussion emoji awards' do + it 'allows user to add emoji reaction to a comment' do + click_link design.filename + + add_diff_note_emoji(note, 'thumbsup') + + expect(page.find("li#note_#{note.id} .awards")).to have_selector('button[title="You reacted with :thumbsup:"]') + end + + it 'allows user to remove emoji reaction from a comment' do + click_link design.filename + + add_diff_note_emoji(note, 'thumbsup') + + # Wait for emoji to be added + wait_for_requests + + remove_diff_note_emoji(note, 'thumbsup') + + # Only award emoji that was present has been removed + expect(page.find("li#note_#{note.id}")).not_to have_selector('.awards') + end + end + it 'opens design detail' do click_link design.filename @@ -25,6 +76,26 @@ RSpec.describe 'User views issue designs', :js, feature_category: :design_manage expect(page).to have_selector('.js-design-image') end + it 'shows a comment within design' do + click_link design.filename + + expect(page.find('.image-notes .design-note .note-text')).to have_content(note.note) + end + + it_behaves_like 'design discussion emoji awards' + + context 'when user is guest' do + before do + enable_design_management + + sign_in(guest_user) + + visit project_issue_path(project, issue) + end + + it_behaves_like 'design discussion emoji awards' + end + context 'when svg file is loaded in design detail' do let_it_be(:file) { Rails.root.join('spec/fixtures/svg_without_attr.svg') } let_it_be(:design) { create(:design, :with_file, filename: 'svg_without_attr.svg', file: file, issue: issue) } diff --git a/spec/features/projects/work_items/work_item_spec.rb b/spec/features/projects/work_items/work_item_spec.rb index 1aa9ae3ac3f..e996a76b1c5 100644 --- a/spec/features/projects/work_items/work_item_spec.rb +++ b/spec/features/projects/work_items/work_item_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Work item', :js, feature_category: :team_planning, quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/416663' do +RSpec.describe 'Work item', :js, feature_category: :team_planning do let_it_be_with_reload(:user) { create(:user) } let_it_be_with_reload(:user2) { create(:user, name: 'John') } diff --git a/spec/frontend/design_management/components/design_notes/__snapshots__/design_note_spec.js.snap b/spec/frontend/design_management/components/design_notes/__snapshots__/design_note_spec.js.snap index 14efe253bd0..9c131b0b650 100644 --- a/spec/frontend/design_management/components/design_notes/__snapshots__/design_note_spec.js.snap +++ b/spec/frontend/design_management/components/design_notes/__snapshots__/design_note_spec.js.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Design note component should match the snapshot 1`] = ` +exports[`Design note component default should match the snapshot 1`] = `
+
+
+ + + +
+
+ @@ -83,6 +147,161 @@ exports[`Design note component should match the snapshot 1`] = ` data-qa-selector="note_content" data-testid="note-text" /> + +
+ + + +
+
+
+ + + +
+
+
+
+ `; diff --git a/spec/frontend/design_management/components/design_notes/design_discussion_spec.js b/spec/frontend/design_management/components/design_notes/design_discussion_spec.js index 2bb67494f3b..797f399eff5 100644 --- a/spec/frontend/design_management/components/design_notes/design_discussion_spec.js +++ b/spec/frontend/design_management/components/design_notes/design_discussion_spec.js @@ -89,6 +89,9 @@ describe('Design discussions component', () => { }, }, }, + stubs: { + EmojiPicker: true, + }, }); } diff --git a/spec/frontend/design_management/components/design_notes/design_note_spec.js b/spec/frontend/design_management/components/design_notes/design_note_spec.js index 51007483caf..55188b6a3ec 100644 --- a/spec/frontend/design_management/components/design_notes/design_note_spec.js +++ b/spec/frontend/design_management/components/design_notes/design_note_spec.js @@ -1,10 +1,18 @@ import { ApolloMutation } from 'vue-apollo'; import { nextTick } from 'vue'; import { GlAvatar, GlAvatarLink, GlDisclosureDropdown, GlDisclosureDropdownItem } from '@gitlab/ui'; -import { mountExtended } from 'helpers/vue_test_utils_helper'; +import * as Sentry from '@sentry/browser'; + +import { mountExtended, shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import waitForPromises from 'helpers/wait_for_promises'; + +import EmojiPicker from '~/emoji/components/picker.vue'; +import DesignNoteAwardsList from '~/design_management/components/design_notes/design_note_awards_list.vue'; import DesignNote from '~/design_management/components/design_notes/design_note.vue'; import DesignReplyForm from '~/design_management/components/design_notes/design_reply_form.vue'; import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; +import designNoteAwardEmojiToggleMutation from '~/design_management/graphql/mutations/design_note_award_emoji_toggle.mutation.graphql'; +import { mockAwardEmoji } from '../../mock_data/apollo_mock'; const scrollIntoViewMock = jest.fn(); const note = { @@ -15,9 +23,11 @@ const note = { avatarUrl: 'https://gitlab.com/avatar', webUrl: 'https://gitlab.com/user', }, + awardEmoji: mockAwardEmoji, body: 'test', userPermissions: { adminNote: false, + awardEmoji: true, }, createdAt: '2019-07-26T15:02:20Z', }; @@ -27,14 +37,14 @@ const $route = { hash: '#note_123', }; -const mutate = jest.fn().mockResolvedValue({ data: { updateNote: {} } }); - describe('Design note component', () => { let wrapper; + let mutate; const findUserAvatar = () => wrapper.findComponent(GlAvatar); const findUserAvatarLink = () => wrapper.findComponent(GlAvatarLink); const findUserLink = () => wrapper.findByTestId('user-link'); + const findDesignNoteAwardsList = () => wrapper.findComponent(DesignNoteAwardsList); const findReplyForm = () => wrapper.findComponent(DesignReplyForm); const findEditButton = () => wrapper.findByTestId('note-edit'); const findNoteContent = () => wrapper.findByTestId('note-text'); @@ -43,101 +53,110 @@ describe('Design note component', () => { const findEditDropdownItem = () => findDropdownItems().at(0); const findDeleteDropdownItem = () => findDropdownItems().at(1); - function createComponent(props = {}, data = { isEditing: false }) { - wrapper = mountExtended(DesignNote, { + function createComponent({ + props = {}, + data = { isEditing: false }, + mountFn = mountExtended, + mocks = { + $route, + $apollo: { + mutate: jest.fn().mockResolvedValue({ data: { updateNote: {} } }), + }, + }, + stubs = { + ApolloMutation, + GlDisclosureDropdown, + GlDisclosureDropdownItem, + TimelineEntryItem: true, + TimeAgoTooltip: true, + GlAvatarLink: true, + GlAvatar: true, + GlLink: true, + }, + } = {}) { + wrapper = mountFn(DesignNote, { propsData: { note: {}, noteableId: 'gid://gitlab/DesignManagement::Design/6', + designVariables: { + atVersion: null, + filenames: ['foo.jpg'], + fullPath: 'gitlab-org/gitlab-test', + iid: '1', + }, ...props, }, + provide: { + issueIid: '1', + projectPath: 'gitlab-org/gitlab-test', + }, data() { return { ...data, }; }, - mocks: { - $route, - $apollo: { - mutate, - }, - }, - stubs: { - ApolloMutation, - GlDisclosureDropdown, - GlDisclosureDropdownItem, - TimelineEntryItem: true, - TimeAgoTooltip: true, - GlAvatarLink: true, - GlAvatar: true, - GlLink: true, - }, + mocks, + stubs, }); } - it('should match the snapshot', () => { - createComponent({ - note, - }); - - expect(wrapper.element).toMatchSnapshot(); + beforeEach(() => { + window.gon = { current_user_id: 1 }; }); - it('should render avatar with correct props', () => { - createComponent({ - note, + describe('default', () => { + beforeEach(() => { + createComponent({ props: { note } }); }); - expect(findUserAvatar().props()).toMatchObject({ - src: note.author.avatarUrl, - entityName: note.author.username, + it('should match the snapshot', () => { + expect(wrapper.element).toMatchSnapshot(); }); - expect(findUserAvatarLink().attributes()).toMatchObject({ - href: note.author.webUrl, - 'data-user-id': '1', - 'data-username': `${note.author.username}`, - }); - }); + it('should render avatar with correct props', () => { + expect(findUserAvatar().props()).toMatchObject({ + src: note.author.avatarUrl, + entityName: note.author.username, + }); - it('should render author details', () => { - createComponent({ - note, + expect(findUserAvatarLink().attributes()).toMatchObject({ + href: note.author.webUrl, + 'data-user-id': '1', + 'data-username': `${note.author.username}`, + }); }); - expect(findUserLink().exists()).toBe(true); - }); - - it('should render a time ago tooltip if note has createdAt property', () => { - createComponent({ - note, + it('should render author details', () => { + expect(findUserLink().exists()).toBe(true); }); - expect(wrapper.findComponent(TimeAgoTooltip).exists()).toBe(true); - }); - - it('should not render edit icon when user does not have a permission', () => { - createComponent({ - note, + it('should render a time ago tooltip if note has createdAt property', () => { + expect(wrapper.findComponent(TimeAgoTooltip).exists()).toBe(true); }); - expect(findEditButton().exists()).toBe(false); - }); - - it('should not display a dropdown if user does not have a permission to delete note', () => { - createComponent({ - note, + it('should render emoji awards list', () => { + expect(findDesignNoteAwardsList().exists()).toBe(true); }); - expect(findDropdown().exists()).toBe(false); + it('should not render edit icon when user does not have a permission', () => { + expect(findEditButton().exists()).toBe(false); + }); + + it('should not display a dropdown if user does not have a permission to delete note', () => { + expect(findDropdown().exists()).toBe(false); + }); }); describe('when user has a permission to edit note', () => { it('should open an edit form on edit button click', async () => { createComponent({ - note: { - ...note, - userPermissions: { - adminNote: true, + props: { + note: { + ...note, + userPermissions: { + adminNote: true, + awardEmoji: true, + }, }, }, }); @@ -151,25 +170,29 @@ describe('Design note component', () => { describe('when edit form is rendered', () => { beforeEach(() => { - createComponent( - { + createComponent({ + props: { note: { ...note, userPermissions: { adminNote: true, + awardEmoji: true, }, }, }, - { isEditing: true }, - ); + data: { isEditing: true }, + }); }); it('should open an edit form on edit button click', async () => { createComponent({ - note: { - ...note, - userPermissions: { - adminNote: true, + props: { + note: { + ...note, + userPermissions: { + adminNote: true, + awardEmoji: true, + }, }, }, }); @@ -207,10 +230,13 @@ describe('Design note component', () => { describe('when user has admin permissions', () => { it('should display a dropdown', () => { createComponent({ - note: { - ...note, - userPermissions: { - adminNote: true, + props: { + note: { + ...note, + userPermissions: { + adminNote: true, + awardEmoji: true, + }, }, }, }); @@ -227,12 +253,15 @@ describe('Design note component', () => { ...note, userPermissions: { adminNote: true, + awardEmoji: true, }, }; createComponent({ - note: { - ...payload, + props: { + note: { + ...payload, + }, }, }); @@ -240,4 +269,91 @@ describe('Design note component', () => { expect(wrapper.emitted()).toEqual({ 'delete-note': [[{ ...payload }]] }); }); + + describe('when user has award emoji permissions', () => { + const findEmojiPicker = () => wrapper.findComponent(EmojiPicker); + const propsData = { + note: { + ...note, + userPermissions: { + adminNote: false, + awardEmoji: true, + }, + }, + }; + + it('should render emoji-picker button', () => { + createComponent({ props: propsData, mountFn: shallowMountExtended }); + + const emojiPicker = findEmojiPicker(); + + expect(emojiPicker.exists()).toBe(true); + expect(emojiPicker.props()).toMatchObject({ + boundary: 'viewport', + right: false, + }); + }); + + it('should call mutation to add an emoji', () => { + mutate = jest.fn().mockResolvedValue({ + data: { + awardEmojiToggle: { + errors: [], + toggledOn: true, + }, + }, + }); + createComponent({ + props: propsData, + mountFn: shallowMountExtended, + mocks: { + $route, + $apollo: { + mutate, + }, + }, + }); + + findEmojiPicker().vm.$emit('click', 'thumbsup'); + + expect(mutate).toHaveBeenCalledWith({ + mutation: designNoteAwardEmojiToggleMutation, + variables: { + name: 'thumbsup', + awardableId: note.id, + }, + optimisticResponse: { + awardEmojiToggle: { + errors: [], + toggledOn: true, + }, + }, + update: expect.any(Function), + }); + }); + + it('should emit an error when mutation fails', async () => { + jest.spyOn(Sentry, 'captureException'); + mutate = jest.fn().mockRejectedValue({}); + createComponent({ + props: propsData, + mountFn: shallowMountExtended, + mocks: { + $route, + $apollo: { + mutate, + }, + }, + }); + + findEmojiPicker().vm.$emit('click', 'thumbsup'); + + expect(mutate).toHaveBeenCalled(); + + await waitForPromises(); + + expect(Sentry.captureException).toHaveBeenCalled(); + expect(wrapper.emitted('error')).toEqual([[{}]]); + }); + }); }); diff --git a/spec/frontend/design_management/mock_data/apollo_mock.js b/spec/frontend/design_management/mock_data/apollo_mock.js index 063df9366e9..0d004baafd0 100644 --- a/spec/frontend/design_management/mock_data/apollo_mock.js +++ b/spec/frontend/design_management/mock_data/apollo_mock.js @@ -1,3 +1,27 @@ +export const mockAuthor = { + id: 'gid://gitlab/User/1', + name: 'John', + webUrl: 'link-to-john-profile', + avatarUrl: 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + username: 'john.doe', +}; + +export const mockAwardEmoji = { + __typename: 'AwardEmojiConnection', + nodes: [ + { + __typename: 'AwardEmoji', + name: 'briefcase', + user: mockAuthor, + }, + { + __typename: 'AwardEmoji', + name: 'baseball', + user: mockAuthor, + }, + ], +}; + export const designListQueryResponseNodes = [ { __typename: 'Design', @@ -237,6 +261,9 @@ export const mockNoteSubmitSuccessMutationResponse = { webUrl: 'http://127.0.0.1:3000/root', __typename: 'UserCore', }, + awardEmoji: { + nodes: [], + }, body: 'New comment', bodyHtml: "

asdd

", createdAt: '2023-02-24T06:49:20Z', @@ -257,6 +284,7 @@ export const mockNoteSubmitSuccessMutationResponse = { userPermissions: { adminNote: true, repositionNote: true, + awardEmoji: true, __typename: 'NotePermissions', }, discussion: { @@ -363,6 +391,7 @@ export const designFactory = ({ }, userPermissions: { updateDesign, + awardEmoji: true, __typename: 'IssuePermissions', }, __typename: 'Issue', diff --git a/spec/frontend/design_management/mock_data/discussion.js b/spec/frontend/design_management/mock_data/discussion.js index 0e59ef29f8f..fbd5a9e0103 100644 --- a/spec/frontend/design_management/mock_data/discussion.js +++ b/spec/frontend/design_management/mock_data/discussion.js @@ -1,3 +1,5 @@ +import { mockAuthor, mockAwardEmoji } from './apollo_mock'; + export default { id: 'discussion-id-1', resolved: false, @@ -12,13 +14,12 @@ export default { x: 10, y: 15, }, - author: { - name: 'John', - webUrl: 'link-to-john-profile', - }, + author: mockAuthor, + awardEmoji: mockAwardEmoji, createdAt: '2020-05-08T07:10:45Z', userPermissions: { repositionNote: true, + awardEmoji: true, }, resolved: false, }, @@ -32,12 +33,15 @@ export default { y: 25, }, author: { + id: 'gid://gitlab/User/2', name: 'Mary', webUrl: 'link-to-mary-profile', }, + awardEmoji: mockAwardEmoji, createdAt: '2020-05-08T07:10:45Z', userPermissions: { adminNote: true, + awardEmoji: true, }, resolved: false, }, diff --git a/spec/frontend/design_management/mock_data/notes.js b/spec/frontend/design_management/mock_data/notes.js index 41cefaca05b..311ce4d1eb9 100644 --- a/spec/frontend/design_management/mock_data/notes.js +++ b/spec/frontend/design_management/mock_data/notes.js @@ -1,3 +1,4 @@ +import { mockAwardEmoji } from './apollo_mock'; import DISCUSSION_1 from './discussion'; const DISCUSSION_2 = { @@ -17,9 +18,11 @@ const DISCUSSION_2 = { name: 'Mary', webUrl: 'link-to-mary-profile', }, + awardEmoji: mockAwardEmoji, createdAt: '2020-05-08T07:10:45Z', userPermissions: { adminNote: true, + awardEmoji: true, }, resolved: true, }, diff --git a/spec/frontend/vue_shared/components/awards_list_spec.js b/spec/frontend/vue_shared/components/awards_list_spec.js index da5516f8db1..6c28347503c 100644 --- a/spec/frontend/vue_shared/components/awards_list_spec.js +++ b/spec/frontend/vue_shared/components/awards_list_spec.js @@ -74,6 +74,7 @@ describe('vue_shared/components/awards_list', () => { return { classes: x.classes(), title: x.attributes('title'), + emojiName: x.attributes('data-emoji-name'), html: x.find('[data-testid="award-html"]').html(), count: Number(x.find('.js-counter').text()), }; @@ -96,48 +97,56 @@ describe('vue_shared/components/awards_list', () => { count: 3, html: matchingEmojiTag(EMOJI_THUMBSUP), title: `Ada, Leonardo, and Marie reacted with :${EMOJI_THUMBSUP}:`, + emojiName: EMOJI_THUMBSUP, }, { classes: [...REACTION_CONTROL_CLASSES, 'selected'], count: 3, html: matchingEmojiTag(EMOJI_THUMBSDOWN), title: `You, Ada, and Marie reacted with :${EMOJI_THUMBSDOWN}:`, + emojiName: EMOJI_THUMBSDOWN, }, { classes: REACTION_CONTROL_CLASSES, count: 1, html: matchingEmojiTag(EMOJI_100), title: `Ada reacted with :${EMOJI_100}:`, + emojiName: EMOJI_100, }, { classes: REACTION_CONTROL_CLASSES, count: 2, html: matchingEmojiTag(EMOJI_SMILE), title: `Ada and Jane reacted with :${EMOJI_SMILE}:`, + emojiName: EMOJI_SMILE, }, { classes: [...REACTION_CONTROL_CLASSES, 'selected'], count: 4, html: matchingEmojiTag(EMOJI_OK), title: `You, Ada, Jane, and Leonardo reacted with :${EMOJI_OK}:`, + emojiName: EMOJI_OK, }, { classes: [...REACTION_CONTROL_CLASSES, 'selected'], count: 1, html: matchingEmojiTag(EMOJI_CACTUS), title: `You reacted with :${EMOJI_CACTUS}:`, + emojiName: EMOJI_CACTUS, }, { classes: REACTION_CONTROL_CLASSES, count: 1, html: matchingEmojiTag(EMOJI_A), title: `Marie reacted with :${EMOJI_A}:`, + emojiName: EMOJI_A, }, { classes: [...REACTION_CONTROL_CLASSES, 'selected'], count: 1, html: matchingEmojiTag(EMOJI_B), title: `You reacted with :${EMOJI_B}:`, + emojiName: EMOJI_B, }, ]); }); @@ -226,12 +235,14 @@ describe('vue_shared/components/awards_list', () => { count: 0, html: matchingEmojiTag(EMOJI_THUMBSUP), title: '', + emojiName: EMOJI_THUMBSUP, }, { classes: REACTION_CONTROL_CLASSES, count: 0, html: matchingEmojiTag(EMOJI_THUMBSDOWN), title: '', + emojiName: EMOJI_THUMBSDOWN, }, // We expect the EMOJI_100 before the EMOJI_SMILE because it was given as a defaultAward { @@ -239,12 +250,14 @@ describe('vue_shared/components/awards_list', () => { count: 1, html: matchingEmojiTag(EMOJI_100), title: `Marie reacted with :${EMOJI_100}:`, + emojiName: EMOJI_100, }, { classes: REACTION_CONTROL_CLASSES, count: 1, html: matchingEmojiTag(EMOJI_SMILE), title: `Marie reacted with :${EMOJI_SMILE}:`, + emojiName: EMOJI_SMILE, }, ]); }); diff --git a/spec/frontend/vue_shared/components/entity_select/entity_select_spec.js b/spec/frontend/vue_shared/components/entity_select/entity_select_spec.js index 6e2e854adae..36772ad03fe 100644 --- a/spec/frontend/vue_shared/components/entity_select/entity_select_spec.js +++ b/spec/frontend/vue_shared/components/entity_select/entity_select_spec.js @@ -125,7 +125,8 @@ describe('EntitySelect', () => { it('emits `input` event with the select value', async () => { createComponent(); await selectGroup(); - expect(wrapper.emitted('input')[0]).toEqual(['1']); + + expect(wrapper.emitted('input')[0][0]).toMatchObject(itemMock); }); it(`uses the selected group's name as the toggle text`, async () => { @@ -153,14 +154,14 @@ describe('EntitySelect', () => { expect(findListbox().props('toggleText')).toBe(defaultToggleText); }); - it('emits `input` event with `null` on reset', async () => { + it('emits `input` event with an empty object on reset', async () => { createComponent(); await selectGroup(); findListbox().vm.$emit('reset'); await nextTick(); - expect(wrapper.emitted('input')[2]).toEqual([null]); + expect(Object.keys(wrapper.emitted('input')[2][0]).length).toBe(0); }); }); }); diff --git a/spec/metrics_server/metrics_server_spec.rb b/spec/metrics_server/metrics_server_spec.rb index ad80835549f..baf15a773b1 100644 --- a/spec/metrics_server/metrics_server_spec.rb +++ b/spec/metrics_server/metrics_server_spec.rb @@ -69,136 +69,29 @@ RSpec.describe MetricsServer, feature_category: :application_performance do # ru end describe '.spawn' do - context 'for legacy Ruby server' do - let(:expected_env) do - { - 'METRICS_SERVER_TARGET' => target, - 'WIPE_METRICS_DIR' => '0', - 'GITLAB_CONFIG' => 'path/to/config/gitlab.yml' - } - end - - before do - stub_env('GITLAB_CONFIG', 'path/to/config/gitlab.yml') - end - - it 'spawns a new server process and returns its PID' do - expect(Process).to receive(:spawn).with( - expected_env, - end_with('bin/metrics-server'), - hash_including(pgroup: true) - ).and_return(99) - expect(Process).to receive(:detach).with(99) - - pid = described_class.spawn(target, metrics_dir: metrics_dir) - - expect(pid).to eq(99) - end + let(:expected_env) do + { + 'METRICS_SERVER_TARGET' => target, + 'WIPE_METRICS_DIR' => '0', + 'GITLAB_CONFIG' => 'path/to/config/gitlab.yml' + } end - context 'for Golang server' do - let(:log_enabled) { false } - let(:settings) do - GitlabSettings::Options.build( - { - 'web_exporter' => { - 'enabled' => true, - 'address' => 'localhost', - 'port' => '8083', - 'log_enabled' => log_enabled - }, - 'sidekiq_exporter' => { - 'enabled' => true, - 'address' => 'localhost', - 'port' => '8082', - 'log_enabled' => log_enabled - } - } - ) - end + before do + stub_env('GITLAB_CONFIG', 'path/to/config/gitlab.yml') + end - let(:expected_port) { target == 'puma' ? '8083' : '8082' } - let(:expected_env) do - { - 'GOGC' => '10', - 'GME_MMAP_METRICS_DIR' => metrics_dir, - 'GME_PROBES' => 'self,mmap,mmap_stats', - 'GME_SERVER_HOST' => 'localhost', - 'GME_SERVER_PORT' => expected_port, - 'GME_LOG_LEVEL' => 'quiet' - } - end + it 'spawns a new server process and returns its PID' do + expect(Process).to receive(:spawn).with( + expected_env, + end_with('bin/metrics-server'), + hash_including(pgroup: true) + ).and_return(99) + expect(Process).to receive(:detach).with(99) - before do - stub_env('GITLAB_GOLANG_METRICS_SERVER', '1') - allow(::Settings).to receive(:monitoring).and_return(settings) - end + pid = described_class.spawn(target, metrics_dir: metrics_dir) - it 'spawns a new server process and returns its PID' do - expect(Process).to receive(:spawn).with( - expected_env, - 'gitlab-metrics-exporter', - hash_including(pgroup: true) - ).and_return(99) - expect(Process).to receive(:detach).with(99) - - pid = described_class.spawn(target, metrics_dir: metrics_dir) - - expect(pid).to eq(99) - end - - it 'can launch from explicit path instead of PATH' do - expect(Process).to receive(:spawn).with( - expected_env, - '/path/to/gme/gitlab-metrics-exporter', - hash_including(pgroup: true) - ).and_return(99) - - described_class.spawn(target, metrics_dir: metrics_dir, path: '/path/to/gme/') - end - - context 'when logs are enabled' do - let(:log_enabled) { true } - let(:expected_log_file) { target == 'puma' ? 'web_exporter.log' : 'sidekiq_exporter.log' } - - it 'sets log related environment variables' do - expect(Process).to receive(:spawn).with( - expected_env.merge( - 'GME_LOG_LEVEL' => 'info', - 'GME_LOG_FILE' => File.join(Rails.root, 'log', expected_log_file) - ), - 'gitlab-metrics-exporter', - hash_including(pgroup: true) - ).and_return(99) - - described_class.spawn(target, metrics_dir: metrics_dir) - end - end - - context 'when TLS settings are present' do - before do - settings.web_exporter['tls_enabled'] = true - settings.web_exporter['tls_cert_path'] = '/path/to/cert.pem' - settings.web_exporter['tls_key_path'] = '/path/to/key.pem' - - settings.sidekiq_exporter['tls_enabled'] = true - settings.sidekiq_exporter['tls_cert_path'] = '/path/to/cert.pem' - settings.sidekiq_exporter['tls_key_path'] = '/path/to/key.pem' - end - - it 'sets the correct environment variables' do - expect(Process).to receive(:spawn).with( - expected_env.merge( - 'GME_CERT_FILE' => '/path/to/cert.pem', - 'GME_CERT_KEY' => '/path/to/key.pem' - ), - '/path/to/gme/gitlab-metrics-exporter', - hash_including(pgroup: true) - ).and_return(99) - - described_class.spawn(target, metrics_dir: metrics_dir, path: '/path/to/gme/') - end - end + expect(pid).to eq(99) end end end @@ -214,21 +107,10 @@ RSpec.describe MetricsServer, feature_category: :application_performance do # ru end describe '.spawn' do - context 'for legacy Ruby server' do - it 'raises an error' do - expect { described_class.spawn('unsupported', metrics_dir: metrics_dir) }.to( - raise_error('Target must be one of [puma,sidekiq]') - ) - end - end - - context 'for Golang server' do - it 'raises an error' do - stub_env('GITLAB_GOLANG_METRICS_SERVER', '1') - expect { described_class.spawn('unsupported', metrics_dir: metrics_dir) }.to( - raise_error('Target must be one of [puma,sidekiq]') - ) - end + it 'raises an error' do + expect { described_class.spawn('unsupported', metrics_dir: metrics_dir) }.to( + raise_error('Target must be one of [puma,sidekiq]') + ) end end end @@ -345,21 +227,10 @@ RSpec.describe MetricsServer, feature_category: :application_performance do # ru end describe '.start_for_sidekiq' do - context 'for legacy Ruby server' do - it 'forks the parent process' do - expect(Process).to receive(:fork).and_return(42) + it 'forks the parent process' do + expect(Process).to receive(:fork).and_return(42) - described_class.start_for_sidekiq(metrics_dir: '/path/to/metrics') - end - end - - context 'for Golang server' do - it 'spawns the server process' do - stub_env('GITLAB_GOLANG_METRICS_SERVER', '1') - expect(Process).to receive(:spawn).and_return(42) - - described_class.start_for_sidekiq(metrics_dir: '/path/to/metrics') - end + described_class.start_for_sidekiq(metrics_dir: '/path/to/metrics') end end diff --git a/spec/support/capybara.rb b/spec/support/capybara.rb index c7b2a03fde2..a855e4468ca 100644 --- a/spec/support/capybara.rb +++ b/spec/support/capybara.rb @@ -233,5 +233,6 @@ RSpec.configure do |config| # We don't reset the session when the example failed, because we need capybara-screenshot to have access to it. Capybara.reset_sessions! unless example.exception block_and_wait_for_requests_complete + block_and_wait_for_action_cable_requests_complete end end diff --git a/spec/support/helpers/wait_for_requests.rb b/spec/support/helpers/wait_for_requests.rb index 5e2e8ad53e0..b7a3b77a694 100644 --- a/spec/support/helpers/wait_for_requests.rb +++ b/spec/support/helpers/wait_for_requests.rb @@ -27,6 +27,17 @@ module WaitForRequests Gitlab::Testing::RequestBlockerMiddleware.allow_requests! end + def block_and_wait_for_action_cable_requests_complete + block_action_cable_requests { wait_for_action_cable_requests } + end + + def block_action_cable_requests + Gitlab::Testing::ActionCableBlocker.block_requests! + yield + ensure + Gitlab::Testing::ActionCableBlocker.allow_requests! + end + # Wait for client-side AJAX requests def wait_for_requests wait_for('JS requests complete', max_wait_time: 2 * Capybara.default_max_wait_time) do @@ -42,6 +53,12 @@ module WaitForRequests end end + def wait_for_action_cable_requests + wait_for('Action Cable requests complete') do + Gitlab::Testing::ActionCableBlocker.num_active_requests == 0 + end + end + private def finished_all_rack_requests? diff --git a/spec/tasks/gitlab/metrics_exporter_rake_spec.rb b/spec/tasks/gitlab/metrics_exporter_rake_spec.rb deleted file mode 100644 index ca37fc1b5d7..00000000000 --- a/spec/tasks/gitlab/metrics_exporter_rake_spec.rb +++ /dev/null @@ -1,81 +0,0 @@ -# frozen_string_literal: true - -require 'rake_helper' -require_relative '../../support/helpers/next_instance_of' - -RSpec.describe 'gitlab:metrics_exporter:install', feature_category: :metrics do - before do - Rake.application.rake_require 'tasks/gitlab/metrics_exporter' - end - - subject(:task) do - Rake::Task['gitlab:metrics_exporter:install'] - end - - context 'when no target directory is specified' do - it 'aborts with an error message' do - expect do - expect { task.execute }.to output(/Please specify the directory/).to_stdout - end.to raise_error(SystemExit) - end - end - - context 'when target directory is specified' do - let(:args) { Rake::TaskArguments.new(%w[dir], %w[path/to/exporter]) } - let(:context) { TOPLEVEL_BINDING.eval('self') } - let(:expected_clone_params) do - { - repo: 'https://gitlab.com/gitlab-org/gitlab-metrics-exporter.git', - version: an_instance_of(String), - target_dir: 'path/to/exporter' - } - end - - context 'when dependencies are missing' do - it 'aborts with an error message' do - expect(Gitlab::Utils).to receive(:which).with('gmake').ordered - expect(Gitlab::Utils).to receive(:which).with('make').ordered - - expect do - expect { task.execute(args) }.to output(/Couldn't find a 'make' binary/).to_stdout - end.to raise_error(SystemExit) - end - end - - it 'installs the exporter with gmake' do - expect(Gitlab::Utils).to receive(:which).with('gmake').and_return('path/to/gmake').ordered - expect(context).to receive(:checkout_or_clone_version).with(hash_including(expected_clone_params)).ordered - expect(Dir).to receive(:chdir).with('path/to/exporter').and_yield.ordered - expect(context).to receive(:run_command!).with(['path/to/gmake']).ordered - - task.execute(args) - end - - it 'installs the exporter with make' do - expect(Gitlab::Utils).to receive(:which).with('gmake').ordered - expect(Gitlab::Utils).to receive(:which).with('make').and_return('path/to/make').ordered - expect(context).to receive(:checkout_or_clone_version).with(hash_including(expected_clone_params)).ordered - expect(Dir).to receive(:chdir).with('path/to/exporter').and_yield.ordered - expect(context).to receive(:run_command!).with(['path/to/make']).ordered - - task.execute(args) - end - - context 'when overriding version via environment variable' do - before do - stub_env('GITLAB_METRICS_EXPORTER_VERSION', '1.0') - end - - it 'clones from repository with that version instead' do - expect(Gitlab::Utils).to receive(:which).with('gmake').and_return('path/to/gmake').ordered - expect(context).to receive(:checkout_or_clone_version).with( - hash_including(expected_clone_params.merge(version: '1.0')) - ).ordered - expect(Dir).to receive(:chdir).with('path/to/exporter').and_yield.ordered - expect(context).to receive(:run_command!).with(['path/to/gmake']).ordered - - task.execute(args) - end - end - end -end diff --git a/yarn.lock b/yarn.lock index d40f56f5fc0..3e4dbc08868 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1132,10 +1132,10 @@ resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-3.54.0.tgz#6002ed7b3c2db832bef34629d6d5677ac36c45d6" integrity sha512-Fvo/0lF/Gx+na21Qg4qr02EsP1OEhVlkuh8ctmHMLu5cr5ho3b/MZYLHLjI8F5FDkTIpennyYuhxqiU8kTVM2Q== -"@gitlab/ui@64.18.3": - version "64.18.3" - resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-64.18.3.tgz#a09576c0568d2e4c2fc122a32125e40bae25dcfa" - integrity sha512-0UJjg70ndEcI2OlHdZ70jAVXY2ETBB5ORMla9oWV+cGpJD8fVmmHj4q+GN51mSDh+ZLzDrcYO8t6xnJhQOvHvQ== +"@gitlab/ui@64.19.0": + version "64.19.0" + resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-64.19.0.tgz#89b77cac7483027c5087e0b5c79f66eed4fa6183" + integrity sha512-aU1+/kM71YOlXqS9HhQ4ya0yF5UJS8mtB+aP8ThWui6SWymgm59VoER1U22bND7P32IOQ5meRXpgNN3T70vZSg== dependencies: "@floating-ui/dom" "1.2.9" bootstrap-vue "2.23.1"