diff options
31 files changed, 879 insertions, 1287 deletions
diff --git a/changelogs/unreleased/46950-systemcheck-ruby-version.yml b/changelogs/unreleased/46950-systemcheck-ruby-version.yml new file mode 100644 index 00000000000..e556e14223b --- /dev/null +++ b/changelogs/unreleased/46950-systemcheck-ruby-version.yml @@ -0,0 +1,5 @@ +--- +title: 'SystemCheck: Use a more reliable way to detect current Ruby version' +merge_request: 23291 +author: +type: changed diff --git a/changelogs/unreleased/mr-origin-23218.yml b/changelogs/unreleased/mr-origin-23218.yml new file mode 100644 index 00000000000..49867f04343 --- /dev/null +++ b/changelogs/unreleased/mr-origin-23218.yml @@ -0,0 +1,5 @@ +--- +title: Fix typo for scheduled pipeline +merge_request: 23218 +author: Davy Defaud +type: other diff --git a/changelogs/unreleased/zj-improve-gitaly-pb.yml b/changelogs/unreleased/zj-improve-gitaly-pb.yml new file mode 100644 index 00000000000..506a0303d8a --- /dev/null +++ b/changelogs/unreleased/zj-improve-gitaly-pb.yml @@ -0,0 +1,5 @@ +--- +title: Show what RPC is called in the performance bar +merge_request: 23140 +author: +type: other diff --git a/doc/api/issues.md b/doc/api/issues.md index 0dc9d706120..0e97d9ea6f5 100644 --- a/doc/api/issues.md +++ b/doc/api/issues.md @@ -658,7 +658,7 @@ POST /projects/:id/issues/:issue_iid/move | `to_project_id` | integer | yes | The ID of the new project | ```bash -curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" --data '{"to_project_id": 5}' https://gitlab.example.com/api/v4/projects/4/issues/85/move +curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" --form to_project_id=5 https://gitlab.example.com/api/v4/projects/4/issues/85/move ``` Example response: diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 66743ea0e47..02f198dbd67 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -1634,6 +1634,10 @@ rspec: - bundle exec rspec ``` +NOTE: **Note:** +`include` requires the external YAML files to have the extensions `.yml` or `.yaml`. +The external file will not be included if the extension is missing. + You can define it either as a single string, or, in case you want to include more than one files, an array of different values . The following examples are both valid cases: diff --git a/doc/user/project/integrations/discord_notifications.md b/doc/user/project/integrations/discord_notifications.md index e157f5cc106..cb98105e0c0 100644 --- a/doc/user/project/integrations/discord_notifications.md +++ b/doc/user/project/integrations/discord_notifications.md @@ -1,6 +1,6 @@ # Discord Notifications service -> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/22684) in GitLab 11.5. +> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/22684) in GitLab 11.6. The Discord Notifications service sends event notifications from GitLab to the channel for which the webhook was created. diff --git a/lib/gitlab/ci/status/build/scheduled.rb b/lib/gitlab/ci/status/build/scheduled.rb index b3452eae189..0a09dbe5f42 100644 --- a/lib/gitlab/ci/status/build/scheduled.rb +++ b/lib/gitlab/ci/status/build/scheduled.rb @@ -10,7 +10,7 @@ module Gitlab image: 'illustrations/illustrations_scheduled-job_countdown.svg', size: 'svg-394', title: _("This is a delayed job to run in %{remainingTime}"), - content: _("This job will automatically run after it's timer finishes. " \ + content: _("This job will automatically run after its timer finishes. " \ "Often they are used for incremental roll-out deploys " \ "to production environments. When unscheduled it converts " \ "into a manual action.") diff --git a/lib/gitlab/git/blob.rb b/lib/gitlab/git/blob.rb index 9dd1c484d59..2d25389594e 100644 --- a/lib/gitlab/git/blob.rb +++ b/lib/gitlab/git/blob.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -# Gitaly note: JV: seems to be completely migrated (behind feature flags). - module Gitlab module Git class Blob diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index e75f4ec1ec5..067f4026f48 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -885,12 +885,6 @@ module Gitlab Gitlab::GitalyClient::ConflictsService.new(self, our_commit_oid, their_commit_oid) end - def gitaly_migrate(method, status: Gitlab::GitalyClient::MigrationStatus::OPT_IN, &block) - wrapped_gitaly_errors do - Gitlab::GitalyClient.migrate(method, status: status, &block) - end - end - def clean_stale_repository_files wrapped_gitaly_errors do gitaly_repository_client.cleanup if exists? diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index 8b455dc7696..9be553a8b86 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -9,11 +9,6 @@ require 'grpc/health/v1/health_services_pb' module Gitlab module GitalyClient include Gitlab::Metrics::Methods - module MigrationStatus - DISABLED = 1 - OPT_IN = 2 - OPT_OUT = 3 - end class TooManyInvocationsError < StandardError attr_reader :call_site, :invocation_count, :max_call_stack @@ -31,7 +26,7 @@ module Gitlab end end - SERVER_VERSION_FILE = 'GITALY_SERVER_VERSION'.freeze + SERVER_VERSION_FILE = 'GITALY_SERVER_VERSION' MAXIMUM_GITALY_CALLS = 35 CLIENT_NAME = (Sidekiq.server? ? 'gitlab-sidekiq' : 'gitlab-web').freeze @@ -43,11 +38,6 @@ module Gitlab self.query_time = 0 - define_histogram :gitaly_migrate_call_duration_seconds do - docstring "Gitaly migration call execution timings" - base_labels gitaly_enabled: nil, feature: nil - end - define_histogram :gitaly_controller_action_duration_seconds do docstring "Gitaly endpoint histogram by controller and action combination" base_labels Gitlab::Metrics::Transaction::BASE_LABELS.merge(gitaly_service: nil, rpc: nil) @@ -126,7 +116,6 @@ module Gitlab def self.call(storage, service, rpc, request, remote_storage: nil, timeout: nil) start = Gitlab::Metrics::System.monotonic_time request_hash = request.is_a?(Google::Protobuf::MessageExts) ? request.to_h : {} - @current_call_id ||= SecureRandom.uuid enforce_gitaly_request_limits(:call) @@ -145,9 +134,7 @@ module Gitlab current_transaction_labels.merge(gitaly_service: service.to_s, rpc: rpc.to_s), duration) - add_call_details(id: @current_call_id, feature: service, duration: duration, request: request_hash) - - @current_call_id = nil + add_call_details(feature: "#{service}##{rpc}", duration: duration, request: request_hash, rpc: rpc) end def self.handle_grpc_unavailable!(ex) @@ -222,7 +209,7 @@ module Gitlab result end - SERVER_FEATURE_FLAGS = %w[gogit_findcommit].freeze + SERVER_FEATURE_FLAGS = %w[].freeze def self.server_feature_flags SERVER_FEATURE_FLAGS.map do |f| @@ -237,82 +224,8 @@ module Gitlab params['gitaly_token'].presence || Gitlab.config.gitaly['token'] end - # Evaluates whether a feature toggle is on or off - def self.feature_enabled?(feature_name, status: MigrationStatus::OPT_IN) - # Disabled features are always off! - return false if status == MigrationStatus::DISABLED - - feature = Feature.get("gitaly_#{feature_name}") - - # If the feature has been set, always evaluate - if Feature.persisted?(feature) - if feature.percentage_of_time_value > 0 - # Probabilistically enable this feature - return Random.rand() * 100 < feature.percentage_of_time_value - end - - return feature.enabled? - end - - # If the feature has not been set, the default depends - # on it's status - case status - when MigrationStatus::OPT_OUT - true - when MigrationStatus::OPT_IN - opt_into_all_features? && !explicit_opt_in_required.include?(feature_name) - else - false - end - rescue => ex - # During application startup feature lookups in SQL can fail - Rails.logger.warn "exception while checking Gitaly feature status for #{feature_name}: #{ex}" - false - end - - # We have a mechanism to let GitLab automatically opt in to all Gitaly - # features. We want to be able to exclude some features from automatic - # opt-in. This function has an override in EE. - def self.explicit_opt_in_required - [] - end - - # opt_into_all_features? returns true when the current environment - # is one in which we opt into features automatically - def self.opt_into_all_features? - Rails.env.development? || ENV["GITALY_FEATURE_DEFAULT_ON"] == "1" - end - private_class_method :opt_into_all_features? - - def self.migrate(feature, status: MigrationStatus::OPT_IN) - # Enforce limits at both the `migrate` and `call` sites to ensure that - # problems are not hidden by a feature being disabled - enforce_gitaly_request_limits(:migrate) - - is_enabled = feature_enabled?(feature, status: status) - metric_name = feature.to_s - metric_name += "_gitaly" if is_enabled - - Gitlab::Metrics.measure(metric_name) do - # Some migrate calls wrap other migrate calls - allow_n_plus_1_calls do - feature_stack = Thread.current[:gitaly_feature_stack] ||= [] - feature_stack.unshift(feature) - begin - start = Gitlab::Metrics::System.monotonic_time - @current_call_id = SecureRandom.uuid - call_details = { id: @current_call_id } - yield is_enabled - ensure - total_time = Gitlab::Metrics::System.monotonic_time - start - gitaly_migrate_call_duration_seconds.observe({ gitaly_enabled: is_enabled, feature: feature }, total_time) - feature_stack.shift - Thread.current[:gitaly_feature_stack] = nil if feature_stack.empty? - - add_call_details(call_details.merge(feature: feature, duration: total_time)) - end - end - end + def self.feature_enabled?(feature_name) + Feature.enabled?("gitaly_#{feature_name}") end # Ensures that Gitaly is not being abuse through n+1 misuse etc @@ -368,48 +281,29 @@ module Gitlab end private_class_method :decrement_call_count - # Returns an estimate of the number of Gitaly calls made for this - # request + # Returns the of the number of Gitaly calls made for this request def self.get_request_count - return 0 unless Gitlab::SafeRequestStore.active? - - gitaly_migrate_count = get_call_count("gitaly_migrate_actual") - gitaly_call_count = get_call_count("gitaly_call_actual") - - # Using the maximum of migrate and call_count will provide an - # indicator of how many Gitaly calls will be made, even - # before a feature is enabled. This provides us with a single - # metric, but not an exact number, but this tradeoff is acceptable - if gitaly_migrate_count > gitaly_call_count - gitaly_migrate_count - else - gitaly_call_count - end + get_call_count("gitaly_call_actual") end def self.reset_counts return unless Gitlab::SafeRequestStore.active? - %w[migrate call].each do |call_site| - Gitlab::SafeRequestStore["gitaly_#{call_site}_actual"] = 0 - Gitlab::SafeRequestStore["gitaly_#{call_site}_permitted"] = 0 - end + Gitlab::SafeRequestStore["gitaly_call_actual"] = 0 + Gitlab::SafeRequestStore["gitaly_call_permitted"] = 0 end def self.add_call_details(details) - id = details.delete(:id) - - return unless id && Gitlab::SafeRequestStore[:peek_enabled] + return unless Gitlab::SafeRequestStore[:peek_enabled] - Gitlab::SafeRequestStore['gitaly_call_details'] ||= {} - Gitlab::SafeRequestStore['gitaly_call_details'][id] ||= {} - Gitlab::SafeRequestStore['gitaly_call_details'][id].merge!(details) + Gitlab::SafeRequestStore['gitaly_call_details'] ||= [] + Gitlab::SafeRequestStore['gitaly_call_details'] << details end def self.list_call_details - return {} unless Gitlab::SafeRequestStore[:peek_enabled] + return [] unless Gitlab::SafeRequestStore[:peek_enabled] - Gitlab::SafeRequestStore['gitaly_call_details'] || {} + Gitlab::SafeRequestStore['gitaly_call_details'] || [] end def self.expected_server_version diff --git a/lib/peek/views/gitaly.rb b/lib/peek/views/gitaly.rb index 860963ef94f..30f95a10024 100644 --- a/lib/peek/views/gitaly.rb +++ b/lib/peek/views/gitaly.rb @@ -23,7 +23,6 @@ module Peek def details ::Gitlab::GitalyClient.list_call_details - .values .sort { |a, b| b[:duration] <=> a[:duration] } .map(&method(:format_call_details)) end diff --git a/lib/system_check/app/ruby_version_check.rb b/lib/system_check/app/ruby_version_check.rb index d73c39f2c3f..60e07718338 100644 --- a/lib/system_check/app/ruby_version_check.rb +++ b/lib/system_check/app/ruby_version_check.rb @@ -11,7 +11,7 @@ module SystemCheck end def self.current_version - @current_version ||= Gitlab::VersionInfo.parse(Gitlab::TaskHelpers.run_command(%w(ruby --version))) + @current_version ||= Gitlab::VersionInfo.parse(RUBY_VERSION) end def check? diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 15d18e6691f..7af36b049d8 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -6446,7 +6446,7 @@ msgstr "" msgid "This job requires a manual action" msgstr "" -msgid "This job will automatically run after it's timer finishes. Often they are used for incremental roll-out deploys to production environments. When unscheduled it converts into a manual action." +msgid "This job will automatically run after its timer finishes. Often they are used for incremental roll-out deploys to production environments. When unscheduled it converts into a manual action." msgstr "" msgid "This means you can not push code until you create an empty repository or import existing one." diff --git a/rubocop/cop/safe_params.rb b/rubocop/cop/safe_params.rb new file mode 100644 index 00000000000..250c16232e4 --- /dev/null +++ b/rubocop/cop/safe_params.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + class SafeParams < RuboCop::Cop::Cop + MSG = 'Use `safe_params` instead of `params` in url_for.'.freeze + + METHOD_NAME_PATTERN = :url_for + UNSAFE_PARAM = :params + + def on_send(node) + return unless method_name(node) == METHOD_NAME_PATTERN + + add_offense(node, location: :expression) unless safe_params?(node) + end + + private + + def safe_params?(node) + node.descendants.each do |param_node| + next unless param_node.descendants.empty? + + return false if method_name(param_node) == UNSAFE_PARAM + end + + true + end + + def method_name(node) + node.children[1] + end + end + end +end diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index 6c9b8753c1a..4489159f422 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -5,6 +5,7 @@ require_relative 'cop/gitlab/httparty' require_relative 'cop/gitlab/finder_with_find_by' require_relative 'cop/gitlab/union' require_relative 'cop/include_sidekiq_worker' +require_relative 'cop/safe_params' require_relative 'cop/avoid_return_from_blocks' require_relative 'cop/avoid_break_from_strong_memoize' require_relative 'cop/avoid_route_redirect_leading_slash' diff --git a/spec/features/projects/jobs_spec.rb b/spec/features/projects/jobs_spec.rb index 99a7fbb63bd..24352be592a 100644 --- a/spec/features/projects/jobs_spec.rb +++ b/spec/features/projects/jobs_spec.rb @@ -596,7 +596,7 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do it 'shows delayed job', :js do expect(page).to have_content('This is a delayed job to run in') - expect(page).to have_content("This job will automatically run after it's timer finishes.") + expect(page).to have_content("This job will automatically run after its timer finishes.") expect(page).to have_link('Unschedule job') end diff --git a/spec/features/projects/wiki/user_updates_wiki_page_spec.rb b/spec/features/projects/wiki/user_updates_wiki_page_spec.rb index 2ce5ee0e87d..7ad7fec922a 100644 --- a/spec/features/projects/wiki/user_updates_wiki_page_spec.rb +++ b/spec/features/projects/wiki/user_updates_wiki_page_spec.rb @@ -1,233 +1,223 @@ require 'spec_helper' describe 'User updates wiki page' do - shared_examples 'wiki page user update' do - let(:user) { create(:user) } + let(:user) { create(:user) } + before do + project.add_maintainer(user) + sign_in(user) + end + + context 'when wiki is empty' do before do - project.add_maintainer(user) - sign_in(user) + visit(project_wikis_path(project)) + click_link "Create your first page" end - context 'when wiki is empty' do - before do - visit(project_wikis_path(project)) - click_link "Create your first page" - end - - context 'in a user namespace' do - let(:project) { create(:project, :wiki_repo, namespace: user.namespace) } + context 'in a user namespace' do + let(:project) { create(:project, :wiki_repo) } - it 'redirects back to the home edit page' do - page.within(:css, '.wiki-form .form-actions') do - click_on('Cancel') - end - - expect(current_path).to eq project_wiki_path(project, :home) + it 'redirects back to the home edit page' do + page.within(:css, '.wiki-form .form-actions') do + click_on('Cancel') end - it 'updates a page that has a path', :js do - click_on('New page') + expect(current_path).to eq project_wiki_path(project, :home) + end - page.within('#modal-new-wiki') do - fill_in(:new_wiki_path, with: 'one/two/three-test') - click_on('Create page') - end + it 'updates a page that has a path', :js do + click_on('New page') - page.within '.wiki-form' do - fill_in(:wiki_content, with: 'wiki content') - click_on('Create page') - end + page.within('#modal-new-wiki') do + fill_in(:new_wiki_path, with: 'one/two/three-test') + click_on('Create page') + end - expect(current_path).to include('one/two/three-test') - expect(find('.wiki-pages')).to have_content('Three') + page.within '.wiki-form' do + fill_in(:wiki_content, with: 'wiki content') + click_on('Create page') + end - first(:link, text: 'Three').click + expect(current_path).to include('one/two/three-test') + expect(find('.wiki-pages')).to have_content('Three') - expect(find('.nav-text')).to have_content('Three') + first(:link, text: 'Three').click - click_on('Edit') + expect(find('.nav-text')).to have_content('Three') - expect(current_path).to include('one/two/three-test') - expect(page).to have_content('Edit Page') + click_on('Edit') - fill_in('Content', with: 'Updated Wiki Content') - click_on('Save changes') + expect(current_path).to include('one/two/three-test') + expect(page).to have_content('Edit Page') - expect(page).to have_content('Updated Wiki Content') - end + fill_in('Content', with: 'Updated Wiki Content') + click_on('Save changes') - it_behaves_like 'wiki file attachments' + expect(page).to have_content('Updated Wiki Content') end - end - context 'when wiki is not empty' do - let(:project_wiki) { create(:project_wiki, project: project, user: project.creator) } - let!(:wiki_page) { create(:wiki_page, wiki: project_wiki, attrs: { title: 'home', content: 'Home page' }) } + it_behaves_like 'wiki file attachments' + end + end - before do - visit(project_wikis_path(project)) + context 'when wiki is not empty' do + let(:project_wiki) { create(:project_wiki, project: project, user: project.creator) } + let!(:wiki_page) { create(:wiki_page, wiki: project_wiki, attrs: { title: 'home', content: 'Home page' }) } - click_link('Edit') - end + before do + visit(project_wikis_path(project)) - context 'in a user namespace' do - let(:project) { create(:project, :wiki_repo, namespace: user.namespace) } + click_link('Edit') + end - it 'updates a page' do - # Commit message field should have correct value. - expect(page).to have_field('wiki[message]', with: 'Update home') + context 'in a user namespace' do + let(:project) { create(:project, :wiki_repo) } - fill_in(:wiki_content, with: 'My awesome wiki!') - click_button('Save changes') + it 'updates a page' do + # Commit message field should have correct value. + expect(page).to have_field('wiki[message]', with: 'Update home') - expect(page).to have_content('Home') - expect(page).to have_content("Last edited by #{user.name}") - expect(page).to have_content('My awesome wiki!') - end + fill_in(:wiki_content, with: 'My awesome wiki!') + click_button('Save changes') - it 'shows a validation error message' do - fill_in(:wiki_content, with: '') - click_button('Save changes') + expect(page).to have_content('Home') + expect(page).to have_content("Last edited by #{user.name}") + expect(page).to have_content('My awesome wiki!') + end - expect(page).to have_selector('.wiki-form') - expect(page).to have_content('Edit Page') - expect(page).to have_content('The form contains the following error:') - expect(page).to have_content("Content can't be blank") - expect(find('textarea#wiki_content').value).to eq('') - end + it 'shows a validation error message' do + fill_in(:wiki_content, with: '') + click_button('Save changes') - it 'shows the emoji autocompletion dropdown', :js do - find('#wiki_content').native.send_keys('') - fill_in(:wiki_content, with: ':') + expect(page).to have_selector('.wiki-form') + expect(page).to have_content('Edit Page') + expect(page).to have_content('The form contains the following error:') + expect(page).to have_content("Content can't be blank") + expect(find('textarea#wiki_content').value).to eq('') + end - expect(page).to have_selector('.atwho-view') - end + it 'shows the emoji autocompletion dropdown', :js do + find('#wiki_content').native.send_keys('') + fill_in(:wiki_content, with: ':') - it 'shows the error message' do - wiki_page.update(content: 'Update') + expect(page).to have_selector('.atwho-view') + end - click_button('Save changes') + it 'shows the error message' do + wiki_page.update(content: 'Update') - expect(page).to have_content('Someone edited the page the same time you did.') - end + click_button('Save changes') - it 'updates a page' do - fill_in('Content', with: 'Updated Wiki Content') - click_on('Save changes') + expect(page).to have_content('Someone edited the page the same time you did.') + end - expect(page).to have_content('Updated Wiki Content') - end + it 'updates a page' do + fill_in('Content', with: 'Updated Wiki Content') + click_on('Save changes') - it 'cancels editing of a page' do - page.within(:css, '.wiki-form .form-actions') do - click_on('Cancel') - end + expect(page).to have_content('Updated Wiki Content') + end - expect(current_path).to eq(project_wiki_path(project, wiki_page)) + it 'cancels editing of a page' do + page.within(:css, '.wiki-form .form-actions') do + click_on('Cancel') end - it_behaves_like 'wiki file attachments' + expect(current_path).to eq(project_wiki_path(project, wiki_page)) end - context 'in a group namespace' do - let(:project) { create(:project, :wiki_repo, namespace: create(:group, :public)) } + it_behaves_like 'wiki file attachments' + end - it 'updates a page' do - # Commit message field should have correct value. - expect(page).to have_field('wiki[message]', with: 'Update home') + context 'in a group namespace' do + let(:project) { create(:project, :wiki_repo, namespace: create(:group, :public)) } - fill_in(:wiki_content, with: 'My awesome wiki!') + it 'updates a page' do + # Commit message field should have correct value. + expect(page).to have_field('wiki[message]', with: 'Update home') - click_button('Save changes') + fill_in(:wiki_content, with: 'My awesome wiki!') - expect(page).to have_content('Home') - expect(page).to have_content("Last edited by #{user.name}") - expect(page).to have_content('My awesome wiki!') - end + click_button('Save changes') - it_behaves_like 'wiki file attachments' + expect(page).to have_content('Home') + expect(page).to have_content("Last edited by #{user.name}") + expect(page).to have_content('My awesome wiki!') end - end - context 'when the page is in a subdir' do - let!(:project) { create(:project, :wiki_repo, namespace: user.namespace) } - let(:project_wiki) { create(:project_wiki, project: project, user: project.creator) } - let(:page_name) { 'page_name' } - let(:page_dir) { "foo/bar/#{page_name}" } - let!(:wiki_page) { create(:wiki_page, wiki: project_wiki, attrs: { title: page_dir, content: 'Home page' }) } + it_behaves_like 'wiki file attachments' + end + end - before do - visit(project_wiki_edit_path(project, wiki_page)) - end + context 'when the page is in a subdir' do + let!(:project) { create(:project, :wiki_repo) } + let(:project_wiki) { create(:project_wiki, project: project, user: project.creator) } + let(:page_name) { 'page_name' } + let(:page_dir) { "foo/bar/#{page_name}" } + let!(:wiki_page) { create(:wiki_page, wiki: project_wiki, attrs: { title: page_dir, content: 'Home page' }) } - it 'moves the page to the root folder', :skip_gitaly_mock do - fill_in(:wiki_title, with: "/#{page_name}") + before do + visit(project_wiki_edit_path(project, wiki_page)) + end - click_button('Save changes') + it 'moves the page to the root folder' do + fill_in(:wiki_title, with: "/#{page_name}") - expect(current_path).to eq(project_wiki_path(project, page_name)) - end + click_button('Save changes') - it 'moves the page to other dir' do - new_page_dir = "foo1/bar1/#{page_name}" + expect(current_path).to eq(project_wiki_path(project, page_name)) + end - fill_in(:wiki_title, with: new_page_dir) + it 'moves the page to other dir' do + new_page_dir = "foo1/bar1/#{page_name}" - click_button('Save changes') + fill_in(:wiki_title, with: new_page_dir) - expect(current_path).to eq(project_wiki_path(project, new_page_dir)) - end + click_button('Save changes') - it 'remains in the same place if title has not changed' do - original_path = project_wiki_path(project, wiki_page) + expect(current_path).to eq(project_wiki_path(project, new_page_dir)) + end - fill_in(:wiki_title, with: page_name) + it 'remains in the same place if title has not changed' do + original_path = project_wiki_path(project, wiki_page) - click_button('Save changes') + fill_in(:wiki_title, with: page_name) - expect(current_path).to eq(original_path) - end + click_button('Save changes') - it 'can be moved to a different dir with a different name' do - new_page_dir = "foo1/bar1/new_page_name" + expect(current_path).to eq(original_path) + end - fill_in(:wiki_title, with: new_page_dir) + it 'can be moved to a different dir with a different name' do + new_page_dir = "foo1/bar1/new_page_name" - click_button('Save changes') + fill_in(:wiki_title, with: new_page_dir) - expect(current_path).to eq(project_wiki_path(project, new_page_dir)) - end + click_button('Save changes') - it 'can be renamed and moved to the root folder' do - new_name = 'new_page_name' + expect(current_path).to eq(project_wiki_path(project, new_page_dir)) + end - fill_in(:wiki_title, with: "/#{new_name}") + it 'can be renamed and moved to the root folder' do + new_name = 'new_page_name' - click_button('Save changes') + fill_in(:wiki_title, with: "/#{new_name}") - expect(current_path).to eq(project_wiki_path(project, new_name)) - end + click_button('Save changes') - it 'squishes the title before creating the page' do - new_page_dir = " foo1 / bar1 / #{page_name} " + expect(current_path).to eq(project_wiki_path(project, new_name)) + end - fill_in(:wiki_title, with: new_page_dir) + it 'squishes the title before creating the page' do + new_page_dir = " foo1 / bar1 / #{page_name} " - click_button('Save changes') + fill_in(:wiki_title, with: new_page_dir) - expect(current_path).to eq(project_wiki_path(project, "foo1/bar1/#{page_name}")) - end + click_button('Save changes') - it_behaves_like 'wiki file attachments' + expect(current_path).to eq(project_wiki_path(project, "foo1/bar1/#{page_name}")) end - end - - context 'when Gitaly is enabled' do - it_behaves_like 'wiki page user update' - end - context 'when Gitaly is disabled', :skip_gitaly_mock do - it_behaves_like 'wiki page user update' + it_behaves_like 'wiki file attachments' end end diff --git a/spec/features/projects/wiki/user_views_wiki_page_spec.rb b/spec/features/projects/wiki/user_views_wiki_page_spec.rb index 4b974a3ca10..3c93d71ab00 100644 --- a/spec/features/projects/wiki/user_views_wiki_page_spec.rb +++ b/spec/features/projects/wiki/user_views_wiki_page_spec.rb @@ -1,174 +1,164 @@ require 'spec_helper' describe 'User views a wiki page' do - shared_examples 'wiki page user view' do - include WikiHelpers - - let(:user) { create(:user) } - let(:project) { create(:project, :wiki_repo, namespace: user.namespace) } - let(:path) { 'image.png' } - let(:wiki_page) do - create(:wiki_page, - wiki: project.wiki, - attrs: { title: 'home', content: "Look at this [image](#{path})\n\n " }) - end + include WikiHelpers + + let(:user) { create(:user) } + let(:project) { create(:project, :wiki_repo, namespace: user.namespace) } + let(:path) { 'image.png' } + let(:wiki_page) do + create(:wiki_page, + wiki: project.wiki, + attrs: { title: 'home', content: "Look at this [image](#{path})\n\n " }) + end - before do - project.add_maintainer(user) - sign_in(user) - end + before do + project.add_maintainer(user) + sign_in(user) + end - context 'when wiki is empty' do - before do - visit(project_wikis_path(project)) - click_link "Create your first page" + context 'when wiki is empty' do + before do + visit(project_wikis_path(project)) + click_link "Create your first page" - click_on('New page') + click_on('New page') - page.within('#modal-new-wiki') do - fill_in(:new_wiki_path, with: 'one/two/three-test') - click_on('Create page') - end + page.within('#modal-new-wiki') do + fill_in(:new_wiki_path, with: 'one/two/three-test') + click_on('Create page') + end - page.within('.wiki-form') do - fill_in(:wiki_content, with: 'wiki content') - click_on('Create page') - end + page.within('.wiki-form') do + fill_in(:wiki_content, with: 'wiki content') + click_on('Create page') end + end - it 'shows the history of a page that has a path', :js do - expect(current_path).to include('one/two/three-test') + it 'shows the history of a page that has a path', :js do + expect(current_path).to include('one/two/three-test') - first(:link, text: 'Three').click - click_on('Page history') + first(:link, text: 'Three').click + click_on('Page history') - expect(current_path).to include('one/two/three-test') + expect(current_path).to include('one/two/three-test') - page.within(:css, '.nav-text') do - expect(page).to have_content('History') - end + page.within(:css, '.nav-text') do + expect(page).to have_content('History') end + end - it 'shows an old version of a page', :js do - expect(current_path).to include('one/two/three-test') - expect(find('.wiki-pages')).to have_content('Three') - - first(:link, text: 'Three').click - - expect(find('.nav-text')).to have_content('Three') + it 'shows an old version of a page', :js do + expect(current_path).to include('one/two/three-test') + expect(find('.wiki-pages')).to have_content('Three') - click_on('Edit') + first(:link, text: 'Three').click - expect(current_path).to include('one/two/three-test') - expect(page).to have_content('Edit Page') + expect(find('.nav-text')).to have_content('Three') - fill_in('Content', with: 'Updated Wiki Content') + click_on('Edit') - click_on('Save changes') - click_on('Page history') + expect(current_path).to include('one/two/three-test') + expect(page).to have_content('Edit Page') - page.within(:css, '.nav-text') do - expect(page).to have_content('History') - end + fill_in('Content', with: 'Updated Wiki Content') - find('a[href*="?version_id"]') - end - end - - context 'when a page does not have history' do - before do - visit(project_wiki_path(project, wiki_page)) - end + click_on('Save changes') + click_on('Page history') - it 'shows all the pages' do - expect(page).to have_content(user.name) - expect(find('.wiki-pages')).to have_content(wiki_page.title.capitalize) + page.within(:css, '.nav-text') do + expect(page).to have_content('History') end - context 'shows a file stored in a page' do - let(:path) { upload_file_to_wiki(project, user, 'dk.png') } + find('a[href*="?version_id"]') + end + end - it do - expect(page).to have_xpath("//img[@data-src='#{project.wiki.wiki_base_path}/#{path}']") - expect(page).to have_link('image', href: "#{project.wiki.wiki_base_path}/#{path}") + context 'when a page does not have history' do + before do + visit(project_wiki_path(project, wiki_page)) + end - click_on('image') + it 'shows all the pages' do + expect(page).to have_content(user.name) + expect(find('.wiki-pages')).to have_content(wiki_page.title.capitalize) + end - expect(current_path).to match("wikis/#{path}") - expect(page).not_to have_xpath('/html') # Page should render the image which means there is no html involved - end - end + context 'shows a file stored in a page' do + let(:path) { upload_file_to_wiki(project, user, 'dk.png') } - it 'shows the creation page if file does not exist' do + it do + expect(page).to have_xpath("//img[@data-src='#{project.wiki.wiki_base_path}/#{path}']") expect(page).to have_link('image', href: "#{project.wiki.wiki_base_path}/#{path}") click_on('image') expect(current_path).to match("wikis/#{path}") - expect(page).to have_content('New Wiki Page') - expect(page).to have_content('Create page') + expect(page).not_to have_xpath('/html') # Page should render the image which means there is no html involved end end - context 'when a page has history' do - before do - wiki_page.update(message: 'updated home', content: 'updated [some link](other-page)') - end + it 'shows the creation page if file does not exist' do + expect(page).to have_link('image', href: "#{project.wiki.wiki_base_path}/#{path}") - it 'shows the page history' do - visit(project_wiki_path(project, wiki_page)) + click_on('image') - expect(page).to have_selector('a.btn', text: 'Edit') + expect(current_path).to match("wikis/#{path}") + expect(page).to have_content('New Wiki Page') + expect(page).to have_content('Create page') + end + end - click_on('Page history') + context 'when a page has history' do + before do + wiki_page.update(message: 'updated home', content: 'updated [some link](other-page)') + end - expect(page).to have_content(user.name) - expect(page).to have_content("#{user.username} created page: home") - expect(page).to have_content('updated home') - end + it 'shows the page history' do + visit(project_wiki_path(project, wiki_page)) - it 'does not show the "Edit" button' do - visit(project_wiki_path(project, wiki_page, version_id: wiki_page.versions.last.id)) + expect(page).to have_selector('a.btn', text: 'Edit') - expect(page).not_to have_selector('a.btn', text: 'Edit') - end + click_on('Page history') + + expect(page).to have_content(user.name) + expect(page).to have_content("#{user.username} created page: home") + expect(page).to have_content('updated home') end - context 'when page has invalid content encoding' do - let(:content) { 'whatever'.force_encoding('ISO-8859-1') } + it 'does not show the "Edit" button' do + visit(project_wiki_path(project, wiki_page, version_id: wiki_page.versions.last.id)) - before do - allow(Gitlab::EncodingHelper).to receive(:encode!).and_return(content) + expect(page).not_to have_selector('a.btn', text: 'Edit') + end + end - visit(project_wiki_path(project, wiki_page)) - end + context 'when page has invalid content encoding' do + let(:content) { 'whatever'.force_encoding('ISO-8859-1') } - it 'does not show "Edit" button' do - expect(page).not_to have_selector('a.btn', text: 'Edit') - end + before do + allow(Gitlab::EncodingHelper).to receive(:encode!).and_return(content) - it 'shows error' do - page.within(:css, '.flash-notice') do - expect(page).to have_content('The content of this page is not encoded in UTF-8. Edits can only be made via the Git repository.') - end - end + visit(project_wiki_path(project, wiki_page)) end - it 'opens a default wiki page', :js do - visit(project_path(project)) - - find('.shortcuts-wiki').click - click_link "Create your first page" + it 'does not show "Edit" button' do + expect(page).not_to have_selector('a.btn', text: 'Edit') + end - expect(page).to have_content('Home · Create Page') + it 'shows error' do + page.within(:css, '.flash-notice') do + expect(page).to have_content('The content of this page is not encoded in UTF-8. Edits can only be made via the Git repository.') + end end end - context 'when Gitaly is enabled' do - it_behaves_like 'wiki page user view' - end + it 'opens a default wiki page', :js do + visit(project_path(project)) + + find('.shortcuts-wiki').click + click_link "Create your first page" - context 'when Gitaly is disabled', :skip_gitaly_mock do - it_behaves_like 'wiki page user view' + expect(page).to have_content('Home · Create Page') end end diff --git a/spec/lib/extracts_path_spec.rb b/spec/lib/extracts_path_spec.rb index 8947e2ac4fb..e0691aba600 100644 --- a/spec/lib/extracts_path_spec.rb +++ b/spec/lib/extracts_path_spec.rb @@ -205,28 +205,18 @@ describe ExtractsPath do end describe '#lfs_blob_ids' do - shared_examples '#lfs_blob_ids' do - let(:tag) { @project.repository.add_tag(@project.owner, 'my-annotated-tag', 'master', 'test tag') } - let(:ref) { tag.target } - let(:params) { { ref: ref, path: 'README.md' } } + let(:tag) { @project.repository.add_tag(@project.owner, 'my-annotated-tag', 'master', 'test tag') } + let(:ref) { tag.target } + let(:params) { { ref: ref, path: 'README.md' } } - before do - @project = create(:project, :repository) - end - - it 'handles annotated tags' do - assign_ref_vars - - expect(lfs_blob_ids).to eq([]) - end + before do + @project = create(:project, :repository) end - context 'when gitaly is enabled' do - it_behaves_like '#lfs_blob_ids' - end + it 'handles annotated tags' do + assign_ref_vars - context 'when gitaly is disabled', :skip_gitaly_mock do - it_behaves_like '#lfs_blob_ids' + expect(lfs_blob_ids).to eq([]) end end end diff --git a/spec/lib/gitlab/diff/file_collection/merge_request_diff_spec.rb b/spec/lib/gitlab/diff/file_collection/merge_request_diff_spec.rb index 4578da70bfc..fbcf515281e 100644 --- a/spec/lib/gitlab/diff/file_collection/merge_request_diff_spec.rb +++ b/spec/lib/gitlab/diff/file_collection/merge_request_diff_spec.rb @@ -37,17 +37,7 @@ describe Gitlab::Diff::FileCollection::MergeRequestDiff do let(:stub_path) { '.gitignore' } end - shared_examples 'initializes a DiffCollection' do - it 'returns a valid instance of a DiffCollection' do - expect(diff_files).to be_a(Gitlab::Git::DiffCollection) - end - end - - context 'with Gitaly disabled', :disable_gitaly do - it_behaves_like 'initializes a DiffCollection' - end - - context 'with Gitaly enabled' do - it_behaves_like 'initializes a DiffCollection' + it 'returns a valid instance of a DiffCollection' do + expect(diff_files).to be_a(Gitlab::Git::DiffCollection) end end diff --git a/spec/lib/gitlab/git/blob_spec.rb b/spec/lib/gitlab/git/blob_spec.rb index b243f0dacae..80dd3dcc58e 100644 --- a/spec/lib/gitlab/git/blob_spec.rb +++ b/spec/lib/gitlab/git/blob_spec.rb @@ -128,7 +128,7 @@ describe Gitlab::Git::Blob, :seed_helper do end end - shared_examples 'finding blobs by ID' do + describe '.raw' do let(:raw_blob) { Gitlab::Git::Blob.raw(repository, SeedRepo::RubyBlob::ID) } let(:bad_blob) { Gitlab::Git::Blob.raw(repository, SeedRepo::BigCommit::ID) } @@ -166,16 +166,6 @@ describe Gitlab::Git::Blob, :seed_helper do end end - describe '.raw' do - context 'when the blob_raw Gitaly feature is enabled' do - it_behaves_like 'finding blobs by ID' - end - - context 'when the blob_raw Gitaly feature is disabled', :skip_gitaly_mock do - it_behaves_like 'finding blobs by ID' - end - end - describe '.batch' do let(:blob_references) do [ diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index 6be35eee0fd..74d542060d5 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -183,110 +183,100 @@ describe Gitlab::Git::Commit, :seed_helper do end end - shared_examples '.where' do - context 'path is empty string' do - subject do - commits = described_class.where( - repo: repository, - ref: 'master', - path: '', - limit: 10 - ) - - commits.map { |c| c.id } - end + context 'path is empty string' do + subject do + commits = described_class.where( + repo: repository, + ref: 'master', + path: '', + limit: 10 + ) - it 'has 10 elements' do - expect(subject.size).to eq(10) - end - it { is_expected.to include(SeedRepo::EmptyCommit::ID) } + commits.map { |c| c.id } end - context 'path is nil' do - subject do - commits = described_class.where( - repo: repository, - ref: 'master', - path: nil, - limit: 10 - ) - - commits.map { |c| c.id } - end - - it 'has 10 elements' do - expect(subject.size).to eq(10) - end - it { is_expected.to include(SeedRepo::EmptyCommit::ID) } + it 'has 10 elements' do + expect(subject.size).to eq(10) end + it { is_expected.to include(SeedRepo::EmptyCommit::ID) } + end - context 'ref is branch name' do - subject do - commits = described_class.where( - repo: repository, - ref: 'master', - path: 'files', - limit: 3, - offset: 1 - ) + context 'path is nil' do + subject do + commits = described_class.where( + repo: repository, + ref: 'master', + path: nil, + limit: 10 + ) - commits.map { |c| c.id } - end + commits.map { |c| c.id } + end - it 'has 3 elements' do - expect(subject.size).to eq(3) - end - it { is_expected.to include("d14d6c0abdd253381df51a723d58691b2ee1ab08") } - it { is_expected.not_to include("eb49186cfa5c4338011f5f590fac11bd66c5c631") } + it 'has 10 elements' do + expect(subject.size).to eq(10) end + it { is_expected.to include(SeedRepo::EmptyCommit::ID) } + end - context 'ref is commit id' do - subject do - commits = described_class.where( - repo: repository, - ref: "874797c3a73b60d2187ed6e2fcabd289ff75171e", - path: 'files', - limit: 3, - offset: 1 - ) + context 'ref is branch name' do + subject do + commits = described_class.where( + repo: repository, + ref: 'master', + path: 'files', + limit: 3, + offset: 1 + ) - commits.map { |c| c.id } - end + commits.map { |c| c.id } + end - it 'has 3 elements' do - expect(subject.size).to eq(3) - end - it { is_expected.to include("2f63565e7aac07bcdadb654e253078b727143ec4") } - it { is_expected.not_to include(SeedRepo::Commit::ID) } + it 'has 3 elements' do + expect(subject.size).to eq(3) end + it { is_expected.to include("d14d6c0abdd253381df51a723d58691b2ee1ab08") } + it { is_expected.not_to include("eb49186cfa5c4338011f5f590fac11bd66c5c631") } + end - context 'ref is tag' do - subject do - commits = described_class.where( - repo: repository, - ref: 'v1.0.0', - path: 'files', - limit: 3, - offset: 1 - ) + context 'ref is commit id' do + subject do + commits = described_class.where( + repo: repository, + ref: "874797c3a73b60d2187ed6e2fcabd289ff75171e", + path: 'files', + limit: 3, + offset: 1 + ) - commits.map { |c| c.id } - end + commits.map { |c| c.id } + end - it 'has 3 elements' do - expect(subject.size).to eq(3) - end - it { is_expected.to include("874797c3a73b60d2187ed6e2fcabd289ff75171e") } - it { is_expected.not_to include(SeedRepo::Commit::ID) } + it 'has 3 elements' do + expect(subject.size).to eq(3) end + it { is_expected.to include("2f63565e7aac07bcdadb654e253078b727143ec4") } + it { is_expected.not_to include(SeedRepo::Commit::ID) } end - describe '.where with gitaly' do - it_should_behave_like '.where' - end + context 'ref is tag' do + subject do + commits = described_class.where( + repo: repository, + ref: 'v1.0.0', + path: 'files', + limit: 3, + offset: 1 + ) - describe '.where without gitaly', :skip_gitaly_mock do - it_should_behave_like '.where' + commits.map { |c| c.id } + end + + it 'has 3 elements' do + expect(subject.size).to eq(3) + end + it { is_expected.to include("874797c3a73b60d2187ed6e2fcabd289ff75171e") } + it { is_expected.not_to include(SeedRepo::Commit::ID) } end describe '.between' do @@ -508,7 +498,7 @@ describe Gitlab::Git::Commit, :seed_helper do end end - shared_examples '#stats' do + describe '#stats' do subject { commit.stats } describe '#additions' do @@ -527,14 +517,6 @@ describe Gitlab::Git::Commit, :seed_helper do end end - describe '#stats with gitaly on' do - it_should_behave_like '#stats' - end - - describe '#stats with gitaly disabled', :skip_gitaly_mock do - it_should_behave_like '#stats' - end - describe '#has_zero_stats?' do it { expect(commit.has_zero_stats?).to eq(false) } end @@ -577,25 +559,15 @@ describe Gitlab::Git::Commit, :seed_helper do commit_ids.map { |id| described_class.get_message(repository, id) } end - shared_examples 'getting commit messages' do - it 'gets commit messages' do - expect(subject).to contain_exactly( - "Added contributing guide\n\nSigned-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>\n", - "Add submodule\n\nSigned-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>\n" - ) - end - end - - context 'when Gitaly commit_messages feature is enabled' do - it_behaves_like 'getting commit messages' - - it 'gets messages in one batch', :request_store do - expect { subject.map(&:itself) }.to change { Gitlab::GitalyClient.get_request_count }.by(1) - end + it 'gets commit messages' do + expect(subject).to contain_exactly( + "Added contributing guide\n\nSigned-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>\n", + "Add submodule\n\nSigned-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>\n" + ) end - context 'when Gitaly commit_messages feature is disabled', :disable_gitaly do - it_behaves_like 'getting commit messages' + it 'gets messages in one batch', :request_store do + expect { subject.map(&:itself) }.to change { Gitlab::GitalyClient.get_request_count }.by(1) end end diff --git a/spec/lib/gitlab/git/tag_spec.rb b/spec/lib/gitlab/git/tag_spec.rb index c5bad062c2a..abee2822d77 100644 --- a/spec/lib/gitlab/git/tag_spec.rb +++ b/spec/lib/gitlab/git/tag_spec.rb @@ -3,7 +3,7 @@ require "spec_helper" describe Gitlab::Git::Tag, :seed_helper do let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } - shared_examples 'Gitlab::Git::Repository#tags' do + describe '#tags' do describe 'first tag' do let(:tag) { repository.tags.first } @@ -25,14 +25,6 @@ describe Gitlab::Git::Tag, :seed_helper do it { expect(repository.tags.size).to eq(SeedRepo::Repo::TAGS.size) } end - context 'when Gitaly tags feature is enabled' do - it_behaves_like 'Gitlab::Git::Repository#tags' - end - - context 'when Gitaly tags feature is disabled', :skip_gitaly_mock do - it_behaves_like 'Gitlab::Git::Repository#tags' - end - describe '.get_message' do let(:tag_ids) { %w[f4e6814c3e4e7a0de82a9e7cd20c626cc963a2f8 8a2a6eb295bb170b34c24c76c49ed0e9b2eaf34b] } @@ -40,23 +32,13 @@ describe Gitlab::Git::Tag, :seed_helper do tag_ids.map { |id| described_class.get_message(repository, id) } end - shared_examples 'getting tag messages' do - it 'gets tag messages' do - expect(subject[0]).to eq("Release\n") - expect(subject[1]).to eq("Version 1.1.0\n") - end - end - - context 'when Gitaly tag_messages feature is enabled' do - it_behaves_like 'getting tag messages' - - it 'gets messages in one batch', :request_store do - expect { subject.map(&:itself) }.to change { Gitlab::GitalyClient.get_request_count }.by(1) - end + it 'gets tag messages' do + expect(subject[0]).to eq("Release\n") + expect(subject[1]).to eq("Version 1.1.0\n") end - context 'when Gitaly tag_messages feature is disabled', :disable_gitaly do - it_behaves_like 'getting tag messages' + it 'gets messages in one batch', :request_store do + expect { subject.map(&:itself) }.to change { Gitlab::GitalyClient.get_request_count }.by(1) end end diff --git a/spec/lib/gitlab/git/tree_spec.rb b/spec/lib/gitlab/git/tree_spec.rb index 3792d6bf67b..bec875fb03d 100644 --- a/spec/lib/gitlab/git/tree_spec.rb +++ b/spec/lib/gitlab/git/tree_spec.rb @@ -80,18 +80,8 @@ describe Gitlab::Git::Tree, :seed_helper do end describe '#where' do - shared_examples '#where' do - it 'returns an empty array when called with an invalid ref' do - expect(described_class.where(repository, 'foobar-does-not-exist')).to eq([]) - end - end - - context 'with gitaly' do - it_behaves_like '#where' - end - - context 'without gitaly', :skip_gitaly_mock do - it_behaves_like '#where' + it 'returns an empty array when called with an invalid ref' do + expect(described_class.where(repository, 'foobar-does-not-exist')).to eq([]) end end end diff --git a/spec/lib/gitlab/gitaly_client_spec.rb b/spec/lib/gitlab/gitaly_client_spec.rb index 81bcd8c28ed..5eda4d041a8 100644 --- a/spec/lib/gitlab/gitaly_client_spec.rb +++ b/spec/lib/gitlab/gitaly_client_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' # We stub Gitaly in `spec/support/gitaly.rb` for other tests. We don't want # those stubs while testing the GitalyClient itself. -describe Gitlab::GitalyClient, skip_gitaly_mock: true do +describe Gitlab::GitalyClient do describe '.stub_class' do it 'returns the gRPC health check stub' do expect(described_class.stub_class(:health_check)).to eq(::Grpc::Health::V1::Health::Stub) @@ -191,102 +191,13 @@ describe Gitlab::GitalyClient, skip_gitaly_mock: true do let(:feature_name) { 'my_feature' } let(:real_feature_name) { "gitaly_#{feature_name}" } - context 'when Gitaly is disabled' do - before do - allow(described_class).to receive(:enabled?).and_return(false) - end - - it 'returns false' do - expect(described_class.feature_enabled?(feature_name)).to be(false) - end - end - - context 'when the feature status is DISABLED' do - let(:feature_status) { Gitlab::GitalyClient::MigrationStatus::DISABLED } - - it 'returns false' do - expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(false) - end - end - - context 'when the feature_status is OPT_IN' do - let(:feature_status) { Gitlab::GitalyClient::MigrationStatus::OPT_IN } - - context "when the feature flag hasn't been set" do - it 'returns false' do - expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(false) - end - end - - context "when the feature flag is set to disable" do - before do - Feature.get(real_feature_name).disable - end - - it 'returns false' do - expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(false) - end - end - - context "when the feature flag is set to enable" do - before do - Feature.get(real_feature_name).enable - end - - it 'returns true' do - expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(true) - end - end - - context "when the feature flag is set to a percentage of time" do - before do - Feature.get(real_feature_name).enable_percentage_of_time(70) - end - - it 'bases the result on pseudo-random numbers' do - expect(Random).to receive(:rand).and_return(0.3) - expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(true) - - expect(Random).to receive(:rand).and_return(0.8) - expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(false) - end - end - - context "when a feature is not persisted" do - it 'returns false when opt_into_all_features is off' do - allow(Feature).to receive(:persisted?).and_return(false) - allow(described_class).to receive(:opt_into_all_features?).and_return(false) - - expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(false) - end - - it 'returns true when the override is on' do - allow(Feature).to receive(:persisted?).and_return(false) - allow(described_class).to receive(:opt_into_all_features?).and_return(true) - - expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(true) - end - end + before do + allow(Feature).to receive(:enabled?).and_return(false) end - context 'when the feature_status is OPT_OUT' do - let(:feature_status) { Gitlab::GitalyClient::MigrationStatus::OPT_OUT } - - context "when the feature flag hasn't been set" do - it 'returns true' do - expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(true) - end - end - - context "when the feature flag is set to disable" do - before do - Feature.get(real_feature_name).disable - end - - it 'returns false' do - expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(false) - end - end + it 'returns false' do + expect(Feature).to receive(:enabled?).with(real_feature_name) + expect(described_class.feature_enabled?(feature_name)).to be(false) end end @@ -305,4 +216,29 @@ describe Gitlab::GitalyClient, skip_gitaly_mock: true do end end end + + describe 'Peek Performance bar details' do + let(:gitaly_server) { Gitaly::Server.all.first } + + before do + Gitlab::SafeRequestStore[:peek_enabled] = true + end + + context 'when the request store is active', :request_store do + it 'records call details if a RPC is called' do + gitaly_server.server_version + + expect(described_class.list_call_details).not_to be_empty + expect(described_class.list_call_details.size).to be(1) + end + end + + context 'when no request store is active' do + it 'records nothing' do + gitaly_server.server_version + + expect(described_class.list_call_details).to be_empty + end + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index a58dc8e25e8..ad55c280399 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -30,48 +30,38 @@ describe MergeRequest do end describe '#squash_in_progress?' do - shared_examples 'checking whether a squash is in progress' do - let(:repo_path) do - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - subject.source_project.repository.path - end - end - let(:squash_path) { File.join(repo_path, "gitlab-worktree", "squash-#{subject.id}") } - - before do - system(*%W(#{Gitlab.config.git.bin_path} -C #{repo_path} worktree add --detach #{squash_path} master)) - end - - it 'returns true when there is a current squash directory' do - expect(subject.squash_in_progress?).to be_truthy + let(:repo_path) do + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + subject.source_project.repository.path end + end + let(:squash_path) { File.join(repo_path, "gitlab-worktree", "squash-#{subject.id}") } - it 'returns false when there is no squash directory' do - FileUtils.rm_rf(squash_path) + before do + system(*%W(#{Gitlab.config.git.bin_path} -C #{repo_path} worktree add --detach #{squash_path} master)) + end - expect(subject.squash_in_progress?).to be_falsey - end + it 'returns true when there is a current squash directory' do + expect(subject.squash_in_progress?).to be_truthy + end - it 'returns false when the squash directory has expired' do - time = 20.minutes.ago.to_time - File.utime(time, time, squash_path) + it 'returns false when there is no squash directory' do + FileUtils.rm_rf(squash_path) - expect(subject.squash_in_progress?).to be_falsey - end + expect(subject.squash_in_progress?).to be_falsey + end - it 'returns false when the source project has been removed' do - allow(subject).to receive(:source_project).and_return(nil) + it 'returns false when the squash directory has expired' do + time = 20.minutes.ago.to_time + File.utime(time, time, squash_path) - expect(subject.squash_in_progress?).to be_falsey - end + expect(subject.squash_in_progress?).to be_falsey end - context 'when Gitaly squash_in_progress is enabled' do - it_behaves_like 'checking whether a squash is in progress' - end + it 'returns false when the source project has been removed' do + allow(subject).to receive(:source_project).and_return(nil) - context 'when Gitaly squash_in_progress is disabled', :disable_gitaly do - it_behaves_like 'checking whether a squash is in progress' + expect(subject.squash_in_progress?).to be_falsey end end @@ -2587,14 +2577,6 @@ describe MergeRequest do expect(subject.rebase_in_progress?).to be_falsey end end - - context 'when Gitaly rebase_in_progress is enabled' do - it_behaves_like 'checking whether a rebase is in progress' - end - - context 'when Gitaly rebase_in_progress is enabled', :disable_gitaly do - it_behaves_like 'checking whether a rebase is in progress' - end end describe '#allow_collaboration' do diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb index cc5e34782ec..48a43801b9f 100644 --- a/spec/models/project_wiki_spec.rb +++ b/spec/models/project_wiki_spec.rb @@ -130,63 +130,53 @@ describe ProjectWiki do end describe "#find_page" do - shared_examples 'finding a wiki page' do - before do - create_page("index page", "This is an awesome Gollum Wiki") - end - - after do - subject.pages.each { |page| destroy_page(page.page) } - end + before do + create_page("index page", "This is an awesome Gollum Wiki") + end - it "returns the latest version of the page if it exists" do - page = subject.find_page("index page") - expect(page.title).to eq("index page") - end + after do + subject.pages.each { |page| destroy_page(page.page) } + end - it "returns nil if the page does not exist" do - expect(subject.find_page("non-existent")).to eq(nil) - end + it "returns the latest version of the page if it exists" do + page = subject.find_page("index page") + expect(page.title).to eq("index page") + end - it "can find a page by slug" do - page = subject.find_page("index-page") - expect(page.title).to eq("index page") - end + it "returns nil if the page does not exist" do + expect(subject.find_page("non-existent")).to eq(nil) + end - it "returns a WikiPage instance" do - page = subject.find_page("index page") - expect(page).to be_a WikiPage - end + it "can find a page by slug" do + page = subject.find_page("index-page") + expect(page.title).to eq("index page") + end - context 'pages with multibyte-character title' do - before do - create_page("autre pagé", "C'est un génial Gollum Wiki") - end + it "returns a WikiPage instance" do + page = subject.find_page("index page") + expect(page).to be_a WikiPage + end - it "can find a page by slug" do - page = subject.find_page("autre pagé") - expect(page.title).to eq("autre pagé") - end + context 'pages with multibyte-character title' do + before do + create_page("autre pagé", "C'est un génial Gollum Wiki") end - context 'pages with invalidly-encoded content' do - before do - create_page("encoding is fun", "f\xFCr".b) - end - - it "can find the page" do - page = subject.find_page("encoding is fun") - expect(page.content).to eq("fr") - end + it "can find a page by slug" do + page = subject.find_page("autre pagé") + expect(page.title).to eq("autre pagé") end end - context 'when Gitaly wiki_find_page is enabled' do - it_behaves_like 'finding a wiki page' - end + context 'pages with invalidly-encoded content' do + before do + create_page("encoding is fun", "f\xFCr".b) + end - context 'when Gitaly wiki_find_page is disabled', :skip_gitaly_mock do - it_behaves_like 'finding a wiki page' + it "can find the page" do + page = subject.find_page("encoding is fun") + expect(page.content).to eq("fr") + end end end @@ -207,100 +197,80 @@ describe ProjectWiki do end describe '#find_file' do - shared_examples 'finding a wiki file' do - let(:image) { File.open(Rails.root.join('spec', 'fixtures', 'big-image.png')) } - - before do - subject.wiki # Make sure the wiki repo exists - - repo_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do - subject.repository.path_to_repo - end - - BareRepoOperations.new(repo_path).commit_file(image, 'image.png') - end + let(:image) { File.open(Rails.root.join('spec', 'fixtures', 'big-image.png')) } - it 'returns the latest version of the file if it exists' do - file = subject.find_file('image.png') - expect(file.mime_type).to eq('image/png') - end + before do + subject.wiki # Make sure the wiki repo exists - it 'returns nil if the page does not exist' do - expect(subject.find_file('non-existent')).to eq(nil) + repo_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + subject.repository.path_to_repo end - it 'returns a Gitlab::Git::WikiFile instance' do - file = subject.find_file('image.png') - expect(file).to be_a Gitlab::Git::WikiFile - end + BareRepoOperations.new(repo_path).commit_file(image, 'image.png') + end - it 'returns the whole file' do - file = subject.find_file('image.png') - image.rewind + it 'returns the latest version of the file if it exists' do + file = subject.find_file('image.png') + expect(file.mime_type).to eq('image/png') + end - expect(file.raw_data.b).to eq(image.read.b) - end + it 'returns nil if the page does not exist' do + expect(subject.find_file('non-existent')).to eq(nil) end - context 'when Gitaly wiki_find_file is enabled' do - it_behaves_like 'finding a wiki file' + it 'returns a Gitlab::Git::WikiFile instance' do + file = subject.find_file('image.png') + expect(file).to be_a Gitlab::Git::WikiFile end - context 'when Gitaly wiki_find_file is disabled', :skip_gitaly_mock do - it_behaves_like 'finding a wiki file' + it 'returns the whole file' do + file = subject.find_file('image.png') + image.rewind + + expect(file.raw_data.b).to eq(image.read.b) end end describe "#create_page" do - shared_examples 'creating a wiki page' do - after do - destroy_page(subject.pages.first.page) - end - - it "creates a new wiki page" do - expect(subject.create_page("test page", "this is content")).not_to eq(false) - expect(subject.pages.count).to eq(1) - end - - it "returns false when a duplicate page exists" do - subject.create_page("test page", "content") - expect(subject.create_page("test page", "content")).to eq(false) - end - - it "stores an error message when a duplicate page exists" do - 2.times { subject.create_page("test page", "content") } - expect(subject.error_message).to match(/Duplicate page:/) - end + after do + destroy_page(subject.pages.first.page) + end - it "sets the correct commit message" do - subject.create_page("test page", "some content", :markdown, "commit message") - expect(subject.pages.first.page.version.message).to eq("commit message") - end + it "creates a new wiki page" do + expect(subject.create_page("test page", "this is content")).not_to eq(false) + expect(subject.pages.count).to eq(1) + end - it 'sets the correct commit email' do - subject.create_page('test page', 'content') + it "returns false when a duplicate page exists" do + subject.create_page("test page", "content") + expect(subject.create_page("test page", "content")).to eq(false) + end - expect(user.commit_email).not_to eq(user.email) - expect(commit.author_email).to eq(user.commit_email) - expect(commit.committer_email).to eq(user.commit_email) - end + it "stores an error message when a duplicate page exists" do + 2.times { subject.create_page("test page", "content") } + expect(subject.error_message).to match(/Duplicate page:/) + end - it 'updates project activity' do - subject.create_page('Test Page', 'This is content') + it "sets the correct commit message" do + subject.create_page("test page", "some content", :markdown, "commit message") + expect(subject.pages.first.page.version.message).to eq("commit message") + end - project.reload + it 'sets the correct commit email' do + subject.create_page('test page', 'content') - expect(project.last_activity_at).to be_within(1.minute).of(Time.now) - expect(project.last_repository_updated_at).to be_within(1.minute).of(Time.now) - end + expect(user.commit_email).not_to eq(user.email) + expect(commit.author_email).to eq(user.commit_email) + expect(commit.committer_email).to eq(user.commit_email) end - context 'when Gitaly wiki_write_page is enabled' do - it_behaves_like 'creating a wiki page' - end + it 'updates project activity' do + subject.create_page('Test Page', 'This is content') - context 'when Gitaly wiki_write_page is disabled', :skip_gitaly_mock do - it_behaves_like 'creating a wiki page' + project.reload + + expect(project.last_activity_at).to be_within(1.minute).of(Time.now) + expect(project.last_repository_updated_at).to be_within(1.minute).of(Time.now) end end @@ -351,41 +321,31 @@ describe ProjectWiki do end describe "#delete_page" do - shared_examples 'deleting a wiki page' do - before do - create_page("index", "some content") - @page = subject.wiki.page(title: "index") - end - - it "deletes the page" do - subject.delete_page(@page) - expect(subject.pages.count).to eq(0) - end - - it 'sets the correct commit email' do - subject.delete_page(@page) - - expect(user.commit_email).not_to eq(user.email) - expect(commit.author_email).to eq(user.commit_email) - expect(commit.committer_email).to eq(user.commit_email) - end + before do + create_page("index", "some content") + @page = subject.wiki.page(title: "index") + end - it 'updates project activity' do - subject.delete_page(@page) + it "deletes the page" do + subject.delete_page(@page) + expect(subject.pages.count).to eq(0) + end - project.reload + it 'sets the correct commit email' do + subject.delete_page(@page) - expect(project.last_activity_at).to be_within(1.minute).of(Time.now) - expect(project.last_repository_updated_at).to be_within(1.minute).of(Time.now) - end + expect(user.commit_email).not_to eq(user.email) + expect(commit.author_email).to eq(user.commit_email) + expect(commit.committer_email).to eq(user.commit_email) end - context 'when Gitaly wiki_delete_page is enabled' do - it_behaves_like 'deleting a wiki page' - end + it 'updates project activity' do + subject.delete_page(@page) + + project.reload - context 'when Gitaly wiki_delete_page is disabled', :skip_gitaly_mock do - it_behaves_like 'deleting a wiki page' + expect(project.last_activity_at).to be_within(1.minute).of(Time.now) + expect(project.last_repository_updated_at).to be_within(1.minute).of(Time.now) end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 56edb0fd6da..187283b284b 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -38,49 +38,29 @@ describe Repository do end describe '#branch_names_contains' do - shared_examples '#branch_names_contains' do - set(:project) { create(:project, :repository) } - let(:repository) { project.repository } + set(:project) { create(:project, :repository) } + let(:repository) { project.repository } - subject { repository.branch_names_contains(sample_commit.id) } + subject { repository.branch_names_contains(sample_commit.id) } - it { is_expected.to include('master') } - it { is_expected.not_to include('feature') } - it { is_expected.not_to include('fix') } + it { is_expected.to include('master') } + it { is_expected.not_to include('feature') } + it { is_expected.not_to include('fix') } - describe 'when storage is broken', :broken_storage do - it 'should raise a storage error' do - expect_to_raise_storage_error do - broken_repository.branch_names_contains(sample_commit.id) - end + describe 'when storage is broken', :broken_storage do + it 'should raise a storage error' do + expect_to_raise_storage_error do + broken_repository.branch_names_contains(sample_commit.id) end end end - - context 'when gitaly is enabled' do - it_behaves_like '#branch_names_contains' - end - - context 'when gitaly is disabled', :skip_gitaly_mock do - it_behaves_like '#branch_names_contains' - end end describe '#tag_names_contains' do - shared_examples '#tag_names_contains' do - subject { repository.tag_names_contains(sample_commit.id) } - - it { is_expected.to include('v1.1.0') } - it { is_expected.not_to include('v1.0.0') } - end - - context 'when gitaly is enabled' do - it_behaves_like '#tag_names_contains' - end + subject { repository.tag_names_contains(sample_commit.id) } - context 'when gitaly is enabled', :skip_gitaly_mock do - it_behaves_like '#tag_names_contains' - end + it { is_expected.to include('v1.1.0') } + it { is_expected.not_to include('v1.0.0') } end describe 'tags_sorted_by' do @@ -238,61 +218,41 @@ describe Repository do end describe '#last_commit_for_path' do - shared_examples 'getting last commit for path' do - subject { repository.last_commit_for_path(sample_commit.id, '.gitignore').id } + subject { repository.last_commit_for_path(sample_commit.id, '.gitignore').id } - it { is_expected.to eq('c1acaa58bbcbc3eafe538cb8274ba387047b69f8') } + it { is_expected.to eq('c1acaa58bbcbc3eafe538cb8274ba387047b69f8') } - describe 'when storage is broken', :broken_storage do - it 'should raise a storage error' do - expect_to_raise_storage_error do - broken_repository.last_commit_id_for_path(sample_commit.id, '.gitignore') - end + describe 'when storage is broken', :broken_storage do + it 'should raise a storage error' do + expect_to_raise_storage_error do + broken_repository.last_commit_id_for_path(sample_commit.id, '.gitignore') end end end - - context 'when Gitaly feature last_commit_for_path is enabled' do - it_behaves_like 'getting last commit for path' - end - - context 'when Gitaly feature last_commit_for_path is disabled', :skip_gitaly_mock do - it_behaves_like 'getting last commit for path' - end end describe '#last_commit_id_for_path' do - shared_examples 'getting last commit ID for path' do - subject { repository.last_commit_id_for_path(sample_commit.id, '.gitignore') } + subject { repository.last_commit_id_for_path(sample_commit.id, '.gitignore') } - it "returns last commit id for a given path" do - is_expected.to eq('c1acaa58bbcbc3eafe538cb8274ba387047b69f8') - end + it "returns last commit id for a given path" do + is_expected.to eq('c1acaa58bbcbc3eafe538cb8274ba387047b69f8') + end - it "caches last commit id for a given path" do - cache = repository.send(:cache) - key = "last_commit_id_for_path:#{sample_commit.id}:#{Digest::SHA1.hexdigest('.gitignore')}" + it "caches last commit id for a given path" do + cache = repository.send(:cache) + key = "last_commit_id_for_path:#{sample_commit.id}:#{Digest::SHA1.hexdigest('.gitignore')}" - expect(cache).to receive(:fetch).with(key).and_return('c1acaa5') - is_expected.to eq('c1acaa5') - end + expect(cache).to receive(:fetch).with(key).and_return('c1acaa5') + is_expected.to eq('c1acaa5') + end - describe 'when storage is broken', :broken_storage do - it 'should raise a storage error' do - expect_to_raise_storage_error do - broken_repository.last_commit_for_path(sample_commit.id, '.gitignore').id - end + describe 'when storage is broken', :broken_storage do + it 'should raise a storage error' do + expect_to_raise_storage_error do + broken_repository.last_commit_for_path(sample_commit.id, '.gitignore').id end end end - - context 'when Gitaly feature last_commit_for_path is enabled' do - it_behaves_like 'getting last commit ID for path' - end - - context 'when Gitaly feature last_commit_for_path is disabled', :skip_gitaly_mock do - it_behaves_like 'getting last commit ID for path' - end end describe '#commits' do @@ -374,78 +334,57 @@ describe Repository do describe '#commits_by' do set(:project) { create(:project, :repository) } + let(:oids) { TestEnv::BRANCH_SHA.values } - shared_examples 'batch commits fetching' do - let(:oids) { TestEnv::BRANCH_SHA.values } + subject { project.repository.commits_by(oids: oids) } - subject { project.repository.commits_by(oids: oids) } + it 'finds each commit' do + expect(subject).not_to include(nil) + expect(subject.size).to eq(oids.size) + end - it 'finds each commit' do - expect(subject).not_to include(nil) - expect(subject.size).to eq(oids.size) - end + it 'returns only Commit instances' do + expect(subject).to all( be_a(Commit) ) + end - it 'returns only Commit instances' do - expect(subject).to all( be_a(Commit) ) + context 'when some commits are not found ' do + let(:oids) do + ['deadbeef'] + TestEnv::BRANCH_SHA.values.first(10) end - context 'when some commits are not found ' do - let(:oids) do - ['deadbeef'] + TestEnv::BRANCH_SHA.values.first(10) - end - - it 'returns only found commits' do - expect(subject).not_to include(nil) - expect(subject.size).to eq(10) - end + it 'returns only found commits' do + expect(subject).not_to include(nil) + expect(subject.size).to eq(10) end + end - context 'when no oids are passed' do - let(:oids) { [] } + context 'when no oids are passed' do + let(:oids) { [] } - it 'does not call #batch_by_oid' do - expect(Gitlab::Git::Commit).not_to receive(:batch_by_oid) + it 'does not call #batch_by_oid' do + expect(Gitlab::Git::Commit).not_to receive(:batch_by_oid) - subject - end + subject end end - - context 'when Gitaly list_commits_by_oid is enabled' do - it_behaves_like 'batch commits fetching' - end - - context 'when Gitaly list_commits_by_oid is enabled', :disable_gitaly do - it_behaves_like 'batch commits fetching' - end end describe '#find_commits_by_message' do - shared_examples 'finding commits by message' do - it 'returns commits with messages containing a given string' do - commit_ids = repository.find_commits_by_message('submodule').map(&:id) - - expect(commit_ids).to include( - '5937ac0a7beb003549fc5fd26fc247adbce4a52e', - '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9', - 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660' - ) - expect(commit_ids).not_to include('913c66a37b4a45b9769037c55c2d238bd0942d2e') - end - - it 'is case insensitive' do - commit_ids = repository.find_commits_by_message('SUBMODULE').map(&:id) + it 'returns commits with messages containing a given string' do + commit_ids = repository.find_commits_by_message('submodule').map(&:id) - expect(commit_ids).to include('5937ac0a7beb003549fc5fd26fc247adbce4a52e') - end + expect(commit_ids).to include( + '5937ac0a7beb003549fc5fd26fc247adbce4a52e', + '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9', + 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660' + ) + expect(commit_ids).not_to include('913c66a37b4a45b9769037c55c2d238bd0942d2e') end - context 'when Gitaly commits_by_message feature is enabled' do - it_behaves_like 'finding commits by message' - end + it 'is case insensitive' do + commit_ids = repository.find_commits_by_message('SUBMODULE').map(&:id) - context 'when Gitaly commits_by_message feature is disabled', :skip_gitaly_mock do - it_behaves_like 'finding commits by message' + expect(commit_ids).to include('5937ac0a7beb003549fc5fd26fc247adbce4a52e') end describe 'when storage is broken', :broken_storage do @@ -1328,34 +1267,23 @@ describe Repository do describe '#merge' do let(:merge_request) { create(:merge_request, source_branch: 'feature', target_branch: 'master', source_project: project) } - let(:message) { 'Test \r\n\r\n message' } - shared_examples '#merge' do - it 'merges the code and returns the commit id' do - expect(merge_commit).to be_present - expect(repository.blob_at(merge_commit.id, 'files/ruby/feature.rb')).to be_present - end - - it 'sets the `in_progress_merge_commit_sha` flag for the given merge request' do - merge_commit_id = merge(repository, user, merge_request, message) - - expect(merge_request.in_progress_merge_commit_sha).to eq(merge_commit_id) - end + it 'merges the code and returns the commit id' do + expect(merge_commit).to be_present + expect(repository.blob_at(merge_commit.id, 'files/ruby/feature.rb')).to be_present + end - it 'removes carriage returns from commit message' do - merge_commit_id = merge(repository, user, merge_request, message) + it 'sets the `in_progress_merge_commit_sha` flag for the given merge request' do + merge_commit_id = merge(repository, user, merge_request, message) - expect(repository.commit(merge_commit_id).message).to eq(message.delete("\r")) - end + expect(merge_request.in_progress_merge_commit_sha).to eq(merge_commit_id) end - context 'with gitaly' do - it_behaves_like '#merge' - end + it 'removes carriage returns from commit message' do + merge_commit_id = merge(repository, user, merge_request, message) - context 'without gitaly', :skip_gitaly_mock do - it_behaves_like '#merge' + expect(repository.commit(merge_commit_id).message).to eq(message.delete("\r")) end def merge(repository, user, merge_request, message) @@ -1392,98 +1320,78 @@ describe Repository do end describe '#revert' do - shared_examples 'reverting a commit' do - let(:new_image_commit) { repository.commit('33f3729a45c02fc67d00adb1b8bca394b0e761d9') } - let(:update_image_commit) { repository.commit('2f63565e7aac07bcdadb654e253078b727143ec4') } - let(:message) { 'revert message' } - - context 'when there is a conflict' do - it 'raises an error' do - expect { repository.revert(user, new_image_commit, 'master', message) }.to raise_error(Gitlab::Git::Repository::CreateTreeError) - end - end + let(:new_image_commit) { repository.commit('33f3729a45c02fc67d00adb1b8bca394b0e761d9') } + let(:update_image_commit) { repository.commit('2f63565e7aac07bcdadb654e253078b727143ec4') } + let(:message) { 'revert message' } - context 'when commit was already reverted' do - it 'raises an error' do - repository.revert(user, update_image_commit, 'master', message) - - expect { repository.revert(user, update_image_commit, 'master', message) }.to raise_error(Gitlab::Git::Repository::CreateTreeError) - end - end - - context 'when commit can be reverted' do - it 'reverts the changes' do - expect(repository.revert(user, update_image_commit, 'master', message)).to be_truthy - end + context 'when there is a conflict' do + it 'raises an error' do + expect { repository.revert(user, new_image_commit, 'master', message) }.to raise_error(Gitlab::Git::Repository::CreateTreeError) end + end - context 'reverting a merge commit' do - it 'reverts the changes' do - merge_commit - expect(repository.blob_at_branch('master', 'files/ruby/feature.rb')).to be_present + context 'when commit was already reverted' do + it 'raises an error' do + repository.revert(user, update_image_commit, 'master', message) - repository.revert(user, merge_commit, 'master', message) - expect(repository.blob_at_branch('master', 'files/ruby/feature.rb')).not_to be_present - end + expect { repository.revert(user, update_image_commit, 'master', message) }.to raise_error(Gitlab::Git::Repository::CreateTreeError) end end - context 'when Gitaly revert feature is enabled' do - it_behaves_like 'reverting a commit' + context 'when commit can be reverted' do + it 'reverts the changes' do + expect(repository.revert(user, update_image_commit, 'master', message)).to be_truthy + end end - context 'when Gitaly revert feature is disabled', :disable_gitaly do - it_behaves_like 'reverting a commit' + context 'reverting a merge commit' do + it 'reverts the changes' do + merge_commit + expect(repository.blob_at_branch('master', 'files/ruby/feature.rb')).to be_present + + repository.revert(user, merge_commit, 'master', message) + expect(repository.blob_at_branch('master', 'files/ruby/feature.rb')).not_to be_present + end end end describe '#cherry_pick' do - shared_examples 'cherry-picking a commit' do - let(:conflict_commit) { repository.commit('c642fe9b8b9f28f9225d7ea953fe14e74748d53b') } - let(:pickable_commit) { repository.commit('7d3b0f7cff5f37573aea97cebfd5692ea1689924') } - let(:pickable_merge) { repository.commit('e56497bb5f03a90a51293fc6d516788730953899') } - let(:message) { 'cherry-pick message' } - - context 'when there is a conflict' do - it 'raises an error' do - expect { repository.cherry_pick(user, conflict_commit, 'master', message) }.to raise_error(Gitlab::Git::Repository::CreateTreeError) - end + let(:conflict_commit) { repository.commit('c642fe9b8b9f28f9225d7ea953fe14e74748d53b') } + let(:pickable_commit) { repository.commit('7d3b0f7cff5f37573aea97cebfd5692ea1689924') } + let(:pickable_merge) { repository.commit('e56497bb5f03a90a51293fc6d516788730953899') } + let(:message) { 'cherry-pick message' } + + context 'when there is a conflict' do + it 'raises an error' do + expect { repository.cherry_pick(user, conflict_commit, 'master', message) }.to raise_error(Gitlab::Git::Repository::CreateTreeError) end + end - context 'when commit was already cherry-picked' do - it 'raises an error' do - repository.cherry_pick(user, pickable_commit, 'master', message) + context 'when commit was already cherry-picked' do + it 'raises an error' do + repository.cherry_pick(user, pickable_commit, 'master', message) - expect { repository.cherry_pick(user, pickable_commit, 'master', message) }.to raise_error(Gitlab::Git::Repository::CreateTreeError) - end + expect { repository.cherry_pick(user, pickable_commit, 'master', message) }.to raise_error(Gitlab::Git::Repository::CreateTreeError) end + end - context 'when commit can be cherry-picked' do - it 'cherry-picks the changes' do - expect(repository.cherry_pick(user, pickable_commit, 'master', message)).to be_truthy - end + context 'when commit can be cherry-picked' do + it 'cherry-picks the changes' do + expect(repository.cherry_pick(user, pickable_commit, 'master', message)).to be_truthy end + end - context 'cherry-picking a merge commit' do - it 'cherry-picks the changes' do - expect(repository.blob_at_branch('improve/awesome', 'foo/bar/.gitkeep')).to be_nil + context 'cherry-picking a merge commit' do + it 'cherry-picks the changes' do + expect(repository.blob_at_branch('improve/awesome', 'foo/bar/.gitkeep')).to be_nil - cherry_pick_commit_sha = repository.cherry_pick(user, pickable_merge, 'improve/awesome', message) - cherry_pick_commit_message = project.commit(cherry_pick_commit_sha).message + cherry_pick_commit_sha = repository.cherry_pick(user, pickable_merge, 'improve/awesome', message) + cherry_pick_commit_message = project.commit(cherry_pick_commit_sha).message - expect(repository.blob_at_branch('improve/awesome', 'foo/bar/.gitkeep')).not_to be_nil - expect(cherry_pick_commit_message).to eq(message) - end + expect(repository.blob_at_branch('improve/awesome', 'foo/bar/.gitkeep')).not_to be_nil + expect(cherry_pick_commit_message).to eq(message) end end - - context 'when Gitaly cherry_pick feature is enabled' do - it_behaves_like 'cherry-picking a commit' - end - - context 'when Gitaly cherry_pick feature is disabled', :disable_gitaly do - it_behaves_like 'cherry-picking a commit' - end end describe '#before_delete' do @@ -2190,33 +2098,23 @@ describe Repository do let(:commit) { repository.commit } let(:ancestor) { commit.parents.first } - shared_examples '#ancestor?' do - it 'it is an ancestor' do - expect(repository.ancestor?(ancestor.id, commit.id)).to eq(true) - end - - it 'it is not an ancestor' do - expect(repository.ancestor?(commit.id, ancestor.id)).to eq(false) - end - - it 'returns false on nil-values' do - expect(repository.ancestor?(nil, commit.id)).to eq(false) - expect(repository.ancestor?(ancestor.id, nil)).to eq(false) - expect(repository.ancestor?(nil, nil)).to eq(false) - end + it 'it is an ancestor' do + expect(repository.ancestor?(ancestor.id, commit.id)).to eq(true) + end - it 'returns false for invalid commit IDs' do - expect(repository.ancestor?(commit.id, Gitlab::Git::BLANK_SHA)).to eq(false) - expect(repository.ancestor?( Gitlab::Git::BLANK_SHA, commit.id)).to eq(false) - end + it 'it is not an ancestor' do + expect(repository.ancestor?(commit.id, ancestor.id)).to eq(false) end - context 'with Gitaly enabled' do - it_behaves_like('#ancestor?') + it 'returns false on nil-values' do + expect(repository.ancestor?(nil, commit.id)).to eq(false) + expect(repository.ancestor?(ancestor.id, nil)).to eq(false) + expect(repository.ancestor?(nil, nil)).to eq(false) end - context 'with Gitaly disabled', :skip_gitaly_mock do - it_behaves_like('#ancestor?') + it 'returns false for invalid commit IDs' do + expect(repository.ancestor?(commit.id, Gitlab::Git::BLANK_SHA)).to eq(false) + expect(repository.ancestor?( Gitlab::Git::BLANK_SHA, commit.id)).to eq(false) end end diff --git a/spec/models/wiki_page_spec.rb b/spec/models/wiki_page_spec.rb index b87a2d871e5..cba22b2cc4e 100644 --- a/spec/models/wiki_page_spec.rb +++ b/spec/models/wiki_page_spec.rb @@ -200,180 +200,160 @@ describe WikiPage do end describe '#create' do - shared_examples 'create method' do - context 'with valid attributes' do - it 'raises an error if a page with the same path already exists' do - create_page('New Page', 'content') - create_page('foo/bar', 'content') - expect { create_page('New Page', 'other content') }.to raise_error Gitlab::Git::Wiki::DuplicatePageError - expect { create_page('foo/bar', 'other content') }.to raise_error Gitlab::Git::Wiki::DuplicatePageError - - destroy_page('New Page') - destroy_page('bar', 'foo') - end + context 'with valid attributes' do + it 'raises an error if a page with the same path already exists' do + create_page('New Page', 'content') + create_page('foo/bar', 'content') + expect { create_page('New Page', 'other content') }.to raise_error Gitlab::Git::Wiki::DuplicatePageError + expect { create_page('foo/bar', 'other content') }.to raise_error Gitlab::Git::Wiki::DuplicatePageError - it 'if the title is preceded by a / it is removed' do - create_page('/New Page', 'content') + destroy_page('New Page') + destroy_page('bar', 'foo') + end - expect(wiki.find_page('New Page')).not_to be_nil + it 'if the title is preceded by a / it is removed' do + create_page('/New Page', 'content') - destroy_page('New Page') - end + expect(wiki.find_page('New Page')).not_to be_nil + + destroy_page('New Page') end end + end - context 'when Gitaly is enabled' do - it_behaves_like 'create method' + describe "#update" do + before do + create_page("Update", "content") + @page = wiki.find_page("Update") end - context 'when Gitaly is disabled', :skip_gitaly_mock do - it_behaves_like 'create method' + after do + destroy_page(@page.title, @page.directory) end - end - describe "#update" do - shared_examples 'update method' do - before do - create_page("Update", "content") + context "with valid attributes" do + it "updates the content of the page" do + new_content = "new content" + + @page.update(content: new_content) @page = wiki.find_page("Update") - end - after do - destroy_page(@page.title, @page.directory) + expect(@page.content).to eq("new content") end - context "with valid attributes" do - it "updates the content of the page" do - new_content = "new content" - - @page.update(content: new_content) - @page = wiki.find_page("Update") - - expect(@page.content).to eq("new content") - end + it "updates the title of the page" do + new_title = "Index v.1.2.4" - it "updates the title of the page" do - new_title = "Index v.1.2.4" + @page.update(title: new_title) + @page = wiki.find_page(new_title) - @page.update(title: new_title) - @page = wiki.find_page(new_title) - - expect(@page.title).to eq(new_title) - end + expect(@page.title).to eq(new_title) + end - it "returns true" do - expect(@page.update(content: "more content")).to be_truthy - end + it "returns true" do + expect(@page.update(content: "more content")).to be_truthy end + end - context 'with same last commit sha' do - it 'returns true' do - expect(@page.update(content: 'more content', last_commit_sha: @page.last_commit_sha)).to be_truthy - end + context 'with same last commit sha' do + it 'returns true' do + expect(@page.update(content: 'more content', last_commit_sha: @page.last_commit_sha)).to be_truthy end + end - context 'with different last commit sha' do - it 'raises exception' do - expect { @page.update(content: 'more content', last_commit_sha: 'xxx') }.to raise_error(WikiPage::PageChangedError) - end + context 'with different last commit sha' do + it 'raises exception' do + expect { @page.update(content: 'more content', last_commit_sha: 'xxx') }.to raise_error(WikiPage::PageChangedError) end + end - context 'when renaming a page' do - it 'raises an error if the page already exists' do - create_page('Existing Page', 'content') + context 'when renaming a page' do + it 'raises an error if the page already exists' do + create_page('Existing Page', 'content') - expect { @page.update(title: 'Existing Page', content: 'new_content') }.to raise_error(WikiPage::PageRenameError) - expect(@page.title).to eq 'Update' - expect(@page.content).to eq 'new_content' + expect { @page.update(title: 'Existing Page', content: 'new_content') }.to raise_error(WikiPage::PageRenameError) + expect(@page.title).to eq 'Update' + expect(@page.content).to eq 'new_content' - destroy_page('Existing Page') - end + destroy_page('Existing Page') + end - it 'updates the content and rename the file' do - new_title = 'Renamed Page' - new_content = 'updated content' + it 'updates the content and rename the file' do + new_title = 'Renamed Page' + new_content = 'updated content' - expect(@page.update(title: new_title, content: new_content)).to be_truthy + expect(@page.update(title: new_title, content: new_content)).to be_truthy - @page = wiki.find_page(new_title) + @page = wiki.find_page(new_title) - expect(@page).not_to be_nil - expect(@page.content).to eq new_content - end + expect(@page).not_to be_nil + expect(@page.content).to eq new_content end + end - context 'when moving a page' do - it 'raises an error if the page already exists' do - create_page('foo/Existing Page', 'content') - - expect { @page.update(title: 'foo/Existing Page', content: 'new_content') }.to raise_error(WikiPage::PageRenameError) - expect(@page.title).to eq 'Update' - expect(@page.content).to eq 'new_content' + context 'when moving a page' do + it 'raises an error if the page already exists' do + create_page('foo/Existing Page', 'content') - destroy_page('Existing Page', 'foo') - end + expect { @page.update(title: 'foo/Existing Page', content: 'new_content') }.to raise_error(WikiPage::PageRenameError) + expect(@page.title).to eq 'Update' + expect(@page.content).to eq 'new_content' - it 'updates the content and moves the file' do - new_title = 'foo/Other Page' - new_content = 'new_content' - - expect(@page.update(title: new_title, content: new_content)).to be_truthy + destroy_page('Existing Page', 'foo') + end - page = wiki.find_page(new_title) + it 'updates the content and moves the file' do + new_title = 'foo/Other Page' + new_content = 'new_content' - expect(page).not_to be_nil - expect(page.content).to eq new_content - end + expect(@page.update(title: new_title, content: new_content)).to be_truthy - context 'in subdir' do - before do - create_page('foo/Existing Page', 'content') - @page = wiki.find_page('foo/Existing Page') - end + page = wiki.find_page(new_title) - it 'moves the page to the root folder if the title is preceded by /', :skip_gitaly_mock do - expect(@page.slug).to eq 'foo/Existing-Page' - expect(@page.update(title: '/Existing Page', content: 'new_content')).to be_truthy - expect(@page.slug).to eq 'Existing-Page' - end + expect(page).not_to be_nil + expect(page.content).to eq new_content + end - it 'does nothing if it has the same title' do - original_path = @page.slug + context 'in subdir' do + before do + create_page('foo/Existing Page', 'content') + @page = wiki.find_page('foo/Existing Page') + end - expect(@page.update(title: 'Existing Page', content: 'new_content')).to be_truthy - expect(@page.slug).to eq original_path - end + it 'moves the page to the root folder if the title is preceded by /' do + expect(@page.slug).to eq 'foo/Existing-Page' + expect(@page.update(title: '/Existing Page', content: 'new_content')).to be_truthy + expect(@page.slug).to eq 'Existing-Page' end - context 'in root dir' do - it 'does nothing if the title is preceded by /' do - original_path = @page.slug + it 'does nothing if it has the same title' do + original_path = @page.slug - expect(@page.update(title: '/Update', content: 'new_content')).to be_truthy - expect(@page.slug).to eq original_path - end + expect(@page.update(title: 'Existing Page', content: 'new_content')).to be_truthy + expect(@page.slug).to eq original_path end end - context "with invalid attributes" do - it 'aborts update if title blank' do - expect(@page.update(title: '', content: 'new_content')).to be_falsey - expect(@page.content).to eq 'new_content' + context 'in root dir' do + it 'does nothing if the title is preceded by /' do + original_path = @page.slug - page = wiki.find_page('Update') - expect(page.content).to eq 'content' - - @page.title = 'Update' + expect(@page.update(title: '/Update', content: 'new_content')).to be_truthy + expect(@page.slug).to eq original_path end end end - context 'when Gitaly is enabled' do - it_behaves_like 'update method' - end + context "with invalid attributes" do + it 'aborts update if title blank' do + expect(@page.update(title: '', content: 'new_content')).to be_falsey + expect(@page.content).to eq 'new_content' - context 'when Gitaly is disabled', :skip_gitaly_mock do - it_behaves_like 'update method' + page = wiki.find_page('Update') + expect(page.content).to eq 'content' + + @page.title = 'Update' + end end end @@ -394,34 +374,24 @@ describe WikiPage do end describe "#versions" do - shared_examples 'wiki page versions' do - let(:page) { wiki.find_page("Update") } + let(:page) { wiki.find_page("Update") } - before do - create_page("Update", "content") - end - - after do - destroy_page("Update") - end - - it "returns an array of all commits for the page" do - 3.times { |i| page.update(content: "content #{i}") } - - expect(page.versions.count).to eq(4) - end + before do + create_page("Update", "content") + end - it 'returns instances of WikiPageVersion' do - expect(page.versions).to all( be_a(Gitlab::Git::WikiPageVersion) ) - end + after do + destroy_page("Update") end - context 'when Gitaly is enabled' do - it_behaves_like 'wiki page versions' + it "returns an array of all commits for the page" do + 3.times { |i| page.update(content: "content #{i}") } + + expect(page.versions.count).to eq(4) end - context 'when Gitaly is disabled', :disable_gitaly do - it_behaves_like 'wiki page versions' + it 'returns instances of WikiPageVersion' do + expect(page.versions).to all( be_a(Gitlab::Git::WikiPageVersion) ) end end @@ -555,23 +525,13 @@ describe WikiPage do end describe '#formatted_content' do - shared_examples 'fetching page formatted content' do - it 'returns processed content of the page' do - subject.create({ title: "RDoc", content: "*bold*", format: "rdoc" }) - page = wiki.find_page('RDoc') - - expect(page.formatted_content).to eq("\n<p><strong>bold</strong></p>\n") + it 'returns processed content of the page' do + subject.create({ title: "RDoc", content: "*bold*", format: "rdoc" }) + page = wiki.find_page('RDoc') - destroy_page('RDoc') - end - end - - context 'when Gitaly wiki_page_formatted_data is enabled' do - it_behaves_like 'fetching page formatted content' - end + expect(page.formatted_content).to eq("\n<p><strong>bold</strong></p>\n") - context 'when Gitaly wiki_page_formatted_data is disabled', :disable_gitaly do - it_behaves_like 'fetching page formatted content' + destroy_page('RDoc') end end diff --git a/spec/rubocop/cop/safe_params_spec.rb b/spec/rubocop/cop/safe_params_spec.rb new file mode 100644 index 00000000000..4f02b8e9008 --- /dev/null +++ b/spec/rubocop/cop/safe_params_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'rubocop' +require 'rubocop/rspec/support' +require_relative '../../../rubocop/cop/safe_params' + +describe RuboCop::Cop::SafeParams do + include CopHelper + + subject(:cop) { described_class.new } + + it 'flags the params as an argument of url_for' do + expect_offense(<<~SOURCE) + url_for(params) + ^^^^^^^^^^^^^^^ Use `safe_params` instead of `params` in url_for. + SOURCE + end + + it 'flags the merged params as an argument of url_for' do + expect_offense(<<~SOURCE) + url_for(params.merge(additional_params)) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `safe_params` instead of `params` in url_for. + SOURCE + end + + it 'flags the merged params arg as an argument of url_for' do + expect_offense(<<~SOURCE) + url_for(something.merge(additional).merge(params)) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `safe_params` instead of `params` in url_for. + SOURCE + end + + it 'does not flag other argument of url_for' do + expect_no_offenses(<<~SOURCE) + url_for(something) + SOURCE + end +end diff --git a/spec/support/gitaly.rb b/spec/support/gitaly.rb deleted file mode 100644 index 614aaa73693..00000000000 --- a/spec/support/gitaly.rb +++ /dev/null @@ -1,16 +0,0 @@ -RSpec.configure do |config| - config.before(:each) do |example| - if example.metadata[:disable_gitaly] - # Use 'and_wrap_original' to make sure the arguments are valid - allow(Gitlab::GitalyClient).to receive(:feature_enabled?).and_wrap_original { |m, *args| m.call(*args) && false } - else - next if example.metadata[:skip_gitaly_mock] - - # Use 'and_wrap_original' to make sure the arguments are valid - allow(Gitlab::GitalyClient).to receive(:feature_enabled?).and_wrap_original do |m, *args| - m.call(*args) - !Gitlab::GitalyClient.explicit_opt_in_required.include?(args.first) - end - end - end -end |