diff options
33 files changed, 253 insertions, 49 deletions
diff --git a/app/assets/javascripts/diffs/components/diff_file_row.vue b/app/assets/javascripts/diffs/components/diff_file_row.vue index 15e63a1c9ca..c8ba8d6040e 100644 --- a/app/assets/javascripts/diffs/components/diff_file_row.vue +++ b/app/assets/javascripts/diffs/components/diff_file_row.vue @@ -35,6 +35,6 @@ export default { <template> <file-row :file="file" v-bind="$attrs" v-on="$listeners"> <file-row-stats v-if="showFileRowStats" :file="file" class="mr-1" /> - <changed-file-icon :file="file" :size="16" /> + <changed-file-icon :file="file" :size="16" :show-tooltip="true" /> </file-row> </template> diff --git a/app/assets/javascripts/vue_shared/components/changed_file_icon.vue b/app/assets/javascripts/vue_shared/components/changed_file_icon.vue index 7acbe949151..1bd320d81e8 100644 --- a/app/assets/javascripts/vue_shared/components/changed_file_icon.vue +++ b/app/assets/javascripts/vue_shared/components/changed_file_icon.vue @@ -1,8 +1,8 @@ <script> import { GlTooltipDirective } from '@gitlab/ui'; import Icon from '~/vue_shared/components/icon.vue'; -import { __ } from '~/locale'; import { getCommitIconMap } from '~/ide/utils'; +import { __ } from '~/locale'; export default { components: { @@ -49,9 +49,17 @@ export default { return `${this.changedIcon} float-left d-block`; }, tooltipTitle() { - if (!this.showTooltip || !this.file.changed) return undefined; + if (!this.showTooltip) { + return undefined; + } else if (this.file.deleted) { + return __('Deleted'); + } else if (this.file.tempFile) { + return __('Added'); + } else if (this.file.changed) { + return __('Modified'); + } - return this.file.tempFile ? __('Added') : __('Modified'); + return undefined; }, showIcon() { return ( diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index d841601cf4b..b7bab9bc226 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -236,7 +236,11 @@ module Ci after_transition any => [:success, :failed] do |pipeline| pipeline.run_after_commit do - PipelineUpdateCiRefStatusWorker.perform_async(pipeline.id) + if Feature.enabled?(:ci_pipeline_fixed_notifications) + PipelineUpdateCiRefStatusWorker.perform_async(pipeline.id) + else + PipelineNotificationWorker.perform_async(pipeline.id) + end end end diff --git a/app/models/snippet.rb b/app/models/snippet.rb index a24428dee50..9e7501b7ce6 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -17,6 +17,8 @@ class Snippet < ApplicationRecord include HasRepository extend ::Gitlab::Utils::Override + MAX_FILE_COUNT = 1 + ignore_column :repository_storage, remove_with: '12.10', remove_after: '2020-03-22' cache_markdown_field :title, pipeline: :single_line diff --git a/app/views/shared/notifications/_custom_notifications.html.haml b/app/views/shared/notifications/_custom_notifications.html.haml index 0e1e3beeb1c..58d02602423 100644 --- a/app/views/shared/notifications/_custom_notifications.html.haml +++ b/app/views/shared/notifications/_custom_notifications.html.haml @@ -23,6 +23,7 @@ #{ paragraph.html_safe } .col-lg-8 - notification_setting.email_events.each_with_index do |event, index| + - next if event == :fixed_pipeline && Feature.disabled?(:ci_pipeline_fixed_notifications) - field_id = "#{notifications_menu_identifier("modal", notification_setting)}_notification_setting[#{event}]" .form-group .form-check{ class: ("prepend-top-0" if index == 0) } diff --git a/changelogs/unreleased/205399-add-tooltip-to-file-tree-state.yml b/changelogs/unreleased/205399-add-tooltip-to-file-tree-state.yml new file mode 100644 index 00000000000..82bffbd80ab --- /dev/null +++ b/changelogs/unreleased/205399-add-tooltip-to-file-tree-state.yml @@ -0,0 +1,5 @@ +--- +title: Add tooltip to modification icon in the file tree +merge_request: 27158 +author: +type: other diff --git a/changelogs/unreleased/209277-introduce-a-feature-flag-for-resolve-notifications-for-when-pipeli.yml b/changelogs/unreleased/209277-introduce-a-feature-flag-for-resolve-notifications-for-when-pipeli.yml new file mode 100644 index 00000000000..8ae56e59371 --- /dev/null +++ b/changelogs/unreleased/209277-introduce-a-feature-flag-for-resolve-notifications-for-when-pipeli.yml @@ -0,0 +1,5 @@ +--- +title: Introduce a feature flag for Notifications for when pipelines are fixed +merge_request: 26682 +author: Jacopo Beschi @jacopo-beschi +type: changed diff --git a/doc/administration/high_availability/nfs.md b/doc/administration/high_availability/nfs.md index e8b6c9af879..8e018b7a6fe 100644 --- a/doc/administration/high_availability/nfs.md +++ b/doc/administration/high_availability/nfs.md @@ -54,6 +54,8 @@ management between systems: ### Improving NFS performance with GitLab +#### Improving NFS performance with Unicorn + NOTE: **Note:** From GitLab 12.1, it will automatically be detected if Rugged can and should be used per storage. If you previously enabled Rugged using the feature flag, you will need to unset the feature flag by using: @@ -64,6 +66,16 @@ sudo gitlab-rake gitlab:features:unset_rugged If the Rugged feature flag is explicitly set to either true or false, GitLab will use the value explicitly set. +#### Improving NFS performance with Puma + +NOTE: **Note:** From GitLab 12.7, Rugged auto-detection is disabled if Puma thread count is greater than 1. + +If you want to use Rugged with Puma, it is recommended to [set Puma thread count to 1](https://docs.gitlab.com/omnibus/settings/puma.html#puma-settings). + +If you want to use Rugged with Puma thread count more than 1, Rugged can be enabled using the [feature flag](../../development/gitaly.md#legacy-rugged-code) + +If the Rugged feature flag is explicitly set to either true or false, GitLab will use the value explicitly set. + ### Known issues On some customer systems, we have seen NFS clients slow precipitously due to diff --git a/doc/ci/review_apps/index.md b/doc/ci/review_apps/index.md index e19167f6d9a..860eab469dd 100644 --- a/doc/ci/review_apps/index.md +++ b/doc/ci/review_apps/index.md @@ -188,6 +188,9 @@ With Visual Reviews, you can provide a feedback form to your Review Apps so that reviewers can post comments directly from the app back to the merge request that spawned the Review App. +NOTE: **Note:** Visual Reviews currently only work for public projects. Support for private +and internal projects [is planned](https://gitlab.com/gitlab-org/gitlab/-/issues/42750). + ### Configuring Visual Reviews Ensure that the `anonymous_visual_review_feedback` feature flag is enabled. diff --git a/doc/development/README.md b/doc/development/README.md index b207f208e3d..75dc54050d8 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -181,11 +181,11 @@ Complementary reads: - [Externalization](i18n/externalization.md) - [Translation](i18n/translation.md) -## Event tracking guides +## Telemetry guides -- [Introduction](event_tracking/index.md) -- [Frontend tracking guide](event_tracking/frontend.md) -- [Backend tracking guide](event_tracking/backend.md) +- [Introduction](../telemetry/index.md) +- [Frontend tracking guide](../telemetry/frontend.md) +- [Backend tracking guide](../telemetry/backend.md) ## Experiment Guide diff --git a/doc/development/experiment_guide/index.md b/doc/development/experiment_guide/index.md index b7f497ecd7a..66b7bd8ed0f 100644 --- a/doc/development/experiment_guide/index.md +++ b/doc/development/experiment_guide/index.md @@ -55,7 +55,7 @@ The author then adds a comment to this piece of code and adds a link to the issu end ``` -- Track necessary events. See the [event tracking guide](../event_tracking/index.md) for details. +- Track necessary events. See the [telemetry guide](../../telemetry/index.md) for details. - After the merge request is merged, use [`chatops`](../../ci/chatops/README.md) to enable the feature flag and start the experiment. For visibility, please run the command in the `#s_growth` channel: ``` diff --git a/doc/development/fe_guide/event_tracking.md b/doc/development/fe_guide/event_tracking.md index 13f107eebb1..ae555e99c6b 100644 --- a/doc/development/fe_guide/event_tracking.md +++ b/doc/development/fe_guide/event_tracking.md @@ -1,5 +1,5 @@ --- -redirect_to: '../event_tracking/index.md' +redirect_to: '../../telemetry/index.md' --- -This document was moved to [another location](../event_tracking/frontend.md). +This document was moved to [another location](../../telemetry/index.md). diff --git a/doc/development/instrumentation.md b/doc/development/instrumentation.md index 290a3eac758..55b42bb6ef9 100644 --- a/doc/development/instrumentation.md +++ b/doc/development/instrumentation.md @@ -5,7 +5,7 @@ blocks of Ruby code. Method instrumentation is the primary form of instrumentation with block-based instrumentation only being used when we want to drill down to specific regions of code within a method. -Please refer to [Event tracking](event_tracking/index.md) if you are tracking product usage patterns. +Please refer to [Telemetry](../telemetry/index.md) if you are tracking product usage patterns. ## Instrumenting Methods diff --git a/doc/development/event_tracking/backend.md b/doc/telemetry/backend.md index c571439af8a..c571439af8a 100644 --- a/doc/development/event_tracking/backend.md +++ b/doc/telemetry/backend.md diff --git a/doc/development/event_tracking/frontend.md b/doc/telemetry/frontend.md index fcd394500ec..fcd394500ec 100644 --- a/doc/development/event_tracking/frontend.md +++ b/doc/telemetry/frontend.md diff --git a/doc/development/event_tracking/index.md b/doc/telemetry/index.md index 8c00930a781..55a7fad86be 100644 --- a/doc/development/event_tracking/index.md +++ b/doc/telemetry/index.md @@ -44,7 +44,7 @@ From the backend, the events that are tracked will likely consist of things like See [Backend tracking guide](backend.md). -Also, see [Instrumenting Ruby code](../instrumentation.md) if you are instrumenting application performance metrics for Ruby code. +Also, see [Instrumenting Ruby code](../development/instrumentation.md) if you are instrumenting application performance metrics for Ruby code. ## Enabling tracking diff --git a/doc/user/group/epics/index.md b/doc/user/group/epics/index.md index 861054c2a0a..7712b86bbe2 100644 --- a/doc/user/group/epics/index.md +++ b/doc/user/group/epics/index.md @@ -5,7 +5,7 @@ type: reference, howto # Epics **(PREMIUM)** > - Introduced in [GitLab Ultimate](https://about.gitlab.com/pricing/) 10.2. -> - In [GitLab 12.8](https://gitlab.com/gitlab-org/gitlab/issues/37081), single-level Epics were moved to Premium tier. +> - In [GitLab 12.8](https://gitlab.com/gitlab-org/gitlab/issues/37081), single-level Epics were moved to the Premium tier. Epics let you manage your portfolio of projects more efficiently and with less effort by tracking groups of issues that share a theme, across projects and diff --git a/doc/user/group/roadmap/index.md b/doc/user/group/roadmap/index.md index 6eca9e84ce9..124aa217272 100644 --- a/doc/user/group/roadmap/index.md +++ b/doc/user/group/roadmap/index.md @@ -2,9 +2,10 @@ type: reference --- -# Roadmap **(ULTIMATE)** +# Roadmap **(PREMIUM)** -> Introduced in [GitLab Ultimate](https://about.gitlab.com/pricing/) 10.5. +> - Introduced in [GitLab Ultimate](https://about.gitlab.com/pricing/) 10.5. +> - In [GitLab 12.9](https://gitlab.com/gitlab-org/gitlab/issues/198062), Roadmaps were moved to the Premium tier. An Epic within a group containing **Start date** and/or **Due date** can be visualized in a form of a timeline (e.g. a Gantt chart). The Epics Roadmap page @@ -35,7 +36,8 @@ Roadmaps can also be [visualized inside an epic](../epics/index.md#roadmap-in-ep ## Timeline duration -> Introduced in [GitLab Ultimate](https://about.gitlab.com/pricing/) 11.0. +> - Introduced in [GitLab Ultimate](https://about.gitlab.com/pricing/) 11.0. +> - In [GitLab 12.9](https://gitlab.com/gitlab-org/gitlab/issues/198062), Timelines were moved to the Premium tier. Roadmap supports the following date ranges: diff --git a/doc/user/profile/notifications.md b/doc/user/profile/notifications.md index 2a624283e05..3f843bf8b5e 100644 --- a/doc/user/profile/notifications.md +++ b/doc/user/profile/notifications.md @@ -181,7 +181,7 @@ To minimize the number of notifications that do not require any action, from [Gi | Remove milestone merge request | Subscribers, participants mentioned, and Custom notification level with this event selected | | New comment | The above, plus anyone mentioned by `@username` in the comment, with notification level "Mention" or higher | | Failed pipeline | The author of the pipeline | -| Fixed pipeline | The author of the pipeline | +| Fixed pipeline | The author of the pipeline. Disabled by default. To activate it you must [enable the `ci_pipeline_fixed_notifications` feature flag](../../development/feature_flags/development.md#enabling-a-feature-flag-in-development). | | Successful pipeline | The author of the pipeline, if they have the custom notification setting for successful pipelines set. If the pipeline failed previously, a `Fixed pipeline` message will be sent for the first successful pipeline after the failure, then a `Successful pipeline` message for any further successful pipelines. | | New epic **(ULTIMATE)** | | | Close epic **(ULTIMATE)** | | diff --git a/lib/gitlab/checks/push_file_count_check.rb b/lib/gitlab/checks/push_file_count_check.rb new file mode 100644 index 00000000000..288a7e0d41a --- /dev/null +++ b/lib/gitlab/checks/push_file_count_check.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +module Gitlab + module Checks + class PushFileCountCheck < BaseChecker + attr_reader :repository, :newrev, :limit, :logger + + LOG_MESSAGES = { + diff_content_check: "Validating diff contents being single file..." + }.freeze + + ERROR_MESSAGES = { + upper_limit: "The repository can contain at most %{limit} file(s).", + lower_limit: "The repository must contain at least 1 file." + }.freeze + + def initialize(change, repository:, limit:, logger:) + @repository = repository + @newrev = change[:newrev] + @limit = limit + @logger = logger + end + + def validate! + file_count = repository.ls_files(newrev).size + + if file_count > limit + raise ::Gitlab::GitAccess::ForbiddenError, ERROR_MESSAGES[:upper_limit] % { limit: limit } + end + + if file_count == 0 + raise ::Gitlab::GitAccess::ForbiddenError, ERROR_MESSAGES[:lower_limit] + end + end + end + end +end diff --git a/lib/gitlab/checks/snippet_check.rb b/lib/gitlab/checks/snippet_check.rb index 8bb6c576204..bcecd0fc251 100644 --- a/lib/gitlab/checks/snippet_check.rb +++ b/lib/gitlab/checks/snippet_check.rb @@ -20,14 +20,11 @@ module Gitlab @logger.append_message("Running checks for ref: #{@branch_name || @tag_name}") end - def exec + def validate! if creation? || deletion? raise GitAccess::ForbiddenError, ERROR_MESSAGES[:create_delete_branch] end - # TODO: https://gitlab.com/gitlab-org/gitlab/issues/205628 - # Check operation will not result in more than one file in the repository - true end diff --git a/lib/gitlab/git_access_snippet.rb b/lib/gitlab/git_access_snippet.rb index 5817b17164e..87399f8d07f 100644 --- a/lib/gitlab/git_access_snippet.rb +++ b/lib/gitlab/git_access_snippet.rb @@ -97,9 +97,8 @@ module Gitlab end def check_single_change_access(change) - change_access = Checks::SnippetCheck.new(change, logger: logger) - - change_access.exec + Checks::SnippetCheck.new(change, logger: logger).validate! + Checks::PushFileCountCheck.new(change, repository: repository, limit: Snippet::MAX_FILE_COUNT, logger: logger).validate! rescue Checks::TimedLogger::TimeoutError raise TimeoutError, logger.full_message end diff --git a/qa/qa/runtime/browser.rb b/qa/qa/runtime/browser.rb index cb475dd4973..7777d06b6f5 100644 --- a/qa/qa/runtime/browser.rb +++ b/qa/qa/runtime/browser.rb @@ -44,11 +44,21 @@ module QA new.visit(address, page_class, &block) end + # rubocop: disable Metrics/AbcSize def self.configure! RSpec.configure do |config| config.define_derived_metadata(file_path: %r{/qa/specs/features/}) do |metadata| metadata[:type] = :feature end + + config.around(:each) do |example| + example.run + + if example.metadata[:screenshot] + screenshot = example.metadata[:screenshot][:image] || example.metadata[:screenshot][:html] + example.metadata[:stdout] = %{[[ATTACHMENT|#{screenshot}]]} + end + end end Capybara.server_port = 9887 + ENV['TEST_ENV_NUMBER'].to_i @@ -140,6 +150,7 @@ module QA config.default_normalize_ws = true end end + # rubocop: enable Metrics/AbcSize class Session include Capybara::DSL diff --git a/spec/features/projects/show/user_manages_notifications_spec.rb b/spec/features/projects/show/user_manages_notifications_spec.rb index 851a09cf28a..0cd6743304e 100644 --- a/spec/features/projects/show/user_manages_notifications_spec.rb +++ b/spec/features/projects/show/user_manages_notifications_spec.rb @@ -7,7 +7,6 @@ describe 'Projects > Show > User manages notifications', :js do before do sign_in(project.owner) - visit project_path(project) end def click_notifications_button @@ -15,6 +14,7 @@ describe 'Projects > Show > User manages notifications', :js do end it 'changes the notification setting' do + visit project_path(project) click_notifications_button click_link 'On mention' @@ -26,6 +26,7 @@ describe 'Projects > Show > User manages notifications', :js do end it 'changes the notification setting to disabled' do + visit project_path(project) click_notifications_button click_link 'Disabled' @@ -50,11 +51,13 @@ describe 'Projects > Show > User manages notifications', :js do :reassign_merge_request, :merge_merge_request, :failed_pipeline, + :fixed_pipeline, :success_pipeline ] end it 'shows notification settings checkbox' do + visit project_path(project) click_notifications_button page.find('a[data-notification-level="custom"]').click @@ -64,12 +67,27 @@ describe 'Projects > Show > User manages notifications', :js do end end end + + context 'when ci_pipeline_fixed_notifications is disabled' do + before do + stub_feature_flags(ci_pipeline_fixed_notifications: false) + end + + it 'hides fixed_pipeline checkbox' do + visit project_path(project) + click_notifications_button + page.find('a[data-notification-level="custom"]').click + + expect(page).not_to have_selector("input[name='notification_setting[fixed_pipeline]']") + end + end end context 'when project emails are disabled' do let(:project) { create(:project, :public, :repository, emails_disabled: true) } it 'is disabled' do + visit project_path(project) expect(page).to have_selector('.notifications-btn.disabled', visible: true) end end diff --git a/spec/frontend/diffs/components/diff_file_row_spec.js b/spec/frontend/diffs/components/diff_file_row_spec.js index 9b7a16d0cb5..856622b89cb 100644 --- a/spec/frontend/diffs/components/diff_file_row_spec.js +++ b/spec/frontend/diffs/components/diff_file_row_spec.js @@ -44,12 +44,14 @@ describe('Diff File Row component', () => { level: 4, file: {}, hideFileStats: false, + showTooltip: true, }); expect(wrapper.find(ChangedFileIcon).props()).toEqual( expect.objectContaining({ file: {}, size: 16, + showTooltip: true, }), ); }); diff --git a/spec/frontend/vue_shared/components/changed_file_icon_spec.js b/spec/frontend/vue_shared/components/changed_file_icon_spec.js index b77116be464..03519a6f803 100644 --- a/spec/frontend/vue_shared/components/changed_file_icon_spec.js +++ b/spec/frontend/vue_shared/components/changed_file_icon_spec.js @@ -5,6 +5,7 @@ import Icon from '~/vue_shared/components/icon.vue'; const changedFile = () => ({ changed: true }); const stagedFile = () => ({ changed: true, staged: true }); const newFile = () => ({ changed: true, tempFile: true }); +const deletedFile = () => ({ changed: false, tempFile: false, staged: false, deleted: true }); const unchangedFile = () => ({ changed: false, tempFile: false, staged: false, deleted: false }); describe('Changed file icon', () => { @@ -58,6 +59,7 @@ describe('Changed file icon', () => { ${changedFile()} | ${'file-modified'} | ${'Modified'} | ${'with file changed'} ${stagedFile()} | ${'file-modified-solid'} | ${'Modified'} | ${'with file staged'} ${newFile()} | ${'file-addition'} | ${'Added'} | ${'with file new'} + ${deletedFile()} | ${'file-deletion'} | ${'Deleted'} | ${'with file deleted'} `('$desc', ({ file, iconName, tooltipText }) => { beforeEach(() => { factory({ file }); diff --git a/spec/lib/gitlab/checks/push_file_count_check_spec.rb b/spec/lib/gitlab/checks/push_file_count_check_spec.rb new file mode 100644 index 00000000000..58ba7d579a3 --- /dev/null +++ b/spec/lib/gitlab/checks/push_file_count_check_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Checks::PushFileCountCheck do + let(:snippet) { create(:personal_snippet, :repository) } + let(:changes) { { oldrev: oldrev, newrev: newrev, ref: ref } } + let(:timeout) { Gitlab::GitAccess::INTERNAL_TIMEOUT } + let(:logger) { Gitlab::Checks::TimedLogger.new(timeout: timeout) } + + subject { described_class.new(changes, repository: snippet.repository, limit: 1, logger: logger) } + + describe '#validate!' do + using RSpec::Parameterized::TableSyntax + + before do + allow(snippet.repository).to receive(:new_commits).and_return( + snippet.repository.commits_between(oldrev, newrev) + ) + end + + context 'initial creation' do + let(:oldrev) { Gitlab::Git::EMPTY_TREE_ID } + let(:newrev) { TestEnv::BRANCH_SHA["snippet/single-file"] } + let(:ref) { "refs/heads/snippet/single-file" } + + it 'allows creation' do + expect { subject.validate! }.not_to raise_error + end + end + + where(:old, :new, :valid, :message) do + 'single-file' | 'edit-file' | true | nil + 'single-file' | 'multiple-files' | false | 'The repository can contain at most 1 file(s).' + 'single-file' | 'no-files' | false | 'The repository must contain at least 1 file.' + 'edit-file' | 'rename-and-edit-file' | true | nil + end + + with_them do + let(:oldrev) { TestEnv::BRANCH_SHA["snippet/#{old}"] } + let(:newrev) { TestEnv::BRANCH_SHA["snippet/#{new}"] } + let(:ref) { "refs/heads/snippet/#{new}" } + + it "verifies" do + if valid + expect { subject.validate! }.not_to raise_error + else + expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, message) + end + end + end + end +end diff --git a/spec/lib/gitlab/checks/snippet_check_spec.rb b/spec/lib/gitlab/checks/snippet_check_spec.rb index 43c69ab7042..3eee5ccfc0a 100644 --- a/spec/lib/gitlab/checks/snippet_check_spec.rb +++ b/spec/lib/gitlab/checks/snippet_check_spec.rb @@ -10,16 +10,16 @@ describe Gitlab::Checks::SnippetCheck do subject { Gitlab::Checks::SnippetCheck.new(changes, logger: logger) } - describe '#exec' do + describe '#validate!' do it 'does not raise any error' do - expect { subject.exec }.not_to raise_error + expect { subject.validate! }.not_to raise_error end context 'trying to delete the branch' do let(:newrev) { '0000000000000000000000000000000000000000' } it 'raises an error' do - expect { subject.exec }.to raise_error(Gitlab::GitAccess::ForbiddenError, 'You can not create or delete branches.') + expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, 'You can not create or delete branches.') end end @@ -28,14 +28,14 @@ describe Gitlab::Checks::SnippetCheck do let(:ref) { 'refs/heads/feature' } it 'raises an error' do - expect { subject.exec }.to raise_error(Gitlab::GitAccess::ForbiddenError, 'You can not create or delete branches.') + expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, 'You can not create or delete branches.') end context "when branch is 'master'" do let(:ref) { 'refs/heads/master' } it "allows the operation" do - expect { subject.exec }.not_to raise_error + expect { subject.validate! }.not_to raise_error end end end diff --git a/spec/lib/gitlab/git_access_snippet_spec.rb b/spec/lib/gitlab/git_access_snippet_spec.rb index ba7b7da7e7d..f52fe8ef612 100644 --- a/spec/lib/gitlab/git_access_snippet_spec.rb +++ b/spec/lib/gitlab/git_access_snippet_spec.rb @@ -188,12 +188,15 @@ describe Gitlab::GitAccessSnippet do end context 'when changes are specific' do - let(:changes) { 'oldrev newrev ref' } + let(:changes) { "2d1db523e11e777e49377cfb22d368deec3f0793 ddd0f15ae83993f5cb66a927a28673882e99100b master" } let(:user) { snippet.author } it 'does not raise error if SnippetCheck does not raise error' do expect_next_instance_of(Gitlab::Checks::SnippetCheck) do |check| - expect(check).to receive(:exec).and_call_original + expect(check).to receive(:validate!).and_call_original + end + expect_next_instance_of(Gitlab::Checks::PushFileCountCheck) do |check| + expect(check).to receive(:validate!) end expect { push_access_check }.not_to raise_error @@ -201,7 +204,7 @@ describe Gitlab::GitAccessSnippet do it 'raises error if SnippetCheck raises error' do expect_next_instance_of(Gitlab::Checks::SnippetCheck) do |check| - allow(check).to receive(:exec).and_raise(Gitlab::GitAccess::ForbiddenError, 'foo') + allow(check).to receive(:validate!).and_raise(Gitlab::GitAccess::ForbiddenError, 'foo') end expect { push_access_check }.to raise_forbidden('foo') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index f775906a545..51a2e2aff67 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -2509,27 +2509,53 @@ describe Ci::Pipeline, :mailer do end end - context 'with success pipeline' do - before do - perform_enqueued_jobs do + shared_examples 'enqueues the notification worker' do + it 'enqueues PipelineUpdateCiRefStatusWorker' do + expect(PipelineUpdateCiRefStatusWorker).to receive(:perform_async).with(pipeline.id) + expect(PipelineNotificationWorker).not_to receive(:perform_async).with(pipeline.id) + + pipeline.succeed + end + + context 'when ci_pipeline_fixed_notifications is disabled' do + before do + stub_feature_flags(ci_pipeline_fixed_notifications: false) + end + + it 'enqueues PipelineNotificationWorker' do + expect(PipelineUpdateCiRefStatusWorker).not_to receive(:perform_async).with(pipeline.id) + expect(PipelineNotificationWorker).to receive(:perform_async).with(pipeline.id) + pipeline.succeed end end + end - it_behaves_like 'sending a notification' + context 'with success pipeline' do + it_behaves_like 'sending a notification' do + before do + perform_enqueued_jobs do + pipeline.succeed + end + end + end + + it_behaves_like 'enqueues the notification worker' end context 'with failed pipeline' do - before do - perform_enqueued_jobs do - create(:ci_build, :failed, pipeline: pipeline) - create(:generic_commit_status, :failed, pipeline: pipeline) + it_behaves_like 'sending a notification' do + before do + perform_enqueued_jobs do + create(:ci_build, :failed, pipeline: pipeline) + create(:generic_commit_status, :failed, pipeline: pipeline) - pipeline.drop + pipeline.drop + end end end - it_behaves_like 'sending a notification' + it_behaves_like 'enqueues the notification worker' end context 'with skipped pipeline' do diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb index c3e52975d6d..426e15faaa6 100644 --- a/spec/requests/api/internal/base_spec.rb +++ b/spec/requests/api/internal/base_spec.rb @@ -12,6 +12,7 @@ describe API::Internal::Base do let_it_be(:personal_snippet) { create(:personal_snippet, :repository, author: user) } let_it_be(:project_snippet) { create(:project_snippet, :repository, author: user, project: project) } + let(:snippet_changes) { "#{TestEnv::BRANCH_SHA['snippet/single-file']} #{TestEnv::BRANCH_SHA['snippet/edit-file']} refs/heads/snippet/edit-file" } describe "GET /internal/check" do it do @@ -336,7 +337,7 @@ describe API::Internal::Base do end context 'git push with personal snippet' do - subject { push(key, personal_snippet, env: env.to_json) } + subject { push(key, personal_snippet, env: env.to_json, changes: snippet_changes) } it 'responds with success' do subject @@ -371,7 +372,7 @@ describe API::Internal::Base do end context 'git push with project snippet' do - subject { push(key, project_snippet, env: env.to_json) } + subject { push(key, project_snippet, env: env.to_json, changes: snippet_changes) } it 'responds with success' do subject @@ -1104,9 +1105,11 @@ describe API::Internal::Base do ) end - def push(key, container, protocol = 'ssh', env: nil) + def push(key, container, protocol = 'ssh', env: nil, changes: nil) + changes ||= 'd14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master' + params = { - changes: 'd14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master', + changes: changes, key_id: key.id, project: full_path_for(container), gl_repository: gl_repository_for(container), diff --git a/spec/support/capybara.rb b/spec/support/capybara.rb index 5d8779ec782..90adfb1a2ee 100644 --- a/spec/support/capybara.rb +++ b/spec/support/capybara.rb @@ -126,6 +126,12 @@ RSpec.configure do |config| Capybara.raise_server_errors = false example.run + + if example.metadata[:screenshot] + screenshot = example.metadata[:screenshot][:image] || example.metadata[:screenshot][:html] + example.metadata[:stdout] = %{[[ATTACHMENT|#{screenshot}]]} + end + ensure Capybara.raise_server_errors = true end diff --git a/spec/support/helpers/test_env.rb b/spec/support/helpers/test_env.rb index 613535b6da5..66c2faac2dd 100644 --- a/spec/support/helpers/test_env.rb +++ b/spec/support/helpers/test_env.rb @@ -61,6 +61,11 @@ module TestEnv 'merge-commit-analyze-before' => '1adbdef', 'merge-commit-analyze-side-branch' => '8a99451', 'merge-commit-analyze-after' => '646ece5', + 'snippet/single-file' => '43e4080', + 'snippet/multiple-files' => 'b80faa8', + 'snippet/rename-and-edit-file' => '220a1e4', + 'snippet/edit-file' => 'c2f074f', + 'snippet/no-files' => '671aaa8', '2-mb-file' => 'bf12d25', 'before-create-delete-modify-move' => '845009f', 'between-create-delete-modify-move' => '3f5f443', |