diff options
author | Douwe Maan <douwe@gitlab.com> | 2016-08-30 21:03:16 +0300 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2016-08-30 21:03:16 +0300 |
commit | 51087cfa1a6b0bb5a7abf35081ed3b669253eb4f (patch) | |
tree | 3a90808c1671f9ba1686c3a593e84081a3162c4b /spec | |
parent | 90c7a4117a68f5e019fed314076f6fcefbed9ac0 (diff) | |
parent | 41a0b7b22f7cdec7d216f32d561442c9fc3587be (diff) |
Merge branch 'artifacts-from-ref-and-build-name' into 'master'
Add a download buttons for Build Artifacts
## What does this MR do?
This MR adds a download buttons for build artifacts of latest succesful pipeline in:
- dashboard of project,
- branches and tags views,
- and tree viewer
Implement #4255
## What are the relevant issue numbers?
Closes #4255, Closes #14419
## Screenshots
### Project main
![](/uploads/29ee2154a214416059a875f2715d4fa3/Screen_Shot_2016-08-24_at_8.00.31_PM.png)
### Branches
![](/uploads/9220c593288370986fbc1d42a1425ef7/Screen_Shot_2016-08-24_at_8.02.01_PM.png)
### Tags
![](/uploads/a843e8103221fea475a0cf9d62a1999d/Screen_Shot_2016-08-24_at_8.03.32_PM.png)
### Source Tree
![](/uploads/63cd3c8c91b6f427c166dc90d8e3c059/Screen_Shot_2016-08-24_at_8.04.56_PM.png)
## Does this MR meet the acceptance criteria?
- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [x] Download buttons
- [x] Models
- [x] Routes
- [x] Projects::ArtifactsController
- [x] API
- Tests
- Rails
- [x] Project#builds_for
- [x] branch name with slashes
- [x] only success builds
- [x] only latest builds
- [x] feature tests for download buttons
- API
- [x] branch name with slashes
- [x] only success builds
- [x] only latest builds
- [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
See merge request !5142
Diffstat (limited to 'spec')
-rw-r--r-- | spec/features/projects/branches/download_buttons_spec.rb | 40 | ||||
-rw-r--r-- | spec/features/projects/builds_spec.rb (renamed from spec/features/builds_spec.rb) | 0 | ||||
-rw-r--r-- | spec/features/projects/files/download_buttons_spec.rb | 41 | ||||
-rw-r--r-- | spec/features/projects/main/download_buttons_spec.rb | 40 | ||||
-rw-r--r-- | spec/features/projects/tags/download_buttons_spec.rb | 41 | ||||
-rw-r--r-- | spec/models/build_spec.rb | 8 | ||||
-rw-r--r-- | spec/models/ci/pipeline_spec.rb | 30 | ||||
-rw-r--r-- | spec/models/merge_request_spec.rb | 4 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 38 | ||||
-rw-r--r-- | spec/requests/api/builds_spec.rb | 20 | ||||
-rw-r--r-- | spec/requests/api/commits_spec.rb | 4 | ||||
-rw-r--r-- | spec/requests/projects/artifacts_controller_spec.rb | 117 | ||||
-rw-r--r-- | spec/services/ci/image_for_build_service_spec.rb | 2 | ||||
-rw-r--r-- | spec/spec_helper.rb | 6 |
14 files changed, 363 insertions, 28 deletions
diff --git a/spec/features/projects/branches/download_buttons_spec.rb b/spec/features/projects/branches/download_buttons_spec.rb new file mode 100644 index 00000000000..04058300570 --- /dev/null +++ b/spec/features/projects/branches/download_buttons_spec.rb @@ -0,0 +1,40 @@ +require 'spec_helper' + +feature 'Download buttons in branches page', feature: true do + given(:user) { create(:user) } + given(:role) { :developer } + given(:status) { 'success' } + given(:project) { create(:project) } + + given(:pipeline) do + create(:ci_pipeline, + project: project, + sha: project.commit('binary-encoding').sha, + ref: 'binary-encoding', # make sure the branch is in the 1st page! + status: status) + end + + given!(:build) do + create(:ci_build, :success, :artifacts, + pipeline: pipeline, + status: pipeline.status, + name: 'build') + end + + background do + login_as(user) + project.team << [user, role] + end + + describe 'when checking branches' do + context 'with artifacts' do + before do + visit namespace_project_branches_path(project.namespace, project) + end + + scenario 'shows download artifacts button' do + expect(page).to have_link "Download '#{build.name}'" + end + end + end +end diff --git a/spec/features/builds_spec.rb b/spec/features/projects/builds_spec.rb index 0cfeb2e57d8..0cfeb2e57d8 100644 --- a/spec/features/builds_spec.rb +++ b/spec/features/projects/builds_spec.rb diff --git a/spec/features/projects/files/download_buttons_spec.rb b/spec/features/projects/files/download_buttons_spec.rb new file mode 100644 index 00000000000..be5cebcd7c9 --- /dev/null +++ b/spec/features/projects/files/download_buttons_spec.rb @@ -0,0 +1,41 @@ +require 'spec_helper' + +feature 'Download buttons in files tree', feature: true do + given(:user) { create(:user) } + given(:role) { :developer } + given(:status) { 'success' } + given(:project) { create(:project) } + + given(:pipeline) do + create(:ci_pipeline, + project: project, + sha: project.commit.sha, + ref: project.default_branch, + status: status) + end + + given!(:build) do + create(:ci_build, :success, :artifacts, + pipeline: pipeline, + status: pipeline.status, + name: 'build') + end + + background do + login_as(user) + project.team << [user, role] + end + + describe 'when files tree' do + context 'with artifacts' do + before do + visit namespace_project_tree_path( + project.namespace, project, project.default_branch) + end + + scenario 'shows download artifacts button' do + expect(page).to have_link "Download '#{build.name}'" + end + end + end +end diff --git a/spec/features/projects/main/download_buttons_spec.rb b/spec/features/projects/main/download_buttons_spec.rb new file mode 100644 index 00000000000..b26c0ea7a14 --- /dev/null +++ b/spec/features/projects/main/download_buttons_spec.rb @@ -0,0 +1,40 @@ +require 'spec_helper' + +feature 'Download buttons in project main page', feature: true do + given(:user) { create(:user) } + given(:role) { :developer } + given(:status) { 'success' } + given(:project) { create(:project) } + + given(:pipeline) do + create(:ci_pipeline, + project: project, + sha: project.commit.sha, + ref: project.default_branch, + status: status) + end + + given!(:build) do + create(:ci_build, :success, :artifacts, + pipeline: pipeline, + status: pipeline.status, + name: 'build') + end + + background do + login_as(user) + project.team << [user, role] + end + + describe 'when checking project main page' do + context 'with artifacts' do + before do + visit namespace_project_path(project.namespace, project) + end + + scenario 'shows download artifacts button' do + expect(page).to have_link "Download '#{build.name}'" + end + end + end +end diff --git a/spec/features/projects/tags/download_buttons_spec.rb b/spec/features/projects/tags/download_buttons_spec.rb new file mode 100644 index 00000000000..6e0022c179f --- /dev/null +++ b/spec/features/projects/tags/download_buttons_spec.rb @@ -0,0 +1,41 @@ +require 'spec_helper' + +feature 'Download buttons in tags page', feature: true do + given(:user) { create(:user) } + given(:role) { :developer } + given(:status) { 'success' } + given(:tag) { 'v1.0.0' } + given(:project) { create(:project) } + + given(:pipeline) do + create(:ci_pipeline, + project: project, + sha: project.commit(tag).sha, + ref: tag, + status: status) + end + + given!(:build) do + create(:ci_build, :success, :artifacts, + pipeline: pipeline, + status: pipeline.status, + name: 'build') + end + + background do + login_as(user) + project.team << [user, role] + end + + describe 'when checking tags' do + context 'with artifacts' do + before do + visit namespace_project_tags_path(project.namespace, project) + end + + scenario 'shows download artifacts button' do + expect(page).to have_link "Download '#{build.name}'" + end + end + end +end diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index ee2c3d04984..c45c2635cf4 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -948,15 +948,17 @@ describe Ci::Build, models: true do before { build.run! } it 'returns false' do - expect(build.retryable?).to be false + expect(build).not_to be_retryable end end context 'when build is finished' do - before { build.success! } + before do + build.success! + end it 'returns true' do - expect(build.retryable?).to be true + expect(build).to be_retryable end end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 721b20e0cb2..598df576001 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -195,6 +195,36 @@ describe Ci::Pipeline, models: true do end end + context 'with non-empty project' do + let(:project) { create(:project) } + + let(:pipeline) do + create(:ci_pipeline, + project: project, + ref: project.default_branch, + sha: project.commit.sha) + end + + describe '#latest?' do + context 'with latest sha' do + it 'returns true' do + expect(pipeline).to be_latest + end + end + + context 'with not latest sha' do + before do + pipeline.update( + sha: project.commit("#{project.default_branch}~1").sha) + end + + it 'returns false' do + expect(pipeline).not_to be_latest + end + end + end + end + describe '#manual_actions' do subject { pipeline.manual_actions } diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 901b7bad007..5bf3b8e609e 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -477,8 +477,8 @@ describe MergeRequest, models: true do allow(subject).to receive(:diff_head_sha).and_return('123abc') - expect(subject.source_project).to receive(:pipeline). - with('123abc', 'master'). + expect(subject.source_project).to receive(:pipeline_for). + with('master', '123abc'). and_return(pipeline) expect(subject.pipeline).to eq(pipeline) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 3b637b0defc..dd309ea1b68 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -697,23 +697,37 @@ describe Project, models: true do end end - describe '#pipeline' do - let(:project) { create :project } - let(:pipeline) { create :ci_pipeline, project: project, ref: 'master' } - - subject { project.pipeline(pipeline.sha, 'master') } + describe '#pipeline_for' do + let(:project) { create(:project) } + let!(:pipeline) { create_pipeline } - it { is_expected.to eq(pipeline) } + shared_examples 'giving the correct pipeline' do + it { is_expected.to eq(pipeline) } - context 'return latest' do - let(:pipeline2) { create :ci_pipeline, project: project, ref: 'master' } + context 'return latest' do + let!(:pipeline2) { create_pipeline } - before do - pipeline - pipeline2 + it { is_expected.to eq(pipeline2) } end + end + + context 'with explicit sha' do + subject { project.pipeline_for('master', pipeline.sha) } + + it_behaves_like 'giving the correct pipeline' + end + + context 'with implicit sha' do + subject { project.pipeline_for('master') } + + it_behaves_like 'giving the correct pipeline' + end - it { is_expected.to eq(pipeline2) } + def create_pipeline + create(:ci_pipeline, + project: project, + ref: 'master', + sha: project.commit('master').sha) end end diff --git a/spec/requests/api/builds_spec.rb b/spec/requests/api/builds_spec.rb index 9a17a705b1e..ee0b61e2ca4 100644 --- a/spec/requests/api/builds_spec.rb +++ b/spec/requests/api/builds_spec.rb @@ -15,7 +15,9 @@ describe API::API, api: true do describe 'GET /projects/:id/builds ' do let(:query) { '' } - before { get api("/projects/#{project.id}/builds?#{query}", api_user) } + before do + get api("/projects/#{project.id}/builds?#{query}", api_user) + end context 'authorized user' do it 'returns project builds' do @@ -122,7 +124,9 @@ describe API::API, api: true do end describe 'GET /projects/:id/builds/:build_id' do - before { get api("/projects/#{project.id}/builds/#{build.id}", api_user) } + before do + get api("/projects/#{project.id}/builds/#{build.id}", api_user) + end context 'authorized user' do it 'returns specific build data' do @@ -141,7 +145,9 @@ describe API::API, api: true do end describe 'GET /projects/:id/builds/:build_id/artifacts' do - before { get api("/projects/#{project.id}/builds/#{build.id}/artifacts", api_user) } + before do + get api("/projects/#{project.id}/builds/#{build.id}/artifacts", api_user) + end context 'build with artifacts' do let(:build) { create(:ci_build, :artifacts, pipeline: pipeline) } @@ -292,7 +298,9 @@ describe API::API, api: true do end describe 'POST /projects/:id/builds/:build_id/cancel' do - before { post api("/projects/#{project.id}/builds/#{build.id}/cancel", api_user) } + before do + post api("/projects/#{project.id}/builds/#{build.id}/cancel", api_user) + end context 'authorized user' do context 'user with :update_build persmission' do @@ -323,7 +331,9 @@ describe API::API, api: true do describe 'POST /projects/:id/builds/:build_id/retry' do let(:build) { create(:ci_build, :canceled, pipeline: pipeline) } - before { post api("/projects/#{project.id}/builds/#{build.id}/retry", api_user) } + before do + post api("/projects/#{project.id}/builds/#{build.id}/retry", api_user) + end context 'authorized user' do context 'user with :update_build permission' do diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index 7ca75d77673..5b3dc60aba2 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -95,7 +95,7 @@ describe API::API, api: true do end it "returns status for CI" do - pipeline = project.ensure_pipeline(project.repository.commit.sha, 'master') + pipeline = project.ensure_pipeline('master', project.repository.commit.sha) pipeline.update(status: 'success') get api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}", user) @@ -105,7 +105,7 @@ describe API::API, api: true do end it "returns status for CI when pipeline is created" do - project.ensure_pipeline(project.repository.commit.sha, 'master') + project.ensure_pipeline('master', project.repository.commit.sha) get api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}", user) diff --git a/spec/requests/projects/artifacts_controller_spec.rb b/spec/requests/projects/artifacts_controller_spec.rb new file mode 100644 index 00000000000..e02f0eacc93 --- /dev/null +++ b/spec/requests/projects/artifacts_controller_spec.rb @@ -0,0 +1,117 @@ +require 'spec_helper' + +describe Projects::ArtifactsController do + let(:user) { create(:user) } + let(:project) { create(:project) } + + let(:pipeline) do + create(:ci_pipeline, + project: project, + sha: project.commit.sha, + ref: project.default_branch, + status: 'success') + end + + let(:build) { create(:ci_build, :success, :artifacts, pipeline: pipeline) } + + describe 'GET /:project/builds/artifacts/:ref_name/browse?job=name' do + before do + project.team << [user, :developer] + + login_as(user) + end + + def path_from_ref( + ref = pipeline.ref, job = build.name, path = 'browse') + latest_succeeded_namespace_project_artifacts_path( + project.namespace, + project, + [ref, path].join('/'), + job: job) + end + + context 'cannot find the build' do + shared_examples 'not found' do + it { expect(response).to have_http_status(:not_found) } + end + + context 'has no such ref' do + before do + get path_from_ref('TAIL', build.name) + end + + it_behaves_like 'not found' + end + + context 'has no such build' do + before do + get path_from_ref(pipeline.ref, 'NOBUILD') + end + + it_behaves_like 'not found' + end + + context 'has no path' do + before do + get path_from_ref(pipeline.sha, build.name, '') + end + + it_behaves_like 'not found' + end + end + + context 'found the build and redirect' do + shared_examples 'redirect to the build' do + it 'redirects' do + path = browse_namespace_project_build_artifacts_path( + project.namespace, + project, + build) + + expect(response).to redirect_to(path) + end + end + + context 'with regular branch' do + before do + pipeline.update(ref: 'master', + sha: project.commit('master').sha) + + get path_from_ref('master') + end + + it_behaves_like 'redirect to the build' + end + + context 'with branch name containing slash' do + before do + pipeline.update(ref: 'improve/awesome', + sha: project.commit('improve/awesome').sha) + + get path_from_ref('improve/awesome') + end + + it_behaves_like 'redirect to the build' + end + + context 'with branch name and path containing slashes' do + before do + pipeline.update(ref: 'improve/awesome', + sha: project.commit('improve/awesome').sha) + + get path_from_ref('improve/awesome', build.name, 'file/README.md') + end + + it 'redirects' do + path = file_namespace_project_build_artifacts_path( + project.namespace, + project, + build, + 'README.md') + + expect(response).to redirect_to(path) + end + end + end + end +end diff --git a/spec/services/ci/image_for_build_service_spec.rb b/spec/services/ci/image_for_build_service_spec.rb index c931c3e4829..b3e0a7b9b58 100644 --- a/spec/services/ci/image_for_build_service_spec.rb +++ b/spec/services/ci/image_for_build_service_spec.rb @@ -5,7 +5,7 @@ module Ci let(:service) { ImageForBuildService.new } let(:project) { FactoryGirl.create(:empty_project) } let(:commit_sha) { '01234567890123456789' } - let(:pipeline) { project.ensure_pipeline(commit_sha, 'master') } + let(:pipeline) { project.ensure_pipeline('master', commit_sha) } let(:build) { FactoryGirl.create(:ci_build, pipeline: pipeline) } describe '#execute' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index c144cd85487..02b2b3ca101 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -26,9 +26,9 @@ RSpec.configure do |config| config.verbose_retry = true config.display_try_failure_messages = true - config.include Devise::TestHelpers, type: :controller - config.include LoginHelpers, type: :feature - config.include LoginHelpers, type: :request + config.include Devise::TestHelpers, type: :controller + config.include Warden::Test::Helpers, type: :request + config.include LoginHelpers, type: :feature config.include StubConfiguration config.include EmailHelpers config.include TestEnv |