From a57c77f6c2aa79b23f9aa59e6552ffeb514607de Mon Sep 17 00:00:00 2001 From: Timothy Andrew <mail@timothyandrew.net> Date: Wed, 21 Sep 2016 02:15:02 +0530 Subject: [PATCH] Add a spec for merge request metric caching while refreshing a merge request from a forked project. --- .../merge_requests/refresh_service.rb | 2 +- .../merge_requests/create_service_spec.rb | 3 +- .../merge_requests/refresh_service_spec.rb | 36 +++++++++++++++---- .../merge_requests/update_service_spec.rb | 9 +++-- 4 files changed, 38 insertions(+), 12 deletions(-) diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index cd4dd96740..22596b4014 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -145,7 +145,7 @@ module MergeRequests # If the merge requests closes any issues, save this information in the # `MergeRequestsClosingIssues` model (as a performance optimization). def cache_merge_requests_closing_issues - merge_requests_for_source_branch.each do |merge_request| + @project.merge_requests.where(source_branch: @branch_name).each do |merge_request| merge_request.cache_merge_request_closes_issues!(@current_user) end end diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb index d400655617..b814288907 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -108,7 +108,8 @@ describe MergeRequests::CreateService, services: true do allow(service).to receive(:execute_hooks) merge_request = service.execute - expect(merge_request.reload.closes_issues(user)).to match_array([first_issue, second_issue]) + issue_ids = MergeRequestsClosingIssues.where(merge_request: merge_request).pluck(:issue_id) + expect(issue_ids).to match_array([first_issue.id, second_issue.id]) end end end diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index dac4ba060c..a162df5fc3 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -174,11 +174,10 @@ describe MergeRequests::RefreshService, services: true do end end - context 'push commits closing issues' do + context 'merge request metrics' do let(:issue) { create :issue, project: @project } let(:commit_author) { create :user } let(:commit) { project.commit } - let!(:merge_request) { create(:merge_request, target_branch: 'master', source_branch: 'feature', source_project: @project) } before do project.team << [commit_author, :developer] @@ -195,12 +194,35 @@ describe MergeRequests::RefreshService, services: true do allow_any_instance_of(MergeRequest).to receive(:commits).and_return([commit]) end - it 'creates a `MergeRequestsClosingIssues` record for each closed issue' do - refresh_service = service.new(@project, @user) - allow(refresh_service).to receive(:execute_hooks) - refresh_service.execute(@oldrev, @newrev, 'refs/heads/feature') + context 'when the merge request is sourced from the same project' do + it 'creates a `MergeRequestsClosingIssues` record for each issue closed by a commit' do + merge_request = create(:merge_request, target_branch: 'master', source_branch: 'feature', source_project: @project) + refresh_service = service.new(@project, @user) + allow(refresh_service).to receive(:execute_hooks) + refresh_service.execute(@oldrev, @newrev, 'refs/heads/feature') + + issue_ids = MergeRequestsClosingIssues.where(merge_request: merge_request).pluck(:issue_id) + expect(issue_ids).to eq([issue.id]) + end + end - expect(merge_request.reload.closes_issues(@user)).to eq([issue]) + context 'when the merge request is sourced from a different project' do + it 'creates a `MergeRequestsClosingIssues` record for each issue closed by a commit' do + forked_project = create(:project) + create(:forked_project_link, forked_to_project: forked_project, forked_from_project: @project) + + merge_request = create(:merge_request, + target_branch: 'master', + source_branch: 'feature', + target_project: @project, + source_project: forked_project) + refresh_service = service.new(@project, @user) + allow(refresh_service).to receive(:execute_hooks) + refresh_service.execute(@oldrev, @newrev, 'refs/heads/feature') + + issue_ids = MergeRequestsClosingIssues.where(merge_request: merge_request).pluck(:issue_id) + expect(issue_ids).to eq([issue.id]) + end end end diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index e9c908ede7..33db34c0f6 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -274,7 +274,8 @@ describe MergeRequests::UpdateService, services: true do allow(service).to receive(:execute_hooks) service.execute(merge_request) - expect(merge_request.reload.closes_issues(user)).to match_array([first_issue, second_issue]) + issue_ids = MergeRequestsClosingIssues.where(merge_request: merge_request).pluck(:issue_id) + expect(issue_ids).to match_array([first_issue.id, second_issue.id]) end it 'removes `MergeRequestsClosingIssues` records when issues are not closed anymore' do @@ -288,13 +289,15 @@ describe MergeRequests::UpdateService, services: true do merge_request = MergeRequests::CreateService.new(project, user, opts).execute - expect(merge_request.reload.closes_issues(user)).to match_array([first_issue, second_issue]) + issue_ids = MergeRequestsClosingIssues.where(merge_request: merge_request).pluck(:issue_id) + expect(issue_ids).to match_array([first_issue.id, second_issue.id]) service = described_class.new(project, user, description: "not closing any issues") allow(service).to receive(:execute_hooks) service.execute(merge_request.reload) - expect(merge_request.reload.closes_issues(user)).to be_empty + issue_ids = MergeRequestsClosingIssues.where(merge_request: merge_request).pluck(:issue_id) + expect(issue_ids).to be_empty end end end -- 2.30.9