diff options
author | Douwe Maan <douwe@gitlab.com> | 2017-03-11 01:49:29 +0300 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2017-03-11 01:49:29 +0300 |
commit | e78aed49cd81e2873d7a536bc13e2e77435e290e (patch) | |
tree | e5c9029c2da55791db852dc7eda2ce7dd41b2750 | |
parent | b4b56faaee3b82b386ce928fac98e02352efdf5b (diff) | |
parent | a996880bf7d5d94b1bca7be84bd00b18e37da337 (diff) |
Merge branch '29046-fix-github-importer-open-prs' into 'master'
Fix GitHub Import deleting branches for open PRs from a fork
Closes #29046
See merge request !9758
5 files changed, 105 insertions, 33 deletions
diff --git a/changelogs/unreleased/29046-fix-github-importer-open-prs.yml b/changelogs/unreleased/29046-fix-github-importer-open-prs.yml new file mode 100644 index 00000000000..d279c269f94 --- /dev/null +++ b/changelogs/unreleased/29046-fix-github-importer-open-prs.yml @@ -0,0 +1,4 @@ +--- +title: Fix GitHub Import deleting branches for open PRs from a fork +merge_request: 9758 +author: diff --git a/lib/gitlab/github_import/importer.rb b/lib/gitlab/github_import/importer.rb index dc73cad93a5..eea4a91f17d 100644 --- a/lib/gitlab/github_import/importer.rb +++ b/lib/gitlab/github_import/importer.rb @@ -171,6 +171,8 @@ module Gitlab end def clean_up_restored_branches(pull_request) + return if pull_request.opened? + remove_branch(pull_request.source_branch_name) unless pull_request.source_branch_exists? remove_branch(pull_request.target_branch_name) unless pull_request.target_branch_exists? end diff --git a/lib/gitlab/github_import/pull_request_formatter.rb b/lib/gitlab/github_import/pull_request_formatter.rb index 28812fd0cb9..add7236e339 100644 --- a/lib/gitlab/github_import/pull_request_formatter.rb +++ b/lib/gitlab/github_import/pull_request_formatter.rb @@ -60,6 +60,10 @@ module Gitlab source_branch.repo.id != target_branch.repo.id end + def opened? + state == 'opened' + end + private def state diff --git a/spec/lib/gitlab/github_import/importer_spec.rb b/spec/lib/gitlab/github_import/importer_spec.rb index 3f080de99dd..8b867fbe322 100644 --- a/spec/lib/gitlab/github_import/importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer_spec.rb @@ -55,9 +55,6 @@ describe Gitlab::GithubImport::Importer, lib: true do allow_any_instance_of(Octokit::Client).to receive(:releases).and_return([release1, release2]) end - let(:octocat) { double(id: 123456, login: 'octocat', email: 'octocat@example.com') } - let(:created_at) { DateTime.strptime('2011-01-26T19:01:12Z') } - let(:updated_at) { DateTime.strptime('2011-01-27T19:01:12Z') } let(:label1) do double( name: 'Bug', @@ -127,32 +124,6 @@ describe Gitlab::GithubImport::Importer, lib: true do ) end - let!(:user) { create(:user, email: octocat.email) } - let(:repository) { double(id: 1, fork: false) } - let(:source_sha) { create(:commit, project: project).id } - let(:source_branch) { double(ref: 'branch-merged', repo: repository, sha: source_sha) } - let(:target_sha) { create(:commit, project: project, git_commit: RepoHelpers.another_sample_commit).id } - let(:target_branch) { double(ref: 'master', repo: repository, sha: target_sha) } - let(:pull_request) do - double( - number: 1347, - milestone: nil, - state: 'open', - title: 'New feature', - body: 'Please pull these awesome changes', - head: source_branch, - base: target_branch, - assignee: nil, - user: octocat, - created_at: created_at, - updated_at: updated_at, - closed_at: nil, - merged_at: nil, - url: "#{api_root}/repos/octocat/Hello-World/pulls/1347", - labels: [double(name: 'Label #2')] - ) - end - let(:release1) do double( tag_name: 'v1.0.0', @@ -177,12 +148,14 @@ describe Gitlab::GithubImport::Importer, lib: true do ) end + subject { described_class.new(project) } + it 'returns true' do - expect(described_class.new(project).execute).to eq true + expect(subject.execute).to eq true end it 'does not raise an error' do - expect { described_class.new(project).execute }.not_to raise_error + expect { subject.execute }.not_to raise_error end it 'stores error messages' do @@ -205,15 +178,93 @@ describe Gitlab::GithubImport::Importer, lib: true do end end + shared_examples 'Gitlab::GithubImport unit-testing' do + describe '#clean_up_restored_branches' do + subject { described_class.new(project) } + + before do + allow(gh_pull_request).to receive(:source_branch_exists?).at_least(:once) { false } + allow(gh_pull_request).to receive(:target_branch_exists?).at_least(:once) { false } + end + + context 'when pull request stills open' do + let(:gh_pull_request) { Gitlab::GithubImport::PullRequestFormatter.new(project, pull_request) } + + it 'does not remove branches' do + expect(subject).not_to receive(:remove_branch) + subject.send(:clean_up_restored_branches, gh_pull_request) + end + end + + context 'when pull request is closed' do + let(:gh_pull_request) { Gitlab::GithubImport::PullRequestFormatter.new(project, closed_pull_request) } + + it 'does remove branches' do + expect(subject).to receive(:remove_branch).at_least(2).times + subject.send(:clean_up_restored_branches, gh_pull_request) + end + end + end + end + let(:project) { create(:project, :wiki_disabled, import_url: "#{repo_root}/octocat/Hello-World.git") } + let(:octocat) { double(id: 123456, login: 'octocat', email: 'octocat@example.com') } let(:credentials) { { user: 'joe' } } + let(:created_at) { DateTime.strptime('2011-01-26T19:01:12Z') } + let(:updated_at) { DateTime.strptime('2011-01-27T19:01:12Z') } + let(:repository) { double(id: 1, fork: false) } + let(:source_sha) { create(:commit, project: project).id } + let(:source_branch) { double(ref: 'branch-merged', repo: repository, sha: source_sha) } + let(:target_sha) { create(:commit, project: project, git_commit: RepoHelpers.another_sample_commit).id } + let(:target_branch) { double(ref: 'master', repo: repository, sha: target_sha) } + let(:pull_request) do + double( + number: 1347, + milestone: nil, + state: 'open', + title: 'New feature', + body: 'Please pull these awesome changes', + head: source_branch, + base: target_branch, + assignee: nil, + user: octocat, + created_at: created_at, + updated_at: updated_at, + closed_at: nil, + merged_at: nil, + url: "#{api_root}/repos/octocat/Hello-World/pulls/1347", + labels: [double(name: 'Label #2')] + ) + end + let(:closed_pull_request) do + double( + number: 1347, + milestone: nil, + state: 'closed', + title: 'New feature', + body: 'Please pull these awesome changes', + head: source_branch, + base: target_branch, + assignee: nil, + user: octocat, + created_at: created_at, + updated_at: updated_at, + closed_at: updated_at, + merged_at: nil, + url: "#{api_root}/repos/octocat/Hello-World/pulls/1347", + labels: [double(name: 'Label #2')] + ) + end + context 'when importing a GitHub project' do let(:api_root) { 'https://api.github.com' } let(:repo_root) { 'https://github.com' } + subject { described_class.new(project) } it_behaves_like 'Gitlab::GithubImport::Importer#execute' it_behaves_like 'Gitlab::GithubImport::Importer#execute an error occurs' + it_behaves_like 'Gitlab::GithubImport unit-testing' describe '#client' do it 'instantiates a Client' do @@ -223,7 +274,7 @@ describe Gitlab::GithubImport::Importer, lib: true do {} ) - described_class.new(project).client + subject.client end end end @@ -231,6 +282,8 @@ describe Gitlab::GithubImport::Importer, lib: true do context 'when importing a Gitea project' do let(:api_root) { 'https://try.gitea.io/api/v1' } let(:repo_root) { 'https://try.gitea.io' } + subject { described_class.new(project) } + before do project.update(import_type: 'gitea', import_url: "#{repo_root}/foo/group/project.git") end @@ -239,6 +292,7 @@ describe Gitlab::GithubImport::Importer, lib: true do let(:expected_not_called) { [:import_releases] } end it_behaves_like 'Gitlab::GithubImport::Importer#execute an error occurs' + it_behaves_like 'Gitlab::GithubImport unit-testing' describe '#client' do it 'instantiates a Client' do @@ -248,7 +302,7 @@ describe Gitlab::GithubImport::Importer, lib: true do { host: "#{repo_root}:443/foo", api_version: 'v1' } ) - described_class.new(project).client + subject.client end end end diff --git a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb index 951cbea7857..44423917944 100644 --- a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb +++ b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb @@ -306,4 +306,12 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do expect(pull_request.url).to eq 'https://api.github.com/repos/octocat/Hello-World/pulls/1347' end end + + describe '#opened?' do + let(:raw_data) { double(base_data.merge(state: 'open')) } + + it 'returns true when state is "open"' do + expect(pull_request.opened?).to be_truthy + end + end end |