From d2601211a0c7a44666501dea82a8488b08f8faa7 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Tue, 29 Dec 2015 23:12:36 +0100 Subject: Add specs for build listings in API --- spec/requests/api/builds_spec.rb | 52 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 spec/requests/api/builds_spec.rb (limited to 'spec') diff --git a/spec/requests/api/builds_spec.rb b/spec/requests/api/builds_spec.rb new file mode 100644 index 00000000000..81c176c9fb0 --- /dev/null +++ b/spec/requests/api/builds_spec.rb @@ -0,0 +1,52 @@ +require 'spec_helper' + +describe API::API, api: true do + include ApiHelpers + + let(:user) { create(:user) } + let(:user2) { create(:user) } + let!(:project) { create(:project, creator_id: user.id) } + let!(:master) { create(:project_member, user: user, project: project, access_level: ProjectMember::MASTER) } + let!(:guest) { create(:project_member, user: user2, project: project, access_level: ProjectMember::GUEST) } + + describe 'GET /projects/:id/builds ' do + context 'authorized user' do + it 'should return project builds' do + get api("/projects/#{project.id}/builds", user) + + puts json_response + expect(response.status).to eq(200) + expect(json_response).to be_an Array + end + end + + context 'unauthorized user' do + it 'should not return project builds' do + get api("/projects/#{project.id}/builds") + + expect(response.status).to eq(401) + end + end + end + + describe 'GET /projects/:id/builds/commit/:sha' do + context 'authorized user' do + it 'should return project builds for specific commit' do + project.ensure_ci_commit(project.repository.commit.sha) + get api("/projects/#{project.id}/builds/commit/#{project.ci_commits.first.sha}", user) + + expect(response.status).to eq(200) + expect(json_response).to be_an Array + end + end + + context 'unauthorized user' do + it 'should not return project builds' do + project.ensure_ci_commit(project.repository.commit.sha) + get api("/projects/#{project.id}/builds/commit/#{project.ci_commits.first.sha}") + + expect(response.status).to eq(401) + end + end + end +end -- cgit v1.2.1 From 593d87ea54eec4d60cf7eeb404af82d9e015b066 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Wed, 30 Dec 2015 15:12:07 +0100 Subject: Add specs for build details/traces features in builds API --- spec/factories/ci/builds.rb | 6 ++++++ spec/requests/api/builds_spec.rb | 40 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 44 insertions(+), 2 deletions(-) (limited to 'spec') diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index f76e826f138..4551ee57d78 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -30,6 +30,7 @@ FactoryGirl.define do name 'test' ref 'master' tag false + created_at 'Di 29. Okt 09:50:00 CET 2013' started_at 'Di 29. Okt 09:51:28 CET 2013' finished_at 'Di 29. Okt 09:53:28 CET 2013' commands 'ls -a' @@ -54,5 +55,10 @@ FactoryGirl.define do factory :ci_build_tag do tag true end + + factory :ci_build_with_trace do + id 999 + trace 'BUILD TRACE' + end end end diff --git a/spec/requests/api/builds_spec.rb b/spec/requests/api/builds_spec.rb index 81c176c9fb0..c68ea0898b8 100644 --- a/spec/requests/api/builds_spec.rb +++ b/spec/requests/api/builds_spec.rb @@ -8,6 +8,9 @@ describe API::API, api: true do let!(:project) { create(:project, creator_id: user.id) } let!(:master) { create(:project_member, user: user, project: project, access_level: ProjectMember::MASTER) } let!(:guest) { create(:project_member, user: user2, project: project, access_level: ProjectMember::GUEST) } + let(:commit) { create(:ci_commit, project: project)} + let(:build) { create(:ci_build, commit: commit) } + let(:build_with_trace) { create(:ci_build_with_trace, commit: commit) } describe 'GET /projects/:id/builds ' do context 'authorized user' do @@ -32,7 +35,7 @@ describe API::API, api: true do describe 'GET /projects/:id/builds/commit/:sha' do context 'authorized user' do it 'should return project builds for specific commit' do - project.ensure_ci_commit(project.repository.commit.sha) + project.ensure_ci_commit(commit.sha) get api("/projects/#{project.id}/builds/commit/#{project.ci_commits.first.sha}", user) expect(response.status).to eq(200) @@ -42,11 +45,44 @@ describe API::API, api: true do context 'unauthorized user' do it 'should not return project builds' do - project.ensure_ci_commit(project.repository.commit.sha) + project.ensure_ci_commit(commit.sha) get api("/projects/#{project.id}/builds/commit/#{project.ci_commits.first.sha}") expect(response.status).to eq(401) end end end + + describe 'GET /projects/:id/builds/:build_id(/trace)?' do + context 'authorized user' do + it 'should return specific build data' do + get api("/projects/#{project.id}/builds/#{build.id}", user) + + expect(response.status).to eq(200) + expect(json_response['name']).to eq('test') + expect(json_response['commit']['sha']).to eq(commit.sha) + end + + it 'should return specific build trace' do + get api("/projects/#{project.id}/builds/#{build_with_trace.id}/trace", user) + + expect(response.status).to eq(200) + expect(response.body).to eq(build_with_trace.trace) + end + end + + context 'unauthorized user' do + it 'should not return specific build data' do + get api("/projects/#{project.id}/builds/#{build.id}") + + expect(response.status).to eq(401) + end + + it 'should not return specific build trace' do + get api("/projects/#{project.id}/builds/#{build_with_trace.id}/trace") + + expect(response.status).to eq(401) + end + end + end end -- cgit v1.2.1 From a17bf380cb4c90696349f268ca4a8c2fedc1f545 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Wed, 30 Dec 2015 16:37:47 +0100 Subject: Add cancel/retry features to builds API --- spec/factories/ci/builds.rb | 4 +++ spec/requests/api/builds_spec.rb | 62 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 65 insertions(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index 4551ee57d78..ce68457f86b 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -60,5 +60,9 @@ FactoryGirl.define do id 999 trace 'BUILD TRACE' end + + factory :ci_build_canceled do + status 'canceled' + end end end diff --git a/spec/requests/api/builds_spec.rb b/spec/requests/api/builds_spec.rb index c68ea0898b8..d4af7639d4b 100644 --- a/spec/requests/api/builds_spec.rb +++ b/spec/requests/api/builds_spec.rb @@ -7,10 +7,11 @@ describe API::API, api: true do let(:user2) { create(:user) } let!(:project) { create(:project, creator_id: user.id) } let!(:master) { create(:project_member, user: user, project: project, access_level: ProjectMember::MASTER) } - let!(:guest) { create(:project_member, user: user2, project: project, access_level: ProjectMember::GUEST) } + let!(:reporter) { create(:project_member, user: user2, project: project, access_level: ProjectMember::REPORTER) } let(:commit) { create(:ci_commit, project: project)} let(:build) { create(:ci_build, commit: commit) } let(:build_with_trace) { create(:ci_build_with_trace, commit: commit) } + let(:build_canceled) { create(:ci_build_canceled, commit: commit) } describe 'GET /projects/:id/builds ' do context 'authorized user' do @@ -85,4 +86,63 @@ describe API::API, api: true do end end end + + describe 'GET /projects/:id/builds/:build_id/cancel' do + context 'authorized user' do + context 'user with :manage_builds persmission' do + it 'should cancel running or pending build' do + post api("/projects/#{project.id}/builds/#{build.id}/cancel", user) + + expect(response.status).to eq(201) + expect(project.builds.first.status).to eq('canceled') + end + end + + context 'user without :manage_builds permission' do + it 'should not cancel build' do + post api("/projects/#{project.id}/builds/#{build.id}/cancel", user2) + + expect(response.status).to eq(403) + end + end + end + + context 'unauthorized user' do + it 'should not cancel build' do + post api("/projects/#{project.id}/builds/#{build.id}/cancel") + + expect(response.status).to eq(401) + end + end + end + + describe 'GET /projects/:id/builds/:build_id/retry' do + context 'authorized user' do + context 'user with :manage_builds persmission' do + it 'should retry non-running build' do + post api("/projects/#{project.id}/builds/#{build_canceled.id}/retry", user) + + expect(response.status).to eq(201) + expect(project.builds.first.status).to eq('canceled') + expect(json_response['status']).to eq('pending') + end + end + + context 'user without :manage_builds permission' do + it 'should not retry build' do + post api("/projects/#{project.id}/builds/#{build_canceled.id}/retry", user2) + + expect(response.status).to eq(403) + end + end + end + + context 'unauthorized user' do + it 'should not retry build' do + post api("/projects/#{project.id}/builds/#{build_canceled.id}/retry") + + expect(response.status).to eq(401) + end + end + end end -- cgit v1.2.1 From ea4777ff501e370a39ae30e76a955136afe3c1fa Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Thu, 31 Dec 2015 15:19:13 +0100 Subject: Add features for list and show details of variables in API --- spec/factories/ci/variables.rb | 25 +++++++++++++ spec/requests/api/variables_spec.rb | 75 +++++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+) create mode 100644 spec/factories/ci/variables.rb create mode 100644 spec/requests/api/variables_spec.rb (limited to 'spec') diff --git a/spec/factories/ci/variables.rb b/spec/factories/ci/variables.rb new file mode 100644 index 00000000000..c3dcb678da7 --- /dev/null +++ b/spec/factories/ci/variables.rb @@ -0,0 +1,25 @@ +# == Schema Information +# +# Table name: ci_variables +# +# id :integer not null, primary key +# project_id :integer not null +# key :string(255) +# value :text +# encrypted_value :text +# encrypted_value_salt :string(255) +# encrypted_value_iv :string(255) +# gl_project_id :integer +# + +# Read about factories at https://github.com/thoughtbot/factory_girl + +FactoryGirl.define do + factory :ci_variable, class: Ci::Variable do + id 1 + key 'TEST_VARIABLE_1' + value 'VALUE_1' + + project factory: :empty_project + end +end diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb new file mode 100644 index 00000000000..8f66f5432b6 --- /dev/null +++ b/spec/requests/api/variables_spec.rb @@ -0,0 +1,75 @@ +require 'spec_helper' + +describe API::API, api: true do + include ApiHelpers + + let(:user) { create(:user) } + let(:user2) { create(:user) } + let!(:project) { create(:project, creator_id: user.id) } + let!(:master) { create(:project_member, user: user, project: project, access_level: ProjectMember::MASTER) } + let!(:developer) { create(:project_member, user: user2, project: project, access_level: ProjectMember::DEVELOPER) } + let!(:variable) { create(:ci_variable, project: project) } + + describe 'GET /projects/:id/variables' do + context 'authorized user with proper permissions' do + it 'should return project variables' do + get api("/projects/#{project.id}/variables", user) + + expect(response.status).to eq(200) + expect(json_response).to be_a(Array) + end + end + + context 'authorized user with invalid permissions' do + it 'should not return project variables' do + get api("/projects/#{project.id}/variables", user2) + + expect(response.status).to eq(403) + end + end + + context 'unauthorized user' do + it 'should not return project variables' do + get api("/projects/#{project.id}/variables") + + expect(response.status).to eq(401) + end + end + end + + describe 'GET /projects/:id/variables/:variable_id' do + context 'authorized user with proper permissions' do + it 'should return project variable details when ID is used as :variable_id' do + get api("/projects/#{project.id}/variables/1", user) + + expect(response.status).to eq(200) + expect(json_response['key']).to eq('TEST_VARIABLE_1') + expect(json_response['value']).to eq('VALUE_1') + end + + it 'should return project variable details when `key` is used as :variable_id' do + get api("/projects/#{project.id}/variables/TEST_VARIABLE_1", user) + + expect(response.status).to eq(200) + expect(json_response['id']).to eq(1) + expect(json_response['value']).to eq('VALUE_1') + end + end + + context 'authorized user with invalid permissions' do + it 'should not return project variable details' do + get api("/projects/#{project.id}/variables/1", user2) + + expect(response.status).to eq(403) + end + end + + context 'unauthorized user' do + it 'should not return project variable details' do + get api("/projects/#{project.id}/variables/1") + + expect(response.status).to eq(401) + end + end + end +end -- cgit v1.2.1 From a692ce1c079703c4f3947e1d0a29547189e94d0f Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Thu, 31 Dec 2015 16:25:49 +0100 Subject: Add update feature for variables API --- spec/requests/api/variables_spec.rb | 52 +++++++++++++++++++++++++++++++------ 1 file changed, 44 insertions(+), 8 deletions(-) (limited to 'spec') diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb index 8f66f5432b6..3f58277c4ae 100644 --- a/spec/requests/api/variables_spec.rb +++ b/spec/requests/api/variables_spec.rb @@ -40,25 +40,25 @@ describe API::API, api: true do describe 'GET /projects/:id/variables/:variable_id' do context 'authorized user with proper permissions' do it 'should return project variable details when ID is used as :variable_id' do - get api("/projects/#{project.id}/variables/1", user) + get api("/projects/#{project.id}/variables/#{variable.id}", user) expect(response.status).to eq(200) - expect(json_response['key']).to eq('TEST_VARIABLE_1') - expect(json_response['value']).to eq('VALUE_1') + expect(json_response['key']).to eq(variable.key) + expect(json_response['value']).to eq(variable.value) end it 'should return project variable details when `key` is used as :variable_id' do - get api("/projects/#{project.id}/variables/TEST_VARIABLE_1", user) + get api("/projects/#{project.id}/variables/#{variable.key}", user) expect(response.status).to eq(200) - expect(json_response['id']).to eq(1) - expect(json_response['value']).to eq('VALUE_1') + expect(json_response['id']).to eq(variable.id) + expect(json_response['value']).to eq(variable.value) end end context 'authorized user with invalid permissions' do it 'should not return project variable details' do - get api("/projects/#{project.id}/variables/1", user2) + get api("/projects/#{project.id}/variables/#{variable.id}", user2) expect(response.status).to eq(403) end @@ -66,7 +66,43 @@ describe API::API, api: true do context 'unauthorized user' do it 'should not return project variable details' do - get api("/projects/#{project.id}/variables/1") + get api("/projects/#{project.id}/variables/#{variable.id}") + + expect(response.status).to eq(401) + end + end + end + + describe 'PUT /projects/:id/variables/:variable_id' do + context 'authorized user with proper permissions' do + it 'should update variable data' do + initial_variable = project.variables.first + key_before = initial_variable.key + value_before = initial_variable.value + + put api("/projects/#{project.id}/variables/#{variable.id}", user), key: 'TEST_VARIABLE_1_UP', value: 'VALUE_1_UP' + + updated_variable = project.variables.first + + expect(response.status).to eq(200) + expect(key_before).to eq(variable.key) + expect(value_before).to eq(variable.value) + expect(updated_variable.key).to eq('TEST_VARIABLE_1_UP') + expect(updated_variable.value).to eq('VALUE_1_UP') + end + end + + context 'authorized user with invalid permissions' do + it 'should not update variable' do + put api("/projects/#{project.id}/variables/#{variable.id}", user2) + + expect(response.status).to eq(403) + end + end + + context 'unauthorized user' do + it 'should not return project variable details' do + put api("/projects/#{project.id}/variables/#{variable.id}") expect(response.status).to eq(401) end -- cgit v1.2.1 From 0d014feb1d216e692882976f0d70c3227eaec4ca Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Thu, 31 Dec 2015 16:56:03 +0100 Subject: Add delete feature to variables API --- spec/requests/api/variables_spec.rb | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb index 3f58277c4ae..385db2409bd 100644 --- a/spec/requests/api/variables_spec.rb +++ b/spec/requests/api/variables_spec.rb @@ -101,11 +101,38 @@ describe API::API, api: true do end context 'unauthorized user' do - it 'should not return project variable details' do + it 'should not update variable' do put api("/projects/#{project.id}/variables/#{variable.id}") expect(response.status).to eq(401) end end end + + describe 'DELETE /projects/:id/variables/:variable_id' do + context 'authorized user with proper permissions' do + it 'should delete variable' do + expect do + delete api("/projects/#{project.id}/variables/#{variable.id}", user) + end.to change{project.variables.count}.by(-1) + expect(response.status).to eq(200) + end + end + + context 'authorized user with invalid permissions' do + it 'should not delete variable' do + delete api("/projects/#{project.id}/variables/#{variable.id}", user2) + + expect(response.status).to eq(403) + end + end + + context 'unauthorized user' do + it 'should not delete variable' do + delete api("/projects/#{project.id}/variables/#{variable.id}") + + expect(response.status).to eq(401) + end + end + end end -- cgit v1.2.1 From c5177dd5e2171b047a695802c979cf779522ba8a Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Thu, 31 Dec 2015 17:03:11 +0100 Subject: Add missing 'not_found' checks in variables API --- spec/requests/api/variables_spec.rb | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'spec') diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb index 385db2409bd..b35ee2d32d1 100644 --- a/spec/requests/api/variables_spec.rb +++ b/spec/requests/api/variables_spec.rb @@ -54,6 +54,12 @@ describe API::API, api: true do expect(json_response['id']).to eq(variable.id) expect(json_response['value']).to eq(variable.value) end + + it 'should responde with 404 Not Found if requesting non-existing variable' do + get api("/projects/#{project.id}/variables/9999", user) + + expect(response.status).to eq(404) + end end context 'authorized user with invalid permissions' do @@ -90,6 +96,12 @@ describe API::API, api: true do expect(updated_variable.key).to eq('TEST_VARIABLE_1_UP') expect(updated_variable.value).to eq('VALUE_1_UP') end + + it 'should responde with 404 Not Found if requesting non-existing variable' do + put api("/projects/#{project.id}/variables/9999", user) + + expect(response.status).to eq(404) + end end context 'authorized user with invalid permissions' do @@ -117,6 +129,12 @@ describe API::API, api: true do end.to change{project.variables.count}.by(-1) expect(response.status).to eq(200) end + + it 'should responde with 404 Not Found if requesting non-existing variable' do + delete api("/projects/#{project.id}/variables/9999", user) + + expect(response.status).to eq(404) + end end context 'authorized user with invalid permissions' do -- cgit v1.2.1 From 937567b767e6d7b34dcaa1d9c83fc75464638683 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Thu, 31 Dec 2015 22:30:07 +0100 Subject: Add create feature to variables API --- spec/factories/ci/variables.rb | 2 +- spec/requests/api/variables_spec.rb | 38 +++++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/factories/ci/variables.rb b/spec/factories/ci/variables.rb index c3dcb678da7..a19b9fc72f2 100644 --- a/spec/factories/ci/variables.rb +++ b/spec/factories/ci/variables.rb @@ -16,7 +16,7 @@ FactoryGirl.define do factory :ci_variable, class: Ci::Variable do - id 1 + id 10 key 'TEST_VARIABLE_1' value 'VALUE_1' diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb index b35ee2d32d1..bf0dd77473a 100644 --- a/spec/requests/api/variables_spec.rb +++ b/spec/requests/api/variables_spec.rb @@ -79,6 +79,44 @@ describe API::API, api: true do end end + describe 'POST /projects/:id/variables' do + context 'authorized user with proper permissions' do + it 'should create variable' do + expect do + post api("/projects/#{project.id}/variables", user), key: 'TEST_VARIABLE_2', value: 'VALUE_2' + end.to change{project.variables.count}.by(1) + + expect(response.status).to eq(201) + expect(json_response['key']).to eq('TEST_VARIABLE_2') + expect(json_response['value']).to eq('VALUE_2') + end + + it 'should not allow to duplicate variable key' do + expect do + post api("/projects/#{project.id}/variables", user), key: 'TEST_VARIABLE_1', value: 'VALUE_2' + end.to change{project.variables.count}.by(0) + + expect(response.status).to eq(400) + end + end + + context 'authorized user with invalid permissions' do + it 'should not create variable' do + post api("/projects/#{project.id}/variables", user2) + + expect(response.status).to eq(403) + end + end + + context 'unauthorized user' do + it 'should not create variable' do + post api("/projects/#{project.id}/variables") + + expect(response.status).to eq(401) + end + end + end + describe 'PUT /projects/:id/variables/:variable_id' do context 'authorized user with proper permissions' do it 'should update variable data' do -- cgit v1.2.1 From d9da81f736b770bb44c4869aef5d5c455e74ab7a Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Mon, 4 Jan 2016 16:38:32 +0100 Subject: Add triggers feature to API --- spec/factories/ci/trigger_requests.rb | 2 ++ spec/requests/api/triggers_spec.rb | 45 +++++++++++++++++++++++++++++++---- 2 files changed, 43 insertions(+), 4 deletions(-) (limited to 'spec') diff --git a/spec/factories/ci/trigger_requests.rb b/spec/factories/ci/trigger_requests.rb index db053c610cd..5298d9fa7c3 100644 --- a/spec/factories/ci/trigger_requests.rb +++ b/spec/factories/ci/trigger_requests.rb @@ -3,6 +3,8 @@ FactoryGirl.define do factory :ci_trigger_request, class: Ci::TriggerRequest do factory :ci_trigger_request_with_variables do + trigger :ci_trigger + variables do { TRIGGER_KEY: 'TRIGGER_VALUE' diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb index 314bd7ddc59..4b356108c80 100644 --- a/spec/requests/api/triggers_spec.rb +++ b/spec/requests/api/triggers_spec.rb @@ -3,11 +3,19 @@ require 'spec_helper' describe API::API do include ApiHelpers + let(:user) { create(:user) } + let(:user2) { create(:user) } + let!(:trigger_token) { 'secure token' } + let!(:trigger_token_2) { 'secure token 2' } + let!(:project) { create(:project, creator_id: user.id) } + let!(:master) { create(:project_member, user: user, project: project, access_level: ProjectMember::MASTER) } + let!(:developer) { create(:project_member, user: user2, project: project, access_level: ProjectMember::DEVELOPER) } + let!(:trigger) { create(:ci_trigger, project: project, token: trigger_token) } + let!(:trigger2) { create(:ci_trigger, project: project, token: trigger_token_2) } + let!(:trigger_request) { create(:ci_trigger_request, trigger: trigger, created_at: '2015-01-01 12:13:14') } + describe 'POST /projects/:project_id/trigger' do - let!(:trigger_token) { 'secure token' } - let!(:project) { FactoryGirl.create(:project) } - let!(:project2) { FactoryGirl.create(:empty_project) } - let!(:trigger) { FactoryGirl.create(:ci_trigger, project: project, token: trigger_token) } + let!(:project2) { create(:empty_project) } let(:options) do { token: trigger_token @@ -77,4 +85,33 @@ describe API::API do end end end + + describe 'GET /projects/:id/triggets' do + context 'authenticated user with valid permissions' do + it 'should return list of triggers' do + get api("/projects/#{project.id}/triggers", user) + + expect(response.status).to eq(200) + expect(json_response).to be_a(Array) + expect(json_response[0]['token']).to eq(trigger_token) + expect(json_response[1]['token']).to eq(trigger_token_2) + end + end + + context 'authenticated user with invalid permissions' do + it 'should not return triggers list' do + get api("/projects/#{project.id}/triggers", user2) + + expect(response.status).to eq(403) + end + end + + context 'unauthentikated user' do + it 'should not return triggers list' do + get api("/projects/#{project.id}/triggers") + + expect(response.status).to eq(401) + end + end + end end -- cgit v1.2.1 From 3098500835aad26748d982fa81c84a2deed97931 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Tue, 5 Jan 2016 11:01:44 +0100 Subject: Fix ci_trigger_request factory --- spec/factories/ci/trigger_requests.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/factories/ci/trigger_requests.rb b/spec/factories/ci/trigger_requests.rb index 5298d9fa7c3..2c0d004d267 100644 --- a/spec/factories/ci/trigger_requests.rb +++ b/spec/factories/ci/trigger_requests.rb @@ -3,7 +3,7 @@ FactoryGirl.define do factory :ci_trigger_request, class: Ci::TriggerRequest do factory :ci_trigger_request_with_variables do - trigger :ci_trigger + trigger factory: :ci_trigger variables do { -- cgit v1.2.1 From f00607431cd13a952731e36701ebc3b39e64d09b Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Tue, 5 Jan 2016 11:27:38 +0100 Subject: Add delete feature to triggers API --- spec/requests/api/triggers_spec.rb | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) (limited to 'spec') diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb index 4b356108c80..4e073a55d9c 100644 --- a/spec/requests/api/triggers_spec.rb +++ b/spec/requests/api/triggers_spec.rb @@ -114,4 +114,37 @@ describe API::API do end end end + + describe 'DELETE /projects/:id/triggets/:trigger_id' do + context 'authenticated user with valid permissions' do + it 'should delete trigger' do + expect do + delete api("/projects/#{project.id}/triggers/#{trigger.id}", user) + end.to change{project.triggers.count}.by(-1) + expect(response.status).to eq(200) + end + + it 'should responde with 404 Not Found if requesting non-existing trigger' do + delete api("/projects/#{project.id}/triggers/9999", user) + + expect(response.status).to eq(404) + end + end + + context 'authenticated user with invalid permissions' do + it 'should not delete trigger' do + delete api("/projects/#{project.id}/triggers/#{trigger.id}", user2) + + expect(response.status).to eq(403) + end + end + + context 'unauthentikated user' do + it 'should not delete trigger' do + delete api("/projects/#{project.id}/triggers/#{trigger.id}") + + expect(response.status).to eq(401) + end + end + end end -- cgit v1.2.1 From 49c8bf4e9b510be51859dcc301cb46b29b750cb0 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Tue, 5 Jan 2016 11:44:10 +0100 Subject: Add create feature to triggers API --- spec/requests/api/triggers_spec.rb | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) (limited to 'spec') diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb index 4e073a55d9c..316c2ae958d 100644 --- a/spec/requests/api/triggers_spec.rb +++ b/spec/requests/api/triggers_spec.rb @@ -115,6 +115,35 @@ describe API::API do end end + describe 'POST /projects/:id/triggets' do + context 'authenticated user with valid permissions' do + it 'should create trigger' do + expect do + post api("/projects/#{project.id}/triggers", user) + end.to change{project.triggers.count}.by(1) + + expect(response.status).to eq(201) + expect(json_response).to be_a(Hash) + end + end + + context 'authenticated user with invalid permissions' do + it 'should not create trigger' do + post api("/projects/#{project.id}/triggers", user2) + + expect(response.status).to eq(403) + end + end + + context 'unauthentikated user' do + it 'should not create trigger' do + post api("/projects/#{project.id}/triggers") + + expect(response.status).to eq(401) + end + end + end + describe 'DELETE /projects/:id/triggets/:trigger_id' do context 'authenticated user with valid permissions' do it 'should delete trigger' do -- cgit v1.2.1 From 8675664655c4e0f1e043afa88ff1fd75ae5a6a9e Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Tue, 5 Jan 2016 12:25:16 +0100 Subject: Get show details feature to triggers API --- spec/requests/api/triggers_spec.rb | 56 ++++++++++++++++++++++++++++++++++---- 1 file changed, 51 insertions(+), 5 deletions(-) (limited to 'spec') diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb index 316c2ae958d..c1c2bb04b29 100644 --- a/spec/requests/api/triggers_spec.rb +++ b/spec/requests/api/triggers_spec.rb @@ -5,8 +5,8 @@ describe API::API do let(:user) { create(:user) } let(:user2) { create(:user) } - let!(:trigger_token) { 'secure token' } - let!(:trigger_token_2) { 'secure token 2' } + let!(:trigger_token) { 'secure_token' } + let!(:trigger_token_2) { 'secure_token_2' } let!(:project) { create(:project, creator_id: user.id) } let!(:master) { create(:project_member, user: user, project: project, access_level: ProjectMember::MASTER) } let!(:developer) { create(:project_member, user: user2, project: project, access_level: ProjectMember::DEVELOPER) } @@ -86,7 +86,7 @@ describe API::API do end end - describe 'GET /projects/:id/triggets' do + describe 'GET /projects/:id/triggers' do context 'authenticated user with valid permissions' do it 'should return list of triggers' do get api("/projects/#{project.id}/triggers", user) @@ -115,7 +115,53 @@ describe API::API do end end - describe 'POST /projects/:id/triggets' do + describe 'GET /projects/:id/triggers/:triggers_id' do + context 'authenticated user with valid permissions' do + context 'ID is used as :trigger_id' do + it 'should return trigger details' do + get api("/projects/#{project.id}/triggers/#{trigger.id}", user) + + expect(response.status).to eq(200) + expect(json_response).to be_a(Hash) + expect(json_response['token']).to eq(trigger_token) + end + end + + context '`token` is used as :trigger_id' do + it 'should return trigger details' do + get api("/projects/#{project.id}/triggers/#{trigger.token}", user) + + expect(response.status).to eq(200) + expect(json_response).to be_a(Hash) + expect(json_response['id']).to eq(trigger.id) + end + end + + it 'should responde with 404 Not Found if requesting non-existing trigger' do + get api("/projects/#{project.id}/triggers/9999", user) + + expect(response.status).to eq(404) + end + end + + context 'authenticated user with invalid permissions' do + it 'should not return triggers list' do + get api("/projects/#{project.id}/triggers/#{trigger.id}", user2) + + expect(response.status).to eq(403) + end + end + + context 'unauthentikated user' do + it 'should not return triggers list' do + get api("/projects/#{project.id}/triggers/#{trigger.id}") + + expect(response.status).to eq(401) + end + end + end + + describe 'POST /projects/:id/triggers' do context 'authenticated user with valid permissions' do it 'should create trigger' do expect do @@ -144,7 +190,7 @@ describe API::API do end end - describe 'DELETE /projects/:id/triggets/:trigger_id' do + describe 'DELETE /projects/:id/triggers/:trigger_id' do context 'authenticated user with valid permissions' do it 'should delete trigger' do expect do -- cgit v1.2.1 From f71642017ebfd409e20735b621dd3a9fe09add12 Mon Sep 17 00:00:00 2001 From: Jacob Schatz Date: Wed, 6 Jan 2016 07:33:50 -0500 Subject: adds tests (and passes them) for ajax open and close merge requests. --- .../fixtures/merge_requests_show.html.haml | 12 ++- spec/javascripts/issue_spec.js.coffee | 2 +- spec/javascripts/merge_request_spec.js.coffee | 88 ++++++++++++++++++++++ 3 files changed, 100 insertions(+), 2 deletions(-) (limited to 'spec') diff --git a/spec/javascripts/fixtures/merge_requests_show.html.haml b/spec/javascripts/fixtures/merge_requests_show.html.haml index 8447dfdda32..fdfa8a273e2 100644 --- a/spec/javascripts/fixtures/merge_requests_show.html.haml +++ b/spec/javascripts/fixtures/merge_requests_show.html.haml @@ -1,4 +1,14 @@ -%a.btn-close +:css + .hidden { display: none !important } + +.flash-container + .flash-alert + .flash-notice + +.status-box.status-box-open Open +.status-box.status-box-closed.hidden Closed +%a.btn-close{"href" => "http://gitlab.com/merge_requests/6/close"} Close +%a.btn-reopen.hidden{"href" => "http://gitlab.com/merge_requests/6/reopen"} Reopen .detail-page-description .description.js-task-list-container diff --git a/spec/javascripts/issue_spec.js.coffee b/spec/javascripts/issue_spec.js.coffee index 7e67c778861..e1c22860da7 100644 --- a/spec/javascripts/issue_spec.js.coffee +++ b/spec/javascripts/issue_spec.js.coffee @@ -44,7 +44,7 @@ describe 'reopen/close issue', -> expect($('div.status-box-closed')).toBeVisible() expect($('div.status-box-open')).toBeHidden() - it 'fails to closes an issue with success:false', -> + it 'fails to close an issue with success:false', -> $.ajax = (obj) -> expect(obj.type).toBe('PUT') diff --git a/spec/javascripts/merge_request_spec.js.coffee b/spec/javascripts/merge_request_spec.js.coffee index 22ebc7039d1..e21bfde38ad 100644 --- a/spec/javascripts/merge_request_spec.js.coffee +++ b/spec/javascripts/merge_request_spec.js.coffee @@ -21,3 +21,91 @@ describe 'MergeRequest', -> expect(req.data.merge_request.description).not.toBe(null) $('.js-task-list-field').trigger('tasklist:changed') + + describe 'reopen/close merge request', -> + fixture.preload('merge_requests_show.html') + beforeEach -> + fixture.load('merge_requests_show.html') + @merge_request = new MergeRequest({}) + it 'closes a merge request', -> + $.ajax = (obj) -> + expect(obj.type).toBe('PUT') + expect(obj.url).toBe('http://gitlab.com/merge_requests/6/close') + obj.success saved:true + + $btnClose = $('a.btn-close') + $btnReopen = $('a.btn-reopen') + expect($btnReopen).toBeHidden() + expect($btnClose.text()).toBe('Close') + expect(typeof $btnClose.prop('disabled')).toBe('undefined') + + $btnClose.trigger('click') + + expect($btnReopen).toBeVisible() + + expect($btnClose).toBeHidden() + expect($('div.status-box-closed')).toBeVisible() + expect($('div.status-box-open')).toBeHidden() + + it 'fails to close a merge request with success:false', -> + + $.ajax = (obj) -> + expect(obj.type).toBe('PUT') + expect(obj.url).toBe('http://goesnowhere.nothing/whereami') + obj.success saved:false + + $btnClose = $('a.btn-close') + $btnReopen = $('a.btn-reopen') + $btnClose.attr('href','http://goesnowhere.nothing/whereami') + expect($btnReopen).toBeHidden() + expect($btnClose.text()).toBe('Close') + expect(typeof $btnClose.prop('disabled')).toBe('undefined') + + $btnClose.trigger('click') + + expect($btnReopen).toBeHidden() + expect($btnClose).toBeVisible() + expect($('div.status-box-closed')).toBeHidden() + expect($('div.status-box-open')).toBeVisible() + expect($('div.flash-alert')).toBeVisible() + expect($('div.flash-alert').text()).toBe('Unable to update this merge request at this time.') + + it 'fails to closes an issue with HTTP error', -> + + $.ajax = (obj) -> + expect(obj.type).toBe('PUT') + expect(obj.url).toBe('http://goesnowhere.nothing/whereami') + obj.error() + + $btnClose = $('a.btn-close') + $btnReopen = $('a.btn-reopen') + $btnClose.attr('href','http://goesnowhere.nothing/whereami') + expect($btnReopen).toBeHidden() + expect($btnClose.text()).toBe('Close') + expect(typeof $btnClose.prop('disabled')).toBe('undefined') + + $btnClose.trigger('click') + + expect($btnReopen).toBeHidden() + expect($btnClose).toBeVisible() + expect($('div.status-box-closed')).toBeHidden() + expect($('div.status-box-open')).toBeVisible() + expect($('div.flash-alert')).toBeVisible() + expect($('div.flash-alert').text()).toBe('Unable to update this merge request at this time.') + + it 'reopens a merge request', -> + $.ajax = (obj) -> + expect(obj.type).toBe('PUT') + expect(obj.url).toBe('http://gitlab.com/merge_requests/6/reopen') + obj.success saved: true + + $btnClose = $('a.btn-close') + $btnReopen = $('a.btn-reopen') + expect($btnReopen.text()).toBe('Reopen') + + $btnReopen.trigger('click') + + expect($btnReopen).toBeHidden() + expect($btnClose).toBeVisible() + expect($('div.status-box-open')).toBeVisible() + expect($('div.status-box-closed')).toBeHidden() \ No newline at end of file -- cgit v1.2.1 From 4d41294d71f2a8910a3aa5d475f8eb3923ca3531 Mon Sep 17 00:00:00 2001 From: Tommy Beadle Date: Wed, 6 Jan 2016 11:42:31 -0500 Subject: Include the user_id in user_*_team system hooks. This fixes an issue where the user_id is not included in the data for user_add_to_team and user_remove_from_team system hooks. The documentation already states that the user_id should be included. --- spec/services/system_hooks_service_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec') diff --git a/spec/services/system_hooks_service_spec.rb b/spec/services/system_hooks_service_spec.rb index 4455ae7b321..41df4951d16 100644 --- a/spec/services/system_hooks_service_spec.rb +++ b/spec/services/system_hooks_service_spec.rb @@ -13,8 +13,8 @@ describe SystemHooksService, services: true do it { expect(event_data(user, :destroy)).to include(:event_name, :name, :created_at, :updated_at, :email, :user_id) } it { expect(event_data(project, :create)).to include(:event_name, :name, :created_at, :updated_at, :path, :project_id, :owner_name, :owner_email, :project_visibility) } it { expect(event_data(project, :destroy)).to include(:event_name, :name, :created_at, :updated_at, :path, :project_id, :owner_name, :owner_email, :project_visibility) } - it { expect(event_data(project_member, :create)).to include(:event_name, :created_at, :updated_at, :project_name, :project_path, :project_path_with_namespace, :project_id, :user_name, :user_email, :access_level, :project_visibility) } - it { expect(event_data(project_member, :destroy)).to include(:event_name, :created_at, :updated_at, :project_name, :project_path, :project_path_with_namespace, :project_id, :user_name, :user_email, :access_level, :project_visibility) } + it { expect(event_data(project_member, :create)).to include(:event_name, :created_at, :updated_at, :project_name, :project_path, :project_path_with_namespace, :project_id, :user_name, :user_email, :user_id, :access_level, :project_visibility) } + it { expect(event_data(project_member, :destroy)).to include(:event_name, :created_at, :updated_at, :project_name, :project_path, :project_path_with_namespace, :project_id, :user_name, :user_email, :user_id, :access_level, :project_visibility) } it { expect(event_data(key, :create)).to include(:username, :key, :id) } it { expect(event_data(key, :destroy)).to include(:username, :key, :id) } -- cgit v1.2.1 From b60c146267dfa8dc1c170426e1817c6b2a168d1a Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Thu, 7 Jan 2016 13:49:38 +0100 Subject: Change :variable_id to :key as resource ID in API --- spec/requests/api/variables_spec.rb | 42 +++++++++++++------------------------ 1 file changed, 15 insertions(+), 27 deletions(-) (limited to 'spec') diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb index bf0dd77473a..214d7d5a0cc 100644 --- a/spec/requests/api/variables_spec.rb +++ b/spec/requests/api/variables_spec.rb @@ -37,26 +37,17 @@ describe API::API, api: true do end end - describe 'GET /projects/:id/variables/:variable_id' do + describe 'GET /projects/:id/variables/:key' do context 'authorized user with proper permissions' do - it 'should return project variable details when ID is used as :variable_id' do - get api("/projects/#{project.id}/variables/#{variable.id}", user) - - expect(response.status).to eq(200) - expect(json_response['key']).to eq(variable.key) - expect(json_response['value']).to eq(variable.value) - end - - it 'should return project variable details when `key` is used as :variable_id' do + it 'should return project variable details' do get api("/projects/#{project.id}/variables/#{variable.key}", user) expect(response.status).to eq(200) - expect(json_response['id']).to eq(variable.id) expect(json_response['value']).to eq(variable.value) end it 'should responde with 404 Not Found if requesting non-existing variable' do - get api("/projects/#{project.id}/variables/9999", user) + get api("/projects/#{project.id}/variables/non_existing_variable", user) expect(response.status).to eq(404) end @@ -64,7 +55,7 @@ describe API::API, api: true do context 'authorized user with invalid permissions' do it 'should not return project variable details' do - get api("/projects/#{project.id}/variables/#{variable.id}", user2) + get api("/projects/#{project.id}/variables/#{variable.key}", user2) expect(response.status).to eq(403) end @@ -72,7 +63,7 @@ describe API::API, api: true do context 'unauthorized user' do it 'should not return project variable details' do - get api("/projects/#{project.id}/variables/#{variable.id}") + get api("/projects/#{project.id}/variables/#{variable.key}") expect(response.status).to eq(401) end @@ -117,26 +108,23 @@ describe API::API, api: true do end end - describe 'PUT /projects/:id/variables/:variable_id' do + describe 'PUT /projects/:id/variables/:key' do context 'authorized user with proper permissions' do it 'should update variable data' do initial_variable = project.variables.first - key_before = initial_variable.key value_before = initial_variable.value - put api("/projects/#{project.id}/variables/#{variable.id}", user), key: 'TEST_VARIABLE_1_UP', value: 'VALUE_1_UP' + put api("/projects/#{project.id}/variables/#{variable.key}", user), value: 'VALUE_1_UP' updated_variable = project.variables.first expect(response.status).to eq(200) - expect(key_before).to eq(variable.key) expect(value_before).to eq(variable.value) - expect(updated_variable.key).to eq('TEST_VARIABLE_1_UP') expect(updated_variable.value).to eq('VALUE_1_UP') end it 'should responde with 404 Not Found if requesting non-existing variable' do - put api("/projects/#{project.id}/variables/9999", user) + put api("/projects/#{project.id}/variables/non_existing_variable", user) expect(response.status).to eq(404) end @@ -144,7 +132,7 @@ describe API::API, api: true do context 'authorized user with invalid permissions' do it 'should not update variable' do - put api("/projects/#{project.id}/variables/#{variable.id}", user2) + put api("/projects/#{project.id}/variables/#{variable.key}", user2) expect(response.status).to eq(403) end @@ -152,24 +140,24 @@ describe API::API, api: true do context 'unauthorized user' do it 'should not update variable' do - put api("/projects/#{project.id}/variables/#{variable.id}") + put api("/projects/#{project.id}/variables/#{variable.key}") expect(response.status).to eq(401) end end end - describe 'DELETE /projects/:id/variables/:variable_id' do + describe 'DELETE /projects/:id/variables/:key' do context 'authorized user with proper permissions' do it 'should delete variable' do expect do - delete api("/projects/#{project.id}/variables/#{variable.id}", user) + delete api("/projects/#{project.id}/variables/#{variable.key}", user) end.to change{project.variables.count}.by(-1) expect(response.status).to eq(200) end it 'should responde with 404 Not Found if requesting non-existing variable' do - delete api("/projects/#{project.id}/variables/9999", user) + delete api("/projects/#{project.id}/variables/non_existing_variable", user) expect(response.status).to eq(404) end @@ -177,7 +165,7 @@ describe API::API, api: true do context 'authorized user with invalid permissions' do it 'should not delete variable' do - delete api("/projects/#{project.id}/variables/#{variable.id}", user2) + delete api("/projects/#{project.id}/variables/#{variable.key}", user2) expect(response.status).to eq(403) end @@ -185,7 +173,7 @@ describe API::API, api: true do context 'unauthorized user' do it 'should not delete variable' do - delete api("/projects/#{project.id}/variables/#{variable.id}") + delete api("/projects/#{project.id}/variables/#{variable.key}") expect(response.status).to eq(401) end -- cgit v1.2.1 From e0ec69d919cb44194e76034f2324ec0d4f5f1df6 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Thu, 7 Jan 2016 18:48:33 +0100 Subject: Change 'trigger_id' to 'token' as resource ID in triggers API --- spec/requests/api/triggers_spec.rb | 42 +++++++++++++------------------------- 1 file changed, 14 insertions(+), 28 deletions(-) (limited to 'spec') diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb index c1c2bb04b29..e8d89426ec0 100644 --- a/spec/requests/api/triggers_spec.rb +++ b/spec/requests/api/triggers_spec.rb @@ -93,8 +93,7 @@ describe API::API do expect(response.status).to eq(200) expect(json_response).to be_a(Array) - expect(json_response[0]['token']).to eq(trigger_token) - expect(json_response[1]['token']).to eq(trigger_token_2) + expect(json_response[0]).to have_key('token') end end @@ -115,30 +114,17 @@ describe API::API do end end - describe 'GET /projects/:id/triggers/:triggers_id' do + describe 'GET /projects/:id/triggers/:token' do context 'authenticated user with valid permissions' do - context 'ID is used as :trigger_id' do - it 'should return trigger details' do - get api("/projects/#{project.id}/triggers/#{trigger.id}", user) + it 'should return trigger details' do + get api("/projects/#{project.id}/triggers/#{trigger.token}", user) - expect(response.status).to eq(200) - expect(json_response).to be_a(Hash) - expect(json_response['token']).to eq(trigger_token) - end - end - - context '`token` is used as :trigger_id' do - it 'should return trigger details' do - get api("/projects/#{project.id}/triggers/#{trigger.token}", user) - - expect(response.status).to eq(200) - expect(json_response).to be_a(Hash) - expect(json_response['id']).to eq(trigger.id) - end + expect(response.status).to eq(200) + expect(json_response).to be_a(Hash) end it 'should responde with 404 Not Found if requesting non-existing trigger' do - get api("/projects/#{project.id}/triggers/9999", user) + get api("/projects/#{project.id}/triggers/abcdef012345", user) expect(response.status).to eq(404) end @@ -146,7 +132,7 @@ describe API::API do context 'authenticated user with invalid permissions' do it 'should not return triggers list' do - get api("/projects/#{project.id}/triggers/#{trigger.id}", user2) + get api("/projects/#{project.id}/triggers/#{trigger.token}", user2) expect(response.status).to eq(403) end @@ -154,7 +140,7 @@ describe API::API do context 'unauthentikated user' do it 'should not return triggers list' do - get api("/projects/#{project.id}/triggers/#{trigger.id}") + get api("/projects/#{project.id}/triggers/#{trigger.token}") expect(response.status).to eq(401) end @@ -190,17 +176,17 @@ describe API::API do end end - describe 'DELETE /projects/:id/triggers/:trigger_id' do + describe 'DELETE /projects/:id/triggers/:token' do context 'authenticated user with valid permissions' do it 'should delete trigger' do expect do - delete api("/projects/#{project.id}/triggers/#{trigger.id}", user) + delete api("/projects/#{project.id}/triggers/#{trigger.token}", user) end.to change{project.triggers.count}.by(-1) expect(response.status).to eq(200) end it 'should responde with 404 Not Found if requesting non-existing trigger' do - delete api("/projects/#{project.id}/triggers/9999", user) + delete api("/projects/#{project.id}/triggers/abcdef012345", user) expect(response.status).to eq(404) end @@ -208,7 +194,7 @@ describe API::API do context 'authenticated user with invalid permissions' do it 'should not delete trigger' do - delete api("/projects/#{project.id}/triggers/#{trigger.id}", user2) + delete api("/projects/#{project.id}/triggers/#{trigger.token}", user2) expect(response.status).to eq(403) end @@ -216,7 +202,7 @@ describe API::API do context 'unauthentikated user' do it 'should not delete trigger' do - delete api("/projects/#{project.id}/triggers/#{trigger.id}") + delete api("/projects/#{project.id}/triggers/#{trigger.token}") expect(response.status).to eq(401) end -- cgit v1.2.1 From dada25d4472ec9ad601447fdd12da2301ac9ee79 Mon Sep 17 00:00:00 2001 From: Tommy Beadle Date: Thu, 7 Jan 2016 12:54:35 -0500 Subject: Include the username in user_create/destroy system hooks. --- spec/services/system_hooks_service_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec') diff --git a/spec/services/system_hooks_service_spec.rb b/spec/services/system_hooks_service_spec.rb index 4455ae7b321..1824e51ebfe 100644 --- a/spec/services/system_hooks_service_spec.rb +++ b/spec/services/system_hooks_service_spec.rb @@ -9,8 +9,8 @@ describe SystemHooksService, services: true do let(:group_member) { create(:group_member) } context 'event data' do - it { expect(event_data(user, :create)).to include(:event_name, :name, :created_at, :updated_at, :email, :user_id) } - it { expect(event_data(user, :destroy)).to include(:event_name, :name, :created_at, :updated_at, :email, :user_id) } + it { expect(event_data(user, :create)).to include(:event_name, :name, :created_at, :updated_at, :email, :user_id, :username) } + it { expect(event_data(user, :destroy)).to include(:event_name, :name, :created_at, :updated_at, :email, :user_id, :username) } it { expect(event_data(project, :create)).to include(:event_name, :name, :created_at, :updated_at, :path, :project_id, :owner_name, :owner_email, :project_visibility) } it { expect(event_data(project, :destroy)).to include(:event_name, :name, :created_at, :updated_at, :path, :project_id, :owner_name, :owner_email, :project_visibility) } it { expect(event_data(project_member, :create)).to include(:event_name, :created_at, :updated_at, :project_name, :project_path, :project_path_with_namespace, :project_id, :user_name, :user_email, :access_level, :project_visibility) } -- cgit v1.2.1 From 1f64332e11949a5954b1e4ac7c6667b03ea70a0b Mon Sep 17 00:00:00 2001 From: Tommy Beadle Date: Thu, 7 Jan 2016 12:54:54 -0500 Subject: Include user_username in user_(add_to/remove_from)_(project/group) system hooks. --- spec/services/system_hooks_service_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'spec') diff --git a/spec/services/system_hooks_service_spec.rb b/spec/services/system_hooks_service_spec.rb index 1824e51ebfe..eb066fe97f3 100644 --- a/spec/services/system_hooks_service_spec.rb +++ b/spec/services/system_hooks_service_spec.rb @@ -13,8 +13,8 @@ describe SystemHooksService, services: true do it { expect(event_data(user, :destroy)).to include(:event_name, :name, :created_at, :updated_at, :email, :user_id, :username) } it { expect(event_data(project, :create)).to include(:event_name, :name, :created_at, :updated_at, :path, :project_id, :owner_name, :owner_email, :project_visibility) } it { expect(event_data(project, :destroy)).to include(:event_name, :name, :created_at, :updated_at, :path, :project_id, :owner_name, :owner_email, :project_visibility) } - it { expect(event_data(project_member, :create)).to include(:event_name, :created_at, :updated_at, :project_name, :project_path, :project_path_with_namespace, :project_id, :user_name, :user_email, :access_level, :project_visibility) } - it { expect(event_data(project_member, :destroy)).to include(:event_name, :created_at, :updated_at, :project_name, :project_path, :project_path_with_namespace, :project_id, :user_name, :user_email, :access_level, :project_visibility) } + it { expect(event_data(project_member, :create)).to include(:event_name, :created_at, :updated_at, :project_name, :project_path, :project_path_with_namespace, :project_id, :user_name, :user_username, :user_email, :access_level, :project_visibility) } + it { expect(event_data(project_member, :destroy)).to include(:event_name, :created_at, :updated_at, :project_name, :project_path, :project_path_with_namespace, :project_id, :user_name, :user_username, :user_email, :access_level, :project_visibility) } it { expect(event_data(key, :create)).to include(:username, :key, :id) } it { expect(event_data(key, :destroy)).to include(:username, :key, :id) } @@ -50,13 +50,13 @@ describe SystemHooksService, services: true do it do expect(event_data(group_member, :create)).to include( :event_name, :created_at, :updated_at, :group_name, :group_path, - :group_id, :user_id, :user_name, :user_email, :group_access + :group_id, :user_id, :user_username, :user_name, :user_email, :group_access ) end it do expect(event_data(group_member, :destroy)).to include( :event_name, :created_at, :updated_at, :group_name, :group_path, - :group_id, :user_id, :user_name, :user_email, :group_access + :group_id, :user_id, :user_username, :user_name, :user_email, :group_access ) end end -- cgit v1.2.1 From 549a2fa7873366b52e9ba3caa849073b7b958b73 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Fri, 8 Jan 2016 14:01:31 +0100 Subject: Modify builds scope filtering in builds API --- spec/requests/api/builds_spec.rb | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/requests/api/builds_spec.rb b/spec/requests/api/builds_spec.rb index d4af7639d4b..a953eb2fac2 100644 --- a/spec/requests/api/builds_spec.rb +++ b/spec/requests/api/builds_spec.rb @@ -18,7 +18,20 @@ describe API::API, api: true do it 'should return project builds' do get api("/projects/#{project.id}/builds", user) - puts json_response + expect(response.status).to eq(200) + expect(json_response).to be_an Array + end + + it 'should filter project with one scope element' do + get api("/projects/#{project.id}/builds?scope=pending", user) + + expect(response.status).to eq(200) + expect(json_response).to be_an Array + end + + it 'should filter project with array of scope elements' do + get api("/projects/#{project.id}/builds?scope[0]=pending&scope[1]=running", user) + expect(response.status).to eq(200) expect(json_response).to be_an Array end -- cgit v1.2.1 From bc7ef8e5b7a002ca6bc2d7a5e6be11b4a59b6710 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Tue, 29 Dec 2015 17:53:55 -0200 Subject: Add ldap_blocked as new state to users state machine --- spec/models/user_spec.rb | 44 ++++++++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 16 deletions(-) (limited to 'spec') diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 3cd63b2b0e8..0bef68e2885 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -569,27 +569,39 @@ describe User, models: true do end end - describe :ldap_user? do - it "is true if provider name starts with ldap" do - user = create(:omniauth_user, provider: 'ldapmain') - expect( user.ldap_user? ).to be_truthy - end + context 'ldap synchronized user' do + describe :ldap_user? do + it 'is true if provider name starts with ldap' do + user = create(:omniauth_user, provider: 'ldapmain') + expect(user.ldap_user?).to be_truthy + end - it "is false for other providers" do - user = create(:omniauth_user, provider: 'other-provider') - expect( user.ldap_user? ).to be_falsey + it 'is false for other providers' do + user = create(:omniauth_user, provider: 'other-provider') + expect(user.ldap_user?).to be_falsey + end + + it 'is false if no extern_uid is provided' do + user = create(:omniauth_user, extern_uid: nil) + expect(user.ldap_user?).to be_falsey + end end - it "is false if no extern_uid is provided" do - user = create(:omniauth_user, extern_uid: nil) - expect( user.ldap_user? ).to be_falsey + describe :ldap_identity do + it 'returns ldap identity' do + user = create :omniauth_user + expect(user.ldap_identity.provider).not_to be_empty + end end - end - describe :ldap_identity do - it "returns ldap identity" do - user = create :omniauth_user - expect(user.ldap_identity.provider).not_to be_empty + describe '#ldap_block' do + let(:user) { create(:omniauth_user, provider: 'ldapmain', name: 'John Smith') } + + it 'blocks user flaging the action caming from ldap' do + user.ldap_block + expect(user.blocked?).to be_truthy + expect(user.ldap_blocked?).to be_truthy + end end end -- cgit v1.2.1 From ba9855d4877998e3574907cc542fcab15a9d1353 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Tue, 29 Dec 2015 18:58:38 -0200 Subject: Prevent ldap_blocked users from being unblocked by the Admin UI --- spec/controllers/admin/users_controller_spec.rb | 35 ++++++++++++++++++------- 1 file changed, 26 insertions(+), 9 deletions(-) (limited to 'spec') diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index 8b7af4d3a0a..5b1f65d7aff 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -34,17 +34,34 @@ describe Admin::UsersController do end describe 'PUT unblock/:id' do - let(:user) { create(:user) } - - before do - user.block + context 'ldap blocked users' do + let(:user) { create(:omniauth_user, provider: 'ldapmain') } + + before do + user.ldap_block + end + + it 'will not unblock user' do + put :unblock, id: user.username + user.reload + expect(user.blocked?).to be_truthy + expect(flash[:alert]).to eq 'This user cannot be unlocked manually from GitLab' + end end - it 'unblocks user' do - put :unblock, id: user.username - user.reload - expect(user.blocked?).to be_falsey - expect(flash[:notice]).to eq 'Successfully unblocked' + context 'manually blocked users' do + let(:user) { create(:user) } + + before do + user.block + end + + it 'unblocks user' do + put :unblock, id: user.username + user.reload + expect(user.blocked?).to be_falsey + expect(flash[:notice]).to eq 'Successfully unblocked' + end end end -- cgit v1.2.1 From 6e7db8e23e169bcbf0847ece27b9e44e00ae572b Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Wed, 30 Dec 2015 16:52:02 -0200 Subject: Prevent ldap_blocked users from being blocked/unblocked by the API --- spec/requests/api/users_spec.rb | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) (limited to 'spec') diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 4f278551d07..b82c5c7685f 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -8,6 +8,8 @@ describe API::API, api: true do let(:key) { create(:key, user: user) } let(:email) { create(:email, user: user) } let(:omniauth_user) { create(:omniauth_user) } + let(:ldap_user) { create(:omniauth_user, provider: 'ldapmain') } + let(:ldap_blocked_user) { create(:omniauth_user, provider: 'ldapmain', state: 'ldap_blocked') } describe "GET /users" do context "when unauthenticated" do @@ -783,6 +785,12 @@ describe API::API, api: true do expect(user.reload.state).to eq('blocked') end + it 'should not re-block ldap blocked users' do + put api("/users/#{ldap_blocked_user.id}/block", admin) + expect(response.status).to eq(403) + expect(ldap_blocked_user.reload.state).to eq('ldap_blocked') + end + it 'should not be available for non admin users' do put api("/users/#{user.id}/block", user) expect(response.status).to eq(403) @@ -797,7 +805,9 @@ describe API::API, api: true do end describe 'PUT /user/:id/unblock' do + let(:blocked_user) { create(:user, state: 'blocked') } before { admin } + it 'should unblock existing user' do put api("/users/#{user.id}/unblock", admin) expect(response.status).to eq(200) @@ -805,12 +815,15 @@ describe API::API, api: true do end it 'should unblock a blocked user' do - put api("/users/#{user.id}/block", admin) - expect(response.status).to eq(200) - expect(user.reload.state).to eq('blocked') - put api("/users/#{user.id}/unblock", admin) + put api("/users/#{blocked_user.id}/unblock", admin) expect(response.status).to eq(200) - expect(user.reload.state).to eq('active') + expect(blocked_user.reload.state).to eq('active') + end + + it 'should not unblock ldap blocked users' do + put api("/users/#{ldap_blocked_user.id}/unblock", admin) + expect(response.status).to eq(403) + expect(ldap_blocked_user.reload.state).to eq('ldap_blocked') end it 'should not be available for non admin users' do -- cgit v1.2.1 From d6dc088affeee4568e771e1d7894e0bcdb955af8 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Wed, 30 Dec 2015 20:56:26 -0200 Subject: LDAP synchronization block/unblock new states --- spec/lib/gitlab/ldap/access_spec.rb | 34 ++++++++++++++-------------------- 1 file changed, 14 insertions(+), 20 deletions(-) (limited to 'spec') diff --git a/spec/lib/gitlab/ldap/access_spec.rb b/spec/lib/gitlab/ldap/access_spec.rb index a628d0c0157..f58d70e809c 100644 --- a/spec/lib/gitlab/ldap/access_spec.rb +++ b/spec/lib/gitlab/ldap/access_spec.rb @@ -13,64 +13,59 @@ describe Gitlab::LDAP::Access, lib: true do end it { is_expected.to be_falsey } - + it 'should block user in GitLab' do access.allowed? expect(user).to be_blocked + expect(user).to be_ldap_blocked end end context 'when the user is found' do before do - allow(Gitlab::LDAP::Person). - to receive(:find_by_dn).and_return(:ldap_user) + allow(Gitlab::LDAP::Person).to receive(:find_by_dn).and_return(:ldap_user) end context 'and the user is disabled via active directory' do before do - allow(Gitlab::LDAP::Person). - to receive(:disabled_via_active_directory?).and_return(true) + allow(Gitlab::LDAP::Person).to receive(:disabled_via_active_directory?).and_return(true) end it { is_expected.to be_falsey } - it "should block user in GitLab" do + it 'should block user in GitLab' do access.allowed? expect(user).to be_blocked + expect(user).to be_ldap_blocked end end context 'and has no disabled flag in active diretory' do before do user.block - - allow(Gitlab::LDAP::Person). - to receive(:disabled_via_active_directory?).and_return(false) + allow(Gitlab::LDAP::Person).to receive(:disabled_via_active_directory?).and_return(false) end it { is_expected.to be_truthy } context 'when auto-created users are blocked' do - before do - allow_any_instance_of(Gitlab::LDAP::Config). - to receive(:block_auto_created_users).and_return(true) + allow_any_instance_of(Gitlab::LDAP::Config).to receive(:block_auto_created_users).and_return(true) end - it "does not unblock user in GitLab" do + it 'does not unblock user in GitLab' do access.allowed? expect(user).to be_blocked + expect(user).not_to be_ldap_blocked # this block is handled by omniauth not by our internal logic end end - context "when auto-created users are not blocked" do - + context 'when auto-created users are not blocked' do before do - allow_any_instance_of(Gitlab::LDAP::Config). - to receive(:block_auto_created_users).and_return(false) + allow_any_instance_of(Gitlab::LDAP::Config).to receive(:block_auto_created_users).and_return(false) end - it "should unblock user in GitLab" do + it 'should unblock user in GitLab' do access.allowed? expect(user).not_to be_blocked end @@ -80,8 +75,7 @@ describe Gitlab::LDAP::Access, lib: true do context 'without ActiveDirectory enabled' do before do allow(Gitlab::LDAP::Config).to receive(:enabled?).and_return(true) - allow_any_instance_of(Gitlab::LDAP::Config). - to receive(:active_directory).and_return(false) + allow_any_instance_of(Gitlab::LDAP::Config).to receive(:active_directory).and_return(false) end it { is_expected.to be_truthy } -- cgit v1.2.1 From ec67e9be1d7486199b47e19c766202a8bfdefe93 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Wed, 6 Jan 2016 05:38:52 -0200 Subject: Repair ldap_blocked state when no ldap identity exist anymore --- .../admin/identities_controller_spec.rb | 26 +++++++++++++++ spec/models/identity_spec.rb | 38 ++++++++++++++++++++++ .../repair_ldap_blocked_user_service_spec.rb | 23 +++++++++++++ 3 files changed, 87 insertions(+) create mode 100644 spec/controllers/admin/identities_controller_spec.rb create mode 100644 spec/models/identity_spec.rb create mode 100644 spec/services/repair_ldap_blocked_user_service_spec.rb (limited to 'spec') diff --git a/spec/controllers/admin/identities_controller_spec.rb b/spec/controllers/admin/identities_controller_spec.rb new file mode 100644 index 00000000000..c131d22a30a --- /dev/null +++ b/spec/controllers/admin/identities_controller_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' + +describe Admin::IdentitiesController do + let(:admin) { create(:admin) } + before { sign_in(admin) } + + describe 'UPDATE identity' do + let(:user) { create(:omniauth_user, provider: 'ldapmain', extern_uid: 'uid=myuser,ou=people,dc=example,dc=com') } + + it 'repairs ldap blocks' do + expect_any_instance_of(RepairLdapBlockedUserService).to receive(:execute) + + put :update, user_id: user.username, id: user.ldap_identity.id, identity: { provider: 'twitter' } + end + end + + describe 'DELETE identity' do + let(:user) { create(:omniauth_user, provider: 'ldapmain', extern_uid: 'uid=myuser,ou=people,dc=example,dc=com') } + + it 'repairs ldap blocks' do + expect_any_instance_of(RepairLdapBlockedUserService).to receive(:execute) + + delete :destroy, user_id: user.username, id: user.ldap_identity.id + end + end +end diff --git a/spec/models/identity_spec.rb b/spec/models/identity_spec.rb new file mode 100644 index 00000000000..107bfc17782 --- /dev/null +++ b/spec/models/identity_spec.rb @@ -0,0 +1,38 @@ +# == Schema Information +# +# Table name: identities +# +# id :integer not null, primary key +# extern_uid :string(255) +# provider :string(255) +# user_id :integer +# created_at :datetime +# updated_at :datetime +# + +require 'spec_helper' + +RSpec.describe Identity, models: true do + + describe 'relations' do + it { is_expected.to belong_to(:user) } + end + + describe 'fields' do + it { is_expected.to respond_to(:provider) } + it { is_expected.to respond_to(:extern_uid) } + end + + describe '#is_ldap?' do + let(:ldap_identity) { create(:identity, provider: 'ldapmain') } + let(:other_identity) { create(:identity, provider: 'twitter') } + + it 'returns true if it is a ldap identity' do + expect(ldap_identity.is_ldap?).to be_truthy + end + + it 'returns false if it is not a ldap identity' do + expect(other_identity.is_ldap?).to be_falsey + end + end +end diff --git a/spec/services/repair_ldap_blocked_user_service_spec.rb b/spec/services/repair_ldap_blocked_user_service_spec.rb new file mode 100644 index 00000000000..2a2114d038c --- /dev/null +++ b/spec/services/repair_ldap_blocked_user_service_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +describe RepairLdapBlockedUserService, services: true do + let(:user) { create(:omniauth_user, provider: 'ldapmain', state: 'ldap_blocked') } + let(:identity) { user.ldap_identity } + subject(:service) { RepairLdapBlockedUserService.new(user, identity) } + + describe '#execute' do + it 'change to normal block after destroying last ldap identity' do + identity.destroy + service.execute + + expect(user.reload).not_to be_ldap_blocked + end + + it 'change to normal block after changing last ldap identity to another provider' do + identity.update_attribute(:provider, 'twitter') + service.execute + + expect(user.reload).not_to be_ldap_blocked + end + end +end -- cgit v1.2.1 From 47e4613f4adc2d6ef4b066a87ec772ef8044bdd5 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Thu, 7 Jan 2016 14:01:01 -0200 Subject: Code style fixes and some code simplified --- spec/services/repair_ldap_blocked_user_service_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/services/repair_ldap_blocked_user_service_spec.rb b/spec/services/repair_ldap_blocked_user_service_spec.rb index 2a2114d038c..ce7d1455975 100644 --- a/spec/services/repair_ldap_blocked_user_service_spec.rb +++ b/spec/services/repair_ldap_blocked_user_service_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe RepairLdapBlockedUserService, services: true do let(:user) { create(:omniauth_user, provider: 'ldapmain', state: 'ldap_blocked') } let(:identity) { user.ldap_identity } - subject(:service) { RepairLdapBlockedUserService.new(user, identity) } + subject(:service) { RepairLdapBlockedUserService.new(user) } describe '#execute' do it 'change to normal block after destroying last ldap identity' do -- cgit v1.2.1 From 1eb7b5ee8d5afeeea74ccbd5627e5a235dffe9fd Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Fri, 8 Jan 2016 22:57:42 +0100 Subject: Modify entities for builds API --- spec/requests/api/builds_spec.rb | 1 - 1 file changed, 1 deletion(-) (limited to 'spec') diff --git a/spec/requests/api/builds_spec.rb b/spec/requests/api/builds_spec.rb index a953eb2fac2..1e58e18fad9 100644 --- a/spec/requests/api/builds_spec.rb +++ b/spec/requests/api/builds_spec.rb @@ -74,7 +74,6 @@ describe API::API, api: true do expect(response.status).to eq(200) expect(json_response['name']).to eq('test') - expect(json_response['commit']['sha']).to eq(commit.sha) end it 'should return specific build trace' do -- cgit v1.2.1 From d54bff2a770c1030056866a47097aee9937390ef Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Fri, 8 Jan 2016 23:04:44 +0100 Subject: Change test access level from MASTER to DEVELOPER This should help us detect potential regressions in the future. --- spec/requests/api/builds_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/requests/api/builds_spec.rb b/spec/requests/api/builds_spec.rb index 1e58e18fad9..b6a1154cf76 100644 --- a/spec/requests/api/builds_spec.rb +++ b/spec/requests/api/builds_spec.rb @@ -6,7 +6,7 @@ describe API::API, api: true do let(:user) { create(:user) } let(:user2) { create(:user) } let!(:project) { create(:project, creator_id: user.id) } - let!(:master) { create(:project_member, user: user, project: project, access_level: ProjectMember::MASTER) } + let!(:developer) { create(:project_member, user: user, project: project, access_level: ProjectMember::DEVELOPER) } let!(:reporter) { create(:project_member, user: user2, project: project, access_level: ProjectMember::REPORTER) } let(:commit) { create(:ci_commit, project: project)} let(:build) { create(:ci_build, commit: commit) } -- cgit v1.2.1 From 4eb27d7c72d57015c7551a00e34a54cefc2d3db9 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Fri, 8 Jan 2016 23:33:45 +0100 Subject: Add some modifications to builds API and specs --- spec/requests/api/builds_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'spec') diff --git a/spec/requests/api/builds_spec.rb b/spec/requests/api/builds_spec.rb index b6a1154cf76..799558d1bdd 100644 --- a/spec/requests/api/builds_spec.rb +++ b/spec/requests/api/builds_spec.rb @@ -50,7 +50,7 @@ describe API::API, api: true do context 'authorized user' do it 'should return project builds for specific commit' do project.ensure_ci_commit(commit.sha) - get api("/projects/#{project.id}/builds/commit/#{project.ci_commits.first.sha}", user) + get api("/projects/#{project.id}/builds/commit/#{commit.sha}", user) expect(response.status).to eq(200) expect(json_response).to be_an Array @@ -60,7 +60,7 @@ describe API::API, api: true do context 'unauthorized user' do it 'should not return project builds' do project.ensure_ci_commit(commit.sha) - get api("/projects/#{project.id}/builds/commit/#{project.ci_commits.first.sha}") + get api("/projects/#{project.id}/builds/commit/#{commit.sha}") expect(response.status).to eq(401) end @@ -99,7 +99,7 @@ describe API::API, api: true do end end - describe 'GET /projects/:id/builds/:build_id/cancel' do + describe 'POST /projects/:id/builds/:build_id/cancel' do context 'authorized user' do context 'user with :manage_builds persmission' do it 'should cancel running or pending build' do @@ -128,7 +128,7 @@ describe API::API, api: true do end end - describe 'GET /projects/:id/builds/:build_id/retry' do + describe 'POST /projects/:id/builds/:build_id/retry' do context 'authorized user' do context 'user with :manage_builds persmission' do it 'should retry non-running build' do -- cgit v1.2.1 From 58867eff46dc6886b85bfe5a787341f224d09421 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Wed, 9 Dec 2015 11:59:25 +0100 Subject: Unsubscribe from thread through link in email footer --- .../sent_notification_controller_spec.rb | 26 +++++++++++++++++ spec/factories.rb | 7 +++++ spec/mailers/notify_spec.rb | 34 +++++++++++++++++++++- 3 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 spec/controllers/sent_notification_controller_spec.rb (limited to 'spec') diff --git a/spec/controllers/sent_notification_controller_spec.rb b/spec/controllers/sent_notification_controller_spec.rb new file mode 100644 index 00000000000..9ced397bd4a --- /dev/null +++ b/spec/controllers/sent_notification_controller_spec.rb @@ -0,0 +1,26 @@ +require 'rails_helper' + +describe SentNotificationsController, type: :controller do + let(:user) { create(:user) } + let(:issue) { create(:issue, author: user) } + let(:sent_notification) { create(:sent_notification, noteable: issue) } + + describe 'GET #unsubscribe' do + it 'returns a 404 when calling without existing id' do + get(:unsubscribe, id: '0' * 32) + + expect(response.status).to be 404 + end + + context 'calling with id' do + it 'shows a flash message to the user' do + get(:unsubscribe, id: sent_notification.reply_key) + + expect(response.status).to be 302 + + expect(response).to redirect_to new_user_session_path + expect(controller).to set_flash[:notice].to(/unsubscribed/).now + end + end + end +end diff --git a/spec/factories.rb b/spec/factories.rb index d6b4efa9a03..2a81684dfcf 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -212,4 +212,11 @@ FactoryGirl.define do provider 'ldapmain' extern_uid 'my-ldap-id' end + + factory :sent_notification do + project + recipient factory: :user + noteable factory: :issue + reply_key "0123456789abcdef" * 2 + end end diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 154901a2fbc..08ade644643 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -104,6 +104,14 @@ describe Notify do it { is_expected.to have_body_text /View Commit/ } end + shared_examples 'an unsubscribeable thread' do + it { is_expected.to have_body_text /unsubscribe/ } + end + + shared_examples "a user can not unsubscribe through footer link" do + it { is_expected.not_to have_body_text /unsubscribe/ } + end + describe 'for new users, the email' do let(:example_site_path) { root_path } let(:new_user) { create(:user, email: new_user_address, created_by_id: 1) } @@ -115,6 +123,7 @@ describe Notify do it_behaves_like 'an email sent from GitLab' it_behaves_like 'a new user email', new_user_address it_behaves_like 'it should not have Gmail Actions links' + it_behaves_like 'a user can not unsubscribe through footer link' it 'contains the password text' do is_expected.to have_body_text /Click here to set your password/ @@ -134,7 +143,6 @@ describe Notify do end end - describe 'for users that signed up, the email' do let(:example_site_path) { root_path } let(:new_user) { create(:user, email: new_user_address, password: "securePassword") } @@ -144,6 +152,7 @@ describe Notify do it_behaves_like 'an email sent from GitLab' it_behaves_like 'a new user email', new_user_address it_behaves_like 'it should not have Gmail Actions links' + it_behaves_like 'a user can not unsubscribe through footer link' it 'should not contain the new user\'s password' do is_expected.not_to have_body_text /password/ @@ -157,6 +166,7 @@ describe Notify do it_behaves_like 'an email sent from GitLab' it_behaves_like 'it should not have Gmail Actions links' + it_behaves_like 'a user can not unsubscribe through footer link' it 'is sent to the new user' do is_expected.to deliver_to key.user.email @@ -181,6 +191,7 @@ describe Notify do subject { Notify.new_email_email(email.id) } it_behaves_like 'it should not have Gmail Actions links' + it_behaves_like 'a user can not unsubscribe through footer link' it 'is sent to the new user' do is_expected.to deliver_to email.user.email @@ -227,6 +238,7 @@ describe Notify do it_behaves_like 'an assignee email' it_behaves_like 'an email starting a new thread', 'issue' it_behaves_like 'it should show Gmail Actions View Issue link' + it_behaves_like 'an unsubscribeable thread' it 'has the correct subject' do is_expected.to have_subject /#{project.name} \| #{issue.title} \(##{issue.iid}\)/ @@ -253,6 +265,7 @@ describe Notify do it_behaves_like 'a multiple recipients email' it_behaves_like 'an answer to an existing thread', 'issue' it_behaves_like 'it should show Gmail Actions View Issue link' + it_behaves_like "an unsubscribeable thread" it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -283,6 +296,7 @@ describe Notify do it_behaves_like 'an answer to an existing thread', 'issue' it_behaves_like 'it should show Gmail Actions View Issue link' + it_behaves_like 'an unsubscribeable thread' it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -319,6 +333,7 @@ describe Notify do it_behaves_like 'an assignee email' it_behaves_like 'an email starting a new thread', 'merge_request' it_behaves_like 'it should show Gmail Actions View Merge request link' + it_behaves_like "an unsubscribeable thread" it 'has the correct subject' do is_expected.to have_subject /#{merge_request.title} \(##{merge_request.iid}\)/ @@ -345,6 +360,7 @@ describe Notify do subject { Notify.new_merge_request_email(merge_request_with_description.assignee_id, merge_request_with_description.id) } it_behaves_like 'it should show Gmail Actions View Merge request link' + it_behaves_like "an unsubscribeable thread" it 'contains the description' do is_expected.to have_body_text /#{merge_request_with_description.description}/ @@ -357,6 +373,7 @@ describe Notify do it_behaves_like 'a multiple recipients email' it_behaves_like 'an answer to an existing thread', 'merge_request' it_behaves_like 'it should show Gmail Actions View Merge request link' + it_behaves_like "an unsubscribeable thread" it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -387,6 +404,7 @@ describe Notify do it_behaves_like 'an answer to an existing thread', 'merge_request' it_behaves_like 'it should show Gmail Actions View Merge request link' + it_behaves_like "an unsubscribeable thread" it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -417,6 +435,7 @@ describe Notify do it_behaves_like 'a multiple recipients email' it_behaves_like 'an answer to an existing thread', 'merge_request' it_behaves_like 'it should show Gmail Actions View Merge request link' + it_behaves_like "an unsubscribeable thread" it 'is sent as the merge author' do sender = subject.header[:from].addrs[0] @@ -446,6 +465,7 @@ describe Notify do it_behaves_like 'an email sent from GitLab' it_behaves_like 'it should not have Gmail Actions links' + it_behaves_like "a user can not unsubscribe through footer link" it 'has the correct subject' do is_expected.to have_subject /Project was moved/ @@ -468,6 +488,7 @@ describe Notify do it_behaves_like 'an email sent from GitLab' it_behaves_like 'it should not have Gmail Actions links' + it_behaves_like "a user can not unsubscribe through footer link" it 'has the correct subject' do is_expected.to have_subject /Access to project was granted/ @@ -518,6 +539,7 @@ describe Notify do it_behaves_like 'a note email' it_behaves_like 'an answer to an existing thread', 'commit' it_behaves_like 'it should show Gmail Actions View Commit link' + it_behaves_like "a user can not unsubscribe through footer link" it 'has the correct subject' do is_expected.to have_subject /#{commit.title} \(#{commit.short_id}\)/ @@ -538,6 +560,7 @@ describe Notify do it_behaves_like 'a note email' it_behaves_like 'an answer to an existing thread', 'merge_request' it_behaves_like 'it should show Gmail Actions View Merge request link' + it_behaves_like 'an unsubscribeable thread' it 'has the correct subject' do is_expected.to have_subject /#{merge_request.title} \(##{merge_request.iid}\)/ @@ -558,6 +581,7 @@ describe Notify do it_behaves_like 'a note email' it_behaves_like 'an answer to an existing thread', 'issue' it_behaves_like 'it should show Gmail Actions View Issue link' + it_behaves_like 'an unsubscribeable thread' it 'has the correct subject' do is_expected.to have_subject /#{issue.title} \(##{issue.iid}\)/ @@ -579,6 +603,7 @@ describe Notify do it_behaves_like 'an email sent from GitLab' it_behaves_like 'it should not have Gmail Actions links' + it_behaves_like "a user can not unsubscribe through footer link" it 'has the correct subject' do is_expected.to have_subject /Access to group was granted/ @@ -607,6 +632,7 @@ describe Notify do subject { ActionMailer::Base.deliveries.last } it_behaves_like 'an email sent from GitLab' + it_behaves_like "a user can not unsubscribe through footer link" it 'is sent to the new user' do is_expected.to deliver_to 'new-email@mail.com' @@ -629,6 +655,7 @@ describe Notify do subject { Notify.repository_push_email(project.id, 'devs@company.name', author_id: user.id, ref: 'refs/heads/master', action: :create) } it_behaves_like 'it should not have Gmail Actions links' + it_behaves_like "a user can not unsubscribe through footer link" it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -657,6 +684,7 @@ describe Notify do subject { Notify.repository_push_email(project.id, 'devs@company.name', author_id: user.id, ref: 'refs/tags/v1.0', action: :create) } it_behaves_like 'it should not have Gmail Actions links' + it_behaves_like "a user can not unsubscribe through footer link" it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -684,6 +712,7 @@ describe Notify do subject { Notify.repository_push_email(project.id, 'devs@company.name', author_id: user.id, ref: 'refs/heads/master', action: :delete) } it_behaves_like 'it should not have Gmail Actions links' + it_behaves_like "a user can not unsubscribe through footer link" it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -707,6 +736,7 @@ describe Notify do subject { Notify.repository_push_email(project.id, 'devs@company.name', author_id: user.id, ref: 'refs/tags/v1.0', action: :delete) } it_behaves_like 'it should not have Gmail Actions links' + it_behaves_like "a user can not unsubscribe through footer link" it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -734,6 +764,7 @@ describe Notify do subject { Notify.repository_push_email(project.id, 'devs@company.name', author_id: user.id, ref: 'refs/heads/master', action: :push, compare: compare, reverse_compare: false, send_from_committer_email: send_from_committer_email) } it_behaves_like 'it should not have Gmail Actions links' + it_behaves_like "a user can not unsubscribe through footer link" it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -839,6 +870,7 @@ describe Notify do subject { Notify.repository_push_email(project.id, 'devs@company.name', author_id: user.id, ref: 'refs/heads/master', action: :push, compare: compare) } it_behaves_like 'it should show Gmail Actions View Commit link' + it_behaves_like "a user can not unsubscribe through footer link" it 'is sent as the author' do sender = subject.header[:from].addrs[0] -- cgit v1.2.1 From 26cedc7e0bd83fc488da3a4dc5265d395639215f Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Sat, 9 Jan 2016 19:32:03 +0100 Subject: Minor improvements, unsubscribe from email footer --- spec/mailers/notify_spec.rb | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) (limited to 'spec') diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 08ade644643..8f86c491d3f 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -108,7 +108,7 @@ describe Notify do it { is_expected.to have_body_text /unsubscribe/ } end - shared_examples "a user can not unsubscribe through footer link" do + shared_examples "a user cannot unsubscribe through footer link" do it { is_expected.not_to have_body_text /unsubscribe/ } end @@ -123,7 +123,7 @@ describe Notify do it_behaves_like 'an email sent from GitLab' it_behaves_like 'a new user email', new_user_address it_behaves_like 'it should not have Gmail Actions links' - it_behaves_like 'a user can not unsubscribe through footer link' + it_behaves_like 'a user cannot unsubscribe through footer link' it 'contains the password text' do is_expected.to have_body_text /Click here to set your password/ @@ -152,7 +152,7 @@ describe Notify do it_behaves_like 'an email sent from GitLab' it_behaves_like 'a new user email', new_user_address it_behaves_like 'it should not have Gmail Actions links' - it_behaves_like 'a user can not unsubscribe through footer link' + it_behaves_like 'a user cannot unsubscribe through footer link' it 'should not contain the new user\'s password' do is_expected.not_to have_body_text /password/ @@ -166,7 +166,7 @@ describe Notify do it_behaves_like 'an email sent from GitLab' it_behaves_like 'it should not have Gmail Actions links' - it_behaves_like 'a user can not unsubscribe through footer link' + it_behaves_like 'a user cannot unsubscribe through footer link' it 'is sent to the new user' do is_expected.to deliver_to key.user.email @@ -191,7 +191,7 @@ describe Notify do subject { Notify.new_email_email(email.id) } it_behaves_like 'it should not have Gmail Actions links' - it_behaves_like 'a user can not unsubscribe through footer link' + it_behaves_like 'a user cannot unsubscribe through footer link' it 'is sent to the new user' do is_expected.to deliver_to email.user.email @@ -465,7 +465,7 @@ describe Notify do it_behaves_like 'an email sent from GitLab' it_behaves_like 'it should not have Gmail Actions links' - it_behaves_like "a user can not unsubscribe through footer link" + it_behaves_like "a user cannot unsubscribe through footer link" it 'has the correct subject' do is_expected.to have_subject /Project was moved/ @@ -488,7 +488,7 @@ describe Notify do it_behaves_like 'an email sent from GitLab' it_behaves_like 'it should not have Gmail Actions links' - it_behaves_like "a user can not unsubscribe through footer link" + it_behaves_like "a user cannot unsubscribe through footer link" it 'has the correct subject' do is_expected.to have_subject /Access to project was granted/ @@ -539,7 +539,7 @@ describe Notify do it_behaves_like 'a note email' it_behaves_like 'an answer to an existing thread', 'commit' it_behaves_like 'it should show Gmail Actions View Commit link' - it_behaves_like "a user can not unsubscribe through footer link" + it_behaves_like "a user cannot unsubscribe through footer link" it 'has the correct subject' do is_expected.to have_subject /#{commit.title} \(#{commit.short_id}\)/ @@ -603,7 +603,7 @@ describe Notify do it_behaves_like 'an email sent from GitLab' it_behaves_like 'it should not have Gmail Actions links' - it_behaves_like "a user can not unsubscribe through footer link" + it_behaves_like "a user cannot unsubscribe through footer link" it 'has the correct subject' do is_expected.to have_subject /Access to group was granted/ @@ -632,7 +632,7 @@ describe Notify do subject { ActionMailer::Base.deliveries.last } it_behaves_like 'an email sent from GitLab' - it_behaves_like "a user can not unsubscribe through footer link" + it_behaves_like "a user cannot unsubscribe through footer link" it 'is sent to the new user' do is_expected.to deliver_to 'new-email@mail.com' @@ -655,7 +655,7 @@ describe Notify do subject { Notify.repository_push_email(project.id, 'devs@company.name', author_id: user.id, ref: 'refs/heads/master', action: :create) } it_behaves_like 'it should not have Gmail Actions links' - it_behaves_like "a user can not unsubscribe through footer link" + it_behaves_like "a user cannot unsubscribe through footer link" it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -684,7 +684,7 @@ describe Notify do subject { Notify.repository_push_email(project.id, 'devs@company.name', author_id: user.id, ref: 'refs/tags/v1.0', action: :create) } it_behaves_like 'it should not have Gmail Actions links' - it_behaves_like "a user can not unsubscribe through footer link" + it_behaves_like "a user cannot unsubscribe through footer link" it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -712,7 +712,7 @@ describe Notify do subject { Notify.repository_push_email(project.id, 'devs@company.name', author_id: user.id, ref: 'refs/heads/master', action: :delete) } it_behaves_like 'it should not have Gmail Actions links' - it_behaves_like "a user can not unsubscribe through footer link" + it_behaves_like "a user cannot unsubscribe through footer link" it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -736,7 +736,7 @@ describe Notify do subject { Notify.repository_push_email(project.id, 'devs@company.name', author_id: user.id, ref: 'refs/tags/v1.0', action: :delete) } it_behaves_like 'it should not have Gmail Actions links' - it_behaves_like "a user can not unsubscribe through footer link" + it_behaves_like "a user cannot unsubscribe through footer link" it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -764,7 +764,7 @@ describe Notify do subject { Notify.repository_push_email(project.id, 'devs@company.name', author_id: user.id, ref: 'refs/heads/master', action: :push, compare: compare, reverse_compare: false, send_from_committer_email: send_from_committer_email) } it_behaves_like 'it should not have Gmail Actions links' - it_behaves_like "a user can not unsubscribe through footer link" + it_behaves_like "a user cannot unsubscribe through footer link" it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -870,7 +870,7 @@ describe Notify do subject { Notify.repository_push_email(project.id, 'devs@company.name', author_id: user.id, ref: 'refs/heads/master', action: :push, compare: compare) } it_behaves_like 'it should show Gmail Actions View Commit link' - it_behaves_like "a user can not unsubscribe through footer link" + it_behaves_like "a user cannot unsubscribe through footer link" it 'is sent as the author' do sender = subject.header[:from].addrs[0] -- cgit v1.2.1 From 96bbc145f31ad029e080ad8903445d81d6c31968 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Mon, 11 Jan 2016 10:20:45 +0100 Subject: Change commit builds URL in builds API --- spec/requests/api/builds_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'spec') diff --git a/spec/requests/api/builds_spec.rb b/spec/requests/api/builds_spec.rb index 799558d1bdd..587fb74750d 100644 --- a/spec/requests/api/builds_spec.rb +++ b/spec/requests/api/builds_spec.rb @@ -46,11 +46,11 @@ describe API::API, api: true do end end - describe 'GET /projects/:id/builds/commit/:sha' do + describe 'GET /projects/:id/repository/commits/:sha/builds' do context 'authorized user' do it 'should return project builds for specific commit' do project.ensure_ci_commit(commit.sha) - get api("/projects/#{project.id}/builds/commit/#{commit.sha}", user) + get api("/projects/#{project.id}/repository/commits/#{commit.sha}/builds", user) expect(response.status).to eq(200) expect(json_response).to be_an Array @@ -60,7 +60,7 @@ describe API::API, api: true do context 'unauthorized user' do it 'should not return project builds' do project.ensure_ci_commit(commit.sha) - get api("/projects/#{project.id}/builds/commit/#{commit.sha}") + get api("/projects/#{project.id}/repository/commits/#{commit.sha}/builds") expect(response.status).to eq(401) end -- cgit v1.2.1 From 5ba0232f7905a0c2a55ff80cbb97ddf2404fa22c Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Mon, 11 Jan 2016 16:30:01 +0100 Subject: Fix :ci_build_with_trace factory --- spec/factories/ci/builds.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index ce68457f86b..558598d4d5c 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -58,7 +58,9 @@ FactoryGirl.define do factory :ci_build_with_trace do id 999 - trace 'BUILD TRACE' + after(:build) do |build, evaluator| + build.trace = 'BUILD TRACE' + end end factory :ci_build_canceled do -- cgit v1.2.1 From ac6a10f3e88c5d2081b8638df63016089517a844 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Tue, 12 Jan 2016 12:29:10 -0200 Subject: Codestyle changes --- spec/models/identity_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec') diff --git a/spec/models/identity_spec.rb b/spec/models/identity_spec.rb index 107bfc17782..5afe042e154 100644 --- a/spec/models/identity_spec.rb +++ b/spec/models/identity_spec.rb @@ -28,11 +28,11 @@ RSpec.describe Identity, models: true do let(:other_identity) { create(:identity, provider: 'twitter') } it 'returns true if it is a ldap identity' do - expect(ldap_identity.is_ldap?).to be_truthy + expect(ldap_identity.ldap?).to be_truthy end it 'returns false if it is not a ldap identity' do - expect(other_identity.is_ldap?).to be_falsey + expect(other_identity.ldap?).to be_falsey end end end -- cgit v1.2.1 From 100cdce21e6dafa1b5d32313436fc610e8c88648 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 12 Jan 2016 13:13:16 -0500 Subject: Make sure time_ago_with_tooltip is using a Time object Somehow this test existed on EE but not in CE, so it started failing after a bad CE-to-EE merge. --- spec/helpers/application_helper_spec.rb | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'spec') diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index efc850eb705..30e353148a8 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -285,6 +285,10 @@ describe ApplicationHelper do it 'allows the script tag to be excluded' do expect(element(skip_js: true)).not_to include 'script' end + + it 'converts to Time' do + expect { helper.time_ago_with_tooltip(Date.today) }.not_to raise_error + end end describe 'render_markup' do -- cgit v1.2.1 From 63363e47f4228e15585b0908d91c8749b1067d37 Mon Sep 17 00:00:00 2001 From: Jacob Schatz Date: Tue, 12 Jan 2016 14:55:54 -0500 Subject: reverting more MR ajax files, will appear in different commit --- .../fixtures/merge_requests_show.html.haml | 12 +-- spec/javascripts/merge_request_spec.js.coffee | 88 ---------------------- 2 files changed, 1 insertion(+), 99 deletions(-) (limited to 'spec') diff --git a/spec/javascripts/fixtures/merge_requests_show.html.haml b/spec/javascripts/fixtures/merge_requests_show.html.haml index fdfa8a273e2..8447dfdda32 100644 --- a/spec/javascripts/fixtures/merge_requests_show.html.haml +++ b/spec/javascripts/fixtures/merge_requests_show.html.haml @@ -1,14 +1,4 @@ -:css - .hidden { display: none !important } - -.flash-container - .flash-alert - .flash-notice - -.status-box.status-box-open Open -.status-box.status-box-closed.hidden Closed -%a.btn-close{"href" => "http://gitlab.com/merge_requests/6/close"} Close -%a.btn-reopen.hidden{"href" => "http://gitlab.com/merge_requests/6/reopen"} Reopen +%a.btn-close .detail-page-description .description.js-task-list-container diff --git a/spec/javascripts/merge_request_spec.js.coffee b/spec/javascripts/merge_request_spec.js.coffee index e21bfde38ad..22ebc7039d1 100644 --- a/spec/javascripts/merge_request_spec.js.coffee +++ b/spec/javascripts/merge_request_spec.js.coffee @@ -21,91 +21,3 @@ describe 'MergeRequest', -> expect(req.data.merge_request.description).not.toBe(null) $('.js-task-list-field').trigger('tasklist:changed') - - describe 'reopen/close merge request', -> - fixture.preload('merge_requests_show.html') - beforeEach -> - fixture.load('merge_requests_show.html') - @merge_request = new MergeRequest({}) - it 'closes a merge request', -> - $.ajax = (obj) -> - expect(obj.type).toBe('PUT') - expect(obj.url).toBe('http://gitlab.com/merge_requests/6/close') - obj.success saved:true - - $btnClose = $('a.btn-close') - $btnReopen = $('a.btn-reopen') - expect($btnReopen).toBeHidden() - expect($btnClose.text()).toBe('Close') - expect(typeof $btnClose.prop('disabled')).toBe('undefined') - - $btnClose.trigger('click') - - expect($btnReopen).toBeVisible() - - expect($btnClose).toBeHidden() - expect($('div.status-box-closed')).toBeVisible() - expect($('div.status-box-open')).toBeHidden() - - it 'fails to close a merge request with success:false', -> - - $.ajax = (obj) -> - expect(obj.type).toBe('PUT') - expect(obj.url).toBe('http://goesnowhere.nothing/whereami') - obj.success saved:false - - $btnClose = $('a.btn-close') - $btnReopen = $('a.btn-reopen') - $btnClose.attr('href','http://goesnowhere.nothing/whereami') - expect($btnReopen).toBeHidden() - expect($btnClose.text()).toBe('Close') - expect(typeof $btnClose.prop('disabled')).toBe('undefined') - - $btnClose.trigger('click') - - expect($btnReopen).toBeHidden() - expect($btnClose).toBeVisible() - expect($('div.status-box-closed')).toBeHidden() - expect($('div.status-box-open')).toBeVisible() - expect($('div.flash-alert')).toBeVisible() - expect($('div.flash-alert').text()).toBe('Unable to update this merge request at this time.') - - it 'fails to closes an issue with HTTP error', -> - - $.ajax = (obj) -> - expect(obj.type).toBe('PUT') - expect(obj.url).toBe('http://goesnowhere.nothing/whereami') - obj.error() - - $btnClose = $('a.btn-close') - $btnReopen = $('a.btn-reopen') - $btnClose.attr('href','http://goesnowhere.nothing/whereami') - expect($btnReopen).toBeHidden() - expect($btnClose.text()).toBe('Close') - expect(typeof $btnClose.prop('disabled')).toBe('undefined') - - $btnClose.trigger('click') - - expect($btnReopen).toBeHidden() - expect($btnClose).toBeVisible() - expect($('div.status-box-closed')).toBeHidden() - expect($('div.status-box-open')).toBeVisible() - expect($('div.flash-alert')).toBeVisible() - expect($('div.flash-alert').text()).toBe('Unable to update this merge request at this time.') - - it 'reopens a merge request', -> - $.ajax = (obj) -> - expect(obj.type).toBe('PUT') - expect(obj.url).toBe('http://gitlab.com/merge_requests/6/reopen') - obj.success saved: true - - $btnClose = $('a.btn-close') - $btnReopen = $('a.btn-reopen') - expect($btnReopen.text()).toBe('Reopen') - - $btnReopen.trigger('click') - - expect($btnReopen).toBeHidden() - expect($btnClose).toBeVisible() - expect($('div.status-box-open')).toBeVisible() - expect($('div.status-box-closed')).toBeHidden() \ No newline at end of file -- cgit v1.2.1 From da40274fdc60fe17f928b80eb71c211e27523d5e Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 12 Jan 2016 20:48:16 -0500 Subject: Block the reported user before destroying the record This is intended to prevent the user from creating new objects while the transaction that removes them is being run, resulting in objects with nil authors which can then not be edited. See https://gitlab.com/gitlab-org/gitlab-ce/issues/7117 --- spec/models/abuse_report_spec.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'spec') diff --git a/spec/models/abuse_report_spec.rb b/spec/models/abuse_report_spec.rb index 46cab1644c7..f9be8fcbcfe 100644 --- a/spec/models/abuse_report_spec.rb +++ b/spec/models/abuse_report_spec.rb @@ -29,6 +29,22 @@ RSpec.describe AbuseReport, type: :model do it { is_expected.to validate_uniqueness_of(:user_id) } end + describe '#remove_user' do + it 'blocks the user' do + report = build(:abuse_report) + + allow(report.user).to receive(:destroy) + + expect { report.remove_user }.to change { report.user.blocked? }.to(true) + end + + it 'removes the user' do + report = build(:abuse_report) + + expect { report.remove_user }.to change { User.count }.by(-1) + end + end + describe '#notify' do it 'delivers' do expect(AbuseReportMailer).to receive(:notify).with(subject.id). -- cgit v1.2.1 From d44653da1f74c2c15fe7ec3f8aa9b16563ffebd6 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Wed, 13 Jan 2016 12:16:27 +0100 Subject: Add some fixes after review --- spec/requests/api/triggers_spec.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'spec') diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb index e8d89426ec0..2a86b60bc4d 100644 --- a/spec/requests/api/triggers_spec.rb +++ b/spec/requests/api/triggers_spec.rb @@ -105,7 +105,7 @@ describe API::API do end end - context 'unauthentikated user' do + context 'unauthenticated user' do it 'should not return triggers list' do get api("/projects/#{project.id}/triggers") @@ -123,7 +123,7 @@ describe API::API do expect(json_response).to be_a(Hash) end - it 'should responde with 404 Not Found if requesting non-existing trigger' do + it 'should respond with 404 Not Found if requesting non-existing trigger' do get api("/projects/#{project.id}/triggers/abcdef012345", user) expect(response.status).to eq(404) @@ -138,7 +138,7 @@ describe API::API do end end - context 'unauthentikated user' do + context 'unauthenticated user' do it 'should not return triggers list' do get api("/projects/#{project.id}/triggers/#{trigger.token}") @@ -167,7 +167,7 @@ describe API::API do end end - context 'unauthentikated user' do + context 'unauthenticated user' do it 'should not create trigger' do post api("/projects/#{project.id}/triggers") @@ -185,7 +185,7 @@ describe API::API do expect(response.status).to eq(200) end - it 'should responde with 404 Not Found if requesting non-existing trigger' do + it 'should respond with 404 Not Found if requesting non-existing trigger' do delete api("/projects/#{project.id}/triggers/abcdef012345", user) expect(response.status).to eq(404) @@ -200,7 +200,7 @@ describe API::API do end end - context 'unauthentikated user' do + context 'unauthenticated user' do it 'should not delete trigger' do delete api("/projects/#{project.id}/triggers/#{trigger.token}") -- cgit v1.2.1 From 057eb824b5d7f38547506303dc80da6164715420 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Wed, 13 Jan 2016 12:57:46 +0100 Subject: Randomize metrics sample intervals Sampling data at a fixed interval means we can potentially miss data from events occurring between sampling intervals. For example, say we sample data every 15 seconds but Unicorn workers get killed after 10 seconds. In this particular case it's possible to miss interesting data as the sampler will never get to actually submitting data. To work around this (at least for the most part) the sampling interval is randomized as following: 1. Take the user specified sampling interval (15 seconds by default) 2. Divide it by 2 (referred to as "half" below) 3. Generate a range (using a step of 0.1) from -"half" to "half" 4. Every time the sampler goes to sleep we'll grab the user provided interval and add a randomly chosen "adjustment" to it while making sure we don't pick the same value twice in a row. For a specified timeout of 15 this means the actual intervals can be anywhere between 7.5 and 22.5, but never can the same interval be used twice in a row. The rationale behind this change is that on dev.gitlab.org I'm sometimes seeing certain Gitlab::Git/Rugged objects being retained, but only for a few minutes every 24 hours. Knowing the code of Gitlab and how much memory it uses/leaks I suspect we're missing data due to workers getting terminated before the sampler can write its data to InfluxDB. --- spec/lib/gitlab/metrics/sampler_spec.rb | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/lib/gitlab/metrics/sampler_spec.rb b/spec/lib/gitlab/metrics/sampler_spec.rb index 27211350fbe..38da77adc9f 100644 --- a/spec/lib/gitlab/metrics/sampler_spec.rb +++ b/spec/lib/gitlab/metrics/sampler_spec.rb @@ -9,7 +9,7 @@ describe Gitlab::Metrics::Sampler do describe '#start' do it 'gathers a sample at a given interval' do - expect(sampler).to receive(:sleep).with(5) + expect(sampler).to receive(:sleep).with(a_kind_of(Numeric)) expect(sampler).to receive(:sample) expect(sampler).to receive(:loop).and_yield @@ -116,4 +116,24 @@ describe Gitlab::Metrics::Sampler do sampler.add_metric('cats', value: 10) end end + + describe '#sleep_interval' do + it 'returns a Numeric' do + expect(sampler.sleep_interval).to be_a_kind_of(Numeric) + end + + # Testing random behaviour is very hard, so treat this test as a basic smoke + # test instead of a very accurate behaviour/unit test. + it 'does not return the same interval twice in a row' do + last = nil + + 100.times do + interval = sampler.sleep_interval + + expect(interval).to_not eq(last) + + last = interval + end + end + end end -- cgit v1.2.1 From 97338496188add9ec8d192c7e78f6a6040befffa Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Wed, 13 Jan 2016 15:17:59 +0100 Subject: Add some fixes after review --- spec/requests/api/builds_spec.rb | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'spec') diff --git a/spec/requests/api/builds_spec.rb b/spec/requests/api/builds_spec.rb index 587fb74750d..4bf3d2681dc 100644 --- a/spec/requests/api/builds_spec.rb +++ b/spec/requests/api/builds_spec.rb @@ -35,6 +35,12 @@ describe API::API, api: true do expect(response.status).to eq(200) expect(json_response).to be_an Array end + + it 'should respond 400 when scope contains invalid state' do + get api("/projects/#{project.id}/builds?scope[0]=pending&scope[1]=unknown_status", user) + + expect(response.status).to eq(400) + end end context 'unauthorized user' do -- cgit v1.2.1 From 990bd06c04bebe6319968aa619990bf4cb60483c Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Wed, 13 Jan 2016 16:05:49 +0100 Subject: Change :ci_build_canceled factory to :canceled trait --- spec/factories/ci/builds.rb | 8 ++++---- spec/requests/api/builds_spec.rb | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) (limited to 'spec') diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index 558598d4d5c..636cdbba1b8 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -43,6 +43,10 @@ FactoryGirl.define do commit factory: :ci_commit + trait :canceled do + status 'canceled' + end + after(:build) do |build, evaluator| build.project = build.commit.project end @@ -62,9 +66,5 @@ FactoryGirl.define do build.trace = 'BUILD TRACE' end end - - factory :ci_build_canceled do - status 'canceled' - end end end diff --git a/spec/requests/api/builds_spec.rb b/spec/requests/api/builds_spec.rb index 4bf3d2681dc..e5567d42500 100644 --- a/spec/requests/api/builds_spec.rb +++ b/spec/requests/api/builds_spec.rb @@ -11,7 +11,7 @@ describe API::API, api: true do let(:commit) { create(:ci_commit, project: project)} let(:build) { create(:ci_build, commit: commit) } let(:build_with_trace) { create(:ci_build_with_trace, commit: commit) } - let(:build_canceled) { create(:ci_build_canceled, commit: commit) } + let(:build_canceled) { create(:ci_build, :canceled, commit: commit) } describe 'GET /projects/:id/builds ' do context 'authorized user' do -- cgit v1.2.1 From 6ae39c2cd1edcc845136739d42baf032120e3ddc Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Thu, 31 Dec 2015 15:56:15 -0500 Subject: Remove alert_type attribute from BroadcastMessage --- spec/models/broadcast_message_spec.rb | 1 - 1 file changed, 1 deletion(-) (limited to 'spec') diff --git a/spec/models/broadcast_message_spec.rb b/spec/models/broadcast_message_spec.rb index e4cac105110..001b2facaab 100644 --- a/spec/models/broadcast_message_spec.rb +++ b/spec/models/broadcast_message_spec.rb @@ -6,7 +6,6 @@ # message :text not null # starts_at :datetime # ends_at :datetime -# alert_type :integer # created_at :datetime # updated_at :datetime # color :string(255) -- cgit v1.2.1 From 8086b2bd2ecb0ff647da766c436eda47b7434599 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Thu, 31 Dec 2015 16:04:07 -0500 Subject: Simplify BroadcastMessage factory Also make the feature tests less brittle. --- spec/factories/broadcast_messages.rb | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'spec') diff --git a/spec/factories/broadcast_messages.rb b/spec/factories/broadcast_messages.rb index ea0039d39e6..0c7bc573612 100644 --- a/spec/factories/broadcast_messages.rb +++ b/spec/factories/broadcast_messages.rb @@ -6,7 +6,6 @@ # message :text not null # starts_at :datetime # ends_at :datetime -# alert_type :integer # created_at :datetime # updated_at :datetime # color :string(255) @@ -18,10 +17,12 @@ FactoryGirl.define do factory :broadcast_message do message "MyText" - starts_at "2013-11-12 13:43:25" - ends_at "2013-11-12 13:43:25" - alert_type 1 - color "#555555" - font "#BBBBBB" + starts_at Date.today + ends_at Date.tomorrow + + trait :expired do + starts_at 5.days.ago + ends_at 3.days.ago + end end end -- cgit v1.2.1 From 5a1706791e46fd69a67fcdebfc384a702b00a7ff Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Thu, 31 Dec 2015 16:42:56 -0500 Subject: Update broadcast_message helper Now it returns the fully-formatted message so we can be consistent about how it's shown. --- spec/helpers/broadcast_messages_helper_spec.rb | 42 ++++++++++++++++++-------- 1 file changed, 30 insertions(+), 12 deletions(-) (limited to 'spec') diff --git a/spec/helpers/broadcast_messages_helper_spec.rb b/spec/helpers/broadcast_messages_helper_spec.rb index c7c6f45d144..0fb8a7284f3 100644 --- a/spec/helpers/broadcast_messages_helper_spec.rb +++ b/spec/helpers/broadcast_messages_helper_spec.rb @@ -1,22 +1,40 @@ require 'spec_helper' describe BroadcastMessagesHelper do - describe 'broadcast_styling' do - let(:broadcast_message) { double(color: '', font: '') } + describe 'broadcast_message' do + it 'returns nil when no current message' do + expect(helper.broadcast_message(nil)).to be_nil + end + + it 'includes the current message' do + current = double(message: 'Current Message') + + allow(helper).to receive(:broadcast_message_style).and_return(nil) + + expect(helper.broadcast_message(current)).to include 'Current Message' + end + + it 'includes custom style' do + current = double(message: 'Current Message') + + allow(helper).to receive(:broadcast_message_style).and_return('foo') + + expect(helper.broadcast_message(current)).to include 'style="foo"' + end + end + + describe 'broadcast_message_style' do + it 'defaults to no style' do + broadcast_message = spy - context "default style" do - it "should have no style" do - expect(broadcast_styling(broadcast_message)).to eq '' - end + expect(helper.broadcast_message_style(broadcast_message)).to eq '' end - context "customized style" do - let(:broadcast_message) { double(color: "#f2dede", font: '#b94a48') } + it 'allows custom style' do + broadcast_message = double(color: '#f2dede', font: '#b94a48') - it "should have a customized style" do - expect(broadcast_styling(broadcast_message)). - to match('background-color: #f2dede; color: #b94a48') - end + expect(helper.broadcast_message_style(broadcast_message)). + to match('background-color: #f2dede; color: #b94a48') end end end -- cgit v1.2.1 From df496fcaf08b085816a45d31d02f1ea230454b63 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Thu, 31 Dec 2015 17:07:11 -0500 Subject: Update BroadcastMessage model - Adds default values for `color` and `font` attributes - Adds `active?`, `started?`, `ended?`, and 'status' methods --- spec/factories/broadcast_messages.rb | 5 ++ spec/models/broadcast_message_spec.rb | 91 +++++++++++++++++++++++++++++++++-- 2 files changed, 91 insertions(+), 5 deletions(-) (limited to 'spec') diff --git a/spec/factories/broadcast_messages.rb b/spec/factories/broadcast_messages.rb index 0c7bc573612..978a7d4cecb 100644 --- a/spec/factories/broadcast_messages.rb +++ b/spec/factories/broadcast_messages.rb @@ -24,5 +24,10 @@ FactoryGirl.define do starts_at 5.days.ago ends_at 3.days.ago end + + trait :future do + starts_at 5.days.from_now + ends_at 6.days.from_now + end end end diff --git a/spec/models/broadcast_message_spec.rb b/spec/models/broadcast_message_spec.rb index 001b2facaab..57550725ae3 100644 --- a/spec/models/broadcast_message_spec.rb +++ b/spec/models/broadcast_message_spec.rb @@ -15,6 +15,8 @@ require 'spec_helper' describe BroadcastMessage, models: true do + include ActiveSupport::Testing::TimeHelpers + subject { create(:broadcast_message) } it { is_expected.to be_valid } @@ -34,20 +36,99 @@ describe BroadcastMessage, models: true do it { is_expected.not_to allow_value('000').for(:font) } end - describe :current do + describe '.current' do it "should return last message if time match" do - broadcast_message = create(:broadcast_message, starts_at: Time.now.yesterday, ends_at: Time.now.tomorrow) - expect(BroadcastMessage.current).to eq(broadcast_message) + message = create(:broadcast_message) + + expect(BroadcastMessage.current).to eq message end it "should return nil if time not come" do - create(:broadcast_message, starts_at: Time.now.tomorrow, ends_at: Time.now + 2.days) + create(:broadcast_message, :future) + expect(BroadcastMessage.current).to be_nil end it "should return nil if time has passed" do - create(:broadcast_message, starts_at: Time.now - 2.days, ends_at: Time.now.yesterday) + create(:broadcast_message, :expired) + expect(BroadcastMessage.current).to be_nil end end + + describe '#active?' do + it 'is truthy when started and not ended' do + message = build(:broadcast_message) + + expect(message).to be_active + end + + it 'is falsey when ended' do + message = build(:broadcast_message, :expired) + + expect(message).not_to be_active + end + + it 'is falsey when not started' do + message = build(:broadcast_message, :future) + + expect(message).not_to be_active + end + end + + describe '#started?' do + it 'is truthy when starts_at has passed' do + message = build(:broadcast_message) + + travel_to(3.days.from_now) do + expect(message).to be_started + end + end + + it 'is falsey when starts_at is in the future' do + message = build(:broadcast_message) + + travel_to(3.days.ago) do + expect(message).not_to be_started + end + end + end + + describe '#ended?' do + it 'is truthy when ends_at has passed' do + message = build(:broadcast_message) + + travel_to(3.days.from_now) do + expect(message).to be_ended + end + end + + it 'is falsey when ends_at is in the future' do + message = build(:broadcast_message) + + travel_to(3.days.ago) do + expect(message).not_to be_ended + end + end + end + + describe '#status' do + it 'returns Active' do + message = build(:broadcast_message) + + expect(message.status).to eq 'Active' + end + + it 'returns Expired' do + message = build(:broadcast_message, :expired) + + expect(message.status).to eq 'Expired' + end + + it 'returns Pending' do + message = build(:broadcast_message, :future) + + expect(message.status).to eq 'Pending' + end + end end -- cgit v1.2.1 From 843662821ddbf2d06aa2da72ce32717cebecb7c6 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 13 Jan 2016 11:46:32 -0500 Subject: Move `BroadcastMessage#status` to a helper since it's presentational --- spec/helpers/broadcast_messages_helper_spec.rb | 20 ++++++++++++++++++++ spec/models/broadcast_message_spec.rb | 20 -------------------- 2 files changed, 20 insertions(+), 20 deletions(-) (limited to 'spec') diff --git a/spec/helpers/broadcast_messages_helper_spec.rb b/spec/helpers/broadcast_messages_helper_spec.rb index 0fb8a7284f3..157cc4665a2 100644 --- a/spec/helpers/broadcast_messages_helper_spec.rb +++ b/spec/helpers/broadcast_messages_helper_spec.rb @@ -37,4 +37,24 @@ describe BroadcastMessagesHelper do to match('background-color: #f2dede; color: #b94a48') end end + + describe 'broadcast_message_status' do + it 'returns Active' do + message = build(:broadcast_message) + + expect(helper.broadcast_message_status(message)).to eq 'Active' + end + + it 'returns Expired' do + message = build(:broadcast_message, :expired) + + expect(helper.broadcast_message_status(message)).to eq 'Expired' + end + + it 'returns Pending' do + message = build(:broadcast_message, :future) + + expect(helper.broadcast_message_status(message)).to eq 'Pending' + end + end end diff --git a/spec/models/broadcast_message_spec.rb b/spec/models/broadcast_message_spec.rb index 57550725ae3..f6f84db57e6 100644 --- a/spec/models/broadcast_message_spec.rb +++ b/spec/models/broadcast_message_spec.rb @@ -111,24 +111,4 @@ describe BroadcastMessage, models: true do end end end - - describe '#status' do - it 'returns Active' do - message = build(:broadcast_message) - - expect(message.status).to eq 'Active' - end - - it 'returns Expired' do - message = build(:broadcast_message, :expired) - - expect(message.status).to eq 'Expired' - end - - it 'returns Pending' do - message = build(:broadcast_message, :future) - - expect(message.status).to eq 'Pending' - end - end end -- cgit v1.2.1 From 1f0b8c32e75b446848cead98c550e750801be534 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Wed, 13 Jan 2016 18:18:59 +0100 Subject: Add spec for Note#cross_reference_not_visible_for? --- spec/models/note_spec.rb | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) (limited to 'spec') diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 151a29e974b..65e6a7df3b4 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -178,6 +178,30 @@ describe Note, models: true do end end + describe "cross_reference_not_visible_for?" do + let(:private_user) { create(:user) } + let(:private_project) { create(:project, namespace: private_user.namespace).tap { |p| p.team << [private_user, :master] } } + let(:private_issue) { create(:issue, project: private_project) } + + let(:ext_proj) { create(:project, :public) } + let(:ext_issue) { create(:issue, project: ext_proj) } + + let(:note) { + create :note, + noteable: ext_issue, project: ext_proj, + note: "mentioned in issue #{private_issue.to_reference(ext_proj)}", + system: true + } + + it "returns true" do + expect(note.cross_reference_not_visible_for?(ext_issue.author)).to be_truthy + end + + it "returns false" do + expect(note.cross_reference_not_visible_for?(private_user)).to be_falsy + end + end + describe "set_award!" do let(:issue) { create :issue } -- cgit v1.2.1 From 13032b713d0943c2b7e2f2a3b886ef06be8e88ef Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Wed, 13 Jan 2016 18:30:49 +0100 Subject: Add seperated 'describe' block for build trace specs --- spec/requests/api/builds_spec.rb | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) (limited to 'spec') diff --git a/spec/requests/api/builds_spec.rb b/spec/requests/api/builds_spec.rb index e5567d42500..8c9f5a382b7 100644 --- a/spec/requests/api/builds_spec.rb +++ b/spec/requests/api/builds_spec.rb @@ -73,7 +73,7 @@ describe API::API, api: true do end end - describe 'GET /projects/:id/builds/:build_id(/trace)?' do + describe 'GET /projects/:id/builds/:build_id' do context 'authorized user' do it 'should return specific build data' do get api("/projects/#{project.id}/builds/#{build.id}", user) @@ -81,7 +81,19 @@ describe API::API, api: true do expect(response.status).to eq(200) expect(json_response['name']).to eq('test') end + end + + context 'unauthorized user' do + it 'should not return specific build data' do + get api("/projects/#{project.id}/builds/#{build.id}") + expect(response.status).to eq(401) + end + end + end + + describe 'GET /projects/:id/builds/:build_id/trace' do + context 'authorized user' do it 'should return specific build trace' do get api("/projects/#{project.id}/builds/#{build_with_trace.id}/trace", user) @@ -91,12 +103,6 @@ describe API::API, api: true do end context 'unauthorized user' do - it 'should not return specific build data' do - get api("/projects/#{project.id}/builds/#{build.id}") - - expect(response.status).to eq(401) - end - it 'should not return specific build trace' do get api("/projects/#{project.id}/builds/#{build_with_trace.id}/trace") -- cgit v1.2.1 From 0c10aee59677e2dadfef6538a74fe1e28fcdd37e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Wed, 13 Jan 2016 19:42:36 +0100 Subject: Ensure the API doesn't return notes that the current user shouldn't see --- spec/requests/api/notes_spec.rb | 51 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) (limited to 'spec') diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index 8b177af4689..565805d870c 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -10,6 +10,24 @@ describe API::API, api: true do let!(:issue_note) { create(:note, noteable: issue, project: project, author: user) } let!(:merge_request_note) { create(:note, noteable: merge_request, project: project, author: user) } let!(:snippet_note) { create(:note, noteable: snippet, project: project, author: user) } + + # For testing the cross-reference of a private issue in a public issue + let(:private_user) { create(:user) } + let(:private_project) { + create(:project, namespace: private_user.namespace). + tap { |p| p.team << [private_user, :master] } + } + let(:private_issue) { create(:issue, project: private_project) } + let(:ext_proj) { create(:project, :public) } + let(:ext_issue) { create(:issue, project: ext_proj) } + + let!(:cross_reference_note) { + create :note, + noteable: ext_issue, project: ext_proj, + note: "mentioned in issue #{private_issue.to_reference(ext_proj)}", + system: true + } + before { project.team << [user, :reporter] } describe "GET /projects/:id/noteable/:noteable_id/notes" do @@ -25,6 +43,24 @@ describe API::API, api: true do get api("/projects/#{project.id}/issues/123/notes", user) expect(response.status).to eq(404) end + + context "that references a private issue" do + it "should return an empty array" do + get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes", user) + expect(response.status).to eq(200) + expect(json_response).to be_an Array + expect(json_response).to be_empty + end + + context "and current user can view the note" do + it "should return an empty array" do + get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes", private_user) + expect(response.status).to eq(200) + expect(json_response).to be_an Array + expect(json_response.first['body']).to eq(cross_reference_note.note) + end + end + end end context "when noteable is a Snippet" do @@ -68,6 +104,21 @@ describe API::API, api: true do get api("/projects/#{project.id}/issues/#{issue.id}/notes/123", user) expect(response.status).to eq(404) end + + context "that references a private issue" do + it "should return a 404 error" do + get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes/#{cross_reference_note.id}", user) + expect(response.status).to eq(404) + end + + context "and current user can view the note" do + it "should return an issue note by id" do + get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes/#{cross_reference_note.id}", private_user) + expect(response.status).to eq(200) + expect(json_response['body']).to eq(cross_reference_note.note) + end + end + end end context "when noteable is a Snippet" do -- cgit v1.2.1 From 3b7f34281e4d1c4ca626578ddc9a1b9eda7e7538 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Wed, 13 Jan 2016 19:57:23 +0100 Subject: Modify :ci_variable factory --- spec/factories/ci/variables.rb | 7 ++----- spec/requests/api/variables_spec.rb | 4 ++-- 2 files changed, 4 insertions(+), 7 deletions(-) (limited to 'spec') diff --git a/spec/factories/ci/variables.rb b/spec/factories/ci/variables.rb index a19b9fc72f2..8f62d64411b 100644 --- a/spec/factories/ci/variables.rb +++ b/spec/factories/ci/variables.rb @@ -16,10 +16,7 @@ FactoryGirl.define do factory :ci_variable, class: Ci::Variable do - id 10 - key 'TEST_VARIABLE_1' - value 'VALUE_1' - - project factory: :empty_project + sequence(:key) { |n| "VARIABLE_#{n}" } + value 'VARIABLE_VALUE' end end diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb index 214d7d5a0cc..9744729ba0c 100644 --- a/spec/requests/api/variables_spec.rb +++ b/spec/requests/api/variables_spec.rb @@ -46,7 +46,7 @@ describe API::API, api: true do expect(json_response['value']).to eq(variable.value) end - it 'should responde with 404 Not Found if requesting non-existing variable' do + it 'should respond with 404 Not Found if requesting non-existing variable' do get api("/projects/#{project.id}/variables/non_existing_variable", user) expect(response.status).to eq(404) @@ -84,7 +84,7 @@ describe API::API, api: true do it 'should not allow to duplicate variable key' do expect do - post api("/projects/#{project.id}/variables", user), key: 'TEST_VARIABLE_1', value: 'VALUE_2' + post api("/projects/#{project.id}/variables", user), key: variable.key, value: 'VALUE_2' end.to change{project.variables.count}.by(0) expect(response.status).to eq(400) -- cgit v1.2.1 From dd6fc01ff8a073880b67a323a547edeb5d63f167 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Thu, 14 Jan 2016 03:31:27 -0200 Subject: fixed LDAP activation on login to use new ldap_blocked state --- spec/lib/gitlab/ldap/access_spec.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'spec') diff --git a/spec/lib/gitlab/ldap/access_spec.rb b/spec/lib/gitlab/ldap/access_spec.rb index f58d70e809c..32a19bf344b 100644 --- a/spec/lib/gitlab/ldap/access_spec.rb +++ b/spec/lib/gitlab/ldap/access_spec.rb @@ -42,7 +42,6 @@ describe Gitlab::LDAP::Access, lib: true do context 'and has no disabled flag in active diretory' do before do - user.block allow(Gitlab::LDAP::Person).to receive(:disabled_via_active_directory?).and_return(false) end @@ -50,7 +49,7 @@ describe Gitlab::LDAP::Access, lib: true do context 'when auto-created users are blocked' do before do - allow_any_instance_of(Gitlab::LDAP::Config).to receive(:block_auto_created_users).and_return(true) + user.block end it 'does not unblock user in GitLab' do @@ -62,7 +61,7 @@ describe Gitlab::LDAP::Access, lib: true do context 'when auto-created users are not blocked' do before do - allow_any_instance_of(Gitlab::LDAP::Config).to receive(:block_auto_created_users).and_return(false) + user.ldap_block end it 'should unblock user in GitLab' do -- cgit v1.2.1 From e918493f55eb27cdb779f0bc2d8cbbace8b69aa9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 14 Jan 2016 10:04:48 +0100 Subject: Fix specs and rubocop warnings --- spec/models/note_spec.rb | 4 ++-- spec/requests/api/notes_spec.rb | 11 ++++++----- 2 files changed, 8 insertions(+), 7 deletions(-) (limited to 'spec') diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 65e6a7df3b4..9182b42661d 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -186,12 +186,12 @@ describe Note, models: true do let(:ext_proj) { create(:project, :public) } let(:ext_issue) { create(:issue, project: ext_proj) } - let(:note) { + let(:note) do create :note, noteable: ext_issue, project: ext_proj, note: "mentioned in issue #{private_issue.to_reference(ext_proj)}", system: true - } + end it "returns true" do expect(note.cross_reference_not_visible_for?(ext_issue.author)).to be_truthy diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index 565805d870c..d8bbd107269 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -13,20 +13,21 @@ describe API::API, api: true do # For testing the cross-reference of a private issue in a public issue let(:private_user) { create(:user) } - let(:private_project) { + let(:private_project) do create(:project, namespace: private_user.namespace). tap { |p| p.team << [private_user, :master] } - } - let(:private_issue) { create(:issue, project: private_project) } + end + let(:private_issue) { create(:issue, project: private_project) } + let(:ext_proj) { create(:project, :public) } let(:ext_issue) { create(:issue, project: ext_proj) } - let!(:cross_reference_note) { + let!(:cross_reference_note) do create :note, noteable: ext_issue, project: ext_proj, note: "mentioned in issue #{private_issue.to_reference(ext_proj)}", system: true - } + end before { project.team << [user, :reporter] } -- cgit v1.2.1 From 6a98fb03e708070641f9fce0eaad761e859a5099 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Thu, 14 Jan 2016 12:04:44 +0100 Subject: Remove hard-coded id from builds factory --- spec/factories/ci/builds.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'spec') diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index 636cdbba1b8..d2db77f6286 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -61,8 +61,7 @@ FactoryGirl.define do end factory :ci_build_with_trace do - id 999 - after(:build) do |build, evaluator| + after(:create) do |build, evaluator| build.trace = 'BUILD TRACE' end end -- cgit v1.2.1 From 3183092ca94b14d6e61f5e8ba51069554646baf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 14 Jan 2016 12:08:44 +0100 Subject: Add pagination headers to already paginated API resources --- spec/requests/api/commit_status_spec.rb | 6 +++++- spec/requests/api/notes_spec.rb | 4 ++++ spec/support/api/pagination_shared_examples.rb | 20 ++++++++++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 spec/support/api/pagination_shared_examples.rb (limited to 'spec') diff --git a/spec/requests/api/commit_status_spec.rb b/spec/requests/api/commit_status_spec.rb index a28607bd240..21482fc1070 100644 --- a/spec/requests/api/commit_status_spec.rb +++ b/spec/requests/api/commit_status_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe API::API, api: true do +describe API::CommitStatus, api: true do include ApiHelpers let(:user) { create(:user) } let(:user2) { create(:user) } @@ -12,6 +12,10 @@ describe API::API, api: true do let(:commit_status) { create(:commit_status, commit: ci_commit) } describe "GET /projects/:id/repository/commits/:sha/statuses" do + it_behaves_like 'a paginated resources' do + let(:request) { get api("/projects/#{project.id}/repository/commits/#{commit.id}/statuses", user) } + end + context "reporter user" do let(:statuses_id) { json_response.map { |status| status['id'] } } diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index d8bbd107269..39f9a06fe1b 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -32,6 +32,10 @@ describe API::API, api: true do before { project.team << [user, :reporter] } describe "GET /projects/:id/noteable/:noteable_id/notes" do + it_behaves_like 'a paginated resources' do + let(:request) { get api("/projects/#{project.id}/issues/#{issue.id}/notes", user) } + end + context "when noteable is an Issue" do it "should return an array of issue notes" do get api("/projects/#{project.id}/issues/#{issue.id}/notes", user) diff --git a/spec/support/api/pagination_shared_examples.rb b/spec/support/api/pagination_shared_examples.rb new file mode 100644 index 00000000000..352a6eeec79 --- /dev/null +++ b/spec/support/api/pagination_shared_examples.rb @@ -0,0 +1,20 @@ +# Specs for paginated resources. +# +# Requires an API request: +# let(:request) { get api("/projects/#{project.id}/repository/branches", user) } +shared_examples 'a paginated resources' do + before do + # Fires the request + request + end + + it 'has pagination headers' do + expect(response.headers).to include('X-Total') + expect(response.headers).to include('X-Total-Pages') + expect(response.headers).to include('X-Per-Page') + expect(response.headers).to include('X-Page') + expect(response.headers).to include('X-Next-Page') + expect(response.headers).to include('X-Prev-Page') + expect(response.headers).to include('Link') + end +end -- cgit v1.2.1 From 2c6b11198aa636b7e54a55bf63d0daabcfb716d8 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 17 Dec 2015 09:22:02 +0100 Subject: Add CI build artifacts tarball as a spec fixture --- spec/fixtures/ci_build_artifacts.tar.gz | Bin 0 -> 34085 bytes 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 spec/fixtures/ci_build_artifacts.tar.gz (limited to 'spec') diff --git a/spec/fixtures/ci_build_artifacts.tar.gz b/spec/fixtures/ci_build_artifacts.tar.gz new file mode 100644 index 00000000000..a60de1bf95e Binary files /dev/null and b/spec/fixtures/ci_build_artifacts.tar.gz differ -- cgit v1.2.1 From f5d530865875440d69217cf249715bffaa3d11b8 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 18 Dec 2015 11:59:10 +0100 Subject: Add implementation of StringPath class `StringPath` class is something similar to Ruby's `Pathname` class, but does not involve any IO operations. `StringPath` objects require passing string representation of path, and array of paths that represents universe to constructor to be intantiated. --- spec/lib/gitlab/string_path_spec.rb | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 spec/lib/gitlab/string_path_spec.rb (limited to 'spec') diff --git a/spec/lib/gitlab/string_path_spec.rb b/spec/lib/gitlab/string_path_spec.rb new file mode 100644 index 00000000000..14a08bcb49b --- /dev/null +++ b/spec/lib/gitlab/string_path_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +describe Gitlab::StringPath do + let(:universe) do + ['path/dir_1/', + 'path/dir_1/file_1', + 'path/second_dir', + 'path/second_dir/dir_3/file_2', + 'path/second_dir/dir_3/file_3', + 'another_file', + '/file/with/absolute_path'] + end + + describe '/file/with/absolute_path' do + subject { described_class.new('/file/with/absolute_path', universe) } + + it { is_expected.to be_absolute } + it { is_expected.to_not be_relative } + it { is_expected.to be_file } + end +end -- cgit v1.2.1 From 73d2c7a553ca239cdce04af793992fd579ad3e4b Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 18 Dec 2015 12:32:21 +0100 Subject: Add new methods to StringPath --- spec/lib/gitlab/string_path_spec.rb | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/lib/gitlab/string_path_spec.rb b/spec/lib/gitlab/string_path_spec.rb index 14a08bcb49b..6e75e1f3ced 100644 --- a/spec/lib/gitlab/string_path_spec.rb +++ b/spec/lib/gitlab/string_path_spec.rb @@ -2,8 +2,10 @@ require 'spec_helper' describe Gitlab::StringPath do let(:universe) do - ['path/dir_1/', + ['path/', + 'path/dir_1/', 'path/dir_1/file_1', + 'path/dir_1/file_b', 'path/second_dir', 'path/second_dir/dir_3/file_2', 'path/second_dir/dir_3/file_3', @@ -17,5 +19,34 @@ describe Gitlab::StringPath do it { is_expected.to be_absolute } it { is_expected.to_not be_relative } it { is_expected.to be_file } + + describe '#basename' do + subject { described_class.new('/file/with/absolute_path', universe).basename } + + it { is_expected.to eq 'absolute_path' } + end + end + + describe 'path/' do + subject { described_class.new('path/', universe) } + + it { is_expected.to be_directory } + it { is_expected.to be_relative } + end + + describe 'path/dir_1/' do + describe '#files' do + subject { described_class.new('path/dir_1/', universe).files } + + pending { is_expected.to all(be_an_instance_of described_class) } + pending { is_expected.to be eq [Gitlab::StringPath.new('path/dir_1/file_1', universe), + Gitlab::StringPath.new('path/dir_1/file_b', universe)] } + end + + describe '#basename' do + subject { described_class.new('path/dir_1/', universe).basename } + + it { is_expected.to eq 'dir_1/' } + end end end -- cgit v1.2.1 From 518b206287318006f9b57382a747b1474b6795a4 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Sat, 19 Dec 2015 09:31:52 +0100 Subject: Add `parent` iteration implementation to `StringPath` --- spec/lib/gitlab/string_path_spec.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/lib/gitlab/string_path_spec.rb b/spec/lib/gitlab/string_path_spec.rb index 6e75e1f3ced..86e48f6ee0b 100644 --- a/spec/lib/gitlab/string_path_spec.rb +++ b/spec/lib/gitlab/string_path_spec.rb @@ -19,6 +19,7 @@ describe Gitlab::StringPath do it { is_expected.to be_absolute } it { is_expected.to_not be_relative } it { is_expected.to be_file } + it { is_expected.to_not have_parent } describe '#basename' do subject { described_class.new('/file/with/absolute_path', universe).basename } @@ -32,9 +33,13 @@ describe Gitlab::StringPath do it { is_expected.to be_directory } it { is_expected.to be_relative } + it { is_expected.to_not have_parent } end describe 'path/dir_1/' do + subject { described_class.new('path/dir_1/', universe) } + it { is_expected.to have_parent } + describe '#files' do subject { described_class.new('path/dir_1/', universe).files } @@ -45,8 +50,12 @@ describe Gitlab::StringPath do describe '#basename' do subject { described_class.new('path/dir_1/', universe).basename } - it { is_expected.to eq 'dir_1/' } end + + describe '#parent' do + subject { described_class.new('path/dir_1/', universe).parent } + it { is_expected.to eq Gitlab::StringPath.new('path/', universe) } + end end end -- cgit v1.2.1 From c184eeb8489a389bf9f3528f7fe012d1edf132cb Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 21 Dec 2015 09:17:43 +0100 Subject: Improve `StringPath` specs (DRY) --- spec/lib/gitlab/string_path_spec.rb | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) (limited to 'spec') diff --git a/spec/lib/gitlab/string_path_spec.rb b/spec/lib/gitlab/string_path_spec.rb index 86e48f6ee0b..d086a011763 100644 --- a/spec/lib/gitlab/string_path_spec.rb +++ b/spec/lib/gitlab/string_path_spec.rb @@ -6,6 +6,8 @@ describe Gitlab::StringPath do 'path/dir_1/', 'path/dir_1/file_1', 'path/dir_1/file_b', + 'path/dir_1/subdir/', + 'path/dir_1/subdir/subfile', 'path/second_dir', 'path/second_dir/dir_3/file_2', 'path/second_dir/dir_3/file_3', @@ -13,8 +15,12 @@ describe Gitlab::StringPath do '/file/with/absolute_path'] end - describe '/file/with/absolute_path' do - subject { described_class.new('/file/with/absolute_path', universe) } + def path(example) + described_class.new(example.metadata[:path], universe) + end + + describe '/file/with/absolute_path', path: '/file/with/absolute_path' do + subject { |example| path(example) } it { is_expected.to be_absolute } it { is_expected.to_not be_relative } @@ -22,26 +28,27 @@ describe Gitlab::StringPath do it { is_expected.to_not have_parent } describe '#basename' do - subject { described_class.new('/file/with/absolute_path', universe).basename } + subject { |example| path(example).basename } it { is_expected.to eq 'absolute_path' } end end - describe 'path/' do - subject { described_class.new('path/', universe) } + describe 'path/', path: 'path/' do + subject { |example| path(example) } it { is_expected.to be_directory } it { is_expected.to be_relative } it { is_expected.to_not have_parent } end - describe 'path/dir_1/' do - subject { described_class.new('path/dir_1/', universe) } + describe 'path/dir_1/', path: 'path/dir_1/' do + subject { |example| path(example) } + it { is_expected.to have_parent } describe '#files' do - subject { described_class.new('path/dir_1/', universe).files } + subject { |example| path(example).files } pending { is_expected.to all(be_an_instance_of described_class) } pending { is_expected.to be eq [Gitlab::StringPath.new('path/dir_1/file_1', universe), @@ -49,12 +56,14 @@ describe Gitlab::StringPath do end describe '#basename' do - subject { described_class.new('path/dir_1/', universe).basename } + subject { |example| path(example).basename } + it { is_expected.to eq 'dir_1/' } end describe '#parent' do - subject { described_class.new('path/dir_1/', universe).parent } + subject { |example| path(example).parent } + it { is_expected.to eq Gitlab::StringPath.new('path/', universe) } end end -- cgit v1.2.1 From d382335dcd9285c9355ed04dc12c5314bca3c024 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 21 Dec 2015 10:11:15 +0100 Subject: Add implementation of remaining methods in `StringPath` --- spec/lib/gitlab/string_path_spec.rb | 53 ++++++++++++++++++++++++++++++------- 1 file changed, 43 insertions(+), 10 deletions(-) (limited to 'spec') diff --git a/spec/lib/gitlab/string_path_spec.rb b/spec/lib/gitlab/string_path_spec.rb index d086a011763..7818e340726 100644 --- a/spec/lib/gitlab/string_path_spec.rb +++ b/spec/lib/gitlab/string_path_spec.rb @@ -16,7 +16,11 @@ describe Gitlab::StringPath do end def path(example) - described_class.new(example.metadata[:path], universe) + string_path(example.metadata[:path]) + end + + def string_path(string_path) + described_class.new(string_path, universe) end describe '/file/with/absolute_path', path: '/file/with/absolute_path' do @@ -47,14 +51,6 @@ describe Gitlab::StringPath do it { is_expected.to have_parent } - describe '#files' do - subject { |example| path(example).files } - - pending { is_expected.to all(be_an_instance_of described_class) } - pending { is_expected.to be eq [Gitlab::StringPath.new('path/dir_1/file_1', universe), - Gitlab::StringPath.new('path/dir_1/file_b', universe)] } - end - describe '#basename' do subject { |example| path(example).basename } @@ -64,7 +60,44 @@ describe Gitlab::StringPath do describe '#parent' do subject { |example| path(example).parent } - it { is_expected.to eq Gitlab::StringPath.new('path/', universe) } + it { is_expected.to eq string_path('path/') } + end + + describe '#descendants' do + subject { |example| path(example).descendants } + + it { is_expected.to be_an_instance_of Array } + it { is_expected.to all(be_an_instance_of described_class) } + it { is_expected.to contain_exactly string_path('path/dir_1/file_1'), + string_path('path/dir_1/file_b'), + string_path('path/dir_1/subdir/'), + string_path('path/dir_1/subdir/subfile') } + end + + describe '#children' do + subject { |example| path(example).children } + + it { is_expected.to all(be_an_instance_of described_class) } + it { is_expected.to contain_exactly string_path('path/dir_1/file_1'), + string_path('path/dir_1/file_b'), + string_path('path/dir_1/subdir/') } + end + + describe '#files' do + subject { |example| path(example).files } + + it { is_expected.to all(be_file) } + it { is_expected.to all(be_an_instance_of described_class) } + it { is_expected.to contain_exactly string_path('path/dir_1/file_1'), + string_path('path/dir_1/file_b') } + end + + describe '#directories' do + subject { |example| path(example).directories } + + it { is_expected.to all(be_directory) } + it { is_expected.to all(be_an_instance_of described_class) } + it { is_expected.to contain_exactly string_path('path/dir_1/subdir/') } end end end -- cgit v1.2.1 From 37b2c5dd5521f25a7195e82538a0ffc528c3ec6d Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 21 Dec 2015 12:08:04 +0100 Subject: Add support for root path for `StringPath` --- spec/lib/gitlab/string_path_spec.rb | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/lib/gitlab/string_path_spec.rb b/spec/lib/gitlab/string_path_spec.rb index 7818e340726..7ee69c7d3cb 100644 --- a/spec/lib/gitlab/string_path_spec.rb +++ b/spec/lib/gitlab/string_path_spec.rb @@ -11,6 +11,7 @@ describe Gitlab::StringPath do 'path/second_dir', 'path/second_dir/dir_3/file_2', 'path/second_dir/dir_3/file_3', + 'another_directory/', 'another_file', '/file/with/absolute_path'] end @@ -30,6 +31,7 @@ describe Gitlab::StringPath do it { is_expected.to_not be_relative } it { is_expected.to be_file } it { is_expected.to_not have_parent } + it { is_expected.to_not have_descendants } describe '#basename' do subject { |example| path(example).basename } @@ -43,7 +45,7 @@ describe Gitlab::StringPath do it { is_expected.to be_directory } it { is_expected.to be_relative } - it { is_expected.to_not have_parent } + it { is_expected.to have_parent } end describe 'path/dir_1/', path: 'path/dir_1/' do @@ -100,4 +102,24 @@ describe Gitlab::StringPath do it { is_expected.to contain_exactly string_path('path/dir_1/subdir/') } end end + + describe './', path: './' do + subject { |example| path(example) } + + it { is_expected.to_not have_parent } + it { is_expected.to have_descendants } + + describe '#descendants' do + subject { |example| path(example).descendants } + + it { expect(subject.count).to eq universe.count - 1 } + it { is_expected.to_not include string_path('./') } + end + + describe '#children' do + subject { |example| path(example).children } + + it { expect(subject.count).to eq 3 } + end + end end -- cgit v1.2.1 From b19e958d86f5363057f006c8dbf9a8e8762618b9 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 22 Dec 2015 11:05:22 +0100 Subject: Add support for parent directories in `StringPath` This support is not completed though, as parent directory that is first in collection returned by `directories!` is not iterable yet. --- spec/lib/gitlab/string_path_spec.rb | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) (limited to 'spec') diff --git a/spec/lib/gitlab/string_path_spec.rb b/spec/lib/gitlab/string_path_spec.rb index 7ee69c7d3cb..c1722977576 100644 --- a/spec/lib/gitlab/string_path_spec.rb +++ b/spec/lib/gitlab/string_path_spec.rb @@ -50,18 +50,20 @@ describe Gitlab::StringPath do describe 'path/dir_1/', path: 'path/dir_1/' do subject { |example| path(example) } - it { is_expected.to have_parent } describe '#basename' do subject { |example| path(example).basename } - it { is_expected.to eq 'dir_1/' } end + describe '#name' do + subject { |example| path(example).name } + it { is_expected.to eq 'dir_1' } + end + describe '#parent' do subject { |example| path(example).parent } - it { is_expected.to eq string_path('path/') } end @@ -101,6 +103,15 @@ describe Gitlab::StringPath do it { is_expected.to all(be_an_instance_of described_class) } it { is_expected.to contain_exactly string_path('path/dir_1/subdir/') } end + + describe '#directories!' do + subject { |example| path(example).directories! } + + it { is_expected.to all(be_directory) } + it { is_expected.to all(be_an_instance_of described_class) } + it { is_expected.to contain_exactly string_path('path/dir_1/subdir/'), + string_path('path/dir_1/../') } + end end describe './', path: './' do @@ -118,7 +129,6 @@ describe Gitlab::StringPath do describe '#children' do subject { |example| path(example).children } - it { expect(subject.count).to eq 3 } end end -- cgit v1.2.1 From 304c39b6dc5878664c0dace4e3af6bdd2fa395f0 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 22 Dec 2015 13:56:54 +0100 Subject: Fix rubocop offenses in `StringPath` specs --- spec/lib/gitlab/string_path_spec.rb | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) (limited to 'spec') diff --git a/spec/lib/gitlab/string_path_spec.rb b/spec/lib/gitlab/string_path_spec.rb index c1722977576..8290aab7701 100644 --- a/spec/lib/gitlab/string_path_spec.rb +++ b/spec/lib/gitlab/string_path_spec.rb @@ -72,19 +72,23 @@ describe Gitlab::StringPath do it { is_expected.to be_an_instance_of Array } it { is_expected.to all(be_an_instance_of described_class) } - it { is_expected.to contain_exactly string_path('path/dir_1/file_1'), - string_path('path/dir_1/file_b'), - string_path('path/dir_1/subdir/'), - string_path('path/dir_1/subdir/subfile') } + it do + is_expected.to contain_exactly string_path('path/dir_1/file_1'), + string_path('path/dir_1/file_b'), + string_path('path/dir_1/subdir/'), + string_path('path/dir_1/subdir/subfile') + end end describe '#children' do subject { |example| path(example).children } it { is_expected.to all(be_an_instance_of described_class) } - it { is_expected.to contain_exactly string_path('path/dir_1/file_1'), - string_path('path/dir_1/file_b'), - string_path('path/dir_1/subdir/') } + it do + is_expected.to contain_exactly string_path('path/dir_1/file_1'), + string_path('path/dir_1/file_b'), + string_path('path/dir_1/subdir/') + end end describe '#files' do @@ -92,8 +96,10 @@ describe Gitlab::StringPath do it { is_expected.to all(be_file) } it { is_expected.to all(be_an_instance_of described_class) } - it { is_expected.to contain_exactly string_path('path/dir_1/file_1'), - string_path('path/dir_1/file_b') } + it do + is_expected.to contain_exactly string_path('path/dir_1/file_1'), + string_path('path/dir_1/file_b') + end end describe '#directories' do @@ -109,8 +115,10 @@ describe Gitlab::StringPath do it { is_expected.to all(be_directory) } it { is_expected.to all(be_an_instance_of described_class) } - it { is_expected.to contain_exactly string_path('path/dir_1/subdir/'), - string_path('path/dir_1/../') } + it do + is_expected.to contain_exactly string_path('path/dir_1/subdir/'), + string_path('path/dir_1/../') + end end end -- cgit v1.2.1 From 87df1df3cd1a38bbb523d365b229e5e03f890788 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 28 Dec 2015 10:54:22 +0100 Subject: Seed db with CI build artifacts using a zip archive --- spec/fixtures/ci_build_artifacts.tar.gz | Bin 34085 -> 0 bytes spec/fixtures/ci_build_artifacts.zip | Bin 0 -> 105373 bytes 2 files changed, 0 insertions(+), 0 deletions(-) delete mode 100644 spec/fixtures/ci_build_artifacts.tar.gz create mode 100644 spec/fixtures/ci_build_artifacts.zip (limited to 'spec') diff --git a/spec/fixtures/ci_build_artifacts.tar.gz b/spec/fixtures/ci_build_artifacts.tar.gz deleted file mode 100644 index a60de1bf95e..00000000000 Binary files a/spec/fixtures/ci_build_artifacts.tar.gz and /dev/null differ diff --git a/spec/fixtures/ci_build_artifacts.zip b/spec/fixtures/ci_build_artifacts.zip new file mode 100644 index 00000000000..ec47005ce85 Binary files /dev/null and b/spec/fixtures/ci_build_artifacts.zip differ -- cgit v1.2.1 From 9e0e9342a47022a9caaa4a5596ec3ddb91fddc58 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 28 Dec 2015 11:21:01 +0100 Subject: Rename method that returns url to CI build artifacts download --- spec/models/build_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec') diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index 1c22e3cb7c4..85cba9c3e57 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -368,8 +368,8 @@ describe Ci::Build, models: true do end end - describe :download_url do - subject { build.download_url } + describe :artifacts_download_url do + subject { build.artifacts_download_url } it "should be nil if artifact doesn't exist" do build.update_attributes(artifacts_file: nil) -- cgit v1.2.1 From 8eeed761a9c25ea8ccfc347fbd3f5894b5957d9e Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 28 Dec 2015 11:43:15 +0100 Subject: Update specs for CI Build, add `artifacts?` method `artifacts?` method checks if artifacts archive is available. --- spec/models/build_spec.rb | 60 +++++++++++++++++++++++++++-------------------- 1 file changed, 34 insertions(+), 26 deletions(-) (limited to 'spec') diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index 85cba9c3e57..33e0eb7d5d7 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -1,28 +1,3 @@ -# == Schema Information -# -# Table name: builds -# -# id :integer not null, primary key -# project_id :integer -# status :string(255) -# finished_at :datetime -# trace :text -# created_at :datetime -# updated_at :datetime -# started_at :datetime -# runner_id :integer -# commit_id :integer -# coverage :float -# commands :text -# job_id :integer -# name :string(255) -# deploy :boolean default(FALSE) -# options :text -# allow_failure :boolean default(FALSE), not null -# stage :string(255) -# trigger_request_id :integer -# - require 'spec_helper' describe Ci::Build, models: true do @@ -376,13 +351,46 @@ describe Ci::Build, models: true do is_expected.to be_nil end - it 'should be nil if artifact exist' do + it 'should not be nil if artifact exist' do gif = fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif') build.update_attributes(artifacts_file: gif) is_expected.to_not be_nil end end + describe :artifacts_browse_url do + subject { build.artifacts_browse_url } + + it "should be nil if artifact doesn't exist" do + build.update_attributes(artifacts_file: nil) + is_expected.to be_nil + end + + it 'should not be nil if artifact exist' do + gif = fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif') + build.update_attributes(artifacts_file: gif) + is_expected.to_not be_nil + end + end + + describe :artifacts? do + subject { build.artifacts? } + + context 'artifacts archive does not exist' do + before { build.update_attributes(artifacts_file: nil) } + it { is_expected.to be_falsy } + end + + context 'artifacts archive exists' do + before do + gif = fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif') + build.update_attributes(artifacts_file: gif) + end + + it { is_expected.to be_truthy } + end + end + describe :repo_url do let(:build) { FactoryGirl.create :ci_build } let(:project) { build.project } -- cgit v1.2.1 From 5ff7ec42dc8759717c485478261128d61ea70b9a Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 28 Dec 2015 12:06:27 +0100 Subject: Add method that checks if artifacts browser is supported This is needed because of backward compatibility. Previously artifacts archive had `.tar.gz` format, but artifacts browser requires ZIP format now. --- spec/models/build_spec.rb | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) (limited to 'spec') diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index 33e0eb7d5d7..108d7d5ff01 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -391,6 +391,29 @@ describe Ci::Build, models: true do end end + + describe :artifacts_browser_supported? do + subject { build.artifacts_browser_supported? } + before do + file = fixture_file_upload(archive_file, archive_type) + build.update_attributes(artifacts_file: file) + end + + context 'artifacts archive is not a zip file' do + let(:archive_file) { Rails.root + 'spec/fixtures/banana_sample.gif' } + let(:archive_type) { 'image/gif' } + + it { is_expected.to be_falsy } + end + + context 'artifacts archive is a zip file' do + let(:archive_file) { Rails.root + 'spec/fixtures/ci_build_artifacts.zip' } + let(:archive_type) { 'application/zip' } + + it { is_expected.to be_truthy } + end + end + describe :repo_url do let(:build) { FactoryGirl.create :ci_build } let(:project) { build.project } -- cgit v1.2.1 From ece114e630f11a7d244ab55a0ffd49a1d2cbcb94 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 28 Dec 2015 12:34:47 +0100 Subject: Update artifacts download specs --- spec/features/builds_spec.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'spec') diff --git a/spec/features/builds_spec.rb b/spec/features/builds_spec.rb index 240e56839df..d37bd103714 100644 --- a/spec/features/builds_spec.rb +++ b/spec/features/builds_spec.rb @@ -80,7 +80,11 @@ describe "Builds" do visit namespace_project_build_path(@project.namespace, @project, @build) end - it { expect(page).to have_content 'Download artifacts' } + it 'has button to download artifacts' do + page.within('.artifacts') do + expect(page).to have_content 'Download' + end + end end end @@ -111,7 +115,7 @@ describe "Builds" do before do @build.update_attributes(artifacts_file: artifacts_file) visit namespace_project_build_path(@project.namespace, @project, @build) - click_link 'Download artifacts' + page.within('.artifacts') { click_link 'Download' } end it { expect(page.response_headers['Content-Type']).to eq(artifacts_file.content_type) } -- cgit v1.2.1 From 662f4b9e1dec8e461c4ea8da3ccc46a259d9d205 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 30 Dec 2015 14:41:44 +0100 Subject: Add artifacts metadata uploader filed Artifacts metadata field will be used to store a filename of gzipped file containing metadata definition for given artifacts archive. --- spec/fixtures/ci_build_artifacts_metadata.gz | Bin 0 -> 279 bytes 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 spec/fixtures/ci_build_artifacts_metadata.gz (limited to 'spec') diff --git a/spec/fixtures/ci_build_artifacts_metadata.gz b/spec/fixtures/ci_build_artifacts_metadata.gz new file mode 100644 index 00000000000..bd0c8ada20a Binary files /dev/null and b/spec/fixtures/ci_build_artifacts_metadata.gz differ -- cgit v1.2.1 From e3ef0ac8f44118465cf5831982d2051d0986cda8 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 31 Dec 2015 09:24:59 +0100 Subject: Update artifacts metadata fixture --- spec/fixtures/ci_build_artifacts_metadata.gz | Bin 279 -> 242 bytes 1 file changed, 0 insertions(+), 0 deletions(-) (limited to 'spec') diff --git a/spec/fixtures/ci_build_artifacts_metadata.gz b/spec/fixtures/ci_build_artifacts_metadata.gz index bd0c8ada20a..82d6a79c72f 100644 Binary files a/spec/fixtures/ci_build_artifacts_metadata.gz and b/spec/fixtures/ci_build_artifacts_metadata.gz differ -- cgit v1.2.1 From 3de8a4620a70c886c815576dc0a30a745cbb8ccb Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 31 Dec 2015 12:21:56 +0100 Subject: Parse artifacts metadata stored in JSON format --- spec/lib/gitlab/string_path_spec.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'spec') diff --git a/spec/lib/gitlab/string_path_spec.rb b/spec/lib/gitlab/string_path_spec.rb index 8290aab7701..af46f9754ac 100644 --- a/spec/lib/gitlab/string_path_spec.rb +++ b/spec/lib/gitlab/string_path_spec.rb @@ -140,4 +140,20 @@ describe Gitlab::StringPath do it { expect(subject.count).to eq 3 } end end + + describe '#metadata' do + let(:universe) do + ['path/', 'path/file1', 'path/file2'] + end + + let(:metadata) do + [{ name: '/path/'}, { name: '/path/file1' }, { name: '/path/file2' }] + end + + subject do + described_class.new('path/file1', universe, metadata).metadata[:name] + end + + it { is_expected.to eq '/path/file1' } + end end -- cgit v1.2.1 From df41148662142ce20a77b092665f48dd4dfa7bfb Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Sat, 2 Jan 2016 20:09:21 +0100 Subject: Improve path sanitization in `StringPath` --- spec/lib/gitlab/string_path_spec.rb | 1 - 1 file changed, 1 deletion(-) (limited to 'spec') diff --git a/spec/lib/gitlab/string_path_spec.rb b/spec/lib/gitlab/string_path_spec.rb index af46f9754ac..0ef2155cb9b 100644 --- a/spec/lib/gitlab/string_path_spec.rb +++ b/spec/lib/gitlab/string_path_spec.rb @@ -45,7 +45,6 @@ describe Gitlab::StringPath do it { is_expected.to be_directory } it { is_expected.to be_relative } - it { is_expected.to have_parent } end describe 'path/dir_1/', path: 'path/dir_1/' do -- cgit v1.2.1 From a7f99b67a0bf1160f41ebf4dc92c618eb13a7a10 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 4 Jan 2016 13:08:49 +0100 Subject: Extract artifacts metadata implementation to separate class --- .../lib/gitlab/ci/build/artifacts/metadata_spec.rb | 76 ++++++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb (limited to 'spec') diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb new file mode 100644 index 00000000000..8c648be5f02 --- /dev/null +++ b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb @@ -0,0 +1,76 @@ +require 'spec_helper' + +describe Gitlab::Ci::Build::Artifacts::Metadata do + def metadata(path = '') + described_class.new(metadata_file_path, path) + end + + let(:metadata_file_path) do + Rails.root + 'spec/fixtures/ci_build_artifacts_metadata.gz' + end + + context 'metadata file exists' do + describe '#exists?' do + subject { metadata.exists? } + it { is_expected.to be true } + end + + describe '#match! ./' do + subject { metadata('./').match! } + + it 'matches correct paths' do + expect(subject.first).to contain_exactly 'ci_artifacts.txt', + 'other_artifacts_0.1.2/', + 'rails_sample.jpg' + end + + it 'matches metadata for every path' do + expect(subject.last.count).to eq 3 + end + + it 'return Hashes for each metadata' do + expect(subject.last).to all(be_kind_of(Hash)) + end + end + + describe '#match! other_artifacts_0.1.2' do + subject { metadata('other_artifacts_0.1.2').match! } + + it 'matches correct paths' do + expect(subject.first). + to contain_exactly 'other_artifacts_0.1.2/doc_sample.txt', + 'other_artifacts_0.1.2/another-subdirectory/' + end + end + + describe '#match! other_artifacts_0.1.2/another-subdirectory' do + subject { metadata('other_artifacts_0.1.2/another-subdirectory/').match! } + + it 'matches correct paths' do + expect(subject.first). + to contain_exactly 'other_artifacts_0.1.2/another-subdirectory/empty_directory/', + 'other_artifacts_0.1.2/another-subdirectory/banana_sample.gif' + end + end + + describe '#to_string_path' do + subject { metadata('').to_string_path } + it { is_expected.to be_an_instance_of(Gitlab::StringPath) } + end + end + + context 'metadata file does not exist' do + let(:metadata_file_path) { '' } + + describe '#exists?' do + subject { metadata.exists? } + it { is_expected.to be false } + end + + describe '#match!' do + it 'raises error' do + expect { metadata.match! }.to raise_error(StandardError, /Metadata file not found/) + end + end + end +end -- cgit v1.2.1 From f948c00757ca9529817c7368610b0c0d6734d48f Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 4 Jan 2016 13:40:42 +0100 Subject: Do not depend on universe when checking parent in `StringPath` --- spec/lib/gitlab/string_path_spec.rb | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/lib/gitlab/string_path_spec.rb b/spec/lib/gitlab/string_path_spec.rb index 0ef2155cb9b..a54bf109c80 100644 --- a/spec/lib/gitlab/string_path_spec.rb +++ b/spec/lib/gitlab/string_path_spec.rb @@ -30,7 +30,7 @@ describe Gitlab::StringPath do it { is_expected.to be_absolute } it { is_expected.to_not be_relative } it { is_expected.to be_file } - it { is_expected.to_not have_parent } + it { is_expected.to have_parent } it { is_expected.to_not have_descendants } describe '#basename' do @@ -140,6 +140,21 @@ describe Gitlab::StringPath do end end + describe '#nodes', path: './' do + subject { |example| path(example).nodes } + it { is_expected.to eq 1 } + end + + describe '#nodes', path: './test' do + subject { |example| path(example).nodes } + it { is_expected.to eq 2 } + end + + describe '#nodes', path: './test/' do + subject { |example| path(example).nodes } + it { is_expected.to eq 2 } + end + describe '#metadata' do let(:universe) do ['path/', 'path/file1', 'path/file2'] -- cgit v1.2.1 From a5e1905d28e490fb4734bff0e02a1ecff4c7c029 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 4 Jan 2016 14:00:49 +0100 Subject: Render 404 when artifacts path is invalid --- spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'spec') diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb index 8c648be5f02..62c86a60ac4 100644 --- a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb +++ b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb @@ -38,7 +38,8 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do it 'matches correct paths' do expect(subject.first). - to contain_exactly 'other_artifacts_0.1.2/doc_sample.txt', + to contain_exactly 'other_artifacts_0.1.2/', + 'other_artifacts_0.1.2/doc_sample.txt', 'other_artifacts_0.1.2/another-subdirectory/' end end @@ -48,7 +49,8 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do it 'matches correct paths' do expect(subject.first). - to contain_exactly 'other_artifacts_0.1.2/another-subdirectory/empty_directory/', + to contain_exactly 'other_artifacts_0.1.2/another-subdirectory/', + 'other_artifacts_0.1.2/another-subdirectory/empty_directory/', 'other_artifacts_0.1.2/another-subdirectory/banana_sample.gif' end end -- cgit v1.2.1 From cd3b8bbd2f8e7ad75a453441f83c46aeb1d37353 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 4 Jan 2016 14:18:06 +0100 Subject: Add method that checks if path exists in `StringPath` --- spec/lib/gitlab/string_path_spec.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'spec') diff --git a/spec/lib/gitlab/string_path_spec.rb b/spec/lib/gitlab/string_path_spec.rb index a54bf109c80..861eb951236 100644 --- a/spec/lib/gitlab/string_path_spec.rb +++ b/spec/lib/gitlab/string_path_spec.rb @@ -32,6 +32,7 @@ describe Gitlab::StringPath do it { is_expected.to be_file } it { is_expected.to have_parent } it { is_expected.to_not have_descendants } + it { is_expected.to exist } describe '#basename' do subject { |example| path(example).basename } @@ -170,4 +171,16 @@ describe Gitlab::StringPath do it { is_expected.to eq '/path/file1' } end + + describe '#exists?', path: 'another_file' do + subject { |example| path(example).exists? } + it { is_expected.to be true } + end + + describe '#exists?', path: './non_existent/' do + let(:universe) { ['./something'] } + subject { |example| path(example).exists? } + + it { is_expected.to be false } + end end -- cgit v1.2.1 From 1b1793c2530d7003d8baa5aa1912a4ab258d4a1c Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 4 Jan 2016 15:07:49 +0100 Subject: Show file size in artifacts browser using metadata --- spec/lib/gitlab/string_path_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/lib/gitlab/string_path_spec.rb b/spec/lib/gitlab/string_path_spec.rb index 861eb951236..7f1d111478b 100644 --- a/spec/lib/gitlab/string_path_spec.rb +++ b/spec/lib/gitlab/string_path_spec.rb @@ -162,7 +162,7 @@ describe Gitlab::StringPath do end let(:metadata) do - [{ name: '/path/'}, { name: '/path/file1' }, { name: '/path/file2' }] + [{ name: '/path/' }, { name: '/path/file1' }, { name: '/path/file2' }] end subject do -- cgit v1.2.1 From cfffc9eff2c394259000090fb8c4c116863c9199 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 5 Jan 2016 13:05:44 +0000 Subject: Update build specs for artifacts browser support --- spec/models/build_spec.rb | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) (limited to 'spec') diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index 108d7d5ff01..ca96e827e04 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -394,21 +394,20 @@ describe Ci::Build, models: true do describe :artifacts_browser_supported? do subject { build.artifacts_browser_supported? } - before do - file = fixture_file_upload(archive_file, archive_type) - build.update_attributes(artifacts_file: file) - end - - context 'artifacts archive is not a zip file' do - let(:archive_file) { Rails.root + 'spec/fixtures/banana_sample.gif' } - let(:archive_type) { 'image/gif' } - + context 'artifacts metadata does not exist' do it { is_expected.to be_falsy } end - context 'artifacts archive is a zip file' do - let(:archive_file) { Rails.root + 'spec/fixtures/ci_build_artifacts.zip' } - let(:archive_type) { 'application/zip' } + context 'artifacts archive is a zip file and metadata exists' do + before do + fixture_dir = Rails.root + 'spec/fixtures/' + archive = fixture_file_upload(fixture_dir + 'ci_build_artifacts.zip', + 'application/zip') + metadata = fixture_file_upload(fixture_dir + 'ci_build_artifacts_metadata.gz', + 'application/x-gzip') + build.update_attributes(artifacts_file: archive) + build.update_attributes(artifacts_metadata: metadata) + end it { is_expected.to be_truthy } end -- cgit v1.2.1 From 387b27813d1d496c015f4f174812b4761c32648d Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 8 Jan 2016 12:35:49 +0100 Subject: Change format of artifacts metadata from text to binary 0.0.1 This changes the format of metadata to handle paths, that may contain whitespace characters, new line characters and non-UTF-8 characters. Now those paths along with metadata in JSON format are stored as length-prefixed strings (uint32 prefix). Metadata file has a custom format: 1. First string field is metadata version field (string) 2. Second string field is metadata errors field (JSON strong) 3. All subsequent fields is pair of path (string) and path metadata in JSON format. Path's metadata contains all fields that where possible to extract from ZIP archive like date of modification, CRC, compressed size, uncompressed size and comment. --- spec/fixtures/ci_build_artifacts_metadata.gz | Bin 242 -> 309 bytes spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb | 15 +++++++++++++++ 2 files changed, 15 insertions(+) (limited to 'spec') diff --git a/spec/fixtures/ci_build_artifacts_metadata.gz b/spec/fixtures/ci_build_artifacts_metadata.gz index 82d6a79c72f..c394f83bf87 100644 Binary files a/spec/fixtures/ci_build_artifacts_metadata.gz and b/spec/fixtures/ci_build_artifacts_metadata.gz differ diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb index 62c86a60ac4..0c8a41cfab7 100644 --- a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb +++ b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb @@ -59,6 +59,21 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do subject { metadata('').to_string_path } it { is_expected.to be_an_instance_of(Gitlab::StringPath) } end + + describe '#full_version' do + subject { metadata('').full_version } + it { is_expected.to eq 'GitLab Build Artifacts Metadata 0.0.1' } + end + + describe '#version' do + subject { metadata('').version } + it { is_expected.to eq '0.0.1' } + end + + describe '#errors' do + subject { metadata('').errors } + it { is_expected.to eq({}) } + end end context 'metadata file does not exist' do -- cgit v1.2.1 From 61fb47a43202332fe9ac57847996da929ba42d3f Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Sat, 9 Jan 2016 14:41:43 +0100 Subject: Simplify implementation of build artifacts browser (refactoring) --- .../ci/build/artifacts/metadata/path_spec.rb | 148 ++++++++++++++++ .../lib/gitlab/ci/build/artifacts/metadata_spec.rb | 22 +-- spec/lib/gitlab/string_path_spec.rb | 186 --------------------- 3 files changed, 154 insertions(+), 202 deletions(-) create mode 100644 spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb delete mode 100644 spec/lib/gitlab/string_path_spec.rb (limited to 'spec') diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb new file mode 100644 index 00000000000..148d05b5902 --- /dev/null +++ b/spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb @@ -0,0 +1,148 @@ +require 'spec_helper' + +describe Gitlab::Ci::Build::Artifacts::Metadata::Path do + let(:universe) do + ['path/', + 'path/dir_1/', + 'path/dir_1/file_1', + 'path/dir_1/file_b', + 'path/dir_1/subdir/', + 'path/dir_1/subdir/subfile', + 'path/second_dir', + 'path/second_dir/dir_3/file_2', + 'path/second_dir/dir_3/file_3', + 'another_directory/', + 'another_file', + '/file/with/absolute_path'] + end + + def path(example) + string_path(example.metadata[:path]) + end + + def string_path(string_path) + described_class.new(string_path, universe) + end + + describe '/file/with/absolute_path', path: '/file/with/absolute_path' do + subject { |example| path(example) } + + it { is_expected.to be_file } + it { is_expected.to have_parent } + + describe '#basename' do + subject { |example| path(example).basename } + it { is_expected.to eq 'absolute_path' } + end + end + + describe 'path/dir_1/', path: 'path/dir_1/' do + subject { |example| path(example) } + it { is_expected.to have_parent } + it { is_expected.to be_directory } + + describe '#basename' do + subject { |example| path(example).basename } + it { is_expected.to eq 'dir_1/' } + end + + describe '#name' do + subject { |example| path(example).name } + it { is_expected.to eq 'dir_1' } + end + + describe '#parent' do + subject { |example| path(example).parent } + it { is_expected.to eq string_path('path/') } + end + + describe '#children' do + subject { |example| path(example).children } + + it { is_expected.to all(be_an_instance_of described_class) } + it do + is_expected.to contain_exactly string_path('path/dir_1/file_1'), + string_path('path/dir_1/file_b'), + string_path('path/dir_1/subdir/') + end + end + + describe '#files' do + subject { |example| path(example).files } + + it { is_expected.to all(be_file) } + it { is_expected.to all(be_an_instance_of described_class) } + it do + is_expected.to contain_exactly string_path('path/dir_1/file_1'), + string_path('path/dir_1/file_b') + end + end + + describe '#directories' do + subject { |example| path(example).directories } + + it { is_expected.to all(be_directory) } + it { is_expected.to all(be_an_instance_of described_class) } + it { is_expected.to contain_exactly string_path('path/dir_1/subdir/') } + end + + describe '#directories!' do + subject { |example| path(example).directories! } + + it { is_expected.to all(be_directory) } + it { is_expected.to all(be_an_instance_of described_class) } + it do + is_expected.to contain_exactly string_path('path/dir_1/subdir/'), + string_path('path/') + end + end + end + + describe 'empty path', path: '' do + subject { |example| path(example) } + it { is_expected.to_not have_parent } + + describe '#children' do + subject { |example| path(example).children } + it { expect(subject.count).to eq 3 } + end + end + + describe '#nodes', path: './test' do + subject { |example| path(example).nodes } + it { is_expected.to eq 2 } + end + + describe '#nodes', path: './test/' do + subject { |example| path(example).nodes } + it { is_expected.to eq 2 } + end + + describe '#metadata' do + let(:universe) do + ['path/', 'path/file1', 'path/file2'] + end + + let(:metadata) do + [{ name: '/path/' }, { name: '/path/file1' }, { name: '/path/file2' }] + end + + subject do + described_class.new('path/file1', universe, metadata).metadata[:name] + end + + it { is_expected.to eq '/path/file1' } + end + + describe '#exists?', path: 'another_file' do + subject { |example| path(example).exists? } + it { is_expected.to be true } + end + + describe '#exists?', path: './non_existent/' do + let(:universe) { ['./something'] } + subject { |example| path(example).exists? } + + it { is_expected.to be false } + end +end diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb index 0c8a41cfab7..36c4851126c 100644 --- a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb +++ b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb @@ -10,13 +10,8 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do end context 'metadata file exists' do - describe '#exists?' do - subject { metadata.exists? } - it { is_expected.to be true } - end - - describe '#match! ./' do - subject { metadata('./').match! } + describe '#match! empty string' do + subject { metadata('').match! } it 'matches correct paths' do expect(subject.first).to contain_exactly 'ci_artifacts.txt', @@ -55,9 +50,9 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do end end - describe '#to_string_path' do - subject { metadata('').to_string_path } - it { is_expected.to be_an_instance_of(Gitlab::StringPath) } + describe '#to_path' do + subject { metadata('').to_path } + it { is_expected.to be_an_instance_of(Gitlab::Ci::Build::Artifacts::Metdata::Path) } end describe '#full_version' do @@ -79,14 +74,9 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do context 'metadata file does not exist' do let(:metadata_file_path) { '' } - describe '#exists?' do - subject { metadata.exists? } - it { is_expected.to be false } - end - describe '#match!' do it 'raises error' do - expect { metadata.match! }.to raise_error(StandardError, /Metadata file not found/) + expect { metadata.match! }.to raise_error(Errno::ENOENT) end end end diff --git a/spec/lib/gitlab/string_path_spec.rb b/spec/lib/gitlab/string_path_spec.rb deleted file mode 100644 index 7f1d111478b..00000000000 --- a/spec/lib/gitlab/string_path_spec.rb +++ /dev/null @@ -1,186 +0,0 @@ -require 'spec_helper' - -describe Gitlab::StringPath do - let(:universe) do - ['path/', - 'path/dir_1/', - 'path/dir_1/file_1', - 'path/dir_1/file_b', - 'path/dir_1/subdir/', - 'path/dir_1/subdir/subfile', - 'path/second_dir', - 'path/second_dir/dir_3/file_2', - 'path/second_dir/dir_3/file_3', - 'another_directory/', - 'another_file', - '/file/with/absolute_path'] - end - - def path(example) - string_path(example.metadata[:path]) - end - - def string_path(string_path) - described_class.new(string_path, universe) - end - - describe '/file/with/absolute_path', path: '/file/with/absolute_path' do - subject { |example| path(example) } - - it { is_expected.to be_absolute } - it { is_expected.to_not be_relative } - it { is_expected.to be_file } - it { is_expected.to have_parent } - it { is_expected.to_not have_descendants } - it { is_expected.to exist } - - describe '#basename' do - subject { |example| path(example).basename } - - it { is_expected.to eq 'absolute_path' } - end - end - - describe 'path/', path: 'path/' do - subject { |example| path(example) } - - it { is_expected.to be_directory } - it { is_expected.to be_relative } - end - - describe 'path/dir_1/', path: 'path/dir_1/' do - subject { |example| path(example) } - it { is_expected.to have_parent } - - describe '#basename' do - subject { |example| path(example).basename } - it { is_expected.to eq 'dir_1/' } - end - - describe '#name' do - subject { |example| path(example).name } - it { is_expected.to eq 'dir_1' } - end - - describe '#parent' do - subject { |example| path(example).parent } - it { is_expected.to eq string_path('path/') } - end - - describe '#descendants' do - subject { |example| path(example).descendants } - - it { is_expected.to be_an_instance_of Array } - it { is_expected.to all(be_an_instance_of described_class) } - it do - is_expected.to contain_exactly string_path('path/dir_1/file_1'), - string_path('path/dir_1/file_b'), - string_path('path/dir_1/subdir/'), - string_path('path/dir_1/subdir/subfile') - end - end - - describe '#children' do - subject { |example| path(example).children } - - it { is_expected.to all(be_an_instance_of described_class) } - it do - is_expected.to contain_exactly string_path('path/dir_1/file_1'), - string_path('path/dir_1/file_b'), - string_path('path/dir_1/subdir/') - end - end - - describe '#files' do - subject { |example| path(example).files } - - it { is_expected.to all(be_file) } - it { is_expected.to all(be_an_instance_of described_class) } - it do - is_expected.to contain_exactly string_path('path/dir_1/file_1'), - string_path('path/dir_1/file_b') - end - end - - describe '#directories' do - subject { |example| path(example).directories } - - it { is_expected.to all(be_directory) } - it { is_expected.to all(be_an_instance_of described_class) } - it { is_expected.to contain_exactly string_path('path/dir_1/subdir/') } - end - - describe '#directories!' do - subject { |example| path(example).directories! } - - it { is_expected.to all(be_directory) } - it { is_expected.to all(be_an_instance_of described_class) } - it do - is_expected.to contain_exactly string_path('path/dir_1/subdir/'), - string_path('path/dir_1/../') - end - end - end - - describe './', path: './' do - subject { |example| path(example) } - - it { is_expected.to_not have_parent } - it { is_expected.to have_descendants } - - describe '#descendants' do - subject { |example| path(example).descendants } - - it { expect(subject.count).to eq universe.count - 1 } - it { is_expected.to_not include string_path('./') } - end - - describe '#children' do - subject { |example| path(example).children } - it { expect(subject.count).to eq 3 } - end - end - - describe '#nodes', path: './' do - subject { |example| path(example).nodes } - it { is_expected.to eq 1 } - end - - describe '#nodes', path: './test' do - subject { |example| path(example).nodes } - it { is_expected.to eq 2 } - end - - describe '#nodes', path: './test/' do - subject { |example| path(example).nodes } - it { is_expected.to eq 2 } - end - - describe '#metadata' do - let(:universe) do - ['path/', 'path/file1', 'path/file2'] - end - - let(:metadata) do - [{ name: '/path/' }, { name: '/path/file1' }, { name: '/path/file2' }] - end - - subject do - described_class.new('path/file1', universe, metadata).metadata[:name] - end - - it { is_expected.to eq '/path/file1' } - end - - describe '#exists?', path: 'another_file' do - subject { |example| path(example).exists? } - it { is_expected.to be true } - end - - describe '#exists?', path: './non_existent/' do - let(:universe) { ['./something'] } - subject { |example| path(example).exists? } - - it { is_expected.to be false } - end -end -- cgit v1.2.1 From 09a4a5aff8c53dd5930044ddbb285a95ef177d8a Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 11 Jan 2016 09:57:03 +0100 Subject: Render only valid paths in artifacts metadata In this version we will support only relative paths in artifacts metadata. Support for absolute paths will be introduced later. --- spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb | 8 ++++---- spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) (limited to 'spec') diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb index 148d05b5902..3b254c3ce2f 100644 --- a/spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb +++ b/spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb @@ -108,14 +108,14 @@ describe Gitlab::Ci::Build::Artifacts::Metadata::Path do end end - describe '#nodes', path: './test' do + describe '#nodes', path: 'test' do subject { |example| path(example).nodes } - it { is_expected.to eq 2 } + it { is_expected.to eq 1 } end - describe '#nodes', path: './test/' do + describe '#nodes', path: 'test/' do subject { |example| path(example).nodes } - it { is_expected.to eq 2 } + it { is_expected.to eq 1 } end describe '#metadata' do diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb index 36c4851126c..456314768be 100644 --- a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb +++ b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb @@ -28,8 +28,8 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do end end - describe '#match! other_artifacts_0.1.2' do - subject { metadata('other_artifacts_0.1.2').match! } + describe '#match! other_artifacts_0.1.2/' do + subject { metadata('other_artifacts_0.1.2/').match! } it 'matches correct paths' do expect(subject.first). @@ -39,7 +39,7 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do end end - describe '#match! other_artifacts_0.1.2/another-subdirectory' do + describe '#match! other_artifacts_0.1.2/another-subdirectory/' do subject { metadata('other_artifacts_0.1.2/another-subdirectory/').match! } it 'matches correct paths' do @@ -52,7 +52,7 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do describe '#to_path' do subject { metadata('').to_path } - it { is_expected.to be_an_instance_of(Gitlab::Ci::Build::Artifacts::Metdata::Path) } + it { is_expected.to be_an_instance_of(Gitlab::Ci::Build::Artifacts::Metadata::Path) } end describe '#full_version' do -- cgit v1.2.1 From f80d7a868e83f7cbba2d0c42ed9464552d9c7a0b Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 11 Jan 2016 11:18:21 +0100 Subject: Update build model specs --- spec/models/build_spec.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'spec') diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index ca96e827e04..0e13456723d 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -361,14 +361,13 @@ describe Ci::Build, models: true do describe :artifacts_browse_url do subject { build.artifacts_browse_url } - it "should be nil if artifact doesn't exist" do - build.update_attributes(artifacts_file: nil) + it "should be nil if artifacts browser is unsupported" do + allow(build).to receive(:artifacts_browser_supported?).and_return(false) is_expected.to be_nil end - it 'should not be nil if artifact exist' do - gif = fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif') - build.update_attributes(artifacts_file: gif) + it 'should not be nil if artifacts browser is supported' do + allow(build).to receive(:artifacts_browser_supported?).and_return(true) is_expected.to_not be_nil end end -- cgit v1.2.1 From 2be76355caa579d444c8e3c0d25563eb9778bfb2 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 12 Jan 2016 10:04:26 +0100 Subject: Support only valid UTF-8 paths in build artifacts browser --- spec/fixtures/ci_build_artifacts.zip | Bin 105373 -> 106365 bytes spec/fixtures/ci_build_artifacts_metadata.gz | Bin 309 -> 415 bytes 2 files changed, 0 insertions(+), 0 deletions(-) (limited to 'spec') diff --git a/spec/fixtures/ci_build_artifacts.zip b/spec/fixtures/ci_build_artifacts.zip index ec47005ce85..dae976d918e 100644 Binary files a/spec/fixtures/ci_build_artifacts.zip and b/spec/fixtures/ci_build_artifacts.zip differ diff --git a/spec/fixtures/ci_build_artifacts_metadata.gz b/spec/fixtures/ci_build_artifacts_metadata.gz index c394f83bf87..fe9d4c8c661 100644 Binary files a/spec/fixtures/ci_build_artifacts_metadata.gz and b/spec/fixtures/ci_build_artifacts_metadata.gz differ -- cgit v1.2.1 From cf00a808cc9896093be209dc5d6bfbea93b6226b Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 12 Jan 2016 11:50:36 +0100 Subject: Fix specs for artifacts metadata after changing fixture content --- spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'spec') diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb index 456314768be..5c1a94974e8 100644 --- a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb +++ b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb @@ -16,11 +16,12 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do it 'matches correct paths' do expect(subject.first).to contain_exactly 'ci_artifacts.txt', 'other_artifacts_0.1.2/', - 'rails_sample.jpg' + 'rails_sample.jpg', + 'tests_encoding/' end it 'matches metadata for every path' do - expect(subject.last.count).to eq 3 + expect(subject.last.count).to eq 4 end it 'return Hashes for each metadata' do -- cgit v1.2.1 From e8995f9fd5c12882eafcf3766627f64a3d6f623d Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 12 Jan 2016 14:39:15 +0100 Subject: Modify artifacts upload API endpoint, add artifacts metadata --- spec/requests/ci/api/builds_spec.rb | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) (limited to 'spec') diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb index c27e87c4acc..4eb5f2e6828 100644 --- a/spec/requests/ci/api/builds_spec.rb +++ b/spec/requests/ci/api/builds_spec.rb @@ -210,6 +210,31 @@ describe Ci::API::API do end end + context "should post artifacts metadata" do + let!(:artifacts) { file_upload } + let!(:metadata) { file_upload2 } + + before do + build.run! + + post_data = { + 'file.path' => artifacts.path, + 'file.name' => artifacts.original_filename, + 'metadata.path' => metadata.path, + 'metadata.name' => metadata.original_filename + } + + post post_url, post_data, headers_with_token + end + + it 'stores artifacts and artifacts metadata' do + expect(response.status).to eq(201) + expect(json_response['artifacts_file']['filename']).to eq(artifacts.original_filename) + expect(json_response['artifacts_metadata']['filename']).to eq(metadata.original_filename) + end + end + + context "should fail to post too large artifact" do before do build.run! -- cgit v1.2.1 From 0b946029a1fb429db39fbec0cddccf40f7e2aa08 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 13 Jan 2016 09:56:05 +0100 Subject: Update build artifacts API We do not want to allow runners to upload a metadata file. This needs to be generated by Workhorse only. --- spec/requests/ci/api/builds_spec.rb | 48 +++++++++++++++++++++++++++---------- 1 file changed, 35 insertions(+), 13 deletions(-) (limited to 'spec') diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb index 4eb5f2e6828..f47ffb00e33 100644 --- a/spec/requests/ci/api/builds_spec.rb +++ b/spec/requests/ci/api/builds_spec.rb @@ -210,27 +210,49 @@ describe Ci::API::API do end end - context "should post artifacts metadata" do + context 'should post artifacts file and metadata file' do let!(:artifacts) { file_upload } let!(:metadata) { file_upload2 } + let(:stored_artifacts_file) { build.reload.artifacts_file.file } + let(:stored_metadata_file) { build.reload.artifacts_metadata.file } + before do build.run! + post(post_url, post_data, headers_with_token) + end - post_data = { - 'file.path' => artifacts.path, - 'file.name' => artifacts.original_filename, - 'metadata.path' => metadata.path, - 'metadata.name' => metadata.original_filename - } - - post post_url, post_data, headers_with_token + context 'post data accelerated by workhorse is correct' do + let(:post_data) do + { 'file.path' => artifacts.path, + 'file.name' => artifacts.original_filename, + 'metadata.path' => metadata.path, + 'metadata.name' => metadata.original_filename } + end + + it 'responds with valid status' do + expect(response.status).to eq(201) + end + + it 'stores artifacts and artifacts metadata' do + expect(stored_artifacts_file.original_filename).to eq(artifacts.original_filename) + expect(stored_metadata_file.original_filename).to eq(metadata.original_filename) + end end - it 'stores artifacts and artifacts metadata' do - expect(response.status).to eq(201) - expect(json_response['artifacts_file']['filename']).to eq(artifacts.original_filename) - expect(json_response['artifacts_metadata']['filename']).to eq(metadata.original_filename) + context 'runner sends metadata file' do + let(:post_data) do + { 'file' => artifacts, 'metadata' => metadata } + end + + it 'is expected to respond with forbbiden' do + expect(response.status).to eq(403) + end + + it 'does not store artifacts or metadata' do + expect(stored_artifacts_file).to be_nil + expect(stored_metadata_file).to be_nil + end end end -- cgit v1.2.1 From 154b8ceba4ac2d92a2387ad50d7f2b4ed5b2dd8a Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 13 Jan 2016 14:02:36 +0100 Subject: Refactor build artifacts upload API endpoint --- spec/requests/ci/api/builds_spec.rb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'spec') diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb index f47ffb00e33..648ea0d5f50 100644 --- a/spec/requests/ci/api/builds_spec.rb +++ b/spec/requests/ci/api/builds_spec.rb @@ -240,17 +240,16 @@ describe Ci::API::API do end end - context 'runner sends metadata file' do + context 'no artifacts file in post data' do let(:post_data) do - { 'file' => artifacts, 'metadata' => metadata } + { 'metadata' => metadata } end - it 'is expected to respond with forbbiden' do - expect(response.status).to eq(403) + it 'is expected to respond with bad request' do + expect(response.status).to eq(400) end - it 'does not store artifacts or metadata' do - expect(stored_artifacts_file).to be_nil + it 'does not store metadata' do expect(stored_metadata_file).to be_nil end end -- cgit v1.2.1 From 6b0a43aff36f0bbb9050b3c04155a3ccd9c1a75b Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 13 Jan 2016 21:17:28 +0100 Subject: Improve readability of artifacts browser `Entry` related code --- .../ci/build/artifacts/metadata/entry_spec.rb | 170 +++++++++++++++++++++ .../ci/build/artifacts/metadata/path_spec.rb | 148 ------------------ .../lib/gitlab/ci/build/artifacts/metadata_spec.rb | 6 +- 3 files changed, 173 insertions(+), 151 deletions(-) create mode 100644 spec/lib/gitlab/ci/build/artifacts/metadata/entry_spec.rb delete mode 100644 spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb (limited to 'spec') diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata/entry_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata/entry_spec.rb new file mode 100644 index 00000000000..a728227f87c --- /dev/null +++ b/spec/lib/gitlab/ci/build/artifacts/metadata/entry_spec.rb @@ -0,0 +1,170 @@ +require 'spec_helper' + +describe Gitlab::Ci::Build::Artifacts::Metadata::Entry do + let(:entries) do + ['path/', + 'path/dir_1/', + 'path/dir_1/file_1', + 'path/dir_1/file_b', + 'path/dir_1/subdir/', + 'path/dir_1/subdir/subfile', + 'path/second_dir', + 'path/second_dir/dir_3/file_2', + 'path/second_dir/dir_3/file_3', + 'another_directory/', + 'another_file', + '/file/with/absolute_path'] + end + + def path(example) + string_path(example.metadata[:path]) + end + + def string_path(string_path) + described_class.new(string_path, entries) + end + + describe '/file/with/absolute_path', path: '/file/with/absolute_path' do + subject { |example| path(example) } + + it { is_expected.to be_file } + it { is_expected.to have_parent } + + describe '#basename' do + subject { |example| path(example).basename } + it { is_expected.to eq 'absolute_path' } + end + end + + describe 'path/dir_1/', path: 'path/dir_1/' do + subject { |example| path(example) } + it { is_expected.to have_parent } + it { is_expected.to be_directory } + + describe '#basename' do + subject { |example| path(example).basename } + it { is_expected.to eq 'dir_1/' } + end + + describe '#name' do + subject { |example| path(example).name } + it { is_expected.to eq 'dir_1' } + end + + describe '#parent' do + subject { |example| path(example).parent } + it { is_expected.to eq string_path('path/') } + end + + describe '#children' do + subject { |example| path(example).children } + + it { is_expected.to all(be_an_instance_of described_class) } + it do + is_expected.to contain_exactly string_path('path/dir_1/file_1'), + string_path('path/dir_1/file_b'), + string_path('path/dir_1/subdir/') + end + end + + describe '#files' do + subject { |example| path(example).files } + + it { is_expected.to all(be_file) } + it { is_expected.to all(be_an_instance_of described_class) } + it do + is_expected.to contain_exactly string_path('path/dir_1/file_1'), + string_path('path/dir_1/file_b') + end + end + + describe '#directories' do + context 'without options' do + subject { |example| path(example).directories } + + it { is_expected.to all(be_directory) } + it { is_expected.to all(be_an_instance_of described_class) } + it { is_expected.to contain_exactly string_path('path/dir_1/subdir/') } + end + + context 'with option parent: true' do + subject { |example| path(example).directories(parent: true) } + + it { is_expected.to all(be_directory) } + it { is_expected.to all(be_an_instance_of described_class) } + it do + is_expected.to contain_exactly string_path('path/dir_1/subdir/'), + string_path('path/') + end + end + + describe '#nodes' do + subject { |example| path(example).nodes } + it { is_expected.to eq 2 } + end + + describe '#exists?' do + subject { |example| path(example).exists? } + it { is_expected.to be true } + end + + describe '#empty?' do + subject { |example| path(example).empty? } + it { is_expected.to be false } + end + end + end + + describe 'empty path', path: '' do + subject { |example| path(example) } + it { is_expected.to_not have_parent } + + describe '#children' do + subject { |example| path(example).children } + it { expect(subject.count).to eq 3 } + end + + end + + describe 'path/dir_1/subdir/subfile', path: 'path/dir_1/subdir/subfile' do + describe '#nodes' do + subject { |example| path(example).nodes } + it { is_expected.to eq 4 } + end + end + + describe 'non-existent/', path: 'non-existent/' do + describe '#empty?' do + subject { |example| path(example).empty? } + it { is_expected.to be true } + end + + describe '#exists?' do + subject { |example| path(example).exists? } + it { is_expected.to be false } + end + end + + describe 'another_directory/', path: 'another_directory/' do + describe '#empty?' do + subject { |example| path(example).empty? } + it { is_expected.to be true } + end + end + + describe '#metadata' do + let(:entries) do + ['path/', 'path/file1', 'path/file2'] + end + + let(:metadata) do + [{ name: '/path/' }, { name: '/path/file1' }, { name: '/path/file2' }] + end + + subject do + described_class.new('path/file1', entries, metadata).metadata[:name] + end + + it { is_expected.to eq '/path/file1' } + end +end diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb deleted file mode 100644 index 3b254c3ce2f..00000000000 --- a/spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb +++ /dev/null @@ -1,148 +0,0 @@ -require 'spec_helper' - -describe Gitlab::Ci::Build::Artifacts::Metadata::Path do - let(:universe) do - ['path/', - 'path/dir_1/', - 'path/dir_1/file_1', - 'path/dir_1/file_b', - 'path/dir_1/subdir/', - 'path/dir_1/subdir/subfile', - 'path/second_dir', - 'path/second_dir/dir_3/file_2', - 'path/second_dir/dir_3/file_3', - 'another_directory/', - 'another_file', - '/file/with/absolute_path'] - end - - def path(example) - string_path(example.metadata[:path]) - end - - def string_path(string_path) - described_class.new(string_path, universe) - end - - describe '/file/with/absolute_path', path: '/file/with/absolute_path' do - subject { |example| path(example) } - - it { is_expected.to be_file } - it { is_expected.to have_parent } - - describe '#basename' do - subject { |example| path(example).basename } - it { is_expected.to eq 'absolute_path' } - end - end - - describe 'path/dir_1/', path: 'path/dir_1/' do - subject { |example| path(example) } - it { is_expected.to have_parent } - it { is_expected.to be_directory } - - describe '#basename' do - subject { |example| path(example).basename } - it { is_expected.to eq 'dir_1/' } - end - - describe '#name' do - subject { |example| path(example).name } - it { is_expected.to eq 'dir_1' } - end - - describe '#parent' do - subject { |example| path(example).parent } - it { is_expected.to eq string_path('path/') } - end - - describe '#children' do - subject { |example| path(example).children } - - it { is_expected.to all(be_an_instance_of described_class) } - it do - is_expected.to contain_exactly string_path('path/dir_1/file_1'), - string_path('path/dir_1/file_b'), - string_path('path/dir_1/subdir/') - end - end - - describe '#files' do - subject { |example| path(example).files } - - it { is_expected.to all(be_file) } - it { is_expected.to all(be_an_instance_of described_class) } - it do - is_expected.to contain_exactly string_path('path/dir_1/file_1'), - string_path('path/dir_1/file_b') - end - end - - describe '#directories' do - subject { |example| path(example).directories } - - it { is_expected.to all(be_directory) } - it { is_expected.to all(be_an_instance_of described_class) } - it { is_expected.to contain_exactly string_path('path/dir_1/subdir/') } - end - - describe '#directories!' do - subject { |example| path(example).directories! } - - it { is_expected.to all(be_directory) } - it { is_expected.to all(be_an_instance_of described_class) } - it do - is_expected.to contain_exactly string_path('path/dir_1/subdir/'), - string_path('path/') - end - end - end - - describe 'empty path', path: '' do - subject { |example| path(example) } - it { is_expected.to_not have_parent } - - describe '#children' do - subject { |example| path(example).children } - it { expect(subject.count).to eq 3 } - end - end - - describe '#nodes', path: 'test' do - subject { |example| path(example).nodes } - it { is_expected.to eq 1 } - end - - describe '#nodes', path: 'test/' do - subject { |example| path(example).nodes } - it { is_expected.to eq 1 } - end - - describe '#metadata' do - let(:universe) do - ['path/', 'path/file1', 'path/file2'] - end - - let(:metadata) do - [{ name: '/path/' }, { name: '/path/file1' }, { name: '/path/file2' }] - end - - subject do - described_class.new('path/file1', universe, metadata).metadata[:name] - end - - it { is_expected.to eq '/path/file1' } - end - - describe '#exists?', path: 'another_file' do - subject { |example| path(example).exists? } - it { is_expected.to be true } - end - - describe '#exists?', path: './non_existent/' do - let(:universe) { ['./something'] } - subject { |example| path(example).exists? } - - it { is_expected.to be false } - end -end diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb index 5c1a94974e8..42fbe40c890 100644 --- a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb +++ b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb @@ -51,9 +51,9 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do end end - describe '#to_path' do - subject { metadata('').to_path } - it { is_expected.to be_an_instance_of(Gitlab::Ci::Build::Artifacts::Metadata::Path) } + describe '#to_entry' do + subject { metadata('').to_entry } + it { is_expected.to be_an_instance_of(Gitlab::Ci::Build::Artifacts::Metadata::Entry) } end describe '#full_version' do -- cgit v1.2.1 From ad2b0358e0facd5c65c4141ce54c2e55bab165e6 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 13 Jan 2016 22:31:27 +0100 Subject: Improve readability of artifacts `Metadata` related code --- spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'spec') diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb index 42fbe40c890..8560493f5b5 100644 --- a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb +++ b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb @@ -10,8 +10,8 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do end context 'metadata file exists' do - describe '#match! empty string' do - subject { metadata('').match! } + describe '#find_entries! empty string' do + subject { metadata('').find_entries! } it 'matches correct paths' do expect(subject.first).to contain_exactly 'ci_artifacts.txt', @@ -29,8 +29,8 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do end end - describe '#match! other_artifacts_0.1.2/' do - subject { metadata('other_artifacts_0.1.2/').match! } + describe '#find_entries! other_artifacts_0.1.2/' do + subject { metadata('other_artifacts_0.1.2/').find_entries! } it 'matches correct paths' do expect(subject.first). @@ -40,8 +40,8 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do end end - describe '#match! other_artifacts_0.1.2/another-subdirectory/' do - subject { metadata('other_artifacts_0.1.2/another-subdirectory/').match! } + describe '#find_entries! other_artifacts_0.1.2/another-subdirectory/' do + subject { metadata('other_artifacts_0.1.2/another-subdirectory/').find_entries! } it 'matches correct paths' do expect(subject.first). @@ -75,9 +75,9 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do context 'metadata file does not exist' do let(:metadata_file_path) { '' } - describe '#match!' do + describe '#find_entries!' do it 'raises error' do - expect { metadata.match! }.to raise_error(Errno::ENOENT) + expect { metadata.find_entries! }.to raise_error(Errno::ENOENT) end end end -- cgit v1.2.1 From 0d6e7b9d3d38e60e5a706956a853e7dc940e4574 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 13 Jan 2016 23:24:28 +0100 Subject: Use Hash to store paths and entries metadata in artifacts browser --- .../ci/build/artifacts/metadata/entry_spec.rb | 58 +++++++++++----------- .../lib/gitlab/ci/build/artifacts/metadata_spec.rb | 16 +++--- 2 files changed, 36 insertions(+), 38 deletions(-) (limited to 'spec') diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata/entry_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata/entry_spec.rb index a728227f87c..41257103ead 100644 --- a/spec/lib/gitlab/ci/build/artifacts/metadata/entry_spec.rb +++ b/spec/lib/gitlab/ci/build/artifacts/metadata/entry_spec.rb @@ -2,26 +2,26 @@ require 'spec_helper' describe Gitlab::Ci::Build::Artifacts::Metadata::Entry do let(:entries) do - ['path/', - 'path/dir_1/', - 'path/dir_1/file_1', - 'path/dir_1/file_b', - 'path/dir_1/subdir/', - 'path/dir_1/subdir/subfile', - 'path/second_dir', - 'path/second_dir/dir_3/file_2', - 'path/second_dir/dir_3/file_3', - 'another_directory/', - 'another_file', - '/file/with/absolute_path'] + { 'path/' => {}, + 'path/dir_1/' => {}, + 'path/dir_1/file_1' => {}, + 'path/dir_1/file_b' => {}, + 'path/dir_1/subdir/' => {}, + 'path/dir_1/subdir/subfile' => {}, + 'path/second_dir' => {}, + 'path/second_dir/dir_3/file_2' => {}, + 'path/second_dir/dir_3/file_3'=> {}, + 'another_directory/'=> {}, + 'another_file' => {}, + '/file/with/absolute_path' => {} } end def path(example) - string_path(example.metadata[:path]) + entry(example.metadata[:path]) end - def string_path(string_path) - described_class.new(string_path, entries) + def entry(path) + described_class.new(path, entries) end describe '/file/with/absolute_path', path: '/file/with/absolute_path' do @@ -53,7 +53,7 @@ describe Gitlab::Ci::Build::Artifacts::Metadata::Entry do describe '#parent' do subject { |example| path(example).parent } - it { is_expected.to eq string_path('path/') } + it { is_expected.to eq entry('path/') } end describe '#children' do @@ -61,9 +61,9 @@ describe Gitlab::Ci::Build::Artifacts::Metadata::Entry do it { is_expected.to all(be_an_instance_of described_class) } it do - is_expected.to contain_exactly string_path('path/dir_1/file_1'), - string_path('path/dir_1/file_b'), - string_path('path/dir_1/subdir/') + is_expected.to contain_exactly entry('path/dir_1/file_1'), + entry('path/dir_1/file_b'), + entry('path/dir_1/subdir/') end end @@ -73,8 +73,8 @@ describe Gitlab::Ci::Build::Artifacts::Metadata::Entry do it { is_expected.to all(be_file) } it { is_expected.to all(be_an_instance_of described_class) } it do - is_expected.to contain_exactly string_path('path/dir_1/file_1'), - string_path('path/dir_1/file_b') + is_expected.to contain_exactly entry('path/dir_1/file_1'), + entry('path/dir_1/file_b') end end @@ -84,7 +84,7 @@ describe Gitlab::Ci::Build::Artifacts::Metadata::Entry do it { is_expected.to all(be_directory) } it { is_expected.to all(be_an_instance_of described_class) } - it { is_expected.to contain_exactly string_path('path/dir_1/subdir/') } + it { is_expected.to contain_exactly entry('path/dir_1/subdir/') } end context 'with option parent: true' do @@ -93,8 +93,8 @@ describe Gitlab::Ci::Build::Artifacts::Metadata::Entry do it { is_expected.to all(be_directory) } it { is_expected.to all(be_an_instance_of described_class) } it do - is_expected.to contain_exactly string_path('path/dir_1/subdir/'), - string_path('path/') + is_expected.to contain_exactly entry('path/dir_1/subdir/'), + entry('path/') end end @@ -154,15 +154,13 @@ describe Gitlab::Ci::Build::Artifacts::Metadata::Entry do describe '#metadata' do let(:entries) do - ['path/', 'path/file1', 'path/file2'] - end - - let(:metadata) do - [{ name: '/path/' }, { name: '/path/file1' }, { name: '/path/file2' }] + { 'path/' => { name: '/path/' }, + 'path/file1' => { name: '/path/file1' }, + 'path/file2' => { name: '/path/file2' } } end subject do - described_class.new('path/file1', entries, metadata).metadata[:name] + described_class.new('path/file1', entries).metadata[:name] end it { is_expected.to eq '/path/file1' } diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb index 8560493f5b5..828eedfa7b0 100644 --- a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb +++ b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb @@ -14,18 +14,18 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do subject { metadata('').find_entries! } it 'matches correct paths' do - expect(subject.first).to contain_exactly 'ci_artifacts.txt', - 'other_artifacts_0.1.2/', - 'rails_sample.jpg', - 'tests_encoding/' + expect(subject.keys).to contain_exactly 'ci_artifacts.txt', + 'other_artifacts_0.1.2/', + 'rails_sample.jpg', + 'tests_encoding/' end it 'matches metadata for every path' do - expect(subject.last.count).to eq 4 + expect(subject.keys.count).to eq 4 end it 'return Hashes for each metadata' do - expect(subject.last).to all(be_kind_of(Hash)) + expect(subject.values).to all(be_kind_of(Hash)) end end @@ -33,7 +33,7 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do subject { metadata('other_artifacts_0.1.2/').find_entries! } it 'matches correct paths' do - expect(subject.first). + expect(subject.keys). to contain_exactly 'other_artifacts_0.1.2/', 'other_artifacts_0.1.2/doc_sample.txt', 'other_artifacts_0.1.2/another-subdirectory/' @@ -44,7 +44,7 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do subject { metadata('other_artifacts_0.1.2/another-subdirectory/').find_entries! } it 'matches correct paths' do - expect(subject.first). + expect(subject.keys). to contain_exactly 'other_artifacts_0.1.2/another-subdirectory/', 'other_artifacts_0.1.2/another-subdirectory/empty_directory/', 'other_artifacts_0.1.2/another-subdirectory/banana_sample.gif' -- cgit v1.2.1 From 78f5eb94fb15f7e4cc4208a4871b9533243bec40 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Wed, 6 Jan 2016 21:15:37 -0200 Subject: Import GitHub wiki into GitLab --- .../gitlab/github_import/wiki_formatter_spec.rb | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 spec/lib/gitlab/github_import/wiki_formatter_spec.rb (limited to 'spec') diff --git a/spec/lib/gitlab/github_import/wiki_formatter_spec.rb b/spec/lib/gitlab/github_import/wiki_formatter_spec.rb new file mode 100644 index 00000000000..a4ef7b60ae1 --- /dev/null +++ b/spec/lib/gitlab/github_import/wiki_formatter_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::WikiFormatter, lib: true do + let(:project) do + create(:project, namespace: create(:namespace, path: 'gitlabhq'), + import_url: 'https://xxx@github.com/gitlabhq/gitlabhq.git') + end + + subject(:wiki) { described_class.new(project)} + + describe '#path_with_namespace' do + it 'appends .wiki to project path' do + expect(wiki.path_with_namespace).to eq 'gitlabhq/gitlabhq.wiki' + end + end + + describe '#import_url' do + it 'returns URL of the wiki repository' do + expect(wiki.import_url).to eq 'https://xxx@github.com/gitlabhq/gitlabhq.wiki.git' + end + end +end -- cgit v1.2.1 From a6a5990ee5f504107944c3bba5c18dbdea9f5207 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 12 Jan 2016 02:05:18 -0200 Subject: Add Banzai::Filter::GollumTagsFilter for parsing Gollum's tags in HTML --- spec/lib/banzai/filter/gollum_tags_filter_spec.rb | 93 +++++++++++++++++++++++ spec/models/project_wiki_spec.rb | 7 ++ 2 files changed, 100 insertions(+) create mode 100644 spec/lib/banzai/filter/gollum_tags_filter_spec.rb (limited to 'spec') diff --git a/spec/lib/banzai/filter/gollum_tags_filter_spec.rb b/spec/lib/banzai/filter/gollum_tags_filter_spec.rb new file mode 100644 index 00000000000..530b37526b3 --- /dev/null +++ b/spec/lib/banzai/filter/gollum_tags_filter_spec.rb @@ -0,0 +1,93 @@ +require 'spec_helper' + +describe Banzai::Filter::GollumTagsFilter, lib: true do + include FilterSpecHelper + + let(:project) { create(:project) } + let(:user) { double } + let(:project_wiki) { ProjectWiki.new(project, user) } + + describe 'validation' do + it 'ensure that a :project_wiki key exists in context' do + expect { filter("See [[images/image.jpg]]", {}) }.to raise_error ArgumentError, "Missing context keys for Banzai::Filter::GollumTagsFilter: :project_wiki" + end + end + + context 'linking internal images' do + it 'creates img tag if image exists' do + file = Gollum::File.new(project_wiki.wiki) + expect(file).to receive(:path).and_return('images/image.jpg') + expect(project_wiki).to receive(:find_file).with('images/image.jpg').and_return(file) + + tag = '[[images/image.jpg]]' + doc = filter("See #{tag}", project_wiki: project_wiki) + + expect(doc.at_css('img')['src']).to eq "#{project_wiki.wiki_base_path}/images/image.jpg" + end + + it 'does not creates img tag if image does not exist' do + expect(project_wiki).to receive(:find_file).with('images/image.jpg').and_return(nil) + + tag = '[[images/image.jpg]]' + doc = filter("See #{tag}", project_wiki: project_wiki) + + expect(doc.css('img').size).to eq 0 + end + end + + context 'linking external images' do + it 'creates img tag for valid URL' do + expect(project_wiki).to receive(:find_file).with('http://example.com/image.jpg').and_return(nil) + + tag = '[[http://example.com/image.jpg]]' + doc = filter("See #{tag}", project_wiki: project_wiki) + + expect(doc.at_css('img')['src']).to eq "http://example.com/image.jpg" + end + + it 'does not creates img tag for invalid URL' do + expect(project_wiki).to receive(:find_file).with('http://example.com/image.pdf').and_return(nil) + + tag = '[[http://example.com/image.pdf]]' + doc = filter("See #{tag}", project_wiki: project_wiki) + + expect(doc.css('img').size).to eq 0 + end + end + + context 'linking external resources' do + it "the created link's text will be equal to the resource's text" do + tag = '[[http://example.com]]' + doc = filter("See #{tag}", project_wiki: project_wiki) + + expect(doc.at_css('a').text).to eq 'http://example.com' + expect(doc.at_css('a')['href']).to eq 'http://example.com' + end + + it "the created link's text will be link-text" do + tag = '[[link-text|http://example.com/pdfs/gollum.pdf]]' + doc = filter("See #{tag}", project_wiki: project_wiki) + + expect(doc.at_css('a').text).to eq 'link-text' + expect(doc.at_css('a')['href']).to eq 'http://example.com/pdfs/gollum.pdf' + end + end + + context 'linking internal resources' do + it "the created link's text will be equal to the resource's text" do + tag = '[[wiki-slug]]' + doc = filter("See #{tag}", project_wiki: project_wiki) + + expect(doc.at_css('a').text).to eq 'wiki-slug' + expect(doc.at_css('a')['href']).to eq 'wiki-slug' + end + + it "the created link's text will be link-text" do + tag = '[[link-text|wiki-slug]]' + doc = filter("See #{tag}", project_wiki: project_wiki) + + expect(doc.at_css('a').text).to eq 'link-text' + expect(doc.at_css('a')['href']).to eq 'wiki-slug' + end + end +end diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb index 876b927eaea..a2085df5bcd 100644 --- a/spec/models/project_wiki_spec.rb +++ b/spec/models/project_wiki_spec.rb @@ -36,6 +36,13 @@ describe ProjectWiki, models: true do end end + describe "#wiki_base_path" do + it "returns the wiki base path" do + wiki_base_path = "/#{project.path_with_namespace}/wikis" + expect(subject.wiki_base_path).to eq(wiki_base_path) + end + end + describe "#wiki" do it "contains a Gollum::Wiki instance" do expect(subject.wiki).to be_a Gollum::Wiki -- cgit v1.2.1 From 4872b319c8152837bd36e7fc0a5ad912d1c3ef90 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 12 Jan 2016 02:10:08 -0200 Subject: Use the WikiPipeline when rendering the wiki markdown content --- spec/helpers/gitlab_markdown_helper_spec.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'spec') diff --git a/spec/helpers/gitlab_markdown_helper_spec.rb b/spec/helpers/gitlab_markdown_helper_spec.rb index 762ec25c4f5..9a05b21335c 100644 --- a/spec/helpers/gitlab_markdown_helper_spec.rb +++ b/spec/helpers/gitlab_markdown_helper_spec.rb @@ -121,12 +121,13 @@ describe GitlabMarkdownHelper do before do @wiki = double('WikiPage') allow(@wiki).to receive(:content).and_return('wiki content') + helper.instance_variable_set(:@project_wiki, @wiki) end - it "should use GitLab Flavored Markdown for markdown files" do + it "should use Wiki pipeline for markdown files" do allow(@wiki).to receive(:format).and_return(:markdown) - expect(helper).to receive(:markdown).with('wiki content') + expect(helper).to receive(:markdown).with('wiki content', pipeline: :wiki, project_wiki: @wiki) helper.render_wiki_content(@wiki) end -- cgit v1.2.1 From 89e8b82b638e440bc487c4135cdfa4402fdffde5 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 12 Jan 2016 14:50:25 -0200 Subject: Make sure the .git is at the end on Gitlab::GithubImport::WikiFormatter --- spec/lib/gitlab/github_import/wiki_formatter_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec') diff --git a/spec/lib/gitlab/github_import/wiki_formatter_spec.rb b/spec/lib/gitlab/github_import/wiki_formatter_spec.rb index a4ef7b60ae1..aed2aa39e3a 100644 --- a/spec/lib/gitlab/github_import/wiki_formatter_spec.rb +++ b/spec/lib/gitlab/github_import/wiki_formatter_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Gitlab::GithubImport::WikiFormatter, lib: true do let(:project) do create(:project, namespace: create(:namespace, path: 'gitlabhq'), - import_url: 'https://xxx@github.com/gitlabhq/gitlabhq.git') + import_url: 'https://xxx@github.com/gitlabhq/sample.gitlabhq.git') end subject(:wiki) { described_class.new(project)} @@ -16,7 +16,7 @@ describe Gitlab::GithubImport::WikiFormatter, lib: true do describe '#import_url' do it 'returns URL of the wiki repository' do - expect(wiki.import_url).to eq 'https://xxx@github.com/gitlabhq/gitlabhq.wiki.git' + expect(wiki.import_url).to eq 'https://xxx@github.com/gitlabhq/sample.gitlabhq.wiki.git' end end end -- cgit v1.2.1 From 765a2c73271cf311311c391e7e64f83e141c79ae Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 12 Jan 2016 18:59:49 -0200 Subject: Refactoring Banzai::Filter::GollumTagsFilter --- spec/lib/banzai/filter/gollum_tags_filter_spec.rb | 4 ---- 1 file changed, 4 deletions(-) (limited to 'spec') diff --git a/spec/lib/banzai/filter/gollum_tags_filter_spec.rb b/spec/lib/banzai/filter/gollum_tags_filter_spec.rb index 530b37526b3..38baa819957 100644 --- a/spec/lib/banzai/filter/gollum_tags_filter_spec.rb +++ b/spec/lib/banzai/filter/gollum_tags_filter_spec.rb @@ -37,8 +37,6 @@ describe Banzai::Filter::GollumTagsFilter, lib: true do context 'linking external images' do it 'creates img tag for valid URL' do - expect(project_wiki).to receive(:find_file).with('http://example.com/image.jpg').and_return(nil) - tag = '[[http://example.com/image.jpg]]' doc = filter("See #{tag}", project_wiki: project_wiki) @@ -46,8 +44,6 @@ describe Banzai::Filter::GollumTagsFilter, lib: true do end it 'does not creates img tag for invalid URL' do - expect(project_wiki).to receive(:find_file).with('http://example.com/image.pdf').and_return(nil) - tag = '[[http://example.com/image.pdf]]' doc = filter("See #{tag}", project_wiki: project_wiki) -- cgit v1.2.1 From a8c836c371cb253d5e611a1080cd54f9cf4698e9 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 12 Jan 2016 19:00:35 -0200 Subject: Add tests for the wiki pipeline --- spec/features/markdown_spec.rb | 63 +++++++++++++++++++++++++++--- spec/fixtures/markdown.md.erb | 9 +++++ spec/support/markdown_feature.rb | 4 ++ spec/support/matchers/markdown_matchers.rb | 18 +++++++++ 4 files changed, 89 insertions(+), 5 deletions(-) (limited to 'spec') diff --git a/spec/features/markdown_spec.rb b/spec/features/markdown_spec.rb index e836d81c40b..12fd8d37210 100644 --- a/spec/features/markdown_spec.rb +++ b/spec/features/markdown_spec.rb @@ -175,13 +175,15 @@ describe 'GitLab Markdown', feature: true do end end - context 'default pipeline' do - before(:all) do - @feat = MarkdownFeature.new + before(:all) do + @feat = MarkdownFeature.new - # `markdown` helper expects a `@project` variable - @project = @feat.project + # `markdown` helper expects a `@project` variable + @project = @feat.project + end + context 'default pipeline' do + before(:all) do @html = markdown(@feat.raw_markdown) end @@ -221,6 +223,57 @@ describe 'GitLab Markdown', feature: true do end end + context 'wiki pipeline' do + before do + @project_wiki = @feat.project_wiki + + file = Gollum::File.new(@project_wiki.wiki) + expect(file).to receive(:path).and_return('images/example.jpg') + expect(@project_wiki).to receive(:find_file).with('images/example.jpg').and_return(file) + + @html = markdown(@feat.raw_markdown, { pipeline: :wiki, project_wiki: @project_wiki }) + end + + it_behaves_like 'all pipelines' + + it 'includes RelativeLinkFilter' do + expect(doc).not_to parse_relative_links + end + + it 'includes EmojiFilter' do + expect(doc).to parse_emoji + end + + it 'includes TableOfContentsFilter' do + expect(doc).to create_header_links + end + + it 'includes AutolinkFilter' do + expect(doc).to create_autolinks + end + + it 'includes all reference filters' do + aggregate_failures do + expect(doc).to reference_users + expect(doc).to reference_issues + expect(doc).to reference_merge_requests + expect(doc).to reference_snippets + expect(doc).to reference_commit_ranges + expect(doc).to reference_commits + expect(doc).to reference_labels + expect(doc).to reference_milestones + end + end + + it 'includes TaskListFilter' do + expect(doc).to parse_task_lists + end + + it 'includes GollumTagsFilter' do + expect(doc).to parse_gollum_tags + end + end + # Fake a `current_user` helper def current_user @feat.user diff --git a/spec/fixtures/markdown.md.erb b/spec/fixtures/markdown.md.erb index 0620096d689..fe6d42acee2 100644 --- a/spec/fixtures/markdown.md.erb +++ b/spec/fixtures/markdown.md.erb @@ -230,3 +230,12 @@ References should be parseable even inside _<%= merge_request.to_reference %>_ e - [ ] Incomplete sub-task 2 - [x] Complete sub-task 1 - [X] Complete task 2 + +#### Gollum Tags + +- [[linked-resource]] +- [[link-text|linked-resource]] +- [[http://example.com]] +- [[link-text|http://example.com/pdfs/gollum.pdf]] +- [[images/example.jpg]] +- [[http://example.com/images/example.jpg]] diff --git a/spec/support/markdown_feature.rb b/spec/support/markdown_feature.rb index 5d97fdd4882..73c6792b65f 100644 --- a/spec/support/markdown_feature.rb +++ b/spec/support/markdown_feature.rb @@ -28,6 +28,10 @@ class MarkdownFeature end end + def project_wiki + @project_wiki ||= ProjectWiki.new(project, user) + end + def issue @issue ||= create(:issue, project: project) end diff --git a/spec/support/matchers/markdown_matchers.rb b/spec/support/matchers/markdown_matchers.rb index b251e7f8f23..1d5c9f6cca4 100644 --- a/spec/support/matchers/markdown_matchers.rb +++ b/spec/support/matchers/markdown_matchers.rb @@ -66,6 +66,24 @@ module MarkdownMatchers end end + # GollumTagsFilter + matcher :parse_gollum_tags do + def have_image(src) + have_css("img[src*='#{src}']") + end + + set_default_markdown_messages + + match do |actual| + expect(actual).to have_link('linked-resource', href: 'linked-resource') + expect(actual).to have_link('link-text', href: 'linked-resource') + expect(actual).to have_link('http://example.com', href: 'http://example.com') + expect(actual).to have_link('link-text', href: 'http://example.com/pdfs/gollum.pdf') + expect(actual).to have_image('/namespace1/gitlabhq/wikis/images/example.jpg') + expect(actual).to have_image('http://example.com/images/example.jpg') + end + end + # UserReferenceFilter matcher :reference_users do set_default_markdown_messages -- cgit v1.2.1 From 2450916dc6d8e022ad03350e42d7df1e1711905a Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Thu, 14 Jan 2016 11:52:52 -0200 Subject: Fix parse_gollum_tags matcher --- spec/support/matchers/markdown_matchers.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec') diff --git a/spec/support/matchers/markdown_matchers.rb b/spec/support/matchers/markdown_matchers.rb index 1d5c9f6cca4..1d52489e804 100644 --- a/spec/support/matchers/markdown_matchers.rb +++ b/spec/support/matchers/markdown_matchers.rb @@ -69,7 +69,7 @@ module MarkdownMatchers # GollumTagsFilter matcher :parse_gollum_tags do def have_image(src) - have_css("img[src*='#{src}']") + have_css("img[src$='#{src}']") end set_default_markdown_messages @@ -79,7 +79,7 @@ module MarkdownMatchers expect(actual).to have_link('link-text', href: 'linked-resource') expect(actual).to have_link('http://example.com', href: 'http://example.com') expect(actual).to have_link('link-text', href: 'http://example.com/pdfs/gollum.pdf') - expect(actual).to have_image('/namespace1/gitlabhq/wikis/images/example.jpg') + expect(actual).to have_image('/gitlabhq/wikis/images/example.jpg') expect(actual).to have_image('http://example.com/images/example.jpg') end end -- cgit v1.2.1 From ac652d82f17d378e485dcef15a8fabdcf9bad76b Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 14 Jan 2016 19:45:43 +0100 Subject: Let the CI runner know about builds that this build depends on This allows us to implement artifacts passing: runner will download artifacts from all prior builds --- spec/models/build_spec.rb | 24 ++++++++++++++++++++++++ spec/requests/ci/api/builds_spec.rb | 12 ++++++++++++ spec/support/gitlab_stubs/gitlab_ci.yml | 8 ++++---- 3 files changed, 40 insertions(+), 4 deletions(-) (limited to 'spec') diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index 0e13456723d..d12b9e65c82 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -426,6 +426,30 @@ describe Ci::Build, models: true do it { is_expected.to include(project.web_url[7..-1]) } end + describe :depends_on_builds do + let!(:build) { FactoryGirl.create :ci_build, commit: commit, name: 'build', stage_idx: 0, stage: 'build' } + let!(:rspec_test) { FactoryGirl.create :ci_build, commit: commit, name: 'rspec', stage_idx: 1, stage: 'test' } + let!(:rubocop_test) { FactoryGirl.create :ci_build, commit: commit, name: 'rubocop', stage_idx: 1, stage: 'test' } + let!(:staging) { FactoryGirl.create :ci_build, commit: commit, name: 'staging', stage_idx: 2, stage: 'deploy' } + + it 'to have no dependents if this is first build' do + expect(build.depends_on_builds).to be_empty + end + + it 'to have one dependent if this is test' do + expect(rspec_test.depends_on_builds.map(&:id)).to contain_exactly(build.id) + end + + it 'to have all builds from build and test stage if this is last' do + expect(staging.depends_on_builds.map(&:id)).to contain_exactly(build.id, rspec_test.id, rubocop_test.id) + end + + it 'to have retried builds instead the original ones' do + retried_rspec = Ci::Build.retry(rspec_test) + expect(staging.depends_on_builds.map(&:id)).to contain_exactly(build.id, retried_rspec.id, rubocop_test.id) + end + end + def create_mr(build, commit, factory: :merge_request, created_at: Time.now) FactoryGirl.create(factory, source_project_id: commit.gl_project_id, diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb index 648ea0d5f50..1c3e27abb9f 100644 --- a/spec/requests/ci/api/builds_spec.rb +++ b/spec/requests/ci/api/builds_spec.rb @@ -101,6 +101,18 @@ describe Ci::API::API do { "key" => "TRIGGER_KEY", "value" => "TRIGGER_VALUE", "public" => false }, ]) end + + it "returns dependent builds" do + commit = FactoryGirl.create(:ci_commit, project: project) + commit.create_builds('master', false, nil, nil) + commit.builds.where(stage: 'test').each(&:success) + + post ci_api("/builds/register"), token: runner.token, info: { platform: :darwin } + + expect(response.status).to eq(201) + expect(json_response["dependencies"]["builds"].count).to eq(2) + expect(json_response["dependencies"]["builds"][0]["name"]).to eq("rspec") + end end describe "PUT /builds/:id" do diff --git a/spec/support/gitlab_stubs/gitlab_ci.yml b/spec/support/gitlab_stubs/gitlab_ci.yml index 3482145404e..a5b256bd3ec 100644 --- a/spec/support/gitlab_stubs/gitlab_ci.yml +++ b/spec/support/gitlab_stubs/gitlab_ci.yml @@ -36,8 +36,8 @@ staging: script: "cap deploy stating" type: deploy tags: - - capistrano - - debian + - ruby + - mysql except: - stable @@ -47,8 +47,8 @@ production: - cap deploy production - cap notify tags: - - capistrano - - debian + - ruby + - mysql only: - master - /^deploy-.*$/ -- cgit v1.2.1 From 1ce766367eb529fe88068be2f34315f87d74a349 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 15 Jan 2016 15:35:33 +0100 Subject: Change dependencies.builds to depends_on_builds --- spec/requests/ci/api/builds_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec') diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb index 1c3e27abb9f..eec927102aa 100644 --- a/spec/requests/ci/api/builds_spec.rb +++ b/spec/requests/ci/api/builds_spec.rb @@ -110,8 +110,8 @@ describe Ci::API::API do post ci_api("/builds/register"), token: runner.token, info: { platform: :darwin } expect(response.status).to eq(201) - expect(json_response["dependencies"]["builds"].count).to eq(2) - expect(json_response["dependencies"]["builds"][0]["name"]).to eq("rspec") + expect(json_response["depends_on_builds"].count).to eq(2) + expect(json_response["depends_on_builds"][0]["name"]).to eq("rspec") end end -- cgit v1.2.1 From d633755350f1549d4643ac527980c9b28aa1287c Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Sat, 16 Jan 2016 16:25:35 -0500 Subject: Use a more sensible message for the AbuseReport uniqueness validation Previously it was "user has already been taken", when really we were saying the user has already been reported. --- spec/models/abuse_report_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/models/abuse_report_spec.rb b/spec/models/abuse_report_spec.rb index f9be8fcbcfe..4799bbaa57c 100644 --- a/spec/models/abuse_report_spec.rb +++ b/spec/models/abuse_report_spec.rb @@ -26,7 +26,7 @@ RSpec.describe AbuseReport, type: :model do it { is_expected.to validate_presence_of(:reporter) } it { is_expected.to validate_presence_of(:user) } it { is_expected.to validate_presence_of(:message) } - it { is_expected.to validate_uniqueness_of(:user_id) } + it { is_expected.to validate_uniqueness_of(:user_id).with_message('has already been reported') } end describe '#remove_user' do -- cgit v1.2.1