From 18a71c47603d703de73c46fef4889887f685bebe Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 7 Nov 2016 23:44:11 +0800 Subject: Show commit status from latest pipeline Rather than compound status from all pipelines. Closes #20560 --- spec/models/commit_spec.rb | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) (limited to 'spec/models/commit_spec.rb') diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index e3bb3482d67..ca277601970 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -206,23 +206,18 @@ eos end describe '#status' do - context 'without arguments for compound status' do - shared_examples 'giving the status from pipeline' do - it do - expect(commit.status).to eq(Ci::Pipeline.status) - end - end - - context 'with pipelines' do - let!(:pipeline) do - create(:ci_empty_pipeline, project: project, sha: commit.sha) + context 'without arguments' do + before do + 5.times do + create(:ci_empty_pipeline, + project: project, + sha: commit.sha, + status: Ci::Pipeline.all_state_names.sample) end - - it_behaves_like 'giving the status from pipeline' end - context 'without pipelines' do - it_behaves_like 'giving the status from pipeline' + it 'gives the status from latest pipeline' do + expect(commit.status).to eq(Ci::Pipeline.latest.first.status) end end @@ -248,8 +243,8 @@ eos expect(commit.status('fix')).to eq(pipeline_from_fix.status) end - it 'gives compound status if ref is nil' do - expect(commit.status(nil)).to eq(commit.status) + it 'gives status from latest pipeline for whatever branch' do + expect(commit.status(nil)).to eq(Ci::Pipeline.latest.first.status) end end end -- cgit v1.2.1 From 721f2d3788ae5e8374f357014bd9e20d62de0a81 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 21 Nov 2016 22:19:16 +0800 Subject: Still use compound pipeline status, but group by ref and sha so that it would show latest pipeline if ref and sha are both specified, otherwise still the same as before. --- spec/models/commit_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'spec/models/commit_spec.rb') diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index ca277601970..21590cd4ff1 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -216,8 +216,8 @@ eos end end - it 'gives the status from latest pipeline' do - expect(commit.status).to eq(Ci::Pipeline.latest.first.status) + it 'gives compound status' do + expect(commit.status).to eq(Ci::Pipeline.latest.status) end end @@ -243,8 +243,8 @@ eos expect(commit.status('fix')).to eq(pipeline_from_fix.status) end - it 'gives status from latest pipeline for whatever branch' do - expect(commit.status(nil)).to eq(Ci::Pipeline.latest.first.status) + it 'gives compound status if ref is nil' do + expect(commit.status(nil)).to eq(Ci::Pipeline.latest.status) end end end -- cgit v1.2.1 From 5ea2628921325bf60abdf9b192abec1f5bcc129e Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 23 Nov 2016 18:25:32 +0800 Subject: Fix test description to mention latest pipeline, feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333#note_18819886 --- spec/models/commit_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec/models/commit_spec.rb') diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 21590cd4ff1..a16b2e73dd7 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -216,7 +216,7 @@ eos end end - it 'gives compound status' do + it 'gives compound status from latest pipelines' do expect(commit.status).to eq(Ci::Pipeline.latest.status) end end @@ -243,7 +243,7 @@ eos expect(commit.status('fix')).to eq(pipeline_from_fix.status) end - it 'gives compound status if ref is nil' do + it 'gives compound status from latest pipelines if ref is nil' do expect(commit.status(nil)).to eq(Ci::Pipeline.latest.status) end end -- cgit v1.2.1 From 3a99e36e4448409bf21a0258dde1a82f6494922e Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 24 Nov 2016 16:27:57 +0800 Subject: Avoid using random in the tests, feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333#note_18860042 --- spec/models/commit_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec/models/commit_spec.rb') diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index a16b2e73dd7..62b77ef86c5 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -208,11 +208,11 @@ eos describe '#status' do context 'without arguments' do before do - 5.times do + %w[success failed created pending].each do |status| create(:ci_empty_pipeline, project: project, sha: commit.sha, - status: Ci::Pipeline.all_state_names.sample) + status: status) end end -- cgit v1.2.1 From 6b4d33566f5f434cc86381a4a1347e42bbe348ee Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Thu, 24 Nov 2016 15:07:44 +0100 Subject: Pass commit data to ProcessCommitWorker By passing commit data to this worker we remove the need for querying the Git repository for every job. This in turn reduces the time spent processing each job. The migration included migrates jobs from the old format to the new format. For this to work properly it requires downtime as otherwise workers may start producing errors until they're using a newer version of the worker code. --- spec/models/commit_spec.rb | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) (limited to 'spec/models/commit_spec.rb') diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index e3bb3482d67..7194c20d3bf 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -302,4 +302,21 @@ eos expect(commit.uri_type('this/path/doesnt/exist')).to be_nil end end + + describe '.from_hash' do + let(:new_commit) { described_class.from_hash(commit.to_hash, project) } + + it 'returns a Commit' do + expect(new_commit).to be_an_instance_of(described_class) + end + + it 'wraps a Gitlab::Git::Commit' do + expect(new_commit.raw).to be_an_instance_of(Gitlab::Git::Commit) + end + + it 'stores the correct commit fields' do + expect(new_commit.id).to eq(commit.id) + expect(new_commit.message).to eq(commit.message) + end + end end -- cgit v1.2.1 From f272ee6eba37548cbd8919139d583a71ffdac8dc Mon Sep 17 00:00:00 2001 From: Oswaldo Ferreira Date: Wed, 2 Nov 2016 21:49:13 -0200 Subject: Add shorthand support to gitlab markdown references --- spec/models/commit_spec.rb | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) (limited to 'spec/models/commit_spec.rb') diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 7194c20d3bf..eb482c7f913 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -34,24 +34,30 @@ describe Commit, models: true do end describe '#to_reference' do + let(:project) { create(:project, path: 'sample-project') } + let(:commit) { project.commit } + it 'returns a String reference to the object' do expect(commit.to_reference).to eq commit.id end it 'supports a cross-project reference' do - cross = double('project') - expect(commit.to_reference(cross)).to eq "#{project.to_reference}@#{commit.id}" + another_project = build(:project, name: 'another-project', namespace: project.namespace) + expect(commit.to_reference(another_project)).to eq "sample-project@#{commit.id}" end end describe '#reference_link_text' do + let(:project) { create(:project, path: 'sample-project') } + let(:commit) { project.commit } + it 'returns a String reference to the object' do expect(commit.reference_link_text).to eq commit.short_id end it 'supports a cross-project reference' do - cross = double('project') - expect(commit.reference_link_text(cross)).to eq "#{project.to_reference}@#{commit.short_id}" + another_project = build(:project, name: 'another-project', namespace: project.namespace) + expect(commit.reference_link_text(another_project)).to eq "sample-project@#{commit.short_id}" end end -- cgit v1.2.1 From f23b1cb453deea2659c0cb9e9047c72d859bbf9d Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 29 Nov 2016 13:47:43 +0000 Subject: Merge branch 'jej-23867-use-mr-finder-instead-of-access-check' into 'security' Replace MR access checks with use of MergeRequestsFinder Split from !2024 to partially solve https://gitlab.com/gitlab-org/gitlab-ce/issues/23867 :warning: - Potentially untested :bomb: - No test coverage :traffic_light: - Test coverage of some sort exists (a test failed when error raised) :vertical_traffic_light: - Test coverage of return value (a test failed when nil used) :white_check_mark: - Permissions check tested - [x] :bomb: app/finders/notes_finder.rb:17 - [x] :warning: app/views/layouts/nav/_project.html.haml:80 [`.count`] - [x] :bomb: app/controllers/concerns/creates_commit.rb:84 - [x] :traffic_light: app/controllers/projects/commits_controller.rb:24 - [x] :traffic_light: app/controllers/projects/compare_controller.rb:56 - [x] :vertical_traffic_light: app/controllers/projects/discussions_controller.rb:29 - [x] :white_check_mark: app/controllers/projects/todos_controller.rb:27 - [x] :vertical_traffic_light: app/models/commit.rb:268 - [x] :white_check_mark: lib/gitlab/search_results.rb:71 - [x] https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/2024/diffs#d1c10892daedb4d4dd3d4b12b6d071091eea83df_267_266 Memoize ` merged_merge_request(current_user)` - [x] https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/2024/diffs#d1c10892daedb4d4dd3d4b12b6d071091eea83df_248_247 Expected side effect for `merged_merge_request!`, consider `skip_authorization: true`. - [x] https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/2024/diffs#d1c10892daedb4d4dd3d4b12b6d071091eea83df_269_269 Scary use of unchecked `merged_merge_request?` See merge request !2033 --- spec/models/commit_spec.rb | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'spec/models/commit_spec.rb') diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index eb482c7f913..0935fc0561c 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -179,25 +179,26 @@ eos describe '#reverts_commit?' do let(:another_commit) { double(:commit, revert_description: "This reverts commit #{commit.sha}") } + let(:user) { commit.author } - it { expect(commit.reverts_commit?(another_commit)).to be_falsy } + it { expect(commit.reverts_commit?(another_commit, user)).to be_falsy } context 'commit has no description' do before { allow(commit).to receive(:description?).and_return(false) } - it { expect(commit.reverts_commit?(another_commit)).to be_falsy } + it { expect(commit.reverts_commit?(another_commit, user)).to be_falsy } end context "another_commit's description does not revert commit" do before { allow(commit).to receive(:description).and_return("Foo Bar") } - it { expect(commit.reverts_commit?(another_commit)).to be_falsy } + it { expect(commit.reverts_commit?(another_commit, user)).to be_falsy } end context "another_commit's description reverts commit" do before { allow(commit).to receive(:description).and_return("Foo #{another_commit.revert_description} Bar") } - it { expect(commit.reverts_commit?(another_commit)).to be_truthy } + it { expect(commit.reverts_commit?(another_commit, user)).to be_truthy } end context "another_commit's description reverts merged merge request" do @@ -207,7 +208,7 @@ eos allow(commit).to receive(:description).and_return("Foo #{another_commit.revert_description} Bar") end - it { expect(commit.reverts_commit?(another_commit)).to be_truthy } + it { expect(commit.reverts_commit?(another_commit, user)).to be_truthy } end end -- cgit v1.2.1 From 7cced60069c248156decf6ceabc4d1f447e47ff7 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 14 Dec 2016 21:00:06 +0800 Subject: Introduce latest_status and add a few tests Feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333#note_20003268 --- spec/models/commit_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec/models/commit_spec.rb') diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index b81fab0372a..a2a8392699e 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -224,7 +224,7 @@ eos end it 'gives compound status from latest pipelines' do - expect(commit.status).to eq(Ci::Pipeline.latest.status) + expect(commit.status).to eq(Ci::Pipeline.latest_status) end end @@ -251,7 +251,7 @@ eos end it 'gives compound status from latest pipelines if ref is nil' do - expect(commit.status(nil)).to eq(Ci::Pipeline.latest.status) + expect(commit.status(nil)).to eq(Ci::Pipeline.latest_status) end end end -- cgit v1.2.1 From cc6f578d5fc45f9c3d4cc7df5af72b15ce47b3a8 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 15 Dec 2016 18:14:48 +0800 Subject: Use described_class and update description Feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333#note_20059124 https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333#note_20059187 https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333#note_20059322 --- spec/models/commit_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/models/commit_spec.rb') diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index a2a8392699e..9aac2abfb27 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -213,7 +213,7 @@ eos end describe '#status' do - context 'without arguments' do + context 'without ref argument' do before do %w[success failed created pending].each do |status| create(:ci_empty_pipeline, -- cgit v1.2.1 From d9000184e5e8a63e0c24fe264e864fc27f6515c4 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 15 Dec 2016 18:52:36 +0800 Subject: Add explicit status test, feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333#note_20058993 https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333#note_20059060 https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333#note_20059357 --- spec/models/commit_spec.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'spec/models/commit_spec.rb') diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 9aac2abfb27..74b50d2908d 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -225,6 +225,7 @@ eos it 'gives compound status from latest pipelines' do expect(commit.status).to eq(Ci::Pipeline.latest_status) + expect(commit.status).to eq('pending') end end @@ -252,6 +253,7 @@ eos it 'gives compound status from latest pipelines if ref is nil' do expect(commit.status(nil)).to eq(Ci::Pipeline.latest_status) + expect(commit.status(nil)).to eq('failed') end end end -- cgit v1.2.1