From 0c51dc19cb6b2a6d9919b129bc6231e3fc1f0bb2 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Fri, 20 Aug 2021 06:11:03 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- app/models/diff_note.rb | 28 ++++---- app/models/note.rb | 3 + app/models/shard.rb | 6 +- app/services/draft_notes/publish_service.rb | 20 +++++- app/workers/all_queues.yml | 27 -------- app/workers/build_success_worker.rb | 2 - app/workers/deployments/finished_worker.rb | 24 ------- .../deployments/forward_deployment_worker.rb | 20 ------ app/workers/deployments/success_worker.rb | 25 ------- .../documentation/styleguide/index.md | 22 +------ .../documentation/styleguide/word_list.md | 37 +++++++++++ locale/gitlab.pot | 8 +-- .../merge_when_pipeline_succeeds_spec.rb | 2 +- spec/features/cycle_analytics_spec.rb | 12 ++-- .../gitlab/ci/parsers/security/common_spec.rb | 2 - spec/models/diff_note_spec.rb | 27 ++++++++ spec/models/shard_spec.rb | 20 +++--- .../draft_notes/publish_service_spec.rb | 27 +++++++- .../deployments/finished_worker_spec.rb | 65 ------------------- .../deployments/success_worker_spec.rb | 38 ----------- 20 files changed, 150 insertions(+), 265 deletions(-) delete mode 100644 app/workers/deployments/finished_worker.rb delete mode 100644 app/workers/deployments/forward_deployment_worker.rb delete mode 100644 app/workers/deployments/success_worker.rb delete mode 100644 spec/workers/deployments/finished_worker_spec.rb delete mode 100644 spec/workers/deployments/success_worker_spec.rb diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb index c8a0773cc5b..6ebac6384bc 100644 --- a/app/models/diff_note.rb +++ b/app/models/diff_note.rb @@ -22,7 +22,7 @@ class DiffNote < Note validate :verify_supported, unless: :importing? before_validation :set_line_code, if: :on_text?, unless: :importing? - after_save :keep_around_commits, unless: :importing? + after_save :keep_around_commits, unless: -> { importing? || skip_keep_around_commits } NoteDiffFileCreationError = Class.new(StandardError) @@ -115,6 +115,20 @@ class DiffNote < Note position&.multiline? end + def shas + [ + self.original_position.base_sha, + self.original_position.start_sha, + self.original_position.head_sha + ].tap do |a| + if self.position != self.original_position + a << self.position.base_sha + a << self.position.start_sha + a << self.position.head_sha + end + end + end + private def enqueue_diff_file_creation_job @@ -173,18 +187,6 @@ class DiffNote < Note end def keep_around_commits - shas = [ - self.original_position.base_sha, - self.original_position.start_sha, - self.original_position.head_sha - ] - - if self.position != self.original_position - shas << self.position.base_sha - shas << self.position.start_sha - shas << self.position.head_sha - end - repository.keep_around(*shas) end diff --git a/app/models/note.rb b/app/models/note.rb index 34ffd7c91af..58a00fe6fa1 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -48,6 +48,9 @@ class Note < ApplicationRecord # Attribute used to store the attributes that have been changed by quick actions. attr_accessor :commands_changes + # Attribute used to determine whether keep_around_commits will be skipped for diff notes. + attr_accessor :skip_keep_around_commits + default_value_for :system, false attr_mentionable :note, pipeline: :note diff --git a/app/models/shard.rb b/app/models/shard.rb index 335a279c6aa..9f0039d8bf9 100644 --- a/app/models/shard.rb +++ b/app/models/shard.rb @@ -18,10 +18,6 @@ class Shard < ApplicationRecord end def self.by_name(name) - transaction(requires_new: true) do - find_or_create_by(name: name) - end - rescue ActiveRecord::RecordNotUnique - retry + safe_find_or_create_by(name: name) end end diff --git a/app/services/draft_notes/publish_service.rb b/app/services/draft_notes/publish_service.rb index 3a1db16aaf4..a82a6e22a5a 100644 --- a/app/services/draft_notes/publish_service.rb +++ b/app/services/draft_notes/publish_service.rb @@ -34,22 +34,24 @@ module DraftNotes created_notes = draft_notes.map do |draft_note| draft_note.review = review - create_note_from_draft(draft_note, skip_capture_diff_note_position: true) + create_note_from_draft(draft_note, skip_capture_diff_note_position: true, skip_keep_around_commits: true) end capture_diff_note_positions(created_notes) + keep_around_commits(created_notes) draft_notes.delete_all set_reviewed notification_service.async.new_review(review) MergeRequests::ResolvedDiscussionNotificationService.new(project: project, current_user: current_user).execute(merge_request) end - def create_note_from_draft(draft, skip_capture_diff_note_position: false) + def create_note_from_draft(draft, skip_capture_diff_note_position: false, skip_keep_around_commits: false) # Make sure the diff file is unfolded in order to find the correct line # codes. draft.diff_file&.unfold_diff_lines(draft.original_position) - note = Notes::CreateService.new(draft.project, draft.author, draft.publish_params).execute( + note_params = draft.publish_params.merge(skip_keep_around_commits: skip_keep_around_commits) + note = Notes::CreateService.new(draft.project, draft.author, note_params).execute( skip_capture_diff_note_position: skip_capture_diff_note_position ) @@ -86,5 +88,17 @@ module DraftNotes capture_service.execute(note.discussion) if note.diff_note? && note.start_of_discussion? end end + + def keep_around_commits(notes) + shas = notes.flat_map do |note| + note.shas if note.diff_note? + end.uniq + + # We are allowing this since gitaly call will be created for each sha and + # even though they're unique, there will still be multiple Gitaly calls. + Gitlab::GitalyClient.allow_n_plus_1_calls do + project.repository.keep_around(*shas) + end + end end end diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index f326ae0dec8..7b0ae683bac 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -673,24 +673,6 @@ :idempotent: :tags: - :exclude_from_kubernetes -- :name: deployment:deployments_finished - :worker_name: Deployments::FinishedWorker - :feature_category: :continuous_delivery - :has_external_dependencies: - :urgency: :low - :resource_boundary: :cpu - :weight: 3 - :idempotent: - :tags: [] -- :name: deployment:deployments_forward_deployment - :worker_name: Deployments::ForwardDeploymentWorker - :feature_category: :continuous_delivery - :has_external_dependencies: - :urgency: :low - :resource_boundary: :unknown - :weight: 3 - :idempotent: - :tags: [] - :name: deployment:deployments_hooks :worker_name: Deployments::HooksWorker :feature_category: :continuous_delivery @@ -709,15 +691,6 @@ :weight: 3 :idempotent: true :tags: [] -- :name: deployment:deployments_success - :worker_name: Deployments::SuccessWorker - :feature_category: :continuous_delivery - :has_external_dependencies: - :urgency: :low - :resource_boundary: :cpu - :weight: 3 - :idempotent: - :tags: [] - :name: deployment:deployments_update_environment :worker_name: Deployments::UpdateEnvironmentWorker :feature_category: :continuous_delivery diff --git a/app/workers/build_success_worker.rb b/app/workers/build_success_worker.rb index 3f3d61a8df1..ce39ac946a9 100644 --- a/app/workers/build_success_worker.rb +++ b/app/workers/build_success_worker.rb @@ -21,8 +21,6 @@ class BuildSuccessWorker # rubocop:disable Scalability/IdempotentWorker private - ## - # TODO: This should be processed in DeploymentSuccessWorker once we started storing `action` value in `deployments` records def stop_environment(build) build.persisted_environment.fire_state_event(:stop) end diff --git a/app/workers/deployments/finished_worker.rb b/app/workers/deployments/finished_worker.rb deleted file mode 100644 index 25121656408..00000000000 --- a/app/workers/deployments/finished_worker.rb +++ /dev/null @@ -1,24 +0,0 @@ -# frozen_string_literal: true - -# This worker is deprecated and will be removed in 14.0 -# See: https://gitlab.com/gitlab-org/gitlab/-/issues/266381 -module Deployments - class FinishedWorker # rubocop:disable Scalability/IdempotentWorker - include ApplicationWorker - - data_consistency :always - - sidekiq_options retry: 3 - - queue_namespace :deployment - feature_category :continuous_delivery - worker_resource_boundary :cpu - - def perform(deployment_id) - if (deploy = Deployment.find_by_id(deployment_id)) - LinkMergeRequestsService.new(deploy).execute - deploy.execute_hooks(Time.current) - end - end - end -end diff --git a/app/workers/deployments/forward_deployment_worker.rb b/app/workers/deployments/forward_deployment_worker.rb deleted file mode 100644 index 7f5eb13b88d..00000000000 --- a/app/workers/deployments/forward_deployment_worker.rb +++ /dev/null @@ -1,20 +0,0 @@ -# frozen_string_literal: true - -# This worker is deprecated and will be removed in 14.0 -# See: https://gitlab.com/gitlab-org/gitlab/-/issues/266381 -module Deployments - class ForwardDeploymentWorker # rubocop:disable Scalability/IdempotentWorker - include ApplicationWorker - - data_consistency :always - - sidekiq_options retry: 3 - - queue_namespace :deployment - feature_category :continuous_delivery - - def perform(deployment_id) - Deployments::OlderDeploymentsDropService.new(deployment_id).execute - end - end -end diff --git a/app/workers/deployments/success_worker.rb b/app/workers/deployments/success_worker.rb deleted file mode 100644 index 401c2d7600c..00000000000 --- a/app/workers/deployments/success_worker.rb +++ /dev/null @@ -1,25 +0,0 @@ -# frozen_string_literal: true - -# This worker is deprecated and will be removed in 14.0 -# See: https://gitlab.com/gitlab-org/gitlab/-/issues/266381 -module Deployments - class SuccessWorker # rubocop:disable Scalability/IdempotentWorker - include ApplicationWorker - - data_consistency :always - - sidekiq_options retry: 3 - - queue_namespace :deployment - feature_category :continuous_delivery - worker_resource_boundary :cpu - - def perform(deployment_id) - Deployment.find_by_id(deployment_id).try do |deployment| - break unless deployment.success? - - Deployments::UpdateEnvironmentService.new(deployment).execute - end - end - end -end diff --git a/doc/development/documentation/styleguide/index.md b/doc/development/documentation/styleguide/index.md index 4e548179b9e..c69b5f63abe 100644 --- a/doc/development/documentation/styleguide/index.md +++ b/doc/development/documentation/styleguide/index.md @@ -1456,27 +1456,9 @@ Follow these styles when you're describing user interface elements in an application: - For elements with a visible label, use that label in bold with matching case. - For example, `the **Cancel** button`. + For example, `Select **Cancel**`. - For elements with a tooltip or hover label, use that label in bold with - matching case. For example, `the **Add status emoji** button`. - -### Verbs for UI elements - -Use these verbs for specific uses with user interface -elements: - -| Recommended | Used for | Replaces | -|:--------------------|:--------------------------------------|:----------------------| -| select | buttons, links, menu items, dropdowns | click, press, hit | -| select or clear | checkboxes | enable, click, press | -| expand | expandable sections | open | -| turn on or turn off | toggles | flip, enable, disable | - -### Other Verbs - -| Recommended | Used for | Replaces | -|:------------|:--------------------------------|:----------------------| -| go to | making a browser go to location | navigate to, open | + matching case. For example, `Select **Add status emoji**`. ## GitLab versions diff --git a/doc/development/documentation/styleguide/word_list.md b/doc/development/documentation/styleguide/word_list.md index bdd25534940..240b78a741f 100644 --- a/doc/development/documentation/styleguide/word_list.md +++ b/doc/development/documentation/styleguide/word_list.md @@ -75,10 +75,17 @@ Use lowercase for **boards**, **issue boards**, and **epic boards**. One word, **checkbox**. Do not use **check box**. +You **select** (not **check** or **enable**) and **clear** (not **deselect** or **disable**) checkboxes. + ## CI/CD Always uppercase. No need to spell out on first use. +## click + +Do not use. Instead, use **select** with buttons, links, menu items, and lists. +**Select** applies to more devices, while **click** is more specific to a mouse. + ## currently Do not use when talking about the product or its features. The documentation describes the product as it is today. ([Vale](../testing.md#vale) rule: [`CurrentStatus.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/doc/.vale/gitlab/CurrentStatus.yml)) @@ -117,6 +124,10 @@ Do not use. If the user doesn't find the process to be easy, we lose their trust Do not use Latin abbreviations. Use **for example**, **such as**, **for instance**, or **like** instead. ([Vale](../testing.md#vale) rule: [`LatinTerms.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/doc/.vale/gitlab/LatinTerms.yml)) +## expand + +Use instead of **open** when you are talking about expanding or collapsing a section in the UI. + ## email Do not use **e-mail** with a hyphen. When plural, use **emails** or **email messages**. @@ -203,6 +214,13 @@ Do not use when talking about version numbers. - Avoid: In GitLab 14.1 and higher. - Use instead: In GitLab 14.1 and later. +## hit + +Don't use to mean **press**. + +- Avoid: Hit the **ENTER** button. +- Use instead: Press **ENTER**. + ## I Do not use first-person singular. Use **you**, **we**, or **us** instead. ([Vale](../testing.md#vale) rule: [`FirstPerson.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/doc/.vale/gitlab/FirstPerson.yml)) @@ -293,6 +311,13 @@ Lowercase. If you use **MR** as the acronym, spell it out on first use. Lowercase. +## navigate + +Do not use. Use **go** instead. For example: + +- Go to this webpage. +- Open a terminal and go to the `runner` directory. + ## need to, should Try to avoid. If something is required, use **must**. @@ -334,6 +359,12 @@ Do not use roles and permissions interchangeably. Each user is assigned a role. Do not use. For details, see the [Microsoft style guide](https://docs.microsoft.com/en-us/style-guide/a-z-word-list-term-collections/p/please). +## press + +Use when talking about keyboard keys. For example: + +- To stop the command, press Control+C. + ## profanity Do not use. Doing so may negatively affect other users and contributors, which is contrary to the GitLab value of [Diversity, Inclusion, and Belonging](https://about.gitlab.com/handbook/values/#diversity-inclusion). @@ -454,6 +485,12 @@ Use lowercase. ([Vale](../testing.md#vale) rule: [`ToDo.yml`](https://gitlab.com Use title case. ([Vale](../testing.md#vale) rule: [`ToDo.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/doc/.vale/gitlab/ToDo.yml)) +## toggle + +You **turn on** or **turn off** a toggle. For example: + +- Turn on the **blah** toggle. + ## useful Do not use. If the user doesn't find the process to be useful, we lose their trust. diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 7f3ee036c07..e337162e5f2 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1539,10 +1539,10 @@ msgstr "" msgid "API?" msgstr "" -msgid "APIFuzzing|$VariableWithPassword" +msgid "APIFuzzing|$VARIABLE_WITH_PASSWORD" msgstr "" -msgid "APIFuzzing|$VariableWithUsername" +msgid "APIFuzzing|$VARIABLE_WITH_USERNAME" msgstr "" msgid "APIFuzzing|API Fuzzing Configuration" @@ -1569,10 +1569,10 @@ msgstr "" msgid "APIFuzzing|Enable authentication" msgstr "" -msgid "APIFuzzing|Enter the name of the variable containing the password. For example, $VariableWithPassword." +msgid "APIFuzzing|Enter the name of the CI variable containing the password. For example, $VARIABLE_WITH_PASSWORD." msgstr "" -msgid "APIFuzzing|Enter the name of the variable containing the username. For example, $VariableWithUsername." +msgid "APIFuzzing|Enter the name of the CI variable containing the username. For example, $VARIABLE_WITH_USERNAME." msgstr "" msgid "APIFuzzing|File path or URL to APIs to be tested. For example, folder/example_fuzz.har. HAR files may contain sensitive information such as authentication tokens, API keys, and session cookies. We recommend that you review the HAR files' contents before adding them to a repository." diff --git a/qa/qa/specs/features/browser_ui/3_create/merge_request/merge_when_pipeline_succeeds_spec.rb b/qa/qa/specs/features/browser_ui/3_create/merge_request/merge_when_pipeline_succeeds_spec.rb index 2b1ac75b9f5..3ee444e56c3 100644 --- a/qa/qa/specs/features/browser_ui/3_create/merge_request/merge_when_pipeline_succeeds_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/merge_request/merge_when_pipeline_succeeds_spec.rb @@ -30,7 +30,7 @@ module QA content: <<~EOF test: tags: ["runner-for-#{project.name}"] - script: sleep 10 + script: sleep 20 only: - merge_requests EOF diff --git a/spec/features/cycle_analytics_spec.rb b/spec/features/cycle_analytics_spec.rb index de6cb53fdfa..20b3ec9d9bb 100644 --- a/spec/features/cycle_analytics_spec.rb +++ b/spec/features/cycle_analytics_spec.rb @@ -43,6 +43,10 @@ RSpec.describe 'Value Stream Analytics', :js do end context "when there's value stream analytics data" do + around do |example| + travel_to(5.days.ago) { example.run } + end + before do project.add_maintainer(user) @@ -53,10 +57,10 @@ RSpec.describe 'Value Stream Analytics', :js do merge_request = issue.merge_requests_closing_issues.first.merge_request merge_request.update!(created_at: issue.metrics.first_associated_with_milestone_at + 1.hour) merge_request.metrics.update!( - latest_build_started_at: 4.hours.ago, - latest_build_finished_at: 3.hours.ago, - merged_at: merge_request.created_at + 1.hour, - first_deployed_to_production_at: merge_request.created_at + 2.hours + latest_build_started_at: merge_request.created_at + 3.hours, + latest_build_finished_at: merge_request.created_at + 4.hours, + merged_at: merge_request.created_at + 4.hours, + first_deployed_to_production_at: merge_request.created_at + 5.hours ) sign_in(user) diff --git a/spec/lib/gitlab/ci/parsers/security/common_spec.rb b/spec/lib/gitlab/ci/parsers/security/common_spec.rb index c6387bf615b..8498e0c993a 100644 --- a/spec/lib/gitlab/ci/parsers/security/common_spec.rb +++ b/spec/lib/gitlab/ci/parsers/security/common_spec.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -# TODO remove duplication from spec/lib/gitlab/ci/parsers/security/common_spec.rb and spec/lib/gitlab/ci/parsers/security/common_spec.rb -# See https://gitlab.com/gitlab-org/gitlab/-/issues/336589 require 'spec_helper' RSpec.describe Gitlab::Ci::Parsers::Security::Common do diff --git a/spec/models/diff_note_spec.rb b/spec/models/diff_note_spec.rb index 11652d9841b..f377b34679c 100644 --- a/spec/models/diff_note_spec.rb +++ b/spec/models/diff_note_spec.rb @@ -558,4 +558,31 @@ RSpec.describe DiffNote do it { is_expected.to eq('note') } end + + describe '#shas' do + it 'returns list of SHAs based on original_position' do + expect(subject.shas).to match_array([ + position.base_sha, + position.start_sha, + position.head_sha + ]) + end + + context 'when position changes' do + before do + subject.position = new_position + end + + it 'includes the new position SHAs' do + expect(subject.shas).to match_array([ + position.base_sha, + position.start_sha, + position.head_sha, + new_position.base_sha, + new_position.start_sha, + new_position.head_sha + ]) + end + end + end end diff --git a/spec/models/shard_spec.rb b/spec/models/shard_spec.rb index a9d11f4290c..38729fa1758 100644 --- a/spec/models/shard_spec.rb +++ b/spec/models/shard_spec.rb @@ -33,19 +33,21 @@ RSpec.describe Shard do expect(result.name).to eq('foo') end - it 'retries if creation races' do + it 'returns existing record if creation races' do + shard_created_by_others = double(described_class) + expect(described_class) - .to receive(:find_or_create_by) - .with(name: 'default') + .to receive(:find_by) + .with(name: 'new_shard') + .and_return(nil, shard_created_by_others) + + expect(described_class) + .to receive(:create) + .with(name: 'new_shard') .and_raise(ActiveRecord::RecordNotUnique, 'fail') .once - expect(described_class) - .to receive(:find_or_create_by) - .with(name: 'default') - .and_call_original - - expect(described_class.by_name('default')).to eq(default_shard) + expect(described_class.by_name('new_shard')).to eq(shard_created_by_others) end end end diff --git a/spec/services/draft_notes/publish_service_spec.rb b/spec/services/draft_notes/publish_service_spec.rb index 4f761454516..51ef30c91c0 100644 --- a/spec/services/draft_notes/publish_service_spec.rb +++ b/spec/services/draft_notes/publish_service_spec.rb @@ -33,7 +33,8 @@ RSpec.describe DraftNotes::PublishService do end it 'does not skip notification', :sidekiq_might_not_need_inline do - expect(Notes::CreateService).to receive(:new).with(project, user, drafts.first.publish_params).and_call_original + note_params = drafts.first.publish_params.merge(skip_keep_around_commits: false) + expect(Notes::CreateService).to receive(:new).with(project, user, note_params).and_call_original expect_next_instance_of(NotificationService) do |notification_service| expect(notification_service).to receive(:new_note) end @@ -127,12 +128,17 @@ RSpec.describe DraftNotes::PublishService do publish end - context 'capturing diff notes positions' do + context 'capturing diff notes positions and keeping around commits' do before do # Need to execute this to ensure that we'll be able to test creation of # DiffNotePosition records as that only happens when the `MergeRequest#merge_ref_head` # is present. This service creates that for the specified merge request. MergeRequests::MergeToRefService.new(project: project, current_user: user).execute(merge_request) + + # Need to re-stub this and call original as we are stubbing + # `Gitlab::Git::KeepAround#execute` in spec_helper for performance reason. + # Enabling it here so we can test the Gitaly calls it makes. + allow(Gitlab::Git::KeepAround).to receive(:execute).and_call_original end it 'creates diff_note_positions for diff notes' do @@ -143,11 +149,26 @@ RSpec.describe DraftNotes::PublishService do expect(notes.last.diff_note_positions).to be_any end + it 'keeps around the commits of each published note' do + publish + + repository = project.repository + notes = merge_request.notes.order(id: :asc) + + notes.first.shas.each do |sha| + expect(repository.ref_exists?("refs/keep-around/#{sha}")).to be_truthy + end + + notes.last.shas.each do |sha| + expect(repository.ref_exists?("refs/keep-around/#{sha}")).to be_truthy + end + end + it 'does not requests a lot from Gitaly', :request_store do # NOTE: This should be reduced as we work on reducing Gitaly calls. # Gitaly requests shouldn't go above this threshold as much as possible # as it may add more to the Gitaly N+1 issue we are experiencing. - expect { publish }.to change { Gitlab::GitalyClient.get_request_count }.by(11) + expect { publish }.to change { Gitlab::GitalyClient.get_request_count }.by(21) end end diff --git a/spec/workers/deployments/finished_worker_spec.rb b/spec/workers/deployments/finished_worker_spec.rb deleted file mode 100644 index d0a26ae1547..00000000000 --- a/spec/workers/deployments/finished_worker_spec.rb +++ /dev/null @@ -1,65 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Deployments::FinishedWorker do - let(:worker) { described_class.new } - - describe '#perform' do - before do - allow(ProjectServiceWorker).to receive(:perform_async) - end - - it 'links merge requests to the deployment' do - deployment = create(:deployment) - service = instance_double(Deployments::LinkMergeRequestsService) - - expect(Deployments::LinkMergeRequestsService) - .to receive(:new) - .with(deployment) - .and_return(service) - - expect(service).to receive(:execute) - - worker.perform(deployment.id) - end - - it 'executes project services for deployment_hooks' do - deployment = create(:deployment) - project = deployment.project - service = create(:service, type: 'SlackService', project: project, deployment_events: true, active: true) - - worker.perform(deployment.id) - - expect(ProjectServiceWorker).to have_received(:perform_async).with(service.id, an_instance_of(Hash)) - end - - it 'does not execute an inactive service' do - deployment = create(:deployment) - project = deployment.project - create(:service, type: 'SlackService', project: project, deployment_events: true, active: false) - - worker.perform(deployment.id) - - expect(ProjectServiceWorker).not_to have_received(:perform_async) - end - - it 'does nothing if a deployment with the given id does not exist' do - worker.perform(0) - - expect(ProjectServiceWorker).not_to have_received(:perform_async) - end - - it 'execute webhooks' do - deployment = create(:deployment) - project = deployment.project - web_hook = create(:project_hook, deployment_events: true, project: project) - - expect_next_instance_of(WebHookService, web_hook, an_instance_of(Hash), "deployment_hooks") do |service| - expect(service).to receive(:async_execute) - end - - worker.perform(deployment.id) - end - end -end diff --git a/spec/workers/deployments/success_worker_spec.rb b/spec/workers/deployments/success_worker_spec.rb deleted file mode 100644 index d9996e66919..00000000000 --- a/spec/workers/deployments/success_worker_spec.rb +++ /dev/null @@ -1,38 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Deployments::SuccessWorker do - subject { described_class.new.perform(deployment&.id) } - - context 'when successful deployment' do - let(:deployment) { create(:deployment, :success) } - - it 'executes Deployments::UpdateEnvironmentService' do - expect(Deployments::UpdateEnvironmentService) - .to receive(:new).with(deployment).and_call_original - - subject - end - end - - context 'when canceled deployment' do - let(:deployment) { create(:deployment, :canceled) } - - it 'does not execute Deployments::UpdateEnvironmentService' do - expect(Deployments::UpdateEnvironmentService).not_to receive(:new) - - subject - end - end - - context 'when deploy record does not exist' do - let(:deployment) { nil } - - it 'does not execute Deployments::UpdateEnvironmentService' do - expect(Deployments::UpdateEnvironmentService).not_to receive(:new) - - subject - end - end -end