From 00155b6c694d7d9a016b532694f5049b70e9a4db Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 8 Dec 2015 11:02:13 +0100 Subject: [PATCH 1/6] Suppress warning about missing `.gitlab-ci.yml` if builds are disabled When user disables GitLab Ci Service in project's settings then warning about missing `.gitlab-ci.yml` file should be supressed. This a matter of user experience as stated in #3761 (closes #3761). --- spec/features/commits_spec.rb | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/spec/features/commits_spec.rb b/spec/features/commits_spec.rb index 90739cd6a28..130a5016b55 100644 --- a/spec/features/commits_spec.rb +++ b/spec/features/commits_spec.rb @@ -60,15 +60,29 @@ describe "Commits" do end describe ".gitlab-ci.yml not found warning" 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" + context 'ci service 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 - 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" + context 'ci service disabled' do + before do + allow_any_instance_of(GitlabCiService).to receive(:active).and_return(false) + 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 65fe4bb1ceed6bae05724f3494fb57e8b09fa616 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 8 Dec 2015 11:59:46 +0100 Subject: [PATCH 2/6] Improve gitlab ci commits specs (refactoring) This minimizes usage of instance variables in this spec, and changes double quotation marks to single when string interpolation is not being used. --- spec/features/commits_spec.rb | 78 +++++++++++++++---------------- spec/support/stub_gitlab_calls.rb | 4 ++ 2 files changed, 43 insertions(+), 39 deletions(-) diff --git a/spec/features/commits_spec.rb b/spec/features/commits_spec.rb index 130a5016b55..80ff4d3751f 100644 --- a/spec/features/commits_spec.rb +++ b/spec/features/commits_spec.rb @@ -1,83 +1,83 @@ require 'spec_helper' -describe "Commits" do +describe 'Commits' do include CiStatusHelper let(:project) { create(:project) } - describe "CI" do + describe 'CI' do before do login_as :user project.team << [@user, :master] - @ci_project = project.ensure_gitlab_ci_project - @commit = FactoryGirl.create :ci_commit, gl_project: project, sha: project.commit.sha - @build = FactoryGirl.create :ci_build, commit: @commit - @generic_status = FactoryGirl.create :generic_commit_status, commit: @commit - end - - before do + project.ensure_gitlab_ci_project stub_ci_commit_to_return_yaml_file end - describe "GET /:project/commits/:sha/ci" 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 } + let!(:commit) do + FactoryGirl.create :ci_commit, gl_project: project, sha: project.commit.sha end - context "Download artifacts" do + let!(:build) { FactoryGirl.create :ci_build, commit: commit } + + describe 'GET /:project/commits/:sha/ci' 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) + build.update_attributes(artifacts_file: artifacts_file) end it do - visit ci_status_path(@commit) - click_on "Download artifacts" + 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" + 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" + 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 + describe '.gitlab-ci.yml not found warning' do context 'ci service 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" + visit ci_status_path(commit) + expect(page).not_to have_content '.gitlab-ci.yml not found in this commit' end - it "shows warning" do + 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" + visit ci_status_path(commit) + expect(page).to have_content '.gitlab-ci.yml not found in this commit' end end context 'ci service disabled' do before do - allow_any_instance_of(GitlabCiService).to receive(:active).and_return(false) + stub_ci_service_disabled stub_ci_commit_yaml_file(nil) - visit ci_status_path(@commit) + visit ci_status_path(commit) end it 'does not show warning' do diff --git a/spec/support/stub_gitlab_calls.rb b/spec/support/stub_gitlab_calls.rb index 5b3eb1bfc5f..90936183824 100644 --- a/spec/support/stub_gitlab_calls.rb +++ b/spec/support/stub_gitlab_calls.rb @@ -21,6 +21,10 @@ module StubGitlabCalls allow_any_instance_of(Ci::Commit).to receive(:ci_yaml_file) { ci_yaml } end + def stub_ci_service_disabled + allow_any_instance_of(GitlabCiService).to receive(:active).and_return(false) + end + private def gitlab_url From 2dafec91dd542ac641fea4750bf8fd68211a58af Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 14 Dec 2015 11:03:33 +0100 Subject: [PATCH 3/6] Add matcher class to ci status link --- app/helpers/ci_status_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/helpers/ci_status_helper.rb b/app/helpers/ci_status_helper.rb index f8f2cbf1319..c202c592067 100644 --- a/app/helpers/ci_status_helper.rb +++ b/app/helpers/ci_status_helper.rb @@ -57,7 +57,7 @@ module CiStatusHelper def render_ci_status(ci_commit) link_to ci_status_path(ci_commit), - class: "c#{ci_status_color(ci_commit)}", + class: "ci-status-link c#{ci_status_color(ci_commit)}", title: "Build #{ci_status_label(ci_commit)}", data: { toggle: 'tooltip', placement: 'left' } do ci_status_icon(ci_commit) From b8f67c5e4735eb25a3d03daeb95900dc87692123 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 14 Dec 2015 11:28:49 +0100 Subject: [PATCH 4/6] Do not display ci build status if builds enabled but no `.gitlab-ci.yml` Ref #3827 --- app/models/ci/commit.rb | 10 ++++++ .../projects/commit/_commit_box.html.haml | 2 +- app/views/projects/commits/_commit.html.haml | 4 +-- spec/features/commits_spec.rb | 31 ++++++++++++++++++- 4 files changed, 43 insertions(+), 4 deletions(-) diff --git a/app/models/ci/commit.rb b/app/models/ci/commit.rb index 75465685e98..fca18ba79be 100644 --- a/app/models/ci/commit.rb +++ b/app/models/ci/commit.rb @@ -220,6 +220,16 @@ module Ci update!(committed_at: DateTime.now) end + ## + # This method checks if build status should be displayed. + # + # Build status should be available only if builds are enabled + # on project level and `.gitlab-ci.yml` file is present. + # + def show_build_status? + gl_project.builds_enabled? && ci_yaml_file + end + private def save_yaml_error(error) diff --git a/app/views/projects/commit/_commit_box.html.haml b/app/views/projects/commit/_commit_box.html.haml index 132cdc35c94..634924db247 100644 --- a/app/views/projects/commit/_commit_box.html.haml +++ b/app/views/projects/commit/_commit_box.html.haml @@ -40,7 +40,7 @@ - @commit.parents.each do |parent| = link_to parent.short_id, namespace_project_commit_path(@project.namespace, @project, parent), class: "monospace" -- if @ci_commit +- if @ci_commit && @ci_commit.show_build_status? .pull-right = link_to ci_status_path(@ci_commit), class: "ci-status ci-#{@ci_commit.status}" do = ci_status_icon(@ci_commit) diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml index 0d64486164e..1303b27c4f3 100644 --- a/app/views/projects/commits/_commit.html.haml +++ b/app/views/projects/commits/_commit.html.haml @@ -9,7 +9,7 @@ - cache_key.push(ci_commit.status) if ci_commit = cache(cache_key) do - %li.commit.js-toggle-container + %li.commit.js-toggle-container{ id: "commit-#{commit.short_id}" } .commit-row-title %strong.str-truncated = link_to_gfm commit.title, namespace_project_commit_path(project.namespace, project, commit.id), class: "commit-row-message" @@ -17,7 +17,7 @@ %a.text-expander.js-toggle-button ... .pull-right - - if ci_commit + - if ci_commit && ci_commit.show_build_status? = render_ci_status(ci_commit)   = clipboard_button(clipboard_text: commit.id) diff --git a/spec/features/commits_spec.rb b/spec/features/commits_spec.rb index 80ff4d3751f..f48d96c35a4 100644 --- a/spec/features/commits_spec.rb +++ b/spec/features/commits_spec.rb @@ -19,7 +19,36 @@ describe 'Commits' do let!(:build) { FactoryGirl.create :ci_build, commit: commit } - describe 'GET /:project/commits/:sha/ci' do + describe 'Project commits' do + context 'builds enabled' do + context '.gitlab-ci.yml found' 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 + + context 'no .gitlab-ci.yml found' do + before do + stub_ci_commit_yaml_file(nil) + visit namespace_project_commits_path(project.namespace, project, :master) + end + + it 'should not show build status' do + page.within("//li[@id='commit-#{commit.short_sha}']") do + expect(page).to have_no_css(".ci-status-link") + end + end + end + end + end + + describe 'Commit builds' do before do visit ci_status_path(commit) end From 9d2b8326b2c106389024a2c3e921fbb6283db748 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 14 Dec 2015 14:25:18 +0100 Subject: [PATCH 5/6] Update commits spinach tests related to displaying build status --- features/project/commits/commits.feature | 3 ++- features/steps/project/commits/commits.rb | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/features/project/commits/commits.feature b/features/project/commits/commits.feature index 21367fd76a0..5bb2d0e976b 100644 --- a/features/project/commits/commits.feature +++ b/features/project/commits/commits.feature @@ -19,7 +19,8 @@ Feature: Project Commits Scenario: I browse commit with ci from list Given commit has ci status - And I click on commit link + And repository contains ".gitlab-ci.yml" file + When I click on commit link Then I see commit ci info And I click status link Then I see builds list diff --git a/features/steps/project/commits/commits.rb b/features/steps/project/commits/commits.rb index 101eb408ba1..a3141fe3be1 100644 --- a/features/steps/project/commits/commits.rb +++ b/features/steps/project/commits/commits.rb @@ -108,6 +108,10 @@ class Spinach::Features::ProjectCommits < Spinach::FeatureSteps create :ci_build, commit: ci_commit end + step 'repository contains ".gitlab-ci.yml" file' do + allow_any_instance_of(Ci::Commit).to receive(:ci_yaml_file).and_return(String.new) + end + step 'I see commit ci info' do expect(page).to have_content "build: pending" end From 0a81a681585bf699a7a41a449a2c0c21e2e335c6 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 14 Dec 2015 18:14:50 +0100 Subject: [PATCH 6/6] Update CHANGELOG [ci skip] --- CHANGELOG | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index 365b9e973fc..6ff06091f61 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -51,6 +51,9 @@ v 8.3.0 (unreleased) - Display referenced merge request statuses in the issue description (Greg Smethells) - Implement new sidebar for issue and merge request pages - Emoji picker improvements + - Suppress warning about missing `.gitlab-ci.yml` if builds are disabled + - Do not show build status unless builds are enabled and `.gitlab-ci.yml` is present + - Persist runners registration token in database v 8.2.3 - Fix application settings cache not expiring after changes (Stan Hu)