Commit 16346eb5 authored by Zeger-Jan van de Weg's avatar Zeger-Jan van de Weg

Revert "Merge branch 'revert-e2aa2177' into 'master'"

This reverts merge request !23229
parent fa5900fb
---
title: Show what RPC is called in the performance bar
merge_request: 23140
author:
type: other
# frozen_string_literal: true # frozen_string_literal: true
# Gitaly note: JV: seems to be completely migrated (behind feature flags).
module Gitlab module Gitlab
module Git module Git
class Blob class Blob
......
...@@ -885,12 +885,6 @@ module Gitlab ...@@ -885,12 +885,6 @@ module Gitlab
Gitlab::GitalyClient::ConflictsService.new(self, our_commit_oid, their_commit_oid) Gitlab::GitalyClient::ConflictsService.new(self, our_commit_oid, their_commit_oid)
end 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 def clean_stale_repository_files
wrapped_gitaly_errors do wrapped_gitaly_errors do
gitaly_repository_client.cleanup if exists? gitaly_repository_client.cleanup if exists?
......
...@@ -9,11 +9,6 @@ require 'grpc/health/v1/health_services_pb' ...@@ -9,11 +9,6 @@ require 'grpc/health/v1/health_services_pb'
module Gitlab module Gitlab
module GitalyClient module GitalyClient
include Gitlab::Metrics::Methods include Gitlab::Metrics::Methods
module MigrationStatus
DISABLED = 1
OPT_IN = 2
OPT_OUT = 3
end
class TooManyInvocationsError < StandardError class TooManyInvocationsError < StandardError
attr_reader :call_site, :invocation_count, :max_call_stack attr_reader :call_site, :invocation_count, :max_call_stack
...@@ -31,7 +26,7 @@ module Gitlab ...@@ -31,7 +26,7 @@ module Gitlab
end end
end end
SERVER_VERSION_FILE = 'GITALY_SERVER_VERSION'.freeze SERVER_VERSION_FILE = 'GITALY_SERVER_VERSION'
MAXIMUM_GITALY_CALLS = 35 MAXIMUM_GITALY_CALLS = 35
CLIENT_NAME = (Sidekiq.server? ? 'gitlab-sidekiq' : 'gitlab-web').freeze CLIENT_NAME = (Sidekiq.server? ? 'gitlab-sidekiq' : 'gitlab-web').freeze
...@@ -43,11 +38,6 @@ module Gitlab ...@@ -43,11 +38,6 @@ module Gitlab
self.query_time = 0 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 define_histogram :gitaly_controller_action_duration_seconds do
docstring "Gitaly endpoint histogram by controller and action combination" docstring "Gitaly endpoint histogram by controller and action combination"
base_labels Gitlab::Metrics::Transaction::BASE_LABELS.merge(gitaly_service: nil, rpc: nil) base_labels Gitlab::Metrics::Transaction::BASE_LABELS.merge(gitaly_service: nil, rpc: nil)
...@@ -126,7 +116,6 @@ module Gitlab ...@@ -126,7 +116,6 @@ module Gitlab
def self.call(storage, service, rpc, request, remote_storage: nil, timeout: nil) def self.call(storage, service, rpc, request, remote_storage: nil, timeout: nil)
start = Gitlab::Metrics::System.monotonic_time start = Gitlab::Metrics::System.monotonic_time
request_hash = request.is_a?(Google::Protobuf::MessageExts) ? request.to_h : {} request_hash = request.is_a?(Google::Protobuf::MessageExts) ? request.to_h : {}
@current_call_id ||= SecureRandom.uuid
enforce_gitaly_request_limits(:call) enforce_gitaly_request_limits(:call)
...@@ -145,9 +134,7 @@ module Gitlab ...@@ -145,9 +134,7 @@ module Gitlab
current_transaction_labels.merge(gitaly_service: service.to_s, rpc: rpc.to_s), current_transaction_labels.merge(gitaly_service: service.to_s, rpc: rpc.to_s),
duration) duration)
add_call_details(id: @current_call_id, feature: service, duration: duration, request: request_hash) add_call_details(feature: "#{service}##{rpc}", duration: duration, request: request_hash, rpc: rpc)
@current_call_id = nil
end end
def self.handle_grpc_unavailable!(ex) def self.handle_grpc_unavailable!(ex)
...@@ -222,7 +209,7 @@ module Gitlab ...@@ -222,7 +209,7 @@ module Gitlab
result result
end end
SERVER_FEATURE_FLAGS = %w[gogit_findcommit].freeze SERVER_FEATURE_FLAGS = %w[].freeze
def self.server_feature_flags def self.server_feature_flags
SERVER_FEATURE_FLAGS.map do |f| SERVER_FEATURE_FLAGS.map do |f|
...@@ -237,82 +224,8 @@ module Gitlab ...@@ -237,82 +224,8 @@ module Gitlab
params['gitaly_token'].presence || Gitlab.config.gitaly['token'] params['gitaly_token'].presence || Gitlab.config.gitaly['token']
end end
# Evaluates whether a feature toggle is on or off def self.feature_enabled?(feature_name)
def self.feature_enabled?(feature_name, status: MigrationStatus::OPT_IN) Feature.enabled?("gitaly_#{feature_name}")
# 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
end end
# Ensures that Gitaly is not being abuse through n+1 misuse etc # Ensures that Gitaly is not being abuse through n+1 misuse etc
...@@ -368,38 +281,20 @@ module Gitlab ...@@ -368,38 +281,20 @@ module Gitlab
end end
private_class_method :decrement_call_count private_class_method :decrement_call_count
# Returns an estimate of the number of Gitaly calls made for this # Returns the of the number of Gitaly calls made for this request
# request
def self.get_request_count def self.get_request_count
return 0 unless Gitlab::SafeRequestStore.active? get_call_count("gitaly_call_actual")
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
end end
def self.reset_counts def self.reset_counts
return unless Gitlab::SafeRequestStore.active? return unless Gitlab::SafeRequestStore.active?
%w[migrate call].each do |call_site| Gitlab::SafeRequestStore["gitaly_call_actual"] = 0
Gitlab::SafeRequestStore["gitaly_#{call_site}_actual"] = 0 Gitlab::SafeRequestStore["gitaly_call_permitted"] = 0
Gitlab::SafeRequestStore["gitaly_#{call_site}_permitted"] = 0
end
end end
def self.add_call_details(details) def self.add_call_details(details)
id = details.delete(:id) return unless Gitlab::SafeRequestStore[:peek_enabled]
return unless id && Gitlab::SafeRequestStore[:peek_enabled]
Gitlab::SafeRequestStore['gitaly_call_details'] ||= {} Gitlab::SafeRequestStore['gitaly_call_details'] ||= {}
Gitlab::SafeRequestStore['gitaly_call_details'][id] ||= {} Gitlab::SafeRequestStore['gitaly_call_details'][id] ||= {}
......
require 'spec_helper' require 'spec_helper'
describe 'User views a wiki page' do describe 'User views a wiki page' do
shared_examples 'wiki page user view' do include WikiHelpers
include WikiHelpers
let(:user) { create(:user) }
let(:user) { create(:user) } let(:project) { create(:project, :wiki_repo, namespace: user.namespace) }
let(:project) { create(:project, :wiki_repo, namespace: user.namespace) } let(:path) { 'image.png' }
let(:path) { 'image.png' } let(:wiki_page) do
let(:wiki_page) do create(:wiki_page,
create(:wiki_page, wiki: project.wiki,
wiki: project.wiki, attrs: { title: 'home', content: "Look at this [image](#{path})\n\n ![alt text](#{path})" })
attrs: { title: 'home', content: "Look at this [image](#{path})\n\n ![alt text](#{path})" }) end
end
before do before do
project.add_maintainer(user) project.add_maintainer(user)
sign_in(user) sign_in(user)
end end
context 'when wiki is empty' do context 'when wiki is empty' do
before do before do
visit(project_wikis_path(project)) visit(project_wikis_path(project))
click_link "Create your first page" click_link "Create your first page"
click_on('New page') click_on('New page')
page.within('#modal-new-wiki') do page.within('#modal-new-wiki') do
fill_in(:new_wiki_path, with: 'one/two/three-test') fill_in(:new_wiki_path, with: 'one/two/three-test')
click_on('Create page') click_on('Create page')
end end
page.within('.wiki-form') do page.within('.wiki-form') do
fill_in(:wiki_content, with: 'wiki content') fill_in(:wiki_content, with: 'wiki content')
click_on('Create page') click_on('Create page')
end
end end
end
it 'shows the history of a page that has a path', :js do it 'shows the history of a page that has a path', :js do
expect(current_path).to include('one/two/three-test') expect(current_path).to include('one/two/three-test')
first(:link, text: 'Three').click first(:link, text: 'Three').click
click_on('Page history') 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 page.within(:css, '.nav-text') do
expect(page).to have_content('History') expect(page).to have_content('History')
end
end end
end
it 'shows an old version of a page', :js do it 'shows an old version of a page', :js do
expect(current_path).to include('one/two/three-test') expect(current_path).to include('one/two/three-test')
expect(find('.wiki-pages')).to have_content('Three') expect(find('.wiki-pages')).to have_content('Three')
first(:link, text: 'Three').click
expect(find('.nav-text')).to have_content('Three')
click_on('Edit') first(:link, text: 'Three').click
expect(current_path).to include('one/two/three-test') expect(find('.nav-text')).to have_content('Three')
expect(page).to have_content('Edit Page')
fill_in('Content', with: 'Updated Wiki Content') click_on('Edit')
click_on('Save changes') expect(current_path).to include('one/two/three-test')
click_on('Page history') expect(page).to have_content('Edit Page')
page.within(:css, '.nav-text') do fill_in('Content', with: 'Updated Wiki Content')
expect(page).to have_content('History')
end
find('a[href*="?version_id"]') click_on('Save changes')
end click_on('Page history')
end
context 'when a page does not have history' do
before do
visit(project_wiki_path(project, wiki_page))
end
it 'shows all the pages' do page.within(:css, '.nav-text') do
expect(page).to have_content(user.name) expect(page).to have_content('History')
expect(find('.wiki-pages')).to have_content(wiki_page.title.capitalize)
end end
context 'shows a file stored in a page' do find('a[href*="?version_id"]')
let(:path) { upload_file_to_wiki(project, user, 'dk.png') } end
end
it do context 'when a page does not have history' do
expect(page).to have_xpath("//img[@data-src='#{project.wiki.wiki_base_path}/#{path}']") before do
expect(page).to have_link('image', href: "#{project.wiki.wiki_base_path}/#{path}") 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}") context 'shows a file stored in a page' do
expect(page).not_to have_xpath('/html') # Page should render the image which means there is no html involved let(:path) { upload_file_to_wiki(project, user, 'dk.png') }
end
end
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}") expect(page).to have_link('image', href: "#{project.wiki.wiki_base_path}/#{path}")
click_on('image') click_on('image')
expect(current_path).to match("wikis/#{path}") expect(current_path).to match("wikis/#{path}")
expect(page).to have_content('New Wiki Page') expect(page).not_to have_xpath('/html') # Page should render the image which means there is no html involved
expect(page).to have_content('Create page')
end end
end end
context 'when a page has history' do it 'shows the creation page if file does not exist' do
before do expect(page).to have_link('image', href: "#{project.wiki.wiki_base_path}/#{path}")
wiki_page.update(message: 'updated home', content: 'updated [some link](other-page)')
end
it 'shows the page history' do click_on('image')
visit(project_wiki_path(project, wiki_page))
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) it 'shows the page history' do
expect(page).to have_content("#{user.username} created page: home") visit(project_wiki_path(project, wiki_page))
expect(page).to have_content('updated home')
end
it 'does not show the "Edit" button' do expect(page).to have_selector('a.btn', text: 'Edit')
visit(project_wiki_path(project, wiki_page, version_id: wiki_page.versions.last.id))
expect(page).not_to have_selector('a.btn', text: 'Edit') click_on('Page history')
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 end
context 'when page has invalid content encoding' do it 'does not show the "Edit" button' do
let(:content) { 'whatever'.force_encoding('ISO-8859-1') } visit(project_wiki_path(project, wiki_page, version_id: wiki_page.versions.last.id))
before do expect(page).not_to have_selector('a.btn', text: 'Edit')
allow(Gitlab::EncodingHelper).to receive(:encode!).and_return(content) end
end
visit(project_wiki_path(project, wiki_page)) context 'when page has invalid content encoding' do
end let(:content) { 'whatever'.force_encoding('ISO-8859-1') }
it 'does not show "Edit" button' do before do
expect(page).not_to have_selector('a.btn', text: 'Edit') allow(Gitlab::EncodingHelper).to receive(:encode!).and_return(content)
end
it 'shows error' do visit(project_wiki_path(project, wiki_page))
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 end
it 'opens a default wiki page', :js do it 'does not show "Edit" button' do
visit(project_path(project)) expect(page).not_to have_selector('a.btn', text: 'Edit')
end
find('.shortcuts-wiki').click
click_link "Create your first page"
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
end end
context 'when Gitaly is enabled' do it 'opens a default wiki page', :js do
it_behaves_like 'wiki page user view' visit(project_path(project))
end
find('.shortcuts-wiki').click
click_link "Create your first page"
context 'when Gitaly is disabled', :skip_gitaly_mock do expect(page).to have_content('Home · Create Page')
it_behaves_like 'wiki page user view'
end end
end end
...@@ -205,28 +205,18 @@ describe ExtractsPath do ...@@ -205,28 +205,18 @@ describe ExtractsPath do
end end
describe '#lfs_blob_ids' do 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(:tag) { @project.repository.add_tag(@project.owner, 'my-annotated-tag', 'master', 'test tag') } let(:ref) { tag.target }
let(:ref) { tag.target } let(:params) { { ref: ref, path: 'README.md' } }
let(:params) { { ref: ref, path: 'README.md' } }
before do before do
@project = create(:project, :repository) @project = create(:project, :repository)
end
it 'handles annotated tags' do
assign_ref_vars
expect(lfs_blob_ids).to eq([])
end
end end
context 'when gitaly is enabled' do it 'handles annotated tags' do
it_behaves_like '#lfs_blob_ids' assign_ref_vars
end
context 'when gitaly is disabled', :skip_gitaly_mock do expect(lfs_blob_ids).to eq([])
it_behaves_like '#lfs_blob_ids'
end end
end end
end end
...@@ -37,17 +37,7 @@ describe Gitlab::Diff::FileCollection::MergeRequestDiff do ...@@ -37,17 +37,7 @@ describe Gitlab::Diff::FileCollection::MergeRequestDiff do
let(:stub_path) { '.gitignore' } let(:stub_path) { '.gitignore' }
end end
shared_examples 'initializes a DiffCollection' do it 'returns a valid instance of a DiffCollection' do
it 'returns a valid instance of a DiffCollection' do expect(diff_files).to be_a(Gitlab::Git::DiffCollection)
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'
end end
end end
...@@ -128,7 +128,7 @@ describe Gitlab::Git::Blob, :seed_helper do ...@@ -128,7 +128,7 @@ describe Gitlab::Git::Blob, :seed_helper do
end end
end end
shared_examples 'finding blobs by ID' do describe '.raw' do
let(:raw_blob) { Gitlab::Git::Blob.raw(repository, SeedRepo::RubyBlob::ID) } let(:raw_blob) { Gitlab::Git::Blob.raw(repository, SeedRepo::RubyBlob::ID) }
let(:bad_blob) { Gitlab::Git::Blob.raw(repository, SeedRepo::BigCommit::ID) } let(:bad_blob) { Gitlab::Git::Blob.raw(repository, SeedRepo::BigCommit::ID) }
...@@ -166,16 +166,6 @@ describe Gitlab::Git::Blob, :seed_helper do ...@@ -166,16 +166,6 @@ describe Gitlab::Git::Blob, :seed_helper do
end end
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 describe '.batch' do
let(:blob_references) do let(:blob_references) do
[ [
......
...@@ -183,110 +183,100 @@ describe Gitlab::Git::Commit, :seed_helper do ...@@ -183,110 +183,100 @@ describe Gitlab::Git::Commit, :seed_helper do
end end
end end
shared_examples '.where' do context 'path is empty string' do
context 'path is empty string' do subject do
subject do commits = described_class.where(
commits = described_class.where( repo: repository,
repo: repository, ref: 'master',
ref: 'master', path: '',
path: '', limit: 10
limit: 10 )
)
commits.map { |c| c.id }
end
it 'has 10 elements' do commits.map { |c| c.id }
expect(subject.size).to eq(10)
end
it { is_expected.to include(SeedRepo::EmptyCommit::ID) }
end end
context 'path is nil' do it 'has 10 elements' do
subject do expect(subject.size).to eq(10)
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) }
end end
it { is_expected.to include(SeedRepo::EmptyCommit::ID) }
end
context 'ref is branch name' do context 'path is nil' do
subject do subject do
commits = described_class.where( commits = described_class.where(
repo: repository, repo: repository,
ref: 'master', ref: 'master',
path: 'files', path: nil,
limit: 3, limit: 10
offset: 1 )
)
commits.map { |c| c.id } commits.map { |c| c.id }
end end
it 'has 3 elements' do it 'has 10 elements' do
expect(subject.size).to eq(3) expect(subject.size).to eq(10)
end
it { is_expected.to include("d14d6c0abdd253381df51a723d58691b2ee1ab08") }
it { is_expected.not_to include("eb49186cfa5c4338011f5f590fac11bd66c5c631") }
end end
it { is_expected.to include(SeedRepo::EmptyCommit::ID) }
end
context 'ref is commit id' do context 'ref is branch name' do
subject do subject do
commits = described_class.where( commits = described_class.where(
repo: repository, repo: repository,
ref: "874797c3a73b60d2187ed6e2fcabd289ff75171e", ref: 'master',
path: 'files', path: 'files',
limit: 3, limit: 3,
offset: 1 offset: 1
) )
commits.map { |c| c.id } commits.map { |c| c.id }
end end
it 'has 3 elements' do it 'has 3 elements' do
expect(subject.size).to eq(3) expect(subject.size).to eq(3)
end
it { is_expected.to include("2f63565e7aac07bcdadb654e253078b727143ec4") }
it { is_expected.not_to include(SeedRepo::Commit::ID) }
end end
it { is_expected.to include("d14d6c0abdd253381df51a723d58691b2ee1ab08") }
it { is_expected.not_to include("eb49186cfa5c4338011f5f590fac11bd66c5c631") }
end
context 'ref is tag' do context 'ref is commit id' do
subject do subject do
commits = described_class.where( commits = described_class.where(
repo: repository, repo: repository,
ref: 'v1.0.0', ref: "874797c3a73b60d2187ed6e2fcabd289ff75171e",
path: 'files', path: 'files',
limit: 3, limit: 3,
offset: 1 offset: 1
) )
commits.map { |c| c.id } commits.map { |c| c.id }
end end
it 'has 3 elements' do it 'has 3 elements' do
expect(subject.size).to eq(3) expect(subject.size).to eq(3)
end
it { is_expected.to include("874797c3a73b60d2187ed6e2fcabd289ff75171e") }
it { is_expected.not_to include(SeedRepo::Commit::ID) }
end end
it { is_expected.to include("2f63565e7aac07bcdadb654e253078b727143ec4") }
it { is_expected.not_to include(SeedRepo::Commit::ID) }
end end
describe '.where with gitaly' do context 'ref is tag' do
it_should_behave_like '.where' subject do
end commits = described_class.where(
repo: repository,
ref: 'v1.0.0',
path: 'files',
limit: 3,
offset: 1
)
describe '.where without gitaly', :skip_gitaly_mock do commits.map { |c| c.id }
it_should_behave_like '.where' 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 end
describe '.between' do describe '.between' do
...@@ -508,7 +498,7 @@ describe Gitlab::Git::Commit, :seed_helper do ...@@ -508,7 +498,7 @@ describe Gitlab::Git::Commit, :seed_helper do
end end
end end
shared_examples '#stats' do describe '#stats' do
subject { commit.stats } subject { commit.stats }
describe '#additions' do describe '#additions' do
...@@ -527,14 +517,6 @@ describe Gitlab::Git::Commit, :seed_helper do ...@@ -527,14 +517,6 @@ describe Gitlab::Git::Commit, :seed_helper do
end end
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 describe '#has_zero_stats?' do
it { expect(commit.has_zero_stats?).to eq(false) } it { expect(commit.has_zero_stats?).to eq(false) }
end end
...@@ -577,25 +559,15 @@ describe Gitlab::Git::Commit, :seed_helper do ...@@ -577,25 +559,15 @@ describe Gitlab::Git::Commit, :seed_helper do
commit_ids.map { |id| described_class.get_message(repository, id) } commit_ids.map { |id| described_class.get_message(repository, id) }
end end
shared_examples 'getting commit messages' do it 'gets commit messages' do
it 'gets commit messages' do expect(subject).to contain_exactly(
expect(subject).to contain_exactly( "Added contributing guide\n\nSigned-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>\n",
"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"
"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
end end
context 'when Gitaly commit_messages feature is disabled', :disable_gitaly do it 'gets messages in one batch', :request_store do
it_behaves_like 'getting commit messages' expect { subject.map(&:itself) }.to change { Gitlab::GitalyClient.get_request_count }.by(1)
end end
end end
......
...@@ -3,7 +3,7 @@ require "spec_helper" ...@@ -3,7 +3,7 @@ require "spec_helper"
describe Gitlab::Git::Tag, :seed_helper do describe Gitlab::Git::Tag, :seed_helper do
let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') }
shared_examples 'Gitlab::Git::Repository#tags' do describe '#tags' do
describe 'first tag' do describe 'first tag' do
let(:tag) { repository.tags.first } let(:tag) { repository.tags.first }
...@@ -25,14 +25,6 @@ describe Gitlab::Git::Tag, :seed_helper do ...@@ -25,14 +25,6 @@ describe Gitlab::Git::Tag, :seed_helper do
it { expect(repository.tags.size).to eq(SeedRepo::Repo::TAGS.size) } it { expect(repository.tags.size).to eq(SeedRepo::Repo::TAGS.size) }
end 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 describe '.get_message' do
let(:tag_ids) { %w[f4e6814c3e4e7a0de82a9e7cd20c626cc963a2f8 8a2a6eb295bb170b34c24c76c49ed0e9b2eaf34b] } let(:tag_ids) { %w[f4e6814c3e4e7a0de82a9e7cd20c626cc963a2f8 8a2a6eb295bb170b34c24c76c49ed0e9b2eaf34b] }
...@@ -40,23 +32,13 @@ describe Gitlab::Git::Tag, :seed_helper do ...@@ -40,23 +32,13 @@ describe Gitlab::Git::Tag, :seed_helper do
tag_ids.map { |id| described_class.get_message(repository, id) } tag_ids.map { |id| described_class.get_message(repository, id) }
end end
shared_examples 'getting tag messages' do it 'gets tag messages' do
it 'gets tag messages' do expect(subject[0]).to eq("Release\n")
expect(subject[0]).to eq("Release\n") expect(subject[1]).to eq("Version 1.1.0\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
end end
context 'when Gitaly tag_messages feature is disabled', :disable_gitaly do it 'gets messages in one batch', :request_store do
it_behaves_like 'getting tag messages' expect { subject.map(&:itself) }.to change { Gitlab::GitalyClient.get_request_count }.by(1)
end end
end end
......
...@@ -80,18 +80,8 @@ describe Gitlab::Git::Tree, :seed_helper do ...@@ -80,18 +80,8 @@ describe Gitlab::Git::Tree, :seed_helper do
end end
describe '#where' do describe '#where' do
shared_examples '#where' do it 'returns an empty array when called with an invalid ref' do
it 'returns an empty array when called with an invalid ref' do expect(described_class.where(repository, 'foobar-does-not-exist')).to eq([])
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'
end end
end end
end end
...@@ -2,7 +2,7 @@ require 'spec_helper' ...@@ -2,7 +2,7 @@ require 'spec_helper'
# We stub Gitaly in `spec/support/gitaly.rb` for other tests. We don't want # We stub Gitaly in `spec/support/gitaly.rb` for other tests. We don't want
# those stubs while testing the GitalyClient itself. # those stubs while testing the GitalyClient itself.
describe Gitlab::GitalyClient, skip_gitaly_mock: true do describe Gitlab::GitalyClient do
describe '.stub_class' do describe '.stub_class' do
it 'returns the gRPC health check stub' do it 'returns the gRPC health check stub' do
expect(described_class.stub_class(:health_check)).to eq(::Grpc::Health::V1::Health::Stub) 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 ...@@ -191,102 +191,13 @@ describe Gitlab::GitalyClient, skip_gitaly_mock: true do
let(:feature_name) { 'my_feature' } let(:feature_name) { 'my_feature' }
let(:real_feature_name) { "gitaly_#{feature_name}" } let(:real_feature_name) { "gitaly_#{feature_name}" }
context 'when Gitaly is disabled' do before do
before do allow(Feature).to receive(:enabled?).and_return(false)
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
end end
context 'when the feature_status is OPT_OUT' do it 'returns false' do
let(:feature_status) { Gitlab::GitalyClient::MigrationStatus::OPT_OUT } expect(Feature).to receive(:enabled?).with(real_feature_name)
expect(described_class.feature_enabled?(feature_name)).to be(false)
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
end end
end end
......
...@@ -30,48 +30,38 @@ describe MergeRequest do ...@@ -30,48 +30,38 @@ describe MergeRequest do
end end
describe '#squash_in_progress?' do describe '#squash_in_progress?' do
shared_examples 'checking whether a squash is in progress' do let(:repo_path) do
let(:repo_path) do Gitlab::GitalyClient::StorageSettings.allow_disk_access do
Gitlab::GitalyClient::StorageSettings.allow_disk_access do subject.source_project.repository.path
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
end end
end
let(:squash_path) { File.join(repo_path, "gitlab-worktree", "squash-#{subject.id}") }
it 'returns false when there is no squash directory' do before do
FileUtils.rm_rf(squash_path) 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 it 'returns true when there is a current squash directory' do
end expect(subject.squash_in_progress?).to be_truthy
end
it 'returns false when the squash directory has expired' do it 'returns false when there is no squash directory' do
time = 20.minutes.ago.to_time FileUtils.rm_rf(squash_path)
File.utime(time, time, squash_path)
expect(subject.squash_in_progress?).to be_falsey expect(subject.squash_in_progress?).to be_falsey
end end
it 'returns false when the source project has been removed' do it 'returns false when the squash directory has expired' do
allow(subject).to receive(:source_project).and_return(nil) time = 20.minutes.ago.to_time
File.utime(time, time, squash_path)
expect(subject.squash_in_progress?).to be_falsey expect(subject.squash_in_progress?).to be_falsey
end
end end
context 'when Gitaly squash_in_progress is enabled' do it 'returns false when the source project has been removed' do
it_behaves_like 'checking whether a squash is in progress' allow(subject).to receive(:source_project).and_return(nil)
end
context 'when Gitaly squash_in_progress is disabled', :disable_gitaly do expect(subject.squash_in_progress?).to be_falsey
it_behaves_like 'checking whether a squash is in progress'
end end
end end
...@@ -2587,14 +2577,6 @@ describe MergeRequest do ...@@ -2587,14 +2577,6 @@ describe MergeRequest do
expect(subject.rebase_in_progress?).to be_falsey expect(subject.rebase_in_progress?).to be_falsey
end end
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 end
describe '#allow_collaboration' do describe '#allow_collaboration' do
......
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment