summaryrefslogtreecommitdiff
path: root/spec
diff options
context:
space:
mode:
Diffstat (limited to 'spec')
-rw-r--r--spec/features/projects/badges/coverage_spec.rb4
-rw-r--r--spec/features/projects/pipeline_schedules_spec.rb30
-rw-r--r--spec/javascripts/ci_variable_list/ci_variable_list_spec.js163
-rw-r--r--spec/javascripts/ci_variable_list/native_form_variable_list_spec.js30
-rw-r--r--spec/javascripts/fixtures/pipeline_schedules.rb43
-rw-r--r--spec/javascripts/pipeline_schedules/setup_pipeline_variable_list_spec.js145
-rw-r--r--spec/lib/gitlab/badge/coverage/template_spec.rb18
-rw-r--r--spec/lib/gitlab/git/repository_spec.rb25
-rw-r--r--spec/lib/gitlab/query_limiting/active_support_subscriber_spec.rb19
-rw-r--r--spec/lib/gitlab/query_limiting/middleware_spec.rb72
-rw-r--r--spec/lib/gitlab/query_limiting/transaction_spec.rb144
-rw-r--r--spec/lib/gitlab/query_limiting_spec.rb65
-rw-r--r--spec/models/repository_spec.rb61
-rw-r--r--spec/requests/api/groups_spec.rb15
14 files changed, 644 insertions, 190 deletions
diff --git a/spec/features/projects/badges/coverage_spec.rb b/spec/features/projects/badges/coverage_spec.rb
index 821ce88a402..f51001edcd7 100644
--- a/spec/features/projects/badges/coverage_spec.rb
+++ b/spec/features/projects/badges/coverage_spec.rb
@@ -18,7 +18,7 @@ feature 'test coverage badge' do
show_test_coverage_badge
- expect_coverage_badge('95%')
+ expect_coverage_badge('95.00%')
end
scenario 'user requests coverage badge for specific job' do
@@ -30,7 +30,7 @@ feature 'test coverage badge' do
show_test_coverage_badge(job: 'coverage')
- expect_coverage_badge('85%')
+ expect_coverage_badge('85.00%')
end
scenario 'user requests coverage badge for pipeline without coverage' do
diff --git a/spec/features/projects/pipeline_schedules_spec.rb b/spec/features/projects/pipeline_schedules_spec.rb
index fa2f7a1fd78..65e24862d43 100644
--- a/spec/features/projects/pipeline_schedules_spec.rb
+++ b/spec/features/projects/pipeline_schedules_spec.rb
@@ -168,11 +168,11 @@ feature 'Pipeline Schedules', :js do
scenario 'user sees the new variable in edit window' do
find(".content-list .pipeline-schedule-table-row:nth-child(1) .btn-group a[title='Edit']").click
- page.within('.pipeline-variable-list') do
- expect(find(".pipeline-variable-row:nth-child(1) .pipeline-variable-key-input").value).to eq('AAA')
- expect(find(".pipeline-variable-row:nth-child(1) .pipeline-variable-value-input").value).to eq('AAA123')
- expect(find(".pipeline-variable-row:nth-child(2) .pipeline-variable-key-input").value).to eq('BBB')
- expect(find(".pipeline-variable-row:nth-child(2) .pipeline-variable-value-input").value).to eq('BBB123')
+ page.within('.ci-variable-list') do
+ expect(find(".ci-variable-row:nth-child(1) .js-ci-variable-input-key").value).to eq('AAA')
+ expect(find(".ci-variable-row:nth-child(1) .js-ci-variable-input-value", visible: false).value).to eq('AAA123')
+ expect(find(".ci-variable-row:nth-child(2) .js-ci-variable-input-key").value).to eq('BBB')
+ expect(find(".ci-variable-row:nth-child(2) .js-ci-variable-input-value", visible: false).value).to eq('BBB123')
end
end
end
@@ -185,16 +185,18 @@ feature 'Pipeline Schedules', :js do
visit_pipelines_schedules
find(".content-list .pipeline-schedule-table-row:nth-child(1) .btn-group a[title='Edit']").click
- all('[name="schedule[variables_attributes][][key]"]')[0].set('foo')
- all('[name="schedule[variables_attributes][][value]"]')[0].set('bar')
+
+ find('.js-ci-variable-list-section .js-secret-value-reveal-button').click
+ first('.js-ci-variable-input-key').set('foo')
+ first('.js-ci-variable-input-value').set('bar')
click_button 'Save pipeline schedule'
end
scenario 'user sees the updated variable in edit window' do
find(".content-list .pipeline-schedule-table-row:nth-child(1) .btn-group a[title='Edit']").click
- page.within('.pipeline-variable-list') do
- expect(find(".pipeline-variable-row:nth-child(1) .pipeline-variable-key-input").value).to eq('foo')
- expect(find(".pipeline-variable-row:nth-child(1) .pipeline-variable-value-input").value).to eq('bar')
+ page.within('.ci-variable-list') do
+ expect(find(".ci-variable-row:nth-child(1) .js-ci-variable-input-key").value).to eq('foo')
+ expect(find(".ci-variable-row:nth-child(1) .js-ci-variable-input-value", visible: false).value).to eq('bar')
end
end
end
@@ -207,15 +209,15 @@ feature 'Pipeline Schedules', :js do
visit_pipelines_schedules
find(".content-list .pipeline-schedule-table-row:nth-child(1) .btn-group a[title='Edit']").click
- find('.pipeline-variable-list .pipeline-variable-row-remove-button').click
+ find('.ci-variable-list .ci-variable-row-remove-button').click
click_button 'Save pipeline schedule'
end
scenario 'user does not see the removed variable in edit window' do
find(".content-list .pipeline-schedule-table-row:nth-child(1) .btn-group a[title='Edit']").click
- page.within('.pipeline-variable-list') do
- expect(find(".pipeline-variable-row:nth-child(1) .pipeline-variable-key-input").value).to eq('')
- expect(find(".pipeline-variable-row:nth-child(1) .pipeline-variable-value-input").value).to eq('')
+ page.within('.ci-variable-list') do
+ expect(find(".ci-variable-row:nth-child(1) .js-ci-variable-input-key").value).to eq('')
+ expect(find(".ci-variable-row:nth-child(1) .js-ci-variable-input-value", visible: false).value).to eq('')
end
end
end
diff --git a/spec/javascripts/ci_variable_list/ci_variable_list_spec.js b/spec/javascripts/ci_variable_list/ci_variable_list_spec.js
new file mode 100644
index 00000000000..0170ab458d4
--- /dev/null
+++ b/spec/javascripts/ci_variable_list/ci_variable_list_spec.js
@@ -0,0 +1,163 @@
+import VariableList from '~/ci_variable_list/ci_variable_list';
+import getSetTimeoutPromise from '../helpers/set_timeout_promise_helper';
+
+describe('VariableList', () => {
+ preloadFixtures('pipeline_schedules/edit.html.raw');
+ preloadFixtures('pipeline_schedules/edit_with_variables.html.raw');
+
+ let $wrapper;
+ let variableList;
+
+ describe('with only key/value inputs', () => {
+ describe('with no variables', () => {
+ beforeEach(() => {
+ loadFixtures('pipeline_schedules/edit.html.raw');
+ $wrapper = $('.js-ci-variable-list-section');
+
+ variableList = new VariableList({
+ container: $wrapper,
+ formField: 'schedule',
+ });
+ variableList.init();
+ });
+
+ it('should remove the row when clicking the remove button', () => {
+ $wrapper.find('.js-row-remove-button').trigger('click');
+
+ expect($wrapper.find('.js-row').length).toBe(0);
+ });
+
+ it('should add another row when editing the last rows key input', () => {
+ const $row = $wrapper.find('.js-row');
+ $row.find('.js-ci-variable-input-key')
+ .val('foo')
+ .trigger('input');
+
+ expect($wrapper.find('.js-row').length).toBe(2);
+
+ // Check for the correct default in the new row
+ const $keyInput = $wrapper.find('.js-row:last-child').find('.js-ci-variable-input-key');
+ expect($keyInput.val()).toBe('');
+ });
+
+ it('should add another row when editing the last rows value textarea', () => {
+ const $row = $wrapper.find('.js-row');
+ $row.find('.js-ci-variable-input-value')
+ .val('foo')
+ .trigger('input');
+
+ expect($wrapper.find('.js-row').length).toBe(2);
+
+ // Check for the correct default in the new row
+ const $valueInput = $wrapper.find('.js-row:last-child').find('.js-ci-variable-input-key');
+ expect($valueInput.val()).toBe('');
+ });
+
+ it('should remove empty row after blurring', () => {
+ const $row = $wrapper.find('.js-row');
+ $row.find('.js-ci-variable-input-key')
+ .val('foo')
+ .trigger('input');
+
+ expect($wrapper.find('.js-row').length).toBe(2);
+
+ $row.find('.js-ci-variable-input-key')
+ .val('')
+ .trigger('input')
+ .trigger('blur');
+
+ expect($wrapper.find('.js-row').length).toBe(1);
+ });
+ });
+
+ describe('with persisted variables', () => {
+ beforeEach(() => {
+ loadFixtures('pipeline_schedules/edit_with_variables.html.raw');
+ $wrapper = $('.js-ci-variable-list-section');
+
+ variableList = new VariableList({
+ container: $wrapper,
+ formField: 'schedule',
+ });
+ variableList.init();
+ });
+
+ it('should have "Reveal values" button initially when there are already variables', () => {
+ expect($wrapper.find('.js-secret-value-reveal-button').text()).toBe('Reveal values');
+ });
+
+ it('should reveal hidden values', () => {
+ const $row = $wrapper.find('.js-row:first-child');
+ const $inputValue = $row.find('.js-ci-variable-input-value');
+ const $placeholder = $row.find('.js-secret-value-placeholder');
+
+ expect($placeholder.hasClass('hide')).toBe(false);
+ expect($inputValue.hasClass('hide')).toBe(true);
+
+ // Reveal values
+ $wrapper.find('.js-secret-value-reveal-button').click();
+
+ expect($placeholder.hasClass('hide')).toBe(true);
+ expect($inputValue.hasClass('hide')).toBe(false);
+ });
+ });
+ });
+
+ describe('with all inputs(key, value, protected)', () => {
+ beforeEach(() => {
+ // This markup will be replaced with a fixture when we can render the
+ // CI/CD settings page with the new dynamic variable list in https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/4110
+ $wrapper = $(`<form class="js-variable-list">
+ <ul>
+ <li class="js-row">
+ <div class="ci-variable-body-item">
+ <input class="js-ci-variable-input-key" name="variables[variables_attributes][][key]">
+ </div>
+
+ <div class="ci-variable-body-item">
+ <textarea class="js-ci-variable-input-value" name="variables[variables_attributes][][value]"></textarea>
+ </div>
+
+ <div class="ci-variable-body-item ci-variable-protected-item">
+ <button type="button" class="js-project-feature-toggle project-feature-toggle">
+ <input
+ type="hidden"
+ class="js-ci-variable-input-protected js-project-feature-toggle-input"
+ name="variables[variables_attributes][][protected]"
+ value="true"
+ />
+ </button>
+ </div>
+
+ <button type="button" class="js-row-remove-button"></button>
+ </li>
+ </ul>
+ <button type="button" class="js-secret-value-reveal-button">
+ Reveal values
+ </button>
+ </form>`);
+
+ variableList = new VariableList({
+ container: $wrapper,
+ formField: 'variables',
+ });
+ variableList.init();
+ });
+
+ it('should add another row when editing the last rows protected checkbox', (done) => {
+ const $row = $wrapper.find('.js-row:last-child');
+ $row.find('.ci-variable-protected-item .js-project-feature-toggle').click();
+
+ getSetTimeoutPromise()
+ .then(() => {
+ expect($wrapper.find('.js-row').length).toBe(2);
+
+ // Check for the correct default in the new row
+ const $protectedInput = $wrapper.find('.js-row:last-child').find('.js-ci-variable-input-protected');
+ expect($protectedInput.val()).toBe('true');
+ })
+ .then(done)
+ .catch(done.fail);
+ });
+ });
+});
diff --git a/spec/javascripts/ci_variable_list/native_form_variable_list_spec.js b/spec/javascripts/ci_variable_list/native_form_variable_list_spec.js
new file mode 100644
index 00000000000..eb508a7f059
--- /dev/null
+++ b/spec/javascripts/ci_variable_list/native_form_variable_list_spec.js
@@ -0,0 +1,30 @@
+import setupNativeFormVariableList from '~/ci_variable_list/native_form_variable_list';
+
+describe('NativeFormVariableList', () => {
+ preloadFixtures('pipeline_schedules/edit.html.raw');
+
+ let $wrapper;
+
+ beforeEach(() => {
+ loadFixtures('pipeline_schedules/edit.html.raw');
+ $wrapper = $('.js-ci-variable-list-section');
+
+ setupNativeFormVariableList({
+ container: $wrapper,
+ formField: 'schedule',
+ });
+ });
+
+ describe('onFormSubmit', () => {
+ it('should clear out the `name` attribute on the inputs for the last empty row on form submission (avoid BE validation)', () => {
+ const $row = $wrapper.find('.js-row');
+ expect($row.find('.js-ci-variable-input-key').attr('name')).toBe('schedule[variables_attributes][][key]');
+ expect($row.find('.js-ci-variable-input-value').attr('name')).toBe('schedule[variables_attributes][][value]');
+
+ $wrapper.closest('form').trigger('trigger-submit');
+
+ expect($row.find('.js-ci-variable-input-key').attr('name')).toBe('');
+ expect($row.find('.js-ci-variable-input-value').attr('name')).toBe('');
+ });
+ });
+});
diff --git a/spec/javascripts/fixtures/pipeline_schedules.rb b/spec/javascripts/fixtures/pipeline_schedules.rb
new file mode 100644
index 00000000000..56f27ea7df1
--- /dev/null
+++ b/spec/javascripts/fixtures/pipeline_schedules.rb
@@ -0,0 +1,43 @@
+require 'spec_helper'
+
+describe Projects::PipelineSchedulesController, '(JavaScript fixtures)', type: :controller do
+ include JavaScriptFixturesHelpers
+
+ let(:admin) { create(:admin) }
+ let(:namespace) { create(:namespace, name: 'frontend-fixtures' )}
+ let(:project) { create(:project, :public, :repository) }
+ let!(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: admin) }
+ let!(:pipeline_schedule_populated) { create(:ci_pipeline_schedule, project: project, owner: admin) }
+ let!(:pipeline_schedule_variable1) { create(:ci_pipeline_schedule_variable, key: 'foo', value: 'foovalue', pipeline_schedule: pipeline_schedule_populated) }
+ let!(:pipeline_schedule_variable2) { create(:ci_pipeline_schedule_variable, key: 'bar', value: 'barvalue', pipeline_schedule: pipeline_schedule_populated) }
+
+ render_views
+
+ before(:all) do
+ clean_frontend_fixtures('pipeline_schedules/')
+ end
+
+ before do
+ sign_in(admin)
+ end
+
+ it 'pipeline_schedules/edit.html.raw' do |example|
+ get :edit,
+ namespace_id: project.namespace.to_param,
+ project_id: project,
+ id: pipeline_schedule.id
+
+ expect(response).to be_success
+ store_frontend_fixture(response, example.description)
+ end
+
+ it 'pipeline_schedules/edit_with_variables.html.raw' do |example|
+ get :edit,
+ namespace_id: project.namespace.to_param,
+ project_id: project,
+ id: pipeline_schedule_populated.id
+
+ expect(response).to be_success
+ store_frontend_fixture(response, example.description)
+ end
+end
diff --git a/spec/javascripts/pipeline_schedules/setup_pipeline_variable_list_spec.js b/spec/javascripts/pipeline_schedules/setup_pipeline_variable_list_spec.js
deleted file mode 100644
index 5b316b319a5..00000000000
--- a/spec/javascripts/pipeline_schedules/setup_pipeline_variable_list_spec.js
+++ /dev/null
@@ -1,145 +0,0 @@
-import {
- setupPipelineVariableList,
- insertRow,
- removeRow,
-} from '~/pipeline_schedules/setup_pipeline_variable_list';
-
-describe('Pipeline Variable List', () => {
- let $markup;
-
- describe('insertRow', () => {
- it('should insert another row', () => {
- $markup = $(`<div>
- <li class="js-row">
- <input>
- <textarea></textarea>
- </li>
- </div>`);
-
- insertRow($markup.find('.js-row'));
-
- expect($markup.find('.js-row').length).toBe(2);
- });
-
- it('should clear `data-is-persisted` on cloned row', () => {
- $markup = $(`<div>
- <li class="js-row" data-is-persisted="true"></li>
- </div>`);
-
- insertRow($markup.find('.js-row'));
-
- const $lastRow = $markup.find('.js-row').last();
- expect($lastRow.attr('data-is-persisted')).toBe(undefined);
- });
-
- it('should clear inputs on cloned row', () => {
- $markup = $(`<div>
- <li class="js-row">
- <input value="foo">
- <textarea>bar</textarea>
- </li>
- </div>`);
-
- insertRow($markup.find('.js-row'));
-
- const $lastRow = $markup.find('.js-row').last();
- expect($lastRow.find('input').val()).toBe('');
- expect($lastRow.find('textarea').val()).toBe('');
- });
- });
-
- describe('removeRow', () => {
- it('should remove dynamic row', () => {
- $markup = $(`<div>
- <li class="js-row">
- <input>
- <textarea></textarea>
- </li>
- </div>`);
-
- removeRow($markup.find('.js-row'));
-
- expect($markup.find('.js-row').length).toBe(0);
- });
-
- it('should hide and mark to destroy with already persisted rows', () => {
- $markup = $(`<div>
- <li class="js-row" data-is-persisted="true">
- <input class="js-destroy-input">
- </li>
- </div>`);
-
- const $row = $markup.find('.js-row');
- removeRow($row);
-
- expect($row.find('.js-destroy-input').val()).toBe('1');
- expect($markup.find('.js-row').length).toBe(1);
- });
- });
-
- describe('setupPipelineVariableList', () => {
- beforeEach(() => {
- $markup = $(`<form>
- <li class="js-row">
- <input class="js-user-input" name="schedule[variables_attributes][][key]">
- <textarea class="js-user-input" name="schedule[variables_attributes][][value]"></textarea>
- <button class="js-row-remove-button"></button>
- <button class="js-row-add-button"></button>
- </li>
- </form>`);
-
- setupPipelineVariableList($markup);
- });
-
- it('should remove the row when clicking the remove button', () => {
- $markup.find('.js-row-remove-button').trigger('click');
-
- expect($markup.find('.js-row').length).toBe(0);
- });
-
- it('should add another row when editing the last rows key input', () => {
- const $row = $markup.find('.js-row');
- $row.find('input.js-user-input')
- .val('foo')
- .trigger('input');
-
- expect($markup.find('.js-row').length).toBe(2);
- });
-
- it('should add another row when editing the last rows value textarea', () => {
- const $row = $markup.find('.js-row');
- $row.find('textarea.js-user-input')
- .val('foo')
- .trigger('input');
-
- expect($markup.find('.js-row').length).toBe(2);
- });
-
- it('should remove empty row after blurring', () => {
- const $row = $markup.find('.js-row');
- $row.find('input.js-user-input')
- .val('foo')
- .trigger('input');
-
- expect($markup.find('.js-row').length).toBe(2);
-
- $row.find('input.js-user-input')
- .val('')
- .trigger('input')
- .trigger('blur');
-
- expect($markup.find('.js-row').length).toBe(1);
- });
-
- it('should clear out the `name` attribute on the inputs for the last empty row on form submission (avoid BE validation)', () => {
- const $row = $markup.find('.js-row');
- expect($row.find('input').attr('name')).toBe('schedule[variables_attributes][][key]');
- expect($row.find('textarea').attr('name')).toBe('schedule[variables_attributes][][value]');
-
- $markup.filter('form').submit();
-
- expect($row.find('input').attr('name')).toBe('');
- expect($row.find('textarea').attr('name')).toBe('');
- });
- });
-});
diff --git a/spec/lib/gitlab/badge/coverage/template_spec.rb b/spec/lib/gitlab/badge/coverage/template_spec.rb
index 383bae6e087..d9c21a22590 100644
--- a/spec/lib/gitlab/badge/coverage/template_spec.rb
+++ b/spec/lib/gitlab/badge/coverage/template_spec.rb
@@ -1,7 +1,7 @@
require 'spec_helper'
describe Gitlab::Badge::Coverage::Template do
- let(:badge) { double(entity: 'coverage', status: 90) }
+ let(:badge) { double(entity: 'coverage', status: 90.00) }
let(:template) { described_class.new(badge) }
describe '#key_text' do
@@ -13,7 +13,17 @@ describe Gitlab::Badge::Coverage::Template do
describe '#value_text' do
context 'when coverage is known' do
it 'returns coverage percentage' do
- expect(template.value_text).to eq '90%'
+ expect(template.value_text).to eq '90.00%'
+ end
+ end
+
+ context 'when coverage is known to many digits' do
+ before do
+ allow(badge).to receive(:status).and_return(92.349)
+ end
+
+ it 'returns rounded coverage percentage' do
+ expect(template.value_text).to eq '92.35%'
end
end
@@ -37,7 +47,7 @@ describe Gitlab::Badge::Coverage::Template do
describe '#value_width' do
context 'when coverage is known' do
it 'is narrower when coverage is known' do
- expect(template.value_width).to eq 36
+ expect(template.value_width).to eq 54
end
end
@@ -113,7 +123,7 @@ describe Gitlab::Badge::Coverage::Template do
describe '#width' do
context 'when coverage is known' do
it 'returns the key width plus value width' do
- expect(template.width).to eq 98
+ expect(template.width).to eq 116
end
end
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index e768ff7735f..ec1c7a96f92 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -1162,14 +1162,27 @@ describe Gitlab::Git::Repository, seed_helper: true do
context 'when Gitaly find_branch feature is disabled', :skip_gitaly_mock do
it_behaves_like 'finding a branch'
- it 'should reload Rugged::Repository and return master' do
- expect(Rugged::Repository).to receive(:new).twice.and_call_original
+ context 'force_reload is true' do
+ it 'should reload Rugged::Repository' do
+ expect(Rugged::Repository).to receive(:new).twice.and_call_original
- repository.find_branch('master')
- branch = repository.find_branch('master', force_reload: true)
+ repository.find_branch('master')
+ branch = repository.find_branch('master', force_reload: true)
- expect(branch).to be_a_kind_of(Gitlab::Git::Branch)
- expect(branch.name).to eq('master')
+ expect(branch).to be_a_kind_of(Gitlab::Git::Branch)
+ expect(branch.name).to eq('master')
+ end
+ end
+
+ context 'force_reload is false' do
+ it 'should not reload Rugged::Repository' do
+ expect(Rugged::Repository).to receive(:new).once.and_call_original
+
+ branch = repository.find_branch('master', force_reload: false)
+
+ expect(branch).to be_a_kind_of(Gitlab::Git::Branch)
+ expect(branch.name).to eq('master')
+ end
end
end
end
diff --git a/spec/lib/gitlab/query_limiting/active_support_subscriber_spec.rb b/spec/lib/gitlab/query_limiting/active_support_subscriber_spec.rb
new file mode 100644
index 00000000000..b49bc5c328c
--- /dev/null
+++ b/spec/lib/gitlab/query_limiting/active_support_subscriber_spec.rb
@@ -0,0 +1,19 @@
+require 'spec_helper'
+
+describe Gitlab::QueryLimiting::ActiveSupportSubscriber do
+ describe '#sql' do
+ it 'increments the number of executed SQL queries' do
+ transaction = double(:transaction)
+
+ allow(Gitlab::QueryLimiting::Transaction)
+ .to receive(:current)
+ .and_return(transaction)
+
+ expect(transaction)
+ .to receive(:increment)
+ .at_least(:once)
+
+ User.count
+ end
+ end
+end
diff --git a/spec/lib/gitlab/query_limiting/middleware_spec.rb b/spec/lib/gitlab/query_limiting/middleware_spec.rb
new file mode 100644
index 00000000000..a04bcdecb4b
--- /dev/null
+++ b/spec/lib/gitlab/query_limiting/middleware_spec.rb
@@ -0,0 +1,72 @@
+require 'spec_helper'
+
+describe Gitlab::QueryLimiting::Middleware do
+ describe '#call' do
+ it 'runs the application with query limiting in place' do
+ middleware = described_class.new(-> (env) { env })
+
+ expect_any_instance_of(Gitlab::QueryLimiting::Transaction)
+ .to receive(:act_upon_results)
+
+ expect(middleware.call({ number: 10 }))
+ .to eq({ number: 10 })
+ end
+ end
+
+ describe '#action_name' do
+ let(:middleware) { described_class.new(-> (env) { env }) }
+
+ context 'using a Rails request' do
+ it 'returns the name of the controller and action' do
+ env = {
+ described_class::CONTROLLER_KEY => double(
+ :controller,
+ action_name: 'show',
+ class: double(:class, name: 'UsersController'),
+ content_type: 'text/html'
+ )
+ }
+
+ expect(middleware.action_name(env)).to eq('UsersController#show')
+ end
+
+ it 'includes the content type if this is not text/html' do
+ env = {
+ described_class::CONTROLLER_KEY => double(
+ :controller,
+ action_name: 'show',
+ class: double(:class, name: 'UsersController'),
+ content_type: 'application/json'
+ )
+ }
+
+ expect(middleware.action_name(env))
+ .to eq('UsersController#show (application/json)')
+ end
+ end
+
+ context 'using a Grape API request' do
+ it 'returns the name of the request method and endpoint path' do
+ env = {
+ described_class::ENDPOINT_KEY => double(
+ :endpoint,
+ route: double(:route, request_method: 'GET', path: '/foo')
+ )
+ }
+
+ expect(middleware.action_name(env)).to eq('GET /foo')
+ end
+
+ it 'returns nil if the route can not be retrieved' do
+ endpoint = double(:endpoint)
+ env = { described_class::ENDPOINT_KEY => endpoint }
+
+ allow(endpoint)
+ .to receive(:route)
+ .and_raise(RuntimeError)
+
+ expect(middleware.action_name(env)).to be_nil
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/query_limiting/transaction_spec.rb b/spec/lib/gitlab/query_limiting/transaction_spec.rb
new file mode 100644
index 00000000000..b4231fcd0fa
--- /dev/null
+++ b/spec/lib/gitlab/query_limiting/transaction_spec.rb
@@ -0,0 +1,144 @@
+require 'spec_helper'
+
+describe Gitlab::QueryLimiting::Transaction do
+ after do
+ Thread.current[described_class::THREAD_KEY] = nil
+ end
+
+ describe '.current' do
+ it 'returns nil when there is no transaction' do
+ expect(described_class.current).to be_nil
+ end
+
+ it 'returns the transaction when present' do
+ Thread.current[described_class::THREAD_KEY] = described_class.new
+
+ expect(described_class.current).to be_an_instance_of(described_class)
+ end
+ end
+
+ describe '.run' do
+ it 'runs a transaction and returns it and its return value' do
+ trans, ret = described_class.run do
+ 10
+ end
+
+ expect(trans).to be_an_instance_of(described_class)
+ expect(ret).to eq(10)
+ end
+
+ it 'removes the transaction from the current thread upon completion' do
+ described_class.run do
+ 10
+ end
+
+ expect(Thread.current[described_class::THREAD_KEY]).to be_nil
+ end
+ end
+
+ describe '#act_upon_results' do
+ context 'when the query threshold is not exceeded' do
+ it 'does nothing' do
+ trans = described_class.new
+
+ expect(trans).not_to receive(:raise)
+
+ trans.act_upon_results
+ end
+ end
+
+ context 'when the query threshold is exceeded' do
+ let(:transaction) do
+ trans = described_class.new
+ trans.count = described_class::THRESHOLD + 1
+
+ trans
+ end
+
+ it 'raises an error when this is enabled' do
+ expect { transaction.act_upon_results }
+ .to raise_error(described_class::ThresholdExceededError)
+ end
+
+ it 'reports the error in Sentry if raising an error is disabled' do
+ expect(transaction)
+ .to receive(:raise_error?)
+ .and_return(false)
+
+ expect(Raven)
+ .to receive(:capture_exception)
+ .with(an_instance_of(described_class::ThresholdExceededError))
+
+ transaction.act_upon_results
+ end
+ end
+ end
+
+ describe '#increment' do
+ it 'increments the number of executed queries' do
+ transaction = described_class.new
+
+ expect(transaction.count).to be_zero
+
+ transaction.increment
+
+ expect(transaction.count).to eq(1)
+ end
+ end
+
+ describe '#raise_error?' do
+ it 'returns true in a test environment' do
+ transaction = described_class.new
+
+ expect(transaction.raise_error?).to eq(true)
+ end
+
+ it 'returns false in a production environment' do
+ transaction = described_class.new
+
+ expect(Rails.env)
+ .to receive(:test?)
+ .and_return(false)
+
+ expect(transaction.raise_error?).to eq(false)
+ end
+ end
+
+ describe '#threshold_exceeded?' do
+ it 'returns false when the threshold is not exceeded' do
+ transaction = described_class.new
+
+ expect(transaction.threshold_exceeded?).to eq(false)
+ end
+
+ it 'returns true when the threshold is exceeded' do
+ transaction = described_class.new
+ transaction.count = described_class::THRESHOLD + 1
+
+ expect(transaction.threshold_exceeded?).to eq(true)
+ end
+ end
+
+ describe '#error_message' do
+ it 'returns the error message to display when the threshold is exceeded' do
+ transaction = described_class.new
+ transaction.count = max = described_class::THRESHOLD
+
+ expect(transaction.error_message).to eq(
+ "Too many SQL queries were executed: a maximum of #{max} " \
+ "is allowed but #{max} SQL queries were executed"
+ )
+ end
+
+ it 'includes the action name in the error message when present' do
+ transaction = described_class.new
+ transaction.count = max = described_class::THRESHOLD
+ transaction.action = 'UsersController#show'
+
+ expect(transaction.error_message).to eq(
+ "Too many SQL queries were executed in UsersController#show: " \
+ "a maximum of #{max} is allowed but #{max} SQL queries were executed"
+ )
+ end
+ end
+end
diff --git a/spec/lib/gitlab/query_limiting_spec.rb b/spec/lib/gitlab/query_limiting_spec.rb
new file mode 100644
index 00000000000..2eddab0b8c3
--- /dev/null
+++ b/spec/lib/gitlab/query_limiting_spec.rb
@@ -0,0 +1,65 @@
+require 'spec_helper'
+
+describe Gitlab::QueryLimiting do
+ describe '.enable?' do
+ it 'returns true in a test environment' do
+ expect(described_class.enable?).to eq(true)
+ end
+
+ it 'returns true in a development environment' do
+ allow(Rails.env).to receive(:development?).and_return(true)
+
+ expect(described_class.enable?).to eq(true)
+ end
+
+ it 'returns true on GitLab.com' do
+ allow(Gitlab).to receive(:com?).and_return(true)
+
+ expect(described_class.enable?).to eq(true)
+ end
+
+ it 'returns true in a non GitLab.com' do
+ expect(Gitlab).to receive(:com?).and_return(false)
+ expect(Rails.env).to receive(:development?).and_return(false)
+ expect(Rails.env).to receive(:test?).and_return(false)
+
+ expect(described_class.enable?).to eq(false)
+ end
+ end
+
+ describe '.whitelist' do
+ it 'raises ArgumentError when an invalid issue URL is given' do
+ expect { described_class.whitelist('foo') }
+ .to raise_error(ArgumentError)
+ end
+
+ context 'without a transaction' do
+ it 'does nothing' do
+ expect { described_class.whitelist('https://example.com') }
+ .not_to raise_error
+ end
+ end
+
+ context 'with a transaction' do
+ let(:transaction) { Gitlab::QueryLimiting::Transaction.new }
+
+ before do
+ allow(Gitlab::QueryLimiting::Transaction)
+ .to receive(:current)
+ .and_return(transaction)
+ end
+
+ it 'does not increment the number of SQL queries executed in the block' do
+ before = transaction.count
+
+ described_class.whitelist('https://example.com')
+
+ 2.times do
+ User.count
+ end
+
+ expect(transaction.count).to eq(before)
+ end
+ end
+ end
+end
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 1102b1c9006..02a5ee54262 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -36,26 +36,49 @@ describe Repository do
end
describe '#branch_names_contains' do
- subject { repository.branch_names_contains(sample_commit.id) }
+ shared_examples '#branch_names_contains' do
+ set(:project) { create(:project, :repository) }
+ let(:repository) { project.repository }
- it { is_expected.to include('master') }
- it { is_expected.not_to include('feature') }
- it { is_expected.not_to include('fix') }
+ subject { repository.branch_names_contains(sample_commit.id) }
- 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)
+ 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
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
- subject { repository.tag_names_contains(sample_commit.id) }
+ 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') }
+ 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
+
+ context 'when gitaly is enabled', :skip_gitaly_mock do
+ it_behaves_like '#tag_names_contains'
+ end
end
describe 'tags_sorted_by' do
@@ -959,19 +982,19 @@ describe Repository do
end
describe '#find_branch' do
- it 'loads a branch with a fresh repo' do
- expect(Gitlab::Git::Repository).to receive(:new).twice.and_call_original
+ context 'fresh_repo is true' do
+ it 'delegates the call to raw_repository' do
+ expect(repository.raw_repository).to receive(:find_branch).with('master', true)
- 2.times do
- expect(repository.find_branch('feature')).not_to be_nil
+ repository.find_branch('master', fresh_repo: true)
end
end
- it 'loads a branch with a cached repo' do
- expect(Gitlab::Git::Repository).to receive(:new).once.and_call_original
+ context 'fresh_repo is false' do
+ it 'delegates the call to raw_repository' do
+ expect(repository.raw_repository).to receive(:find_branch).with('master', false)
- 2.times do
- expect(repository.find_branch('feature', fresh_repo: false)).not_to be_nil
+ repository.find_branch('master', fresh_repo: false)
end
end
end
diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb
index 3c0b4728dc2..bb0034e3237 100644
--- a/spec/requests/api/groups_spec.rb
+++ b/spec/requests/api/groups_spec.rb
@@ -30,6 +30,21 @@ describe API::Groups do
expect(json_response)
.to satisfy_one { |group| group['name'] == group1.name }
end
+
+ it 'avoids N+1 queries' do
+ # Establish baseline
+ get api("/groups", admin)
+
+ control = ActiveRecord::QueryRecorder.new do
+ get api("/groups", admin)
+ end
+
+ create(:group)
+
+ expect do
+ get api("/groups", admin)
+ end.not_to exceed_query_limit(control)
+ end
end
context "when authenticated as user" do
View Lorry Controller Status