From 8d397862cd90f39cc70dd775d613635a542ab72c Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Sat, 23 Jan 2016 19:33:18 +0100 Subject: [PATCH 1/4] Use generic method to checks if artifacts are available --- app/views/projects/commit_statuses/_commit_status.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/projects/commit_statuses/_commit_status.html.haml b/app/views/projects/commit_statuses/_commit_status.html.haml index 1736dccaf3c..2e3c956ddc4 100644 --- a/app/views/projects/commit_statuses/_commit_status.html.haml +++ b/app/views/projects/commit_statuses/_commit_status.html.haml @@ -66,7 +66,7 @@ %td .pull-right - - if current_user && can?(current_user, :read_build_artifacts, commit_status.project) && commit_status.artifacts? + - if current_user && can?(current_user, :read_build_artifacts, commit_status.project) && commit_status.artifacts_download_url = link_to commit_status.artifacts_download_url, title: 'Download artifacts' do %i.fa.fa-download - if current_user && can?(current_user, :manage_builds, commit_status.project) From 5583b9526baa17b3f2e86322ee36fc1f94b322dd Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 26 Jan 2016 08:32:36 +0100 Subject: [PATCH 2/4] Add specs for build created using generic commit status --- spec/factories/commit_statuses.rb | 9 +- spec/features/commits_spec.rb | 165 +++++++++++++++++------------- 2 files changed, 100 insertions(+), 74 deletions(-) diff --git a/spec/factories/commit_statuses.rb b/spec/factories/commit_statuses.rb index 8898b71e2a3..45b11edcef3 100644 --- a/spec/factories/commit_statuses.rb +++ b/spec/factories/commit_statuses.rb @@ -1,11 +1,16 @@ FactoryGirl.define do factory :commit_status, class: CommitStatus do - started_at 'Di 29. Okt 09:51:28 CET 2013' - finished_at 'Di 29. Okt 09:53:28 CET 2013' name 'default' + ref 'master' status 'success' description 'commit status' commit factory: :ci_commit_with_one_job + started_at 'Tue, 26 Jan 2016 08:23:42 +0100' + finished_at 'Tue, 26 Jan 2016 08:23:42 +0100' + + after(:build) do |build, evaluator| + build.project = build.commit.project + end factory :generic_commit_status, class: GenericCommitStatus do name 'generic' diff --git a/spec/features/commits_spec.rb b/spec/features/commits_spec.rb index fe7f07f5b75..5a62da10619 100644 --- a/spec/features/commits_spec.rb +++ b/spec/features/commits_spec.rb @@ -16,83 +16,104 @@ describe 'Commits' do FactoryGirl.create :ci_commit, project: project, sha: project.commit.sha end - let!(:build) { FactoryGirl.create :ci_build, commit: commit } + context 'commit status is Generic Commit Status' do + let!(:status) { FactoryGirl.create :generic_commit_status, commit: commit } - describe 'Project commits' do - before do - visit namespace_project_commits_path(project.namespace, project, :master) - end - - it 'should show build status' do - page.within("//li[@id='commit-#{commit.short_sha}']") do - expect(page).to have_css(".ci-status-link") - end - end - end - - describe 'Commit builds' do - before do - visit ci_status_path(commit) - end - - it { expect(page).to have_content commit.sha[0..7] } - it { expect(page).to have_content commit.git_commit_message } - it { expect(page).to have_content commit.git_author_name } - end - - context 'Download artifacts' do - let(:artifacts_file) { fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif') } - - before do - build.update_attributes(artifacts_file: artifacts_file) - end - - it do - visit ci_status_path(commit) - click_on 'Download artifacts' - expect(page.response_headers['Content-Type']).to eq(artifacts_file.content_type) - end - end - - describe 'Cancel all builds' do - it 'cancels commit' do - visit ci_status_path(commit) - click_on 'Cancel running' - expect(page).to have_content 'canceled' - end - end - - describe 'Cancel build' do - it 'cancels build' do - visit ci_status_path(commit) - click_on 'Cancel' - expect(page).to have_content 'canceled' - end - end - - describe '.gitlab-ci.yml not found warning' do - context 'ci builds enabled' do - it "does not show warning" do - visit ci_status_path(commit) - expect(page).not_to have_content '.gitlab-ci.yml not found in this commit' - end - - it 'shows warning' do - stub_ci_commit_yaml_file(nil) - visit ci_status_path(commit) - expect(page).to have_content '.gitlab-ci.yml not found in this commit' - end - end - - context 'ci builds disabled' do + describe 'Commit builds' do before do - stub_ci_builds_disabled - stub_ci_commit_yaml_file(nil) visit ci_status_path(commit) end - it 'does not show warning' do - expect(page).not_to have_content '.gitlab-ci.yml not found in this commit' + it { expect(page).to have_content commit.sha[0..7] } + + it 'contains generic commit status build' do + page.within('.table-holder') do + expect(page).to have_content "##{status.id}" # build id + expect(page).to have_content 'generic' # build name + end + end + end + end + + context 'commit status is Ci Build' do + let!(:build) { FactoryGirl.create :ci_build, commit: commit } + + describe 'Project commits' do + before do + visit namespace_project_commits_path(project.namespace, project, :master) + end + + it 'should show build status' do + page.within("//li[@id='commit-#{commit.short_sha}']") do + expect(page).to have_css(".ci-status-link") + end + end + end + + describe 'Commit builds' do + before do + visit ci_status_path(commit) + end + + it { expect(page).to have_content commit.sha[0..7] } + it { expect(page).to have_content commit.git_commit_message } + it { expect(page).to have_content commit.git_author_name } + end + + context 'Download artifacts' do + let(:artifacts_file) { fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif') } + + before do + build.update_attributes(artifacts_file: artifacts_file) + end + + it do + visit ci_status_path(commit) + click_on 'Download artifacts' + expect(page.response_headers['Content-Type']).to eq(artifacts_file.content_type) + end + end + + describe 'Cancel all builds' do + it 'cancels commit' do + visit ci_status_path(commit) + click_on 'Cancel running' + expect(page).to have_content 'canceled' + end + end + + describe 'Cancel build' do + it 'cancels build' do + visit ci_status_path(commit) + click_on 'Cancel' + expect(page).to have_content 'canceled' + end + end + + describe '.gitlab-ci.yml not found warning' do + context 'ci builds enabled' do + it "does not show warning" do + visit ci_status_path(commit) + expect(page).not_to have_content '.gitlab-ci.yml not found in this commit' + end + + it 'shows warning' do + stub_ci_commit_yaml_file(nil) + visit ci_status_path(commit) + expect(page).to have_content '.gitlab-ci.yml not found in this commit' + end + end + + context 'ci builds disabled' do + before do + stub_ci_builds_disabled + stub_ci_commit_yaml_file(nil) + visit ci_status_path(commit) + end + + it 'does not show warning' do + expect(page).not_to have_content '.gitlab-ci.yml not found in this commit' + end end end end From 88d59d0161ac2f4890cfe77bb81c687a561b17ac Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 26 Jan 2016 08:36:54 +0100 Subject: [PATCH 3/4] Add Changelog entry for undefined method fix in commit builds --- CHANGELOG | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index 8d1f8d59b3c..ed018e5ebbb 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -9,6 +9,9 @@ v 8.5.0 (unreleased) - Whitelist raw "abbr" elements when parsing Markdown (Benedict Etzel) - Don't vendor minified JS +v 8.4.2 (unreleased) + - Fix method undefined when using external commit status in builds + v 8.4.1 - Apply security updates for Rails (4.2.5.1), rails-html-sanitizer (1.0.3), and Nokogiri (1.6.7.2) From 966176512797f037eb933691a2a21a8c3bb280b7 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 26 Jan 2016 15:23:57 +0100 Subject: [PATCH 4/4] Update commit status factory to reflect recent changes --- spec/factories/commit_statuses.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/factories/commit_statuses.rb b/spec/factories/commit_statuses.rb index 45b11edcef3..b7c2b32cb13 100644 --- a/spec/factories/commit_statuses.rb +++ b/spec/factories/commit_statuses.rb @@ -1,11 +1,10 @@ FactoryGirl.define do factory :commit_status, class: CommitStatus do name 'default' - ref 'master' status 'success' description 'commit status' commit factory: :ci_commit_with_one_job - started_at 'Tue, 26 Jan 2016 08:23:42 +0100' + started_at 'Tue, 26 Jan 2016 08:21:42 +0100' finished_at 'Tue, 26 Jan 2016 08:23:42 +0100' after(:build) do |build, evaluator|