diff options
author | Robert Speicher <robert@gitlab.com> | 2016-03-21 14:42:25 +0000 |
---|---|---|
committer | Robert Speicher <robert@gitlab.com> | 2016-03-21 14:42:25 +0000 |
commit | ffc3acd498e5cfeb98f6104421cba32acdd7ee64 (patch) | |
tree | 7127c4a4b0221703dce53948d8de48bb2dd152bc /spec | |
parent | 3fca30d27f90a52bdbca746b1244c95e6c7db2d2 (diff) | |
parent | 75aaf91cb1d08c4350e2881b18118faf30e1f310 (diff) | |
download | gitlab-ce-ffc3acd498e5cfeb98f6104421cba32acdd7ee64.tar.gz |
Merge branch 'issues-show-performance' into 'master'
Improve performance of viewing individual issues
This MR does two things:
1. `Issue#related_branches` no longer performs Git operations that aren't needed
2. The output of `Repository#exists?` is now cached and flushed properly
Combined these two changes should further cut down the amount of Git operations performed when viewing individual issues (and possibly other pages).
See merge request !3296
Diffstat (limited to 'spec')
-rw-r--r-- | spec/controllers/projects/issues_controller_spec.rb | 4 | ||||
-rw-r--r-- | spec/models/issue_spec.rb | 4 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 41 | ||||
-rw-r--r-- | spec/models/project_wiki_spec.rb | 12 | ||||
-rw-r--r-- | spec/models/repository_spec.rb | 30 | ||||
-rw-r--r-- | spec/workers/repository_fork_worker_spec.rb | 19 |
6 files changed, 102 insertions, 8 deletions
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 2cd81231144..a49bd8960ee 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -2,7 +2,7 @@ require('spec_helper') describe Projects::IssuesController do describe "GET #index" do - let(:project) { create(:project) } + let(:project) { create(:project_empty_repo) } let(:user) { create(:user) } let(:issue) { create(:issue, project: project) } @@ -41,7 +41,7 @@ describe Projects::IssuesController do end describe 'Confidential Issues' do - let(:project) { create(:empty_project, :public) } + let(:project) { create(:project_empty_repo, :public) } let(:assignee) { create(:assignee) } let(:author) { create(:user) } let(:non_member) { create(:user) } diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 05e4233243d..8334545b759 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -183,9 +183,9 @@ describe Issue, models: true do describe '#related_branches' do it "selects the right branches" do allow(subject.project.repository).to receive(:branch_names). - and_return(["mpempe", "#{subject.iid}mepmep", subject.to_branch_name]) + and_return(["mpempe", "#{subject.iid}mepmep", subject.to_branch_name]) - expect(subject.related_branches).to eq [subject.to_branch_name] + expect(subject.related_branches).to eq([subject.to_branch_name]) end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index b8b9a455b83..624022c1dda 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -720,4 +720,45 @@ describe Project, models: true do expect(described_class.search_by_title('KITTENS')).to eq([project]) end end + + describe '#create_repository' do + let(:project) { create(:project) } + let(:shell) { Gitlab::Shell.new } + + before do + allow(project).to receive(:gitlab_shell).and_return(shell) + end + + context 'using a regular repository' do + it 'creates the repository' do + expect(shell).to receive(:add_repository). + with(project.path_with_namespace). + and_return(true) + + expect(project.repository).to receive(:after_create) + + expect(project.create_repository).to eq(true) + end + + it 'adds an error if the repository could not be created' do + expect(shell).to receive(:add_repository). + with(project.path_with_namespace). + and_return(false) + + expect(project.repository).not_to receive(:after_create) + + expect(project.create_repository).to eq(false) + expect(project.errors).not_to be_empty + end + end + + context 'using a forked repository' do + it 'does nothing' do + expect(project).to receive(:forked?).and_return(true) + expect(shell).not_to receive(:add_repository) + + project.create_repository + end + end + end end diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb index a2085df5bcd..532e3f013fd 100644 --- a/spec/models/project_wiki_spec.rb +++ b/spec/models/project_wiki_spec.rb @@ -244,6 +244,18 @@ describe ProjectWiki, models: true do end end + describe '#create_repo!' do + it 'creates a repository' do + expect(subject).to receive(:init_repo). + with(subject.path_with_namespace). + and_return(true) + + expect(subject.repository).to receive(:after_create) + + expect(subject.create_repo!).to be_an_instance_of(Gollum::Wiki) + end + end + private def create_temp_repo(path) diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index a57229a4fdf..7eac70ae948 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -537,6 +537,12 @@ describe Repository, models: true do repository.before_delete end + + it 'flushes the exists cache' do + expect(repository).to receive(:expire_exists_cache) + + repository.before_delete + end end describe 'when a repository exists' do @@ -593,6 +599,12 @@ describe Repository, models: true do repository.after_import end + + it 'flushes the exists cache' do + expect(repository).to receive(:expire_exists_cache) + + repository.after_import + end end describe '#after_push_commit' do @@ -619,6 +631,14 @@ describe Repository, models: true do end end + describe '#after_create' do + it 'flushes the exists cache' do + expect(repository).to receive(:expire_exists_cache) + + repository.after_create + end + end + describe "#main_language" do it 'shows the main language of the project' do expect(repository.main_language).to eq("Ruby") @@ -781,6 +801,16 @@ describe Repository, models: true do end end + describe '#expire_exists_cache' do + let(:cache) { repository.send(:cache) } + + it 'expires the cache' do + expect(cache).to receive(:expire).with(:exists?) + + repository.expire_exists_cache + end + end + describe '#build_cache' do let(:cache) { repository.send(:cache) } diff --git a/spec/workers/repository_fork_worker_spec.rb b/spec/workers/repository_fork_worker_spec.rb index 172537474ee..4ef05eb29d2 100644 --- a/spec/workers/repository_fork_worker_spec.rb +++ b/spec/workers/repository_fork_worker_spec.rb @@ -3,12 +3,17 @@ require 'spec_helper' describe RepositoryForkWorker do let(:project) { create(:project) } let(:fork_project) { create(:project, forked_from_project: project) } + let(:shell) { Gitlab::Shell.new } subject { RepositoryForkWorker.new } + before do + allow(subject).to receive(:gitlab_shell).and_return(shell) + end + describe "#perform" do it "creates a new repository from a fork" do - expect_any_instance_of(Gitlab::Shell).to receive(:fork_repository).with( + expect(shell).to receive(:fork_repository).with( project.path_with_namespace, fork_project.namespace.path ).and_return(true) @@ -19,20 +24,26 @@ describe RepositoryForkWorker do fork_project.namespace.path) end - it 'flushes the empty caches' do - expect_any_instance_of(Gitlab::Shell).to receive(:fork_repository). + it 'flushes various caches' do + expect(shell).to receive(:fork_repository). with(project.path_with_namespace, fork_project.namespace.path). and_return(true) expect_any_instance_of(Repository).to receive(:expire_emptiness_caches). and_call_original + expect_any_instance_of(Repository).to receive(:expire_exists_cache). + and_call_original + subject.perform(project.id, project.path_with_namespace, fork_project.namespace.path) end it "handles bad fork" do - expect_any_instance_of(Gitlab::Shell).to receive(:fork_repository).and_return(false) + expect(shell).to receive(:fork_repository).and_return(false) + + expect(subject.logger).to receive(:error) + subject.perform( project.id, project.path_with_namespace, |