From 412db1874fbf2847ad9d84e9d2344d4c4d4b9fef Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 9 Aug 2017 21:41:45 +0800 Subject: [PATCH] Fix some tests and report the error message --- app/models/repository.rb | 4 ++-- .../projects/issues_controller_spec.rb | 2 +- spec/factories/deployments.rb | 4 ++++ spec/factories/merge_requests.rb | 3 ++- spec/requests/api/merge_requests_spec.rb | 20 +++++++++++++++---- .../ci/create_pipeline_service_spec.rb | 2 +- .../issues/resolve_discussions_spec.rb | 2 +- ...issuables_list_metadata_shared_examples.rb | 4 ++-- 8 files changed, 29 insertions(+), 12 deletions(-) diff --git a/app/models/repository.rb b/app/models/repository.rb index 02f7f70ecb0..c032baa5d2c 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -1021,10 +1021,10 @@ class Repository def fetch_ref(source_path, source_ref, target_ref) args = %W(fetch --no-tags -f #{source_path} #{source_ref}:#{target_ref}) - run_git(args) + message, status = run_git(args) # Make sure ref was created, and raise Rugged::ReferenceError when not - raise Rugged::ReferenceError unless ref_exists?(target_ref) + raise Rugged::ReferenceError, message unless ref_exists?(target_ref) end def create_ref(ref, ref_path) diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index bdee3894a13..7cda5c8e40e 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -1,7 +1,7 @@ require('spec_helper') describe Projects::IssuesController do - let(:project) { create(:project_empty_repo) } + let(:project) { create(:project) } let(:user) { create(:user) } let(:issue) { create(:issue, project: project) } diff --git a/spec/factories/deployments.rb b/spec/factories/deployments.rb index 29ad1af9fd9..e5abfd67d60 100644 --- a/spec/factories/deployments.rb +++ b/spec/factories/deployments.rb @@ -10,6 +10,10 @@ FactoryGirl.define do after(:build) do |deployment, evaluator| deployment.project ||= deployment.environment.project + + unless deployment.project.repository_exists? + allow(deployment.project.repository).to receive(:fetch_ref) + end end end end diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb index 19bf7582747..04493981945 100644 --- a/spec/factories/merge_requests.rb +++ b/spec/factories/merge_requests.rb @@ -70,10 +70,11 @@ FactoryGirl.define do after(:build) do |merge_request| target_project = merge_request.target_project + source_project = merge_request.source_project # Fake `fetch_ref` if we don't have repository # We have too many existing tests replying on this behaviour - unless target_project.repository_exists? + unless [target_project, source_project].all?(&:repository_exists?) allow(target_project.repository).to receive(:fetch_ref) end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 9eda6836ded..aa52d240e39 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -580,15 +580,27 @@ describe API::MergeRequests do let!(:fork_project) { create(:project, forked_from_project: project, namespace: user2.namespace, creator_id: user2.id) } let!(:unrelated_project) { create(:project, namespace: create(:user).namespace, creator_id: user2.id) } - before :each do |each| - fork_project.team << [user2, :reporter] + before do + fork_project.add_reporter(user2) + + Project.all.each(&method(:stub_project_repository_fetch_ref)) + end + + def stub_project_repository_fetch_ref(project) + allow(Project).to receive(:find_by).with(id: project.id.to_s) + .and_return(project) + + allow(Project).to receive(:find).with(project.id) + .and_return(project) + + allow(project.repository).to receive(:fetch_ref) end it "returns merge_request" do post api("/projects/#{fork_project.id}/merge_requests", user2), title: 'Test merge_request', source_branch: "feature_conflict", target_branch: "master", author: user2, target_project_id: project.id, description: 'Test description for Test merge_request' - expect(response).to have_http_status(201) + expect(response).to have_gitlab_http_status(201) expect(json_response['title']).to eq('Test merge_request') expect(json_response['description']).to eq('Test description for Test merge_request') end @@ -599,7 +611,7 @@ describe API::MergeRequests do expect(fork_project.forked_from_project).to eq(project) post api("/projects/#{fork_project.id}/merge_requests", user2), title: 'Test merge_request', source_branch: "master", target_branch: "master", author: user2, target_project_id: project.id - expect(response).to have_http_status(201) + expect(response).to have_gitlab_http_status(201) expect(json_response['title']).to eq('Test merge_request') end diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 730df4e0336..53d4fcfed18 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -66,7 +66,7 @@ describe Ci::CreatePipelineService do context 'when there is no pipeline for source branch' do it "does not update merge request head pipeline" do - merge_request = create(:merge_request, source_branch: 'other_branch', target_branch: "branch_1", source_project: project) + merge_request = create(:merge_request, source_branch: 'feature', target_branch: "branch_1", source_project: project) head_pipeline = pipeline diff --git a/spec/services/issues/resolve_discussions_spec.rb b/spec/services/issues/resolve_discussions_spec.rb index fac66791ffb..67a86c50fc1 100644 --- a/spec/services/issues/resolve_discussions_spec.rb +++ b/spec/services/issues/resolve_discussions_spec.rb @@ -20,7 +20,7 @@ describe Issues::ResolveDiscussions do describe "for resolving discussions" do let(:discussion) { create(:diff_note_on_merge_request, project: project, note: "Almost done").to_discussion } let(:merge_request) { discussion.noteable } - let(:other_merge_request) { create(:merge_request, source_project: project, source_branch: "other") } + let(:other_merge_request) { create(:merge_request, source_project: project, source_branch: "fix") } describe "#merge_request_for_resolving_discussion" do let(:service) { DummyService.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid) } diff --git a/spec/support/issuables_list_metadata_shared_examples.rb b/spec/support/issuables_list_metadata_shared_examples.rb index a60d3b0d22d..75982432ab4 100644 --- a/spec/support/issuables_list_metadata_shared_examples.rb +++ b/spec/support/issuables_list_metadata_shared_examples.rb @@ -2,12 +2,12 @@ shared_examples 'issuables list meta-data' do |issuable_type, action = nil| before do @issuable_ids = [] - 2.times do |n| + %w[fix improve/awesome].each do |source_branch| issuable = if issuable_type == :issue create(issuable_type, project: project) else - create(issuable_type, source_project: project, source_branch: "#{n}-feature") + create(issuable_type, source_project: project, source_branch: source_branch) end @issuable_ids << issuable.id